diff --git a/api/core/v1beta2/cluster_types.go b/api/core/v1beta2/cluster_types.go index 48b697f22d43..d221bb390935 100644 --- a/api/core/v1beta2/cluster_types.go +++ b/api/core/v1beta2/cluster_types.go @@ -80,17 +80,24 @@ const ( // failing due to an error. ClusterTopologyReconciledFailedReason = "ReconcileFailed" + // ClusterTopologyReconciledClusterCreatingReason documents reconciliation of a Cluster topology + // not yet created because the BeforeClusterCreate hook is blocking. + ClusterTopologyReconciledClusterCreatingReason = "ClusterCreating" + // ClusterTopologyReconciledControlPlaneUpgradePendingReason documents reconciliation of a Cluster topology // not yet completed because Control Plane is not yet updated to match the desired topology spec. + // Deprecated: please use ClusterUpgrading instead. ClusterTopologyReconciledControlPlaneUpgradePendingReason = "ControlPlaneUpgradePending" // ClusterTopologyReconciledMachineDeploymentsCreatePendingReason documents reconciliation of a Cluster topology // not yet completed because at least one of the MachineDeployments is yet to be created. // This generally happens because new MachineDeployment creations are held off while the ControlPlane is not stable. + // Deprecated: please use ClusterUpgrading instead. ClusterTopologyReconciledMachineDeploymentsCreatePendingReason = "MachineDeploymentsCreatePending" // ClusterTopologyReconciledMachineDeploymentsUpgradePendingReason documents reconciliation of a Cluster topology // not yet completed because at least one of the MachineDeployments is not yet updated to match the desired topology spec. + // Deprecated: please use ClusterUpgrading instead. ClusterTopologyReconciledMachineDeploymentsUpgradePendingReason = "MachineDeploymentsUpgradePending" // ClusterTopologyReconciledMachineDeploymentsUpgradeDeferredReason documents reconciliation of a Cluster topology @@ -99,11 +106,13 @@ const ( // ClusterTopologyReconciledMachinePoolsUpgradePendingReason documents reconciliation of a Cluster topology // not yet completed because at least one of the MachinePools is not yet updated to match the desired topology spec. + // Deprecated: please use ClusterUpgrading instead. ClusterTopologyReconciledMachinePoolsUpgradePendingReason = "MachinePoolsUpgradePending" // ClusterTopologyReconciledMachinePoolsCreatePendingReason documents reconciliation of a Cluster topology // not yet completed because at least one of the MachinePools is yet to be created. // This generally happens because new MachinePool creations are held off while the ControlPlane is not stable. + // Deprecated: please use ClusterUpgrading instead. ClusterTopologyReconciledMachinePoolsCreatePendingReason = "MachinePoolsCreatePending" // ClusterTopologyReconciledMachinePoolsUpgradeDeferredReason documents reconciliation of a Cluster topology @@ -112,8 +121,12 @@ const ( // ClusterTopologyReconciledHookBlockingReason documents reconciliation of a Cluster topology // not yet completed because at least one of the lifecycle hooks is blocking. + // Deprecated: please use ClusterUpgrading instead. ClusterTopologyReconciledHookBlockingReason = "LifecycleHookBlocking" + // ClusterTopologyReconciledClusterUpgradingReason documents reconciliation of a Cluster topology + // not yet completed because a cluster upgrade is still in progress. + ClusterTopologyReconciledClusterUpgradingReason = "ClusterUpgrading" // ClusterTopologyReconciledClusterClassNotReconciledReason documents reconciliation of a Cluster topology not // yet completed because the ClusterClass has not reconciled yet. If this condition persists there may be an issue // with the ClusterClass surfaced in the ClusterClass status or controller logs. diff --git a/api/core/v1beta2/v1beta1_condition_consts.go b/api/core/v1beta2/v1beta1_condition_consts.go index 66a764a790a7..fd5fa8f50f6b 100644 --- a/api/core/v1beta2/v1beta1_condition_consts.go +++ b/api/core/v1beta2/v1beta1_condition_consts.go @@ -300,17 +300,24 @@ const ( // failing due to an error. TopologyReconcileFailedV1Beta1Reason = "TopologyReconcileFailed" + // TopologyReconciledClusterCreatingV1Beta1Reason documents reconciliation of a Cluster topology + // not yet created because the BeforeClusterCreate hook is blocking. + TopologyReconciledClusterCreatingV1Beta1Reason = "ClusterCreating" + // TopologyReconciledControlPlaneUpgradePendingV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology // not yet completed because Control Plane is not yet updated to match the desired topology spec. + // Deprecated: please use ClusterUpgrading instead. TopologyReconciledControlPlaneUpgradePendingV1Beta1Reason = "ControlPlaneUpgradePending" // TopologyReconciledMachineDeploymentsCreatePendingV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology // not yet completed because at least one of the MachineDeployments is yet to be created. // This generally happens because new MachineDeployment creations are held off while the ControlPlane is not stable. + // Deprecated: please use ClusterUpgrading instead. TopologyReconciledMachineDeploymentsCreatePendingV1Beta1Reason = "MachineDeploymentsCreatePending" // TopologyReconciledMachineDeploymentsUpgradePendingV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology // not yet completed because at least one of the MachineDeployments is not yet updated to match the desired topology spec. + // Deprecated: please use ClusterUpgrading instead. TopologyReconciledMachineDeploymentsUpgradePendingV1Beta1Reason = "MachineDeploymentsUpgradePending" // TopologyReconciledMachineDeploymentsUpgradeDeferredV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology @@ -319,11 +326,13 @@ const ( // TopologyReconciledMachinePoolsUpgradePendingV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology // not yet completed because at least one of the MachinePools is not yet updated to match the desired topology spec. + // Deprecated: please use ClusterUpgrading instead. TopologyReconciledMachinePoolsUpgradePendingV1Beta1Reason = "MachinePoolsUpgradePending" // TopologyReconciledMachinePoolsCreatePendingV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology // not yet completed because at least one of the MachinePools is yet to be created. // This generally happens because new MachinePool creations are held off while the ControlPlane is not stable. + // Deprecated: please use ClusterUpgrading instead. TopologyReconciledMachinePoolsCreatePendingV1Beta1Reason = "MachinePoolsCreatePending" // TopologyReconciledMachinePoolsUpgradeDeferredV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology @@ -332,8 +341,13 @@ const ( // TopologyReconciledHookBlockingV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology // not yet completed because at least one of the lifecycle hooks is blocking. + // Deprecated: please use ClusterUpgrading instead. TopologyReconciledHookBlockingV1Beta1Reason = "LifecycleHookBlocking" + // TopologyReconciledClusterUpgradingV1Beta1Reason documents reconciliation of a Cluster topology + // not yet completed because a cluster upgrade is still in progress. + TopologyReconciledClusterUpgradingV1Beta1Reason = "ClusterUpgrading" + // TopologyReconciledClusterClassNotReconciledV1Beta1Reason (Severity=Info) documents reconciliation of a Cluster topology not // yet completed because the ClusterClass has not reconciled yet. If this condition persists there may be an issue // with the ClusterClass surfaced in the ClusterClass status or controller logs. diff --git a/exp/topology/desiredstate/desired_state.go b/exp/topology/desiredstate/desired_state.go index 69e98e14be5a..128c42c1bc3c 100644 --- a/exp/topology/desiredstate/desired_state.go +++ b/exp/topology/desiredstate/desired_state.go @@ -19,6 +19,7 @@ package desiredstate import ( "context" + "fmt" "maps" "reflect" "time" @@ -30,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/klog/v2" "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" @@ -507,6 +509,8 @@ func (g *generator) computeControlPlane(ctx context.Context, s *scope.Scope, inf // The version is calculated using the state of the current machine deployments, the current control plane // and the version defined in the topology. func (g *generator) computeControlPlaneVersion(ctx context.Context, s *scope.Scope) (string, error) { + log := ctrl.LoggerFrom(ctx) + topologyVersion := s.Blueprint.Topology.Version // If we are creating the control plane object (current control plane is nil), use version from topology. if s.Current.ControlPlane == nil || s.Current.ControlPlane.Object == nil { @@ -599,8 +603,7 @@ func (g *generator) computeControlPlaneVersion(ctx context.Context, s *scope.Sco // Also check if MachineDeployments/MachinePools are already upgrading. // If the MachineDeployments/MachinePools are upgrading, then do not pick up the next control plane version yet. // We will pick up the new version after the MachineDeployments/MachinePools finish upgrading. - if len(s.UpgradeTracker.MachineDeployments.UpgradingNames()) > 0 || - len(s.UpgradeTracker.MachinePools.UpgradingNames()) > 0 { + if s.UpgradeTracker.MachineDeployments.IsAnyUpgrading() || s.UpgradeTracker.MachinePools.IsAnyUpgrading() { return *currentVersion, nil } @@ -692,6 +695,11 @@ func (g *generator) computeControlPlaneVersion(ctx context.Context, s *scope.Sco s.UpgradeTracker.ControlPlane.IsStartingUpgrade = true s.UpgradeTracker.ControlPlane.IsPendingUpgrade = false + log.Info(fmt.Sprintf("Control plane %s upgraded from version %s to version %s", klog.KObj(s.Current.ControlPlane.Object), *currentVersion, nextVersion), + "ControlPlaneUpgrades", toUpgradeStep(s.UpgradeTracker.ControlPlane.UpgradePlan), + "WorkersUpgrades", toUpgradeStep(s.UpgradeTracker.MachineDeployments.UpgradePlan, s.UpgradeTracker.MachinePools.UpgradePlan), + s.Current.ControlPlane.Object.GetKind(), klog.KObj(s.Current.ControlPlane.Object), + ) return nextVersion, nil } @@ -857,7 +865,7 @@ func (g *generator) computeMachineDeployment(ctx context.Context, s *scope.Scope // Add ClusterTopologyMachineDeploymentLabel to the generated InfrastructureMachine template infraMachineTemplateLabels[clusterv1.ClusterTopologyMachineDeploymentNameLabel] = machineDeploymentTopology.Name desiredMachineDeployment.InfrastructureMachineTemplate.SetLabels(infraMachineTemplateLabels) - version, err := g.computeMachineDeploymentVersion(s, machineDeploymentTopology, currentMachineDeployment) + version, err := g.computeMachineDeploymentVersion(ctx, s, machineDeploymentTopology, currentMachineDeployment) if err != nil { return nil, err } @@ -1039,7 +1047,9 @@ func (g *generator) computeMachineDeployment(ctx context.Context, s *scope.Scope // computeMachineDeploymentVersion calculates the version of the desired machine deployment. // The version is calculated using the state of the current machine deployments, // the current control plane and the version defined in the topology. -func (g *generator) computeMachineDeploymentVersion(s *scope.Scope, machineDeploymentTopology clusterv1.MachineDeploymentTopology, currentMDState *scope.MachineDeploymentState) (string, error) { +func (g *generator) computeMachineDeploymentVersion(ctx context.Context, s *scope.Scope, machineDeploymentTopology clusterv1.MachineDeploymentTopology, currentMDState *scope.MachineDeploymentState) (string, error) { + log := ctrl.LoggerFrom(ctx) + topologyVersion := s.Blueprint.Topology.Version // If creating a new machine deployment, mark it as pending if the control plane is not // yet stable. Creating a new MD while the control plane is upgrading can lead to unexpected race conditions. @@ -1111,6 +1121,12 @@ func (g *generator) computeMachineDeploymentVersion(s *scope.Scope, machineDeplo s.UpgradeTracker.MachineDeployments.MarkUpgrading(currentMDState.Object.Name) nextVersion := s.UpgradeTracker.MachineDeployments.UpgradePlan[0] + + log.Info(fmt.Sprintf("MachineDeployment %s upgraded from version %s to version %s", klog.KObj(currentMDState.Object), currentVersion, nextVersion), + "ControlPlaneUpgrades", toUpgradeStep(s.UpgradeTracker.ControlPlane.UpgradePlan), + "WorkersUpgrades", toUpgradeStep(s.UpgradeTracker.MachineDeployments.UpgradePlan, s.UpgradeTracker.MachinePools.UpgradePlan), + "MachineDeployment", klog.KObj(currentMDState.Object), + ) return nextVersion, nil } @@ -1165,7 +1181,7 @@ func (g *generator) computeMachinePools(ctx context.Context, s *scope.Scope) (sc // computeMachinePool computes the desired state for a MachinePoolTopology. // The generated machinePool object is calculated using the values from the machinePoolTopology and // the machinePool class. -func (g *generator) computeMachinePool(_ context.Context, s *scope.Scope, machinePoolTopology clusterv1.MachinePoolTopology) (*scope.MachinePoolState, error) { +func (g *generator) computeMachinePool(ctx context.Context, s *scope.Scope, machinePoolTopology clusterv1.MachinePoolTopology) (*scope.MachinePoolState, error) { desiredMachinePool := &scope.MachinePoolState{} // Gets the blueprint for the MachinePool class. @@ -1243,7 +1259,7 @@ func (g *generator) computeMachinePool(_ context.Context, s *scope.Scope, machin // Add ClusterTopologyMachinePoolLabel to the generated InfrastructureMachinePool object infraMachinePoolObjectLabels[clusterv1.ClusterTopologyMachinePoolNameLabel] = machinePoolTopology.Name desiredMachinePool.InfrastructureMachinePoolObject.SetLabels(infraMachinePoolObjectLabels) - version, err := g.computeMachinePoolVersion(s, machinePoolTopology, currentMachinePool) + version, err := g.computeMachinePoolVersion(ctx, s, machinePoolTopology, currentMachinePool) if err != nil { return nil, err } @@ -1359,7 +1375,9 @@ func (g *generator) computeMachinePool(_ context.Context, s *scope.Scope, machin // computeMachinePoolVersion calculates the version of the desired machine pool. // The version is calculated using the state of the current machine pools, // the current control plane and the version defined in the topology. -func (g *generator) computeMachinePoolVersion(s *scope.Scope, machinePoolTopology clusterv1.MachinePoolTopology, currentMPState *scope.MachinePoolState) (string, error) { +func (g *generator) computeMachinePoolVersion(ctx context.Context, s *scope.Scope, machinePoolTopology clusterv1.MachinePoolTopology, currentMPState *scope.MachinePoolState) (string, error) { + log := ctrl.LoggerFrom(ctx) + topologyVersion := s.Blueprint.Topology.Version // If creating a new machine pool, mark it as pending if the control plane is not // yet stable. Creating a new MP while the control plane is upgrading can lead to unexpected race conditions. @@ -1431,6 +1449,12 @@ func (g *generator) computeMachinePoolVersion(s *scope.Scope, machinePoolTopolog s.UpgradeTracker.MachinePools.MarkUpgrading(currentMPState.Object.Name) nextVersion := s.UpgradeTracker.MachinePools.UpgradePlan[0] + + log.Info(fmt.Sprintf("MachinePool %s upgraded from version %s to version %s", klog.KObj(currentMPState.Object), currentVersion, nextVersion), + "ControlPlaneUpgrades", toUpgradeStep(s.UpgradeTracker.ControlPlane.UpgradePlan), + "WorkersUpgrades", toUpgradeStep(s.UpgradeTracker.MachineDeployments.UpgradePlan, s.UpgradeTracker.MachinePools.UpgradePlan), + "MachinePool", klog.KObj(currentMPState.Object), + ) return nextVersion, nil } diff --git a/exp/topology/desiredstate/desired_state_test.go b/exp/topology/desiredstate/desired_state_test.go index 62a17b3ff621..59249ee442d1 100644 --- a/exp/topology/desiredstate/desired_state_test.go +++ b/exp/topology/desiredstate/desired_state_test.go @@ -1062,11 +1062,11 @@ func TestComputeControlPlaneVersion(t *testing.T) { catalog := runtimecatalog.New() _ = runtimehooksv1.AddToCatalog(catalog) + beforeClusterUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.BeforeClusterUpgrade) + beforeControlPlaneUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.BeforeControlPlaneUpgrade) + beforeWorkersUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.BeforeWorkersUpgrade) + afterWorkersUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.AfterWorkersUpgrade) - beforeClusterUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.BeforeClusterUpgrade) - if err != nil { - panic("unable to compute GVH") - } nonBlockingBeforeClusterUpgradeResponse := &runtimehooksv1.BeforeClusterUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -1090,10 +1090,6 @@ func TestComputeControlPlaneVersion(t *testing.T) { }, } - beforeControlPlaneUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.BeforeControlPlaneUpgrade) - if err != nil { - panic("unable to compute GVH") - } nonBlockingBeforeControlPlaneUpgradeResponse := &runtimehooksv1.BeforeControlPlaneUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -1117,10 +1113,6 @@ func TestComputeControlPlaneVersion(t *testing.T) { }, } - beforeWorkersUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.BeforeWorkersUpgrade) - if err != nil { - panic("unable to compute GVH") - } nonBlockingBeforeWorkersUpgradeResponse := &runtimehooksv1.BeforeWorkersUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -1144,10 +1136,6 @@ func TestComputeControlPlaneVersion(t *testing.T) { }, } - afterWorkersUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.AfterWorkersUpgrade) - if err != nil { - panic("unable to compute GVH") - } nonBlockingAfterWorkersUpgradeResponse := &runtimehooksv1.AfterWorkersUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -1705,6 +1693,12 @@ func TestComputeControlPlaneVersion(t *testing.T) { runtimeClient := fakeruntimeclient.NewRuntimeClientBuilder(). WithCatalog(catalog). + WithGetAllExtensionResponses(map[runtimecatalog.GroupVersionHook][]string{ + beforeClusterUpgradeGVH: {"foo"}, + beforeControlPlaneUpgradeGVH: {"foo"}, + beforeWorkersUpgradeGVH: {"foo"}, + afterWorkersUpgradeGVH: {"foo"}, + }). WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{ beforeClusterUpgradeGVH: tt.beforeClusterUpgradeResponse, beforeControlPlaneUpgradeGVH: tt.beforeControlPlaneUpgradeResponse, @@ -2969,7 +2963,7 @@ func TestComputeMachineDeploymentVersion(t *testing.T) { e := generator{} - version, err := e.computeMachineDeploymentVersion(s, tt.machineDeploymentTopology, tt.currentMachineDeploymentState) + version, err := e.computeMachineDeploymentVersion(ctx, s, tt.machineDeploymentTopology, tt.currentMachineDeploymentState) g.Expect(err).NotTo(HaveOccurred()) g.Expect(version).To(Equal(tt.expectedVersion)) @@ -3214,7 +3208,7 @@ func TestComputeMachinePoolVersion(t *testing.T) { e := generator{} - version, err := e.computeMachinePoolVersion(s, tt.machinePoolTopology, tt.currentMachinePoolState) + version, err := e.computeMachinePoolVersion(ctx, s, tt.machinePoolTopology, tt.currentMachinePoolState) g.Expect(err).NotTo(HaveOccurred()) g.Expect(version).To(Equal(tt.expectedVersion)) diff --git a/exp/topology/desiredstate/lifecycle_hooks.go b/exp/topology/desiredstate/lifecycle_hooks.go index 8eeb6bc9001b..91e125571541 100644 --- a/exp/topology/desiredstate/lifecycle_hooks.go +++ b/exp/topology/desiredstate/lifecycle_hooks.go @@ -48,9 +48,9 @@ func (g *generator) callBeforeClusterUpgradeHook(ctx context.Context, s *scope.S } if len(hookAnnotations) > 0 { slices.Sort(hookAnnotations) - message := fmt.Sprintf("annotations [%s] are set", strings.Join(hookAnnotations, ", ")) + message := fmt.Sprintf("annotations %s are set", strings.Join(hookAnnotations, ", ")) if len(hookAnnotations) == 1 { - message = fmt.Sprintf("annotation [%s] is set", strings.Join(hookAnnotations, ", ")) + message = fmt.Sprintf("annotation %s is set", strings.Join(hookAnnotations, ", ")) } // Add the hook with a response to the tracker so we can later update the condition. s.HookResponseTracker.Add(runtimehooksv1.BeforeClusterUpgrade, &runtimehooksv1.BeforeClusterUpgradeResponse{ @@ -72,6 +72,15 @@ func (g *generator) callBeforeClusterUpgradeHook(ctx context.Context, s *scope.S return false, nil } + // Return quickly if the hook is not defined. + extensionHandlers, err := g.RuntimeClient.GetAllExtensions(ctx, runtimehooksv1.BeforeClusterUpgrade, s.Current.Cluster) + if err != nil { + return false, err + } + if len(extensionHandlers) == 0 { + return true, nil + } + v1beta1Cluster := &clusterv1beta1.Cluster{} // DeepCopy cluster because ConvertFrom has side effects like adding the conversion annotation. if err := v1beta1Cluster.ConvertFrom(s.Current.Cluster.DeepCopy()); err != nil { @@ -94,12 +103,17 @@ func (g *generator) callBeforeClusterUpgradeHook(ctx context.Context, s *scope.S if hookResponse.RetryAfterSeconds != 0 { // Cannot pickup the new version right now. Need to try again later. - log.Info(fmt.Sprintf("Cluster upgrade from version %s to version %s is blocked by %s hook", *currentVersion, topologyVersion, runtimecatalog.HookName(runtimehooksv1.BeforeClusterUpgrade)), + log.Info(fmt.Sprintf("Cluster upgrade from version %s to version %s is blocked by %s hook", hookRequest.FromKubernetesVersion, hookRequest.ToKubernetesVersion, runtimecatalog.HookName(runtimehooksv1.BeforeClusterUpgrade)), "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, "WorkersUpgrades", hookRequest.WorkersUpgrades, ) return false, nil } + + log.Info(fmt.Sprintf("Cluster upgrade from version %s to version %s unblocked by %s hook", hookRequest.FromKubernetesVersion, hookRequest.ToKubernetesVersion, runtimecatalog.HookName(runtimehooksv1.BeforeClusterUpgrade)), + "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, + "WorkersUpgrades", hookRequest.WorkersUpgrades, + ) } return true, nil } @@ -111,6 +125,15 @@ func (g *generator) callBeforeClusterUpgradeHook(ctx context.Context, s *scope.S func (g *generator) callBeforeControlPlaneUpgradeHook(ctx context.Context, s *scope.Scope, currentVersion *string, nextVersion string) (bool, error) { log := ctrl.LoggerFrom(ctx) + // Return quickly if the hook is not defined. + extensionHandlers, err := g.RuntimeClient.GetAllExtensions(ctx, runtimehooksv1.BeforeControlPlaneUpgrade, s.Current.Cluster) + if err != nil { + return false, err + } + if len(extensionHandlers) == 0 { + return true, nil + } + // NOTE: the hook should always be called before piking up a new version. v1beta1Cluster := &clusterv1beta1.Cluster{} // DeepCopy cluster because ConvertFrom has side effects like adding the conversion annotation. @@ -134,13 +157,18 @@ func (g *generator) callBeforeControlPlaneUpgradeHook(ctx context.Context, s *sc if hookResponse.RetryAfterSeconds != 0 { // Cannot pickup the new version right now. Need to try again later. - log.Info(fmt.Sprintf("Control plane upgrade from version %s to version %s is blocked by %s hook", *currentVersion, nextVersion, runtimecatalog.HookName(runtimehooksv1.BeforeControlPlaneUpgrade)), + log.Info(fmt.Sprintf("Control plane upgrade from version %s to version %s is blocked by %s hook", hookRequest.FromKubernetesVersion, hookRequest.ToKubernetesVersion, runtimecatalog.HookName(runtimehooksv1.BeforeControlPlaneUpgrade)), "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, "WorkersUpgrades", hookRequest.WorkersUpgrades, ) return false, nil } + log.Info(fmt.Sprintf("Control plane upgrade from version %s to version %s unblocked by %s hook", hookRequest.FromKubernetesVersion, hookRequest.ToKubernetesVersion, runtimecatalog.HookName(runtimehooksv1.BeforeControlPlaneUpgrade)), + "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, + "WorkersUpgrades", hookRequest.WorkersUpgrades, + ) + return true, nil } @@ -153,8 +181,20 @@ func (g *generator) callAfterControlPlaneUpgradeHook(ctx context.Context, s *sco // Call the hook only if we are tracking the intent to do so. If it is not tracked it means we don't need to call the // hook because we didn't go through an upgrade or we already called the hook after the upgrade. if hooks.IsPending(runtimehooksv1.AfterControlPlaneUpgrade, s.Current.Cluster) { - v1beta1Cluster := &clusterv1beta1.Cluster{} + // Return quickly if the hook is not defined. + extensionHandlers, err := g.RuntimeClient.GetAllExtensions(ctx, runtimehooksv1.AfterControlPlaneUpgrade, s.Current.Cluster) + if err != nil { + return false, err + } + if len(extensionHandlers) == 0 { + if err := hooks.MarkAsDone(ctx, g.Client, s.Current.Cluster, runtimehooksv1.AfterControlPlaneUpgrade); err != nil { + return false, err + } + return true, nil + } + // DeepCopy cluster because ConvertFrom has side effects like adding the conversion annotation. + v1beta1Cluster := &clusterv1beta1.Cluster{} if err := v1beta1Cluster.ConvertFrom(s.Current.Cluster.DeepCopy()); err != nil { return false, errors.Wrap(err, "error converting Cluster to v1beta1 Cluster") } @@ -174,7 +214,7 @@ func (g *generator) callAfterControlPlaneUpgradeHook(ctx context.Context, s *sco s.HookResponseTracker.Add(runtimehooksv1.AfterControlPlaneUpgrade, hookResponse) if hookResponse.RetryAfterSeconds != 0 { - log.Info(fmt.Sprintf("Cluster upgrade is blocked after control plane upgrade to version %s by %s hook", *currentVersion, runtimecatalog.HookName(runtimehooksv1.AfterControlPlaneUpgrade)), + log.Info(fmt.Sprintf("Control plane upgrade to version %s completed but next steps are blocked by %s hook", hookRequest.KubernetesVersion, runtimecatalog.HookName(runtimehooksv1.AfterControlPlaneUpgrade)), "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, "WorkersUpgrades", hookRequest.WorkersUpgrades, ) @@ -183,6 +223,11 @@ func (g *generator) callAfterControlPlaneUpgradeHook(ctx context.Context, s *sco if err := hooks.MarkAsDone(ctx, g.Client, s.Current.Cluster, runtimehooksv1.AfterControlPlaneUpgrade); err != nil { return false, err } + + log.Info(fmt.Sprintf("Control plane upgrade to version %s and %s hook completed, next steps unblocked", hookRequest.KubernetesVersion, runtimecatalog.HookName(runtimehooksv1.AfterControlPlaneUpgrade)), + "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, + "WorkersUpgrades", hookRequest.WorkersUpgrades, + ) } return true, nil } @@ -197,8 +242,20 @@ func (g *generator) callBeforeWorkersUpgradeHook(ctx context.Context, s *scope.S // Call the hook only if we are tracking the intent to do so. If it is not tracked it means we don't need to call the // hook because we didn't go through an upgrade or we already called the hook after the upgrade. if hooks.IsPending(runtimehooksv1.BeforeWorkersUpgrade, s.Current.Cluster) { - v1beta1Cluster := &clusterv1beta1.Cluster{} + // Return quickly if the hook is not defined. + extensionHandlers, err := g.RuntimeClient.GetAllExtensions(ctx, runtimehooksv1.BeforeWorkersUpgrade, s.Current.Cluster) + if err != nil { + return false, err + } + if len(extensionHandlers) == 0 { + if err := hooks.MarkAsDone(ctx, g.Client, s.Current.Cluster, runtimehooksv1.BeforeWorkersUpgrade); err != nil { + return false, err + } + return true, nil + } + // DeepCopy cluster because ConvertFrom has side effects like adding the conversion annotation. + v1beta1Cluster := &clusterv1beta1.Cluster{} if err := v1beta1Cluster.ConvertFrom(s.Current.Cluster.DeepCopy()); err != nil { return false, errors.Wrap(err, "error converting Cluster to v1beta1 Cluster") } @@ -219,7 +276,7 @@ func (g *generator) callBeforeWorkersUpgradeHook(ctx context.Context, s *scope.S if hookResponse.RetryAfterSeconds != 0 { // Cannot pickup the new version right now. Need to try again later. - log.Info(fmt.Sprintf("Workers upgrade from version %s to version %s is blocked by %s hook", *currentVersion, nextVersion, runtimecatalog.HookName(runtimehooksv1.BeforeWorkersUpgrade)), + log.Info(fmt.Sprintf("Workers upgrade from version %s to version %s is blocked by %s hook", hookRequest.FromKubernetesVersion, hookRequest.ToKubernetesVersion, runtimecatalog.HookName(runtimehooksv1.BeforeWorkersUpgrade)), "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, "WorkersUpgrades", hookRequest.WorkersUpgrades, ) @@ -228,6 +285,11 @@ func (g *generator) callBeforeWorkersUpgradeHook(ctx context.Context, s *scope.S if err := hooks.MarkAsDone(ctx, g.Client, s.Current.Cluster, runtimehooksv1.BeforeWorkersUpgrade); err != nil { return false, err } + + log.Info(fmt.Sprintf("Workers upgrade from version %s to version %s unblocked by %s hook", hookRequest.FromKubernetesVersion, hookRequest.ToKubernetesVersion, runtimecatalog.HookName(runtimehooksv1.BeforeWorkersUpgrade)), + "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, + "WorkersUpgrades", hookRequest.WorkersUpgrades, + ) } return true, nil @@ -243,8 +305,20 @@ func (g *generator) callAfterWorkersUpgradeHook(ctx context.Context, s *scope.Sc // Call the hook only if we are tracking the intent to do so. If it is not tracked it means we don't need to call the // hook because we didn't go through an upgrade or we already called the hook after the upgrade. if hooks.IsPending(runtimehooksv1.AfterWorkersUpgrade, s.Current.Cluster) { - v1beta1Cluster := &clusterv1beta1.Cluster{} + // Return quickly if the hook is not defined. + extensionHandlers, err := g.RuntimeClient.GetAllExtensions(ctx, runtimehooksv1.AfterWorkersUpgrade, s.Current.Cluster) + if err != nil { + return false, err + } + if len(extensionHandlers) == 0 { + if err := hooks.MarkAsDone(ctx, g.Client, s.Current.Cluster, runtimehooksv1.AfterWorkersUpgrade); err != nil { + return false, err + } + return true, nil + } + // DeepCopy cluster because ConvertFrom has side effects like adding the conversion annotation. + v1beta1Cluster := &clusterv1beta1.Cluster{} if err := v1beta1Cluster.ConvertFrom(s.Current.Cluster.DeepCopy()); err != nil { return false, errors.Wrap(err, "error converting Cluster to v1beta1 Cluster") } @@ -264,7 +338,7 @@ func (g *generator) callAfterWorkersUpgradeHook(ctx context.Context, s *scope.Sc s.HookResponseTracker.Add(runtimehooksv1.AfterWorkersUpgrade, hookResponse) if hookResponse.RetryAfterSeconds != 0 { - log.Info(fmt.Sprintf("Cluster upgrade is blocked after workers upgrade to version %s by %s hook", *currentVersion, runtimecatalog.HookName(runtimehooksv1.AfterWorkersUpgrade)), + log.Info(fmt.Sprintf("Workers upgrade to version %s completed but next steps are blocked by %s hook", hookRequest.KubernetesVersion, runtimecatalog.HookName(runtimehooksv1.AfterWorkersUpgrade)), "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, "WorkersUpgrades", hookRequest.WorkersUpgrades, ) @@ -273,6 +347,11 @@ func (g *generator) callAfterWorkersUpgradeHook(ctx context.Context, s *scope.Sc if err := hooks.MarkAsDone(ctx, g.Client, s.Current.Cluster, runtimehooksv1.AfterWorkersUpgrade); err != nil { return false, err } + + log.Info(fmt.Sprintf("Workers upgrade to version %s and %s hook completed, next steps unblocked", hookRequest.KubernetesVersion, runtimecatalog.HookName(runtimehooksv1.AfterWorkersUpgrade)), + "ControlPlaneUpgrades", hookRequest.ControlPlaneUpgrades, + "WorkersUpgrades", hookRequest.WorkersUpgrades, + ) } return true, nil } diff --git a/exp/topology/desiredstate/lifecycle_hooks_test.go b/exp/topology/desiredstate/lifecycle_hooks_test.go index 0612cfc17ba1..c37db235b42c 100644 --- a/exp/topology/desiredstate/lifecycle_hooks_test.go +++ b/exp/topology/desiredstate/lifecycle_hooks_test.go @@ -72,11 +72,12 @@ func TestComputeControlPlaneVersion_LifecycleHooksSequences(t *testing.T) { catalog := runtimecatalog.New() _ = runtimehooksv1.AddToCatalog(catalog) + beforeClusterUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.BeforeClusterUpgrade) + beforeControlPlaneUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.BeforeControlPlaneUpgrade) + afterControlPlaneUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.AfterControlPlaneUpgrade) + beforeWorkersUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.BeforeWorkersUpgrade) + afterWorkersUpgradeGVH, _ := catalog.GroupVersionHook(runtimehooksv1.AfterWorkersUpgrade) - beforeClusterUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.BeforeClusterUpgrade) - if err != nil { - panic("unable to compute GVH") - } blockingBeforeClusterUpgradeResponse := &runtimehooksv1.BeforeClusterUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -93,11 +94,6 @@ func TestComputeControlPlaneVersion_LifecycleHooksSequences(t *testing.T) { }, } - beforeControlPlaneUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.BeforeControlPlaneUpgrade) - if err != nil { - panic("unable to compute GVH") - } - blockingBeforeControlPlaneUpgradeResponse := &runtimehooksv1.BeforeControlPlaneUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -114,11 +110,6 @@ func TestComputeControlPlaneVersion_LifecycleHooksSequences(t *testing.T) { }, } - afterControlPlaneUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.AfterControlPlaneUpgrade) - if err != nil { - panic("unable to compute GVH") - } - blockingAfterControlPlaneUpgradeResponse := &runtimehooksv1.AfterControlPlaneUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -135,11 +126,6 @@ func TestComputeControlPlaneVersion_LifecycleHooksSequences(t *testing.T) { }, } - beforeWorkersUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.BeforeWorkersUpgrade) - if err != nil { - panic("unable to compute GVH") - } - blockingBeforeWorkersUpgradeResponse := &runtimehooksv1.BeforeWorkersUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -156,10 +142,6 @@ func TestComputeControlPlaneVersion_LifecycleHooksSequences(t *testing.T) { }, } - afterWorkersUpgradeGVH, err := catalog.GroupVersionHook(runtimehooksv1.AfterWorkersUpgrade) - if err != nil { - panic("unable to compute GVH") - } blockingAfterWorkersUpgradeResponse := &runtimehooksv1.AfterWorkersUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ @@ -1242,6 +1224,13 @@ func TestComputeControlPlaneVersion_LifecycleHooksSequences(t *testing.T) { runtimeClient := fakeruntimeclient.NewRuntimeClientBuilder(). WithCatalog(catalog). + WithGetAllExtensionResponses(map[runtimecatalog.GroupVersionHook][]string{ + beforeClusterUpgradeGVH: {"foo"}, + beforeControlPlaneUpgradeGVH: {"foo"}, + afterControlPlaneUpgradeGVH: {"foo"}, + beforeWorkersUpgradeGVH: {"foo"}, + afterWorkersUpgradeGVH: {"foo"}, + }). WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{ beforeClusterUpgradeGVH: tt.beforeClusterUpgradeResponse, beforeControlPlaneUpgradeGVH: tt.beforeControlPlaneUpgradeResponse, diff --git a/exp/topology/scope/hookresponsetracker.go b/exp/topology/scope/hookresponsetracker.go index e822961fda60..c8201ea3e6fd 100644 --- a/exp/topology/scope/hookresponsetracker.go +++ b/exp/topology/scope/hookresponsetracker.go @@ -64,6 +64,11 @@ func (h *HookResponseTracker) IsBlocking(hook runtimecatalog.Hook) bool { return true } +// IsAnyBlocking return true if at least one hook is blocking. +func (h *HookResponseTracker) IsAnyBlocking() bool { + return h.AggregateRetryAfter() > 0 +} + // AggregateRetryAfter calculates the lowest non-zero retryAfterSeconds time from all the tracked responses. func (h *HookResponseTracker) AggregateRetryAfter() time.Duration { res := int32(0) @@ -76,7 +81,7 @@ func (h *HookResponseTracker) AggregateRetryAfter() time.Duration { } // AggregateMessage returns a human friendly message about the blocking status of hooks. -func (h *HookResponseTracker) AggregateMessage() string { +func (h *HookResponseTracker) AggregateMessage(action string) string { blockingHooks := map[string]string{} for hook, resp := range h.responses { if retryResponse, ok := resp.(runtimehooksv1.RetryResponseObject); ok { @@ -92,10 +97,10 @@ func (h *HookResponseTracker) AggregateMessage() string { hookAndMessages := []string{} for hook, message := range blockingHooks { if message == "" { - hookAndMessages = append(hookAndMessages, fmt.Sprintf("hook %q is blocking", hook)) + hookAndMessages = append(hookAndMessages, hook) } else { - hookAndMessages = append(hookAndMessages, fmt.Sprintf("hook %q is blocking: %s", hook, message)) + hookAndMessages = append(hookAndMessages, fmt.Sprintf("%s: %s", hook, message)) } } - return strings.Join(hookAndMessages, "; ") + return fmt.Sprintf("Following hooks are blocking %s: %s", action, strings.Join(hookAndMessages, "; ")) } diff --git a/exp/topology/scope/hookresponsetracker_test.go b/exp/topology/scope/hookresponsetracker_test.go index e022e2e19863..a6b80dc04ec9 100644 --- a/exp/topology/scope/hookresponsetracker_test.go +++ b/exp/topology/scope/hookresponsetracker_test.go @@ -132,8 +132,8 @@ func TestHookResponseTracker_AggregateMessage(t *testing.T) { hrt.Add(runtimehooksv1.BeforeClusterCreate, blockingBeforeClusterCreateResponse) hrt.Add(runtimehooksv1.BeforeClusterUpgrade, blockingBeforeClusterUpgradeResponse) - g.Expect(hrt.AggregateMessage()).To(ContainSubstring(runtimecatalog.HookName(runtimehooksv1.BeforeClusterCreate))) - g.Expect(hrt.AggregateMessage()).To(ContainSubstring(runtimecatalog.HookName(runtimehooksv1.BeforeClusterUpgrade))) + g.Expect(hrt.AggregateMessage("upgrade")).To(ContainSubstring(runtimecatalog.HookName(runtimehooksv1.BeforeClusterCreate))) + g.Expect(hrt.AggregateMessage("upgrade")).To(ContainSubstring(runtimecatalog.HookName(runtimehooksv1.BeforeClusterUpgrade))) }) t.Run("AggregateMessage should return empty string if there are no blocking hook responses", func(t *testing.T) { g := NewWithT(t) @@ -142,7 +142,7 @@ func TestHookResponseTracker_AggregateMessage(t *testing.T) { hrt.Add(runtimehooksv1.BeforeClusterCreate, nonBlockingBeforeClusterCreateResponse) hrt.Add(runtimehooksv1.BeforeClusterUpgrade, nonBlockingBeforeClusterUpgradeResponse) - g.Expect(hrt.AggregateMessage()).To(Equal("")) + g.Expect(hrt.AggregateMessage("upgrade")).To(Equal("")) }) } diff --git a/exp/topology/scope/upgradetracker.go b/exp/topology/scope/upgradetracker.go index 7384dce71666..fef4649a1e69 100644 --- a/exp/topology/scope/upgradetracker.go +++ b/exp/topology/scope/upgradetracker.go @@ -251,6 +251,12 @@ func (m *WorkerUpgradeTracker) UpgradingNames() []string { return sets.List(m.upgradingNames) } +// IsAnyUpgrading returns true if any of the machine deployments are upgrading. +// Returns false, otherwise. +func (m *WorkerUpgradeTracker) IsAnyUpgrading() bool { + return len(m.upgradingNames) != 0 +} + // UpgradeConcurrencyReached returns true if the number of MachineDeployments/MachinePools upgrading is at the concurrency limit. func (m *WorkerUpgradeTracker) UpgradeConcurrencyReached() bool { return m.upgradingNames.Len() >= m.maxUpgradeConcurrency @@ -316,8 +322,8 @@ func (m *WorkerUpgradeTracker) DeferredUpgradeNames() []string { return sets.List(m.deferredNames) } -// DeferredUpgrade returns true if the upgrade has been deferred for any of the +// IsAnyUpgradeDeferred returns true if the upgrade has been deferred for any of the // MachineDeployments/MachinePools. Returns false, otherwise. -func (m *WorkerUpgradeTracker) DeferredUpgrade() bool { +func (m *WorkerUpgradeTracker) IsAnyUpgradeDeferred() bool { return len(m.deferredNames) != 0 } diff --git a/internal/contract/controlplane.go b/internal/contract/controlplane.go index 3a14238c91f0..1bbf510987ce 100644 --- a/internal/contract/controlplane.go +++ b/internal/contract/controlplane.go @@ -187,7 +187,7 @@ func (c *ControlPlaneContract) ExternalManagedControlPlane() *Bool { // IsProvisioning returns true if the control plane is being created for the first time. // Returns false, if the control plane was already previously provisioned. func (c *ControlPlaneContract) IsProvisioning(obj *unstructured.Unstructured) (bool, error) { - // We can know if the control plane was previously created or is being cretaed for the first + // We can know if the control plane was previously created or is being created for the first // time by looking at controlplane.status.version. If the version in status is set to a valid // value then the control plane was already provisioned at a previous time. If not, we can // assume that the control plane is being created for the first time. diff --git a/internal/controllers/cluster/cluster_controller_status_test.go b/internal/controllers/cluster/cluster_controller_status_test.go index 59d86077720d..49fdc008140d 100644 --- a/internal/controllers/cluster/cluster_controller_status_test.go +++ b/internal/controllers/cluster/cluster_controller_status_test.go @@ -2844,19 +2844,22 @@ func TestSetAvailableCondition(t *testing.T) { Reason: "Foo", }, { - Type: clusterv1.ClusterTopologyReconciledCondition, - Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterTopologyReconciledControlPlaneUpgradePendingReason, - Message: "Control plane rollout and upgrade to version v1.29.0 on hold.", + Type: clusterv1.ClusterTopologyReconciledCondition, + Status: metav1.ConditionFalse, + Reason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + Message: "Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md1 upgrading to version v1.22.0", }, }, }, }, expectCondition: metav1.Condition{ - Type: clusterv1.ClusterAvailableCondition, - Status: metav1.ConditionTrue, - Reason: clusterv1.ClusterAvailableReason, - Message: "* TopologyReconciled: Control plane rollout and upgrade to version v1.29.0 on hold.", + Type: clusterv1.ClusterAvailableCondition, + Status: metav1.ConditionTrue, + Reason: clusterv1.ClusterAvailableReason, + Message: "* TopologyReconciled:\n" + + " * Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md1 upgrading to version v1.22.0", }, }, { @@ -2902,10 +2905,11 @@ func TestSetAvailableCondition(t *testing.T) { Reason: "Foo", }, { - Type: clusterv1.ClusterTopologyReconciledCondition, - Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterTopologyReconciledControlPlaneUpgradePendingReason, - Message: "Control plane rollout and upgrade to version v1.29.0 on hold.", + Type: clusterv1.ClusterTopologyReconciledCondition, + Status: metav1.ConditionFalse, + Reason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + Message: "Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md1 upgrading to version v1.22.0", }, }, }, diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go index 0fd7706b4f12..c444ef0b65cf 100644 --- a/internal/controllers/topology/cluster/cluster_controller.go +++ b/internal/controllers/topology/cluster/cluster_controller.go @@ -314,7 +314,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re // In case the object is deleted, the managed topology stops to reconcile; // (the other controllers will take care of deletion). if !cluster.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, cluster) + return r.reconcileDelete(ctx, s) } // Handle normal reconciliation loop. @@ -435,8 +435,17 @@ func (r *Reconciler) callBeforeClusterCreateHook(ctx context.Context, s *scope.S log := ctrl.LoggerFrom(ctx) if !s.Current.Cluster.Spec.InfrastructureRef.IsDefined() && !s.Current.Cluster.Spec.ControlPlaneRef.IsDefined() { - v1beta1Cluster := &clusterv1beta1.Cluster{} + // Return quickly if the hook is not defined. + extensionHandlers, err := r.RuntimeClient.GetAllExtensions(ctx, runtimehooksv1.BeforeClusterCreate, s.Current.Cluster) + if err != nil { + return ctrl.Result{}, err + } + if len(extensionHandlers) == 0 { + return ctrl.Result{}, nil + } + // DeepCopy cluster because ConvertFrom has side effects like adding the conversion annotation. + v1beta1Cluster := &clusterv1beta1.Cluster{} if err := v1beta1Cluster.ConvertFrom(s.Current.Cluster.DeepCopy()); err != nil { return ctrl.Result{}, errors.Wrap(err, "error converting Cluster to v1beta1 Cluster") } @@ -449,10 +458,13 @@ func (r *Reconciler) callBeforeClusterCreateHook(ctx context.Context, s *scope.S return ctrl.Result{}, err } s.HookResponseTracker.Add(runtimehooksv1.BeforeClusterCreate, hookResponse) + if hookResponse.RetryAfterSeconds != 0 { log.Info(fmt.Sprintf("Creation of Cluster topology is blocked by %s hook", runtimecatalog.HookName(runtimehooksv1.BeforeClusterCreate))) return ctrl.Result{RequeueAfter: time.Duration(hookResponse.RetryAfterSeconds) * time.Second}, nil } + + log.Info(fmt.Sprintf("Creation of Cluster topology unblocked by %s hook", runtimecatalog.HookName(runtimehooksv1.BeforeClusterCreate))) } return ctrl.Result{}, nil } @@ -523,12 +535,26 @@ func (r *Reconciler) machinePoolToCluster(_ context.Context, o client.Object) [] }} } -func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Cluster) (ctrl.Result, error) { +func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope.Scope) (ctrl.Result, error) { + cluster := s.Current.Cluster + // Call the BeforeClusterDelete hook if the 'ok-to-delete' annotation is not set // and add the annotation to the cluster after receiving a successful non-blocking response. log := ctrl.LoggerFrom(ctx) if feature.Gates.Enabled(feature.RuntimeSDK) { if !hooks.IsOkToDelete(cluster) { + // Return quickly if the hook is not defined. + extensionHandlers, err := r.RuntimeClient.GetAllExtensions(ctx, runtimehooksv1.BeforeClusterDelete, s.Current.Cluster) + if err != nil { + return ctrl.Result{}, err + } + if len(extensionHandlers) == 0 { + if err := hooks.MarkAsOkToDelete(ctx, r.Client, cluster); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + v1beta1Cluster := &clusterv1beta1.Cluster{} // DeepCopy cluster because ConvertFrom has side effects like adding the conversion annotation. if err := v1beta1Cluster.ConvertFrom(cluster.DeepCopy()); err != nil { @@ -542,6 +568,9 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu if err := r.RuntimeClient.CallAllExtensions(ctx, runtimehooksv1.BeforeClusterDelete, cluster, hookRequest, hookResponse); err != nil { return ctrl.Result{}, err } + // Add the response to the tracker so we can later update condition or requeue when required. + s.HookResponseTracker.Add(runtimehooksv1.BeforeClusterDelete, hookResponse) + if hookResponse.RetryAfterSeconds != 0 { log.Info(fmt.Sprintf("Cluster deletion is blocked by %q hook", runtimecatalog.HookName(runtimehooksv1.BeforeClusterDelete))) return ctrl.Result{RequeueAfter: time.Duration(hookResponse.RetryAfterSeconds) * time.Second}, nil @@ -551,6 +580,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu if err := hooks.MarkAsOkToDelete(ctx, r.Client, cluster); err != nil { return ctrl.Result{}, err } + log.Info(fmt.Sprintf("Cluster deletion js unblocked by %s hook", runtimecatalog.HookName(runtimehooksv1.BeforeClusterDelete))) } } return ctrl.Result{}, nil diff --git a/internal/controllers/topology/cluster/cluster_controller_test.go b/internal/controllers/topology/cluster/cluster_controller_test.go index bd6a1d7bc672..93d1f9ad784d 100644 --- a/internal/controllers/topology/cluster/cluster_controller_test.go +++ b/internal/controllers/topology/cluster/cluster_controller_test.go @@ -454,7 +454,8 @@ func TestClusterReconciler_reconcileDelete(t *testing.T) { CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ RetryAfterSeconds: int32(10), CommonResponse: runtimehooksv1.CommonResponse{ - Status: runtimehooksv1.ResponseStatusSuccess, + Status: runtimehooksv1.ResponseStatusSuccess, + Message: "hook is blocking", }, }, } @@ -580,7 +581,10 @@ func TestClusterReconciler_reconcileDelete(t *testing.T) { tt.cluster.Annotations[conversion.DataAnnotation] = "should be cleaned up" fakeClient := fake.NewClientBuilder().WithObjects(tt.cluster).Build() - fakeRuntimeClient := fakeruntimeclient.NewRuntimeClientBuilder(). + fakeRuntimeClient := (fakeruntimeclient.NewRuntimeClientBuilder(). + WithGetAllExtensionResponses(map[runtimecatalog.GroupVersionHook][]string{ + beforeClusterDeleteGVH: {"foo"}, + })). WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{ beforeClusterDeleteGVH: tt.hookResponse, }). @@ -594,7 +598,9 @@ func TestClusterReconciler_reconcileDelete(t *testing.T) { RuntimeClient: fakeRuntimeClient, } - res, err := r.reconcileDelete(ctx, tt.cluster) + s := scope.New(tt.cluster) + + res, err := r.reconcileDelete(ctx, s) if tt.wantErr { g.Expect(err).To(HaveOccurred()) } else { @@ -604,6 +610,10 @@ func TestClusterReconciler_reconcileDelete(t *testing.T) { if tt.wantHookToBeCalled { g.Expect(fakeRuntimeClient.CallAllCount(runtimehooksv1.BeforeClusterDelete)).To(Equal(1), "Expected hook to be called once") + if !tt.wantOkToDelete { + g.Expect(s.HookResponseTracker.AggregateRetryAfter()).ToNot(BeZero()) + g.Expect(s.HookResponseTracker.AggregateMessage("delete")).To(Equal("Following hooks are blocking delete: BeforeClusterDelete: hook is blocking")) + } } else { g.Expect(fakeRuntimeClient.CallAllCount(runtimehooksv1.BeforeClusterDelete)).To(Equal(0), "Did not expect hook to be called") } @@ -756,6 +766,9 @@ func TestReconciler_callBeforeClusterCreateHook(t *testing.T) { runtimeClient := fakeruntimeclient.NewRuntimeClientBuilder(). WithCatalog(catalog). + WithGetAllExtensionResponses(map[runtimecatalog.GroupVersionHook][]string{ + gvh: {"foo"}, + }). WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{ gvh: tt.hookResponse, }). @@ -764,6 +777,7 @@ func TestReconciler_callBeforeClusterCreateHook(t *testing.T) { r := &Reconciler{ RuntimeClient: runtimeClient, + Client: fake.NewClientBuilder().WithScheme(fakeScheme).WithObjects(s.Current.Cluster).Build(), } res, err := r.callBeforeClusterCreateHook(ctx, s) if tt.wantErr { @@ -1754,6 +1768,7 @@ func validateClusterParameter(originalCluster *clusterv1.Cluster) func(req runti } originalClusterCopy := originalCluster.DeepCopy() + originalClusterCopy.TypeMeta = metav1.TypeMeta{} originalClusterCopy.SetManagedFields(nil) if originalClusterCopy.Annotations != nil { annotations := maps.Clone(cluster.Annotations) diff --git a/internal/controllers/topology/cluster/conditions.go b/internal/controllers/topology/cluster/conditions.go index 25858b4336d7..55295ebdbc25 100644 --- a/internal/controllers/topology/cluster/conditions.go +++ b/internal/controllers/topology/cluster/conditions.go @@ -18,15 +18,19 @@ package cluster import ( "fmt" + "sort" "strings" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" + runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1" "sigs.k8s.io/cluster-api/exp/topology/scope" "sigs.k8s.io/cluster-api/internal/contract" + "sigs.k8s.io/cluster-api/internal/hooks" "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/conditions" v1beta1conditions "sigs.k8s.io/cluster-api/util/conditions/deprecated/v1beta1" @@ -74,25 +78,6 @@ func (r *Reconciler) reconcileTopologyReconciledCondition(s *scope.Scope, cluste return nil } - // Mark TopologyReconciled as false due to cluster deletion. - if !cluster.DeletionTimestamp.IsZero() { - v1beta1conditions.Set(cluster, - v1beta1conditions.FalseCondition( - clusterv1.TopologyReconciledV1Beta1Condition, - clusterv1.DeletedV1Beta1Reason, - clusterv1.ConditionSeverityInfo, - "", - ), - ) - conditions.Set(cluster, metav1.Condition{ - Type: clusterv1.ClusterTopologyReconciledCondition, - Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterTopologyReconciledDeletingReason, - Message: "Cluster is deleting", - }) - return nil - } - // If an error occurred during reconciliation set the TopologyReconciled condition to false. // Add the error message from the reconcile function to the message of the condition. if reconcileErr != nil { @@ -115,6 +100,29 @@ func (r *Reconciler) reconcileTopologyReconciledCondition(s *scope.Scope, cluste return nil } + // Mark TopologyReconciled as false due to cluster deletion. + if !cluster.DeletionTimestamp.IsZero() { + message := "Cluster is deleting" + if s.HookResponseTracker.AggregateRetryAfter() != 0 { + message += ". " + s.HookResponseTracker.AggregateMessage("delete") + } + v1beta1conditions.Set(cluster, + v1beta1conditions.FalseCondition( + clusterv1.TopologyReconciledV1Beta1Condition, + clusterv1.DeletingV1Beta1Reason, + clusterv1.ConditionSeverityInfo, + "%s", message, + ), + ) + conditions.Set(cluster, metav1.Condition{ + Type: clusterv1.ClusterTopologyReconciledCondition, + Status: metav1.ConditionFalse, + Reason: clusterv1.ClusterTopologyReconciledDeletingReason, + Message: message, + }) + return nil + } + // If the ClusterClass `metadata.Generation` doesn't match the `status.ObservedGeneration` requeue as the ClusterClass // is not up to date. if s.Blueprint != nil && s.Blueprint.ClusterClass != nil && @@ -138,122 +146,148 @@ func (r *Reconciler) reconcileTopologyReconciledCondition(s *scope.Scope, cluste return nil } - // If any of the lifecycle hooks are blocking any part of the reconciliation then topology - // is not considered as fully reconciled. - if s.HookResponseTracker.AggregateRetryAfter() != 0 { + // If the BeforeClusterCreate hook is blocking, reports it + if !s.Current.Cluster.Spec.InfrastructureRef.IsDefined() && !s.Current.Cluster.Spec.ControlPlaneRef.IsDefined() { + if s.HookResponseTracker.AggregateRetryAfter() != 0 { + v1beta1conditions.Set(cluster, + v1beta1conditions.FalseCondition( + clusterv1.TopologyReconciledV1Beta1Condition, + clusterv1.TopologyReconciledClusterCreatingV1Beta1Reason, + clusterv1.ConditionSeverityInfo, + "%s", s.HookResponseTracker.AggregateMessage("Cluster topology creation"), + ), + ) + conditions.Set(cluster, metav1.Condition{ + Type: clusterv1.ClusterTopologyReconciledCondition, + Status: metav1.ConditionFalse, + Reason: clusterv1.ClusterTopologyReconciledClusterCreatingReason, + Message: s.HookResponseTracker.AggregateMessage("Cluster topology creation"), + }) + return nil + } + + // Note: this should never happen, controlPlane and infrastructure ref should be set at the first reconcile of the topology controller if the hook is not blocking. v1beta1conditions.Set(cluster, - v1beta1conditions.FalseCondition( - clusterv1.TopologyReconciledV1Beta1Condition, - clusterv1.TopologyReconciledHookBlockingV1Beta1Reason, - clusterv1.ConditionSeverityInfo, - // TODO: Add a protection for messages continuously changing leading to Cluster object changes/reconcile. - "%s", s.HookResponseTracker.AggregateMessage(), - ), + v1beta1conditions.TrueCondition(clusterv1.TopologyReconciledV1Beta1Condition), ) conditions.Set(cluster, metav1.Condition{ Type: clusterv1.ClusterTopologyReconciledCondition, - Status: metav1.ConditionFalse, - Reason: clusterv1.ClusterTopologyReconciledHookBlockingReason, - // TODO: Add a protection for messages continuously changing leading to Cluster object changes/reconcile. - Message: s.HookResponseTracker.AggregateMessage(), + Status: metav1.ConditionTrue, + Reason: clusterv1.ClusterTopologyReconcileSucceededReason, }) return nil } - // The topology is not considered as fully reconciled if one of the following is true: - // * either the Control Plane or any of the MachineDeployments/MachinePools are still pending to pick up the new version - // (generally happens when upgrading the cluster) - // * when there are MachineDeployments/MachinePools for which the upgrade has been deferred - // * when new MachineDeployments/MachinePools are pending to be created - // (generally happens when upgrading the cluster) + // If Cluster is updating surface it. + // Note: intentionally checking all the signal about upgrade in progress to make sure to avoid edge cases. if s.UpgradeTracker.ControlPlane.IsPendingUpgrade || - s.UpgradeTracker.MachineDeployments.IsAnyPendingCreate() || + s.UpgradeTracker.ControlPlane.IsStartingUpgrade || + s.UpgradeTracker.ControlPlane.IsUpgrading || s.UpgradeTracker.MachineDeployments.IsAnyPendingUpgrade() || - s.UpgradeTracker.MachineDeployments.DeferredUpgrade() || - s.UpgradeTracker.MachinePools.IsAnyPendingCreate() || + s.UpgradeTracker.MachineDeployments.IsAnyUpgrading() || + s.UpgradeTracker.MachineDeployments.IsAnyUpgradeDeferred() || + s.UpgradeTracker.MachineDeployments.IsAnyPendingCreate() || s.UpgradeTracker.MachinePools.IsAnyPendingUpgrade() || - s.UpgradeTracker.MachinePools.DeferredUpgrade() { + s.UpgradeTracker.MachinePools.IsAnyUpgrading() || + s.UpgradeTracker.MachinePools.IsAnyUpgradeDeferred() || + s.UpgradeTracker.MachinePools.IsAnyPendingCreate() || + hooks.IsPending(runtimehooksv1.AfterClusterUpgrade, cluster) { + // Start building the condition message showing upgrade progress. msgBuilder := &strings.Builder{} - var reason string - var v1beta2Reason string - - // TODO(ykakarap): Evaluate potential improvements to building the condition. Multiple causes can trigger the - // condition to be false at the same time (Example: ControlPlane.IsPendingUpgrade and MachineDeployments.IsAnyPendingCreate can - // occur at the same time). Find better wording and `Reason` for the condition so that the condition can be rich - // with all the relevant information. - switch { - case s.UpgradeTracker.ControlPlane.IsPendingUpgrade: - fmt.Fprintf(msgBuilder, "Control plane rollout and upgrade to version %s on hold.", s.Blueprint.Topology.Version) - reason = clusterv1.TopologyReconciledControlPlaneUpgradePendingV1Beta1Reason - v1beta2Reason = clusterv1.ClusterTopologyReconciledControlPlaneUpgradePendingReason - case s.UpgradeTracker.MachineDeployments.IsAnyPendingUpgrade(): - fmt.Fprintf(msgBuilder, "MachineDeployment(s) %s rollout and upgrade to version %s on hold.", - computeNameList(s.UpgradeTracker.MachineDeployments.PendingUpgradeNames()), - s.Blueprint.Topology.Version, - ) - reason = clusterv1.TopologyReconciledMachineDeploymentsUpgradePendingV1Beta1Reason - v1beta2Reason = clusterv1.ClusterTopologyReconciledMachineDeploymentsUpgradePendingReason - case s.UpgradeTracker.MachineDeployments.IsAnyPendingCreate(): - fmt.Fprintf(msgBuilder, "MachineDeployment(s) for Topologies %s creation on hold.", - computeNameList(s.UpgradeTracker.MachineDeployments.PendingCreateTopologyNames()), - ) - reason = clusterv1.TopologyReconciledMachineDeploymentsCreatePendingV1Beta1Reason - v1beta2Reason = clusterv1.ClusterTopologyReconciledMachineDeploymentsCreatePendingReason - case s.UpgradeTracker.MachineDeployments.DeferredUpgrade(): - fmt.Fprintf(msgBuilder, "MachineDeployment(s) %s rollout and upgrade to version %s deferred.", - computeNameList(s.UpgradeTracker.MachineDeployments.DeferredUpgradeNames()), - s.Blueprint.Topology.Version, - ) - reason = clusterv1.TopologyReconciledMachineDeploymentsUpgradeDeferredV1Beta1Reason - v1beta2Reason = clusterv1.ClusterTopologyReconciledMachineDeploymentsUpgradeDeferredReason - case s.UpgradeTracker.MachinePools.IsAnyPendingUpgrade(): - fmt.Fprintf(msgBuilder, "MachinePool(s) %s rollout and upgrade to version %s on hold.", - computeNameList(s.UpgradeTracker.MachinePools.PendingUpgradeNames()), - s.Blueprint.Topology.Version, - ) - reason = clusterv1.TopologyReconciledMachinePoolsUpgradePendingV1Beta1Reason - v1beta2Reason = clusterv1.ClusterTopologyReconciledMachinePoolsUpgradePendingReason - case s.UpgradeTracker.MachinePools.IsAnyPendingCreate(): - fmt.Fprintf(msgBuilder, "MachinePool(s) for Topologies %s creation on hold.", - computeNameList(s.UpgradeTracker.MachinePools.PendingCreateTopologyNames()), - ) - reason = clusterv1.TopologyReconciledMachinePoolsCreatePendingV1Beta1Reason - v1beta2Reason = clusterv1.ClusterTopologyReconciledMachinePoolsCreatePendingReason - case s.UpgradeTracker.MachinePools.DeferredUpgrade(): - fmt.Fprintf(msgBuilder, "MachinePool(s) %s rollout and upgrade to version %s deferred.", - computeNameList(s.UpgradeTracker.MachinePools.DeferredUpgradeNames()), - s.Blueprint.Topology.Version, - ) - reason = clusterv1.TopologyReconciledMachinePoolsUpgradeDeferredV1Beta1Reason - v1beta2Reason = clusterv1.ClusterTopologyReconciledMachinePoolsUpgradeDeferredReason + fmt.Fprintf(msgBuilder, "Cluster is upgrading to %s", cluster.Spec.Topology.Version) + + // Setting condition reasons showing upgrade is progress; this will be overridden only + // when users are blocking upgrade to make further progress, e.g. deferred upgrades + reason := clusterv1.ClusterTopologyReconciledClusterUpgradingReason + v1Beta1Reason := clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason + + cpVersion, err := contract.ControlPlane().Version().Get(s.Current.ControlPlane.Object) + if err != nil { + return errors.Wrap(err, "failed to get control plane spec version") } - switch { - case s.UpgradeTracker.ControlPlane.IsProvisioning: - msgBuilder.WriteString(" Control plane is completing initial provisioning") + // If any of the lifecycle hooks are blocking the upgrade surface it as a first detail. + if s.HookResponseTracker.IsAnyBlocking() { + fmt.Fprintf(msgBuilder, "\n * %s", s.HookResponseTracker.AggregateMessage("upgrade")) + } - case s.UpgradeTracker.ControlPlane.IsUpgrading: - cpVersion, err := contract.ControlPlane().Version().Get(s.Current.ControlPlane.Object) - if err != nil { - return errors.Wrap(err, "failed to get control plane spec version") + // If control plane is upgrading surface it, otherwise surface the pending upgrade plan. + if s.UpgradeTracker.ControlPlane.IsStartingUpgrade || s.UpgradeTracker.ControlPlane.IsUpgrading { + fmt.Fprintf(msgBuilder, "\n * %s upgrading to version %s", s.Current.ControlPlane.Object.GetKind(), *cpVersion) + if len(s.UpgradeTracker.ControlPlane.UpgradePlan) > 0 { + fmt.Fprintf(msgBuilder, " (%s pending)", strings.Join(s.UpgradeTracker.ControlPlane.UpgradePlan, ", ")) } - fmt.Fprintf(msgBuilder, " Control plane is upgrading to version %s", *cpVersion) + } else if len(s.UpgradeTracker.ControlPlane.UpgradePlan) > 0 { + fmt.Fprintf(msgBuilder, "\n * %s pending upgrade to version %s", s.Current.ControlPlane.Object.GetKind(), strings.Join(s.UpgradeTracker.ControlPlane.UpgradePlan, ", ")) + } - case len(s.UpgradeTracker.MachineDeployments.UpgradingNames()) > 0: - fmt.Fprintf(msgBuilder, " MachineDeployment(s) %s are upgrading", - computeNameList(s.UpgradeTracker.MachineDeployments.UpgradingNames()), - ) + // If MachineDeployments are upgrading surface it, if MachineDeployments are pending upgrades then surface the upgrade plans. + upgradingMachineDeploymentNames, pendingMachineDeploymentNames, deferredMachineDeploymentNames := dedupNames(s.UpgradeTracker.MachineDeployments) + if len(upgradingMachineDeploymentNames) > 0 { + fmt.Fprintf(msgBuilder, "\n * %s upgrading to version %s", nameList("MachineDeployment", "MachineDeployments", upgradingMachineDeploymentNames), *cpVersion) + if len(s.UpgradeTracker.ControlPlane.UpgradePlan) > 0 { + fmt.Fprintf(msgBuilder, " (%s pending)", strings.Join(s.UpgradeTracker.MachineDeployments.UpgradePlan, ", ")) + } + } - case len(s.UpgradeTracker.MachinePools.UpgradingNames()) > 0: - fmt.Fprintf(msgBuilder, " MachinePool(s) %s are upgrading", - computeNameList(s.UpgradeTracker.MachinePools.UpgradingNames()), - ) + if len(pendingMachineDeploymentNames) > 0 && len(s.UpgradeTracker.MachineDeployments.UpgradePlan) > 0 { + fmt.Fprintf(msgBuilder, "\n * %s pending upgrade to version %s", nameList("MachineDeployment", "MachineDeployments", pendingMachineDeploymentNames), strings.Join(s.UpgradeTracker.MachineDeployments.UpgradePlan, ", ")) + } + + // If MachineDeployments has been deferred or put on hold, surface it. + if len(deferredMachineDeploymentNames) > 0 { + fmt.Fprintf(msgBuilder, "\n * %s upgrade to version %s deferred using defer-upgrade or hold-upgrade-sequence annotations", nameList("MachineDeployment", "MachineDeployments", deferredMachineDeploymentNames), *cpVersion) + // If Deferred upgrades are blocking an upgrade, surface it. + // Note: Hook blocking takes the precedence on this signal. + if !s.HookResponseTracker.IsAnyBlocking() && + (!s.UpgradeTracker.ControlPlane.IsStartingUpgrade && !s.UpgradeTracker.ControlPlane.IsUpgrading) && + !s.UpgradeTracker.MachineDeployments.IsAnyUpgrading() && len(pendingMachineDeploymentNames) == 0 { + reason = clusterv1.ClusterTopologyReconciledMachineDeploymentsUpgradeDeferredReason + v1Beta1Reason = clusterv1.TopologyReconciledMachineDeploymentsUpgradeDeferredV1Beta1Reason + } + } + + // If creation of MachineDeployments has been deferred due to control plane upgrade in progress, surface it. + if s.UpgradeTracker.MachineDeployments.IsAnyPendingCreate() { + fmt.Fprintf(msgBuilder, "\n * %s creation deferred while control plane upgrade is in progress", nameList("MachineDeployment", "MachineDeployments", s.UpgradeTracker.MachineDeployments.PendingCreateTopologyNames())) + } + + // If MachinePools are upgrading surface it, if MachinePools are pending upgrades then surface the upgrade plans. + upgradingMachinePoolNames, pendingMachinePoolNames, deferredMachinePoolNames := dedupNames(s.UpgradeTracker.MachinePools) + if len(upgradingMachinePoolNames) > 0 { + fmt.Fprintf(msgBuilder, "\n * %s upgrading to version %s", nameList("MachinePool", "MachinePools", upgradingMachinePoolNames), *cpVersion) + if len(s.UpgradeTracker.ControlPlane.UpgradePlan) > 0 { + fmt.Fprintf(msgBuilder, " (%s pending)", strings.Join(s.UpgradeTracker.MachinePools.UpgradePlan, ", ")) + } + } + + if len(pendingMachinePoolNames) > 0 && len(s.UpgradeTracker.MachinePools.UpgradePlan) > 0 { + fmt.Fprintf(msgBuilder, "\n * %s pending upgrade to version %s", nameList("MachinePool", "MachinePools", pendingMachinePoolNames), strings.Join(s.UpgradeTracker.MachinePools.UpgradePlan, ", ")) + } + + // If MachinePools has been deferred or put on hold, surface it. + if len(deferredMachinePoolNames) > 0 { + fmt.Fprintf(msgBuilder, "\n * %s upgrade to version %s deferred using topology.cluster.x-k8s.io/defer-upgrade or hold-upgrade-sequence annotations", nameList("MachinePool", "MachinePools", deferredMachinePoolNames), *cpVersion) + // If Deferred upgrades are blocking an upgrade, surface it. + // Note: Hook blocking takes the precedence on this signal. + if !s.HookResponseTracker.IsAnyBlocking() && + (!s.UpgradeTracker.ControlPlane.IsStartingUpgrade && !s.UpgradeTracker.ControlPlane.IsUpgrading) && + !s.UpgradeTracker.MachinePools.IsAnyUpgrading() && len(pendingMachinePoolNames) == 0 && + reason != clusterv1.ClusterTopologyReconciledMachineDeploymentsUpgradeDeferredReason { + reason = clusterv1.ClusterTopologyReconciledMachinePoolsUpgradeDeferredReason + v1Beta1Reason = clusterv1.TopologyReconciledMachinePoolsUpgradeDeferredV1Beta1Reason + } + } + + // If creation of MachinePools has been deferred due to control plane upgrade in progress, surface it. + if s.UpgradeTracker.MachinePools.IsAnyPendingCreate() { + fmt.Fprintf(msgBuilder, "\n * %s creation deferred while control plane upgrade is in progress", nameList("MachinePool", "MachinePools", s.UpgradeTracker.MachinePools.PendingCreateTopologyNames())) } v1beta1conditions.Set(cluster, v1beta1conditions.FalseCondition( clusterv1.TopologyReconciledV1Beta1Condition, - reason, + v1Beta1Reason, clusterv1.ConditionSeverityInfo, "%s", msgBuilder.String(), ), @@ -261,7 +295,7 @@ func (r *Reconciler) reconcileTopologyReconciledCondition(s *scope.Scope, cluste conditions.Set(cluster, metav1.Condition{ Type: clusterv1.ClusterTopologyReconciledCondition, Status: metav1.ConditionFalse, - Reason: v1beta2Reason, + Reason: reason, Message: msgBuilder.String(), }) return nil @@ -281,13 +315,39 @@ func (r *Reconciler) reconcileTopologyReconciledCondition(s *scope.Scope, cluste return nil } +// dedupNames take care of names that might exist in multiple lists. +func dedupNames(t scope.WorkerUpgradeTracker) ([]string, []string, []string) { + // upgrading names are preserved + upgradingSet := sets.Set[string]{}.Insert(t.UpgradingNames()...) + // upgrading names are removed from deferred names (give precedence to the fact that it is upgrading now) + deferredSet := sets.Set[string]{}.Insert(t.DeferredUpgradeNames()...).Difference(upgradingSet) + // upgrading and deferred names are removed from pending names (it is pending if not upgrading or deferred) + pendingSet := sets.Set[string]{}.Insert(t.PendingUpgradeNames()...).Difference(upgradingSet).Difference(deferredSet) + return upgradingSet.UnsortedList(), pendingSet.UnsortedList(), deferredSet.UnsortedList() +} + // computeNameList computes list of names from the given list to be shown in conditions. // It shortens the list to at most 5 names and adds an ellipsis at the end if the list -// has more than 5 elements. +// has more than 3 elements. func computeNameList(list []string) any { - if len(list) > 5 { - list = append(list[:5], "...") + if len(list) > 3 { + list = append(list[:3], "...") } return strings.Join(list, ", ") } + +// nameList computes list of names from the given list to be shown in conditions. +// It shortens the list to at most 5 names and adds an ellipsis at the end if the list +// has more than 3 elements. +func nameList(kind, kindPlural string, names []string) any { + sort.Strings(names) + switch { + case len(names) == 1: + return fmt.Sprintf("%s %s", kind, names[0]) + case len(names) <= 3: + return fmt.Sprintf("%s %s", kindPlural, strings.Join(names, ", ")) + default: + return fmt.Sprintf("%s %s, ... (%d more)", kindPlural, strings.Join(names[:3], ", "), len(names)-3) + } +} diff --git a/internal/controllers/topology/cluster/conditions_test.go b/internal/controllers/topology/cluster/conditions_test.go index b937dde217c2..ff3e27d7a912 100644 --- a/internal/controllers/topology/cluster/conditions_test.go +++ b/internal/controllers/topology/cluster/conditions_test.go @@ -32,6 +32,7 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1" + runtimev1 "sigs.k8s.io/cluster-api/api/runtime/v1beta2" "sigs.k8s.io/cluster-api/exp/topology/scope" "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/util/conditions" @@ -49,32 +50,165 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) { name string reconcileErr error s *scope.Scope - cluster *clusterv1.Cluster machines []*clusterv1.Machine - wantConditionStatus corev1.ConditionStatus + wantV1Beta1ConditionStatus corev1.ConditionStatus + wantV1Beta1ConditionReason string + wantV1Beta1ConditionMessage string + wantConditionStatus metav1.ConditionStatus wantConditionReason string wantConditionMessage string - wantV1Beta2ConditionStatus metav1.ConditionStatus - wantV1Beta2ConditionReason string - wantV1Beta2ConditionMessage string wantErr bool }{ + // Reconcile error + { - name: "should set the condition to false if there is a reconcile error", - reconcileErr: errors.New("reconcile error"), - cluster: &clusterv1.Cluster{}, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconcileFailedV1Beta1Reason, + name: "should set the condition to false if there is a reconcile error", + reconcileErr: errors.New("reconcile error"), + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{}, + }, + }, + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconcileFailedV1Beta1Reason, + wantV1Beta1ConditionMessage: "reconcile error", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledFailedReason, wantConditionMessage: "reconcile error", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledFailedReason, - wantV1Beta2ConditionMessage: "reconcile error", wantErr: false, }, + + // Paused + + { + name: "should set the TopologyReconciledCondition to False if spec.paused is set to true", + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Paused: ptr.To(true), + }, + }, + }, + }, + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledPausedV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster spec.paused is set to true", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconcilePausedReason, + wantConditionMessage: "Cluster spec.paused is set to true", + }, { - name: "should set the condition to false if the ClusterClass is out of date", - cluster: &clusterv1.Cluster{}, + name: "should set the TopologyReconciledCondition to False if cluster.x-k8s.io/paused annotation is set to true", s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.PausedAnnotation: "", + }, + }, + }, + }, + }, + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledPausedV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster has the cluster.x-k8s.io/paused annotation", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconcilePausedReason, + wantConditionMessage: "Cluster has the cluster.x-k8s.io/paused annotation", + }, + { + name: "should set the TopologyReconciledCondition to False if spec.paused is set to true and if cluster.x-k8s.io/paused annotation is set to true", + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.PausedAnnotation: "", + }, + }, + Spec: clusterv1.ClusterSpec{ + Paused: ptr.To(true), + }, + }, + }, + }, + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledPausedV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster spec.paused is set to true, Cluster has the cluster.x-k8s.io/paused annotation", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconcilePausedReason, + wantConditionMessage: "Cluster spec.paused is set to true, Cluster has the cluster.x-k8s.io/paused annotation", + }, + + // Delete + { + name: "should set the TopologyReconciledCondition to False if the cluster has been deleted but the BeforeClusterDelete hook is blocking", + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + DeletionTimestamp: &deletionTime, + }, + }, + }, + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.BeforeClusterDelete, &runtimehooksv1.BeforeClusterDeleteResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{ + Message: "msg", + }, + RetryAfterSeconds: 10, + }, + }) + return hrt + }(), + }, + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.DeletingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is deleting. Following hooks are blocking delete: BeforeClusterDelete: msg", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledDeletingReason, + wantConditionMessage: "Cluster is deleting. Following hooks are blocking delete: BeforeClusterDelete: msg", + }, + { + name: "should set the TopologyReconciledCondition to False if the cluster has been deleted", + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + DeletionTimestamp: &deletionTime, + }, + }, + }, + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.BeforeClusterDelete, &runtimehooksv1.BeforeClusterDeleteResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{}, + }, + }) + return hrt + }(), + }, + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.DeletingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is deleting", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledDeletingReason, + wantConditionMessage: "Cluster is deleting", + }, + + // Cluster Class out of date + + { + name: "should set the condition to false if the ClusterClass is out of date", + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{}, + }, Blueprint: &scope.ClusterBlueprint{ ClusterClass: &clusterv1.ClusterClass{ ObjectMeta: metav1.ObjectMeta{ @@ -87,33 +221,97 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) { }, }, }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledClusterClassNotReconciledV1Beta1Reason, - wantConditionMessage: "ClusterClass not reconciled. If this condition persists please check ClusterClass status. A ClusterClass is reconciled if" + + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterClassNotReconciledV1Beta1Reason, + wantV1Beta1ConditionMessage: "ClusterClass not reconciled. If this condition persists please check ClusterClass status. A ClusterClass is reconciled if" + ".status.observedGeneration == .metadata.generation is true. If this is not the case either ClusterClass reconciliation failed or the ClusterClass is paused", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledClusterClassNotReconciledReason, - wantV1Beta2ConditionMessage: "ClusterClass not reconciled. If this condition persists please check ClusterClass status. A ClusterClass is reconciled if" + + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterClassNotReconciledReason, + wantConditionMessage: "ClusterClass not reconciled. If this condition persists please check ClusterClass status. A ClusterClass is reconciled if" + ".status.observedGeneration == .metadata.generation is true. If this is not the case either ClusterClass reconciliation failed or the ClusterClass is paused", wantErr: false, }, + + // BeforeClusterCreate hook is blocking + { - name: "should set the condition to false if the there is a blocking annotation hook", + name: "should set the condition to false if BeforeClusterCreate hook is blocking", reconcileErr: nil, - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test": "true", + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, }, }, + UpgradeTracker: scope.NewUpgradeTracker(), + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.BeforeClusterCreate, &runtimehooksv1.BeforeClusterCreateResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{ + Message: "msg", + }, + RetryAfterSeconds: 10, + }, + }) + return hrt + }(), }, + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterCreatingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Following hooks are blocking Cluster topology creation: BeforeClusterCreate: msg", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterCreatingReason, + wantConditionMessage: "Following hooks are blocking Cluster topology creation: BeforeClusterCreate: msg", + }, + + // Upgrade + + // First upgrade step (CP only, MD are skipping this step) + { + name: "should set the condition to false if control plane is pending upgrade and BeforeClusterUpgrade hook is blocking via an annotation", + reconcileErr: nil, s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test": "true", + }, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.20.5").Build(), + }, + }, + UpgradeTracker: func() *scope.UpgradeTracker { + ut := scope.NewUpgradeTracker() + ut.ControlPlane.IsPendingUpgrade = true + ut.ControlPlane.UpgradePlan = []string{"v1.21.2", "v1.22.0"} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") + return ut + }(), HookResponseTracker: func() *scope.HookResponseTracker { hrt := scope.NewHookResponseTracker() hrt.Add(runtimehooksv1.BeforeClusterUpgrade, &runtimehooksv1.BeforeClusterUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ - Message: fmt.Sprintf("annotation [%s] is set", clusterv1.BeforeClusterUpgradeHookAnnotationPrefix+"/test"), + Message: fmt.Sprintf("annotation %s is set", clusterv1.BeforeClusterUpgradeHookAnnotationPrefix+"/test"), }, RetryAfterSeconds: int32(20 * 60), }, @@ -121,53 +319,106 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) { return hrt }(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledHookBlockingV1Beta1Reason, - wantConditionMessage: "hook \"BeforeClusterUpgrade\" is blocking: annotation [" + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test] is set", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledHookBlockingReason, - wantV1Beta2ConditionMessage: "hook \"BeforeClusterUpgrade\" is blocking: annotation [" + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test] is set", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeClusterUpgrade: annotation before-upgrade.hook.cluster.cluster.x-k8s.io/test is set\n" + + " * GenericControlPlane pending upgrade to version v1.21.2, v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeClusterUpgrade: annotation before-upgrade.hook.cluster.cluster.x-k8s.io/test is set\n" + + " * GenericControlPlane pending upgrade to version v1.21.2, v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to false if the there are multiple blocking annotation hooks", + name: "should set the condition to false if control plane is pending upgrade and BeforeClusterUpgrade hook is blocking", reconcileErr: nil, - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test": "true", - clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test2": "true", + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.20.5").Build(), }, }, - }, - s: &scope.Scope{ + UpgradeTracker: func() *scope.UpgradeTracker { + ut := scope.NewUpgradeTracker() + ut.ControlPlane.IsPendingUpgrade = true + ut.ControlPlane.UpgradePlan = []string{"v1.21.2", "v1.22.0"} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") + return ut + }(), HookResponseTracker: func() *scope.HookResponseTracker { hrt := scope.NewHookResponseTracker() hrt.Add(runtimehooksv1.BeforeClusterUpgrade, &runtimehooksv1.BeforeClusterUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ - Message: "annotations [" + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test, " + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test2] are set", + Message: "msg", }, - RetryAfterSeconds: 20 * 60, + RetryAfterSeconds: 10, }, }) return hrt }(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledHookBlockingV1Beta1Reason, - wantConditionMessage: "hook \"BeforeClusterUpgrade\" is blocking: annotations [" + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test, " + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test2] are set", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledHookBlockingReason, - wantV1Beta2ConditionMessage: "hook \"BeforeClusterUpgrade\" is blocking: annotations [" + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test, " + clusterv1.BeforeClusterUpgradeHookAnnotationPrefix + "/test2] are set", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeClusterUpgrade: msg\n" + + " * GenericControlPlane pending upgrade to version v1.21.2, v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeClusterUpgrade: msg\n" + + " * GenericControlPlane pending upgrade to version v1.21.2, v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to false if the there is a blocking hook", + name: "should set the condition to false if control plane is pending upgrade and BeforeControlPlaneUpgrade hook is blocking (first upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.20.5").Build(), + }, + }, + UpgradeTracker: func() *scope.UpgradeTracker { + ut := scope.NewUpgradeTracker() + ut.ControlPlane.IsPendingUpgrade = true + ut.ControlPlane.UpgradePlan = []string{"v1.21.2", "v1.22.0"} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") + return ut + }(), HookResponseTracker: func() *scope.HookResponseTracker { hrt := scope.NewHookResponseTracker() - hrt.Add(runtimehooksv1.BeforeClusterUpgrade, &runtimehooksv1.BeforeClusterUpgradeResponse{ + hrt.Add(runtimehooksv1.BeforeControlPlaneUpgrade, &runtimehooksv1.BeforeClusterUpgradeResponse{ CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ CommonResponse: runtimehooksv1.CommonResponse{ Message: "msg", @@ -178,735 +429,698 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) { return hrt }(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledHookBlockingV1Beta1Reason, - wantConditionMessage: "hook \"BeforeClusterUpgrade\" is blocking: msg", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledHookBlockingReason, - wantV1Beta2ConditionMessage: "hook \"BeforeClusterUpgrade\" is blocking: msg", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeControlPlaneUpgrade: msg\n" + + " * GenericControlPlane pending upgrade to version v1.21.2, v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeControlPlaneUpgrade: msg\n" + + " * GenericControlPlane pending upgrade to version v1.21.2, v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to false if new version is not picked up because control plane is provisioning", + name: "should set the condition to false if control plane is starting upgrade (first upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.21.2"). - Build(), + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.21.2").Build(), }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() ut.ControlPlane.IsPendingUpgrade = true - ut.ControlPlane.IsProvisioning = true + ut.ControlPlane.IsStartingUpgrade = true + ut.ControlPlane.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), HookResponseTracker: scope.NewHookResponseTracker(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledControlPlaneUpgradePendingV1Beta1Reason, - wantConditionMessage: "Control plane rollout and upgrade to version v1.22.0 on hold. Control plane is completing initial provisioning", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledControlPlaneUpgradePendingReason, - wantV1Beta2ConditionMessage: "Control plane rollout and upgrade to version v1.22.0 on hold. Control plane is completing initial provisioning", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.21.2 (v1.22.0 pending)\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.21.2 (v1.22.0 pending)\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to false if new version is not picked up because control plane is upgrading", + name: "should set the condition to false if control plane is upgrading (first upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.21.2"). - WithReplicas(3). - Build(), + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.21.2").Build(), }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() ut.ControlPlane.IsPendingUpgrade = true ut.ControlPlane.IsUpgrading = true + ut.ControlPlane.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), HookResponseTracker: scope.NewHookResponseTracker(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledControlPlaneUpgradePendingV1Beta1Reason, - wantConditionMessage: "Control plane rollout and upgrade to version v1.22.0 on hold. Control plane is upgrading to version v1.21.2", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledControlPlaneUpgradePendingReason, - wantV1Beta2ConditionMessage: "Control plane rollout and upgrade to version v1.22.0 on hold. Control plane is upgrading to version v1.21.2", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.21.2 (v1.22.0 pending)\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.21.2 (v1.22.0 pending)\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to false if new version is not picked up because at least one of the machine deployment is upgrading", + name: "should set the condition to false if control plane is upgraded and AfterControlPlaneUpgrade hook is blocking (first upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.21.2"). - WithReplicas(3). - Build(), - }, - MachineDeployments: scope.MachineDeploymentsStateMap{ - "md0": &scope.MachineDeploymentState{ - Object: builder.MachineDeployment("ns1", "md0-abc123"). - WithReplicas(2). - WithStatus(clusterv1.MachineDeploymentStatus{ - Replicas: ptr.To[int32](1), - ReadyReplicas: ptr.To[int32](1), - UpToDateReplicas: ptr.To[int32](1), - AvailableReplicas: ptr.To[int32](1), - }). - Build(), + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, }, }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.21.2").Build(), + }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() ut.ControlPlane.IsPendingUpgrade = true - ut.MachineDeployments.MarkUpgrading("md0-abc123") + ut.ControlPlane.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), - HookResponseTracker: scope.NewHookResponseTracker(), + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.AfterControlPlaneUpgrade, &runtimehooksv1.AfterControlPlaneUpgradeResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{ + Message: "msg", + }, + RetryAfterSeconds: 10, + }, + }) + return hrt + }(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledControlPlaneUpgradePendingV1Beta1Reason, - wantConditionMessage: "Control plane rollout and upgrade to version v1.22.0 on hold. MachineDeployment(s) md0-abc123 are upgrading", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledControlPlaneUpgradePendingReason, - wantV1Beta2ConditionMessage: "Control plane rollout and upgrade to version v1.22.0 on hold. MachineDeployment(s) md0-abc123 are upgrading", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: AfterControlPlaneUpgrade: msg\n" + + " * GenericControlPlane pending upgrade to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: AfterControlPlaneUpgrade: msg\n" + + " * GenericControlPlane pending upgrade to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, + // Second upgrade step (CP and MachineDeployments) { - name: "should set the condition to false if new version is not picked up because at least one of the machine pool is upgrading", + name: "should set the condition to false if control plane is upgrading and BeforeControlPlaneUpgrade hook is blocking (second upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.21.2"). - WithReplicas(3). - Build(), - }, - MachinePools: scope.MachinePoolsStateMap{ - "mp0": &scope.MachinePoolState{ - Object: builder.MachinePool("ns1", "mp0-abc123"). - WithReplicas(2). - WithStatus(clusterv1.MachinePoolStatus{ - Replicas: ptr.To(int32(1)), - Deprecated: &clusterv1.MachinePoolDeprecatedStatus{ - V1Beta1: &clusterv1.MachinePoolV1Beta1DeprecatedStatus{ - ReadyReplicas: int32(1), - AvailableReplicas: int32(1), - UnavailableReplicas: int32(0), - }, - }, - }). - Build(), + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, }, }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.21.2").Build(), + }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() ut.ControlPlane.IsPendingUpgrade = true - ut.MachinePools.MarkUpgrading("mp0-abc123") + ut.ControlPlane.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), - HookResponseTracker: scope.NewHookResponseTracker(), + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.BeforeControlPlaneUpgrade, &runtimehooksv1.BeforeClusterUpgradeResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{ + Message: "msg", + }, + RetryAfterSeconds: 10, + }, + }) + return hrt + }(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledControlPlaneUpgradePendingV1Beta1Reason, - wantConditionMessage: "Control plane rollout and upgrade to version v1.22.0 on hold. MachinePool(s) mp0-abc123 are upgrading", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledControlPlaneUpgradePendingReason, - wantV1Beta2ConditionMessage: "Control plane rollout and upgrade to version v1.22.0 on hold. MachinePool(s) mp0-abc123 are upgrading", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeControlPlaneUpgrade: msg\n" + + " * GenericControlPlane pending upgrade to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeControlPlaneUpgrade: msg\n" + + " * GenericControlPlane pending upgrade to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to false if control plane picked the new version but machine deployments did not because control plane is upgrading", + name: "should set the condition to false if control plane is starting upgrade (second upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), - }, - MachineDeployments: scope.MachineDeploymentsStateMap{ - "md0": &scope.MachineDeploymentState{ - Object: builder.MachineDeployment("ns1", "md0-abc123"). - WithReplicas(2). - WithStatus(clusterv1.MachineDeploymentStatus{ - Replicas: ptr.To[int32](2), - ReadyReplicas: ptr.To[int32](2), - UpToDateReplicas: ptr.To[int32](2), - AvailableReplicas: ptr.To[int32](2), - }). - Build(), + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, }, }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), + }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false - ut.ControlPlane.IsUpgrading = true - ut.MachineDeployments.MarkPendingUpgrade("md0-abc123") + ut.ControlPlane.IsStartingUpgrade = true + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), HookResponseTracker: scope.NewHookResponseTracker(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledMachineDeploymentsUpgradePendingV1Beta1Reason, - wantConditionMessage: "MachineDeployment(s) md0-abc123 rollout and upgrade to version v1.22.0 on hold. Control plane is upgrading to version v1.22.0", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledMachineDeploymentsUpgradePendingReason, - wantV1Beta2ConditionMessage: "MachineDeployment(s) md0-abc123 rollout and upgrade to version v1.22.0 on hold. Control plane is upgrading to version v1.22.0", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to false if control plane picked the new version but machine pools did not because control plane is upgrading", + name: "should set the condition to false if control plane is upgrading (second upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), - }, - MachinePools: scope.MachinePoolsStateMap{ - "mp0": &scope.MachinePoolState{ - Object: builder.MachinePool("ns1", "mp0-abc123"). - WithReplicas(2). - WithStatus(clusterv1.MachinePoolStatus{ - Replicas: ptr.To(int32(2)), - Deprecated: &clusterv1.MachinePoolDeprecatedStatus{ - V1Beta1: &clusterv1.MachinePoolV1Beta1DeprecatedStatus{ - ReadyReplicas: int32(2), - AvailableReplicas: int32(2), - UnavailableReplicas: int32(0), - }, - }, - }). - Build(), + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, }, }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), + }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false ut.ControlPlane.IsUpgrading = true - ut.MachinePools.MarkPendingUpgrade("mp0-abc123") + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), HookResponseTracker: scope.NewHookResponseTracker(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledMachinePoolsUpgradePendingV1Beta1Reason, - wantConditionMessage: "MachinePool(s) mp0-abc123 rollout and upgrade to version v1.22.0 on hold. Control plane is upgrading to version v1.22.0", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledMachinePoolsUpgradePendingReason, - wantV1Beta2ConditionMessage: "MachinePool(s) mp0-abc123 rollout and upgrade to version v1.22.0 on hold. Control plane is upgrading to version v1.22.0", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to true if control plane picked the new version and is upgrading but there are no machine deployments or machine pools", + name: "should set the condition to false if control plane is upgraded and AfterControlPlaneUpgrade hook is blocking (second upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false - ut.ControlPlane.IsUpgrading = true + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), - HookResponseTracker: scope.NewHookResponseTracker(), + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.AfterControlPlaneUpgrade, &runtimehooksv1.AfterControlPlaneUpgradeResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{ + Message: "msg", + }, + RetryAfterSeconds: 10, + }, + }) + return hrt + }(), }, - wantConditionStatus: corev1.ConditionTrue, - wantV1Beta2ConditionStatus: metav1.ConditionTrue, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconcileSucceededReason, - wantV1Beta2ConditionMessage: "", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: AfterControlPlaneUpgrade: msg\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: AfterControlPlaneUpgrade: msg\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to true if control plane picked the new version and is scaling but there are no machine deployments or machine pools", + name: "should set the condition to false if control plane is upgraded and BeforeWorkersUpgrade hook is blocking (second upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), - HookResponseTracker: scope.NewHookResponseTracker(), + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.BeforeWorkersUpgrade, &runtimehooksv1.BeforeWorkersUpgradeResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{ + Message: "msg", + }, + RetryAfterSeconds: 10, + }, + }) + return hrt + }(), }, - wantConditionStatus: corev1.ConditionTrue, - wantV1Beta2ConditionStatus: metav1.ConditionTrue, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconcileSucceededReason, - wantV1Beta2ConditionMessage: "", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeWorkersUpgrade: msg\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: BeforeWorkersUpgrade: msg\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0", }, { - name: "should set the condition to false is some machine deployments have not picked the new version because other machine deployments are upgrading", + name: "should set the condition to false if MachineDeployments are upgrading (second upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), - }, - MachineDeployments: scope.MachineDeploymentsStateMap{ - "md0": &scope.MachineDeploymentState{ - Object: builder.MachineDeployment("ns1", "md0-abc123"). - WithReplicas(2). - WithVersion("v1.22.0"). - WithSelector(metav1.LabelSelector{ - MatchLabels: map[string]string{ - clusterv1.ClusterTopologyMachineDeploymentNameLabel: "md0", - }, - }). - WithStatus(clusterv1.MachineDeploymentStatus{ - // MD is not ready because we don't have 2 updated, ready and available replicas. - Replicas: ptr.To[int32](2), - ReadyReplicas: ptr.To[int32](1), - UpToDateReplicas: ptr.To[int32](1), - AvailableReplicas: ptr.To[int32](1), - }). - Build(), - }, - "md1": &scope.MachineDeploymentState{ - Object: builder.MachineDeployment("ns1", "md1-abc123"). - WithReplicas(2). - WithVersion("v1.21.2"). - WithSelector(metav1.LabelSelector{ - MatchLabels: map[string]string{ - clusterv1.ClusterTopologyMachineDeploymentNameLabel: "md1", - }, - }). - WithStatus(clusterv1.MachineDeploymentStatus{ - Replicas: ptr.To[int32](2), - ReadyReplicas: ptr.To[int32](2), - UpToDateReplicas: ptr.To[int32](2), - AvailableReplicas: ptr.To[int32](2), - }). - Build(), + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, }, }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), + }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false - ut.MachineDeployments.MarkUpgrading("md0-abc123") - ut.MachineDeployments.MarkPendingUpgrade("md1-abc123") + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkUpgrading("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), HookResponseTracker: scope.NewHookResponseTracker(), }, - machines: []*clusterv1.Machine{ - builder.Machine("ns1", "md0-machine0"). - WithLabels(map[string]string{clusterv1.ClusterTopologyMachineDeploymentNameLabel: "md0"}). - WithVersion("v1.21.2"). // Machine's version does not match MachineDeployment's version - Build(), - builder.Machine("ns1", "md1-machine0"). - WithLabels(map[string]string{clusterv1.ClusterTopologyMachineDeploymentNameLabel: "md1"}). - WithVersion("v1.21.2"). - Build(), - }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledMachineDeploymentsUpgradePendingV1Beta1Reason, - wantConditionMessage: "MachineDeployment(s) md1-abc123 rollout and upgrade to version v1.22.0 on hold. MachineDeployment(s) md0-abc123 are upgrading", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledMachineDeploymentsUpgradePendingReason, - wantV1Beta2ConditionMessage: "MachineDeployment(s) md1-abc123 rollout and upgrade to version v1.22.0 on hold. MachineDeployment(s) md0-abc123 are upgrading", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md1 upgrading to version v1.22.0\n" + + " * MachineDeployments md2, md3, md4 pending upgrade to version v1.22.0", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md1 upgrading to version v1.22.0\n" + + " * MachineDeployments md2, md3, md4 pending upgrade to version v1.22.0", }, { - name: "should set the condition to false is some machine pools have not picked the new version because other machine pools are upgrading", + name: "should set the condition to false if MachineDeployments are upgraded and AfterWorkersUpgrade hook is blocking (second upgrade step)", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), - }, - MachinePools: scope.MachinePoolsStateMap{ - "mp0": &scope.MachinePoolState{ - Object: builder.MachinePool("ns1", "mp0-abc123"). - WithReplicas(2). - WithVersion("v1.22.0"). - WithStatus(clusterv1.MachinePoolStatus{ - // mp is not ready because we don't have 2 updated, ready and available replicas. - Replicas: ptr.To(int32(2)), - Deprecated: &clusterv1.MachinePoolDeprecatedStatus{ - V1Beta1: &clusterv1.MachinePoolV1Beta1DeprecatedStatus{ - ReadyReplicas: int32(1), - AvailableReplicas: int32(1), - UnavailableReplicas: int32(0), - }, - }, - }). - Build(), - }, - "mp1": &scope.MachinePoolState{ - Object: builder.MachinePool("ns1", "mp1-abc123"). - WithReplicas(2). - WithVersion("v1.21.2"). - WithStatus(clusterv1.MachinePoolStatus{ - Replicas: ptr.To(int32(2)), - Deprecated: &clusterv1.MachinePoolDeprecatedStatus{ - V1Beta1: &clusterv1.MachinePoolV1Beta1DeprecatedStatus{ - ReadyReplicas: int32(2), - AvailableReplicas: int32(2), - UnavailableReplicas: int32(0), - }, - }, - }). - Build(), + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + runtimev1.PendingHooksAnnotation: "AfterClusterUpgrade", + }, }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false - ut.MachinePools.MarkUpgrading("mp0-abc123") - ut.MachinePools.MarkPendingUpgrade("mp1-abc123") + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{} return ut }(), - HookResponseTracker: scope.NewHookResponseTracker(), - }, - machines: []*clusterv1.Machine{ - builder.Machine("ns1", "mp0-machine0"). - WithLabels(map[string]string{clusterv1.ClusterTopologyMachinePoolNameLabel: "mp0"}). - WithVersion("v1.21.2"). // Machine's version does not match MachinePool's version - Build(), - builder.Machine("ns1", "mp1-machine0"). - WithLabels(map[string]string{clusterv1.ClusterTopologyMachinePoolNameLabel: "mp1"}). - WithVersion("v1.21.2"). - Build(), + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.AfterWorkersUpgrade, &runtimehooksv1.AfterWorkersUpgradeResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{ + Message: "msg", + }, + RetryAfterSeconds: 10, + }, + }) + return hrt + }(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledMachinePoolsUpgradePendingV1Beta1Reason, - wantConditionMessage: "MachinePool(s) mp1-abc123 rollout and upgrade to version v1.22.0 on hold. MachinePool(s) mp0-abc123 are upgrading", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledMachinePoolsUpgradePendingReason, - wantV1Beta2ConditionMessage: "MachinePool(s) mp1-abc123 rollout and upgrade to version v1.22.0 on hold. MachinePool(s) mp0-abc123 are upgrading", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: AfterWorkersUpgrade: msg", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: AfterWorkersUpgrade: msg", }, + // TODO(chained): uncomment this as soon as AfterClusterUpgrade will be blocking + /* + { + name: "should set the condition to false if AfterClusterUpgrade hook is blocking", + reconcileErr: nil, + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + runtimev1.PendingHooksAnnotation: "AfterClusterUpgrade", + }, + }, + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), + }, + }, + UpgradeTracker: scope.NewUpgradeTracker(), + HookResponseTracker: func() *scope.HookResponseTracker { + hrt := scope.NewHookResponseTracker() + hrt.Add(runtimehooksv1.AfterClusterUpgrade, &runtimehooksv1.AfterClusterUpgradeResponse{ + CommonRetryResponse: runtimehooksv1.CommonRetryResponse{ + CommonResponse: runtimehooksv1.CommonResponse{ + Message: "msg", + }, + RetryAfterSeconds: 10, + }, + }) + return hrt + }(), + }, + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: AfterClusterUpgrade: msg", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * Following hooks are blocking upgrade: AfterClusterUpgrade: msg", + }, + */ + + // Hold & defer upgrade + { - name: "should set the condition to false if some machine deployments have not picked the new version because their upgrade has been deferred", + name: "should report deferred MD upgrades", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), - }, - MachineDeployments: scope.MachineDeploymentsStateMap{ - "md0": &scope.MachineDeploymentState{ - Object: builder.MachineDeployment("ns1", "md0-abc123"). - WithReplicas(2). - WithVersion("v1.22.0"). - WithStatus(clusterv1.MachineDeploymentStatus{ - Replicas: ptr.To[int32](2), - ReadyReplicas: ptr.To[int32](2), - UpToDateReplicas: ptr.To[int32](2), - AvailableReplicas: ptr.To[int32](2), - }). - Build(), - }, - "md1": &scope.MachineDeploymentState{ - Object: builder.MachineDeployment("ns1", "md1-abc123"). - WithReplicas(2). - WithVersion("v1.21.2"). - WithStatus(clusterv1.MachineDeploymentStatus{ - Replicas: ptr.To[int32](2), - ReadyReplicas: ptr.To[int32](2), - UpToDateReplicas: ptr.To[int32](2), - AvailableReplicas: ptr.To[int32](2), - }). - Build(), + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, }, }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), + }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false - ut.MachineDeployments.MarkDeferredUpgrade("md1-abc123") + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkDeferredUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") return ut }(), HookResponseTracker: scope.NewHookResponseTracker(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledMachineDeploymentsUpgradeDeferredV1Beta1Reason, - wantConditionMessage: "MachineDeployment(s) md1-abc123 rollout and upgrade to version v1.22.0 deferred.", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledMachineDeploymentsUpgradeDeferredReason, - wantV1Beta2ConditionMessage: "MachineDeployment(s) md1-abc123 rollout and upgrade to version v1.22.0 deferred.", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md4 pending upgrade to version v1.22.0\n" + + " * MachineDeployment md3 upgrade to version v1.22.0 deferred using defer-upgrade or hold-upgrade-sequence annotations", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md4 pending upgrade to version v1.22.0\n" + + " * MachineDeployment md3 upgrade to version v1.22.0 deferred using defer-upgrade or hold-upgrade-sequence annotations", }, { - name: "should set the condition to false if some machine pools have not picked the new version because their upgrade has been deferred", + name: "should report when deferred MD upgrades are blocking further progress", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), - }, - MachinePools: scope.MachinePoolsStateMap{ - "mp0": &scope.MachinePoolState{ - Object: builder.MachinePool("ns1", "mp0-abc123"). - WithReplicas(2). - WithVersion("v1.22.0"). - WithStatus(clusterv1.MachinePoolStatus{ - Replicas: ptr.To(int32(2)), - Deprecated: &clusterv1.MachinePoolDeprecatedStatus{ - V1Beta1: &clusterv1.MachinePoolV1Beta1DeprecatedStatus{ - ReadyReplicas: int32(2), - AvailableReplicas: int32(2), - UnavailableReplicas: int32(0), - }, - }, - }). - Build(), - }, - "mp1": &scope.MachinePoolState{ - Object: builder.MachinePool("ns1", "mp1-abc123"). - WithReplicas(2). - WithVersion("v1.21.2"). - WithStatus(clusterv1.MachinePoolStatus{ - Replicas: ptr.To(int32(2)), - Deprecated: &clusterv1.MachinePoolDeprecatedStatus{ - V1Beta1: &clusterv1.MachinePoolV1Beta1DeprecatedStatus{ - ReadyReplicas: int32(2), - AvailableReplicas: int32(2), - UnavailableReplicas: int32(0), - }, - }, - }). - Build(), + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, }, }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), + }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false - ut.MachinePools.MarkDeferredUpgrade("mp1-abc123") + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkDeferredUpgrade("md2") return ut }(), HookResponseTracker: scope.NewHookResponseTracker(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.TopologyReconciledMachinePoolsUpgradeDeferredV1Beta1Reason, - wantConditionMessage: "MachinePool(s) mp1-abc123 rollout and upgrade to version v1.22.0 deferred.", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledMachinePoolsUpgradeDeferredReason, - wantV1Beta2ConditionMessage: "MachinePool(s) mp1-abc123 rollout and upgrade to version v1.22.0 deferred.", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledMachineDeploymentsUpgradeDeferredV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md2 upgrade to version v1.22.0 deferred using defer-upgrade or hold-upgrade-sequence annotations", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledMachineDeploymentsUpgradeDeferredReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * MachineDeployment md2 upgrade to version v1.22.0 deferred using defer-upgrade or hold-upgrade-sequence annotations", }, + + // Create deferred { - name: "should set the condition to true if there are no reconcile errors and control plane and all machine deployments and machine pools picked up the new version", + name: "should report MachineDeployment creation deferred while CP is upgrading", reconcileErr: nil, - cluster: &clusterv1.Cluster{}, s: &scope.Scope{ - Blueprint: &scope.ClusterBlueprint{ - Topology: clusterv1.Topology{ - Version: "v1.22.0", - }, - }, Current: &scope.ClusterState{ - Cluster: &clusterv1.Cluster{}, - ControlPlane: &scope.ControlPlaneState{ - Object: builder.ControlPlane("ns1", "controlplane1"). - WithVersion("v1.22.0"). - WithReplicas(3). - Build(), - }, - MachineDeployments: scope.MachineDeploymentsStateMap{ - "md0": &scope.MachineDeploymentState{ - Object: builder.MachineDeployment("ns1", "md0-abc123"). - WithReplicas(2). - WithVersion("v1.22.0"). - WithStatus(clusterv1.MachineDeploymentStatus{ - Replicas: ptr.To[int32](1), - ReadyReplicas: ptr.To[int32](1), - UpToDateReplicas: ptr.To[int32](1), - AvailableReplicas: ptr.To[int32](1), - }). - Build(), - }, - "md1": &scope.MachineDeploymentState{ - Object: builder.MachineDeployment("ns1", "md1-abc123"). - WithReplicas(2). - WithVersion("v1.22.0"). - WithStatus(clusterv1.MachineDeploymentStatus{ - Replicas: ptr.To[int32](2), - ReadyReplicas: ptr.To[int32](2), - UpToDateReplicas: ptr.To[int32](2), - AvailableReplicas: ptr.To[int32](2), - }). - Build(), - }, - }, - MachinePools: scope.MachinePoolsStateMap{ - "mp0": &scope.MachinePoolState{ - Object: builder.MachinePool("ns1", "mp0-abc123"). - WithReplicas(2). - WithVersion("v1.22.0"). - WithStatus(clusterv1.MachinePoolStatus{ - Replicas: ptr.To(int32(1)), - Deprecated: &clusterv1.MachinePoolDeprecatedStatus{ - V1Beta1: &clusterv1.MachinePoolV1Beta1DeprecatedStatus{ - ReadyReplicas: int32(1), - AvailableReplicas: int32(1), - UnavailableReplicas: int32(0), - }, - }, - }). - Build(), - }, - "mp1": &scope.MachinePoolState{ - Object: builder.MachinePool("ns1", "mp1-abc123"). - WithReplicas(2). - WithVersion("v1.22.0"). - WithStatus(clusterv1.MachinePoolStatus{ - Replicas: ptr.To(int32(2)), - Deprecated: &clusterv1.MachinePoolDeprecatedStatus{ - V1Beta1: &clusterv1.MachinePoolV1Beta1DeprecatedStatus{ - ReadyReplicas: int32(2), - AvailableReplicas: int32(2), - UnavailableReplicas: int32(0), - }, - }, - }). - Build(), + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, }, }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), + }, }, UpgradeTracker: func() *scope.UpgradeTracker { ut := scope.NewUpgradeTracker() - ut.ControlPlane.IsPendingUpgrade = false + ut.ControlPlane.IsUpgrading = true + ut.ControlPlane.UpgradePlan = []string{} + ut.MachineDeployments.UpgradePlan = []string{"v1.22.0"} + ut.MachineDeployments.MarkPendingUpgrade("md1") + ut.MachineDeployments.MarkPendingUpgrade("md2") + ut.MachineDeployments.MarkPendingUpgrade("md3") + ut.MachineDeployments.MarkPendingUpgrade("md4") + ut.MachineDeployments.MarkPendingCreate("md5") return ut }(), HookResponseTracker: scope.NewHookResponseTracker(), }, - wantConditionStatus: corev1.ConditionTrue, - wantV1Beta2ConditionStatus: metav1.ConditionTrue, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconcileSucceededReason, - wantV1Beta2ConditionMessage: "", + wantV1Beta1ConditionStatus: corev1.ConditionFalse, + wantV1Beta1ConditionReason: clusterv1.TopologyReconciledClusterUpgradingV1Beta1Reason, + wantV1Beta1ConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0\n" + + " * MachineDeployment md5 creation deferred while control plane upgrade is in progress", + wantConditionStatus: metav1.ConditionFalse, + wantConditionReason: clusterv1.ClusterTopologyReconciledClusterUpgradingReason, + wantConditionMessage: "Cluster is upgrading to v1.22.0\n" + + " * GenericControlPlane upgrading to version v1.22.0\n" + + " * MachineDeployments md1, md2, md3, ... (1 more) pending upgrade to version v1.22.0\n" + + " * MachineDeployment md5 creation deferred while control plane upgrade is in progress", }, + { - name: "should set the TopologyReconciledCondition to False if the cluster has been deleted", - cluster: &clusterv1.Cluster{ - ObjectMeta: metav1.ObjectMeta{ - DeletionTimestamp: &deletionTime, + name: "should set the condition to true if cluster is not upgrading", + reconcileErr: nil, + s: &scope.Scope{ + Current: &scope.ClusterState{ + Cluster: &clusterv1.Cluster{ + Spec: clusterv1.ClusterSpec{ + ControlPlaneRef: clusterv1.ContractVersionedObjectReference{Name: "controlplane1"}, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{Name: "infra1"}, + Topology: clusterv1.Topology{ + Version: "v1.22.0", + }, + }, + }, + ControlPlane: &scope.ControlPlaneState{ + Object: builder.ControlPlane("ns1", "controlplane1").WithVersion("v1.22.0").Build(), + }, }, + UpgradeTracker: scope.NewUpgradeTracker(), + HookResponseTracker: scope.NewHookResponseTracker(), }, - wantConditionStatus: corev1.ConditionFalse, - wantConditionReason: clusterv1.DeletedV1Beta1Reason, - wantConditionMessage: "", - wantV1Beta2ConditionStatus: metav1.ConditionFalse, - wantV1Beta2ConditionReason: clusterv1.ClusterTopologyReconciledDeletingReason, - wantV1Beta2ConditionMessage: "Cluster is deleting", + wantV1Beta1ConditionStatus: corev1.ConditionTrue, + wantConditionStatus: metav1.ConditionTrue, + wantConditionReason: clusterv1.ClusterTopologyReconcileSucceededReason, + wantConditionMessage: "", }, } @@ -931,23 +1145,23 @@ func TestReconcileTopologyReconciledCondition(t *testing.T) { fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() r := &Reconciler{Client: fakeClient} - err := r.reconcileTopologyReconciledCondition(tt.s, tt.cluster, tt.reconcileErr) + err := r.reconcileTopologyReconciledCondition(tt.s, tt.s.Current.Cluster, tt.reconcileErr) if tt.wantErr { g.Expect(err).To(HaveOccurred()) } else { g.Expect(err).ToNot(HaveOccurred()) - actualV1Beta1Condition := v1beta1conditions.Get(tt.cluster, clusterv1.TopologyReconciledV1Beta1Condition) + actualV1Beta1Condition := v1beta1conditions.Get(tt.s.Current.Cluster, clusterv1.TopologyReconciledV1Beta1Condition) g.Expect(actualV1Beta1Condition).ToNot(BeNil()) - g.Expect(actualV1Beta1Condition.Status).To(BeEquivalentTo(tt.wantConditionStatus)) - g.Expect(actualV1Beta1Condition.Reason).To(BeEquivalentTo(tt.wantConditionReason)) - g.Expect(actualV1Beta1Condition.Message).To(BeEquivalentTo(tt.wantConditionMessage)) + g.Expect(actualV1Beta1Condition.Status).To(BeEquivalentTo(tt.wantV1Beta1ConditionStatus)) + g.Expect(actualV1Beta1Condition.Reason).To(BeEquivalentTo(tt.wantV1Beta1ConditionReason)) + g.Expect(actualV1Beta1Condition.Message).To(BeEquivalentTo(tt.wantV1Beta1ConditionMessage)) - actualCondition := conditions.Get(tt.cluster, clusterv1.ClusterTopologyReconciledCondition) + actualCondition := conditions.Get(tt.s.Current.Cluster, clusterv1.ClusterTopologyReconciledCondition) g.Expect(actualCondition).ToNot(BeNil()) - g.Expect(actualCondition.Status).To(BeEquivalentTo(tt.wantV1Beta2ConditionStatus)) - g.Expect(actualCondition.Reason).To(BeEquivalentTo(tt.wantV1Beta2ConditionReason)) - g.Expect(actualCondition.Message).To(BeEquivalentTo(tt.wantV1Beta2ConditionMessage)) + g.Expect(actualCondition.Status).To(BeEquivalentTo(tt.wantConditionStatus)) + g.Expect(actualCondition.Reason).To(BeEquivalentTo(tt.wantConditionReason)) + g.Expect(actualCondition.Message).To(BeEquivalentTo(tt.wantConditionMessage)) } }) } @@ -960,19 +1174,19 @@ func TestComputeNameList(t *testing.T) { expected string }{ { - name: "mdList with 4 names", - mdList: []string{"md-1", "md-2", "md-3", "md-4"}, - expected: "md-1, md-2, md-3, md-4", + name: "mdList with 2 names", + mdList: []string{"md-1", "md-2"}, + expected: "md-1, md-2", }, { - name: "mdList with 5 names", - mdList: []string{"md-1", "md-2", "md-3", "md-4", "md-5"}, - expected: "md-1, md-2, md-3, md-4, md-5", + name: "mdList with 3 names", + mdList: []string{"md-1", "md-2", "md-3"}, + expected: "md-1, md-2, md-3", }, { - name: "mdList with 6 names is shortened", - mdList: []string{"md-1", "md-2", "md-3", "md-4", "md-5", "md-6"}, - expected: "md-1, md-2, md-3, md-4, md-5, ...", + name: "mdList with 4 names is shortened", + mdList: []string{"md-1", "md-2", "md-3", "md-4"}, + expected: "md-1, md-2, md-3, ...", }, } diff --git a/internal/controllers/topology/cluster/reconcile_state.go b/internal/controllers/topology/cluster/reconcile_state.go index 4ea82cb9b042..d51bac00bb9b 100644 --- a/internal/controllers/topology/cluster/reconcile_state.go +++ b/internal/controllers/topology/cluster/reconcile_state.go @@ -37,6 +37,7 @@ import ( clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1" + runtimecatalog "sigs.k8s.io/cluster-api/exp/runtime/catalog" "sigs.k8s.io/cluster-api/exp/topology/scope" "sigs.k8s.io/cluster-api/feature" "sigs.k8s.io/cluster-api/internal/contract" @@ -231,6 +232,8 @@ func isControlPlaneInitialized(cluster *clusterv1.Cluster) bool { } func (r *Reconciler) callAfterClusterUpgrade(ctx context.Context, s *scope.Scope) error { + log := ctrl.LoggerFrom(ctx) + // Call the hook only if we are tracking the intent to do so. If it is not tracked it means we don't need to call the // hook because we didn't go through an upgrade or we already called the hook after the upgrade. // Note: also check that the AfterControlPlaneUpgrade hooks and the AfterWorkersUpgrade hooks already have been called. @@ -242,16 +245,25 @@ func (r *Reconciler) callAfterClusterUpgrade(ctx context.Context, s *scope.Scope // - MachineDeployments/MachinePools are not pending an upgrade // - MachineDeployments/MachinePools are not pending create if s.UpgradeTracker.ControlPlane.IsControlPlaneStable() && // Control Plane stable checks - len(s.UpgradeTracker.MachineDeployments.UpgradingNames()) == 0 && // Machine deployments are not upgrading or not about to upgrade + !s.UpgradeTracker.MachineDeployments.IsAnyUpgrading() && // Machine deployments are not upgrading or not about to upgrade !s.UpgradeTracker.MachineDeployments.IsAnyPendingCreate() && // No MachineDeployments are pending create !s.UpgradeTracker.MachineDeployments.IsAnyPendingUpgrade() && // No MachineDeployments are pending an upgrade - !s.UpgradeTracker.MachineDeployments.DeferredUpgrade() && // No MachineDeployments have deferred an upgrade - len(s.UpgradeTracker.MachinePools.UpgradingNames()) == 0 && // Machine pools are not upgrading or not about to upgrade + !s.UpgradeTracker.MachineDeployments.IsAnyUpgradeDeferred() && // No MachineDeployments have deferred an upgrade + !s.UpgradeTracker.MachinePools.IsAnyUpgrading() && // Machine pools are not upgrading or not about to upgrade !s.UpgradeTracker.MachinePools.IsAnyPendingCreate() && // No MachinePools are pending create !s.UpgradeTracker.MachinePools.IsAnyPendingUpgrade() && // No MachinePools are pending an upgrade - !s.UpgradeTracker.MachinePools.DeferredUpgrade() { // No MachinePools have deferred an upgrade - v1beta1Cluster := &clusterv1beta1.Cluster{} + !s.UpgradeTracker.MachinePools.IsAnyUpgradeDeferred() { // No MachinePools have deferred an upgrade + // Return quickly if the hook is not defined. + extensionHandlers, err := r.RuntimeClient.GetAllExtensions(ctx, runtimehooksv1.AfterClusterUpgrade, s.Current.Cluster) + if err != nil { + return err + } + if len(extensionHandlers) == 0 { + return hooks.MarkAsDone(ctx, r.Client, s.Current.Cluster, runtimehooksv1.AfterClusterUpgrade) + } + // DeepCopy cluster because ConvertFrom has side effects like adding the conversion annotation. + v1beta1Cluster := &clusterv1beta1.Cluster{} if err := v1beta1Cluster.ConvertFrom(s.Current.Cluster.DeepCopy()); err != nil { return errors.Wrap(err, "error converting Cluster to v1beta1 Cluster") } @@ -266,10 +278,22 @@ func (r *Reconciler) callAfterClusterUpgrade(ctx context.Context, s *scope.Scope return err } s.HookResponseTracker.Add(runtimehooksv1.AfterClusterUpgrade, hookResponse) + + // TODO(chained): uncomment this as soon as AfterClusterUpgrade will be blocking + //nolint:gocritic + /* + if hookResponse.RetryAfterSeconds != 0 { + log.Info(fmt.Sprintf("Cluster upgrade to version %s completed but %s hook is still blocking", hookRequest.KubernetesVersion, runtimecatalog.HookName(runtimehooksv1.AfterClusterUpgrade))) + return nil + } + */ + // The hook is successfully called; we can remove this hook from the list of pending-hooks. if err := hooks.MarkAsDone(ctx, r.Client, s.Current.Cluster, runtimehooksv1.AfterClusterUpgrade); err != nil { return err } + + log.Info(fmt.Sprintf("Cluster upgrade to version %s and %s hook completed", hookRequest.KubernetesVersion, runtimecatalog.HookName(runtimehooksv1.AfterClusterUpgrade))) } } diff --git a/internal/controllers/topology/cluster/reconcile_state_test.go b/internal/controllers/topology/cluster/reconcile_state_test.go index f9efff40ba8c..3af582261841 100644 --- a/internal/controllers/topology/cluster/reconcile_state_test.go +++ b/internal/controllers/topology/cluster/reconcile_state_test.go @@ -1232,6 +1232,9 @@ func TestReconcile_callAfterClusterUpgrade(t *testing.T) { tt.s.Current.Cluster.Annotations[conversion.DataAnnotation] = "should be cleaned up" fakeRuntimeClient := fakeruntimeclient.NewRuntimeClientBuilder(). + WithGetAllExtensionResponses(map[runtimecatalog.GroupVersionHook][]string{ + afterClusterUpgradeGVH: {"foo"}, + }). WithCallAllExtensionResponses(map[runtimecatalog.GroupVersionHook]runtimehooksv1.ResponseObject{ afterClusterUpgradeGVH: tt.hookResponse, }). diff --git a/test/e2e/cluster_upgrade_runtimesdk.go b/test/e2e/cluster_upgrade_runtimesdk.go index d18c411ff5da..113daca1f34e 100644 --- a/test/e2e/cluster_upgrade_runtimesdk.go +++ b/test/e2e/cluster_upgrade_runtimesdk.go @@ -1074,21 +1074,14 @@ func beforeClusterDeleteHandler(ctx context.Context, c client.Client, cluster *c func annotationHookTestHandler(ctx context.Context, c client.Client, cluster *clusterv1.Cluster, hook, annotation string, blockingCondition func() bool) { log.Logf("Blocking with the %s annotation hook for 60 seconds", hook) - expectedBlockingMessage := fmt.Sprintf("hook %q is blocking: annotation [%s] is set", hook, annotation) + expectedBlockingMessage := fmt.Sprintf("annotation %s is set", annotation) // Check if TopologyReconciledCondition reports if the annotation hook is blocking topologyConditionCheck := func() bool { cluster = framework.GetClusterByName(ctx, framework.GetClusterByNameInput{ Name: cluster.Name, Namespace: cluster.Namespace, Getter: c}) - if conditions.GetReason(cluster, clusterv1.ClusterTopologyReconciledCondition) != clusterv1.ClusterTopologyReconciledHookBlockingReason { - return false - } - if !strings.Contains(conditions.GetMessage(cluster, clusterv1.ClusterTopologyReconciledCondition), expectedBlockingMessage) { - return false - } - - return true + return strings.Contains(conditions.GetMessage(cluster, clusterv1.ClusterTopologyReconciledCondition), expectedBlockingMessage) } Byf("Waiting for %s hook (via annotation %s) to start blocking", hook, annotation) @@ -1227,8 +1220,7 @@ func computeHookName(hook string, attributes []string) string { // clusterConditionShowsHookBlocking checks if the TopologyReconciled condition message contains both the hook name and hookFailedMessage. func clusterConditionShowsHookBlocking(cluster *clusterv1.Cluster, hookName string) bool { - return conditions.GetReason(cluster, clusterv1.ClusterTopologyReconciledCondition) == clusterv1.ClusterTopologyReconciledHookBlockingReason && - strings.Contains(conditions.GetMessage(cluster, clusterv1.ClusterTopologyReconciledCondition), hookName) + return strings.Contains(conditions.GetMessage(cluster, clusterv1.ClusterTopologyReconciledCondition), hookName) } func waitControlPlaneVersion(ctx context.Context, c client.Client, cluster *clusterv1.Cluster, version string, intervals []interface{}) {