Move getSubports() to nodeAddresses()

Author: Jing Zhang

pkg/openstack/instances.go

@@ -45,6 +45,7 @@ import (
 	"k8s.io/cloud-provider-openstack/pkg/util"
 	"k8s.io/cloud-provider-openstack/pkg/util/errors"
 	"k8s.io/cloud-provider-openstack/pkg/util/metadata"
+	"k8s.io/cloud-provider-openstack/pkg/util/openstack"
 )
 
 // Instances encapsulates an implementation of Instances for OpenStack.
@@ -241,7 +242,7 @@ func (i *Instances) NodeAddressesByProviderID(ctx context.Context, providerID st
 		return []v1.NodeAddress{}, err
 	}
 
-	addresses, err := nodeAddresses(server, ports, i.networkingOpts)
+	addresses, err := nodeAddresses(server, ports, i.network, i.networkingOpts)
 	if err != nil {
 		return []v1.NodeAddress{}, err
 	}
@@ -346,7 +347,7 @@ func (i *Instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud
 	if err != nil {
 		return nil, err
 	}
-	addresses, err := nodeAddresses(srv, ports, i.networkingOpts)
+	addresses, err := nodeAddresses(srv, ports, i.network, i.networkingOpts)
 	if err != nil {
 		return nil, err
 	}
@@ -569,21 +570,15 @@ func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*S
 	return &serverList[0], nil
 }
 
-type Address struct {
-	IPType string `mapstructure:"OS-EXT-IPS:type"`
-	Addr   string
-}
-
 // IP addresses order:
 // * interfaces private IPs
 // * access IPs
 // * metadata hostname
 // * server object Addresses (floating type)
-func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) {
+func nodeAddresses(srv *servers.Server, ports []ports.Port, client *gophercloud.ServiceClient, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) {
 	addrs := []v1.NodeAddress{}
 
 	// parse private IP addresses first in an ordered manner
-	addressesByNetworkID := make(map[string][]Address)
 	for _, port := range ports {
 		for _, fixedIP := range port.FixedIPs {
 			if port.Status == "ACTIVE" {
@@ -595,8 +590,6 @@ func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts Netwo
 							Address: fixedIP.IPAddress,
 						},
 					)
-					addr := Address{IPType: "fixed", Addr: fixedIP.IPAddress}
-					addressesByNetworkID[port.NetworkID] = append(addressesByNetworkID[port.NetworkID], addr)
 				}
 			}
 		}
@@ -631,17 +624,43 @@ func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts Netwo
 	}
 
 	// process the rest
+	type Address struct {
+		IPType string `mapstructure:"OS-EXT-IPS:type"`
+		Addr   string
+	}
+
 	var addresses map[string][]Address
 	err := mapstructure.Decode(srv.Addresses, &addresses)
 	if err != nil {
 		return nil, err
 	}
 
 	// Add the addresses assigned on subports via trunk
-	// This adds the vlan networks to which subports are attached
-	if len(addresses) < len(addressesByNetworkID) {
-		nrSubportAddresses := addSubportNodeAddresses(addresses, addressesByNetworkID)
-		klog.V(5).Infof("Node '%s' is added %d subport addresses", srv.Name, nrSubportAddresses)
+	// This exposes the vlan networks to which subports are attached
+	if client != nil {
+		subports, err := getSubports(client, srv.ID)
+		if err == nil {
+			var subportAddresses map[string][]Address
+			for _, subport := range subports {
+				for _, fixedIP := range subport.FixedIPs {
+					isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil
+					if !(isIPv6 && networkingOpts.IPv6SupportDisabled) {
+						addr := Address{IPType: "fixed", Addr: fixedIP.IPAddress}
+						subportAddresses[subport.NetworkID] = append(subportAddresses[subport.NetworkID], addr)
+					}
+				}
+			}
+			for nw, ips := range subportAddresses {
+				srvAddresses, ok := addresses[nw]
+				if !ok {
+					addresses[nw] = ips
+				} else {
+					// this is to take care the corner case
+					// where the same network is attached to the node both directly and via trunk
+					addresses[nw] = append(srvAddresses, ips...)
+				}
+			}
+		}
 	}
 
 	var networks []string
@@ -697,58 +716,8 @@ func nodeAddresses(srv *servers.Server, ports []ports.Port, networkingOpts Netwo
 	return addrs, nil
 }
 
-// addSubportNodeAddresses adds addresses assigned on subports
-func addSubportNodeAddresses(addresses, addressesByNetworkID map[string][]Address) int {
-	subportAddresses := make(map[string][]Address)
-	for network, portAddresses := range addressesByNetworkID {
-		found := false
-		for _, portAddress := range portAddresses {
-			// Check if the address is directly attached to the node
-			for _, nodeAddresses := range addresses {
-				for _, nodeAddress := range nodeAddresses {
-					if portAddress.Addr == nodeAddress.Addr {
-						found = true
-						break
-					}
-				}
-			}
-		}
-		// All the addresses in the vlan network are not directly attached to the node
-		// Save the vlan network
-		if !found {
-			subportAddresses[network] = portAddresses
-		}
-	}
-	// add subport addresses to the node addresses
-	for network, portAddresses := range subportAddresses {
-		srvAddresses, ok := addresses[network]
-		if !ok {
-			addresses[network] = portAddresses
-		} else {
-			// this is to take care the corner case
-			// where the same network is attached to the node both directly and via trunk
-			addresses[network] = append(srvAddresses, portAddresses...)
-		}
-	}
-	return len(subportAddresses)
-}
-
-func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) {
-	srv, err := getServerByName(client, name)
-	if err != nil {
-		return nil, err
-	}
-
-	ports, err := getAttachedPorts(client, srv.ID)
-	if err != nil {
-		return nil, err
-	}
-
-	return nodeAddresses(&srv.Server, ports, networkingOpts)
-}
-
-// getAttachedPorts returns a list of ports attached to a server.
-func getAttachedPorts(client *gophercloud.ServiceClient, serverID string) ([]ports.Port, error) {
+// getSubports returns subports attached to the node via trunk
+func getSubports(client *gophercloud.ServiceClient, serverID string) ([]ports.Port, error) {
 	listOpts := ports.ListOpts{
 		DeviceID:    serverID,
 		DeviceOwner: "compute:nova",
@@ -759,20 +728,19 @@ func getAttachedPorts(client *gophercloud.ServiceClient, serverID string) ([]por
 		trunk_details.TrunkDetailsExt
 	}
 
-	var allPorts []ports.Port
+	var subports []ports.Port
 	var allPortDetails []portWithTrunkDetails
 
 	allPages, err := ports.List(client, listOpts).AllPages()
 	if err != nil {
-		return allPorts, err
+		return subports, err
 	}
 	err = ports.ExtractPortsInto(allPages, &allPortDetails)
 	if err != nil {
-		return allPorts, err
+		return subports, err
 	}
 
 	for _, portExt := range allPortDetails {
-		allPorts = append(allPorts, portExt.Port)
 		// Get subport addresses
 		for _, subport := range portExt.TrunkDetails.SubPorts {
 			p, err := ports.Get(client, subport.PortID).Extract()
@@ -785,12 +753,34 @@ func getAttachedPorts(client *gophercloud.ServiceClient, serverID string) ([]por
 				klog.Errorf("Failed to get subport %s network details: %v", subport.PortID, err)
 				continue
 			}
-			p.Status = "ACTIVE"
 			p.NetworkID = n.Name
-			allPorts = append(allPorts, *p)
+			subports = append(subports, *p)
 		}
 	}
 
-	klog.V(5).Infof("Node %s has %d ports '%v'", serverID, len(allPorts), allPorts)
-	return allPorts, nil
+	return subports, nil
+}
+
+func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) {
+	srv, err := getServerByName(client, name)
+	if err != nil {
+		return nil, err
+	}
+
+	ports, err := getAttachedPorts(client, srv.ID)
+	if err != nil {
+		return nil, err
+	}
+
+	return nodeAddresses(&srv.Server, ports, client, networkingOpts)
+}
+
+// getAttachedPorts returns a list of ports attached to a server.
+func getAttachedPorts(client *gophercloud.ServiceClient, serverID string) ([]ports.Port, error) {
+	listOpts := ports.ListOpts{
+		DeviceID:    serverID,
+		DeviceOwner: "compute:nova",
+	}
+
+	return openstack.GetPorts(client, listOpts)
 }

pkg/openstack/instancesv2.go

@@ -128,7 +128,7 @@ func (i *InstancesV2) InstanceMetadata(ctx context.Context, node *v1.Node) (*clo
 		return nil, err
 	}
 
-	addresses, err := nodeAddresses(&server.Server, ports, i.networkingOpts)
+	addresses, err := nodeAddresses(&server.Server, ports, i.network, i.networkingOpts)
 	if err != nil {
 		return nil, err
 	}