diff --git a/api/v1beta1/machineset_types.go b/api/v1beta1/machineset_types.go index df85ea9a11af..8b9fc282d9ee 100644 --- a/api/v1beta1/machineset_types.go +++ b/api/v1beta1/machineset_types.go @@ -157,9 +157,23 @@ const ( const ( // MachineSetRemediatingV1Beta2Condition surfaces details about ongoing remediation of the controlled machines, if any. MachineSetRemediatingV1Beta2Condition = RemediatingV1Beta2Condition +) +// MachineSet's Deleting condition and corresponding reasons that will be used in v1Beta2 API version. +const ( // MachineSetDeletingV1Beta2Condition surfaces details about ongoing deletion of the controlled machines. MachineSetDeletingV1Beta2Condition = DeletingV1Beta2Condition + + // MachineSetDeletingDeletionTimestampNotSetV1Beta2Reason surfaces when the MachineSet is not deleting because the + // DeletionTimestamp is not set. + MachineSetDeletingDeletionTimestampNotSetV1Beta2Reason = DeletionTimestampNotSetV1Beta2Reason + + // MachineSetDeletingDeletionTimestampSetV1Beta2Reason surfaces when the MachineSet is deleting because the + // DeletionTimestamp is set. + MachineSetDeletingDeletionTimestampSetV1Beta2Reason = DeletionTimestampSetV1Beta2Reason + + // MachineSetDeletingInternalErrorV1Beta2Reason surfaces unexpected failures when deleting a MachineSet. + MachineSetDeletingInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason ) // ANCHOR_END: MachineSetSpec diff --git a/controlplane/kubeadm/api/v1beta1/v1beta2_condition_consts.go b/controlplane/kubeadm/api/v1beta1/v1beta2_condition_consts.go index c56ac98f4843..d3049763ae92 100644 --- a/controlplane/kubeadm/api/v1beta1/v1beta2_condition_consts.go +++ b/controlplane/kubeadm/api/v1beta1/v1beta2_condition_consts.go @@ -213,12 +213,27 @@ const ( const ( // KubeadmControlPlaneDeletingV1Beta2Condition surfaces details about ongoing deletion of the controlled machines. KubeadmControlPlaneDeletingV1Beta2Condition = clusterv1.DeletingV1Beta2Condition -) -// KubeadmControlPlane's Paused condition and corresponding reasons that will be used in v1Beta2 API version. -const ( - // KubeadmControlPlanePausedV1Beta2Condition is true if this resource or the Cluster it belongs to are paused. - KubeadmControlPlanePausedV1Beta2Condition = clusterv1.PausedV1Beta2Condition + // KubeadmControlPlaneDeletingDeletionTimestampNotSetV1Beta2Reason surfaces when the KCP is not deleting because the + // DeletionTimestamp is not set. + KubeadmControlPlaneDeletingDeletionTimestampNotSetV1Beta2Reason = clusterv1.DeletionTimestampNotSetV1Beta2Reason + + // KubeadmControlPlaneDeletingWaitingForWorkersDeletionV1Beta2Reason surfaces when the KCP deletion + // waits for the workers to be deleted. + KubeadmControlPlaneDeletingWaitingForWorkersDeletionV1Beta2Reason = "WaitingForWorkersDeletion" + + // KubeadmControlPlaneDeletingWaitingForMachineDeletionV1Beta2Reason surfaces when the KCP deletion + // waits for the control plane Machines to be deleted. + KubeadmControlPlaneDeletingWaitingForMachineDeletionV1Beta2Reason = "WaitingForMachineDeletion" + + // KubeadmControlPlaneDeletingDeletionCompletedV1Beta2Reason surfaces when the KCP deletion has been completed. + // This reason is set right after the `kubeadm.controlplane.cluster.x-k8s.io` finalizer is removed. + // This means that the object will go away (i.e. be removed from etcd), except if there are other + // finalizers on the KCP object. + KubeadmControlPlaneDeletingDeletionCompletedV1Beta2Reason = clusterv1.DeletionCompletedV1Beta2Reason + + // KubeadmControlPlaneDeletingInternalErrorV1Beta2Reason surfaces unexpected failures when deleting a KCP object. + KubeadmControlPlaneDeletingInternalErrorV1Beta2Reason = clusterv1.InternalErrorV1Beta2Reason ) // APIServerPodHealthy, ControllerManagerPodHealthy, SchedulerPodHealthy and EtcdPodHealthy condition and corresponding diff --git a/controlplane/kubeadm/internal/control_plane.go b/controlplane/kubeadm/internal/control_plane.go index 41b1ed8ffed0..7d624645305a 100644 --- a/controlplane/kubeadm/internal/control_plane.go +++ b/controlplane/kubeadm/internal/control_plane.go @@ -60,6 +60,12 @@ type ControlPlane struct { managementCluster ManagementCluster workloadCluster WorkloadCluster + + // deletingReason is the reason that should be used when setting the Deleting condition. + DeletingReason string + + // deletingMessage is the message that should be used when setting the Deleting condition. + DeletingMessage string } // PreflightCheckResults contains description about pre flight check results blocking machines creation or deletion. diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go index 891cc27ac9e3..f1ece2c75a90 100644 --- a/controlplane/kubeadm/internal/controllers/controller.go +++ b/controlplane/kubeadm/internal/controllers/controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "sort" "strings" "time" @@ -52,6 +53,7 @@ import ( "sigs.k8s.io/cluster-api/util/conditions" v1beta2conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2" "sigs.k8s.io/cluster-api/util/finalizers" + clog "sigs.k8s.io/cluster-api/util/log" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/cluster-api/util/paused" "sigs.k8s.io/cluster-api/util/predicates" @@ -220,7 +222,7 @@ func (r *KubeadmControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl. } } - r.updateV1beta2Status(ctx, controlPlane) + r.updateV1Beta2Status(ctx, controlPlane) // Always attempt to Patch the KubeadmControlPlane object and status after each reconciliation. patchOpts := []patch.Option{} @@ -596,6 +598,9 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, con // If no control plane machines remain, remove the finalizer if len(controlPlane.Machines) == 0 { + controlPlane.DeletingReason = controlplanev1.KubeadmControlPlaneDeletingDeletionCompletedV1Beta2Reason + controlPlane.DeletingMessage = "" + controllerutil.RemoveFinalizer(controlPlane.KCP, controlplanev1.KubeadmControlPlaneFinalizer) return ctrl.Result{}, nil } @@ -615,6 +620,8 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, con // Gets all machines, not just control plane machines. allMachines, err := r.managementCluster.GetMachinesForCluster(ctx, controlPlane.Cluster) if err != nil { + controlPlane.DeletingReason = controlplanev1.KubeadmControlPlaneDeletingInternalErrorV1Beta2Reason + controlPlane.DeletingMessage = "Please check controller logs for errors" //nolint:goconst // Not making this a constant for now return ctrl.Result{}, err } @@ -623,6 +630,8 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, con if feature.Gates.Enabled(feature.MachinePool) { allMachinePools, err = r.managementCluster.GetMachinePoolsForCluster(ctx, controlPlane.Cluster) if err != nil { + controlPlane.DeletingReason = controlplanev1.KubeadmControlPlaneDeletingInternalErrorV1Beta2Reason + controlPlane.DeletingMessage = "Please check controller logs for errors" return ctrl.Result{}, err } } @@ -630,13 +639,16 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, con if len(allMachines) != len(controlPlane.Machines) || len(allMachinePools.Items) != 0 { log.Info("Waiting for worker nodes to be deleted first") conditions.MarkFalse(controlPlane.KCP, controlplanev1.ResizedCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "Waiting for worker nodes to be deleted first") + + controlPlane.DeletingReason = controlplanev1.KubeadmControlPlaneDeletingWaitingForWorkersDeletionV1Beta2Reason + controlPlane.DeletingMessage = fmt.Sprintf("KCP deletion blocked because %s still exist", objectsPendingDeleteNames(allMachines, allMachinePools, controlPlane.Cluster)) return ctrl.Result{RequeueAfter: deleteRequeueAfter}, nil } // Delete control plane machines in parallel - machinesToDelete := controlPlane.Machines + machines := controlPlane.Machines var errs []error - for _, machineToDelete := range machinesToDelete { + for _, machineToDelete := range machines { log := log.WithValues("Machine", klog.KObj(machineToDelete)) ctx := ctrl.LoggerInto(ctx, log) @@ -665,15 +677,61 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, con err := kerrors.NewAggregate(errs) r.recorder.Eventf(controlPlane.KCP, corev1.EventTypeWarning, "FailedDelete", "Failed to delete control plane Machines for cluster %s control plane: %v", klog.KObj(controlPlane.Cluster), err) + + controlPlane.DeletingReason = controlplanev1.KubeadmControlPlaneDeletingInternalErrorV1Beta2Reason + controlPlane.DeletingMessage = "Please check controller logs for errors" return ctrl.Result{}, err } log.Info("Waiting for control plane Machines to not exist anymore") conditions.MarkFalse(controlPlane.KCP, controlplanev1.ResizedCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "") + + message := "" + if len(machines) > 0 { + if len(machines) == 1 { + message = fmt.Sprintf("Deleting %d Machine", len(machines)) + } else { + message = fmt.Sprintf("Deleting %d Machines", len(machines)) + } + staleMessage := aggregateStaleMachines(machines) + if staleMessage != "" { + message += fmt.Sprintf(" and %s", staleMessage) + } + } + controlPlane.DeletingReason = controlplanev1.KubeadmControlPlaneDeletingWaitingForMachineDeletionV1Beta2Reason + controlPlane.DeletingMessage = message return ctrl.Result{RequeueAfter: deleteRequeueAfter}, nil } +// objectsPendingDeleteNames return the names of worker Machines and MachinePools pending delete. +func objectsPendingDeleteNames(allMachines collections.Machines, allMachinePools *expv1.MachinePoolList, cluster *clusterv1.Cluster) string { + controlPlaneMachines := allMachines.Filter(collections.ControlPlaneMachines(cluster.Name)) + workerMachines := allMachines.Difference(controlPlaneMachines) + + descendants := make([]string, 0) + if feature.Gates.Enabled(feature.MachinePool) { + machinePoolNames := make([]string, len(allMachinePools.Items)) + for i, machinePool := range allMachinePools.Items { + machinePoolNames[i] = machinePool.Name + } + if len(machinePoolNames) > 0 { + sort.Strings(machinePoolNames) + descendants = append(descendants, "MachinePools: "+clog.StringListToString(machinePoolNames)) + } + } + + workerMachineNames := make([]string, len(workerMachines)) + for i, workerMachine := range workerMachines.UnsortedList() { + workerMachineNames[i] = workerMachine.Name + } + if len(workerMachineNames) > 0 { + sort.Strings(workerMachineNames) + descendants = append(descendants, "worker Machines: "+clog.StringListToString(workerMachineNames)) + } + return strings.Join(descendants, "; ") +} + func (r *KubeadmControlPlaneReconciler) removePreTerminateHookAnnotationFromMachine(ctx context.Context, machine *clusterv1.Machine) error { if _, exists := machine.Annotations[controlplanev1.PreTerminateHookCleanupAnnotation]; !exists { // Nothing to do, the annotation is not set (anymore) on the Machine diff --git a/controlplane/kubeadm/internal/controllers/controller_test.go b/controlplane/kubeadm/internal/controllers/controller_test.go index 29f20b500319..fe1e893e84fb 100644 --- a/controlplane/kubeadm/internal/controllers/controller_test.go +++ b/controlplane/kubeadm/internal/controllers/controller_test.go @@ -3041,6 +3041,8 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { g.Expect(result).To(Equal(ctrl.Result{RequeueAfter: deleteRequeueAfter})) g.Expect(err).ToNot(HaveOccurred()) g.Expect(kcp.Finalizers).To(ContainElement(controlplanev1.KubeadmControlPlaneFinalizer)) + g.Expect(controlPlane.DeletingReason).To(Equal(controlplanev1.KubeadmControlPlaneDeletingWaitingForMachineDeletionV1Beta2Reason)) + g.Expect(controlPlane.DeletingMessage).To(Equal("Deleting 3 Machines")) controlPlaneMachines := clusterv1.MachineList{} g.Expect(fakeClient.List(ctx, &controlPlaneMachines)).To(Succeed()) @@ -3067,6 +3069,8 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { g.Expect(result).To(BeComparableTo(ctrl.Result{})) g.Expect(err).ToNot(HaveOccurred()) g.Expect(kcp.Finalizers).To(BeEmpty()) + g.Expect(controlPlane.DeletingReason).To(Equal(controlplanev1.KubeadmControlPlaneDeletingDeletionCompletedV1Beta2Reason)) + g.Expect(controlPlane.DeletingMessage).To(BeEmpty()) }) t.Run("does not remove any control plane Machines if other Machines exist", func(t *testing.T) { @@ -3075,18 +3079,20 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { cluster, kcp, _ := createClusterWithControlPlane(metav1.NamespaceDefault) controllerutil.AddFinalizer(kcp, controlplanev1.KubeadmControlPlaneFinalizer) - workerMachine := &clusterv1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "worker", - Namespace: cluster.Namespace, - Labels: map[string]string{ - clusterv1.ClusterNameLabel: cluster.Name, + initObjs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy()} + + for i := range 10 { + initObjs = append(initObjs, &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("worker-%d", i), + Namespace: cluster.Namespace, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: cluster.Name, + }, }, - }, + }) } - initObjs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), workerMachine.DeepCopy()} - machines := collections.New() for i := range 3 { m, _ := createMachineNodePair(fmt.Sprintf("test-%d", i), cluster, kcp, true) @@ -3115,8 +3121,9 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { result, err := r.reconcileDelete(ctx, controlPlane) g.Expect(result).To(BeComparableTo(ctrl.Result{RequeueAfter: deleteRequeueAfter})) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(kcp.Finalizers).To(ContainElement(controlplanev1.KubeadmControlPlaneFinalizer)) + g.Expect(controlPlane.DeletingReason).To(Equal(controlplanev1.KubeadmControlPlaneDeletingWaitingForWorkersDeletionV1Beta2Reason)) + g.Expect(controlPlane.DeletingMessage).To(Equal("KCP deletion blocked because worker Machines: worker-0, worker-1, worker-2, worker-3, worker-4, ... (5 more) still exist")) controlPlaneMachines := clusterv1.MachineList{} labels := map[string]string{ @@ -3133,18 +3140,20 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { cluster, kcp, _ := createClusterWithControlPlane(metav1.NamespaceDefault) controllerutil.AddFinalizer(kcp, controlplanev1.KubeadmControlPlaneFinalizer) - workerMachinePool := &expv1.MachinePool{ - ObjectMeta: metav1.ObjectMeta{ - Name: "worker", - Namespace: cluster.Namespace, - Labels: map[string]string{ - clusterv1.ClusterNameLabel: cluster.Name, + initObjs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy()} + + for i := range 10 { + initObjs = append(initObjs, &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("mp-%d", i), + Namespace: cluster.Namespace, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: cluster.Name, + }, }, - }, + }) } - initObjs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), workerMachinePool.DeepCopy()} - machines := collections.New() for i := range 3 { m, _ := createMachineNodePair(fmt.Sprintf("test-%d", i), cluster, kcp, true) @@ -3173,8 +3182,9 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { result, err := r.reconcileDelete(ctx, controlPlane) g.Expect(result).To(BeComparableTo(ctrl.Result{RequeueAfter: deleteRequeueAfter})) g.Expect(err).ToNot(HaveOccurred()) - g.Expect(kcp.Finalizers).To(ContainElement(controlplanev1.KubeadmControlPlaneFinalizer)) + g.Expect(controlPlane.DeletingReason).To(Equal(controlplanev1.KubeadmControlPlaneDeletingWaitingForWorkersDeletionV1Beta2Reason)) + g.Expect(controlPlane.DeletingMessage).To(Equal("KCP deletion blocked because MachinePools: mp-0, mp-1, mp-2, mp-3, mp-4, ... (5 more) still exist")) controlPlaneMachines := clusterv1.MachineList{} labels := map[string]string{ @@ -3211,9 +3221,56 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) { g.Expect(result).To(BeComparableTo(ctrl.Result{})) g.Expect(err).ToNot(HaveOccurred()) g.Expect(kcp.Finalizers).To(BeEmpty()) + g.Expect(controlPlane.DeletingReason).To(Equal(controlplanev1.KubeadmControlPlaneDeletingDeletionCompletedV1Beta2Reason)) + g.Expect(controlPlane.DeletingMessage).To(BeEmpty()) }) } +func TestObjectsPendingDelete(t *testing.T) { + c := &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + }, + } + + cpMachineLabels := map[string]string{ + clusterv1.ClusterNameLabel: c.Name, + clusterv1.MachineControlPlaneLabel: "", + } + workerMachineLabels := map[string]string{ + clusterv1.ClusterNameLabel: c.Name, + } + + allMachines := collections.FromMachineList(&clusterv1.MachineList{ + Items: []clusterv1.Machine{ + *machine("cp1", withLabels(cpMachineLabels)), + *machine("cp2", withLabels(cpMachineLabels)), + *machine("cp3", withLabels(cpMachineLabels)), + *machine("w1", withLabels(workerMachineLabels)), + *machine("w2", withLabels(workerMachineLabels)), + *machine("w3", withLabels(workerMachineLabels)), + *machine("w4", withLabels(workerMachineLabels)), + *machine("w5", withLabels(workerMachineLabels)), + *machine("w6", withLabels(workerMachineLabels)), + *machine("w7", withLabels(workerMachineLabels)), + *machine("w8", withLabels(workerMachineLabels)), + }, + }) + machinePools := &expv1.MachinePoolList{ + Items: []expv1.MachinePool{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "mp1", + }, + }, + }, + } + + g := NewWithT(t) + + g.Expect(objectsPendingDeleteNames(allMachines, machinePools, c)).To(Equal("MachinePools: mp1; worker Machines: w1, w2, w3, w4, w5, ... (3 more)")) +} + // test utils. func newFakeClient(initObjs ...client.Object) client.Client { diff --git a/controlplane/kubeadm/internal/controllers/scale_test.go b/controlplane/kubeadm/internal/controllers/scale_test.go index b0b6fd7ad63d..f74b69439027 100644 --- a/controlplane/kubeadm/internal/controllers/scale_test.go +++ b/controlplane/kubeadm/internal/controllers/scale_test.go @@ -758,6 +758,12 @@ func withAnnotation(annotation string) machineOpt { } } +func withLabels(labels map[string]string) machineOpt { + return func(m *clusterv1.Machine) { + m.ObjectMeta.Labels = labels + } +} + func withTimestamp(t time.Time) machineOpt { return func(m *clusterv1.Machine) { m.CreationTimestamp = metav1.NewTime(t) diff --git a/controlplane/kubeadm/internal/controllers/status.go b/controlplane/kubeadm/internal/controllers/status.go index 5656e4cfd02b..2327d19f2cc5 100644 --- a/controlplane/kubeadm/internal/controllers/status.go +++ b/controlplane/kubeadm/internal/controllers/status.go @@ -145,9 +145,9 @@ func (r *KubeadmControlPlaneReconciler) updateStatus(ctx context.Context, contro return nil } -// updateV1beta2Status reconciles KubeadmControlPlane's status during the entire lifecycle of the object. +// updateV1Beta2Status reconciles KubeadmControlPlane's status during the entire lifecycle of the object. // Note: v1beta1 conditions and fields are not managed by this func. -func (r *KubeadmControlPlaneReconciler) updateV1beta2Status(ctx context.Context, controlPlane *internal.ControlPlane) { +func (r *KubeadmControlPlaneReconciler) updateV1Beta2Status(ctx context.Context, controlPlane *internal.ControlPlane) { // If the code failed initializing the control plane, do not update the status. if controlPlane == nil { return @@ -167,8 +167,8 @@ func (r *KubeadmControlPlaneReconciler) updateV1beta2Status(ctx context.Context, setMachinesReadyCondition(ctx, controlPlane.KCP, controlPlane.Machines) setMachinesUpToDateCondition(ctx, controlPlane.KCP, controlPlane.Machines) setRemediatingCondition(ctx, controlPlane.KCP, controlPlane.MachinesToBeRemediatedByKCP(), controlPlane.UnhealthyMachines()) - - // TODO: Available, Deleting + setDeletingCondition(ctx, controlPlane.KCP, controlPlane.DeletingReason, controlPlane.DeletingMessage) + // TODO: Available } func setReplicas(_ context.Context, kcp *controlplanev1.KubeadmControlPlane, machines collections.Machines) { @@ -423,6 +423,24 @@ func setRemediatingCondition(ctx context.Context, kcp *controlplanev1.KubeadmCon }) } +func setDeletingCondition(_ context.Context, kcp *controlplanev1.KubeadmControlPlane, deletingReason, deletingMessage string) { + if kcp.DeletionTimestamp.IsZero() { + v1beta2conditions.Set(kcp, metav1.Condition{ + Type: controlplanev1.KubeadmControlPlaneDeletingV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: controlplanev1.KubeadmControlPlaneDeletingDeletionTimestampNotSetV1Beta2Reason, + }) + return + } + + v1beta2conditions.Set(kcp, metav1.Condition{ + Type: controlplanev1.KubeadmControlPlaneDeletingV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: deletingReason, + Message: deletingMessage, + }) +} + func aggregateStaleMachines(machines collections.Machines) string { if len(machines) == 0 { return "" diff --git a/controlplane/kubeadm/internal/controllers/status_test.go b/controlplane/kubeadm/internal/controllers/status_test.go index d5ac66272b51..aee7b88a53d4 100644 --- a/controlplane/kubeadm/internal/controllers/status_test.go +++ b/controlplane/kubeadm/internal/controllers/status_test.go @@ -599,6 +599,63 @@ func Test_setRemediatingCondition(t *testing.T) { } } +func TestDeletingCondition(t *testing.T) { + testCases := []struct { + name string + kcp *controlplanev1.KubeadmControlPlane + deletingReason string + deletingMessage string + expectCondition metav1.Condition + }{ + { + name: "deletionTimestamp not set", + kcp: &controlplanev1.KubeadmControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kcp-test", + Namespace: metav1.NamespaceDefault, + }, + }, + deletingReason: "", + deletingMessage: "", + expectCondition: metav1.Condition{ + Type: controlplanev1.KubeadmControlPlaneDeletingV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: controlplanev1.KubeadmControlPlaneDeletingDeletionTimestampNotSetV1Beta2Reason, + }, + }, + { + name: "deletionTimestamp set (waiting for control plane Machine deletion)", + kcp: &controlplanev1.KubeadmControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kcp-test", + Namespace: metav1.NamespaceDefault, + DeletionTimestamp: &metav1.Time{Time: time.Now()}, + }, + }, + deletingReason: controlplanev1.KubeadmControlPlaneDeletingWaitingForMachineDeletionV1Beta2Reason, + deletingMessage: "Deleting 3 Machines", + expectCondition: metav1.Condition{ + Type: controlplanev1.KubeadmControlPlaneDeletingV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: controlplanev1.KubeadmControlPlaneDeletingWaitingForMachineDeletionV1Beta2Reason, + Message: "Deleting 3 Machines", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + setDeletingCondition(ctx, tc.kcp, tc.deletingReason, tc.deletingMessage) + + deletingCondition := v1beta2conditions.Get(tc.kcp, controlplanev1.KubeadmControlPlaneDeletingV1Beta2Condition) + g.Expect(deletingCondition).ToNot(BeNil()) + g.Expect(*deletingCondition).To(v1beta2conditions.MatchCondition(tc.expectCondition, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + } +} + func TestKubeadmControlPlaneReconciler_updateStatusNoMachines(t *testing.T) { g := NewWithT(t) diff --git a/internal/controllers/cluster/cluster_controller.go b/internal/controllers/cluster/cluster_controller.go index 3414b4597ada..de05d0e5f4ce 100644 --- a/internal/controllers/cluster/cluster_controller.go +++ b/internal/controllers/cluster/cluster_controller.go @@ -514,6 +514,7 @@ func (c *clusterDescendants) objectsPendingDeleteNames(cluster *clusterv1.Cluste machineDeploymentNames[i] = machineDeployment.Name } if len(machineDeploymentNames) > 0 { + sort.Strings(machineDeploymentNames) descendants = append(descendants, "MachineDeployments: "+clog.StringListToString(machineDeploymentNames)) } machineSetNames := make([]string, len(c.machineSets.Items)) @@ -521,6 +522,7 @@ func (c *clusterDescendants) objectsPendingDeleteNames(cluster *clusterv1.Cluste machineSetNames[i] = machineSet.Name } if len(machineSetNames) > 0 { + sort.Strings(machineSetNames) descendants = append(descendants, "MachineSets: "+clog.StringListToString(machineSetNames)) } if feature.Gates.Enabled(feature.MachinePool) { @@ -529,6 +531,7 @@ func (c *clusterDescendants) objectsPendingDeleteNames(cluster *clusterv1.Cluste machinePoolNames[i] = machinePool.Name } if len(machinePoolNames) > 0 { + sort.Strings(machinePoolNames) descendants = append(descendants, "MachinePools: "+clog.StringListToString(machinePoolNames)) } } diff --git a/internal/controllers/cluster/cluster_controller_test.go b/internal/controllers/cluster/cluster_controller_test.go index 52dacd2946a5..c4ad6a58416a 100644 --- a/internal/controllers/cluster/cluster_controller_test.go +++ b/internal/controllers/cluster/cluster_controller_test.go @@ -840,39 +840,42 @@ func TestFilterOwnedDescendants(t *testing.T) { } func TestObjectsPendingDelete(t *testing.T) { + // Note: Intentionally using random order to validate sorting. d := clusterDescendants{ machineDeployments: clusterv1.MachineDeploymentList{ Items: []clusterv1.MachineDeployment{ + newMachineDeploymentBuilder().named("md2").build(), newMachineDeploymentBuilder().named("md1").build(), }, }, machineSets: clusterv1.MachineSetList{ Items: []clusterv1.MachineSet{ - newMachineSetBuilder().named("ms1").build(), newMachineSetBuilder().named("ms2").build(), + newMachineSetBuilder().named("ms1").build(), }, }, controlPlaneMachines: collections.FromMachineList(&clusterv1.MachineList{ Items: []clusterv1.Machine{ newMachineBuilder().named("cp1").build(), - newMachineBuilder().named("cp2").build(), newMachineBuilder().named("cp3").build(), + newMachineBuilder().named("cp2").build(), }, }), workerMachines: collections.FromMachineList(&clusterv1.MachineList{ Items: []clusterv1.Machine{ - newMachineBuilder().named("w1").build(), newMachineBuilder().named("w2").build(), - newMachineBuilder().named("w3").build(), - newMachineBuilder().named("w4").build(), + newMachineBuilder().named("w1").build(), newMachineBuilder().named("w5").build(), newMachineBuilder().named("w6").build(), - newMachineBuilder().named("w7").build(), + newMachineBuilder().named("w3").build(), + newMachineBuilder().named("w4").build(), newMachineBuilder().named("w8").build(), + newMachineBuilder().named("w7").build(), }, }), machinePools: expv1.MachinePoolList{ Items: []expv1.MachinePool{ + newMachinePoolBuilder().named("mp2").build(), newMachinePoolBuilder().named("mp1").build(), }, }, @@ -882,16 +885,16 @@ func TestObjectsPendingDelete(t *testing.T) { g := NewWithT(t) c := &clusterv1.Cluster{} - g.Expect(d.objectsPendingDeleteCount(c)).To(Equal(15)) - g.Expect(d.objectsPendingDeleteNames(c)).To(Equal("Control plane Machines: cp1, cp2, cp3; MachineDeployments: md1; MachineSets: ms1, ms2; MachinePools: mp1; Worker Machines: w1, w2, w3, w4, w5, ... (3 more)")) + g.Expect(d.objectsPendingDeleteCount(c)).To(Equal(17)) + g.Expect(d.objectsPendingDeleteNames(c)).To(Equal("Control plane Machines: cp1, cp2, cp3; MachineDeployments: md1, md2; MachineSets: ms1, ms2; MachinePools: mp1, mp2; Worker Machines: w1, w2, w3, w4, w5, ... (3 more)")) }) t.Run("With a control plane object", func(t *testing.T) { g := NewWithT(t) c := &clusterv1.Cluster{Spec: clusterv1.ClusterSpec{ControlPlaneRef: &corev1.ObjectReference{Kind: "SomeKind"}}} - g.Expect(d.objectsPendingDeleteCount(c)).To(Equal(12)) - g.Expect(d.objectsPendingDeleteNames(c)).To(Equal("MachineDeployments: md1; MachineSets: ms1, ms2; MachinePools: mp1; Worker Machines: w1, w2, w3, w4, w5, ... (3 more)")) + g.Expect(d.objectsPendingDeleteCount(c)).To(Equal(14)) + g.Expect(d.objectsPendingDeleteNames(c)).To(Equal("MachineDeployments: md1, md2; MachineSets: ms1, ms2; MachinePools: mp1, mp2; Worker Machines: w1, w2, w3, w4, w5, ... (3 more)")) }) } diff --git a/internal/controllers/machineset/machineset_controller_status.go b/internal/controllers/machineset/machineset_controller_status.go index 72309c4b98be..910f47896eb0 100644 --- a/internal/controllers/machineset/machineset_controller_status.go +++ b/internal/controllers/machineset/machineset_controller_status.go @@ -55,7 +55,7 @@ func (r *Reconciler) updateStatus(ctx context.Context, s *scope) { // MachinesUpToDate condition: aggregate the Machine's UpToDate condition. setMachinesUpToDateCondition(ctx, s.machineSet, s.machines, s.getAndAdoptMachinesForMachineSetSucceeded) - // TODO Deleting + setDeletingCondition(ctx, s.machineSet, s.machines, s.getAndAdoptMachinesForMachineSetSucceeded) } func setReplicas(_ context.Context, ms *clusterv1.MachineSet, machines []*clusterv1.Machine, getAndAdoptMachinesForMachineSetSucceeded bool) { @@ -275,6 +275,47 @@ func setMachinesUpToDateCondition(ctx context.Context, machineSet *clusterv1.Mac v1beta2conditions.Set(machineSet, *upToDateCondition) } +func setDeletingCondition(_ context.Context, machineSet *clusterv1.MachineSet, machines []*clusterv1.Machine, getAndAdoptMachinesForMachineSetSucceeded bool) { + // If we got unexpected errors in listing the machines (this should never happen), surface them. + if !getAndAdoptMachinesForMachineSetSucceeded { + v1beta2conditions.Set(machineSet, metav1.Condition{ + Type: clusterv1.MachineSetDeletingV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.MachineSetDeletingInternalErrorV1Beta2Reason, + Message: "Please check controller logs for errors", + }) + return + } + + if machineSet.DeletionTimestamp.IsZero() { + v1beta2conditions.Set(machineSet, metav1.Condition{ + Type: clusterv1.MachineSetDeletingV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: clusterv1.MachineSetDeletingDeletionTimestampNotSetV1Beta2Reason, + }) + return + } + + message := "" + if len(machines) > 0 { + if len(machines) == 1 { + message = fmt.Sprintf("Deleting %d Machine", len(machines)) + } else { + message = fmt.Sprintf("Deleting %d Machines", len(machines)) + } + staleMessage := aggregateStaleMachines(machines) + if staleMessage != "" { + message += fmt.Sprintf(" and %s", staleMessage) + } + } + v1beta2conditions.Set(machineSet, metav1.Condition{ + Type: clusterv1.MachineSetDeletingV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: clusterv1.MachineSetDeletingDeletionTimestampSetV1Beta2Reason, + Message: message, + }) +} + func calculateMissingReferencesMessage(ms *clusterv1.MachineSet, bootstrapTemplateNotFound, infraMachineTemplateNotFound bool) string { missingObjects := []string{} if bootstrapTemplateNotFound { diff --git a/internal/controllers/machineset/machineset_controller_status_test.go b/internal/controllers/machineset/machineset_controller_status_test.go index bb5fb48751e3..e157563fd4ba 100644 --- a/internal/controllers/machineset/machineset_controller_status_test.go +++ b/internal/controllers/machineset/machineset_controller_status_test.go @@ -753,6 +753,80 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { } } +func Test_setDeletingCondition(t *testing.T) { + tests := []struct { + name string + machineSet *clusterv1.MachineSet + getAndAdoptMachinesForMachineSetSucceeded bool + machines []*clusterv1.Machine + expectCondition metav1.Condition + }{ + { + name: "get machines failed", + machineSet: &clusterv1.MachineSet{}, + machines: nil, + getAndAdoptMachinesForMachineSetSucceeded: false, + expectCondition: metav1.Condition{ + Type: clusterv1.MachineSetDeletingV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.MachineSetDeletingInternalErrorV1Beta2Reason, + Message: "Please check controller logs for errors", + }, + }, + { + name: "not deleting", + machineSet: &clusterv1.MachineSet{}, + getAndAdoptMachinesForMachineSetSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.MachineSetDeletingV1Beta2Condition, + Status: metav1.ConditionFalse, + Reason: clusterv1.MachineSetDeletingDeletionTimestampNotSetV1Beta2Reason, + }, + }, + { + name: "Deleting with still some machine", + machineSet: &clusterv1.MachineSet{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &metav1.Time{Time: time.Now()}}}, + getAndAdoptMachinesForMachineSetSucceeded: true, + machines: []*clusterv1.Machine{ + newMachine("m1"), + }, + expectCondition: metav1.Condition{ + Type: clusterv1.MachineSetDeletingV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: clusterv1.MachineSetDeletingDeletionTimestampSetV1Beta2Reason, + Message: "Deleting 1 Machine", + }, + }, + { + name: "Deleting with still some stale machine", + machineSet: &clusterv1.MachineSet{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &metav1.Time{Time: time.Now()}}}, + getAndAdoptMachinesForMachineSetSucceeded: true, + machines: []*clusterv1.Machine{ + newStaleDeletingMachine("m1"), + newStaleDeletingMachine("m2"), + newMachine("m3"), + }, + expectCondition: metav1.Condition{ + Type: clusterv1.MachineSetDeletingV1Beta2Condition, + Status: metav1.ConditionTrue, + Reason: clusterv1.MachineSetDeletingDeletionTimestampSetV1Beta2Reason, + Message: "Deleting 3 Machines and Machines m1, m2 are in deletion since more than 30m", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + setDeletingCondition(ctx, tt.machineSet, tt.machines, tt.getAndAdoptMachinesForMachineSetSucceeded) + + condition := v1beta2conditions.Get(tt.machineSet, clusterv1.MachineSetDeletingV1Beta2Condition) + g.Expect(condition).ToNot(BeNil()) + g.Expect(*condition).To(v1beta2conditions.MatchCondition(tt.expectCondition, v1beta2conditions.IgnoreLastTransitionTime(true))) + }) + } +} + func newMachine(name string, conditions ...metav1.Condition) *clusterv1.Machine { m := &clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{