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

kubeadm: Demote controlplane passthrough flags to phases alpha #59882

Merged
merged 1 commit into from
Feb 25, 2018
Merged

kubeadm: Demote controlplane passthrough flags to phases alpha #59882

merged 1 commit into from
Feb 25, 2018

Conversation

krisnova
Copy link
Contributor

@krisnova krisnova commented Feb 14, 2018 •
edited
Loading

After a discussion in sig cluster lifecycle we agreed that the passthrough flags should live in phases alpha, and not be 1st class flags. They already exist in the alpha command, so just removing from here.

What this PR does / why we need it:

We introduced some flags as 1st class flags in #58080 and decided as a sig that the flags should only live in the alpha command. This PR removes the flags from the init command so they only exist in the alpha command

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

relates to /pull/58080
fixes kubernetes/kubeadm/issues/676

Special notes for your reviewer:

This is a cosmetic change, and doesn't alter any functionality of the program, only the avenue in which a user access functionality in the program.

Release note:

kubeadm: Demote controlplane passthrough flags to alpha flags

@krisnova
kubeadm: Demote controlplane passthrough flags to phases alpha
c66841d 
After a discussion in sig cluster lifecycle we agreed that the passthrough flags should live in phases alpha, and not be 1st class flags.
Relates to /pull/58080
Closes kubernetes/kubeadm/issues/676
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 14, 2018
@krisnova
Copy link
Contributor Author

/assign @timothysc

@timothysc
Copy link
Member

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews

@k8s-ci-robot k8s-ci-robot added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label Feb 21, 2018
timothysc
timothysc suggested changes Feb 21, 2018
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't there a flag also added to one of the phases that should also be removed?

timothysc
timothysc approved these changes Feb 23, 2018
View reviewed changes
Copy link
Member

@timothysc timothysc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

I'm going to greenlight this and we can fix as a bug any other removals.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 23, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kris-nova, timothysc

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 23, 2018
@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit fa710e7 into kubernetes:master Feb 25, 2018
@krisnova
Copy link
Contributor Author

@timothysc should we open an issue about whatever flag needs to be removed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timothysc timothysc timothysc approved these changes

@fabriziopandini fabriziopandini Awaiting requested review from fabriziopandini

@luxas luxas Awaiting requested review from luxas

Assignees

@timothysc timothysc

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move passthrough commands to either phases or feature gated flags.
5 participants
@krisnova @timothysc @k8s-ci-robot @k8s-github-robot @fejta-bot