Repository: keikoproj/instance-manager
Pull request #132 (2020-06-02 15:40:21)

Suspend processes
Fixes https://github.com/keikoproj/instance-manager/issues/119

Assumption: In the desired/target state, only the processes mentioned in the input yaml need to be suspended on the ASG
**Scenarios tested:**
**During Create**
1. When the suspendProcesses config is not present, then no action taken
2. When the config is passed, post ASG creation, the processes mentioned are suspended using a single AWS API call

**During Update**

1. When the target list mentioned in yaml is different from current list of suspended processes in ASG, only then it is updated (resumed/suspended)


@eytan-avisror please review and let me know if you would like me to improve anything.
Thank you

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/keikoproj/instance-manager?pullRequest=132) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/keikoproj/instance-manager?pullRequest=132) before we can accept your contribution.<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/keikoproj/instance-manager?pullRequest=132) it.</sub>
codecov[bot]: # [Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/132?src=pr&el=h1) Report
> Merging [#132](https://codecov.io/gh/keikoproj/instance-manager/pull/132?src=pr&el=desc) into [master](https://codecov.io/gh/keikoproj/instance-manager/commit/138d4b1ae3b0f7f987f0825ffda45040ba3a5880&el=desc) will **decrease** coverage by `0.17%`.
> The diff coverage is `81.57%`.

[![Impacted file tree graph](https://codecov.io/gh/keikoproj/instance-manager/pull/132/graphs/tree.svg?width=650&height=150&src=pr&token=IJbjmSBliL)](https://codecov.io/gh/keikoproj/instance-manager/pull/132?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
- Coverage   88.87%   88.70%   -0.18%     
==========================================
  Files          10       10              
  Lines        1187     1231      +44     
==========================================
+ Hits         1055     1092      +37     
- Misses         91       95       +4     
- Partials       41       44       +3     
```


| [Impacted Files](https://codecov.io/gh/keikoproj/instance-manager/pull/132?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [controllers/provisioners/eks/create.go](https://codecov.io/gh/keikoproj/instance-manager/pull/132/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9jcmVhdGUuZ28=) | `93.10% <60.00%> (-2.02%)` | :arrow_down: |
| [controllers/provisioners/eks/update.go](https://codecov.io/gh/keikoproj/instance-manager/pull/132/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy91cGRhdGUuZ28=) | `95.84% <84.84%> (-1.57%)` | :arrow_down: |
| [controllers/provisioners/eks/eks.go](https://codecov.io/gh/keikoproj/instance-manager/pull/132/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9la3MuZ28=) | `65.78% <0.00%> (ø)` | |
| [controllers/provisioners/eks/helpers.go](https://codecov.io/gh/keikoproj/instance-manager/pull/132/diff?src=pr&el=tree#diff-Y29udHJvbGxlcnMvcHJvdmlzaW9uZXJzL2Vrcy9oZWxwZXJzLmdv) | `87.92% <0.00%> (+0.22%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/132?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/keikoproj/instance-manager/pull/132?src=pr&el=footer). Last update [138d4b1...bd2447f](https://codecov.io/gh/keikoproj/instance-manager/pull/132?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

eytan-avisror: Thanks for the PR and contribution @SaiVishwas 
Some minor comments added.
SaiVishwas: Hi @eytan-avisror ,
I have made the suggested changes. The newly added methods are being tested as part of the unit tests. I went through the code coverage report and drop in coverage is due to the 
```
err != nil { return err }
```
code block.

Thank you
eytan-avisror: Thanks @SaiVishwas 
We should be able to merge this shortly, had one more question above.
eytan-avisror: 15 scenarios (15 passed)
72 steps (72 passed)
22m41.99080058s
testing: warning: no tests to run
PASS
ok  	github.com/keikoproj/instance-manager/test-bdd	1362.315s [no tests to run]

Patch:
diff --git a/api/v1alpha1/instancegroup_types.go b/api/v1alpha1/instancegroup_types.go
index f368cab..5406e20 100644
--- a/api/v1alpha1/instancegroup_types.go
+++ b/api/v1alpha1/instancegroup_types.go
@@ -153,6 +153,7 @@ type EKSConfiguration struct {
 	NodeSecurityGroups          []string            `json:"securityGroups,omitempty"`
 	Volumes                     []NodeVolume        `json:"volumes,omitempty"`
 	Subnets                     []string            `json:"subnets"`
+	SuspendedProcesses          []string            `json:"suspendProcesses,omitempty"`
 	BootstrapArguments          string              `json:"bootstrapArguments,omitempty"`
 	SpotPrice                   string              `json:"spotPrice,omitempty"`
 	Tags                        []map[string]string `json:"tags,omitempty"`
@@ -372,6 +373,12 @@ func (c *EKSConfiguration) GetSubnets() []string {
 	}
 	return c.Subnets
 }
+func (c *EKSConfiguration) GetSuspendProcesses() []string {
+	if c.SuspendedProcesses == nil {
+		return []string{}
+	}
+	return c.SuspendedProcesses
+}
 func (c *EKSConfiguration) GetSpotPrice() string {
 	return c.SpotPrice
 }
@@ -381,6 +388,9 @@ func (c *EKSConfiguration) SetSpotPrice(price string) {
 func (c *EKSConfiguration) SetSubnets(subnets []string) {
 	c.Subnets = subnets
 }
+func (c *EKSConfiguration) SetSuspendProcesses(suspendProcesses []string) {
+	c.SuspendedProcesses = suspendProcesses
+}
 func (spec *EKSSpec) GetMaxSize() int64 {
 	return spec.MaxSize
 }
diff --git a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
index 5ff1a38..54bb3f7 100644
--- a/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
+++ b/config/crd/bases/instancemgr.keikoproj.io_instancegroups.yaml
@@ -110,6 +110,10 @@ spec:
                       items:
                         type: string
                       type: array
+                    suspendProcesses:
+                      items:
+                        type: string
+                      type: array
                     tags:
                       items:
                         additionalProperties:
diff --git a/controllers/common/utils.go b/controllers/common/utils.go
index e508c6b..a72e79b 100644
--- a/controllers/common/utils.go
+++ b/controllers/common/utils.go
@@ -125,3 +125,20 @@ func GetTimeString() string {
 	n := time.Now().UTC()
 	return n.Format("20060102150405")
 }
+
+// Set Difference: A - B
+func Difference(a, b []string) []string {
+	var diff []string
+	m := make(map[string]bool)
+
+	for _, item := range b {
+		m[item] = true
+	}
+
+	for _, item := range a {
+		if _, ok := m[item]; !ok {
+			diff = append(diff, item)
+		}
+	}
+	return diff
+}
diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go
index c68dd3d..d98152f 100644
--- a/controllers/providers/aws/aws.go
+++ b/controllers/providers/aws/aws.go
@@ -201,6 +201,30 @@ func (w *AwsWorker) DeleteScalingGroup(name string) error {
 	return nil
 }
 
+func (w *AwsWorker) SetSuspendProcesses(name string, processesToSuspend []string) error {
+	input := &autoscaling.ScalingProcessQuery{
+		AutoScalingGroupName: aws.String(name),
+		ScalingProcesses:     aws.StringSlice(processesToSuspend),
+	}
+	_, err := w.AsgClient.SuspendProcesses(input)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func (w *AwsWorker) SetResumeProcesses(name string, processesToResume []string) error {
+	input := &autoscaling.ScalingProcessQuery{
+		AutoScalingGroupName: aws.String(name),
+		ScalingProcesses:     aws.StringSlice(processesToResume),
+	}
+	_, err := w.AsgClient.ResumeProcesses(input)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 func (w *AwsWorker) GetBasicUserData(clusterName, bootstrapArgs string) string {
 	userData := fmt.Sprintf(DefaultEksUserDataFmt(), clusterName, bootstrapArgs)
 	return base64.StdEncoding.EncodeToString([]byte(userData))
diff --git a/controllers/provisioners/eks/create.go b/controllers/provisioners/eks/create.go
index 6243c16..cccec81 100644
--- a/controllers/provisioners/eks/create.go
+++ b/controllers/provisioners/eks/create.go
@@ -89,6 +89,10 @@ func (ctx *EksInstanceGroupContext) CreateScalingGroup(lcName string) error {
 	}
 	ctx.Log.Info("created scaling group", "instancegroup", instanceGroup.GetName(), "scalinggroup", asgName)
 
+	if err := ctx.UpdateScalingProcesses(asgName); err != nil {
+		return err
+	}
+
 	if err := ctx.UpdateMetricsCollection(asgName); err != nil {
 		return err
 	}
diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go
index 1285b5e..34980c8 100644
--- a/controllers/provisioners/eks/helpers.go
+++ b/controllers/provisioners/eks/helpers.go
@@ -110,6 +110,37 @@ func (ctx *EksInstanceGroupContext) GetRemovedTags(asgName string) []*autoscalin
 	return removal
 }
 
+func (ctx *EksInstanceGroupContext) UpdateScalingProcesses(asgName string) error {
+	var (
+		instanceGroup         = ctx.GetInstanceGroup()
+		configuration         = instanceGroup.GetEKSConfiguration()
+		state                 = ctx.GetDiscoveredState()
+		scalingGroup          = state.GetScalingGroup()
+		specSuspendProcesses  = configuration.GetSuspendProcesses()
+		groupSuspendProcesses []string
+	)
+
+	for _, element := range scalingGroup.SuspendedProcesses {
+		groupSuspendProcesses = append(groupSuspendProcesses, *element.ProcessName)
+	}
+
+	if suspend := common.Difference(specSuspendProcesses, groupSuspendProcesses); len(suspend) > 0 {
+		if err := ctx.AwsWorker.SetSuspendProcesses(asgName, suspend); err != nil {
+			return err
+		}
+		ctx.Log.Info("suspended scaling processes", "instancegroup", instanceGroup.GetName(), "scalinggroup", asgName, "processes", suspend)
+	}
+
+	if resume := common.Difference(groupSuspendProcesses, specSuspendProcesses); len(resume) > 0 {
+		if err := ctx.AwsWorker.SetResumeProcesses(asgName, resume); err != nil {
+			return err
+		}
+		ctx.Log.Info("resumed scaling processes", "instancegroup", instanceGroup.GetName(), "scalinggroup", asgName, "processes", resume)
+	}
+
+	return nil
+}
+
 func (ctx *EksInstanceGroupContext) GetBlockDeviceList() []*autoscaling.BlockDeviceMapping {
 	var (
 		devices       []*autoscaling.BlockDeviceMapping
diff --git a/controllers/provisioners/eks/update.go b/controllers/provisioners/eks/update.go
index 3e907d7..ecadfbe 100644
--- a/controllers/provisioners/eks/update.go
+++ b/controllers/provisioners/eks/update.go
@@ -116,6 +116,10 @@ func (ctx *EksInstanceGroupContext) UpdateScalingGroup() error {
 		ctx.Log.Info("updated scaling group tags", "instancegroup", instanceGroup.GetName(), "scalinggroup", asgName)
 	}
 
+	if err := ctx.UpdateScalingProcesses(asgName); err != nil {
+		return err
+	}
+
 	if err := ctx.UpdateMetricsCollection(asgName); err != nil {
 		return err
 	}


Test patch:
diff --git a/controllers/provisioners/eks/eks_test.go b/controllers/provisioners/eks/eks_test.go
index 2e2a3c6..54a140c 100644
--- a/controllers/provisioners/eks/eks_test.go
+++ b/controllers/provisioners/eks/eks_test.go
@@ -102,6 +102,9 @@ func MockInstanceGroup() *v1alpha1.InstanceGroup {
 				MinSize: 1,
 				EKSConfiguration: &v1alpha1.EKSConfiguration{
 					EksClusterName: "my-cluster",
+					SuspendedProcesses: []string{
+						"AZRebalance",
+					},
 				},
 			},
 			AwsUpgradeStrategy: v1alpha1.AwsUpgradeStrategy{
@@ -294,6 +297,7 @@ type MockAutoScalingClient struct {
 	TerminateInstanceInAutoScalingGroupErr error
 	EnableMetricsCollectionErr             error
 	DisableMetricsCollectionErr            error
+	UpdateSuspendProcessesErr              error
 	DeleteLaunchConfigurationCallCount     int
 	LaunchConfiguration                    *autoscaling.LaunchConfiguration
 	LaunchConfigurations                   []*autoscaling.LaunchConfiguration
@@ -368,6 +372,14 @@ func (a *MockAutoScalingClient) UpdateAutoScalingGroup(input *autoscaling.Update
 	return &autoscaling.UpdateAutoScalingGroupOutput{}, a.UpdateAutoScalingGroupErr
 }
 
+func (a *MockAutoScalingClient) SuspendProcesses(input *autoscaling.ScalingProcessQuery) (*autoscaling.SuspendProcessesOutput, error) {
+	return &autoscaling.SuspendProcessesOutput{}, a.UpdateSuspendProcessesErr
+}
+
+func (a *MockAutoScalingClient) ResumeProcesses(input *autoscaling.ScalingProcessQuery) (*autoscaling.ResumeProcessesOutput, error) {
+	return &autoscaling.ResumeProcessesOutput{}, a.UpdateSuspendProcessesErr
+}
+
 type MockEksClient struct {
 	eksiface.EKSAPI
 	DescribeClusterErr error
diff --git a/controllers/provisioners/eks/update_test.go b/controllers/provisioners/eks/update_test.go
index 380a601..899a0c1 100644
--- a/controllers/provisioners/eks/update_test.go
+++ b/controllers/provisioners/eks/update_test.go
@@ -81,6 +81,11 @@ func TestUpdateScalingGroupPositive(t *testing.T) {
 				Value: aws.String("some-value"),
 			},
 		},
+		SuspendedProcesses: []*autoscaling.SuspendedProcess{
+			{
+				ProcessName: aws.String("ScheduledActions"),
+			},
+		},
 	}
 	asgMock.AutoScalingGroups = []*autoscaling.Group{mockScalingGroup}
 
@@ -120,6 +125,7 @@ func TestUpdateScalingGroupPositive(t *testing.T) {
 	err = ctx.Update()
 	g.Expect(err).NotTo(gomega.HaveOccurred())
 	g.Expect(ctx.TagsUpdateNeeded()).To(gomega.BeTrue())
+
 	g.Expect(ctx.GetState()).To(gomega.Equal(v1alpha1.ReconcileModified))
 }