Merge pull request #318 from mgugino-upstream-stage/custom-dns-name-s…

…upport

Bug 1822200: Custom dns name support

pkg/actuators/machine/actuator_test.go

@@ -177,6 +177,8 @@ func TestMachineEvents(t *testing.T) {
 			mockAWSClient.EXPECT().ELBv2DescribeLoadBalancers(gomock.Any()).Return(stubDescribeLoadBalancersOutput(), nil).AnyTimes()
 			mockAWSClient.EXPECT().ELBv2DescribeTargetGroups(gomock.Any()).Return(stubDescribeTargetGroupsOutput(), nil).AnyTimes()
 			mockAWSClient.EXPECT().ELBv2RegisterTargets(gomock.Any()).Return(nil, nil).AnyTimes()
+			mockAWSClient.EXPECT().DescribeVpcs(gomock.Any()).Return(StubDescribeVPCs()).AnyTimes()
+			mockAWSClient.EXPECT().DescribeDHCPOptions(gomock.Any()).Return(StubDescribeDHCPOptions()).AnyTimes()
 
 			params := ActuatorParams{
 				Client:           k8sClient,

pkg/actuators/machine/machine_scope.go

@@ -3,6 +3,7 @@ package machine
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"github.com/aws/aws-sdk-go/service/ec2"
 	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
@@ -18,6 +19,9 @@ const (
 	userDataSecretKey = "userData"
 )
 
+// dhcpDomainKeyName is a variable so we can reference it in unit tests.
+var dhcpDomainKeyName = "domain-name"
+
 // machineScopeParams defines the input parameters used to create a new MachineScope.
 type machineScopeParams struct {
 	context.Context
@@ -144,7 +148,13 @@ func (s *machineScope) setProviderStatus(instance *ec2.Instance, condition awspr
 		s.providerStatus.InstanceID = instance.InstanceId
 		s.providerStatus.InstanceState = instance.State.Name
 
-		addresses, err := extractNodeAddresses(instance)
+		domainNames, err := s.getCustomDomainFromDHCP(instance.VpcId)
+
+		if err != nil {
+			return err
+		}
+
+		addresses, err := extractNodeAddresses(instance, domainNames)
 		if err != nil {
 			klog.Errorf("%s: Error extracting instance IP addresses: %v", s.machine.Name, err)
 			return err
@@ -159,3 +169,36 @@ func (s *machineScope) setProviderStatus(instance *ec2.Instance, condition awspr
 
 	return nil
 }
+
+func (s *machineScope) getCustomDomainFromDHCP(vpcID *string) ([]string, error) {
+	vpc, err := s.awsClient.DescribeVpcs(&ec2.DescribeVpcsInput{
+		VpcIds: []*string{vpcID},
+	})
+	if err != nil {
+		klog.Errorf("%s: error describing vpc: %v", s.machine.Name, err)
+		return nil, err
+	}
+
+	if len(vpc.Vpcs) == 0 || vpc.Vpcs[0] == nil || vpc.Vpcs[0].DhcpOptionsId == nil {
+		return nil, nil
+	}
+
+	dhcp, err := s.awsClient.DescribeDHCPOptions(&ec2.DescribeDhcpOptionsInput{
+		DhcpOptionsIds: []*string{vpc.Vpcs[0].DhcpOptionsId},
+	})
+	if err != nil {
+		klog.Errorf("%s: error describing dhcp: %v", s.machine.Name, err)
+		return nil, err
+	}
+
+	if dhcp == nil || len(dhcp.DhcpOptions) == 0 || dhcp.DhcpOptions[0] == nil {
+		return nil, nil
+	}
+
+	for _, i := range dhcp.DhcpOptions[0].DhcpConfigurations {
+		if i.Key != nil && *i.Key == dhcpDomainKeyName && len(i.Values) > 0 && i.Values[0] != nil && i.Values[0].Value != nil {
+			return strings.Split(*i.Values[0].Value, " "), nil
+		}
+	}
+	return nil, nil
+}

pkg/actuators/machine/machine_scope_test.go

@@ -5,8 +5,11 @@ import (
 	"context"
 	"fmt"
 	"path/filepath"
+	"reflect"
 	"testing"
 
+	"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/golang/mock/gomock"
 	. "github.com/onsi/gomega"
 	machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1"
 	corev1 "k8s.io/api/core/v1"
@@ -15,6 +18,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	awsproviderv1 "sigs.k8s.io/cluster-api-provider-aws/pkg/apis/awsprovider/v1beta1"
 	awsclient "sigs.k8s.io/cluster-api-provider-aws/pkg/client"
+	mockaws "sigs.k8s.io/cluster-api-provider-aws/pkg/client/mock"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -289,3 +293,55 @@ func TestPatchMachine(t *testing.T) {
 		})
 	}
 }
+
+func TestGetCustomDomainFromDHCP(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	mockAWSClient := mockaws.NewMockClient(mockCtrl)
+	dhcpID := "someID"
+	expectedDomains := "openshift.com openshift.io"
+	mockAWSClient.EXPECT().DescribeVpcs(gomock.Any()).Return(&ec2.DescribeVpcsOutput{
+		Vpcs: []*ec2.Vpc{
+			&ec2.Vpc{DhcpOptionsId: &dhcpID},
+		},
+	}, nil).AnyTimes()
+
+	mockAWSClient.EXPECT().DescribeDHCPOptions(gomock.Any()).Return(&ec2.DescribeDhcpOptionsOutput{
+		DhcpOptions: []*ec2.DhcpOptions{
+			&ec2.DhcpOptions{
+				DhcpConfigurations: []*ec2.DhcpConfiguration{
+					&ec2.DhcpConfiguration{
+						Key: &dhcpDomainKeyName,
+						Values: []*ec2.AttributeValue{
+							&ec2.AttributeValue{
+								Value: &expectedDomains,
+							},
+						},
+					},
+				},
+			},
+		},
+	}, nil).AnyTimes()
+
+	testCases := []struct {
+		description string
+		expected    []string
+	}{
+		{
+			description: "Returns two",
+			expected:    []string{"openshift.com", "openshift.io"},
+		},
+	}
+
+	for _, tc := range testCases {
+		mS := machineScope{
+			awsClient: mockAWSClient,
+		}
+		got, err := mS.getCustomDomainFromDHCP(nil)
+		if err != nil {
+			t.Errorf("error when calling getCustomDomainFromDHCP: %v", err)
+		}
+		if !reflect.DeepEqual(got, tc.expected) {
+			t.Errorf("Case: %s. Got: %v, expected: %v", tc.description, got, tc.expected)
+		}
+	}
+}

pkg/actuators/machine/reconciler_test.go

@@ -144,6 +144,8 @@ func TestAvailabilityZone(t *testing.T) {
 			mockAWSClient.EXPECT().RegisterInstancesWithLoadBalancer(gomock.Any()).AnyTimes()
 			mockAWSClient.EXPECT().DescribeAvailabilityZones(gomock.Any()).Return(nil, nil).AnyTimes()
 			mockAWSClient.EXPECT().DescribeSubnets(gomock.Any()).Return(&ec2.DescribeSubnetsOutput{}, nil)
+			mockAWSClient.EXPECT().DescribeVpcs(gomock.Any()).Return(StubDescribeVPCs()).AnyTimes()
+			mockAWSClient.EXPECT().DescribeDHCPOptions(gomock.Any()).Return(StubDescribeDHCPOptions()).AnyTimes()
 
 			err = reconciler.create()
 			if tc.expectedError != nil {
@@ -195,6 +197,8 @@ func TestCreate(t *testing.T) {
 	mockAWSClient.EXPECT().ELBv2DescribeLoadBalancers(gomock.Any()).Return(stubDescribeLoadBalancersOutput(), nil)
 	mockAWSClient.EXPECT().ELBv2DescribeTargetGroups(gomock.Any()).Return(stubDescribeTargetGroupsOutput(), nil).AnyTimes()
 	mockAWSClient.EXPECT().ELBv2RegisterTargets(gomock.Any()).Return(nil, nil).AnyTimes()
+	mockAWSClient.EXPECT().DescribeVpcs(gomock.Any()).Return(StubDescribeVPCs()).AnyTimes()
+	mockAWSClient.EXPECT().DescribeDHCPOptions(gomock.Any()).Return(StubDescribeDHCPOptions()).AnyTimes()
 
 	testCases := []struct {
 		testcase             string

pkg/actuators/machine/stubs.go

@@ -279,3 +279,31 @@ func stubDescribeInstancesOutput(imageID, instanceID string, state string) *ec2.
 		},
 	}
 }
+
+// StubDescribeDHCPOptions provides fake output
+func StubDescribeDHCPOptions() (*ec2.DescribeDhcpOptionsOutput, error) {
+	key := "key"
+	return &ec2.DescribeDhcpOptionsOutput{
+		DhcpOptions: []*ec2.DhcpOptions{
+			&ec2.DhcpOptions{
+				DhcpConfigurations: []*ec2.DhcpConfiguration{
+					&ec2.DhcpConfiguration{
+						Key:    &key,
+						Values: []*ec2.AttributeValue{},
+					},
+				},
+			},
+		},
+	}, nil
+}
+
+// StubDescribeVPCs provides fake output
+func StubDescribeVPCs() (*ec2.DescribeVpcsOutput, error) {
+	return &ec2.DescribeVpcsOutput{
+		Vpcs: []*ec2.Vpc{
+			{
+				VpcId: aws.String("vpc-32677e0e794418639"),
+			},
+		},
+	}, nil
+}

pkg/actuators/machine/utils.go

@@ -249,7 +249,7 @@ func shouldUpdateCondition(newCondition, existingCondition *awsproviderv1.AWSMac
 }
 
 // extractNodeAddresses maps the instance information from EC2 to an array of NodeAddresses
-func extractNodeAddresses(instance *ec2.Instance) ([]corev1.NodeAddress, error) {
+func extractNodeAddresses(instance *ec2.Instance, domainNames []string) ([]corev1.NodeAddress, error) {
 	// Not clear if the order matters here, but we might as well indicate a sensible preference order
 
 	if instance == nil {
@@ -304,6 +304,12 @@ func extractNodeAddresses(instance *ec2.Instance) ([]corev1.NodeAddress, error)
 	if privateDNSName != "" {
 		addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeInternalDNS, Address: privateDNSName})
 		addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeHostName, Address: privateDNSName})
+		for _, dn := range domainNames {
+			customHostName := strings.Join([]string{strings.Split(privateDNSName, ".")[0], dn}, ".")
+			if customHostName != privateDNSName {
+				addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeInternalDNS, Address: customHostName})
+			}
+		}
 	}
 
 	publicDNSName := aws.StringValue(instance.PublicDnsName)

pkg/actuators/machine/utils_test.go

@@ -21,6 +21,7 @@ func TestExtractNodeAddresses(t *testing.T) {
 		testcase          string
 		instance          *ec2.Instance
 		expectedAddresses []corev1.NodeAddress
+		domainNames       []string
 	}{
 		{
 			testcase: "one-public",
@@ -32,6 +33,7 @@ func TestExtractNodeAddresses(t *testing.T) {
 				{Type: corev1.NodeExternalIP, Address: "1.1.1.1"},
 				{Type: corev1.NodeExternalDNS, Address: "ec2.example.net"},
 			},
+			domainNames: nil,
 		},
 		{
 			testcase: "one-private",
@@ -54,6 +56,55 @@ func TestExtractNodeAddresses(t *testing.T) {
 				{Type: corev1.NodeInternalDNS, Address: "ec2.example.net"},
 				{Type: corev1.NodeHostName, Address: "ec2.example.net"},
 			},
+			domainNames: nil,
+		},
+		{
+			testcase: "custom-domain",
+			instance: &ec2.Instance{
+				PrivateDnsName: aws.String("ec2.example.net"),
+				NetworkInterfaces: []*ec2.InstanceNetworkInterface{
+					{
+						Status: aws.String(ec2.NetworkInterfaceStatusInUse),
+						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
+							{
+								Primary:          aws.Bool(true),
+								PrivateIpAddress: aws.String("10.0.0.5"),
+							},
+						},
+					},
+				},
+			},
+			expectedAddresses: []corev1.NodeAddress{
+				{Type: corev1.NodeInternalIP, Address: "10.0.0.5"},
+				{Type: corev1.NodeInternalDNS, Address: "ec2.example.net"},
+				{Type: corev1.NodeHostName, Address: "ec2.example.net"},
+				{Type: corev1.NodeInternalDNS, Address: "ec2.openshift.com"},
+				{Type: corev1.NodeInternalDNS, Address: "ec2.openshift.io"},
+			},
+			domainNames: []string{"openshift.com", "openshift.io"},
+		},
+		{
+			testcase: "custom-domain no duplicates",
+			instance: &ec2.Instance{
+				PrivateDnsName: aws.String("ec2.example.net"),
+				NetworkInterfaces: []*ec2.InstanceNetworkInterface{
+					{
+						Status: aws.String(ec2.NetworkInterfaceStatusInUse),
+						PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{
+							{
+								Primary:          aws.Bool(true),
+								PrivateIpAddress: aws.String("10.0.0.5"),
+							},
+						},
+					},
+				},
+			},
+			expectedAddresses: []corev1.NodeAddress{
+				{Type: corev1.NodeInternalIP, Address: "10.0.0.5"},
+				{Type: corev1.NodeInternalDNS, Address: "ec2.example.net"},
+				{Type: corev1.NodeHostName, Address: "ec2.example.net"},
+			},
+			domainNames: []string{"example.net", "example.net"},
 		},
 		{
 			testcase: "multiple-private",
@@ -86,6 +137,7 @@ func TestExtractNodeAddresses(t *testing.T) {
 				{Type: corev1.NodeInternalDNS, Address: "ec2.example.net"},
 				{Type: corev1.NodeHostName, Address: "ec2.example.net"},
 			},
+			domainNames: nil,
 		},
 		{
 			testcase: "ipv6-private",
@@ -114,12 +166,13 @@ func TestExtractNodeAddresses(t *testing.T) {
 				{Type: corev1.NodeInternalDNS, Address: "ec2.example.net"},
 				{Type: corev1.NodeHostName, Address: "ec2.example.net"},
 			},
+			domainNames: nil,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.testcase, func(t *testing.T) {
-			addresses, err := extractNodeAddresses(tc.instance)
+			addresses, err := extractNodeAddresses(tc.instance, tc.domainNames)
 			if err != nil {
 				t.Errorf("Unexpected extractNodeAddresses error: %v", err)
 			}

pkg/client/client.go

@@ -52,6 +52,7 @@ type AwsClientBuilderFuncType func(client client.Client, secretName, namespace,
 // Client is a wrapper object for actual AWS SDK clients to allow for easier testing.
 type Client interface {
 	DescribeImages(*ec2.DescribeImagesInput) (*ec2.DescribeImagesOutput, error)
+	DescribeDHCPOptions(input *ec2.DescribeDhcpOptionsInput) (*ec2.DescribeDhcpOptionsOutput, error)
 	DescribeVpcs(*ec2.DescribeVpcsInput) (*ec2.DescribeVpcsOutput, error)
 	DescribeSubnets(*ec2.DescribeSubnetsInput) (*ec2.DescribeSubnetsOutput, error)
 	DescribeAvailabilityZones(*ec2.DescribeAvailabilityZonesInput) (*ec2.DescribeAvailabilityZonesOutput, error)
@@ -73,6 +74,10 @@ type awsClient struct {
 	elbv2Client elbv2iface.ELBV2API
 }
 
+func (c *awsClient) DescribeDHCPOptions(input *ec2.DescribeDhcpOptionsInput) (*ec2.DescribeDhcpOptionsOutput, error) {
+	return c.ec2Client.DescribeDhcpOptions(input)
+}
+
 func (c *awsClient) DescribeImages(input *ec2.DescribeImagesInput) (*ec2.DescribeImagesOutput, error) {
 	return c.ec2Client.DescribeImages(input)
 }

pkg/client/fake/fake.go

@@ -8,6 +8,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/elb"
 	"github.com/aws/aws-sdk-go/service/elbv2"
 	"k8s.io/client-go/kubernetes"
+	"sigs.k8s.io/cluster-api-provider-aws/pkg/actuators/machine"
 	"sigs.k8s.io/cluster-api-provider-aws/pkg/client"
 )
 
@@ -25,13 +26,7 @@ func (c *awsClient) DescribeImages(input *ec2.DescribeImagesInput) (*ec2.Describ
 }
 
 func (c *awsClient) DescribeVpcs(input *ec2.DescribeVpcsInput) (*ec2.DescribeVpcsOutput, error) {
-	return &ec2.DescribeVpcsOutput{
-		Vpcs: []*ec2.Vpc{
-			{
-				VpcId: aws.String("vpc-32677e0e794418639"),
-			},
-		},
-	}, nil
+	return machine.StubDescribeVPCs()
 }
 
 func (c *awsClient) DescribeSubnets(input *ec2.DescribeSubnetsInput) (*ec2.DescribeSubnetsOutput, error) {
@@ -58,6 +53,10 @@ func (c *awsClient) DescribeSecurityGroups(input *ec2.DescribeSecurityGroupsInpu
 	}, nil
 }
 
+func (c *awsClient) DescribeDHCPOptions(input *ec2.DescribeDhcpOptionsInput) (*ec2.DescribeDhcpOptionsOutput, error) {
+	return machine.StubDescribeDHCPOptions()
+}
+
 func (c *awsClient) RunInstances(input *ec2.RunInstancesInput) (*ec2.Reservation, error) {
 	return &ec2.Reservation{
 		Instances: []*ec2.Instance{