Merge pull request #292 from enxebre/fix-deletion-4.3

Bug 1798597: [release-4.3] take into consideration all instance states for deletion

pkg/actuators/machine/actuator.go

@@ -370,17 +370,20 @@ func (a *Actuator) DeleteMachine(cluster *clusterv1.Cluster, machine *machinev1.
 		return a.handleMachineError(machine, err, deleteEventAction)
 	}
 
-	instances, err := getRunningInstances(machine, client)
+	// Get all instances not terminated.
+	existingInstances, err := a.getMachineInstances(cluster, machine)
 	if err != nil {
-		glog.Errorf("%s: error getting running instances: %v", machine.Name, err)
+		glog.Errorf("%s: error getting existing instances: %v", machine.Name, err)
 		return err
 	}
-	if len(instances) == 0 {
+	existingLen := len(existingInstances)
+	glog.Infof("%s: found %d existing instances for machine", machine.Name, existingLen)
+	if existingLen == 0 {
 		glog.Warningf("%s: no instances found to delete for machine", machine.Name)
 		return nil
 	}
 
-	terminatingInstances, err := terminateInstances(client, instances)
+	terminatingInstances, err := terminateInstances(client, existingInstances)
 	if err != nil {
 		return a.handleMachineError(machine, mapierrors.DeleteMachine(err.Error()), noEventAction)
 	}

pkg/actuators/machine/actuator_test.go

@@ -142,6 +142,24 @@ func TestMachineEvents(t *testing.T) {
 			},
 			event: "Normal Deleted Deleted machine aws-actuator-testing-machine",
 		},
+		{
+			name:                    "Delete machine event succeed with pending instances",
+			machine:                 machine,
+			describeInstancesOutput: stubDescribeInstancesOutput("ami-a9acbbd6", "i-02fcb933c5da7085c", ec2.InstanceStateNamePending),
+			operation: func(actuator *Actuator, cluster *clusterv1.Cluster, machine *machinev1.Machine) {
+				actuator.DeleteMachine(cluster, machine)
+			},
+			event: "Normal Deleted Deleted machine aws-actuator-testing-machine",
+		},
+		{
+			name:                    "Delete machine event succeed with stopped instances",
+			machine:                 machine,
+			describeInstancesOutput: stubDescribeInstancesOutput("ami-a9acbbd6", "i-02fcb933c5da7085c", ec2.InstanceStateNameStopped),
+			operation: func(actuator *Actuator, cluster *clusterv1.Cluster, machine *machinev1.Machine) {
+				actuator.DeleteMachine(cluster, machine)
+			},
+			event: "Normal Deleted Deleted machine aws-actuator-testing-machine",
+		},
 	}
 
 	for _, tc := range cases {

pkg/actuators/machine/instances_test.go

@@ -221,35 +221,6 @@ func TestRemoveStoppedMachine(t *testing.T) {
 	}
 }
 
-func TestRunningInstance(t *testing.T) {
-	machine, err := stubMachine()
-	if err != nil {
-		t.Fatalf("Unable to build test machine manifest: %v", err)
-	}
-
-	mockCtrl := gomock.NewController(t)
-	mockAWSClient := mockaws.NewMockClient(mockCtrl)
-
-	// Error describing instances
-	mockAWSClient.EXPECT().DescribeInstances(gomock.Any()).Return(&ec2.DescribeInstancesOutput{
-		Reservations: []*ec2.Reservation{
-			{
-				Instances: []*ec2.Instance{
-					{
-						ImageId:    aws.String("ami-a9acbbd6"),
-						InstanceId: aws.String("i-02fcb933c5da7085c"),
-						State: &ec2.InstanceState{
-							Name: aws.String(ec2.InstanceStateNameRunning),
-						},
-						LaunchTime: aws.Time(time.Now()),
-					},
-				},
-			},
-		},
-	}, nil).AnyTimes()
-	getRunningInstance(machine, mockAWSClient)
-}
-
 func TestLaunchInstance(t *testing.T) {
 	machine, err := stubMachine()
 	if err != nil {

pkg/actuators/machine/utils.go

@@ -47,28 +47,6 @@ func existingInstanceStates() []*string {
 	}
 }
 
-// getRunningInstance returns the AWS instance for a given machine. If multiple instances match our machine,
-// the most recently launched will be returned. If no instance exists, an error will be returned.
-func getRunningInstance(machine *machinev1.Machine, client awsclient.Client) (*ec2.Instance, error) {
-	instances, err := getRunningInstances(machine, client)
-	if err != nil {
-		return nil, err
-	}
-	if len(instances) == 0 {
-		return nil, fmt.Errorf("no instance found for machine: %s", machine.Name)
-	}
-
-	sortInstances(instances)
-	return instances[0], nil
-}
-
-// getRunningInstances returns all running instances that have a tag matching our machine name,
-// and cluster ID.
-func getRunningInstances(machine *machinev1.Machine, client awsclient.Client) ([]*ec2.Instance, error) {
-	runningInstanceStateFilter := []*string{aws.String(ec2.InstanceStateNameRunning)}
-	return getInstances(machine, client, runningInstanceStateFilter)
-}
-
 // getRunningFromInstances returns all running instances from a list of instances.
 func getRunningFromInstances(instances []*ec2.Instance) []*ec2.Instance {
 	var runningInstances []*ec2.Instance