Skip to content
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

e2e: strict --provider checking #70200

Closed
pohly opened this issue Oct 24, 2018 · 6 comments · Fixed by #73402
Closed

e2e: strict --provider checking #70200

pohly opened this issue Oct 24, 2018 · 6 comments · Fixed by #73402
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/testing Categorizes an issue or PR as relevant to SIG Testing.

Comments

@pohly
Copy link
Contributor

pohly commented Oct 24, 2018

Is this a BUG REPORT or FEATURE REQUEST?:

/kind cleanup

What happened:

In PR #68483 we tightened the --provider parameter checking of the e2e.test binary such that it only accepted known providers. This turned out to be problematic (#70058) because people had started to depend on the previous behavior, which was to accept anything and merely warn about it.

For the sake of giving people time to adapt, PR #70141 reverted to the older behavior.

What you expected to happen:

In 1.14 we want to allow only known providers. While at it, the error handling for unknown providers should be enhanced (print proper error message instead of calling Failf, which panics).

/assign
/sig testing

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Oct 24, 2018
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 22, 2019
@pohly
Copy link
Contributor Author

pohly commented Jan 23, 2019

@neolit123 kubeadm was one of the tools which got broken by the tightening of the --provider parameter checking. Does kubeadm still rely on the acceptance of unknown provider parameters or can the current warning be turned into an error now?

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 23, 2019
@neolit123
Copy link
Member

@pohly yes the kubernetes-anywhere deployer is still bound to provider=kubernetes-anywhere (unknown) in test infra jobs.
https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes/sig-cluster-lifecycle/kubeadm-x-on-y.yaml#L25

we went back and forth on this topic, but if you wish to replace this support for unknown deployers i can switch those jobs to skeleton.
LMK.

@neolit123
Copy link
Member

given one release has passed with the deprecated message on the CLI, it's probably safe to remove this functionality now. :)

@pohly
Copy link
Contributor Author

pohly commented Jan 23, 2019 via email

@neolit123
Copy link
Member

done:
kubernetes/test-infra#10913

pohly added a commit to pohly/kubernetes that referenced this issue Jan 28, 2019
This finishes the work started for 1.13: instead of merely warning
about an unknown value given to --profile, the test/e2e/e2e.test
binary will now print an error and refuse to run.

Fixes: kubernetes#70200
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants