Merge pull request #5503 from mikesplain/fix_suspendprocess

Fix suspendprocess

pkg/model/awsmodel/autoscalinggroup.go

@@ -230,11 +230,11 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
 			}
 			t.Tags = tags
 
-			if ig.Spec.SuspendProcesses != nil {
-				for _, p := range ig.Spec.SuspendProcesses {
-					t.SuspendProcesses = append(t.SuspendProcesses, p)
-				}
+			processes := []string{}
+			for _, p := range ig.Spec.SuspendProcesses {
+				processes = append(processes, p)
 			}
+			t.SuspendProcesses = &processes
 
 			c.AddTask(t)
 		}

pkg/model/bootstrapscript.go

@@ -191,7 +191,6 @@ func (b *BootstrapScript) ResourceNodeUp(ig *kops.InstanceGroup, cluster *kops.C
 			spec["kubelet"] = ig.Spec.Kubelet
 			spec["nodeLabels"] = ig.Spec.NodeLabels
 			spec["taints"] = ig.Spec.Taints
-			spec["suspendProcesses"] = ig.Spec.SuspendProcesses
 
 			hooks, err := b.getRelevantHooks(ig.Spec.Hooks, ig.Spec.Role)
 			if err != nil {

pkg/model/tests/data/bootstrapscript_0.txt

@@ -200,8 +200,6 @@ kubelet:
 nodeLabels:
   label2: value2
   labelname: labelvalue
-suspendProcesses:
-- AZRebalance
 taints:
 - key1=value1:NoSchedule
 - key2=value2:NoExecute

pkg/model/tests/data/bootstrapscript_1.txt

@@ -217,8 +217,6 @@ kubelet:
 nodeLabels:
   label2: value2
   labelname: labelvalue
-suspendProcesses:
-- AZRebalance
 taints:
 - key1=value1:NoSchedule
 - key2=value2:NoExecute

pkg/model/tests/data/bootstrapscript_2.txt

@@ -217,8 +217,6 @@ kubelet:
 nodeLabels:
   label2: value2
   labelname: labelvalue
-suspendProcesses:
-- AZRebalance
 taints:
 - key1=value1:NoSchedule
 - key2=value2:NoExecute

pkg/model/tests/data/bootstrapscript_3.txt

@@ -185,8 +185,6 @@ kubelet:
 nodeLabels:
   label2: value2
   labelname: labelvalue
-suspendProcesses:
-- AZRebalance
 taints:
 - key1=value1:NoSchedule
 - key2=value2:NoExecute

pkg/model/tests/data/bootstrapscript_4.txt

@@ -202,8 +202,6 @@ kubelet:
 nodeLabels:
   label2: value2
   labelname: labelvalue
-suspendProcesses:
-- AZRebalance
 taints:
 - key1=value1:NoSchedule
 - key2=value2:NoExecute

pkg/model/tests/data/bootstrapscript_5.txt

@@ -202,8 +202,6 @@ kubelet:
 nodeLabels:
   label2: value2
   labelname: labelvalue
-suspendProcesses:
-- AZRebalance
 taints:
 - key1=value1:NoSchedule
 - key2=value2:NoExecute

tests/integration/update_cluster/additional_cidr/cloudformation.json.extracted.yaml

@@ -247,7 +247,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1bmastersadditionalcidrex
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC
@@ -455,7 +454,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesadditionalcidrexamplecom.Propert
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC

tests/integration/update_cluster/additional_user-data/cloudformation.json.extracted.yaml

@@ -256,7 +256,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersadditionaluserda
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC
@@ -485,7 +484,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesadditionaluserdataexamplecom.Pro
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC

tests/integration/update_cluster/existing_iam_cloudformation/cloudformation.json.extracted.yaml

@@ -247,7 +247,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersminimalexampleco
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC
@@ -455,7 +454,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesminimalexamplecom.Properties.Use
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC

tests/integration/update_cluster/externallb/cloudformation.json.extracted.yaml

@@ -267,7 +267,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersexternallbexampl
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC
@@ -480,7 +479,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesexternallbexamplecom.Properties.
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC

tests/integration/update_cluster/minimal-cloudformation/cloudformation.json.extracted.yaml

@@ -247,7 +247,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersminimalexampleco
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC
@@ -455,7 +454,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesminimalexamplecom.Properties.Use
   cat > ig_spec.yaml << '__EOF_IG_SPEC'
   kubelet: null
   nodeLabels: null
-  suspendProcesses: null
   taints: null
 
   __EOF_IG_SPEC

upup/pkg/fi/cloudup/awstasks/autoscalinggroup.go

@@ -49,7 +49,7 @@ type AutoscalingGroup struct {
 
 	LaunchConfiguration *LaunchConfiguration
 
-	SuspendProcesses []string
+	SuspendProcesses *[]string
 }
 
 var _ fi.CompareWithID = &AutoscalingGroup{}
@@ -146,6 +146,13 @@ func (e *AutoscalingGroup) Find(c *fi.Context) (*AutoscalingGroup, error) {
 		actual.Subnets = e.Subnets
 	}
 
+	processes := []string{}
+	for _, p := range g.SuspendedProcesses {
+		processes = append(processes, *p.ProcessName)
+	}
+
+	actual.SuspendProcesses = &processes
+
 	// Avoid spurious changes
 	actual.Lifecycle = e.Lifecycle
 
@@ -227,6 +234,21 @@ func (_ *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos
 			return fmt.Errorf("error enabling metrics collection for AutoscalingGroup: %v", err)
 		}
 
+		if len(*e.SuspendProcesses) > 0 {
+			toSuspend := []*string{}
+			for _, p := range *e.SuspendProcesses {
+				toSuspend = append(toSuspend, &p)
+			}
+
+			processQuery := &autoscaling.ScalingProcessQuery{}
+			processQuery.AutoScalingGroupName = e.Name
+			processQuery.ScalingProcesses = toSuspend
+
+			_, err := t.Cloud.Autoscaling().SuspendProcesses(processQuery)
+			if err != nil {
+				return fmt.Errorf("error suspending processes: %v", err)
+			}
+		}
 	} else {
 		request := &autoscaling.UpdateAutoScalingGroupInput{
 			AutoScalingGroupName: e.Name,
@@ -282,6 +304,33 @@ func (_ *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos
 			}
 		}
 
+		if changes.SuspendProcesses != nil {
+			toSuspend := processCompare(e.SuspendProcesses, a.SuspendProcesses)
+			toResume := processCompare(a.SuspendProcesses, e.SuspendProcesses)
+
+			if len(toSuspend) > 0 {
+				suspendProcessQuery := &autoscaling.ScalingProcessQuery{}
+				suspendProcessQuery.AutoScalingGroupName = e.Name
+				suspendProcessQuery.ScalingProcesses = toSuspend
+
+				_, err := t.Cloud.Autoscaling().SuspendProcesses(suspendProcessQuery)
+				if err != nil {
+					return fmt.Errorf("error suspending processes: %v", err)
+				}
+			}
+			if len(toResume) > 0 {
+				resumeProcessQuery := &autoscaling.ScalingProcessQuery{}
+				resumeProcessQuery.AutoScalingGroupName = e.Name
+				resumeProcessQuery.ScalingProcesses = toResume
+
+				_, err := t.Cloud.Autoscaling().ResumeProcesses(resumeProcessQuery)
+				if err != nil {
+					return fmt.Errorf("error resuming processes: %v", err)
+				}
+			}
+			changes.SuspendProcesses = nil
+		}
+
 		empty := &AutoscalingGroup{}
 		if !reflect.DeepEqual(empty, changes) {
 			glog.Warningf("cannot apply changes to AutoScalingGroup: %v", changes)
@@ -306,26 +355,30 @@ func (_ *AutoscalingGroup) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Autos
 		}
 	}
 
-	if e.SuspendProcesses != nil {
-		processQuery := &autoscaling.ScalingProcessQuery{}
-		processQuery.AutoScalingGroupName = e.Name
-		processQuery.ScalingProcesses = []*string{}
-
-		for _, p := range e.SuspendProcesses {
-			processQuery.ScalingProcesses = append(processQuery.ScalingProcesses, &p)
-		}
-
-		_, err := t.Cloud.Autoscaling().SuspendProcesses(processQuery)
-		if err != nil {
-			return fmt.Errorf("error suspending processes: %v", err)
-		}
-	}
-
 	// TODO: Use PropagateAtLaunch = false for tagging?
 
 	return nil // We have
 }
 
+// processCompare returns processes that exist in a but not in b
+func processCompare(a *[]string, b *[]string) []*string {
+	notInB := []*string{}
+	for _, ap := range *a {
+		found := false
+		for _, bp := range *b {
+			if ap == bp {
+				found = true
+				break
+			}
+		}
+		if !found {
+			notFound := ap
+			notInB = append(notInB, &notFound)
+		}
+	}
+	return notInB
+}
+
 // getASGTagsToDelete loops through the currently set tags and builds a list of
 // tags to be deleted from the Autoscaling Group
 func (e *AutoscalingGroup) getASGTagsToDelete(currentTags map[string]string) []*autoscaling.Tag {
@@ -429,7 +482,7 @@ func (_ *AutoscalingGroup) RenderTerraform(t *terraform.TerraformTarget, a, e, c
 
 	var processes []*string
 	if e.SuspendProcesses != nil {
-		for _, p := range e.SuspendProcesses {
+		for _, p := range *e.SuspendProcesses {
 			processes = append(processes, fi.String(p))
 		}
 	}

upup/pkg/fi/cloudup/awstasks/autoscalinggroup_test.go

@@ -127,3 +127,72 @@ func TestGetASGTagsToDelete(t *testing.T) {
 		}
 	}
 }
+
+func TestProcessCompare(t *testing.T) {
+	rebalance := "AZRebalance"
+	healthcheck := "HealthCheck"
+
+	a := []string{}
+	b := []string{
+		rebalance,
+	}
+	c := []string{
+		rebalance,
+		healthcheck,
+	}
+
+	cases := []struct {
+		A                 *[]string
+		B                 *[]string
+		ExpectedProcesses []*string
+	}{
+		{
+			A:                 &a,
+			B:                 &b,
+			ExpectedProcesses: []*string{},
+		},
+		{
+			A: &b,
+			B: &a,
+			ExpectedProcesses: []*string{
+				&rebalance,
+			},
+		},
+		{
+			A: &c,
+			B: &b,
+			ExpectedProcesses: []*string{
+				&healthcheck,
+			},
+		},
+		{
+			A: &c,
+			B: &a,
+			ExpectedProcesses: []*string{
+				&rebalance,
+				&healthcheck,
+			},
+		},
+	}
+
+	for i, x := range cases {
+		result := processCompare(x.A, x.B)
+
+		expected, err := yaml.Marshal(x.ExpectedProcesses)
+		if err != nil {
+			t.Errorf("case %d, unexpected error converting expected processes to yaml: %v", i, err)
+		}
+
+		actual, err := yaml.Marshal(result)
+		if err != nil {
+			t.Errorf("case %d, unexpected error converting actual result to yaml: %v", i, err)
+		}
+
+		if string(expected) != string(actual) {
+			diffString := diff.FormatDiff(string(expected), string(actual))
+			t.Errorf("case %d failed, actual output differed from expected.", i)
+			t.Logf("diff:\n%s\n", diffString)
+
+		}
+	}
+}