Skip to content

Commit

Permalink
Merge pull request #508 from jparrill/upstream_backport_r4_15/OCPBUGS…
Browse files Browse the repository at this point in the history 
…-29391

OCPBUGS-32114: UPSTREAM: <carry>: Fix instance PrivateDNSName when domain-name is set in dhcpOpts
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] committed Apr 16, 2024
2 parents 17565ff + f6686b8 commit 52e9ec7
Show file tree
Hide file tree
Showing 18 changed files with 444 additions and 3 deletions.
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/cluster_api_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ func (t Template) ControllersPolicy() *iamv1.PolicyDocument {
"ec2:DescribeSecurityGroups",
"ec2:DescribeSubnets",
"ec2:DescribeVpcs",
"ec2:DescribeDhcpOptions",
"ec2:DescribeVpcAttribute",
"ec2:DescribeVolumes",
"ec2:DescribeTags",
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/customsuffix.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/default.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_all_secret_backends.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_allow_assume_role.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_bootstrap_user.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_custom_bootstrap_user.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_different_instance_profiles.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_eks_console.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_eks_default_roles.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_eks_disable.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_eks_kms_prefix.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_extra_statements.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_s3_bucket.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
1 change: 1 addition & 0 deletions cmd/clusterawsadm/cloudformation/bootstrap/fixtures/with_ssm_secret_backend.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ Resources:
- ec2:DescribeSecurityGroups
- ec2:DescribeSubnets
- ec2:DescribeVpcs
- ec2:DescribeDhcpOptions
- ec2:DescribeVpcAttribute
- ec2:DescribeVolumes
- ec2:DescribeTags
Expand Down
12 changes: 10 additions & 2 deletions controllers/awsmachine_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ func TestAWSMachineReconcilerIntegrationTests(t *testing.T) {
}}})
g.Expect(err).To(BeNil())
cs.Cluster = &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}}
cs.AWSCluster.Spec.NetworkSpec.VPC = infrav1.VPCSpec{
ID: "vpc-exists",
CidrBlock: "10.0.0.0/16",
}
cs.AWSCluster.Status.Network.APIServerELB.DNSName = DNSName
cs.AWSCluster.Spec.ControlPlaneLoadBalancer = &infrav1.AWSLoadBalancerSpec{
LoadBalancerType: infrav1.LoadBalancerTypeClassic,
Expand Down Expand Up @@ -282,6 +286,10 @@ func TestAWSMachineReconcilerIntegrationTests(t *testing.T) {
g.Expect(err).To(BeNil())
cs.Cluster = &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}}
cs.AWSCluster.Status.Network.APIServerELB.DNSName = DNSName
cs.AWSCluster.Spec.NetworkSpec.VPC = infrav1.VPCSpec{
ID: "vpc-exists",
CidrBlock: "10.0.0.0/16",
}
cs.AWSCluster.Spec.ControlPlaneLoadBalancer = &infrav1.AWSLoadBalancerSpec{
LoadBalancerType: infrav1.LoadBalancerTypeClassic,
}
Expand Down Expand Up @@ -531,7 +539,7 @@ func mockedCreateInstanceCalls(m *mocks.MockEC2APIMockRecorder) {
Filters: []*ec2.Filter{
{
Name: aws.String("vpc-id"),
Values: aws.StringSlice([]string{""}),
Values: aws.StringSlice([]string{"vpc-exists"}),
},
{
Name: aws.String("tag:sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"),
Expand Down Expand Up @@ -643,7 +651,7 @@ func mockedCreateInstanceCalls(m *mocks.MockEC2APIMockRecorder) {
},
{
Name: aws.String("vpc-id"),
Values: aws.StringSlice([]string{""}),
Values: aws.StringSlice([]string{"vpc-exists"}),
},
{
Name: aws.String("subnet-id"),
Expand Down
62 changes: 62 additions & 0 deletions pkg/cloud/services/ec2/instances.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
"github.com/pkg/errors"
"k8s.io/utils/pointer"

Expand Down Expand Up @@ -869,6 +870,8 @@ func (s *Service) SDKToInstance(v *ec2.Instance) (*infrav1.Instance, error) {

func (s *Service) getInstanceAddresses(instance *ec2.Instance) []clusterv1.MachineAddress {
addresses := []clusterv1.MachineAddress{}
// Check if the DHCP Option Set has domain name set
domainName := s.GetDHCPOptionSetDomainName(s.EC2Client, instance.VpcId)
for _, eni := range instance.NetworkInterfaces {
privateDNSAddress := clusterv1.MachineAddress{
Type: clusterv1.MachineInternalDNS,
Expand All @@ -878,8 +881,18 @@ func (s *Service) getInstanceAddresses(instance *ec2.Instance) []clusterv1.Machi
Type: clusterv1.MachineInternalIP,
Address: aws.StringValue(eni.PrivateIpAddress),
}

addresses = append(addresses, privateDNSAddress, privateIPAddress)

if domainName != nil {
// Add secondary private DNS Name with domain name set in DHCP Option Set
additionalPrivateDNSAddress := clusterv1.MachineAddress{
Type: clusterv1.MachineInternalDNS,
Address: fmt.Sprintf("%s.%s", strings.Split(privateDNSAddress.Address, ".")[0], *domainName),
}
addresses = append(addresses, additionalPrivateDNSAddress)
}

// An elastic IP is attached if association is non nil pointer
if eni.Association != nil {
publicDNSAddress := clusterv1.MachineAddress{
Expand All @@ -893,6 +906,7 @@ func (s *Service) getInstanceAddresses(instance *ec2.Instance) []clusterv1.Machi
addresses = append(addresses, publicDNSAddress, publicIPAddress)
}
}

return addresses
}

Expand Down Expand Up @@ -991,6 +1005,54 @@ func (s *Service) ModifyInstanceMetadataOptions(instanceID string, options *infr
return nil
}

// GetDHCPOptionSetDomainName returns the domain DNS name for the VPC from the DHCP Options.
func (s *Service) GetDHCPOptionSetDomainName(ec2client ec2iface.EC2API, vpcID *string) *string {
log := s.scope.GetLogger()

if vpcID == nil {
log.Info("vpcID is nil, skipping DHCP Option Set discovery")
return nil
}

vpcInput := &ec2.DescribeVpcsInput{
VpcIds: []*string{vpcID},
}

vpcResult, err := ec2client.DescribeVpcs(vpcInput)
if err != nil {
log.Info("failed to describe VPC, skipping DHCP Option Set discovery", "vpcID", *vpcID, "Error", err.Error())
return nil
}

dhcpInput := &ec2.DescribeDhcpOptionsInput{
DhcpOptionsIds: []*string{vpcResult.Vpcs[0].DhcpOptionsId},
}

dhcpResult, err := ec2client.DescribeDhcpOptions(dhcpInput)
if err != nil {
log.Error(err, "failed to describe DHCP Options Set", "DhcpOptionsSet", *dhcpResult)
return nil
}

for _, dhcpConfig := range dhcpResult.DhcpOptions[0].DhcpConfigurations {
if *dhcpConfig.Key == "domain-name" {
if len(dhcpConfig.Values) == 0 {
return nil
}
domainName := dhcpConfig.Values[0].Value
// default domainName is 'ec2.internal' in us-east-1 and 'region.compute.internal' in the other regions.
if (s.scope.Region() == "us-east-1" && *domainName == "ec2.internal") ||
(s.scope.Region() != "us-east-1" && *domainName == fmt.Sprintf("%s.compute.internal", s.scope.Region())) {
return nil
}

return domainName
}
}

return nil
}

// filterGroups filters a list for a string.
func filterGroups(list []string, strToFilter string) (newList []string) {
for _, item := range list {
Expand Down

0 comments on commit 52e9ec7

Please sign in to comment.