Terminate AWS instances through EC2 instead of Autoscaling

cloudmock/aws/mockautoscaling/BUILD.bazel

@@ -5,6 +5,7 @@ go_library(
     srcs = [
         "api.go",
         "attach.go",
+        "ec2shim.go",
         "group.go",
         "launchconfigurations.go",
         "tags.go",
@@ -16,6 +17,8 @@ go_library(
         "//vendor/github.com/aws/aws-sdk-go/aws/request:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/service/autoscaling:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface:go_default_library",
+        "//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library",
+        "//vendor/github.com/aws/aws-sdk-go/service/ec2/ec2iface:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
     ],
 )

cloudmock/aws/mockautoscaling/ec2shim.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package mockautoscaling
+
+import (
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/autoscaling"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
+)
+
+type ec2Shim struct {
+	ec2iface.EC2API
+	mockAutoscaling *MockAutoscaling
+}
+
+func (m *MockAutoscaling) GetEC2Shim(e ec2iface.EC2API) ec2iface.EC2API {
+	return &ec2Shim{
+		EC2API:          e,
+		mockAutoscaling: m,
+	}
+}
+
+func (e *ec2Shim) TerminateInstances(input *ec2.TerminateInstancesInput) (*ec2.TerminateInstancesOutput, error) {
+	if input.DryRun != nil && *input.DryRun {
+		return &ec2.TerminateInstancesOutput{}, nil
+	}
+	for _, id := range input.InstanceIds {
+		request := &autoscaling.TerminateInstanceInAutoScalingGroupInput{
+			InstanceId:                     id,
+			ShouldDecrementDesiredCapacity: aws.Bool(false),
+		}
+		if _, err := e.mockAutoscaling.TerminateInstanceInAutoScalingGroup(request); err != nil {
+			return nil, err
+		}
+	}
+	return &ec2.TerminateInstancesOutput{}, nil
+}

pkg/instancegroups/BUILD.bazel

@@ -44,7 +44,8 @@ go_test(
         "//upup/pkg/fi/cloudup/awsup:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
         "//vendor/github.com/aws/aws-sdk-go/service/autoscaling:go_default_library",
-        "//vendor/github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface:go_default_library",
+        "//vendor/github.com/aws/aws-sdk-go/service/ec2:go_default_library",
+        "//vendor/github.com/aws/aws-sdk-go/service/ec2/ec2iface:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

pkg/instancegroups/rollingupdate_test.go

@@ -25,7 +25,8 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
-	"github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
 	v1meta "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -49,7 +50,9 @@ func getTestSetup() (*RollingUpdateCluster, *awsup.MockAWSCloud, *kopsapi.Cluste
 	k8sClient := fake.NewSimpleClientset()
 
 	mockcloud := awsup.BuildMockAWSCloud("us-east-1", "abc")
-	mockcloud.MockAutoscaling = &mockautoscaling.MockAutoscaling{}
+	mockAutoscaling := &mockautoscaling.MockAutoscaling{}
+	mockcloud.MockAutoscaling = mockAutoscaling
+	mockcloud.MockEC2 = mockAutoscaling.GetEC2Shim(mockcloud.MockEC2)
 
 	cluster := &kopsapi.Cluster{}
 	cluster.Name = "test.k8s.local"
@@ -672,7 +675,7 @@ func TestRollingUpdateDisabledCloudonly(t *testing.T) {
 //                                 <-- validated
 
 type concurrentTest struct {
-	autoscalingiface.AutoScalingAPI
+	ec2iface.EC2API
 	t                       *testing.T
 	mutex                   sync.Mutex
 	terminationRequestsLeft int
@@ -727,29 +730,35 @@ func (c *concurrentTest) Validate() (*validation.ValidationCluster, error) {
 	return &validation.ValidationCluster{}, nil
 }
 
-func (c *concurrentTest) TerminateInstanceInAutoScalingGroup(input *autoscaling.TerminateInstanceInAutoScalingGroupInput) (*autoscaling.TerminateInstanceInAutoScalingGroupOutput, error) {
+func (c *concurrentTest) TerminateInstances(input *ec2.TerminateInstancesInput) (*ec2.TerminateInstancesOutput, error) {
+	if input.DryRun != nil && *input.DryRun {
+		return &ec2.TerminateInstancesOutput{}, nil
+	}
+
 	c.mutex.Lock()
 	defer c.mutex.Unlock()
 
-	terminationRequestsLeft := c.terminationRequestsLeft
+	for range input.InstanceIds {
-	c.terminationRequestsLeft--
+		terminationRequestsLeft := c.terminationRequestsLeft
-	switch terminationRequestsLeft {
+		c.terminationRequestsLeft--
-	case 7, 2, 1:
+		switch terminationRequestsLeft {
-		assert.Equal(c.t, terminationRequestsLeft, c.previousValidation, "previous validation")
+		case 7, 2, 1:
-	case 6, 4:
+			assert.Equal(c.t, terminationRequestsLeft, c.previousValidation, "previous validation")
-		assert.Equal(c.t, terminationRequestsLeft, c.previousValidation, "previous validation")
+		case 6, 4:
-		c.mutex.Unlock()
+			assert.Equal(c.t, terminationRequestsLeft, c.previousValidation, "previous validation")
-		select {
+			c.mutex.Unlock()
-		case <-c.terminationChan:
+			select {
-		case <-time.After(1 * time.Second):
+			case <-c.terminationChan:
-			c.t.Error("timed out reading from terminationChan")
+			case <-time.After(1 * time.Second):
+				c.t.Error("timed out reading from terminationChan")
+			}
+			c.mutex.Lock()
+			go c.delayThenWakeValidation()
+		case 5, 3:
+			assert.Equal(c.t, terminationRequestsLeft+1, c.previousValidation, "previous validation")
 		}
-		c.mutex.Lock()
-		go c.delayThenWakeValidation()
-	case 5, 3:
-		assert.Equal(c.t, terminationRequestsLeft+1, c.previousValidation, "previous validation")
 	}
-	return c.AutoScalingAPI.TerminateInstanceInAutoScalingGroup(input)
+	return c.EC2API.TerminateInstances(input)
 }
 
 func (c *concurrentTest) delayThenWakeValidation() {
@@ -769,7 +778,7 @@ func (c *concurrentTest) AssertComplete() {
 
 func newConcurrentTest(t *testing.T, cloud *awsup.MockAWSCloud, allNeedUpdate bool) *concurrentTest {
 	test := concurrentTest{
-		AutoScalingAPI:          cloud.MockAutoscaling,
+		EC2API:                  cloud.MockEC2,
 		t:                       t,
 		terminationRequestsLeft: 6,
 		validationChan:          make(chan bool),
@@ -788,7 +797,7 @@ func TestRollingUpdateMaxUnavailableAllNeedUpdate(t *testing.T) {
 	concurrentTest := newConcurrentTest(t, cloud, true)
 	c.ValidateSuccessDuration = 0
 	c.ClusterValidator = concurrentTest
-	cloud.MockAutoscaling = concurrentTest
+	cloud.MockEC2 = concurrentTest
 
 	two := intstr.FromInt(2)
 	cluster.Spec.RollingUpdate = &kopsapi.RollingUpdate{
@@ -811,7 +820,7 @@ func TestRollingUpdateMaxUnavailableAllButOneNeedUpdate(t *testing.T) {
 	concurrentTest := newConcurrentTest(t, cloud, false)
 	c.ValidateSuccessDuration = 0
 	c.ClusterValidator = concurrentTest
-	cloud.MockAutoscaling = concurrentTest
+	cloud.MockEC2 = concurrentTest
 
 	two := intstr.FromInt(2)
 	cluster.Spec.RollingUpdate = &kopsapi.RollingUpdate{
@@ -833,7 +842,7 @@ func TestRollingUpdateMaxUnavailableAllNeedUpdateMaster(t *testing.T) {
 	concurrentTest := newConcurrentTest(t, cloud, true)
 	c.ValidateSuccessDuration = 0
 	c.ClusterValidator = concurrentTest
-	cloud.MockAutoscaling = concurrentTest
+	cloud.MockEC2 = concurrentTest
 
 	two := intstr.FromInt(2)
 	cluster.Spec.RollingUpdate = &kopsapi.RollingUpdate{

upup/pkg/fi/cloudup/awsup/aws_cloud.go

@@ -414,12 +414,11 @@ func deleteInstance(c AWSCloud, i *cloudinstances.CloudInstanceGroupMember) erro
 		return fmt.Errorf("id was not set on CloudInstanceGroupMember: %v", i)
 	}
 
-	request := &autoscaling.TerminateInstanceInAutoScalingGroupInput{
+	request := &ec2.TerminateInstancesInput{
-		InstanceId:                     aws.String(id),
+		InstanceIds: []*string{aws.String(id)},
-		ShouldDecrementDesiredCapacity: aws.Bool(false),
 	}
 
-	if _, err := c.Autoscaling().TerminateInstanceInAutoScalingGroup(request); err != nil {
+	if _, err := c.EC2().TerminateInstances(request); err != nil {
 		return fmt.Errorf("error deleting instance %q: %v", id, err)
 	}