Edge cases corrected where all machineSets were scaling down to zero

pkg/controller/controller_suite_test.go

@@ -29,6 +29,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/watch"
 	coreinformers "k8s.io/client-go/informers"
 	v1core "k8s.io/client-go/kubernetes/typed/core/v1"
@@ -47,7 +48,7 @@ import (
 )
 
 func TestMachineControllerManagerSuite(t *testing.T) {
-	//for filtering out warning logs. Reflector short watch warning logs won't print now
+	// for filtering out warning logs. Reflector short watch warning logs won't print now
 	klog.SetOutput(io.Discard)
 	flags := &flag.FlagSet{}
 	klog.InitFlags(flags)
@@ -64,6 +65,99 @@ var (
 	TestMachineClass = "machineClass-0"
 )
 
+func newMachineDeployment(
+	specTemplate *v1alpha1.MachineTemplateSpec,
+	replicas int32,
+	minReadySeconds int32,
+	maxSurge int,
+	maxUnavailable int,
+	statusTemplate *v1alpha1.MachineDeploymentStatus,
+	owner *metav1.OwnerReference,
+	annotations map[string]string,
+	labels map[string]string,
+) *v1alpha1.MachineDeployment {
+	md := newMachineDeployments(1, specTemplate, replicas, minReadySeconds, statusTemplate, owner, annotations, labels)[0]
+	intStrMaxSurge := intstr.FromInt(maxSurge)
+	intStrMaxUnavailable := intstr.FromInt(maxUnavailable)
+	md.Spec.Strategy.RollingUpdate.MaxSurge = &intStrMaxSurge
+	md.Spec.Strategy.RollingUpdate.MaxUnavailable = &intStrMaxUnavailable
+
+	return md
+}
+
+func newMachineDeployments(
+	machineDeploymentCount int,
+	specTemplate *v1alpha1.MachineTemplateSpec,
+	replicas int32,
+	minReadySeconds int32,
+	statusTemplate *v1alpha1.MachineDeploymentStatus,
+	owner *metav1.OwnerReference,
+	annotations map[string]string,
+	labels map[string]string,
+) []*v1alpha1.MachineDeployment {
+
+	intStr1 := intstr.FromInt(1)
+	machineDeployments := make([]*v1alpha1.MachineDeployment, machineDeploymentCount)
+	for i := range machineDeployments {
+		machineDeployment := &v1alpha1.MachineDeployment{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "machine.sapcloud.io",
+				Kind:       "MachineDeployment",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      fmt.Sprintf("machinedeployment-%d", i),
+				Namespace: testNamespace,
+				Labels:    labels,
+			},
+			Spec: v1alpha1.MachineDeploymentSpec{
+				MinReadySeconds: minReadySeconds,
+				Replicas:        replicas,
+				Selector: &metav1.LabelSelector{
+					MatchLabels: deepCopy(specTemplate.ObjectMeta.Labels),
+				},
+				Strategy: v1alpha1.MachineDeploymentStrategy{
+					Type: v1alpha1.RollingUpdateMachineDeploymentStrategyType,
+					RollingUpdate: &v1alpha1.RollingUpdateMachineDeployment{
+						MaxSurge:       &intStr1,
+						MaxUnavailable: &intStr1,
+					},
+				},
+				Template: *specTemplate.DeepCopy(),
+			},
+		}
+
+		if statusTemplate != nil {
+			machineDeployment.Status = *statusTemplate.DeepCopy()
+		}
+
+		if owner != nil {
+			machineDeployment.OwnerReferences = append(machineDeployment.OwnerReferences, *owner.DeepCopy())
+		}
+
+		if annotations != nil {
+			machineDeployment.Annotations = annotations
+		}
+
+		machineDeployments[i] = machineDeployment
+	}
+	return machineDeployments
+}
+
+func newMachineSet(
+	specTemplate *v1alpha1.MachineTemplateSpec,
+	name string,
+	replicas int32,
+	minReadySeconds int32,
+	statusTemplate *v1alpha1.MachineSetStatus,
+	owner *metav1.OwnerReference,
+	annotations map[string]string,
+	labels map[string]string,
+) *v1alpha1.MachineSet {
+	ms := newMachineSets(1, specTemplate, replicas, minReadySeconds, statusTemplate, owner, annotations, labels)[0]
+	ms.Name = name
+	return ms
+}
+
 func newMachineSets(
 	machineSetCount int,
 	specTemplate *v1alpha1.MachineTemplateSpec,
@@ -115,6 +209,41 @@ func newMachineSets(
 	return machineSets
 }
 
+func modifyMachineSet(ms *v1alpha1.MachineSet, specTemplate *v1alpha1.MachineTemplateSpec,
+	replicas int32,
+	minReadySeconds int32,
+	statusTemplate *v1alpha1.MachineSetStatus,
+	owner *metav1.OwnerReference,
+	annotations map[string]string,
+	labels map[string]string) *v1alpha1.MachineSet {
+
+	if specTemplate != nil {
+		ms.Spec.Template = *specTemplate.DeepCopy()
+	}
+
+	Expect(ms).To(Not(BeNil()))
+	fmt.Println(*ms)
+	Expect(ms.Spec).To(Not(BeEmpty()))
+	ms.Spec.Replicas = replicas
+	ms.Spec.MinReadySeconds = minReadySeconds
+	if statusTemplate != nil {
+		ms.Status = *statusTemplate.DeepCopy()
+
+	}
+	if owner != nil {
+		ms.OwnerReferences = append(ms.OwnerReferences, *owner.DeepCopy())
+	}
+	if annotations != nil {
+		ms.Annotations = annotations
+	}
+
+	if labels != nil {
+		ms.Labels = labels
+	}
+
+	return ms
+}
+
 func deepCopy(m map[string]string) map[string]string {
 	r := make(map[string]string, len(m))
 	for k := range m {

pkg/controller/deployment_sync.go

@@ -414,6 +414,7 @@ func (dc *controller) scale(ctx context.Context, deployment *v1alpha1.MachineDep
 	// If the new machine set is saturated, old machine sets should be fully scaled down.
 	// This case handles machine set adoption during a saturated new machine set.
 	if IsSaturated(deployment, newIS) {
+		fmt.Printf("Scaling old active machineSets as new machineSet %s is saturated", newIS.Name)
 		klog.V(3).Infof("Scaling old active machineSets as new machineSet %s is saturated", newIS.Name)
 		for _, old := range FilterActiveMachineSets(oldISs) {
 			if _, _, err := dc.scaleMachineSetAndRecordEvent(ctx, old, 0, deployment); err != nil {
@@ -429,6 +430,7 @@ func (dc *controller) scale(ctx context.Context, deployment *v1alpha1.MachineDep
 	// - Scale down ? -> scale down the old machineSets proportionally
 	if IsRollingUpdate(deployment) {
 		klog.V(3).Infof("Scaling all active machineSets proportionally for scale-in, while scaling up latest machineSet only for scale-out, machineDeployment %s", deployment.Name)
+		fmt.Printf("Scaling all active machineSets proportionally for scale-in, while scaling up latest machineSet only for scale-out, machineDeployment %s", deployment.Name)
 		allISs := FilterActiveMachineSets(append(oldISs, newIS))
 		allISsReplicas := GetReplicaCountForMachineSets(allISs)
 
@@ -451,7 +453,7 @@ func (dc *controller) scale(ctx context.Context, deployment *v1alpha1.MachineDep
 		nameToSize := make(map[string]int32)
 		deploymentReplicasAdded := int32(0)
 		switch {
-		case deploymentReplicasToAdd > 0:
+		case deploymentReplicasToAdd >= 0:
 			scalingOperation = "up"
 			nameToSize = dc.scaleNewMachineSet(newIS, allISs, deploymentReplicasToAdd, deployment)
 			deploymentReplicasAdded = deploymentReplicasToAdd
@@ -673,6 +675,7 @@ func (dc *controller) isScalingEvent(ctx context.Context, d *v1alpha1.MachineDep
 			continue
 		}
 		if desired != (d.Spec.Replicas) {
+			klog.V(2).Infof("Desired replicas annotation value: %d on machineSet %s, Spec Desired Replicas value: %d on corresponding machineDeployment, so scaling has happened.", desired, is.Name, d.Spec.Replicas)
 			return true, nil
 		}
 	}