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

Make Kubernetes version sticky for a cluster instead of auto-upgrading #5798

Merged
merged 1 commit into from
Dec 9, 2019
Merged

Make Kubernetes version sticky for a cluster instead of auto-upgrading #5798

merged 1 commit into from
Dec 9, 2019

Conversation

nanikjava
Copy link
Contributor

@nanikjava nanikjava commented Oct 31, 2019 •
edited
Loading

#2570

The fix should do the following

  • Not specifying the --kubernetes-version flag means just use the currently deployed version.
  • If an update is available inform the user that they may use --kubernetes-version=.
  • When --kubernetes-version is specifically set, upgrade the cluster.

@k8s-ci-robot
Copy link
Contributor

Hi @nanikjava. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 31, 2019
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@tstromberg tstromberg added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2019
@medyagh medyagh changed the title Initial proptotyping fix config --kubernetes-version Oct 31, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2019
@medyagh medyagh changed the title fix config --kubernetes-version wip: fix config --kubernetes-version Oct 31, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 31, 2019
@medyagh medyagh changed the title wip: fix config --kubernetes-version wip: fix --kubernetes-version not being set in config Oct 31, 2019
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 1, 2019
@nanikjava nanikjava force-pushed the f-fix-2570 branch 2 times, most recently from fd4b54a to c176788 Compare November 5, 2019 20:40
@nanikjava nanikjava changed the title wip: fix --kubernetes-version not being set in config Fix --kubernetes-version not being set in config Nov 5, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 5, 2019
@medyagh medyagh changed the title Fix --kubernetes-version not being set in config wip: Fix --kubernetes-version not being set in config Nov 5, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 5, 2019
@nanikjava nanikjava changed the title wip: Fix --kubernetes-version not being set in config Fix --kubernetes-version not being set in config Nov 6, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 6, 2019
@medyagh
Copy link
Member

medyagh commented Nov 7, 2019

@nanikjava uni tests are failing, plz check the travis logs

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nanikjava
To complete the pull request process, please assign ra489
You can assign the PR to them by writing /assign @ra489 in a comment when ready.

The full list of commands accepted by this bot can be found 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

@medyagh
Copy link
Member

medyagh commented Nov 7, 2019

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 7, 2019
@minikube-bot
Copy link
Collaborator

Error: running mkcmp: exit status 1

@medyagh
Copy link
Member

medyagh commented Nov 8, 2019

@minikube-bot OK to test

@codecov-io
Copy link

codecov-io commented Nov 12, 2019 •
edited
Loading

Codecov Report

Merging #5798 into master will increase coverage by 0.21%.
The diff coverage is 78.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5798      +/-   ##
==========================================
+ Coverage   36.09%   36.31%   +0.21%     
==========================================
  Files         110      110              
  Lines        8208     8215       +7     
==========================================
+ Hits         2963     2983      +20     
+ Misses       4854     4835      -19     
- Partials      391      397       +6
Impacted Files Coverage Δ
cmd/minikube/cmd/start.go 22.22% <78.57%> (+2.68%) ⬆️

tstromberg
tstromberg suggested changes Nov 13, 2019
View reviewed changes
Copy link
Contributor

@tstromberg tstromberg left a comment

Choose a reason for hiding this comment

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

I tested this PR and it seems to do the right thing. Just one comment to reduce the integration test load if possible.

test/integration/version_upgrade_test.go Outdated Show resolved Hide resolved
@tstromberg tstromberg changed the title Fix --kubernetes-version not being set in config Let us Nov 13, 2019
@tstromberg tstromberg changed the title Let us Make the Kubernetes version sticky for a cluster rather than auto-upgrading it Nov 13, 2019
@tstromberg tstromberg changed the title Make the Kubernetes version sticky for a cluster rather than auto-upgrading it Make Kubernetes version sticky for a cluster, rather than auto-upgrading it for new releases Nov 13, 2019
@tstromberg tstromberg changed the title Make Kubernetes version sticky for a cluster, rather than auto-upgrading it for new releases Make Kubernetes version sticky for a cluster instead of auto-upgrading Nov 13, 2019
tstromberg
tstromberg suggested changes Nov 14, 2019
View reviewed changes
test/integration/version_upgrade_test.go Outdated Show resolved Hide resolved
test/integration/version_upgrade_test.go Outdated Show resolved Hide resolved
test/integration/version_upgrade_test.go Outdated Show resolved Hide resolved
@nanikjava
Fix --kubernetes-version for upgrading cluster to correct version
f680d1b 
This PR fix the issue of checking kubernetes-version and upgrading it. The fix now
will do the following

* Not specifying the --kubernetes-version flag means just use the currently deployed version.
* If an update is available inform the user that they may use --kubernetes-version=<version>.
* When --kubernetes-version is specifically set, upgrade the cluster.

Also add integration testing to test upgrading and downgrading
@tstromberg tstromberg merged commit a6c749f into kubernetes:master Dec 9, 2019
@robrich robrich mentioned this pull request Dec 19, 2019
Closed
prasadkatti added a commit to prasadkatti/minikube that referenced this pull request Apr 8, 2020
@prasadkatti
start does not auto-upgrade k8s version
1497b94 
`minikube start` does not auto-upgrade k8s version since kubernetes#5798
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tstromberg tstromberg tstromberg requested changes

@afbjorklund afbjorklund Awaiting requested review from afbjorklund

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nanikjava @k8s-ci-robot @minikube-bot @medyagh @codecov-io @tstromberg