Merge pull request #132 from shiftstack/merge-bot-master

Merge https://github.com/kubernetes/cloud-provider-openstack:master into master

Author: openshift-merge-robot

charts/openstack-cloud-controller-manager/templates/_helpers.tpl

@@ -51,27 +51,27 @@ Create cloud-config makro.
 {{- define "cloudConfig" -}}
 [Global]
 {{- range $key, $value := .Values.cloudConfig.global }}
-{{ $key }} = {{ $value }}
+{{ $key }} = {{ $value | quote }}
 {{- end }}
 
 [Networking]
 {{- range $key, $value := .Values.cloudConfig.networking }}
-{{ $key }} = {{ $value }}
+{{ $key }} = {{ $value | quote }}
 {{- end }}
 
 [LoadBalancer]
 {{- range $key, $value := .Values.cloudConfig.loadBalancer }}
-{{ $key }} = {{ $value }}
+{{ $key }} = {{ $value | quote }}
 {{- end }}
 
 [BlockStorage]
 {{- range $key, $value := .Values.cloudConfig.blockStorage }}
-{{ $key }} = {{ $value }}
+{{ $key }} = {{ $value | quote }}
 {{- end }}
 
 [Metadata]
 {{- range $key, $value := .Values.cloudConfig.metadata }}
-{{ $key }} = {{ $value }}
+{{ $key }} = {{ $value | quote }}
 {{- end }}
 {{- end }}
 

charts/openstack-cloud-controller-manager/templates/daemonset.yaml

@@ -2,6 +2,7 @@ apiVersion: apps/v1
 kind: DaemonSet
 metadata:
   name: {{ include "occm.name" . }}
+  namespace: {{ .Release.Namespace }}
   labels:
     {{- include "occm.labels" . | nindent 4 }}
 spec:

charts/openstack-cloud-controller-manager/templates/secret.yaml

@@ -3,6 +3,7 @@ apiVersion: v1
 kind: Secret
 metadata:
  name: {{ .Values.secret.name | default "cloud-config" }}
+ namespace: {{ .Release.Namespace }}
 type: Opaque
 data:
  {{ if .Values.cloudConfigContents -}}

charts/openstack-cloud-controller-manager/templates/service-sm.yaml

@@ -5,6 +5,7 @@ metadata:
   labels:
     {{- include "occm.labels" . | nindent 4 }}
   name: {{ include "occm.name" . }}
+  namespace: {{ .Release.Namespace }}
 spec:
   ports:
   - name: http

charts/openstack-cloud-controller-manager/templates/serviceaccount.yaml

@@ -2,3 +2,4 @@ apiVersion: v1
 kind: ServiceAccount
 metadata:
   name: openstack-cloud-controller-manager
+  namespace: {{ .Release.Namespace }}

charts/openstack-cloud-controller-manager/templates/servicemonitor.yaml

@@ -5,6 +5,7 @@ metadata:
   labels:
     {{- include "occm.labels" . | nindent 4 }}
   name: {{ include "occm.name" . }}
+  namespace: {{ .Release.Namespace }}
 spec:
   endpoints:
   - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token

docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md

@@ -114,6 +114,10 @@ Request Body:
 
   VIP subnet ID of load balancer created.
 
+- `loadbalancer.openstack.org/member-subnet-id`
+
+  Member subnet ID of the load balancer created.
+
 - `loadbalancer.openstack.org/network-id`
 
   The network ID which will allocate virtual IP for loadbalancer.

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

@@ -200,7 +200,10 @@ Although the openstack-cloud-controller-manager was initially implemented with N
   Optional. If specified, only "v2" is supported.
 
 * `subnet-id`
-  ID of the Neutron subnet on which to create load balancer VIP, this ID is also used to create pool members.
+  ID of the Neutron subnet on which to create load balancer VIP. This ID is also used to create pool members, if `member-subnet-id` is not set.
+
+* `member-subnet-id`
+  ID of the Neutron network on which to create the members of the load balancer. The load balancer gets another network port on this subnet. Defaults to `subnet-id` if not set.
 
 * `network-id`
   ID of the Neutron network on which to create load balancer VIP, not needed if `subnet-id` is set.
@@ -243,6 +246,7 @@ Although the openstack-cloud-controller-manager was initially implemented with N
   * floating-subnet-tags. The same with `floating-subnet-tags` option above.
   * network-id. The same with `network-id` option above.
   * subnet-id. The same with `subnet-id` option above.
+  * member-subnet-id. The same with `member-subnet-id` option above.
 
 * `enable-ingress-hostname`
 
@@ -285,6 +289,10 @@ NOTE:
 
   Not all OpenStack clouds provide both configuration drive and metadata service though and only one or the other may be available which is why the default is to check both. Especially, the metadata on the config drive may grow stale over time, whereas the metadata service always provides the most up to date data.
 
+### Multi region support (alpha)
+
+* environment variable `OS_CCM_REGIONAL` is set to `true` - allow CCM to set ProviderID with region name `${ProviderName}://${REGION}/${instance-id}`. Default: false.
+
 ## Exposing applications using services of LoadBalancer type
 
 Refer to [Exposing applications using services of LoadBalancer type](./expose-applications-using-loadbalancer-type-service.md)

pkg/ingress/utils/utils.go

@@ -74,7 +74,7 @@ func stringSlicesEqual(x, y []string) bool {
 
 // GetNodeID get instance ID from the Node spec.
 func GetNodeID(node *apiv1.Node) (string, error) {
-	var providerIDRegexp = regexp.MustCompile(`^openstack:///([^/]+)$`)
+	var providerIDRegexp = regexp.MustCompile(`^openstack://(?:[^/]*)/([^/]+)$`)
 
 	matches := providerIDRegexp.FindStringSubmatch(node.Spec.ProviderID)
 	if len(matches) != 2 {

pkg/openstack/instances.go

@@ -20,7 +20,7 @@ import (
 	"context"
 	"fmt"
 	"net"
-	"os"
+	sysos "os"
 	"regexp"
 	"sort"
 	"strings"
@@ -46,13 +46,16 @@ import (
 
 // Instances encapsulates an implementation of Instances for OpenStack.
 type Instances struct {
-	compute        *gophercloud.ServiceClient
-	opts           metadata.Opts
-	networkingOpts NetworkingOpts
+	compute          *gophercloud.ServiceClient
+	region           string
+	regionProviderID bool
+	opts             metadata.Opts
+	networkingOpts   NetworkingOpts
 }
 
 const (
-	instanceShutoff = "SHUTOFF"
+	instanceShutoff       = "SHUTOFF"
+	RegionalProviderIDEnv = "OS_CCM_REGIONAL"
 )
 
 var _ cloudprovider.Instances = &Instances{}
@@ -77,10 +80,17 @@ func (os *OpenStack) instances() (*Instances, bool) {
 		return nil, false
 	}
 
+	regionalProviderID := false
+	if isRegionalProviderID := sysos.Getenv(RegionalProviderIDEnv); isRegionalProviderID == "true" {
+		regionalProviderID = true
+	}
+
 	return &Instances{
-		compute:        compute,
-		opts:           os.metadataOpts,
-		networkingOpts: os.networkingOpts,
+		compute:          compute,
+		region:           os.epOpts.Region,
+		regionProviderID: regionalProviderID,
+		opts:             os.metadataOpts,
+		networkingOpts:   os.networkingOpts,
 	}, true
 }
 
@@ -128,14 +138,19 @@ func (i *Instances) NodeAddresses(ctx context.Context, name types.NodeName) ([]v
 // This method will not be called from the node that is requesting this ID. i.e. metadata service
 // and other local methods cannot be used here
 func (i *Instances) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]v1.NodeAddress, error) {
-	klog.V(4).Infof("NodeAddressesByProviderID (%v) called", providerID)
-
-	instanceID, err := instanceIDFromProviderID(providerID)
+	klog.V(4).Infof("NodeAddressesByProviderID(%v) called", providerID)
 
+	instanceID, instanceRegion, err := instanceIDFromProviderID(providerID)
 	if err != nil {
 		return []v1.NodeAddress{}, err
 	}
 
+	if instanceRegion != "" && instanceRegion != i.region {
+		klog.V(4).Infof("NodeAddressesByProviderID(%v) has foreign region %v, skipped", providerID, instanceRegion)
+
+		return []v1.NodeAddress{}, nil
+	}
+
 	mc := metrics.NewMetricContext("server", "get")
 	server, err := servers.Get(i.compute, instanceID).Extract()
 
@@ -162,12 +177,18 @@ func (i *Instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, er
 	return i.InstanceExistsByProviderID(ctx, node.Spec.ProviderID)
 }
 
-func instanceExistsByProviderID(ctx context.Context, compute *gophercloud.ServiceClient, providerID string) (bool, error) {
-	instanceID, err := instanceIDFromProviderID(providerID)
+func instanceExistsByProviderID(ctx context.Context, compute *gophercloud.ServiceClient, providerID string, region string) (bool, error) {
+	instanceID, instanceRegion, err := instanceIDFromProviderID(providerID)
 	if err != nil {
 		return false, err
 	}
 
+	if instanceRegion != "" && instanceRegion != region {
+		klog.V(4).Infof("instanceExistsByProviderID(%v) has foreign region %v, skipped", providerID, instanceRegion)
+
+		return true, nil
+	}
+
 	mc := metrics.NewMetricContext("server", "get")
 	_, err = servers.Get(compute, instanceID).Extract()
 	if mc.ObserveRequest(err) != nil {
@@ -183,7 +204,7 @@ func instanceExistsByProviderID(ctx context.Context, compute *gophercloud.Servic
 // InstanceExistsByProviderID returns true if the instance with the given provider id still exists.
 // If false is returned with no error, the instance will be immediately deleted by the cloud controller manager.
 func (i *Instances) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) {
-	return instanceExistsByProviderID(ctx, i.compute, providerID)
+	return instanceExistsByProviderID(ctx, i.compute, providerID, i.region)
 }
 
 // InstanceShutdown returns true if the instances is in safe state to detach volumes.
@@ -192,12 +213,16 @@ func (i *Instances) InstanceShutdown(ctx context.Context, node *v1.Node) (bool,
 	return i.InstanceShutdownByProviderID(ctx, node.Spec.ProviderID)
 }
 
-func instanceShutdownByProviderID(ctx context.Context, compute *gophercloud.ServiceClient, providerID string) (bool, error) {
-	instanceID, err := instanceIDFromProviderID(providerID)
+func instanceShutdownByProviderID(ctx context.Context, compute *gophercloud.ServiceClient, providerID string, region string) (bool, error) {
+	instanceID, instanceRegion, err := instanceIDFromProviderID(providerID)
 	if err != nil {
 		return false, err
 	}
 
+	if instanceRegion != "" && instanceRegion != region {
+		return false, fmt.Errorf("ProviderID \"%s\" didn't match supported region \"%s\"", providerID, region)
+	}
+
 	mc := metrics.NewMetricContext("server", "get")
 	server, err := servers.Get(compute, instanceID).Extract()
 	if mc.ObserveRequest(err) != nil {
@@ -214,16 +239,20 @@ func instanceShutdownByProviderID(ctx context.Context, compute *gophercloud.Serv
 // InstanceShutdownByProviderID returns true if the instances is in safe state to detach volumes.
 // It is the only state, where volumes can be detached immediately.
 func (i *Instances) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) {
-	return instanceShutdownByProviderID(ctx, i.compute, providerID)
+	return instanceShutdownByProviderID(ctx, i.compute, providerID, i.region)
 }
 
 // InstanceMetadata returns metadata of the specified instance.
 func (i *Instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
-	instanceID, err := instanceIDFromProviderID(node.Spec.ProviderID)
+	instanceID, instanceRegion, err := instanceIDFromProviderID(node.Spec.ProviderID)
 	if err != nil {
 		return nil, err
 	}
 
+	if instanceRegion != "" && instanceRegion != i.region {
+		return nil, fmt.Errorf("ProviderID \"%s\" didn't match supported region \"%s\"", node.Spec.ProviderID, i.region)
+	}
+
 	mc := metrics.NewMetricContext("server", "get")
 	srv, err := servers.Get(i.compute, instanceID).Extract()
 	if mc.ObserveRequest(err) != nil {
@@ -260,6 +289,11 @@ func (i *Instances) InstanceID(ctx context.Context, name types.NodeName) (string
 		}
 		return "", err
 	}
+
+	if i.regionProviderID {
+		return i.region + "/" + srv.ID, nil
+	}
+
 	// In the future it is possible to also return an endpoint as:
 	// <endpoint>/<instanceid>
 	return "/" + srv.ID, nil
@@ -269,12 +303,15 @@ func (i *Instances) InstanceID(ctx context.Context, name types.NodeName) (string
 // This method will not be called from the node that is requesting this ID. i.e. metadata service
 // and other local methods cannot be used here
 func (i *Instances) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) {
-	instanceID, err := instanceIDFromProviderID(providerID)
-
+	instanceID, instanceRegion, err := instanceIDFromProviderID(providerID)
 	if err != nil {
 		return "", err
 	}
 
+	if instanceRegion != "" && instanceRegion != i.region {
+		return "", fmt.Errorf("ProviderID \"%s\" didn't match supported region \"%s\"", providerID, i.region)
+	}
+
 	mc := metrics.NewMetricContext("server", "get")
 	server, err := servers.Get(i.compute, instanceID).Extract()
 
@@ -335,23 +372,24 @@ func isValidLabelValue(v string) bool {
 }
 
 // If Instances.InstanceID or cloudprovider.GetInstanceProviderID is changed, the regexp should be changed too.
-var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `:///([^/]+)$`)
+var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `://([^/]*)/([^/]+)$`)
 
 // instanceIDFromProviderID splits a provider's id and return instanceID.
-// A providerID is build out of '${ProviderName}:///${instance-id}'which contains ':///'.
+// A providerID is build out of '${ProviderName}:///${instance-id}' which contains ':///'.
+// or '${ProviderName}://${region}/${instance-id}' which contains '://'.
 // See cloudprovider.GetInstanceProviderID and Instances.InstanceID.
-func instanceIDFromProviderID(providerID string) (instanceID string, err error) {
+func instanceIDFromProviderID(providerID string) (instanceID string, region string, err error) {
 
 	// https://github.com/kubernetes/kubernetes/issues/85731
 	if providerID != "" && !strings.Contains(providerID, "://") {
 		providerID = ProviderName + "://" + providerID
 	}
 
 	matches := providerIDRegexp.FindStringSubmatch(providerID)
-	if len(matches) != 2 {
-		return "", fmt.Errorf("ProviderID \"%s\" didn't match expected format \"openstack:///InstanceID\"", providerID)
+	if len(matches) != 3 {
+		return "", "", fmt.Errorf("ProviderID \"%s\" didn't match expected format \"openstack://region/InstanceID\"", providerID)
 	}
-	return matches[1], nil
+	return matches[2], matches[1], nil
 }
 
 // AddToNodeAddresses appends the NodeAddresses to the passed-by-pointer slice,
@@ -414,7 +452,7 @@ func readInstanceID(searchOrder string) (string, error) {
 
 	// Try to find instance ID on the local filesystem (created by cloud-init)
 	const instanceIDFile = "/var/lib/cloud/data/instance-id"
-	idBytes, err := os.ReadFile(instanceIDFile)
+	idBytes, err := sysos.ReadFile(instanceIDFile)
 	if err == nil {
 		instanceID := string(idBytes)
 		instanceID = strings.TrimSpace(instanceID)