Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

TKR Status Controller: Cluster UpdatesAvailable #2223

Merged
merged 7 commits into from
Apr 29, 2022

Conversation

imikushin
Copy link
Contributor

@imikushin imikushin commented Apr 24, 2022

What this PR does / why we need it

Implement the TKR Status Controller that:

  • labels available TKRs and OSImages based on their status conditions,
  • calculates Cluster status condition UpdatesAvailable for individual Clusters.

UpdatesAvailable condition will only show the latest upgradable versions matching the cluster current Kubernetes version prefixes: major.minor and major.minor+1.

Also, make sure we return the TKR version (instead of the Kubernetes version) as available update if the update TKR has the same Kubernetes version.

TKR version can now be used directly in Cluster spec.topology.version field. The resolver webhook will use the TKR as though it was already resolved (if it satisfies the TKR and OSImage selectors, of course) and will set spec.topology.version to the Kubernetes version of the TKR.

Make sure resolved TKR is the same for both CP and MDs:

  • Intersect resolved TKRs for the controlPlane and machineDeployments.
  • Also, do not use status conditions for OSImage label selector calculation, because these conditions are only applicable to TKR.

Which issue(s) this PR fixes

Fixes #1925

Describe testing done for PR

Unit testing with "generative" tests and hundreds of test runs.

Release note

Add Cluster UpdatesAvailable status condition which lists version strings that can be used to update
the Cluster. These version strings can be used directly in Cluster `spec.topology.version` field.

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

Intersect resolved TKRs for the controlPlane and
machineDeployments.

Also, do not use status conditions for OSImage label selector
calculation, because these conditions are only applicable to TKR.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@imikushin imikushin requested review from a team as code owners April 24, 2022 21:51
Implement the TKR Status Controller that:
- labels available TKRs and OSImages based on their status conditions,
- calculates Cluster status condition UpdatesAvailable for individual
Clusters.

Also, make sure we return the TKR version as available
update if the update TKR has the same K8s version.

TKR version can be used directly in Cluster.spec.topology.version
field. The resolver webhook will use the TKR as though it was already
resolved (if it satisfies the TKR and OSImage selectors, of course).

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@imikushin imikushin force-pushed the cluster-updates-available branch from 086e426 to 99b6958 Compare April 24, 2022 22:20
@imikushin imikushin added the ok-to-merge PRs should be labelled with this before merging label Apr 24, 2022
Enqueue clusters updating to the TKR we're being notified about
(vs clusters using the TKR):
If case we have a certain Kubernetes version (or the TKR
version directly) in the UpdatesAvailable message for a cluster, and
the TKR is deactivated, we want to re-calculate the UpdatesAvailable
message for the cluster.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
Copy link
Contributor

@tenczar tenczar left a comment

Choose a reason for hiding this comment

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

LGTM with one question

pkg/v2/tkr/controller/status/Makefile Outdated Show resolved Hide resolved
pkg/v2/tkr/util/topology/clusterclass.go Show resolved Hide resolved
Ivan Mikushin added 2 commits April 26, 2022 17:00
Catch only label changes for Cluster objects.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
Copy link
Contributor

@prkalle prkalle left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!
Since UpdatesAvailable condition would only show the latest upgradable version matching the cluster current kubernetes version's (major,minor) and (major,minor+1), It would be nice to mention this in the PR summary

Ivan Mikushin added 2 commits April 28, 2022 18:22
Since BOLT takes the base of the filepath, it would create the
manifest with the name "status" which would confusing.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@imikushin imikushin merged commit 18f2400 into vmware-tanzu:main Apr 29, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cluster UpdatesAvailable condition message: calculate based on the new TKR API
5 participants