Update clusters according to the version skew policy #9375
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kubermatic-bot
added
release-note
xrstf
force-pushed
the
new-update-controller
branch
5 times, most recently
from
dcc57f5
to
c4c7ff5
Compare
xrstf
changed the title
xrstf
force-pushed
the
new-update-controller
branch
from
c4c7ff5
to
9b252d1
Compare
kubermatic-bot
added
the
approved
xrstf
changed the title
kubermatic-bot
removed
the
do-not-merge/work-in-progress
xrstf
force-pushed
the
new-update-controller
branch
from
9b252d1
to
fd028bd
Compare
xmudrii
reviewed
Mar 22, 2022
pkg/controller/user-cluster-controller-manager/node-version-controller/doc.go
Outdated
Show resolved
Hide resolved
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kron4eg, xrstf 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 |
xrstf
force-pushed
the
new-update-controller
branch
from
ea409f0
to
f66b792
Compare
xrstf
force-pushed
the
new-update-controller
branch
from
64686a9
to
007595c
Compare
|
/lgtm |
|
LGTM label has been added. Git tree hash: 28f833e6d5b569dc2fe14ec9e6f35f9c5406ce6e
|
|
/hold prioritize other PRs to get merged first |
kubermatic-bot
added
the
do-not-merge/hold
|
/hold cancel |
kubermatic-bot
removed
the
do-not-merge/hold
|
/retest |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do / Why do we need it:
This PR implements a new update controller that implements version-skew-policy-aware updating for userclusters. It can handle the
cluster.spec.versionjumping from 1.20 to 1.23 properly.The old update controller
KKP already had an
updatecontroller, which was responsible for watching Clusters and applying the auto-updates rules to bump the cluster's version accordingly. Likewise it would update MachineDeployments when an auto-node-update rule matches.This controller still exists and does the exact same thing, but was renamed to
auto-update-controller. Note that this controller does not care or know how to gradually roll out a version change. It could very well jump from 1.20 to 1.69 (right now it doesn't, but it could).The new update controller
The new
update-controllerimplements the gradual cluster update. It uses the new version information in the ClusterStatus, the version from the cluster spec and gathers version info from the ReplicaSets in the cluster namespace. Based on these 3 pieces of information it makes decisions whether or not to update and which component to update. These decisions are then persisted in the ClusterStatus, which in turn will lead to reconciliations by other controllers.In contrast the the auto-update controller, the new update controller does not care why
.Spec.Versionwas updated, it just focussed on making it happen.Also importantly, all our reconciling coding (the creators that create the apiserver Deployment, the etcd StatefulSet, etc.) all simply rely on the new ClusterStatus. None of them have anything to do with update logic, they do not care why apiserver vX.Y is going to be deployed and they also do not need to dynamically query the control plane status. That makes the creators faster, easier to understand and easier to test. Creators should always be stateless.
Cluster.Spec.Versionnow considered harmfulSince we now keep track of the effective current versions of the control plane components in the ClusterStatus, pretty much no code should ever use
.Spec.Versionif it needs to know the current cluster version..Spec.Versioncould be far, far in the future and might not even be reachable (because old old nodes or no update path being configured).Instead, use
Cluster.Status.Versions.ControlPlaneto know what is actually running right now.The node-version controller
A new controller in the user-cluster-controller-manager is responsible for effective "caching" node information. It watches all nodes in the user cluster and stores the oldest version in
Cluster.Status.Versions.OldestNodeVersion. This is both useful for speeding up the update controller (as it doesn't need to query the nodes every time a Cluster object changes) and is a nice trigger signal for when old nodes disappear from the cluster.Update Procedure
As per version skew policy, the new controller will always first ensure that the cluster namespace is completely healthy before doing anything. This is required to ensure that when for example etcd is updated (because the apiserver went from 1.21 to 1.22), we wait for etcd to roll out completely before progressing.
The controller then queries the cluster namespace and collects version information from the ReplicaSets and the
OldestNodeVersionfield. It now knows what is going on in the cluster and what is currently intended (ClusterStatus.Versions). It also knows the ultimate goal (.Spec.Version).The current apiserver version is always mirrored in the
Cluster.Status.Versions.ControlPlane, so once a new apiserver comes alive, this field is updated and in turn triggers a whooooole lot of other reconciliations by other controllers. So firstVersions.Apiserveris updated, then we wait, once the apiservers are all rotated we bumpVersions.ControlPlane.Once the apiserver is updated, controller-manager and scheduler are updated. Then we wait again for them to roll out and for the cluster namespace to become healthy again.
Once all 3 control plane components are updated, the controller checks the
OldestNodeVersion. If this version is more than 1 minor releases away from the current cluster version, no further updates will happen, as the nodes need to be rotated/replaced first. This is outside of the responsibility of this controller.Debuggability
Putting the version numbers in the ClusterStatus makes debugging much, much easier. It also allows for emergency interventions, where an KKP admin might choose to manually edit the ClusterStatus. It also made writing and testing the controller pretty straightforward, as no reconciling is involved directly.
To further improve the process, the new controller emits an event every time it changes something. It also makes use of a new ClusterCondition to inform about the current state (e.g. whether or not it's waiting right now and what it is waiting for).
kubectl describeing a cluster would look this this (in this example, I updated a 1.20 cluster to 1.22):(Note that in the example above, the controller would refuse to update to 1.23.x, as there are 1.20 nodes in the cluster).
If one follows the individual steps of this cluster upgrade, it would look like this:
Does this PR close any issues?:
Fixes #5949
Does this PR introduce a user-facing change?: