-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Prevent e2e-tests from running on non-local clusters #3829
Conversation
I'm wondering whether or not there is a better way to do this. Just checking the context name against a few common strings seems hacky, and annoying if someone is using something different as they would have to edit the Makefile. Maybe the right thing to do would be to explicitly require a |
Makefile
Outdated
--serviceaccount=default:ingress-nginx-e2e || true | ||
|
||
kubectl run --rm -i --tty \ | ||
--attach \ | ||
--restart=Never \ | ||
--generator=run-pod/v1 \ | ||
--context $(KUBE_CONTEXT) \ |
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 think this will not age well - we can easily forget to add --context
later when we decide to create anothe resource.
@alexkursell wile hacky I liked your previous approach better. If we see it creates an extra friction for developers then maybe we can introduce an env variable like SKIP_CONTEXT_CHECK to skip that check but for now let's just whitelist those three contexts. |
01f419f
to
d6eb2f6
Compare
@ElvinEfendi fair enough. I've rolled back to that commit. |
@alexkursell squash the commits, looking good to me. |
d6eb2f6
to
047aeb5
Compare
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aledbf, alexkursell 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 |
Right now, if you run
make e2e-test
with your kubectl context set to some other cluster, it will dump a bunch of stuff into it. With this PRmake e2e-test
will only run if the context is a local one (currently just checking if it's one of minikube, kind, and docker-for-desktop).Just putting up PR now to see how CI reacts.