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 upgrade same version #62568

Merged
merged 3 commits into from
Apr 18, 2018
Merged

Kubeadm upgrade same version #62568

merged 3 commits into from
Apr 18, 2018

Conversation

liztio
Copy link
Contributor

@liztio liztio commented Apr 14, 2018 •
edited

What this PR does / why we need it:

When kubeadm 1.10 came out, it inadvertently introduced a backwards incompatible config change. Because the kubeadm MasterConfiguration is written by the old version of kubeadm and read by the new one, this incompatibility causes the upgrade to fail.

To mitigate this, I've written a simple transform that operates on a map-based version of the config. This map is mutated to make it compatible with the new structure, then serialised to JSON and deserialised by the usual APIMachinery.

Because of complications with the multiple versions, this PR enforces kubeadm only being used to upgrade to kubernetes of the same minor and major versions.

Which issue(s) this PR fixes (optional, in fixes #(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes kubeadm#744
This PR is an alternate take on #62353. Instead of trying to gate migration on versions, this constrains kubeadm to only upgrade versions from the same major and minor versions.

Special notes for your reviewer:

fixes configuration error when upgrading kubeadm from 1.9 to 1.10+
enforces  kubeadm  upgrading kubernetes from the same major and minor versions as the kubeadm binary.

@liztio
Generated build files
58a9063
@liztio
Adds migrations to the kubeadm upgrade phase config
55f28a6 
This fixes a previous issue with kubeadm where a backwards-incompatible
struct change broke deserialising configs as part of the upgrade.
@liztio
Enforce not using newer kubeadm to upgrade older kubeadm
3c92e35
@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/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. area/kubeadm sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Apr 14, 2018
@liztio
Copy link
Contributor Author

liztio commented Apr 14, 2018

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

@k8s-ci-robot
Copy link
Contributor

@liztio: Reiterating the mentions to trigger a notification:
@kubernetes/sig-cluster-lifecycle-pr-reviews

In response to this:

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

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.

BenTheElder
BenTheElder approved these changes Apr 14, 2018
View reviewed changes
Copy link
Member

@BenTheElder BenTheElder left a comment

Choose a reason for hiding this comment

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

/ok-to-test

@BenTheElder
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 14, 2018
@BenTheElder
Copy link
Member

flake
/retest

@timothysc timothysc mentioned this pull request Apr 14, 2018
Closed
@timothysc
Copy link
Member

/assign @fabriziopandini @timothysc

timothysc
timothysc approved these changes Apr 16, 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.

I'm not happy that we have todo this, but at this stage I don't see any other good options until we revamp configs.

/approve

@fabriziopandini has been deeply involved in this PR and I'll pend on his lgtm.

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2018
@fabriziopandini
Copy link
Member

@timothysc as for discussion in today sig-meeting
/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini, liztio, 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-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 62568, 62220, 62743, 62751, 62753). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit edee03e into kubernetes:master Apr 18, 2018
@timothysc timothysc added cherrypick-candidate cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. labels Apr 18, 2018
@timothysc timothysc added this to the v1.10 milestone Apr 18, 2018
@liztio liztio mentioned this pull request Apr 18, 2018
Merged
k8s-github-robot pushed a commit that referenced this pull request Apr 19, 2018
Merge pull request #62804 from liztio/automated-cherry-pick-of-#62568…
f15083b 
…-upstream-release-1.10

Automatic merge from submit-queue.

Automated cherry pick of #62568: Generated build files

Cherry pick of #62568 on release-1.10.

#62568: Generated build files
@praseodym praseodym mentioned this pull request Apr 23, 2018
Closed
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

@BenTheElder BenTheElder BenTheElder approved these changes

@dmmcquay dmmcquay Awaiting requested review from dmmcquay

@krousey krousey Awaiting requested review from krousey

Assignees

@timothysc timothysc

@fabriziopandini fabriziopandini

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubeadm cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.10
Development

Successfully merging this pull request may close these issues.

upgrade plan from 1.9.6 to 1.10.0 fails with "unable to decode config"
6 participants
@liztio @k8s-ci-robot @BenTheElder @timothysc @fabriziopandini @k8s-github-robot