Automated cherry pick of kubernetes#1946: Support specifying a sort order for node addresses (kubernetes#2031)

* Support specifying a sort order for node addresses

This introduces a configuration key which influences the way the
provider reports the node addresses to the Kubernetes node resource.

The default order depends on the hard-coded order the provider queries
the addresses and what the cloud returns, which does not guarantee a
specific order.

To override this behavior it is possible to specify a comma separated
list of CIDRs. Essentially, this will sort and group all addresses
matching a CIDR in a prioritized manner, where the first item having a
higher priority than the last. All non-matching addresses will remain in
the same order they are already in.

For example, this option can be useful when having multiple or
dual-stack interfaces attached to a node and needing a user-controlled,
deterministic way of sorting the addresses.

* Update gofmt for various files

Author: ederst

docs/openstack-cloud-controller-manager/using-openstack-cloud-controller-manager.md

@@ -167,6 +167,13 @@ The options in `Global` section are used for openstack-cloud-controller-manager
   The name of Neutron external network. openstack-cloud-controller-manager uses this option when getting the external IP of the Kubernetes node. Can be specified multiple times. Specified network names will be ORed. Default: ""
 * `internal-network-name`
   The name of Neutron internal network. openstack-cloud-controller-manager uses this option when getting the internal IP of the Kubernetes node, this is useful if the node has multiple interfaces. Can be specified multiple times. Specified network names will be ORed. Default: ""
+* `address-sort-order`
+  This configuration key influences the way the provider reports the node addresses to the Kubernetes node resource. The default order depends on the hard-coded order the provider queries the addresses and what the cloud returns, which does not guarantee a specific order.
+
+  To override this behavior it is possible to specify a comma separated list of CIDRs. Essentially, this will sort and group all addresses matching a CIDR in a prioritized manner, where the first item having a higher priority than the last. All non-matching addresses will remain in the same order they are already in.
+
+  For example, this option can be useful when having multiple or dual-stack interfaces attached to a node and needing a user-controlled, deterministic way of sorting the addresses.
+  Default: ""
 
 ###  Load Balancer
 

pkg/autohealing/cloudprovider/openstack/provider.go

@@ -251,10 +251,11 @@ func (provider OpenStackCloudProvider) waitForServerDetachVolumes(serverID strin
 }
 
 // FirstTimeRepair Handle the first time repair for a node
-// 1) If the node is the first time in error, reboot and uncordon it
-// 2) If the node is not the first time in error, check if the last reboot time is in provider.Config.RebuildDelayAfterReboot
-//    That said, if the node has been found in broken status before but has been long time since then, the processed variable
-//    will be kept as False, which means the node need to be rebuilt to fix it, otherwise it means the has been processed.
+//  1. If the node is the first time in error, reboot and uncordon it
+//  2. If the node is not the first time in error, check if the last reboot time is in provider.Config.RebuildDelayAfterReboot
+//     That said, if the node has been found in broken status before but has been long time since then, the processed variable
+//     will be kept as False, which means the node need to be rebuilt to fix it, otherwise it means the has been processed.
+//
 // The bool type return value means that if the node has been processed from a first time repair PoV
 func (provider OpenStackCloudProvider) firstTimeRepair(n healthcheck.NodeInfo, serverID string, firstTimeRebootNodes map[string]healthcheck.NodeInfo) (bool, error) {
 	var firstTimeUnhealthy = true
@@ -312,12 +313,14 @@ func (provider OpenStackCloudProvider) firstTimeRepair(n healthcheck.NodeInfo, s
 }
 
 // Repair  For master nodes: detach etcd and docker volumes, find the root
-//         volume, then shutdown the VM, marks the both the VM and the root
-//         volume (heat resource) as "unhealthy" then trigger Heat stack update
-//         in order to rebuild the node. The information this function needs:
-//         - Nova VM ID
-//         - Root volume ID
-// 	       - Heat stack ID and resource ID.
+//
+//	        volume, then shutdown the VM, marks the both the VM and the root
+//	        volume (heat resource) as "unhealthy" then trigger Heat stack update
+//	        in order to rebuild the node. The information this function needs:
+//	        - Nova VM ID
+//	        - Root volume ID
+//		       - Heat stack ID and resource ID.
+//
 // For worker nodes: Call Magnum resize API directly.
 func (provider OpenStackCloudProvider) Repair(nodes []healthcheck.NodeInfo) error {
 	if len(nodes) == 0 {

pkg/csi/cinder/openstack/openstack_snapshots.go

@@ -142,7 +142,7 @@ func (os *OpenStack) DeleteSnapshot(snapID string) error {
 	return err
 }
 
-//GetSnapshotByID returns snapshot details by id
+// GetSnapshotByID returns snapshot details by id
 func (os *OpenStack) GetSnapshotByID(snapshotID string) (*snapshots.Snapshot, error) {
 	s, err := snapshots.Get(os.blockstorage, snapshotID).Extract()
 	if err != nil {

pkg/csi/cinder/openstack/openstack_volumes.go

@@ -232,7 +232,7 @@ func (os *OpenStack) WaitDiskAttached(instanceID string, volumeID string) error
 	return err
 }
 
-//WaitVolumeTargetStatus waits for volume to be in target state
+// WaitVolumeTargetStatus waits for volume to be in target state
 func (os *OpenStack) WaitVolumeTargetStatus(volumeID string, tStatus []string) error {
 	backoff := wait.Backoff{
 		Duration: operationFinishInitDelay,
@@ -367,7 +367,7 @@ func (os *OpenStack) ExpandVolume(volumeID string, status string, newSize int) e
 	return fmt.Errorf("volume cannot be resized, when status is %s", status)
 }
 
-//GetMaxVolLimit returns max vol limit
+// GetMaxVolLimit returns max vol limit
 func (os *OpenStack) GetMaxVolLimit() int64 {
 	if os.bsOpts.NodeVolumeAttachLimit > 0 && os.bsOpts.NodeVolumeAttachLimit <= 256 {
 		return os.bsOpts.NodeVolumeAttachLimit

pkg/csi/manila/validator/validator.go

@@ -28,20 +28,20 @@ import (
 // By default, the input data has to contain a value for each struct field.
 //
 // Available tags:
-// * name:"FIELD-NAME" : key of the value in the input map
-// * value: modifies value requirements:
-//   value:"required" : field is required
-//   value:"optional" : field is optional
-//   value:"requiredIf:FIELD-NAME=REGEXP-PATTERN" : field is required if the value of FIELD-NAME matches REGEXP-PATTERN
-//   value:"optionalIf:FIELD-NAME=REGEXP-PATTERN" : field is optional if the value of FIELD-NAME matches REGEXP-PATTERN
-//   value:"default:VALUE" : field value defaults to VALUE
-// * dependsOn:"FIELD-NAMES|,..." : if this field is not empty, the specified fields are required to be present
-//   operator ',' acts as AND
-//   operator '|' acts as XOR
-//   e.g.: dependsOn:"f1|f2|f3,f4,f5" : if this field is not empty, exactly one of {f1,f2,f3} is required to be present,
-//         and f4 and f5 is required to be present
-// * precludes:"FIELD-NAMES,..." : if this field is not empty, all specified fields are required to be empty
-// * matches:"REGEXP-PATTERN" : if this field is not empty, it's required to match REGEXP-PATTERN
+//   - name:"FIELD-NAME" : key of the value in the input map
+//   - value: modifies value requirements:
+//     value:"required" : field is required
+//     value:"optional" : field is optional
+//     value:"requiredIf:FIELD-NAME=REGEXP-PATTERN" : field is required if the value of FIELD-NAME matches REGEXP-PATTERN
+//     value:"optionalIf:FIELD-NAME=REGEXP-PATTERN" : field is optional if the value of FIELD-NAME matches REGEXP-PATTERN
+//     value:"default:VALUE" : field value defaults to VALUE
+//   - dependsOn:"FIELD-NAMES|,..." : if this field is not empty, the specified fields are required to be present
+//     operator ',' acts as AND
+//     operator '|' acts as XOR
+//     e.g.: dependsOn:"f1|f2|f3,f4,f5" : if this field is not empty, exactly one of {f1,f2,f3} is required to be present,
+//     and f4 and f5 is required to be present
+//   - precludes:"FIELD-NAMES,..." : if this field is not empty, all specified fields are required to be empty
+//   - matches:"REGEXP-PATTERN" : if this field is not empty, it's required to match REGEXP-PATTERN
 type Validator struct {
 	t reflect.Type
 

pkg/ingress/controller/controller.go

@@ -1033,7 +1033,6 @@ func privateKeyFromPEM(pemData []byte) (crypto.PrivateKey, error) {
 
 // parsePEMBundle parses a certificate bundle from top to bottom and returns
 // a slice of x509 certificates. This function will error if no certificates are found.
-//
 func parsePEMBundle(bundle []byte) ([]*x509.Certificate, error) {
 	var certificates []*x509.Certificate
 	var certDERBlock *pem.Block

pkg/kms/barbican/barbican.go

@@ -15,7 +15,7 @@ type KMSOpts struct {
 	KeyID string `gcfg:"key-id"`
 }
 
-//Config to read config options
+// Config to read config options
 type Config struct {
 	Global     client.AuthOpts
 	KeyManager KMSOpts

pkg/openstack/instances.go

@@ -17,6 +17,7 @@ limitations under the License.
 package openstack
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"io/ioutil"
@@ -53,10 +54,79 @@ type Instances struct {
 
 const (
 	instanceShutoff = "SHUTOFF"
+	noSortPriority  = 0
 )
 
 var _ cloudprovider.Instances = &Instances{}
 
+// buildAddressSortOrderList builds a list containing only valid CIDRs based on the content of addressSortOrder.
+//
+// It will ignore and warn about invalid sort order items.
+func buildAddressSortOrderList(addressSortOrder string) []*net.IPNet {
+	var list []*net.IPNet
+	for _, item := range strings.Split(addressSortOrder, ",") {
+		item = strings.TrimSpace(item)
+
+		_, cidr, err := net.ParseCIDR(item)
+		if err != nil {
+			klog.Warningf("Ignoring invalid sort order item '%s': %v.", item, err)
+			continue
+		}
+
+		list = append(list, cidr)
+	}
+
+	return list
+}
+
+// getSortPriority returns the priority as int of an address.
+//
+// The priority depends on the index of the CIDR in the list the address is matching,
+// where the first item of the list has higher priority than the last.
+//
+// If the address does not match any CIDR or is not an IP address the function returns noSortPriority.
+func getSortPriority(list []*net.IPNet, address string) int {
+	parsedAddress := net.ParseIP(address)
+	if parsedAddress == nil {
+		return noSortPriority
+	}
+
+	for i, cidr := range list {
+		if cidr.Contains(parsedAddress) {
+			fmt.Println(i, cidr, len(list)-i)
+			return len(list) - i
+		}
+	}
+
+	return noSortPriority
+}
+
+// sortNodeAddresses sorts node addresses based on comma separated list of CIDRs represented by addressSortOrder.
+//
+// The function only sorts addresses which match the CIDR and leaves the other addresses in the same order they are in.
+// Essentially, it will also group the addresses matching a CIDR together and sort them ascending in this group,
+// whereas the inter-group sorting depends on the priority.
+//
+// The priority depends on the order of the item in addressSortOrder, where the first item has higher priority than the last.
+func sortNodeAddresses(addresses []v1.NodeAddress, addressSortOrder string) {
+	list := buildAddressSortOrderList(addressSortOrder)
+
+	sort.SliceStable(addresses, func(i int, j int) bool {
+		addressLeft := addresses[i]
+		addressRight := addresses[j]
+
+		priorityLeft := getSortPriority(list, addressLeft.Address)
+		priorityRight := getSortPriority(list, addressRight.Address)
+
+		// ignore priorities of value 0 since this means the address has noSortPriority and we need to sort by priority
+		if priorityLeft > noSortPriority && priorityLeft == priorityRight {
+			return bytes.Compare(net.ParseIP(addressLeft.Address), net.ParseIP(addressRight.Address)) < 0
+		}
+
+		return priorityLeft > priorityRight
+	})
+}
+
 // Instances returns an implementation of Instances for OpenStack.
 func (os *OpenStack) Instances() (cloudprovider.Instances, bool) {
 	return os.instances()
@@ -561,6 +631,10 @@ func nodeAddresses(srv *servers.Server, interfaces []attachinterfaces.Interface,
 		}
 	}
 
+	if networkingOpts.AddressSortOrder != "" {
+		sortNodeAddresses(addrs, networkingOpts.AddressSortOrder)
+	}
+
 	return addrs, nil
 }