Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 15 additions & 14 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -938,30 +938,31 @@ test-cover: ## Run unit and integration tests and generate a coverage report
go tool cover -func=out/coverage.out -o out/coverage.txt
go tool cover -html=out/coverage.out -o out/coverage.html

.PHONY: test-docker-infrastructure
test-docker-infrastructure: $(SETUP_ENVTEST) ## Run unit and integration tests with race detector for docker infrastructure provider
.PHONY: test-infrastructure
test-infrastructure: $(SETUP_ENVTEST) ## Run unit and integration tests with race detector for docker infrastructure provider
# Note: Fuzz tests are not executed with race detector because they would just time out.
# To achieve that, all files with fuzz tests have the "!race" build tag, to still run fuzz tests
# we have an additional `go test` run that focuses on "TestFuzzyConversion".
cd $(CAPD_DIR); KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -race ./... $(TEST_ARGS)
$(MAKE) test-docker-infrastructure-conversions TEST_ARGS="$(TEST_ARGS)"
cd test/infrastructure; KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -race ./... $(TEST_ARGS)
$(MAKE) test-infrastructure-conversions TEST_ARGS="$(TEST_ARGS)"

.PHONY: test-docker-infrastructure-conversions
test-docker-infrastructure-conversions: $(SETUP_ENVTEST) ## Run conversions test for docker infrastructure provider
cd $(CAPD_DIR); KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -run "^TestFuzzyConversion$$" ./... $(TEST_ARGS)
.PHONY: test-infrastructure-conversions
test-infrastructure-conversions: $(SETUP_ENVTEST) ## Run conversions test for docker infrastructure provider
cd test/infrastructure; KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -run "^TestFuzzyConversion$$" ./... $(TEST_ARGS)

.PHONY: test-docker-infrastructure-verbose
test-docker-infrastructure-verbose: ## Run unit and integration tests with race detector and with verbose flag for docker infrastructure provider
$(MAKE) test-docker-infrastructure TEST_ARGS="$(TEST_ARGS) -v"
.PHONY: test-infrastructure-verbose
test-infrastructure-verbose: ## Run unit and integration tests with race detector and with verbose flag for docker infrastructure provider
$(MAKE) test-infrastructure TEST_ARGS="$(TEST_ARGS) -v"

.PHONY: test-docker-infrastructure-junit
test-docker-infrastructure-junit: $(SETUP_ENVTEST) $(GOTESTSUM) ## Run unit and integration tests with race detector and generate a junit report for docker infrastructure provider
cd $(CAPD_DIR); set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -race -json ./... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit.infra_docker.exitcode) | tee $(ARTIFACTS)/junit.infra_docker.stdout
.PHONY: test-infrastructure-junit
test-infrastructure-junit: $(SETUP_ENVTEST) $(GOTESTSUM) ## Run unit and integration tests with race detector and generate a junit report for docker infrastructure provider
cd test/infrastructure; set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -race -json ./... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit.infra_docker.exitcode) | tee $(ARTIFACTS)/junit.infra_docker.stdout
$(GOTESTSUM) --junitfile $(ARTIFACTS)/junit.infra_docker.xml --raw-command cat $(ARTIFACTS)/junit.infra_docker.stdout
exit $$(cat $(ARTIFACTS)/junit.infra_docker.exitcode)
cd $(CAPD_DIR); set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -run "^TestFuzzyConversion$$" -json ./... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit-fuzz.infra_docker.exitcode) | tee $(ARTIFACTS)/junit-fuzz.infra_docker.stdout
cd test/infrastructure; set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -run "^TestFuzzyConversion$$" -json ./... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit-fuzz.infra_docker.exitcode) | tee $(ARTIFACTS)/junit-fuzz.infra_docker.stdout
$(GOTESTSUM) --junitfile $(ARTIFACTS)/junit-fuzz.infra_docker.xml --raw-command cat $(ARTIFACTS)/junit-fuzz.infra_docker.stdout
exit $$(cat $(ARTIFACTS)/junit-fuzz.infra_docker.exitcode)

.PHONY: test-test-extension
test-test-extension: $(SETUP_ENVTEST) ## Run unit and integration tests for the test extension
cd $(TEST_EXTENSION_DIR); KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" go test -race ./... $(TEST_ARGS)
Expand Down
2 changes: 2 additions & 0 deletions api/core/v1beta1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ func (src *ClusterClass) ConvertTo(dstRaw conversion.Hub) error {
dst.Status.Variables[i] = variable
}

dst.Spec.KubernetesVersions = restored.Spec.KubernetesVersions

return nil
}

Expand Down
1 change: 1 addition & 0 deletions api/core/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions api/core/v1beta2/clusterclass_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,18 @@ type ClusterClassSpec struct {
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=1000
Patches []ClusterClassPatch `json:"patches,omitempty"`

// kubernetesVersions is the list of Kubernetes versions that can be
// used for clusters using this ClusterClass.
// The list of version must be ordered from the older to the newer version, and there should be
// at least one version for every minor in between the first and the last version.
// +optional
// +listType=atomic
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=100
// +kubebuilder:validation:items:MinLength=1
// +kubebuilder:validation:items:MaxLength=256
KubernetesVersions []string `json:"kubernetesVersions,omitempty"`
}

// InfrastructureClass defines the class for the infrastructure cluster.
Expand Down
4 changes: 4 additions & 0 deletions api/core/v1beta2/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ const (
// to track the name of the MachineDeployment topology it represents.
ClusterTopologyMachineDeploymentNameLabel = "topology.cluster.x-k8s.io/deployment-name"

// ClusterTopologyControlPlaneUpgradeStepAnnotation tracks version current upgrade step.
Copy link
Member

Choose a reason for hiding this comment

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

There's a bit of a mismatch between const name, godoc and annotation key

Should this be: topology.cluster.x-k8s.io/upgrade-step-control-plane-version? (+ const name should then be aligned accordingly

Let's document the annotation in docs/book/src/reference/api/labels-and-annotations.md

Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer to not surface implementations details, and establish a pattern where we should add internal. prefix to annotations that are meant to be used by CAPI only

e.g internal.topology.cluster.x-k8s.io/upgrade would be described as "This is an annotation used by CAPI internally to track upgrade steps. Name, meaning and semantics of annotation can change anytime and it should not be used outside of CAPI controllers.".

Copy link
Member

@sbueringer sbueringer Sep 10, 2025

Choose a reason for hiding this comment

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

I'm fine with adding internal, but maybe it would be still good to have a name that tells us what it exactly contains. Or we could json marshal that context into the value, but that's never fun :/

(also will be more helpful during troubleshooting)

// NOTE: The annotation exists only during upgrades.
ClusterTopologyControlPlaneUpgradeStepAnnotation = "topology.cluster.x-k8s.io/upgrade-step"

// ClusterTopologyHoldUpgradeSequenceAnnotation can be used to hold the entire MachineDeployment upgrade sequence.
// If the annotation is set on a MachineDeployment topology in Cluster.spec.topology.workers, the Kubernetes upgrade
// for this MachineDeployment topology and all subsequent ones is deferred.
Expand Down
5 changes: 5 additions & 0 deletions api/core/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions api/core/v1beta2/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion controlplane/kubeadm/internal/controllers/scale.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,16 @@ func (r *KubeadmControlPlaneReconciler) preflightChecks(ctx context.Context, con

if feature.Gates.Enabled(feature.ClusterTopology) {
// Block when we expect an upgrade to be propagated for topology clusters.
if controlPlane.Cluster.Spec.Topology.IsDefined() && controlPlane.Cluster.Spec.Topology.Version != controlPlane.KCP.Spec.Version {
// NOTE: in case the cluster is performing an upgrade, allow creation of machines for the intermediate step.
isUpgradeForVersion := false
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
isUpgradeForVersion := false
isKCPVersionUpgradeStepVersion := false

I think a different name would be better

if versions, ok := controlPlane.Cluster.GetAnnotations()[clusterv1.ClusterTopologyControlPlaneUpgradeStepAnnotation]; ok {
v := strings.Split(versions, ",")
Copy link
Member

Choose a reason for hiding this comment

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

This makes it look like the annotation contains a list of versions. As far as I can tell it's just a single version

(If this should be a list of versions, let's update the godoc on the annotation to mention that)

Copy link
Member Author

Choose a reason for hiding this comment

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

that's correct, it is a version
I have cleaned up the core here and in MS preflight checks (left over from a previous iteration)

if len(v) > 0 {
isUpgradeForVersion = strings.TrimSpace(v[0]) == controlPlane.KCP.Spec.Version
}
}

if controlPlane.Cluster.Spec.Topology.IsDefined() && controlPlane.Cluster.Spec.Topology.Version != controlPlane.KCP.Spec.Version && !isUpgradeForVersion {
logger.Info(fmt.Sprintf("Waiting for a version upgrade to %s to be propagated from Cluster.spec.topology", controlPlane.Cluster.Spec.Topology.Version))
Copy link
Member

Choose a reason for hiding this comment

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

I think we should adjust the message here and in getPreflightMessages if we are waiting for the version from the annotation to be propagated to the CP

(Ideally just set the desired version ]here on PreflightCheckResuls somehow and not recompute in getPreflightMessages)

controlPlane.PreflightCheckResults.TopologyVersionMismatch = true
return ctrl.Result{RequeueAfter: preflightFailedRequeueAfter}, nil
Expand Down
80 changes: 80 additions & 0 deletions controlplane/kubeadm/internal/controllers/scale_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,37 @@ func TestPreflightChecks(t *testing.T) {
TopologyVersionMismatch: true,
},
},
{
name: "control plane with a pending upgrade, but not yet at the current step of the upgrade plan, should requeue",
cluster: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
clusterv1.ClusterTopologyControlPlaneUpgradeStepAnnotation: "v1.32.0",
},
},
Spec: clusterv1.ClusterSpec{
Topology: clusterv1.Topology{
Version: "v1.33.0",
},
},
},
kcp: &controlplanev1.KubeadmControlPlane{
Spec: controlplanev1.KubeadmControlPlaneSpec{
Version: "v1.31.0",
},
},
machines: []*clusterv1.Machine{
{},
},

expectResult: ctrl.Result{RequeueAfter: preflightFailedRequeueAfter},
expectPreflight: internal.PreflightCheckResults{
HasDeletingMachine: false,
ControlPlaneComponentsNotHealthy: false,
EtcdClusterNotHealthy: false,
TopologyVersionMismatch: true,
},
},
{
name: "control plane with a deleting machine should requeue",
kcp: &controlplanev1.KubeadmControlPlane{},
Expand Down Expand Up @@ -687,6 +718,55 @@ func TestPreflightChecks(t *testing.T) {
TopologyVersionMismatch: false,
},
},
{
name: "control plane with a pending upgrade, but already at the current step of the upgrade plan, should pass",
cluster: &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
clusterv1.ClusterTopologyControlPlaneUpgradeStepAnnotation: "v1.32.0",
},
},
Spec: clusterv1.ClusterSpec{
Topology: clusterv1.Topology{
Version: "v1.33.0",
},
},
},
kcp: &controlplanev1.KubeadmControlPlane{
Spec: controlplanev1.KubeadmControlPlaneSpec{
Version: "v1.32.0",
}, Status: controlplanev1.KubeadmControlPlaneStatus{
Conditions: []metav1.Condition{
{Type: controlplanev1.KubeadmControlPlaneControlPlaneComponentsHealthyCondition, Status: metav1.ConditionTrue},
{Type: controlplanev1.KubeadmControlPlaneEtcdClusterHealthyCondition, Status: metav1.ConditionTrue},
},
},
},
machines: []*clusterv1.Machine{
{
Status: clusterv1.MachineStatus{
NodeRef: clusterv1.MachineNodeReference{
Name: "node-1",
},
Conditions: []metav1.Condition{
{Type: controlplanev1.KubeadmControlPlaneMachineAPIServerPodHealthyCondition, Status: metav1.ConditionTrue},
{Type: controlplanev1.KubeadmControlPlaneMachineControllerManagerPodHealthyCondition, Status: metav1.ConditionTrue},
{Type: controlplanev1.KubeadmControlPlaneMachineSchedulerPodHealthyCondition, Status: metav1.ConditionTrue},
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdPodHealthyCondition, Status: metav1.ConditionTrue},
{Type: controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyCondition, Status: metav1.ConditionTrue},
},
},
},
},

expectResult: ctrl.Result{},
expectPreflight: internal.PreflightCheckResults{
HasDeletingMachine: false,
ControlPlaneComponentsNotHealthy: false,
EtcdClusterNotHealthy: false,
TopologyVersionMismatch: false,
},
},
}

for _, tt := range testCases {
Expand Down
Loading