Skip to content

Commit

Permalink
Revert "Allow user to specify the Name tag value for AWS tags"
Browse files Browse the repository at this point in the history 
This reverts commit e0dbf3e.

This revert allows the findInstance function to reliably find the
corresponding EC2 instance for a given AWSMachine again. Previously,
when we allowed users to specify a Name tag, findInstance would not be
able to uniquely guarantee that it could find an EC2 instance since all
created EC2 instances would have the same Name tag.
  • Loading branch information
@mjlshen
mjlshen committed Nov 14, 2023
1 parent abd444c commit a93e910
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 148 deletions.
10 changes: 4 additions & 6 deletions api/v1beta2/tags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,12 +243,6 @@ func (b BuildParams) WithCloudProvider(name string) BuildParams {
// Build builds tags including the cluster tag and returns them in map form.
func Build(params BuildParams) Tags {
tags := make(Tags)

// Add the name tag first so that it can be overwritten by a user-provided tag in the `Additional` tags.
if params.Name != nil {
tags["Name"] = *params.Name
}

for k, v := range params.Additional {
tags[k] = v
}
Expand All @@ -260,5 +254,9 @@ func Build(params BuildParams) Tags {
tags[NameAWSClusterAPIRole] = *params.Role
}

if params.Name != nil {
tags["Name"] = *params.Name
}

return tags
}
15 changes: 5 additions & 10 deletions pkg/cloud/services/network/subnets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,17 +554,12 @@ func (s *Service) getSubnetTagParams(unmanagedVPC bool, id string, public bool,
additionalTags[k] = v
}

// Prefer `Name` tag if given, else generate a name
var name strings.Builder
if manualTagName, ok := manualTags["Name"]; ok {
name.WriteString(manualTagName)
} else {
name.WriteString(s.scope.Name())
name.WriteString("-subnet-")
name.WriteString(role)
name.WriteString("-")
name.WriteString(zone)
}
name.WriteString(s.scope.Name())
name.WriteString("-subnet-")
name.WriteString(role)
name.WriteString("-")
name.WriteString(zone)

return infrav1.BuildParams{
ClusterName: s.scope.Name(),
Expand Down
132 changes: 0 additions & 132 deletions pkg/cloud/services/network/subnets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1920,138 +1920,6 @@ func TestReconcileSubnets(t *testing.T) {
Return(nil, nil)
},
},
{
name: "Managed VPC, existing public subnet, 2 subnets in spec, should create 1 subnet, custom Name tag",
input: NewClusterScope().WithNetwork(&infrav1.NetworkSpec{
VPC: infrav1.VPCSpec{
ID: subnetsVPCID,
Tags: infrav1.Tags{
infrav1.ClusterTagKey("test-cluster"): "owned",
},
},
Subnets: []infrav1.SubnetSpec{
{
ID: "subnet-1",
AvailabilityZone: "us-east-1a",
CidrBlock: "10.0.0.0/17",
IsPublic: true,
},
{
AvailabilityZone: "us-east-1a",
CidrBlock: "10.0.128.0/17",
IsPublic: false,
Tags: map[string]string{"Name": "custom-sub"},
},
},
}),
expect: func(m *mocks.MockEC2APIMockRecorder) {
m.DescribeSubnetsWithContext(context.TODO(), gomock.Eq(&ec2.DescribeSubnetsInput{
Filters: []*ec2.Filter{
{
Name: aws.String("state"),
Values: []*string{aws.String("pending"), aws.String("available")},
},
{
Name: aws.String("vpc-id"),
Values: []*string{aws.String(subnetsVPCID)},
},
},
})).
Return(&ec2.DescribeSubnetsOutput{
Subnets: []*ec2.Subnet{
{
VpcId: aws.String(subnetsVPCID),
SubnetId: aws.String("subnet-1"),
AvailabilityZone: aws.String("us-east-1a"),
CidrBlock: aws.String("10.0.0.0/17"),
Tags: []*ec2.Tag{
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("public"),
},
{
Key: aws.String("Name"),
Value: aws.String("test-cluster-subnet-public"),
},
{
Key: aws.String("kubernetes.io/cluster/test-cluster"),
Value: aws.String("shared"),
},
},
},
},
}, nil)

m.DescribeRouteTablesWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.DescribeRouteTablesInput{})).
Return(&ec2.DescribeRouteTablesOutput{}, nil)

m.DescribeNatGatewaysPagesWithContext(context.TODO(),
gomock.Eq(&ec2.DescribeNatGatewaysInput{
Filter: []*ec2.Filter{
{
Name: aws.String("vpc-id"),
Values: []*string{aws.String(subnetsVPCID)},
},
{
Name: aws.String("state"),
Values: []*string{aws.String("pending"), aws.String("available")},
},
},
}),
gomock.Any()).Return(nil)

m.CreateSubnetWithContext(context.TODO(), gomock.Eq(&ec2.CreateSubnetInput{
VpcId: aws.String(subnetsVPCID),
CidrBlock: aws.String("10.0.128.0/17"),
AvailabilityZone: aws.String("us-east-1a"),
TagSpecifications: []*ec2.TagSpecification{
{
ResourceType: aws.String("subnet"),
Tags: []*ec2.Tag{
{
Key: aws.String("Name"),
Value: aws.String("custom-sub"), // must use the provided `Name` tag, not generate a name
},
{
Key: aws.String("kubernetes.io/cluster/test-cluster"),
Value: aws.String("shared"),
},
{
Key: aws.String("kubernetes.io/role/internal-elb"),
Value: aws.String("1"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Value: aws.String("owned"),
},
{
Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"),
Value: aws.String("private"),
},
},
},
},
})).
Return(&ec2.CreateSubnetOutput{
Subnet: &ec2.Subnet{
VpcId: aws.String(subnetsVPCID),
SubnetId: aws.String("subnet-2"),
CidrBlock: aws.String("10.0.128.0/17"),
AvailabilityZone: aws.String("us-east-1a"),
},
}, nil)

m.WaitUntilSubnetAvailableWithContext(context.TODO(), gomock.Any())

// Public subnet
m.CreateTagsWithContext(context.TODO(), gomock.AssignableToTypeOf(&ec2.CreateTagsInput{})).
Return(nil, nil)
},
},
{
name: "With ManagedControlPlaneScope, Managed VPC, no existing subnets exist, two az's, expect two private and two public from default, created with tag including eksClusterName not a name of Cluster resource",
input: NewManagedControlPlaneScope().
Expand Down

0 comments on commit a93e910

Please sign in to comment.