e2e test for node deleted in cloud provider

Author: andrewsykim

hack/.golint_failures

@@ -695,6 +695,7 @@ test/e2e/apps
 test/e2e/auth
 test/e2e/autoscaling
 test/e2e/chaosmonkey
+test/e2e/cloud
 test/e2e/common
 test/e2e/framework
 test/e2e/framework/ingress

pkg/cloudprovider/providers/aws/aws.go

@@ -1332,7 +1332,7 @@ func extractNodeAddresses(instance *ec2.Instance) ([]v1.NodeAddress, error) {
 // This method will not be called from the node that is requesting this ID. i.e. metadata service
 // and other local methods cannot be used here
 func (c *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]v1.NodeAddress, error) {
-	instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
+	instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
 	if err != nil {
 		return nil, err
 	}
@@ -1348,7 +1348,7 @@ func (c *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID string
 // InstanceExistsByProviderID returns true if the instance with the given provider id still exists.
 // If false is returned with no error, the instance will be immediately deleted by the cloud controller manager.
 func (c *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) {
-	instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
+	instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
 	if err != nil {
 		return false, err
 	}
@@ -1379,7 +1379,7 @@ func (c *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID strin
 
 // InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes
 func (c *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) {
-	instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
+	instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
 	if err != nil {
 		return false, err
 	}
@@ -1435,7 +1435,7 @@ func (c *Cloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string
 // This method will not be called from the node that is requesting this ID. i.e. metadata service
 // and other local methods cannot be used here
 func (c *Cloud) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) {
-	instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
+	instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
 	if err != nil {
 		return "", err
 	}
@@ -1521,7 +1521,7 @@ func (c *Cloud) GetZone(ctx context.Context) (cloudprovider.Zone, error) {
 // This is particularly useful in external cloud providers where the kubelet
 // does not initialize node data.
 func (c *Cloud) GetZoneByProviderID(ctx context.Context, providerID string) (cloudprovider.Zone, error) {
-	instanceID, err := kubernetesInstanceID(providerID).mapToAWSInstanceID()
+	instanceID, err := KubernetesInstanceID(providerID).MapToAWSInstanceID()
 	if err != nil {
 		return cloudprovider.Zone{}, err
 	}
@@ -1602,7 +1602,7 @@ func newAWSInstance(ec2Service EC2, instance *ec2.Instance) *awsInstance {
 
 // Gets the full information about this instance from the EC2 API
 func (i *awsInstance) describeInstance() (*ec2.Instance, error) {
-	return describeInstance(i.ec2, awsInstanceID(i.awsID))
+	return describeInstance(i.ec2, InstanceID(i.awsID))
 }
 
 // Gets the mountDevice already assigned to the volume, or assigns an unused mountDevice.
@@ -3787,7 +3787,7 @@ func (c *Cloud) getTaggedSecurityGroups() (map[string]*ec2.SecurityGroup, error)
 
 // Open security group ingress rules on the instances so that the load balancer can talk to them
 // Will also remove any security groups ingress rules for the load balancer that are _not_ needed for allInstances
-func (c *Cloud) updateInstanceSecurityGroupsForLoadBalancer(lb *elb.LoadBalancerDescription, instances map[awsInstanceID]*ec2.Instance) error {
+func (c *Cloud) updateInstanceSecurityGroupsForLoadBalancer(lb *elb.LoadBalancerDescription, instances map[InstanceID]*ec2.Instance) error {
 	if c.cfg.Global.DisableSecurityGroupIngress {
 		return nil
 	}

pkg/cloudprovider/providers/aws/aws_loadbalancer.go

@@ -863,7 +863,7 @@ func (c *Cloud) updateInstanceSecurityGroupsForNLBTraffic(actualGroups []*ec2.Se
 }
 
 // Add SG rules for a given NLB
-func (c *Cloud) updateInstanceSecurityGroupsForNLB(mappings []nlbPortMapping, instances map[awsInstanceID]*ec2.Instance, lbName string, clientCidrs []string) error {
+func (c *Cloud) updateInstanceSecurityGroupsForNLB(mappings []nlbPortMapping, instances map[InstanceID]*ec2.Instance, lbName string, clientCidrs []string) error {
 	if c.cfg.Global.DisableSecurityGroupIngress {
 		return nil
 	}
@@ -1380,7 +1380,7 @@ func (c *Cloud) ensureLoadBalancerHealthCheck(loadBalancer *elb.LoadBalancerDesc
 }
 
 // Makes sure that exactly the specified hosts are registered as instances with the load balancer
-func (c *Cloud) ensureLoadBalancerInstances(loadBalancerName string, lbInstances []*elb.Instance, instanceIDs map[awsInstanceID]*ec2.Instance) error {
+func (c *Cloud) ensureLoadBalancerInstances(loadBalancerName string, lbInstances []*elb.Instance, instanceIDs map[InstanceID]*ec2.Instance) error {
 	expected := sets.NewString()
 	for id := range instanceIDs {
 		expected.Insert(string(id))
@@ -1557,7 +1557,7 @@ func proxyProtocolEnabled(backend *elb.BackendServerDescription) bool {
 // findInstancesForELB gets the EC2 instances corresponding to the Nodes, for setting up an ELB
 // We ignore Nodes (with a log message) where the instanceid cannot be determined from the provider,
 // and we ignore instances which are not found
-func (c *Cloud) findInstancesForELB(nodes []*v1.Node) (map[awsInstanceID]*ec2.Instance, error) {
+func (c *Cloud) findInstancesForELB(nodes []*v1.Node) (map[InstanceID]*ec2.Instance, error) {
 	// Map to instance ids ignoring Nodes where we cannot find the id (but logging)
 	instanceIDs := mapToAWSInstanceIDsTolerant(nodes)
 

pkg/cloudprovider/providers/aws/instances.go

@@ -34,26 +34,26 @@ import (
 // awsInstanceRegMatch represents Regex Match for AWS instance.
 var awsInstanceRegMatch = regexp.MustCompile("^i-[^/]*$")
 
-// awsInstanceID represents the ID of the instance in the AWS API, e.g. i-12345678
+// InstanceID represents the ID of the instance in the AWS API, e.g. i-12345678
 // The "traditional" format is "i-12345678"
 // A new longer format is also being introduced: "i-12345678abcdef01"
 // We should not assume anything about the length or format, though it seems
 // reasonable to assume that instances will continue to start with "i-".
-type awsInstanceID string
+type InstanceID string
 
-func (i awsInstanceID) awsString() *string {
+func (i InstanceID) awsString() *string {
 	return aws.String(string(i))
 }
 
-// kubernetesInstanceID represents the id for an instance in the kubernetes API;
+// KubernetesInstanceID represents the id for an instance in the kubernetes API;
 // the following form
 //  * aws:///<zone>/<awsInstanceId>
 //  * aws:////<awsInstanceId>
 //  * <awsInstanceId>
-type kubernetesInstanceID string
+type KubernetesInstanceID string
 
-// mapToAWSInstanceID extracts the awsInstanceID from the kubernetesInstanceID
-func (name kubernetesInstanceID) mapToAWSInstanceID() (awsInstanceID, error) {
+// MapToAWSInstanceID extracts the InstanceID from the KubernetesInstanceID
+func (name KubernetesInstanceID) MapToAWSInstanceID() (InstanceID, error) {
 	s := string(name)
 
 	if !strings.HasPrefix(s, "aws://") {
@@ -85,17 +85,17 @@ func (name kubernetesInstanceID) mapToAWSInstanceID() (awsInstanceID, error) {
 		return "", fmt.Errorf("Invalid format for AWS instance (%s)", name)
 	}
 
-	return awsInstanceID(awsID), nil
+	return InstanceID(awsID), nil
 }
 
-// mapToAWSInstanceID extracts the awsInstanceIDs from the Nodes, returning an error if a Node cannot be mapped
-func mapToAWSInstanceIDs(nodes []*v1.Node) ([]awsInstanceID, error) {
-	var instanceIDs []awsInstanceID
+// mapToAWSInstanceID extracts the InstanceIDs from the Nodes, returning an error if a Node cannot be mapped
+func mapToAWSInstanceIDs(nodes []*v1.Node) ([]InstanceID, error) {
+	var instanceIDs []InstanceID
 	for _, node := range nodes {
 		if node.Spec.ProviderID == "" {
 			return nil, fmt.Errorf("node %q did not have ProviderID set", node.Name)
 		}
-		instanceID, err := kubernetesInstanceID(node.Spec.ProviderID).mapToAWSInstanceID()
+		instanceID, err := KubernetesInstanceID(node.Spec.ProviderID).MapToAWSInstanceID()
 		if err != nil {
 			return nil, fmt.Errorf("unable to parse ProviderID %q for node %q", node.Spec.ProviderID, node.Name)
 		}
@@ -105,15 +105,15 @@ func mapToAWSInstanceIDs(nodes []*v1.Node) ([]awsInstanceID, error) {
 	return instanceIDs, nil
 }
 
-// mapToAWSInstanceIDsTolerant extracts the awsInstanceIDs from the Nodes, skipping Nodes that cannot be mapped
-func mapToAWSInstanceIDsTolerant(nodes []*v1.Node) []awsInstanceID {
-	var instanceIDs []awsInstanceID
+// mapToAWSInstanceIDsTolerant extracts the InstanceIDs from the Nodes, skipping Nodes that cannot be mapped
+func mapToAWSInstanceIDsTolerant(nodes []*v1.Node) []InstanceID {
+	var instanceIDs []InstanceID
 	for _, node := range nodes {
 		if node.Spec.ProviderID == "" {
 			klog.Warningf("node %q did not have ProviderID set", node.Name)
 			continue
 		}
-		instanceID, err := kubernetesInstanceID(node.Spec.ProviderID).mapToAWSInstanceID()
+		instanceID, err := KubernetesInstanceID(node.Spec.ProviderID).MapToAWSInstanceID()
 		if err != nil {
 			klog.Warningf("unable to parse ProviderID %q for node %q", node.Spec.ProviderID, node.Name)
 			continue
@@ -125,7 +125,7 @@ func mapToAWSInstanceIDsTolerant(nodes []*v1.Node) []awsInstanceID {
 }
 
 // Gets the full information about this instance from the EC2 API
-func describeInstance(ec2Client EC2, instanceID awsInstanceID) (*ec2.Instance, error) {
+func describeInstance(ec2Client EC2, instanceID InstanceID) (*ec2.Instance, error) {
 	request := &ec2.DescribeInstancesInput{
 		InstanceIds: []*string{instanceID.awsString()},
 	}
@@ -164,9 +164,9 @@ func (c *instanceCache) describeAllInstancesUncached() (*allInstancesSnapshot, e
 		return nil, err
 	}
 
-	m := make(map[awsInstanceID]*ec2.Instance)
+	m := make(map[InstanceID]*ec2.Instance)
 	for _, i := range instances {
-		id := awsInstanceID(aws.StringValue(i.InstanceId))
+		id := InstanceID(aws.StringValue(i.InstanceId))
 		m[id] = i
 	}
 
@@ -191,9 +191,9 @@ type cacheCriteria struct {
 	// If set to 0 (i.e. unset), cached values will not time out because of age.
 	MaxAge time.Duration
 
-	// HasInstances is a list of awsInstanceIDs that must be in a cached snapshot for it to be considered valid.
+	// HasInstances is a list of InstanceIDs that must be in a cached snapshot for it to be considered valid.
 	// If an instance is not found in the cached snapshot, the snapshot be ignored and we will re-fetch.
-	HasInstances []awsInstanceID
+	HasInstances []InstanceID
 }
 
 // describeAllInstancesCached returns all instances, using cached results if applicable
@@ -257,12 +257,12 @@ func (s *allInstancesSnapshot) MeetsCriteria(criteria cacheCriteria) bool {
 // along with the timestamp for cache-invalidation purposes
 type allInstancesSnapshot struct {
 	timestamp time.Time
-	instances map[awsInstanceID]*ec2.Instance
+	instances map[InstanceID]*ec2.Instance
 }
 
 // FindInstances returns the instances corresponding to the specified ids.  If an id is not found, it is ignored.
-func (s *allInstancesSnapshot) FindInstances(ids []awsInstanceID) map[awsInstanceID]*ec2.Instance {
-	m := make(map[awsInstanceID]*ec2.Instance)
+func (s *allInstancesSnapshot) FindInstances(ids []InstanceID) map[InstanceID]*ec2.Instance {
+	m := make(map[InstanceID]*ec2.Instance)
 	for _, id := range ids {
 		instance := s.instances[id]
 		if instance != nil {

pkg/cloudprovider/providers/aws/instances_test.go

@@ -29,8 +29,8 @@ import (
 
 func TestMapToAWSInstanceIDs(t *testing.T) {
 	tests := []struct {
-		Kubernetes  kubernetesInstanceID
-		Aws         awsInstanceID
+		Kubernetes  KubernetesInstanceID
+		Aws         InstanceID
 		ExpectError bool
 	}{
 		{
@@ -80,7 +80,7 @@ func TestMapToAWSInstanceIDs(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		awsID, err := test.Kubernetes.mapToAWSInstanceID()
+		awsID, err := test.Kubernetes.MapToAWSInstanceID()
 		if err != nil {
 			if !test.ExpectError {
 				t.Errorf("unexpected error parsing %s: %v", test.Kubernetes, err)
@@ -139,18 +139,18 @@ func TestSnapshotMeetsCriteria(t *testing.T) {
 		t.Errorf("Snapshot did not honor MaxAge")
 	}
 
-	if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []awsInstanceID{awsInstanceID("i-12345678")}}) {
+	if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678")}}) {
 		t.Errorf("Snapshot did not honor HasInstances with missing instances")
 	}
 
-	snapshot.instances = make(map[awsInstanceID]*ec2.Instance)
-	snapshot.instances[awsInstanceID("i-12345678")] = &ec2.Instance{}
+	snapshot.instances = make(map[InstanceID]*ec2.Instance)
+	snapshot.instances[InstanceID("i-12345678")] = &ec2.Instance{}
 
-	if !snapshot.MeetsCriteria(cacheCriteria{HasInstances: []awsInstanceID{awsInstanceID("i-12345678")}}) {
+	if !snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678")}}) {
 		t.Errorf("Snapshot did not honor HasInstances with matching instances")
 	}
 
-	if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []awsInstanceID{awsInstanceID("i-12345678"), awsInstanceID("i-00000000")}}) {
+	if snapshot.MeetsCriteria(cacheCriteria{HasInstances: []InstanceID{InstanceID("i-12345678"), InstanceID("i-00000000")}}) {
 		t.Errorf("Snapshot did not honor HasInstances with partially matching instances")
 	}
 }
@@ -170,22 +170,22 @@ func TestOlderThan(t *testing.T) {
 func TestSnapshotFindInstances(t *testing.T) {
 	snapshot := &allInstancesSnapshot{}
 
-	snapshot.instances = make(map[awsInstanceID]*ec2.Instance)
+	snapshot.instances = make(map[InstanceID]*ec2.Instance)
 	{
-		id := awsInstanceID("i-12345678")
+		id := InstanceID("i-12345678")
 		snapshot.instances[id] = &ec2.Instance{InstanceId: id.awsString()}
 	}
 	{
-		id := awsInstanceID("i-23456789")
+		id := InstanceID("i-23456789")
 		snapshot.instances[id] = &ec2.Instance{InstanceId: id.awsString()}
 	}
 
-	instances := snapshot.FindInstances([]awsInstanceID{awsInstanceID("i-12345678"), awsInstanceID("i-23456789"), awsInstanceID("i-00000000")})
+	instances := snapshot.FindInstances([]InstanceID{InstanceID("i-12345678"), InstanceID("i-23456789"), InstanceID("i-00000000")})
 	if len(instances) != 2 {
 		t.Errorf("findInstances returned %d results, expected 2", len(instances))
 	}
 
-	for _, id := range []awsInstanceID{awsInstanceID("i-12345678"), awsInstanceID("i-23456789")} {
+	for _, id := range []InstanceID{InstanceID("i-12345678"), InstanceID("i-23456789")} {
 		i := instances[id]
 		if i == nil {
 			t.Errorf("findInstances did not return %s", id)

test/e2e/BUILD

@@ -16,6 +16,7 @@ go_test(
         "//test/e2e/apps:go_default_library",
         "//test/e2e/auth:go_default_library",
         "//test/e2e/autoscaling:go_default_library",
+        "//test/e2e/cloud:go_default_library",
         "//test/e2e/common:go_default_library",
         "//test/e2e/framework:go_default_library",
         "//test/e2e/framework/testfiles:go_default_library",
@@ -104,6 +105,7 @@ filegroup(
         "//test/e2e/auth:all-srcs",
         "//test/e2e/autoscaling:all-srcs",
         "//test/e2e/chaosmonkey:all-srcs",
+        "//test/e2e/cloud:all-srcs",
         "//test/e2e/common:all-srcs",
         "//test/e2e/framework:all-srcs",
         "//test/e2e/generated:all-srcs",

test/e2e/cloud/BUILD

@@ -0,0 +1,33 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = [
+        "framework.go",
+        "nodes.go",
+    ],
+    importpath = "k8s.io/kubernetes/test/e2e/cloud",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
+        "//test/e2e/framework:go_default_library",
+        "//vendor/github.com/onsi/ginkgo:go_default_library",
+        "//vendor/github.com/onsi/gomega:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

test/e2e/cloud/OWNERS

@@ -0,0 +1,14 @@
+# See the OWNERS docs at https://go.k8s.io/owners
+approvers:
+- andrewsykim
+- cheftako
+- mcrute
+- d-nishi
+reviewers:
+- andrewsykim
+- cheftako
+- mcrute
+- d-nishi
+
+labels:
+- sig/cloud-provider

test/e2e/cloud/framework.go

@@ -0,0 +1,23 @@
+/*
+Copyright 2019 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 cloud
+
+import "k8s.io/kubernetes/test/e2e/framework"
+
+func SIGDescribe(text string, body func()) bool {
+	return framework.KubeDescribe("[sig-cloud-provider] "+text, body)
+}