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.

Sign up for GitHub

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

Jump to bottom

Apply ValidateMixedArguments to all kubeadm commands #376

Closed
fabriziopandini opened this issue Aug 9, 2017 · 3 comments · Fixed by kubernetes/kubernetes#50692
Closed

Apply ValidateMixedArguments to all kubeadm commands #376

fabriziopandini opened this issue Aug 9, 2017 · 3 comments · Fixed by kubernetes/kubernetes#50692
Labels
priority/backlog Higher priority than priority/awaiting-more-evidence.
Milestone
v1.8

Comments

@fabriziopandini
Copy link
Member

PR #43558 introduced a validation check avoiding usage of ---config (to provide a configuration) mixed with other CLI flags for kubeadm init and kubeadm join commands.
The same implementation should be done also for any other kubeadm commands/sub-commands with ---config flag.
NB. I'm already doing this for some kubeadm alpha phases I'm working on; this issue is a reminder to check ValidateMixedArguments in the remaining parts
@luxas luxas added this to the v1.8 milestone Aug 14, 2017
@luxas luxas added kind/enhancement priority/backlog Higher priority than priority/awaiting-more-evidence. labels Aug 14, 2017
@fabriziopandini fabriziopandini mentioned this issue Aug 15, 2017
Merged
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Aug 16, 2017
Merge pull request #50692 from fabriziopandini/kubeadm-phases-small-c…
7b26438 
…leanups

Automatic merge from submit-queue (batch tested with PRs 50692, 50727)

kubeadm: Small cleanups from the phases refactoring

**What this PR does / why we need it**:
Small cleanups on kubeadm phases

**Which issue this PR fixes**: 
fixes pending comments in [#49419](#49419)
fixes [#376](kubernetes/kubeadm#376)

**Special notes for your reviewer**:
cc @luxas
@luxas
Copy link
Member

luxas commented Aug 19, 2017

@fabriziopandini Is this now fixed for all phases?

@fabriziopandini
Copy link
Member Author

Yes!
As of today MixedArguments can happen in following cases; between parenthesis you can find the PR that implemented the MixedArguments check/and or explanation of exception:

  • init (#43558)
  • join (#43558)
  • alpha phase certs (#50691)
  • alpha phase controlplane (#50302)
  • alpha phase etcd (#50302)
  • alpha phase kubeconfig (#50692)
  • alpha phase upload-config(mixed arguments should be allowed by design).

Nb. There is in flight a PR that will introduce MixedArguments condition in alpha phase self-hosting, but the PR will implement MixedArguments check as well.

@luxas
Copy link
Member

luxas commented Sep 7, 2017

This was fixed by @fabriziopandini for most phases, fully fixed now that @andrewrynhard's PR kubernetes/kubernetes#51171 merged 🎉

@luxas luxas closed this as completed Sep 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Milestone
v1.8
Development

Successfully merging a pull request may close this issue.

kubeadm: Small cleanups from the phases refactoring fabriziopandini/kubernetes
2 participants
@fabriziopandini @luxas