-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ensure proper kubectl is used #3328
Conversation
/lgtm |
hack/ci/e2e-k8s.sh
Outdated
@@ -73,6 +73,9 @@ build() { | |||
fi | |||
# make sure we have e2e requirements | |||
make all WHAT="cmd/kubectl test/e2e/e2e.test ${GINKGO_SRC_DIR}" | |||
|
|||
# Ensure the built kubecl is used instead of system | |||
export PATH="${PWD}/bin/kubectl:$PATH" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that works? For one thing Kubernetes doesn't build to bin/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, I was looking at the wrong dir. I think it'll need to be export PATH="${PWD}/_output/bin/kubectl:$PATH"
. This is based on https://github.com/kubernetes/kubernetes/blob/master/build/root/Makefile#L45-L46 and running e2e-k8s.sh locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still isn't the right fix. Looking more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check in the kubernetes/hack folder, there are multiple scripts that looks for the kubectl
binary there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😓 PATH="${PWD}/_output/bin/kubectl:$PATH"
vs PATH="${PWD}/_output/bin:$PATH"
.... Too much time spent on realizing that. Update incoming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forget if we can rely on this always being the case but we can start here. When you build dockerized vs not the k8s build system emits to different subpaths under _output, but they might both be linked to this path? In the past in kubernetes upstream we've had to use a small utility to check multiple locations.
If this is working in CI we can start here though.
20ee665
to
82a4a01
Compare
Signed-off-by: Ricky Sadowski <[email protected]>
not related, this is because kubernetes/kubernetes#119927 This looks correct now
/assign @BenTheElder |
Co-authored-by: Antonio Ojea <[email protected]>
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
w/ comment about why we may need a more elaborate follow-up
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BenTheElder, rjsadow The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Ensure that the kubectl built is the one used throughout the script.
ref: kubernetes/test-infra#30345
/cc @BenTheElder @aojea