Add kubectlPath flag to e2e_node.test#82308
Conversation
|
@zhlhahaha: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Hi @zhlhahaha. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/ok-to-test |
|
/assign @tallclair |
|
/kind failing-test |
test/e2e_node/e2e_node_suite_test.go
Outdated
There was a problem hiding this comment.
Rather than adding the flag here, move the cluster flag to common flags:
kubernetes/test/e2e/framework/test_context.go
Line 311 in 0c9288e
5369759 to
203a5a0
Compare
|
/retest |
1 similar comment
|
/retest |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
test/e2e_node/e2e_node_suite_test.go
Outdated
There was a problem hiding this comment.
I think you misunderstood my previous comment. ClusterFlags are not relevant to the node e2e suite, so they shouldn't be registered here. The flags that are common to both suites (node & cluster) are in RegisterCommonFlags. My suggestion was to move the specific flag you need from RegisterClusterFlags to RegisterCommonFlags
There was a problem hiding this comment.
Thanks, very clear explanation. Updated
e2e_node.test does not set default kubectlPath, which lead to test errors as following: [Fail] [sig-storage] EmptyDir volumes [It] pod should support shared volumes between containers [Conformance] When the test trying to read file in shared volume, it uses "kubeclt exec namespace -c container_name -- cat file_name". However, as variable framework.TestContext.KubectlPath not set, kubectl binary can not be found in the test and the tast fails. This patch move kubectlPath flag from RegisterClusterFlags to RegisterCommonFlags, thus default value for framework.TestContext.KubectlPath will be set,and user can also use --kubectl-path flag to set kubectl path. Signed-off-by: Howard Zhang <howard.zhang@arm.com>
203a5a0 to
1c9da19
Compare
|
/assign @fejta @tallclair |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fejta, zhlhahaha The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/release-note-none |
|
/retest Review the full test history for this PR. Silence the bot with an |
What this PR does / why we need it:
e2e_node.test does not set default kubectlPath, which lead to test
errors as following:
[Fail] [sig-storage] EmptyDir volumes [It] pod should support
shared volumes between containers [Conformance]
When the test trying to read file in shared volume, it uses
"kubeclt exec namespace -c container_name -- cat file_name".
However, as variable framework.TestContext.KubectlPath not set,
kubectl binary can not be found in the test and the tast fails.
This patch set default value for framework.TestContext.KubectlPath,
which is same to e2e.test. User can also use --kubectl-path flag to
set kubectl path.
Which issue(s) this PR fixes:
Fixes #82418
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?:
None
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
None