diff --git a/docs/book/src/reference/labels_and_annotations.md b/docs/book/src/reference/labels_and_annotations.md index e1eb0de5900a..3d20686f853c 100644 --- a/docs/book/src/reference/labels_and_annotations.md +++ b/docs/book/src/reference/labels_and_annotations.md @@ -2,7 +2,7 @@ | Label | Note | -| :---------------------------------------- | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +|:------------------------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | cluster.x-k8s.io/cluster-name | It is set on machines linked to a cluster and external objects(bootstrap and infrastructure providers). | | topology.cluster.x-k8s.io/owned | It is set on all the object which are managed as part of a ClusterTopology. | | topology.cluster.x-k8s.io/deployment-name | It is set on the generated MachineDeployment objects to track the name of the MachineDeployment topology it represents. | @@ -21,11 +21,12 @@ **Supported Annotations:** | Annotation | Note | -| :--------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +|:-----------------------------------------------------------------|:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | clusterctl.cluster.x-k8s.io/skip-crd-name-preflight-check | Can be placed on provider CRDs, so that clusterctl doesn't emit an error if the CRD doesn't comply with Cluster APIs naming scheme. Only CRDs that are referenced by core Cluster API CRDs have to comply with the naming scheme. | | clusterctl.cluster.x-k8s.io/delete-for-move | DeleteForMoveAnnotation will be set to objects that are going to be deleted from the source cluster after being moved to the target cluster during the clusterctl move operation. It will help any validation webhook to take decision based on it. | | clusterctl.cluster.x-k8s.io/block-move | BlockMoveAnnotation prevents the cluster move operation from starting if it is defined on at least one of the objects in scope. Provider controllers are expected to set the annotation on resources that cannot be instantaneously paused and remove the annotation when the resource has been actually paused. | | unsafe.topology.cluster.x-k8s.io/disable-update-class-name-check | It can be used to disable the webhook check on update that disallows a pre-existing Cluster to be populated with Topology information and Class. | +| unsafe.topology.cluster.x-k8s.io/disable-update-version-check | It can be used to disable the webhook checks on update that disallows updating the `.topology.spec.version` on certain conditions. | | cluster.x-k8s.io/cluster-name | It is set on nodes identifying the name of the cluster the node belongs to. | | cluster.x-k8s.io/cluster-namespace | It is set on nodes identifying the namespace of the cluster the node belongs to. | | cluster.x-k8s.io/machine | It is set on nodes identifying the machine the node belongs to. | diff --git a/internal/webhooks/cluster.go b/internal/webhooks/cluster.go index 3226a0c14c4b..b3d4ceb387e0 100644 --- a/internal/webhooks/cluster.go +++ b/internal/webhooks/cluster.go @@ -28,6 +28,7 @@ import ( "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" + kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apimachinery/pkg/util/wait" @@ -363,8 +364,8 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu log.Info(warningMsg) allWarnings = append(allWarnings, warningMsg) } else { - if errs := webhook.validateTopologyVersion(ctx, fldPath.Child("version"), newCluster.Spec.Topology.Version, inVersion, oldVersion, oldCluster); len(errs) > 0 { - allErrs = append(allErrs, errs...) + if err := webhook.validateTopologyVersion(ctx, fldPath.Child("version"), newCluster.Spec.Topology.Version, inVersion, oldVersion, oldCluster); err != nil { + allErrs = append(allErrs, err) } } @@ -390,14 +391,14 @@ func (webhook *Cluster) validateTopology(ctx context.Context, oldCluster, newClu return allWarnings, allErrs } -func (webhook *Cluster) validateTopologyVersion(ctx context.Context, fldPath *field.Path, fldValue string, inVersion, oldVersion semver.Version, oldCluster *clusterv1.Cluster) field.ErrorList { +func (webhook *Cluster) validateTopologyVersion(ctx context.Context, fldPath *field.Path, fldValue string, inVersion, oldVersion semver.Version, oldCluster *clusterv1.Cluster) *field.Error { // Version could only be increased. if inVersion.NE(semver.Version{}) && oldVersion.NE(semver.Version{}) && version.Compare(inVersion, oldVersion, version.WithBuildTags()) == -1 { - return field.ErrorList{field.Invalid( + return field.Invalid( fldPath, fldValue, fmt.Sprintf("version cannot be decreased from %q to %q", oldVersion, inVersion), - )} + ) } // A +2 minor version upgrade is not allowed. @@ -407,12 +408,11 @@ func (webhook *Cluster) validateTopologyVersion(ctx context.Context, fldPath *fi Patch: 0, } if inVersion.GTE(ceilVersion) { - return field.ErrorList{field.Invalid( + return field.Invalid( fldPath, fldValue, fmt.Sprintf("version cannot be increased from %q to %q", oldVersion, inVersion), - ), - } + ) } // Only check the following cases if the minor version increases by 1 (we already return above for >= 2). @@ -427,36 +427,28 @@ func (webhook *Cluster) validateTopologyVersion(ctx context.Context, fldPath *fi return nil } - allErrs := field.ErrorList{} + allErrs := []error{} // minor version cannot be increased if control plane is upgrading or not yet on the current version if err := validateTopologyControlPlaneVersion(ctx, webhook.Client, oldCluster, oldVersion); err != nil { - allErrs = append(allErrs, field.Invalid( - fldPath, - fldValue, - fmt.Sprintf("blocking version update due to ControlPlane version check: %v", err), - )) + allErrs = append(allErrs, fmt.Errorf("blocking version update due to ControlPlane version check: %v", err)) } // minor version cannot be increased if MachineDeployments are upgrading or not yet on the current version if err := validateTopologyMachineDeploymentVersions(ctx, webhook.Client, oldCluster, oldVersion); err != nil { - allErrs = append(allErrs, field.Invalid( - fldPath, - fldValue, - fmt.Sprintf("blocking version update due to MachineDeployment version check: %v", err), - )) + allErrs = append(allErrs, fmt.Errorf("blocking version update due to MachineDeployment version check: %v", err)) } // minor version cannot be increased if MachinePools are upgrading or not yet on the current version if err := validateTopologyMachinePoolVersions(ctx, webhook.Client, webhook.Tracker, oldCluster, oldVersion); err != nil { - allErrs = append(allErrs, field.Invalid( - fldPath, - fldValue, - fmt.Sprintf("blocking version update due to MachinePool version check: %v", err), - )) + allErrs = append(allErrs, fmt.Errorf("blocking version update due to MachinePool version check: %v", err)) } if len(allErrs) > 0 { - return allErrs + return field.Invalid( + fldPath, + fldValue, + fmt.Sprintf("minor version update cannot happen at this time: %v", kerrors.NewAggregate(allErrs)), + ) } return nil @@ -497,7 +489,7 @@ func validateTopologyControlPlaneVersion(ctx context.Context, ctrlClient client. } if upgrading { - return errors.New("ControlPlane is currently upgrading") + return errors.New("ControlPlane is still completing a previous upgrade") } return nil @@ -548,7 +540,7 @@ func validateTopologyMachineDeploymentVersions(ctx context.Context, ctrlClient c } if len(mdUpgradingNames) > 0 { - return fmt.Errorf("upgrading MachineDeployments: [%s]", strings.Join(mdUpgradingNames, ", ")) + return fmt.Errorf("there are MachineDeployments still completing a previous upgrade: [%s]", strings.Join(mdUpgradingNames, ", ")) } return nil @@ -605,7 +597,7 @@ func validateTopologyMachinePoolVersions(ctx context.Context, ctrlClient client. } if len(mpUpgradingNames) > 0 { - return fmt.Errorf("upgrading MachinePools: [%s]", strings.Join(mpUpgradingNames, ", ")) + return fmt.Errorf("there are MachinePools still completing a previous upgrade: [%s]", strings.Join(mpUpgradingNames, ", ")) } return nil diff --git a/internal/webhooks/cluster_test.go b/internal/webhooks/cluster_test.go index d2d03063eeb2..eefdb817a6d4 100644 --- a/internal/webhooks/cluster_test.go +++ b/internal/webhooks/cluster_test.go @@ -1400,6 +1400,14 @@ func TestClusterTopologyValidation(t *testing.T) { WithTopology(builder.ClusterTopology(). WithClass("foo"). WithVersion("v1.19.1"). + WithMachineDeployment( + builder.MachineDeploymentTopology("workers1"). + WithClass("aa"). + Build()). + WithMachinePool( + builder.MachinePoolTopology("pool1"). + WithClass("aa"). + Build()). Build()). Build(), in: builder.Cluster("fooboo", "cluster1"). @@ -1413,6 +1421,16 @@ func TestClusterTopologyValidation(t *testing.T) { builder.ControlPlane("fooboo", "cluster1-cp").WithVersion("v1.19.1"). WithStatusFields(map[string]interface{}{"status.version": "v1.19.1"}). Build(), + builder.MachineDeployment("fooboo", "cluster1-workers1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachineDeploymentNameLabel: "workers1", + }).WithVersion("v1.19.1").Build(), + builder.MachinePool("fooboo", "cluster1-pool1").WithLabels(map[string]string{ + clusterv1.ClusterNameLabel: "cluster1", + clusterv1.ClusterTopologyOwnedLabel: "", + clusterv1.ClusterTopologyMachinePoolNameLabel: "pool1", + }).WithVersion("v1.19.1").Build(), }, }, { diff --git a/main.go b/main.go index 200eda70a54e..4156c082a94d 100644 --- a/main.go +++ b/main.go @@ -372,7 +372,7 @@ func setupIndexes(ctx context.Context, mgr ctrl.Manager) { } } -func setupReconcilers(ctx context.Context, mgr ctrl.Manager) *remote.ClusterCacheTracker { +func setupReconcilers(ctx context.Context, mgr ctrl.Manager) webhooks.ClusterCacheTrackerReader { secretCachingClient, err := client.New(mgr.GetConfig(), client.Options{ HTTPClient: mgr.GetHTTPClient(), Cache: &client.CacheOptions{ @@ -568,7 +568,7 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) *remote.ClusterCach return tracker } -func setupWebhooks(mgr ctrl.Manager, tracker *remote.ClusterCacheTracker) { +func setupWebhooks(mgr ctrl.Manager, tracker webhooks.ClusterCacheTrackerReader) { // NOTE: ClusterClass and managed topologies are behind ClusterTopology feature gate flag; the webhook // is going to prevent creating or updating new objects in case the feature flag is disabled. if err := (&webhooks.ClusterClass{Client: mgr.GetClient()}).SetupWebhookWithManager(mgr); err != nil {