diff --git a/pkg/providers/v1/aws.go b/pkg/providers/v1/aws.go index 024dab2ada..9e020fdf1c 100644 --- a/pkg/providers/v1/aws.go +++ b/pkg/providers/v1/aws.go @@ -990,6 +990,18 @@ func (c *Cloud) getInstanceType(instance *ec2types.Instance) string { return string(instance.InstanceType) } +func (c *Cloud) getInstanceTag(instance *ec2types.Instance, key string) *string { + for _, tag := range instance.Tags { + if strings.EqualFold(*tag.Key, key) { // case-insensitive match + if tag.Value == nil || *tag.Value == "" { + return nil + } + return tag.Value + } + } + return nil // return empty string if tag not found +} + // InstanceType returns the type of the node with the specified nodeName. func (c *Cloud) InstanceType(ctx context.Context, nodeName types.NodeName) (string, error) { if c.selfAWSInstance.nodeName == nodeName { diff --git a/pkg/providers/v1/aws_test.go b/pkg/providers/v1/aws_test.go index 1feb780fc9..821cc02393 100644 --- a/pkg/providers/v1/aws_test.go +++ b/pkg/providers/v1/aws_test.go @@ -487,14 +487,14 @@ func testHasNodeAddress(t *testing.T, addrs []v1.NodeAddress, addressType v1.Nod } func makeMinimalInstance(instanceID string) ec2types.Instance { - return makeInstance(instanceID, "", "", "", "", nil, false) + return makeInstance(instanceID, "", "", "", "", nil, false, nil) } -func makeInstance(instanceID string, privateIP, publicIP, privateDNSName, publicDNSName string, ipv6s []string, setNetInterface bool) ec2types.Instance { +func makeInstance(instanceID string, privateIP, publicIP, privateDNSName, publicDNSName string, ipv6s []string, setNetInterface bool, additionalTags []ec2types.Tag) ec2types.Instance { var tag ec2types.Tag tag.Key = aws.String(TagNameKubernetesClusterLegacy) tag.Value = aws.String(TestClusterID) - tags := []ec2types.Tag{tag} + tags := append(additionalTags, tag) instance := ec2types.Instance{ InstanceId: &instanceID, @@ -584,7 +584,7 @@ func TestNodeAddressesByProviderID(t *testing.T) { }, } { t.Run(tc.Name, func(t *testing.T) { - instance := makeInstance(tc.InstanceID, tc.PrivateIP, tc.PublicIP, tc.PrivateDNSName, tc.PublicDNSName, tc.Ipv6s, tc.SetNetInterface) + instance := makeInstance(tc.InstanceID, tc.PrivateIP, tc.PublicIP, tc.PrivateDNSName, tc.PublicDNSName, tc.Ipv6s, tc.SetNetInterface, nil) aws1, _ := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) _, err := aws1.NodeAddressesByProviderID(context.TODO(), "i-xxx") if err == nil { @@ -690,7 +690,7 @@ func TestNodeAddresses(t *testing.T) { }, } { t.Run(tc.Name, func(t *testing.T) { - instance := makeInstance(tc.InstanceID, tc.PrivateIP, tc.PublicIP, tc.PrivateDNSName, tc.PublicDNSName, tc.Ipv6s, tc.SetNetInterface) + instance := makeInstance(tc.InstanceID, tc.PrivateIP, tc.PublicIP, tc.PrivateDNSName, tc.PublicDNSName, tc.Ipv6s, tc.SetNetInterface, nil) aws1, _ := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) _, err := aws1.NodeAddresses(context.TODO(), "instance-mismatch.ec2.internal") if err == nil { diff --git a/pkg/providers/v1/instances_v2.go b/pkg/providers/v1/instances_v2.go index 1d6f32f9b5..117bf21880 100644 --- a/pkg/providers/v1/instances_v2.go +++ b/pkg/providers/v1/instances_v2.go @@ -68,7 +68,7 @@ func (c *Cloud) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, erro } func (c *Cloud) getAdditionalLabels(ctx context.Context, zoneName string, instanceID string, instanceType string, - region string, existingLabels map[string]string) (map[string]string, error) { + region string, existingLabels map[string]string, asgName *string) (map[string]string, error) { additionalLabels := map[string]string{} // If zone ID label is already set, skip. @@ -107,6 +107,12 @@ func (c *Cloud) getAdditionalLabels(ctx context.Context, zoneName string, instan } } + if _, ok := existingLabels[LabelAutoScalingGroupName]; !ok { + if asgName != nil { + additionalLabels[LabelAutoScalingGroupName] = *asgName + } + } + return additionalLabels, nil } @@ -129,6 +135,7 @@ func (c *Cloud) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprov instanceType string zone cloudprovider.Zone nodeAddresses []v1.NodeAddress + asg_name *string ) if variant.IsVariantNode(string(instanceID)) { instanceType, err = c.InstanceTypeByProviderID(ctx, providerID) @@ -157,9 +164,10 @@ func (c *Cloud) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprov if err != nil { return nil, fmt.Errorf("failed to get node addresses for instance %s: %w", instanceID, err) } + asg_name = c.getInstanceTag(instance, "aws:autoscaling:groupName") } - additionalLabels, err := c.getAdditionalLabels(ctx, zone.FailureDomain, string(instanceID), instanceType, zone.Region, node.Labels) + additionalLabels, err := c.getAdditionalLabels(ctx, zone.FailureDomain, string(instanceID), instanceType, zone.Region, node.Labels, asg_name) if err != nil { return nil, err } diff --git a/pkg/providers/v1/instances_v2_test.go b/pkg/providers/v1/instances_v2_test.go index 6a98a433aa..29e2a20520 100644 --- a/pkg/providers/v1/instances_v2_test.go +++ b/pkg/providers/v1/instances_v2_test.go @@ -167,7 +167,104 @@ func TestInstanceShutdown(t *testing.T) { func TestInstanceMetadata(t *testing.T) { t.Run("Should return populated InstanceMetadata", func(t *testing.T) { - instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true) + instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true, nil) + c, _ := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) + var mockedTopologyManager MockedInstanceTopologyManager + c.instanceTopologyManager = &mockedTopologyManager + mockedTopologyManager.On("GetNodeTopology", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&ec2types.InstanceTopology{ + AvailabilityZone: aws.String("us-west-2b"), + GroupName: new(string), + InstanceId: aws.String("i-123456789"), + InstanceType: new(string), + NetworkNodes: []string{"nn-123456789", "nn-234567890", "nn-345678901"}, + ZoneId: aws.String("az2"), + }, nil) + node := &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: fmt.Sprintf("aws:///us-west-2c/1abc-2def/%s", *instance.InstanceId), + }, + } + + result, err := c.InstanceMetadata(context.TODO(), node) + if err != nil { + t.Errorf("Should not error getting InstanceMetadata: %s", err) + } + + mockedTopologyManager.AssertNumberOfCalls(t, "GetNodeTopology", 1) + assert.Equal(t, "aws:///us-west-2c/1abc-2def/i-00000000000000000", result.ProviderID) + assert.Equal(t, "c3.large", result.InstanceType) + assert.Equal(t, []v1.NodeAddress{ + {Type: "InternalIP", Address: "192.168.0.1"}, + {Type: "ExternalIP", Address: "1.2.3.4"}, + {Type: "InternalDNS", Address: "instance-same.ec2.internal"}, + {Type: "Hostname", Address: "instance-same.ec2.internal"}, + {Type: "ExternalDNS", Address: "instance-same.ec2.external"}, + }, result.NodeAddresses) + assert.Equal(t, "us-west-2a", result.Zone) + assert.Equal(t, "us-west-2", result.Region) + assert.Equal(t, map[string]string{ + LabelZoneID: "az1", + LabelNetworkNodePrefix + "1": "nn-123456789", + LabelNetworkNodePrefix + "2": "nn-234567890", + LabelNetworkNodePrefix + "3": "nn-345678901", + }, result.AdditionalLabels) + }) + + t.Run("Should return populated InstanceMetadata with asg label when asg tag is present", func(t *testing.T) { + asgTag := ec2types.Tag{ + Key: aws.String("aws:autoscaling:groupName"), + Value: aws.String("test-asg"), + } + instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true, []ec2types.Tag{asgTag}) + c, _ := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) + var mockedTopologyManager MockedInstanceTopologyManager + c.instanceTopologyManager = &mockedTopologyManager + mockedTopologyManager.On("GetNodeTopology", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&ec2types.InstanceTopology{ + AvailabilityZone: aws.String("us-west-2b"), + GroupName: new(string), + InstanceId: aws.String("i-123456789"), + InstanceType: new(string), + NetworkNodes: []string{"nn-123456789", "nn-234567890", "nn-345678901"}, + ZoneId: aws.String("az2"), + }, nil) + node := &v1.Node{ + Spec: v1.NodeSpec{ + ProviderID: fmt.Sprintf("aws:///us-west-2c/1abc-2def/%s", *instance.InstanceId), + }, + } + + result, err := c.InstanceMetadata(context.TODO(), node) + if err != nil { + t.Errorf("Should not error getting InstanceMetadata: %s", err) + } + + mockedTopologyManager.AssertNumberOfCalls(t, "GetNodeTopology", 1) + assert.Equal(t, "aws:///us-west-2c/1abc-2def/i-00000000000000000", result.ProviderID) + assert.Equal(t, "c3.large", result.InstanceType) + assert.Equal(t, []v1.NodeAddress{ + {Type: "InternalIP", Address: "192.168.0.1"}, + {Type: "ExternalIP", Address: "1.2.3.4"}, + {Type: "InternalDNS", Address: "instance-same.ec2.internal"}, + {Type: "Hostname", Address: "instance-same.ec2.internal"}, + {Type: "ExternalDNS", Address: "instance-same.ec2.external"}, + }, result.NodeAddresses) + assert.Equal(t, "us-west-2a", result.Zone) + assert.Equal(t, "us-west-2", result.Region) + assert.Equal(t, map[string]string{ + LabelZoneID: "az1", + LabelNetworkNodePrefix + "1": "nn-123456789", + LabelNetworkNodePrefix + "2": "nn-234567890", + LabelNetworkNodePrefix + "3": "nn-345678901", + LabelAutoScalingGroupName: "test-asg", + }, result.AdditionalLabels) + }) + + t.Run("Should return populated InstanceMetadata without asg label when asg tag value is empty", func(t *testing.T) { + asgTag := ec2types.Tag{ + Key: aws.String("aws:autoscaling:groupName"), + Value: aws.String(""), + } + instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true, []ec2types.Tag{asgTag}) c, _ := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) var mockedTopologyManager MockedInstanceTopologyManager c.instanceTopologyManager = &mockedTopologyManager @@ -211,7 +308,7 @@ func TestInstanceMetadata(t *testing.T) { }) t.Run("Should skip additional labels if already set", func(t *testing.T) { - instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true) + instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true, nil) c, _ := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) var mockedTopologyManager MockedInstanceTopologyManager c.instanceTopologyManager = &mockedTopologyManager @@ -239,7 +336,7 @@ func TestInstanceMetadata(t *testing.T) { }) t.Run("Should swallow errors if getting node topology fails if instance type not expected to be supported", func(t *testing.T) { - instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true) + instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true, nil) c, _ := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) var mockedTopologyManager MockedInstanceTopologyManager c.instanceTopologyManager = &mockedTopologyManager @@ -264,7 +361,7 @@ func TestInstanceMetadata(t *testing.T) { }) t.Run("Should not swallow errors if getting node topology fails if instance type is expected to be supported", func(t *testing.T) { - instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true) + instance := makeInstance("i-00000000000000000", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true, nil) c, _ := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) var mockedTopologyManager MockedInstanceTopologyManager c.instanceTopologyManager = &mockedTopologyManager @@ -286,7 +383,7 @@ func TestInstanceMetadata(t *testing.T) { }) t.Run("Should limit ec2:DescribeInstances calls to a single request per instance", func(t *testing.T) { - instance := makeInstance("i-00000000000001234", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true) + instance := makeInstance("i-00000000000001234", "192.168.0.1", "1.2.3.4", "instance-same.ec2.internal", "instance-same.ec2.external", nil, true, nil) c, awsServices := mockInstancesResp(&instance, []*ec2types.Instance{&instance}) // Add mock for DescribeInstanceTopology on the EC2 mock diff --git a/pkg/providers/v1/well_known_labels.go b/pkg/providers/v1/well_known_labels.go index be9179c957..083d0540db 100644 --- a/pkg/providers/v1/well_known_labels.go +++ b/pkg/providers/v1/well_known_labels.go @@ -24,4 +24,6 @@ const ( // but will be initially applied to nodes. The suffix should be an incremented // integer starting at 1. LabelNetworkNodePrefix = "topology.k8s.aws/network-node-layer-" + + LabelAutoScalingGroupName = "node.k8s.aws/auto-scaling-group-name" )