Refactor instance.isProtectedFromTermination (LeanerCloud#393)

Split off from LeanerCloud#354 with some small tweaks to get tests to pass.

core/autoscaling.go

@@ -294,9 +294,20 @@ func (a *autoScalingGroup) getInstance(
 				continue
 			}
 
-			if considerInstanceProtection && (i.isProtectedFromScaleIn() || i.isProtectedFromTermination()) {
-				debug.Println(a.name, "skipping protected instance", *i.InstanceId)
-				continue
+			if considerInstanceProtection {
+				protected := i.isProtectedFromScaleIn()
+				if !protected {
+					protectedT, err := i.isProtectedFromTermination()
+					if err != nil {
+						debug.Println(a.name, "failed to determine termination protection for", *i.InstanceId)
+					}
+					protected = protectedT
+				}
+
+				if protected {
+					debug.Println(a.name, "skipping protected instance", *i.InstanceId)
+					continue
+				}
 			}
 
 			if (availabilityZone != nil) && (*availabilityZone != *i.Placement.AvailabilityZone) {

core/autoscaling_test.go

@@ -1210,22 +1210,8 @@ func TestGetOnDemandInstanceInAZ(t *testing.T) {
 					},
 				},
 			),
-			az: aws.String("1b"),
-			expected: &instance{
-				Instance: &ec2.Instance{
-					InstanceId:        aws.String("ondemand-running"),
-					State:             &ec2.InstanceState{Name: aws.String(ec2.InstanceStateNameRunning)},
-					Placement:         &ec2.Placement{AvailabilityZone: aws.String("1b")},
-					InstanceLifecycle: aws.String(""),
-				},
-				region: &region{
-					services: connections{
-						ec2: mockEC2{
-							diaerr: errors.New("error when determining instance termination protection"),
-						},
-					},
-				},
-			},
+			az:       aws.String("1b"),
+			expected: nil,
 		},
 
 		{

core/instance.go

@@ -146,24 +146,32 @@ func (i *instance) isSpot() bool {
 		*i.InstanceLifecycle == "spot"
 }
 
-func (i *instance) isProtectedFromTermination() bool {
+func (i *instance) isProtectedFromTermination() (bool, error) {
 
+	debug.Println("\tChecking termination protection for instance: ", *i.InstanceId)
 	// determine and set the API termination protection field
 	diaRes, err := i.region.services.ec2.DescribeInstanceAttribute(
 		&ec2.DescribeInstanceAttributeInput{
 			Attribute:  aws.String("disableApiTermination"),
 			InstanceId: i.InstanceId,
 		})
 
-	if err == nil &&
+	if err != nil {
+		// better safe than sorry!
+		logger.Printf("Couldn't describe instance attributes, assuming instance %v is protected: %v\n",
+			*i.InstanceId, err.Error())
+		return true, err
+	}
+
+	if diaRes != nil &&
 		diaRes.DisableApiTermination != nil &&
 		diaRes.DisableApiTermination.Value != nil &&
 		*diaRes.DisableApiTermination.Value {
 		logger.Printf("\t: %v Instance, %v is protected from termination\n",
 			*i.Placement.AvailabilityZone, *i.InstanceId)
-		return true
+		return true, nil
 	}
-	return false
+	return false, nil
 }
 
 func (i *instance) isProtectedFromScaleIn() bool {