diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go index 3d59abc8879..230e23dabd9 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer.go @@ -26,6 +26,7 @@ import ( "regexp" "strconv" "strings" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" @@ -52,6 +53,9 @@ const ( lbAttrAccessLogsS3Enabled = "access_logs.s3.enabled" lbAttrAccessLogsS3Bucket = "access_logs.s3.bucket" lbAttrAccessLogsS3Prefix = "access_logs.s3.prefix" + + // defaultEC2InstanceCacheMaxAge is the max age for the EC2 instance cache + defaultEC2InstanceCacheMaxAge = 10 * time.Minute ) var ( @@ -1606,7 +1610,7 @@ func (c *Cloud) findInstancesForELB(nodes []*v1.Node, annotations map[string]str instanceIDs := mapToAWSInstanceIDsTolerant(targetNodes) cacheCriteria := cacheCriteria{ - // MaxAge not required, because we only care about security groups, which should not change + MaxAge: defaultEC2InstanceCacheMaxAge, HasInstances: instanceIDs, // Refresh if any of the instance ids are missing } snapshot, err := c.instanceCache.describeAllInstancesCached(cacheCriteria) diff --git a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go index 15bc8eec0ba..3bd3016ad50 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/aws/aws_loadbalancer_test.go @@ -19,15 +19,20 @@ limitations under the License. package aws import ( - "k8s.io/apimachinery/pkg/types" + "fmt" + "reflect" "testing" + "time" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/elb" "github.com/aws/aws-sdk-go/service/elbv2" "github.com/stretchr/testify/assert" - - "k8s.io/api/core/v1" ) func TestElbProtocolsAreEqual(t *testing.T) { @@ -542,6 +547,86 @@ func TestFilterTargetNodes(t *testing.T) { } } +func makeNodeInstancePair(offset int) (*v1.Node, *ec2.Instance) { + instanceID := fmt.Sprintf("i-%x", int64(0x03bcc3496da09f78e)+int64(offset)) + instance := &ec2.Instance{ + InstanceId: aws.String(instanceID), + Placement: &ec2.Placement{ + AvailabilityZone: aws.String("us-east-1b"), + }, + PrivateDnsName: aws.String(fmt.Sprintf("ip-192-168-32-%d.ec2.internal", 101+offset)), + PrivateIpAddress: aws.String(fmt.Sprintf("192.168.32.%d", 101+offset)), + PublicIpAddress: aws.String(fmt.Sprintf("1.2.3.%d", 1+offset)), + } + + var tag ec2.Tag + tag.Key = aws.String(fmt.Sprintf("%s%s", TagNameKubernetesClusterPrefix, TestClusterID)) + tag.Value = aws.String("owned") + instance.Tags = []*ec2.Tag{&tag} + + node := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("ip-192-168-0-%d.ec2.internal", 101+offset), + }, + Spec: v1.NodeSpec{ + ProviderID: fmt.Sprintf("aws:///us-east-1b/%s", instanceID), + }, + } + return node, instance +} + +func TestCloud_findInstancesForELB(t *testing.T) { + defaultNode := &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ip-172-20-0-100.ec2.internal", + }, + Spec: v1.NodeSpec{ + ProviderID: "aws:///us-east-1a/i-self", + }, + } + newNode, newInstance := makeNodeInstancePair(1) + awsServices := NewFakeAWSServices(TestClusterID) + c, err := newAWSCloud(CloudConfig{}, awsServices) + if err != nil { + t.Errorf("Error building aws cloud: %v", err) + return + } + + want := map[InstanceID]*ec2.Instance{ + "i-self": awsServices.selfInstance, + } + got, err := c.findInstancesForELB([]*v1.Node{defaultNode}, nil) + assert.NoError(t, err) + assert.True(t, reflect.DeepEqual(want, got)) + + // Add a new EC2 instance + awsServices.instances = append(awsServices.instances, newInstance) + want = map[InstanceID]*ec2.Instance{ + "i-self": awsServices.selfInstance, + InstanceID(aws.StringValue(newInstance.InstanceId)): newInstance, + } + got, err = c.findInstancesForELB([]*v1.Node{defaultNode, newNode}, nil) + assert.NoError(t, err) + assert.True(t, reflect.DeepEqual(want, got)) + + // Verify existing instance cache gets used + cacheExpiryOld := c.instanceCache.snapshot.timestamp + got, err = c.findInstancesForELB([]*v1.Node{defaultNode, newNode}, nil) + assert.NoError(t, err) + assert.True(t, reflect.DeepEqual(want, got)) + cacheExpiryNew := c.instanceCache.snapshot.timestamp + assert.Equal(t, cacheExpiryOld, cacheExpiryNew) + + // Force cache expiry and verify cache gets updated with new timestamp + cacheExpiryOld = c.instanceCache.snapshot.timestamp + c.instanceCache.snapshot.timestamp = c.instanceCache.snapshot.timestamp.Add(-(defaultEC2InstanceCacheMaxAge + 1*time.Second)) + got, err = c.findInstancesForELB([]*v1.Node{defaultNode, newNode}, nil) + assert.NoError(t, err) + assert.True(t, reflect.DeepEqual(want, got)) + cacheExpiryNew = c.instanceCache.snapshot.timestamp + assert.True(t, cacheExpiryNew.After(cacheExpiryOld)) +} + func TestCloud_chunkTargetDescriptions(t *testing.T) { type args struct { targets []*elbv2.TargetDescription