From 43e606a5da4fe9cb5f092f9312bcfb21a6595b0c Mon Sep 17 00:00:00 2001 From: Jason Zhao Date: Thu, 3 Aug 2017 15:57:59 -0700 Subject: [PATCH 1/3] support multiple ec2 ips in aws provider --- pkg/cloudprovider/providers/aws/aws.go | 40 +++++++++--- pkg/cloudprovider/providers/aws/aws_test.go | 72 +++++++++++++++------ 2 files changed, 85 insertions(+), 27 deletions(-) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 13db9301a5b..a892bd0a10c 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -50,6 +50,7 @@ import ( "k8s.io/kubernetes/pkg/controller" kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" "k8s.io/kubernetes/pkg/volume" + "path" ) // ProviderName is the name of this cloud provider. @@ -976,11 +977,21 @@ func (c *Cloud) NodeAddresses(name types.NodeName) ([]v1.NodeAddress, error) { if c.selfAWSInstance.nodeName == name || len(name) == 0 { addresses := []v1.NodeAddress{} - internalIP, err := c.metadata.GetMetadata("local-ipv4") + macs, err := c.metadata.GetMetadata("network/interfaces/macs/") if err != nil { - return nil, fmt.Errorf("error querying AWS metadata for %q: %q", "local-ipv4", err) + return nil, fmt.Errorf("error querying AWS metadata for %q: %q", "network/interfaces/macs", err) + } + + for _, macID := range strings.Split(macs, "\n") { + macPath := path.Join("network/interfaces/macs/", macID, "local-ipv4s") + internalIPs, err := c.metadata.GetMetadata(macPath) + if err != nil { + return nil, fmt.Errorf("error querying AWS metadata for %q: %q", macPath, err) + } + for _, internalIP := range strings.Split(internalIPs, "\n") { + addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: internalIP}) + } } - addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: internalIP}) externalIP, err := c.metadata.GetMetadata("public-ipv4") if err != nil { @@ -1029,13 +1040,24 @@ func extractNodeAddresses(instance *ec2.Instance) ([]v1.NodeAddress, error) { addresses := []v1.NodeAddress{} - privateIPAddress := aws.StringValue(instance.PrivateIpAddress) - if privateIPAddress != "" { - ip := net.ParseIP(privateIPAddress) - if ip == nil { - return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", aws.StringValue(instance.InstanceId), privateIPAddress) + // handle internal network interfaces + for _, networkInterface := range instance.NetworkInterfaces { + // skip network interfaces that are not currently in use + if isNilOrEmpty(networkInterface.Status) || *networkInterface.Status != ec2.NetworkInterfaceStatusInUse { + continue + } + + for _, internalIP := range networkInterface.PrivateIpAddresses { + if !isNilOrEmpty(internalIP.PrivateIpAddress) { + ipAddress := *internalIP.PrivateIpAddress + ip := net.ParseIP(ipAddress) + if ip == nil { + return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", orEmpty(instance.InstanceId), ipAddress) + } + addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: ip.String()}) + } + } - addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: ip.String()}) } // TODO: Other IP addresses (multiple ips)? diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index b1302510e63..cfcde0a5331 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -113,16 +113,17 @@ func TestReadAWSCloudConfig(t *testing.T) { } type FakeAWSServices struct { - region string - instances []*ec2.Instance - selfInstance *ec2.Instance - networkInterfacesMacs []string - networkInterfacesVpcIDs []string + region string + instances []*ec2.Instance + selfInstance *ec2.Instance + networkInterfacesMacs []string + networkInterfacesPrivateIPs [][]string + networkInterfacesVpcIDs []string - ec2 *FakeEC2 - elb *FakeELB - asg *FakeASG - metadata *FakeMetadata + ec2 *FakeEC2 + elb *FakeELB + asg *FakeASG + metadata *FakeMetadata } func NewFakeAWSServices() *FakeAWSServices { @@ -359,6 +360,13 @@ func (self *FakeMetadata) GetMetadata(key string) (string, error) { } } } + if len(keySplit) == 5 && keySplit[4] == "local-ipv4s" { + for i, macElem := range self.aws.networkInterfacesMacs { + if macParam == macElem { + return strings.Join(self.aws.networkInterfacesPrivateIPs[i], "/\n"), nil + } + } + } return "", nil } } else { @@ -553,6 +561,16 @@ func TestNodeAddresses(t *testing.T) { instance0.PrivateIpAddress = aws.String("192.168.0.1") instance0.PublicDnsName = aws.String("instance-same.ec2.external") instance0.PublicIpAddress = aws.String("1.2.3.4") + instance0.NetworkInterfaces = []*ec2.InstanceNetworkInterface{ + { + Status: aws.String(ec2.NetworkInterfaceStatusInUse), + PrivateIpAddresses: []*ec2.InstancePrivateIpAddress{ + { + PrivateIpAddress: aws.String("192.168.0.1"), + }, + }, + }, + } instance0.InstanceType = aws.String("c3.large") instance0.Placement = &ec2.Placement{AvailabilityZone: aws.String("us-east-1a")} state0 := ec2.InstanceState{ @@ -598,6 +616,8 @@ func TestNodeAddresses(t *testing.T) { } aws3, _ := mockInstancesResp(&instance0, instances[0:1]) + // change node name so it uses the instance instead of metadata + aws3.selfAWSInstance.nodeName = "foo" addrs3, err3 := aws3.NodeAddresses("instance-same.ec2.internal") if err3 != nil { t.Errorf("Should not error when instance found") @@ -609,18 +629,34 @@ func TestNodeAddresses(t *testing.T) { testHasNodeAddress(t, addrs3, v1.NodeExternalIP, "1.2.3.4") testHasNodeAddress(t, addrs3, v1.NodeExternalDNS, "instance-same.ec2.external") testHasNodeAddress(t, addrs3, v1.NodeInternalDNS, "instance-same.ec2.internal") +} - // Fetch from metadata - aws4, fakeServices := mockInstancesResp(&instance0, []*ec2.Instance{&instance0}) - fakeServices.selfInstance.PublicIpAddress = aws.String("2.3.4.5") - fakeServices.selfInstance.PrivateIpAddress = aws.String("192.168.0.2") +func TestNodeAddressesWithMetadata(t *testing.T) { + var instance ec2.Instance - addrs4, err4 := aws4.NodeAddresses(mapInstanceToNodeName(&instance0)) - if err4 != nil { - t.Errorf("unexpected error: %v", err4) + instanceName := "instance.ec2.internal" + instance.InstanceId = aws.String("i-0") + instance.PrivateDnsName = &instanceName + instance.PublicIpAddress = aws.String("2.3.4.5") + instance.InstanceType = aws.String("c3.large") + instance.Placement = &ec2.Placement{AvailabilityZone: aws.String("us-east-1a")} + state := ec2.InstanceState{ + Name: aws.String("running"), } - testHasNodeAddress(t, addrs4, v1.NodeInternalIP, "192.168.0.2") - testHasNodeAddress(t, addrs4, v1.NodeExternalIP, "2.3.4.5") + instance.State = &state + + instances := []*ec2.Instance{&instance} + awsCloud, awsServices := mockInstancesResp(&instance, instances) + + awsServices.networkInterfacesMacs = []string{ "0a:26:89:f3:9c:f6", "0a:77:64:c4:6a:48" ,} + awsServices.networkInterfacesPrivateIPs = [][]string{ []string{"192.168.0.1"}, []string{"192.168.0.2"} ,} + addrs, err := awsCloud.NodeAddresses("") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + testHasNodeAddress(t, addrs, v1.NodeInternalIP, "192.168.0.1") + testHasNodeAddress(t, addrs, v1.NodeInternalIP, "192.168.0.2") + testHasNodeAddress(t, addrs, v1.NodeExternalIP, "2.3.4.5") } func TestGetRegion(t *testing.T) { From 8b16c3dafbada6a685aea333704b671497b14c4f Mon Sep 17 00:00:00 2001 From: Jason Zhao Date: Mon, 21 Aug 2017 14:21:02 -0700 Subject: [PATCH 2/3] addressed comments --- pkg/cloudprovider/providers/aws/aws.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index a892bd0a10c..9e2c675fabf 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -983,12 +983,18 @@ func (c *Cloud) NodeAddresses(name types.NodeName) ([]v1.NodeAddress, error) { } for _, macID := range strings.Split(macs, "\n") { + if macID == "" { + continue + } macPath := path.Join("network/interfaces/macs/", macID, "local-ipv4s") internalIPs, err := c.metadata.GetMetadata(macPath) if err != nil { return nil, fmt.Errorf("error querying AWS metadata for %q: %q", macPath, err) } for _, internalIP := range strings.Split(internalIPs, "\n") { + if internalIP == "" { + continue + } addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: internalIP}) } } @@ -1043,20 +1049,18 @@ func extractNodeAddresses(instance *ec2.Instance) ([]v1.NodeAddress, error) { // handle internal network interfaces for _, networkInterface := range instance.NetworkInterfaces { // skip network interfaces that are not currently in use - if isNilOrEmpty(networkInterface.Status) || *networkInterface.Status != ec2.NetworkInterfaceStatusInUse { + if aws.StringValue(networkInterface.Status) != ec2.NetworkInterfaceStatusInUse { continue } for _, internalIP := range networkInterface.PrivateIpAddresses { - if !isNilOrEmpty(internalIP.PrivateIpAddress) { - ipAddress := *internalIP.PrivateIpAddress + if ipAddress := aws.StringValue(internalIP.PrivateIpAddress); ipAddress != "" { ip := net.ParseIP(ipAddress) if ip == nil { - return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%s)", orEmpty(instance.InstanceId), ipAddress) + return nil, fmt.Errorf("EC2 instance had invalid private address: %s (%q)", aws.StringValue(instance.InstanceId), ipAddress) } addresses = append(addresses, v1.NodeAddress{Type: v1.NodeInternalIP, Address: ip.String()}) } - } } From 35777b29977f2597212d6bd2aa80018db984e774 Mon Sep 17 00:00:00 2001 From: Jason Zhao Date: Mon, 21 Aug 2017 15:24:44 -0700 Subject: [PATCH 3/3] run go fmt --- pkg/cloudprovider/providers/aws/aws_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/cloudprovider/providers/aws/aws_test.go b/pkg/cloudprovider/providers/aws/aws_test.go index cfcde0a5331..17ee476862b 100644 --- a/pkg/cloudprovider/providers/aws/aws_test.go +++ b/pkg/cloudprovider/providers/aws/aws_test.go @@ -120,10 +120,10 @@ type FakeAWSServices struct { networkInterfacesPrivateIPs [][]string networkInterfacesVpcIDs []string - ec2 *FakeEC2 - elb *FakeELB - asg *FakeASG - metadata *FakeMetadata + ec2 *FakeEC2 + elb *FakeELB + asg *FakeASG + metadata *FakeMetadata } func NewFakeAWSServices() *FakeAWSServices { @@ -648,8 +648,8 @@ func TestNodeAddressesWithMetadata(t *testing.T) { instances := []*ec2.Instance{&instance} awsCloud, awsServices := mockInstancesResp(&instance, instances) - awsServices.networkInterfacesMacs = []string{ "0a:26:89:f3:9c:f6", "0a:77:64:c4:6a:48" ,} - awsServices.networkInterfacesPrivateIPs = [][]string{ []string{"192.168.0.1"}, []string{"192.168.0.2"} ,} + awsServices.networkInterfacesMacs = []string{"0a:26:89:f3:9c:f6", "0a:77:64:c4:6a:48"} + awsServices.networkInterfacesPrivateIPs = [][]string{{"192.168.0.1"}, {"192.168.0.2"}} addrs, err := awsCloud.NodeAddresses("") if err != nil { t.Errorf("unexpected error: %v", err)