zones: initial implementation of Zones interface

Author: nicolehanjing

pkg/providers/v2/instances.go

@@ -22,12 +22,15 @@ import (
 	"net"
 	"strings"
 
+	"k8s.io/klog/v2"
+
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/credentials"
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/ec2"
 
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	cloudprovider "k8s.io/cloud-provider"
 )
 
@@ -71,20 +74,10 @@ type instances struct {
 
 // InstanceExists indicates whether a given node exists according to the cloud provider
 func (i *instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) {
-	var err error
-	if node.Spec.ProviderID == "" {
-		_, err = getInstanceByPrivateDNSName(ctx, node.Name, i.ec2)
-		if err == cloudprovider.InstanceNotFound {
-			return false, nil
-		}
-
-		if err != nil {
-			return false, err
-		}
-	}
+	_, err := i.getInstance(ctx, node)
 
-	_, err = getInstanceByProviderID(ctx, node.Spec.ProviderID, i.ec2)
 	if err == cloudprovider.InstanceNotFound {
+		klog.V(6).Infof("instance not found for node: %s", node.Name)
 		return false, nil
 	}
 
@@ -97,28 +90,31 @@ func (i *instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, er
 
 // InstanceShutdown returns true if the instance is shutdown according to the cloud provider.
 func (i *instances) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) {
-	if node.Spec.ProviderID == "" {
-		return i.instanceShutdownByPrivateDNSName(ctx, node.Name)
+	ec2Instance, err := i.getInstance(ctx, node)
+	if err != nil {
+		return false, err
+	}
+
+	if ec2Instance.State != nil {
+		state := aws.StringValue(ec2Instance.State.Name)
+		// valid state for detaching volumes
+		if state == ec2.InstanceStateNameStopped {
+			return true, nil
+		}
 	}
 
-	return i.instanceShutdownByProviderID(ctx, node.Spec.ProviderID)
+	return false, nil
 }
 
 // InstanceMetadata returns the instance's metadata.
 func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
 	var err error
 	var ec2Instance *ec2.Instance
-	if node.Spec.ProviderID == "" {
-		//  TODO: support node name policy other than private DNS names
-		ec2Instance, err = getInstanceByPrivateDNSName(ctx, node.Name, i.ec2)
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		ec2Instance, err = getInstanceByProviderID(ctx, node.Spec.ProviderID, i.ec2)
-		if err != nil {
-			return nil, err
-		}
+
+	//  TODO: support node name policy other than private DNS names
+	ec2Instance, err = i.getInstance(ctx, node)
+	if err != nil {
+		return nil, err
 	}
 
 	nodeAddresses, err := nodeAddressesForInstance(ec2Instance)
@@ -140,20 +136,29 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud
 	return metadata, nil
 }
 
-// getInstanceByProviderID returns the instance if the instance with the given provider id still exists.
+// getInstance returns the instance if the instance with the given node info still exists.
 // If false an error will be returned, the instance will be immediately deleted by the cloud controller manager.
-func getInstanceByProviderID(ctx context.Context, providerID string, ec2Client EC2) (*ec2.Instance, error) {
-	instanceID, err := parseInstanceIDFromProviderID(providerID)
-	if err != nil {
-		return nil, err
+func (i *instances) getInstance(ctx context.Context, node *v1.Node) (*ec2.Instance, error) {
+	if node.Spec.ProviderID == "" {
+		// get Instance by node name
+		klog.V(4).Infof("looking for node by private DNS name %v", node.Name)
+		return getInstanceByPrivateDNSName(ctx, types.NodeName(node.Name), i.ec2)
 	}
 
+	// get Instance by provider ID
+	klog.V(4).Infof("looking for node by provider ID %v", node.Spec.ProviderID)
+	return getInstanceByProviderID(ctx, node.Spec.ProviderID, i.ec2)
+}
+
+// getInstanceByPrivateDNSName returns the instance if the instance with the given nodeName exists.
+// If false an error will be returned, the instance will be immediately deleted by the cloud controller manager.
+func getInstanceByPrivateDNSName(ctx context.Context, nodeName types.NodeName, ec2Client EC2) (*ec2.Instance, error) {
 	request := &ec2.DescribeInstancesInput{
-		InstanceIds: []*string{aws.String(instanceID)},
 		Filters: []*ec2.Filter{
-			newEc2Filter("instance-state-name", aliveFilter...),
+			newEc2Filter("private-dns-name", string(nodeName)),
 		},
 	}
+	klog.V(4).Infof("looking for node by private DNS name %v", nodeName)
 
 	instances := []*ec2.Instance{}
 	var nextToken *string
@@ -179,21 +184,29 @@ func getInstanceByProviderID(ctx context.Context, providerID string, ec2Client E
 	}
 
 	if len(instances) > 1 {
-		return nil, fmt.Errorf("multiple instances found with provider ID: %s", instanceID)
+		return nil, fmt.Errorf("getInstance: multiple instances found")
+	}
+
+	state := instances[0].State.Name
+	if *state == ec2.InstanceStateNameTerminated {
+		return nil, fmt.Errorf("instance %v is terminated", instances[0].InstanceId)
 	}
 
 	return instances[0], nil
 }
 
-// getInstanceByPrivateDNSName returns the instance if the instance with the given provider id still exists.
+// getInstanceByProviderID returns the instance if the instance with the given providerID exists.
 // If false an error will be returned, the instance will be immediately deleted by the cloud controller manager.
-func getInstanceByPrivateDNSName(ctx context.Context, nodeName string, ec2Client EC2) (*ec2.Instance, error) {
+func getInstanceByProviderID(ctx context.Context, providerID string, ec2Client EC2) (*ec2.Instance, error) {
+	instanceID, err := parseInstanceIDFromProviderID(providerID)
+	if err != nil {
+		return nil, err
+	}
+
 	request := &ec2.DescribeInstancesInput{
-		Filters: []*ec2.Filter{
-			newEc2Filter("private-dns-name", nodeName),
-			newEc2Filter("instance-state-name", aliveFilter...),
-		},
+		InstanceIds: []*string{aws.String(instanceID)},
 	}
+	klog.V(4).Infof("looking for node by provider ID %v", providerID)
 
 	instances := []*ec2.Instance{}
 	var nextToken *string
@@ -219,55 +232,18 @@ func getInstanceByPrivateDNSName(ctx context.Context, nodeName string, ec2Client
 	}
 
 	if len(instances) > 1 {
-		return nil, fmt.Errorf("multiple instances found with private DNS name: %s", nodeName)
-	}
-
-	return instances[0], nil
-}
-
-// instanceShutdownByProviderID returns true if the instance is shutdown according to the cloud provider.
-func (i *instances) instanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) {
-	ec2Instance, err := getInstanceByProviderID(ctx, providerID, i.ec2)
-	if err != nil {
-		if err == cloudprovider.InstanceNotFound {
-			// We don't have live ec2Instance
-			return true, nil
-		}
-		return false, err
+		return nil, fmt.Errorf("getInstance: multiple instances found")
 	}
 
-	if ec2Instance.State != nil {
-		state := aws.StringValue(ec2Instance.State.Name)
-		if state == ec2.InstanceStateNameStopping || state == ec2.InstanceStateNameStopped {
-			return true, nil
-		}
+	state := instances[0].State.Name
+	if *state == ec2.InstanceStateNameTerminated {
+		return nil, fmt.Errorf("instance %v is terminated", instances[0].InstanceId)
 	}
 
-	return false, nil
-}
-
-// instanceShutdownByPrivateDNSName returns true if the instance is shutdown according to the cloud provider.
-func (i *instances) instanceShutdownByPrivateDNSName(ctx context.Context, nodeName string) (bool, error) {
-	ec2Instance, err := getInstanceByPrivateDNSName(ctx, nodeName, i.ec2)
-	if err != nil {
-		if err == cloudprovider.InstanceNotFound {
-			// We don't have live ec2Instance
-			return true, nil
-		}
-		return false, err
-	}
-
-	if ec2Instance.State != nil {
-		state := aws.StringValue(ec2Instance.State.Name)
-		if state == ec2.InstanceStateNameStopping || state == ec2.InstanceStateNameStopped {
-			return true, nil
-		}
-	}
-
-	return false, nil
+	return instances[0], nil
 }
 
-// nodeAddressesForInstance returns a list of v1.NodeAddress for the give instance.
+// nodeAddresses for Instance returns a list of v1.NodeAddress for the give instance.
 // TODO: should we support ExternalIP by default?
 func nodeAddressesForInstance(instance *ec2.Instance) ([]v1.NodeAddress, error) {
 	if instance == nil {
@@ -340,14 +316,6 @@ func parseInstanceIDFromProviderID(providerID string) (string, error) {
 	return instanceID, nil
 }
 
-var aliveFilter = []string{
-	ec2.InstanceStateNamePending,
-	ec2.InstanceStateNameRunning,
-	ec2.InstanceStateNameShuttingDown,
-	ec2.InstanceStateNameStopping,
-	ec2.InstanceStateNameStopped,
-}
-
 func newEc2Filter(name string, values ...string) *ec2.Filter {
 	filter := &ec2.Filter{
 		Name: aws.String(name),