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

[kubeadm] Print required flags when running kubeadm upgrade plan #65802

Merged
merged 2 commits into from Jul 7, 2018

Conversation

@xlgao-zju
Copy link
Member

xlgao-zju commented Jul 4, 2018

What this PR does / why we need it:
print required flags when running kubeadm upgrade plan

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

Special notes for your reviewer:
/assign @chuckha
/assign @neolit123

Release note:

kubeadm: print required flags when running kubeadm upgrade plan
print required flags when running kubeadm upgrade plan
Signed-off-by: Xianglin Gao <xianglin.gxl@alibaba-inc.com>

@k8s-ci-robot k8s-ci-robot requested review from liztio and stealthybox Jul 4, 2018

@xlgao-zju xlgao-zju changed the title print required flags when running kubeadm upgrade plan [kubeadm] Print required flags when running kubeadm upgrade plan Jul 4, 2018

@neolit123

This comment has been minimized.

Copy link
Member

neolit123 commented Jul 4, 2018

thank you @xlgao-zju
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 4, 2018

@neolit123

This comment has been minimized.

Copy link
Member

neolit123 commented Jul 4, 2018

/kind bug
@kubernetes/sig-cluster-lifecycle-pr-reviews

potential cherry-pick?

@neolit123

This comment has been minimized.

Copy link
Member

neolit123 commented Jul 4, 2018

@chuckha @xlgao-zju
i think we should handle --allow-release-candidate-upgrades as well?

@xlgao-zju

This comment has been minimized.

Copy link
Member Author

xlgao-zju commented Jul 4, 2018

i think we should handle --allow-release-candidate-upgrades as well?

@neolit123 I think we can handle RC by using --allow-experimental-upgrades too?

--allow-experimental-upgrades        Show unstable versions of Kubernetes as an upgrade alternative and allow upgrading to an alpha/beta/release candidate versions of Kubernetes.
--allow-release-candidate-upgrades   Show release candidate versions of Kubernetes as an upgrade alternative and allow upgrading to a release candidate versions of Kubernetes.
@neolit123

This comment has been minimized.

Copy link
Member

neolit123 commented Jul 4, 2018

that's true.

if the user types:

kubeadm upgrade plan --allow-release-candidate-upgrades

to my understanding it would print:

You can now apply the upgrade by executing the following command:

        kubeadm upgrade apply <some-RC-version> --allow-experimental-upgrades

so it's a question if we want to print the exact --allow-..... flag that the user initially entered.
--allow-experimental-upgrades would be valid.

@xlgao-zju

This comment has been minimized.

Copy link
Member Author

xlgao-zju commented Jul 4, 2018

Maybe this is better instead of using the user's initial flag?

When the version is RC:

You can now apply the upgrade by executing the following command:

        kubeadm upgrade apply <some-RC-version> --allow-release-candidate-upgrades

when the version is some other unstable version:

You can now apply the upgrade by executing the following command:

        kubeadm upgrade apply <some-other-unstable-version> --allow-experimental-upgrades
@xlgao-zju

This comment has been minimized.

Copy link
Member Author

xlgao-zju commented Jul 5, 2018

@neolit123 And using the user's initial flag will make it difficult to deal with the test cases....

@chuckha

This comment has been minimized.

Copy link
Member

chuckha commented Jul 5, 2018

@xlgao-zju we should support both flags, I like the idea of printing the correct one based on the suggested version instead of printing the flag the user passed in even though that's the only time we'll see this output.

add test case
Signed-off-by: Xianglin Gao <xianglin.gxl@alibaba-inc.com>

@xlgao-zju xlgao-zju force-pushed the xlgao-zju:improve-output branch from 96ba539 to 0055276 Jul 6, 2018

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jul 6, 2018

@xlgao-zju

This comment has been minimized.

Copy link
Member Author

xlgao-zju commented Jul 6, 2018

@chuckha @neolit123 Updated based on:

the correct one based on the suggested version

PTAL

@neolit123

This comment has been minimized.

Copy link
Member

neolit123 commented Jul 6, 2018

@xlgao-zju
thanks, the change seems good.

waiting on @chuckha for confirmation.

@luxas

luxas approved these changes Jul 6, 2018

Copy link
Member

luxas left a comment

/lgtm

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Jul 6, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: luxas, neolit123, xlgao-zju

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-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented Jul 7, 2018

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

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

k8s-github-robot commented Jul 7, 2018

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 2d288a7 into kubernetes:master Jul 7, 2018

16 of 17 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-kubemark-e2e-gce-big
Details
cla/linuxfoundation xlgao-zju authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@xlgao-zju xlgao-zju deleted the xlgao-zju:improve-output branch Jul 8, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.