diff --git a/api/v1beta1/machinedeployment_types.go b/api/v1beta1/machinedeployment_types.go index 5594f829fcf3..b0111748755a 100644 --- a/api/v1beta1/machinedeployment_types.go +++ b/api/v1beta1/machinedeployment_types.go @@ -88,7 +88,7 @@ const ( // field of the MachineDeployment is not set. MachineDeploymentAvailableWaitingForReplicasSetV1Beta2Reason = WaitingForReplicasSetV1Beta2Reason - // MachineDeploymentAvailableWaitingForAvailableReplicasSetV1Beta2Reason surfaces when the .status.v1beta2.availableReplicas + // MachineDeploymentAvailableWaitingForAvailableReplicasSetV1Beta2Reason surfaces when the .status.v1beta2.availableReplicas // field of the MachineDeployment is not set. MachineDeploymentAvailableWaitingForAvailableReplicasSetV1Beta2Reason = "WaitingForAvailableReplicasSet" @@ -130,7 +130,7 @@ const ( // MachineDeployment's ScalingUp condition and corresponding reasons that will be used in v1Beta2 API version. const ( - // MachineDeploymentScalingUpV1Beta2Condition is true if available replicas < desired replicas. + // MachineDeploymentScalingUpV1Beta2Condition is true if actual replicas < desired replicas. MachineDeploymentScalingUpV1Beta2Condition = ScalingUpV1Beta2Condition // MachineDeploymentScalingUpV1Beta2Reason surfaces when actual replicas < desired replicas. @@ -149,7 +149,7 @@ const ( // MachineDeployment's ScalingDown condition and corresponding reasons that will be used in v1Beta2 API version. const ( - // MachineDeploymentScalingDownV1Beta2Condition is true if replicas > desired replicas. + // MachineDeploymentScalingDownV1Beta2Condition is true if actual replicas > desired replicas. MachineDeploymentScalingDownV1Beta2Condition = ScalingDownV1Beta2Condition // MachineDeploymentScalingDownV1Beta2Reason surfaces when actual replicas > desired replicas. @@ -168,7 +168,7 @@ const ( // MachineDeployment's Remediating condition and corresponding reasons that will be used in v1Beta2 API version. const ( - // MachineDeploymentRemediatingV1Beta2Condition details about ongoing remediation of the controlled machines, if any. + // MachineDeploymentRemediatingV1Beta2Condition surfaces details about ongoing remediation of the controlled machines, if any. MachineDeploymentRemediatingV1Beta2Condition = RemediatingV1Beta2Condition // MachineDeploymentRemediatingV1Beta2Reason surfaces when the MachineDeployment has at least one machine with HealthCheckSucceeded set to false diff --git a/api/v1beta1/v1beta2_condition_consts.go b/api/v1beta1/v1beta2_condition_consts.go index df7146608bad..409c2559eba2 100644 --- a/api/v1beta1/v1beta2_condition_consts.go +++ b/api/v1beta1/v1beta2_condition_consts.go @@ -90,7 +90,7 @@ const ( // AvailableV1Beta2Reason applies to a condition surfacing object availability. AvailableV1Beta2Reason = "Available" - // NotAvailableV1Beta2Reason applies to a condition surfacing object not satisfying availability per-requisites. + // NotAvailableV1Beta2Reason applies to a condition surfacing object not satisfying availability criteria. NotAvailableV1Beta2Reason = "NotAvailable" // ScalingUpV1Beta2Reason surfaces when an object is scaling up. diff --git a/controlplane/kubeadm/internal/controllers/status.go b/controlplane/kubeadm/internal/controllers/status.go index 0ab1d370f911..09cef5bdba45 100644 --- a/controlplane/kubeadm/internal/controllers/status.go +++ b/controlplane/kubeadm/internal/controllers/status.go @@ -406,6 +406,10 @@ func setRemediatingCondition(ctx context.Context, kcp *controlplanev1.KubeadmCon } func aggregateStaleMachines(machines collections.Machines) string { + if len(machines) == 0 { + return "" + } + machineNames := []string{} for _, machine := range machines { if !machine.GetDeletionTimestamp().IsZero() && time.Since(machine.GetDeletionTimestamp().Time) > time.Minute*30 { @@ -436,6 +440,10 @@ func aggregateStaleMachines(machines collections.Machines) string { } func aggregateUnhealthyMachines(machines collections.Machines) string { + if len(machines) == 0 { + return "" + } + machineNames := []string{} for _, machine := range machines { machineNames = append(machineNames, machine.GetName()) diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go index f3ec111f1a1f..0c8272508916 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller.go @@ -116,7 +116,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt return nil } -func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { +func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (retres ctrl.Result, reterr error) { log := ctrl.LoggerFrom(ctx) // Fetch the MachineDeployment instance. @@ -173,6 +173,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re if err := patchMachineDeployment(ctx, patchHelper, deployment, patchOpts...); err != nil { reterr = kerrors.NewAggregate([]error{reterr, err}) } + + if reterr != nil { + retres = ctrl.Result{} + } }() // Handle deletion reconciliation loop. @@ -188,7 +192,9 @@ type scope struct { cluster *clusterv1.Cluster machineSets []*clusterv1.MachineSet bootstrapTemplateNotFound bool + bootstrapTemplateExists bool infrastructureTemplateNotFound bool + infrastructureTemplateExists bool getAndAdoptMachineSetsForDeploymentSucceeded bool } @@ -206,6 +212,15 @@ func patchMachineDeployment(ctx context.Context, patchHelper *patch.Helper, md * clusterv1.ReadyCondition, clusterv1.MachineDeploymentAvailableCondition, }}, + patch.WithOwnedV1Beta2Conditions{Conditions: []string{ + clusterv1.MachineDeploymentAvailableV1Beta2Condition, + clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition, + clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, + clusterv1.MachineDeploymentScalingDownV1Beta2Condition, + clusterv1.MachineDeploymentScalingUpV1Beta2Condition, + clusterv1.MachineDeploymentRemediatingV1Beta2Condition, + clusterv1.MachineDeploymentDeletingV1Beta2Condition, + }}, ) return patchHelper.Patch(ctx, md, options...) } @@ -280,8 +295,10 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope) error { } } + templateExists := s.infrastructureTemplateExists && (md.Spec.Template.Spec.Bootstrap.ConfigRef == nil || s.bootstrapTemplateExists) + if md.Spec.Paused { - return r.sync(ctx, md, s.machineSets) + return r.sync(ctx, md, s.machineSets, templateExists) } if md.Spec.Strategy == nil { @@ -292,11 +309,11 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope) error { if md.Spec.Strategy.RollingUpdate == nil { return errors.Errorf("missing MachineDeployment settings for strategy type: %s", md.Spec.Strategy.Type) } - return r.rolloutRolling(ctx, md, s.machineSets) + return r.rolloutRolling(ctx, md, s.machineSets, templateExists) } if md.Spec.Strategy.Type == clusterv1.OnDeleteMachineDeploymentStrategyType { - return r.rolloutOnDelete(ctx, md, s.machineSets) + return r.rolloutOnDelete(ctx, md, s.machineSets, templateExists) } return errors.Errorf("unexpected deployment strategy type: %s", md.Spec.Strategy.Type) @@ -314,8 +331,6 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) error { return nil } - log.Info("Waiting for MachineSets to be deleted", "MachineSets", clog.ObjNamesString(s.machineSets)) - // else delete owned machinesets. for _, ms := range s.machineSets { if ms.DeletionTimestamp.IsZero() { @@ -326,6 +341,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) error { } } + log.Info("Waiting for MachineSets to be deleted", "MachineSets", clog.ObjNamesString(s.machineSets)) return nil } @@ -470,6 +486,8 @@ func (r *Reconciler) getTemplatesAndSetOwner(ctx context.Context, s *scope) erro return err } s.infrastructureTemplateNotFound = true + } else { + s.infrastructureTemplateExists = true } // Make sure to reconcile the external bootstrap reference, if any. if md.Spec.Template.Spec.Bootstrap.ConfigRef != nil { @@ -478,6 +496,8 @@ func (r *Reconciler) getTemplatesAndSetOwner(ctx context.Context, s *scope) erro return err } s.bootstrapTemplateNotFound = true + } else { + s.bootstrapTemplateExists = true } } return nil diff --git a/internal/controllers/machinedeployment/machinedeployment_rolling.go b/internal/controllers/machinedeployment/machinedeployment_rolling.go index 0c79bf365a83..31bd697d8b24 100644 --- a/internal/controllers/machinedeployment/machinedeployment_rolling.go +++ b/internal/controllers/machinedeployment/machinedeployment_rolling.go @@ -29,8 +29,8 @@ import ( ) // rolloutRolling implements the logic for rolling a new MachineSet. -func (r *Reconciler) rolloutRolling(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) error { - newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, true) +func (r *Reconciler) rolloutRolling(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, templateExists bool) error { + newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, true, templateExists) if err != nil { return err } diff --git a/internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go b/internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go index 64d15d1e3361..b8e7fcd75297 100644 --- a/internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go +++ b/internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go @@ -32,8 +32,8 @@ import ( ) // rolloutOnDelete implements the logic for the OnDelete MachineDeploymentStrategyType. -func (r *Reconciler) rolloutOnDelete(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) error { - newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, true) +func (r *Reconciler) rolloutOnDelete(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, templateExists bool) error { + newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, true, templateExists) if err != nil { return err } diff --git a/internal/controllers/machinedeployment/machinedeployment_status.go b/internal/controllers/machinedeployment/machinedeployment_status.go index ef078e73a1cd..24cee4926993 100644 --- a/internal/controllers/machinedeployment/machinedeployment_status.go +++ b/internal/controllers/machinedeployment/machinedeployment_status.go @@ -37,35 +37,38 @@ import ( func (r *Reconciler) updateStatus(ctx context.Context, s *scope) (retErr error) { // Get all Machines controlled by this MachineDeployment. - var machines, machinesToBeRemediated, unHealthyMachines collections.Machines + var machines, machinesToBeRemediated, unhealthyMachines collections.Machines + var getMachinesSucceeded bool if selectorMap, err := metav1.LabelSelectorAsMap(&s.machineDeployment.Spec.Selector); err == nil { machineList := &clusterv1.MachineList{} if err := r.Client.List(ctx, machineList, client.InNamespace(s.machineDeployment.Namespace), client.MatchingLabels(selectorMap)); err != nil { retErr = errors.Wrap(err, "failed to list machines") + } else { + getMachinesSucceeded = true + machines = collections.FromMachineList(machineList) + machinesToBeRemediated = machines.Filter(collections.IsUnhealthyAndOwnerRemediated) + unhealthyMachines = machines.Filter(collections.IsUnhealthy) } - machines = collections.FromMachineList(machineList) - machinesToBeRemediated = machines.Filter(collections.IsUnhealthyAndOwnerRemediated) - unHealthyMachines = machines.Filter(collections.IsUnhealthy) } else { retErr = errors.Wrap(err, "failed to convert label selector to a map") } - // If the controller failed to read MachineSets, do not update replica counters. - if !s.getAndAdoptMachineSetsForDeploymentSucceeded { + // If the controller could read MachineSets, update replica counters. + if s.getAndAdoptMachineSetsForDeploymentSucceeded { setReplicas(s.machineDeployment, s.machineSets) } setAvailableCondition(ctx, s.machineDeployment, s.getAndAdoptMachineSetsForDeploymentSucceeded) setScalingUpCondition(ctx, s.machineDeployment, s.machineSets, s.bootstrapTemplateNotFound, s.infrastructureTemplateNotFound, s.getAndAdoptMachineSetsForDeploymentSucceeded) - setScalingDownCondition(ctx, s.machineDeployment, s.machineSets, machines, s.getAndAdoptMachineSetsForDeploymentSucceeded) + setScalingDownCondition(ctx, s.machineDeployment, s.machineSets, machines, s.getAndAdoptMachineSetsForDeploymentSucceeded, getMachinesSucceeded) - setMachinesReadyCondition(ctx, s.machineDeployment, machines) - setMachinesUpToDateCondition(ctx, s.machineDeployment, machines) + setMachinesReadyCondition(ctx, s.machineDeployment, machines, getMachinesSucceeded) + setMachinesUpToDateCondition(ctx, s.machineDeployment, machines, getMachinesSucceeded) - setRemediatingCondition(ctx, s.machineDeployment, machinesToBeRemediated, unHealthyMachines) + setRemediatingCondition(ctx, s.machineDeployment, machinesToBeRemediated, unhealthyMachines, getMachinesSucceeded) - setDeletingCondition(ctx, s.machineDeployment, s.machineSets, machines, s.getAndAdoptMachineSetsForDeploymentSucceeded) + setDeletingCondition(ctx, s.machineDeployment, s.machineSets, machines, s.getAndAdoptMachineSetsForDeploymentSucceeded, getMachinesSucceeded) return retErr } @@ -132,7 +135,7 @@ func setAvailableCondition(_ context.Context, machineDeployment *clusterv1.Machi message := fmt.Sprintf("%d available replicas, at least %d required", *machineDeployment.Status.V1Beta2.AvailableReplicas, minReplicasNeeded) if machineDeployment.Spec.Strategy != nil && mdutil.IsRollingUpdate(machineDeployment) && machineDeployment.Spec.Strategy.RollingUpdate != nil { - message += fmt.Sprintf(" (spec.strategy.rollout.maxUnavailable is %s)", machineDeployment.Spec.Strategy.RollingUpdate.MaxUnavailable) + message += fmt.Sprintf(" (spec.strategy.rollout.maxUnavailable is %s, spec.replicas is %d)", machineDeployment.Spec.Strategy.RollingUpdate.MaxUnavailable, *machineDeployment.Spec.Replicas) } v1beta2conditions.Set(machineDeployment, metav1.Condition{ Type: clusterv1.MachineDeploymentAvailableV1Beta2Condition, @@ -168,7 +171,7 @@ func setScalingUpCondition(_ context.Context, machineDeployment *clusterv1.Machi if !machineDeployment.DeletionTimestamp.IsZero() { desiredReplicas = 0 } - currentReplicas := mdutil.GetReplicaCountForMachineSets(machineSets) + currentReplicas := mdutil.GetActualReplicaCountForMachineSets(machineSets) missingReferencesMessage := calculateMissingReferencesMessage(machineDeployment, bootstrapObjectNotFound, infrastructureObjectNotFound) @@ -199,7 +202,7 @@ func setScalingUpCondition(_ context.Context, machineDeployment *clusterv1.Machi }) } -func setScalingDownCondition(_ context.Context, machineDeployment *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet, machines collections.Machines, getAndAdoptMachineSetsForDeploymentSucceeded bool) { +func setScalingDownCondition(_ context.Context, machineDeployment *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet, machines collections.Machines, getAndAdoptMachineSetsForDeploymentSucceeded, getMachinesSucceeded bool) { // If we got unexpected errors in listing the machines sets (this should never happen), surface them. if !getAndAdoptMachineSetsForDeploymentSucceeded { v1beta2conditions.Set(machineDeployment, metav1.Condition{ @@ -225,14 +228,16 @@ func setScalingDownCondition(_ context.Context, machineDeployment *clusterv1.Mac if !machineDeployment.DeletionTimestamp.IsZero() { desiredReplicas = 0 } - currentReplicas := mdutil.GetReplicaCountForMachineSets(machineSets) + currentReplicas := mdutil.GetActualReplicaCountForMachineSets(machineSets) // Scaling down. if currentReplicas > desiredReplicas { message := fmt.Sprintf("Scaling down from %d to %d replicas", currentReplicas, desiredReplicas) - staleMessage := aggregateStaleMachines(machines) - if staleMessage != "" { - message += fmt.Sprintf(" and %s", staleMessage) + if getMachinesSucceeded { + staleMessage := aggregateStaleMachines(machines) + if staleMessage != "" { + message += fmt.Sprintf(" and %s", staleMessage) + } } v1beta2conditions.Set(machineDeployment, metav1.Condition{ Type: clusterv1.MachineDeploymentScalingDownV1Beta2Condition, @@ -251,10 +256,10 @@ func setScalingDownCondition(_ context.Context, machineDeployment *clusterv1.Mac }) } -func setMachinesReadyCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machines collections.Machines) { +func setMachinesReadyCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machines collections.Machines, getMachinesSucceeded bool) { log := ctrl.LoggerFrom(ctx) // If we got unexpected errors in listing the machines (this should never happen), surface them. - if machines == nil { + if !getMachinesSucceeded { v1beta2conditions.Set(machineDeployment, metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -291,10 +296,10 @@ func setMachinesReadyCondition(ctx context.Context, machineDeployment *clusterv1 v1beta2conditions.Set(machineDeployment, *readyCondition) } -func setMachinesUpToDateCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machines collections.Machines) { +func setMachinesUpToDateCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machines collections.Machines, getMachinesSucceeded bool) { log := ctrl.LoggerFrom(ctx) // If we got unexpected errors in listing the machines (this should never happen), surface them. - if machines == nil { + if !getMachinesSucceeded { v1beta2conditions.Set(machineDeployment, metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -331,8 +336,8 @@ func setMachinesUpToDateCondition(ctx context.Context, machineDeployment *cluste v1beta2conditions.Set(machineDeployment, *upToDateCondition) } -func setRemediatingCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machinesToBeRemediated, unhealthyMachines collections.Machines) { - if machinesToBeRemediated == nil || unhealthyMachines == nil { +func setRemediatingCondition(ctx context.Context, machineDeployment *clusterv1.MachineDeployment, machinesToBeRemediated, unhealthyMachines collections.Machines, getMachinesSucceeded bool) { + if !getMachinesSucceeded { v1beta2conditions.Set(machineDeployment, metav1.Condition{ Type: clusterv1.MachineDeploymentRemediatingV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -378,9 +383,9 @@ func setRemediatingCondition(ctx context.Context, machineDeployment *clusterv1.M }) } -func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet, machines collections.Machines, getAndAdoptMachineSetsForDeploymentSucceeded bool) { +func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.MachineDeployment, machineSets []*clusterv1.MachineSet, machines collections.Machines, getAndAdoptMachineSetsForDeploymentSucceeded, getMachinesSucceeded bool) { // If we got unexpected errors in listing the machines sets or machines (this should never happen), surface them. - if !getAndAdoptMachineSetsForDeploymentSucceeded || machines == nil { + if !getAndAdoptMachineSetsForDeploymentSucceeded || !getMachinesSucceeded { v1beta2conditions.Set(machineDeployment, metav1.Condition{ Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -401,7 +406,11 @@ func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.Machin message := "" if len(machines) > 0 { - message = fmt.Sprintf("Deleting %d replicas", len(machines)) + 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) @@ -413,7 +422,7 @@ func setDeletingCondition(_ context.Context, machineDeployment *clusterv1.Machin } v1beta2conditions.Set(machineDeployment, metav1.Condition{ Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition, - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: clusterv1.MachineDeploymentDeletingDeletionTimestampSetV1Beta2Reason, Message: message, }) @@ -474,6 +483,10 @@ func aggregateStaleMachines(machines collections.Machines) string { } func aggregateUnhealthyMachines(machines collections.Machines) string { + if len(machines) == 0 { + return "" + } + machineNames := []string{} for _, machine := range machines { machineNames = append(machineNames, machine.GetName()) @@ -496,7 +509,7 @@ func aggregateUnhealthyMachines(machines collections.Machines) string { } else { message += " are " } - message += "not healthy (not to be remediated by KCP)" + message += "not healthy (not to be remediated by MachineDeployment/MachineSet)" return message } diff --git a/internal/controllers/machinedeployment/machinedeployment_status_test.go b/internal/controllers/machinedeployment/machinedeployment_status_test.go index 1b9f921ea7b7..ce10e30cff06 100644 --- a/internal/controllers/machinedeployment/machinedeployment_status_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_status_test.go @@ -22,7 +22,6 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -124,7 +123,7 @@ func Test_setAvailableCondition(t *testing.T) { }, }, { - name: "all the expected replicase are available", + name: "all the expected replicas are available", machineDeployment: &clusterv1.MachineDeployment{ Spec: clusterv1.MachineDeploymentSpec{ Replicas: ptr.To(int32(5)), @@ -146,7 +145,7 @@ func Test_setAvailableCondition(t *testing.T) { }, }, { - name: "some replicase are not available, but within MaxUnavailable range", + name: "some replicas are not available, but within MaxUnavailable range", machineDeployment: &clusterv1.MachineDeployment{ Spec: clusterv1.MachineDeploymentSpec{ Replicas: ptr.To(int32(5)), @@ -168,30 +167,7 @@ func Test_setAvailableCondition(t *testing.T) { }, }, { - name: "some replicase are not available, less than MaxUnavailable", - machineDeployment: &clusterv1.MachineDeployment{ - Spec: clusterv1.MachineDeploymentSpec{ - Replicas: ptr.To(int32(5)), - Strategy: &clusterv1.MachineDeploymentStrategy{ - Type: clusterv1.RollingUpdateMachineDeploymentStrategyType, - RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{ - MaxSurge: ptr.To(intstr.FromInt32(1)), - MaxUnavailable: ptr.To(intstr.FromInt32(1)), - }, - }, - }, - Status: clusterv1.MachineDeploymentStatus{V1Beta2: &clusterv1.MachineDeploymentV1Beta2Status{AvailableReplicas: ptr.To(int32(4))}}, - }, - getAndAdoptMachineSetsForDeploymentSucceeded: true, - expectCondition: metav1.Condition{ - Type: clusterv1.MachineDeploymentAvailableV1Beta2Condition, - Status: metav1.ConditionTrue, - Reason: clusterv1.MachineDeploymentAvailableV1Beta2Reason, - Message: "", - }, - }, - { - name: "some replicase are not available, more than MaxUnavailable", + name: "some replicas are not available, more than MaxUnavailable", machineDeployment: &clusterv1.MachineDeployment{ Spec: clusterv1.MachineDeploymentSpec{ Replicas: ptr.To(int32(5)), @@ -210,7 +186,7 @@ func Test_setAvailableCondition(t *testing.T) { Type: clusterv1.MachineDeploymentAvailableV1Beta2Condition, Status: metav1.ConditionFalse, Reason: clusterv1.MachineDeploymentNotAvailableV1Beta2Reason, - Message: "3 available replicas, at least 4 required (spec.strategy.rollout.maxUnavailable is 1)", + Message: "3 available replicas, at least 4 required (spec.strategy.rollout.maxUnavailable is 1, spec.replicas is 5)", }, }, } @@ -279,6 +255,22 @@ func Test_setScalingUpCondition(t *testing.T) { Message: "Please check controller logs for errors", }, }, + { + name: "replicas not set", + machineDeployment: func() *clusterv1.MachineDeployment { + md := defaultMachineDeployment.DeepCopy() + md.Spec.Replicas = nil + return md + }(), + bootstrapTemplateNotFound: false, + infrastructureTemplateNotFound: false, + getAndAdoptMachineSetsForDeploymentSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.MachineDeploymentScalingUpV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.MachineDeploymentScalingUpWaitingForReplicasSetV1Beta2Reason, + }, + }, { name: "not scaling up and no machines", machineDeployment: defaultMachineDeployment, @@ -295,8 +287,8 @@ func Test_setScalingUpCondition(t *testing.T) { name: "not scaling up with machines", machineDeployment: deletingMachineDeploymentWith3Replicas, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(1)), - fakeMachineSet("ms2", withSpecReplicas(2)), + fakeMachineSet("ms1", withStatusReplicas(1)), + fakeMachineSet("ms2", withStatusReplicas(2)), }, bootstrapTemplateNotFound: false, infrastructureTemplateNotFound: false, @@ -363,8 +355,8 @@ func Test_setScalingUpCondition(t *testing.T) { name: "scaling up with machines", machineDeployment: scalingUpMachineDeploymentWith3Replicas, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(1)), - fakeMachineSet("ms2", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(1)), + fakeMachineSet("ms2", withStatusReplicas(1)), }, bootstrapTemplateNotFound: false, infrastructureTemplateNotFound: false, @@ -463,6 +455,20 @@ func Test_setScalingDownCondition(t *testing.T) { Message: "Please check controller logs for errors", }, }, + { + name: "replicas not set", + machineDeployment: func() *clusterv1.MachineDeployment { + md := defaultMachineDeployment.DeepCopy() + md.Spec.Replicas = nil + return md + }(), + getAndAdoptMachineSetsForDeploymentSucceeded: true, + expectCondition: metav1.Condition{ + Type: clusterv1.MachineDeploymentScalingDownV1Beta2Condition, + Status: metav1.ConditionUnknown, + Reason: clusterv1.MachineDeploymentScalingDownWaitingForReplicasSetV1Beta2Reason, + }, + }, { name: "not scaling down and no machines", machineDeployment: defaultMachineDeployment, @@ -489,7 +495,7 @@ func Test_setScalingDownCondition(t *testing.T) { name: "scaling down to zero", machineDeployment: defaultMachineDeployment, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(1)), }, getAndAdoptMachineSetsForDeploymentSucceeded: true, expectCondition: metav1.Condition{ @@ -503,8 +509,8 @@ func Test_setScalingDownCondition(t *testing.T) { name: "scaling down with 1 stale machine", machineDeployment: machineDeploymentWith1Replica, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(1)), - fakeMachineSet("ms2", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(1)), + fakeMachineSet("ms2", withStatusReplicas(1)), }, machines: []*clusterv1.Machine{ fakeMachine("m1"), @@ -522,8 +528,8 @@ func Test_setScalingDownCondition(t *testing.T) { name: "scaling down with 3 stale machines", machineDeployment: machineDeploymentWith1Replica, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(3)), - fakeMachineSet("ms2", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(3)), + fakeMachineSet("ms2", withStatusReplicas(1)), }, machines: []*clusterv1.Machine{ fakeMachine("m1"), @@ -543,8 +549,8 @@ func Test_setScalingDownCondition(t *testing.T) { name: "scaling down with 5 stale machines", machineDeployment: machineDeploymentWith1Replica, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(5)), - fakeMachineSet("ms2", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(5)), + fakeMachineSet("ms2", withStatusReplicas(1)), }, machines: []*clusterv1.Machine{ fakeMachine("m1"), @@ -577,7 +583,7 @@ func Test_setScalingDownCondition(t *testing.T) { name: "deleting machine deployment having 1 replica", machineDeployment: deletingMachineDeployment, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(1)), }, getAndAdoptMachineSetsForDeploymentSucceeded: true, expectCondition: metav1.Condition{ @@ -592,7 +598,7 @@ func Test_setScalingDownCondition(t *testing.T) { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - setScalingDownCondition(ctx, tt.machineDeployment, tt.machineSets, collections.FromMachines(tt.machines...), tt.getAndAdoptMachineSetsForDeploymentSucceeded) + setScalingDownCondition(ctx, tt.machineDeployment, tt.machineSets, collections.FromMachines(tt.machines...), tt.getAndAdoptMachineSetsForDeploymentSucceeded, true) condition := v1beta2conditions.Get(tt.machineDeployment, clusterv1.MachineDeploymentScalingDownV1Beta2Condition) g.Expect(condition).ToNot(BeNil()) @@ -609,15 +615,17 @@ func Test_setMachinesReadyCondition(t *testing.T) { } tests := []struct { - name string - machineDeployment *clusterv1.MachineDeployment - machines []*clusterv1.Machine - expectCondition metav1.Condition + name string + machineDeployment *clusterv1.MachineDeployment + machines []*clusterv1.Machine + getMachinesSucceeded bool + expectCondition metav1.Condition }{ { - name: "get machines failed", - machineDeployment: &clusterv1.MachineDeployment{}, - machines: nil, + name: "get machines failed", + machineDeployment: &clusterv1.MachineDeployment{}, + machines: nil, + getMachinesSucceeded: false, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -626,9 +634,10 @@ func Test_setMachinesReadyCondition(t *testing.T) { }, }, { - name: "no machines", - machineDeployment: &clusterv1.MachineDeployment{}, - machines: []*clusterv1.Machine{}, + name: "no machines", + machineDeployment: &clusterv1.MachineDeployment{}, + machines: []*clusterv1.Machine{}, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition, Status: metav1.ConditionTrue, @@ -642,6 +651,7 @@ func Test_setMachinesReadyCondition(t *testing.T) { fakeMachine("machine-1", withV1Beta2Condition(readyCondition)), fakeMachine("machine-2", withV1Beta2Condition(readyCondition)), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition, Status: metav1.ConditionTrue, @@ -655,6 +665,7 @@ func Test_setMachinesReadyCondition(t *testing.T) { fakeMachine("machine-1", withV1Beta2Condition(readyCondition)), fakeMachine("machine-2"), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -686,6 +697,7 @@ func Test_setMachinesReadyCondition(t *testing.T) { Message: "Deleting: Machine deletion in progress, stage: DrainingNode", })), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition, Status: metav1.ConditionFalse, @@ -702,7 +714,7 @@ func Test_setMachinesReadyCondition(t *testing.T) { if tt.machines != nil { machines = collections.FromMachines(tt.machines...) } - setMachinesReadyCondition(ctx, tt.machineDeployment, machines) + setMachinesReadyCondition(ctx, tt.machineDeployment, machines, tt.getMachinesSucceeded) condition := v1beta2conditions.Get(tt.machineDeployment, clusterv1.MachineDeploymentMachinesReadyV1Beta2Condition) g.Expect(condition).ToNot(BeNil()) @@ -713,15 +725,17 @@ func Test_setMachinesReadyCondition(t *testing.T) { func Test_setMachinesUpToDateCondition(t *testing.T) { tests := []struct { - name string - machineDeployment *clusterv1.MachineDeployment - machines []*clusterv1.Machine - expectCondition metav1.Condition + name string + machineDeployment *clusterv1.MachineDeployment + machines []*clusterv1.Machine + getMachinesSucceeded bool + expectCondition metav1.Condition }{ { - name: "get machines failed", - machineDeployment: &clusterv1.MachineDeployment{}, - machines: nil, + name: "get machines failed", + machineDeployment: &clusterv1.MachineDeployment{}, + machines: nil, + getMachinesSucceeded: false, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -730,9 +744,10 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { }, }, { - name: "no machines", - machineDeployment: &clusterv1.MachineDeployment{}, - machines: []*clusterv1.Machine{}, + name: "no machines", + machineDeployment: &clusterv1.MachineDeployment{}, + machines: []*clusterv1.Machine{}, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionTrue, @@ -750,6 +765,7 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { Reason: "some-reason-1", })), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionTrue, @@ -768,6 +784,7 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { Message: "some unknown message", })), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -786,6 +803,7 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { Message: "some not up-to-date message", })), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionFalse, @@ -799,6 +817,7 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { machines: []*clusterv1.Machine{ fakeMachine("no-condition-machine-1"), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -835,6 +854,7 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { fakeMachine("no-condition-machine-1"), fakeMachine("no-condition-machine-2"), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition, Status: metav1.ConditionFalse, @@ -851,7 +871,7 @@ func Test_setMachinesUpToDateCondition(t *testing.T) { if tt.machines != nil { machines = collections.FromMachines(tt.machines...) } - setMachinesUpToDateCondition(ctx, tt.machineDeployment, machines) + setMachinesUpToDateCondition(ctx, tt.machineDeployment, machines, tt.getMachinesSucceeded) condition := v1beta2conditions.Get(tt.machineDeployment, clusterv1.MachineDeploymentMachinesUpToDateV1Beta2Condition) g.Expect(condition).ToNot(BeNil()) @@ -867,15 +887,17 @@ func Test_setRemediatingCondition(t *testing.T) { ownerRemediatedV1Beta2 := metav1.Condition{Type: clusterv1.MachineOwnerRemediatedV1Beta2Condition, Status: metav1.ConditionFalse, Message: "Remediation in progress"} tests := []struct { - name string - machineDeployment *clusterv1.MachineDeployment - machines []*clusterv1.Machine - expectCondition metav1.Condition + name string + machineDeployment *clusterv1.MachineDeployment + machines []*clusterv1.Machine + getMachinesSucceeded bool + expectCondition metav1.Condition }{ { - name: "get machines failed", - machineDeployment: &clusterv1.MachineDeployment{}, - machines: nil, + name: "get machines failed", + machineDeployment: &clusterv1.MachineDeployment{}, + machines: nil, + getMachinesSucceeded: false, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentRemediatingV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -890,6 +912,7 @@ func Test_setRemediatingCondition(t *testing.T) { fakeMachine("m1"), fakeMachine("m2"), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentRemediatingV1Beta2Condition, Status: metav1.ConditionFalse, @@ -897,13 +920,14 @@ func Test_setRemediatingCondition(t *testing.T) { }, }, { - name: "With machines to be remediated by KCP", + name: "With machines to be remediated by MD/MS", machineDeployment: &clusterv1.MachineDeployment{}, machines: []*clusterv1.Machine{ fakeMachine("m1", withConditions(healthCheckSucceeded)), // Healthy machine fakeMachine("m2", withConditions(healthCheckNotSucceeded)), // Unhealthy machine, not yet marked for remediation fakeMachine("m3", withConditions(healthCheckNotSucceeded, ownerRemediated), withV1Beta2Condition(ownerRemediatedV1Beta2)), }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentRemediatingV1Beta2Condition, Status: metav1.ConditionTrue, @@ -912,33 +936,35 @@ func Test_setRemediatingCondition(t *testing.T) { }, }, { - name: "With one unhealthy machine not to be remediated by KCP", + name: "With one unhealthy machine not to be remediated by MD/MS", machineDeployment: &clusterv1.MachineDeployment{}, machines: []*clusterv1.Machine{ fakeMachine("m1", withConditions(healthCheckSucceeded)), // Healthy machine fakeMachine("m2", withConditions(healthCheckNotSucceeded)), // Unhealthy machine, not yet marked for remediation fakeMachine("m3", withConditions(healthCheckSucceeded)), // Healthy machine }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentRemediatingV1Beta2Condition, Status: metav1.ConditionFalse, Reason: clusterv1.MachineDeploymentNotRemediatingV1Beta2Reason, - Message: "Machine m2 is not healthy (not to be remediated by KCP)", + Message: "Machine m2 is not healthy (not to be remediated by MachineDeployment/MachineSet)", }, }, { - name: "With two unhealthy machine not to be remediated by KCP", + name: "With two unhealthy machine not to be remediated by MD/MS", machineDeployment: &clusterv1.MachineDeployment{}, machines: []*clusterv1.Machine{ fakeMachine("m1", withConditions(healthCheckNotSucceeded)), // Unhealthy machine, not yet marked for remediation fakeMachine("m2", withConditions(healthCheckNotSucceeded)), // Unhealthy machine, not yet marked for remediation fakeMachine("m3", withConditions(healthCheckSucceeded)), // Healthy machine }, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentRemediatingV1Beta2Condition, Status: metav1.ConditionFalse, Reason: clusterv1.MachineDeploymentNotRemediatingV1Beta2Reason, - Message: "Machines m1, m2 are not healthy (not to be remediated by KCP)", + Message: "Machines m1, m2 are not healthy (not to be remediated by MachineDeployment/MachineSet)", }, }, } @@ -947,12 +973,12 @@ func Test_setRemediatingCondition(t *testing.T) { g := NewWithT(t) var machinesToBeRemediated, unHealthyMachines collections.Machines - if tt.machines != nil { + if tt.getMachinesSucceeded { machines := collections.FromMachines(tt.machines...) machinesToBeRemediated = machines.Filter(collections.IsUnhealthyAndOwnerRemediated) unHealthyMachines = machines.Filter(collections.IsUnhealthy) } - setRemediatingCondition(ctx, tt.machineDeployment, machinesToBeRemediated, unHealthyMachines) + setRemediatingCondition(ctx, tt.machineDeployment, machinesToBeRemediated, unHealthyMachines, tt.getMachinesSucceeded) condition := v1beta2conditions.Get(tt.machineDeployment, clusterv1.MachineDeploymentRemediatingV1Beta2Condition) g.Expect(condition).ToNot(BeNil()) @@ -966,16 +992,18 @@ func Test_setDeletingCondition(t *testing.T) { name string machineDeployment *clusterv1.MachineDeployment machineSets []*clusterv1.MachineSet - machines []*clusterv1.Machine getAndAdoptMachineSetsForDeploymentSucceeded bool + machines []*clusterv1.Machine + getMachinesSucceeded bool expectCondition metav1.Condition }{ { name: "get machine sets failed", machineDeployment: &clusterv1.MachineDeployment{}, machineSets: nil, - machines: []*clusterv1.Machine{}, getAndAdoptMachineSetsForDeploymentSucceeded: false, + machines: []*clusterv1.Machine{}, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -987,8 +1015,9 @@ func Test_setDeletingCondition(t *testing.T) { name: "get machines failed", machineDeployment: &clusterv1.MachineDeployment{}, machineSets: []*clusterv1.MachineSet{}, - machines: nil, getAndAdoptMachineSetsForDeploymentSucceeded: true, + machines: nil, + getMachinesSucceeded: false, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition, Status: metav1.ConditionUnknown, @@ -1000,8 +1029,9 @@ func Test_setDeletingCondition(t *testing.T) { name: "not deleting", machineDeployment: &clusterv1.MachineDeployment{}, machineSets: []*clusterv1.MachineSet{}, - machines: []*clusterv1.Machine{}, getAndAdoptMachineSetsForDeploymentSucceeded: true, + machines: []*clusterv1.Machine{}, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition, Status: metav1.ConditionFalse, @@ -1012,47 +1042,50 @@ func Test_setDeletingCondition(t *testing.T) { name: "Deleting with still some machine", machineDeployment: &clusterv1.MachineDeployment{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &metav1.Time{Time: time.Now()}}}, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(1)), }, + getAndAdoptMachineSetsForDeploymentSucceeded: true, machines: []*clusterv1.Machine{ fakeMachine("m1"), }, - getAndAdoptMachineSetsForDeploymentSucceeded: true, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition, - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: clusterv1.MachineDeploymentDeletingDeletionTimestampSetV1Beta2Reason, - Message: "Deleting 1 replicas", + Message: "Deleting 1 Machine", }, }, { name: "Deleting with still some stale machine", machineDeployment: &clusterv1.MachineDeployment{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &metav1.Time{Time: time.Now()}}}, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(1)), }, + getAndAdoptMachineSetsForDeploymentSucceeded: true, machines: []*clusterv1.Machine{ fakeMachine("m1", withStaleDeletion()), }, - getAndAdoptMachineSetsForDeploymentSucceeded: true, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition, - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: clusterv1.MachineDeploymentDeletingDeletionTimestampSetV1Beta2Reason, - Message: "Deleting 1 replicas and Machine m1 is in deletion since more than 30m", + Message: "Deleting 1 Machine and Machine m1 is in deletion since more than 30m", }, }, { name: "Deleting with no machines and a machine set still around", machineDeployment: &clusterv1.MachineDeployment{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &metav1.Time{Time: time.Now()}}}, machineSets: []*clusterv1.MachineSet{ - fakeMachineSet("ms1", withSpecReplicas(1)), + fakeMachineSet("ms1", withStatusReplicas(1)), }, - machines: []*clusterv1.Machine{}, getAndAdoptMachineSetsForDeploymentSucceeded: true, + machines: []*clusterv1.Machine{}, + getMachinesSucceeded: true, expectCondition: metav1.Condition{ Type: clusterv1.MachineDeploymentDeletingV1Beta2Condition, - Status: metav1.ConditionFalse, + Status: metav1.ConditionTrue, Reason: clusterv1.MachineDeploymentDeletingDeletionTimestampSetV1Beta2Reason, Message: "Deleting 1 MachineSets", }, @@ -1066,7 +1099,7 @@ func Test_setDeletingCondition(t *testing.T) { if tt.machines != nil { machines = collections.FromMachines(tt.machines...) } - setDeletingCondition(ctx, tt.machineDeployment, tt.machineSets, machines, tt.getAndAdoptMachineSetsForDeploymentSucceeded) + setDeletingCondition(ctx, tt.machineDeployment, tt.machineSets, machines, tt.getAndAdoptMachineSetsForDeploymentSucceeded, tt.getMachinesSucceeded) condition := v1beta2conditions.Get(tt.machineDeployment, clusterv1.MachineDeploymentDeletingV1Beta2Condition) g.Expect(condition).ToNot(BeNil()) @@ -1089,9 +1122,9 @@ func fakeMachineSet(name string, options ...fakeMachineSetOption) *clusterv1.Mac return p } -func withSpecReplicas(n int32) fakeMachineSetOption { +func withStatusReplicas(n int32) fakeMachineSetOption { return func(ms *clusterv1.MachineSet) { - ms.Spec.Replicas = &n + ms.Status.Replicas = n } } @@ -1147,7 +1180,7 @@ func withV1Beta2Condition(c metav1.Condition) fakeMachinesOption { if m.Status.V1Beta2 == nil { m.Status.V1Beta2 = &clusterv1.MachineV1Beta2Status{} } - meta.SetStatusCondition(&m.Status.V1Beta2.Conditions, c) + v1beta2conditions.Set(m, c) } } diff --git a/internal/controllers/machinedeployment/machinedeployment_sync.go b/internal/controllers/machinedeployment/machinedeployment_sync.go index db3ad69da27b..a2da7824b233 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync.go +++ b/internal/controllers/machinedeployment/machinedeployment_sync.go @@ -45,8 +45,8 @@ import ( // sync is responsible for reconciling deployments on scaling events or when they // are paused. -func (r *Reconciler) sync(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) error { - newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, false) +func (r *Reconciler) sync(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, templateExists bool) error { + newMS, oldMSs, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, false, templateExists) if err != nil { return err } @@ -76,7 +76,7 @@ func (r *Reconciler) sync(ctx context.Context, md *clusterv1.MachineDeployment, // // Note that currently the deployment controller is using caches to avoid querying the server for reads. // This may lead to stale reads of machine sets, thus incorrect deployment status. -func (r *Reconciler) getAllMachineSetsAndSyncRevision(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, createIfNotExisted bool) (*clusterv1.MachineSet, []*clusterv1.MachineSet, error) { +func (r *Reconciler) getAllMachineSetsAndSyncRevision(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, createIfNotExisted, templateExists bool) (*clusterv1.MachineSet, []*clusterv1.MachineSet, error) { reconciliationTime := metav1.Now() allOldMSs, err := mdutil.FindOldMachineSets(md, msList, &reconciliationTime) if err != nil { @@ -84,7 +84,7 @@ func (r *Reconciler) getAllMachineSetsAndSyncRevision(ctx context.Context, md *c } // Get new machine set with the updated revision number - newMS, err := r.getNewMachineSet(ctx, md, msList, allOldMSs, createIfNotExisted, &reconciliationTime) + newMS, err := r.getNewMachineSet(ctx, md, msList, allOldMSs, createIfNotExisted, templateExists, &reconciliationTime) if err != nil { return nil, nil, err } @@ -95,7 +95,7 @@ func (r *Reconciler) getAllMachineSetsAndSyncRevision(ctx context.Context, md *c // Returns a MachineSet that matches the intent of the given MachineDeployment. // If there does not exist such a MachineSet and createIfNotExisted is true, create a new MachineSet. // If there is already such a MachineSet, update it to propagate in-place mutable fields from the MachineDeployment. -func (r *Reconciler) getNewMachineSet(ctx context.Context, md *clusterv1.MachineDeployment, msList, oldMSs []*clusterv1.MachineSet, createIfNotExists bool, reconciliationTime *metav1.Time) (*clusterv1.MachineSet, error) { +func (r *Reconciler) getNewMachineSet(ctx context.Context, md *clusterv1.MachineDeployment, msList, oldMSs []*clusterv1.MachineSet, createIfNotExists, templateExists bool, reconciliationTime *metav1.Time) (*clusterv1.MachineSet, error) { // Try to find a MachineSet which matches the MachineDeployments intent, while ignore diffs between // the in-place mutable fields. // If we find a matching MachineSet we just update it to propagate any changes to the in-place mutable @@ -125,6 +125,10 @@ func (r *Reconciler) getNewMachineSet(ctx context.Context, md *clusterv1.Machine return nil, nil } + if !templateExists { + return nil, errors.New("cannot create a new MachineSet when templates do not exist") + } + // Create a new MachineSet and wait until the new MachineSet exists in the cache. newMS, err := r.createMachineSetAndWait(ctx, md, oldMSs, createReason) if err != nil { diff --git a/internal/controllers/machinedeployment/mdutil/util.go b/internal/controllers/machinedeployment/mdutil/util.go index 7e60ffa016a7..21b119a1861e 100644 --- a/internal/controllers/machinedeployment/mdutil/util.go +++ b/internal/controllers/machinedeployment/mdutil/util.go @@ -547,6 +547,7 @@ func GetAvailableReplicaCountForMachineSets(machineSets []*clusterv1.MachineSet) } // GetV1Beta2ReadyReplicaCountForMachineSets returns the number of ready machines corresponding to the given machine sets. +// Note: When none of the ms.Status.V1Beta2.ReadyReplicas are set, the func returns nil. func GetV1Beta2ReadyReplicaCountForMachineSets(machineSets []*clusterv1.MachineSet) *int32 { var totalReadyReplicas *int32 for _, ms := range machineSets { @@ -558,6 +559,7 @@ func GetV1Beta2ReadyReplicaCountForMachineSets(machineSets []*clusterv1.MachineS } // GetV1Beta2AvailableReplicaCountForMachineSets returns the number of available machines corresponding to the given machine sets. +// Note: When none of the ms.Status.V1Beta2.AvailableReplicas are set, the func returns nil. func GetV1Beta2AvailableReplicaCountForMachineSets(machineSets []*clusterv1.MachineSet) *int32 { var totalAvailableReplicas *int32 for _, ms := range machineSets { @@ -569,6 +571,7 @@ func GetV1Beta2AvailableReplicaCountForMachineSets(machineSets []*clusterv1.Mach } // GetV1Beta2UptoDateReplicaCountForMachineSets returns the number of up to date machines corresponding to the given machine sets. +// Note: When none of the ms.Status.V1Beta2.UpToDateReplicas are set, the func returns nil. func GetV1Beta2UptoDateReplicaCountForMachineSets(machineSets []*clusterv1.MachineSet) *int32 { var totalUpToDateReplicas *int32 for _, ms := range machineSets { diff --git a/internal/controllers/machineset/machineset_controller_status.go b/internal/controllers/machineset/machineset_controller_status.go index a47aee17777d..72309c4b98be 100644 --- a/internal/controllers/machineset/machineset_controller_status.go +++ b/internal/controllers/machineset/machineset_controller_status.go @@ -296,6 +296,10 @@ func calculateMissingReferencesMessage(ms *clusterv1.MachineSet, bootstrapTempla } func aggregateStaleMachines(machines []*clusterv1.Machine) string { + if len(machines) == 0 { + return "" + } + machineNames := []string{} for _, machine := range machines { if !machine.GetDeletionTimestamp().IsZero() && time.Since(machine.GetDeletionTimestamp().Time) > time.Minute*30 {