Add e2e test to cover region in providerID clusterv1 machine object

Makefile

@@ -174,9 +174,11 @@ e2e-templates: $(addprefix $(E2E_NO_ARTIFACT_TEMPLATES_DIR)/, \
 		 cluster-template-without-lb.yaml \
 		 cluster-template.yaml \
 		 cluster-template-flatcar.yaml \
-                 cluster-template-k8s-upgrade.yaml \
+		 cluster-template-k8s-upgrade.yaml \
 		 cluster-template-flatcar-sysext.yaml \
-		 cluster-template-no-bastion.yaml)
+		 cluster-template-no-bastion.yaml \
+		 cluster-template-providerID-with-region-override.yaml \
+		 cluster-template-providerID-with-region-default.yaml)
 # Currently no templates that require CI artifacts
 # $(addprefix $(E2E_TEMPLATES_DIR)/, add-templates-here.yaml) \
 

controllers/openstackmachine_controller.go

@@ -392,6 +392,9 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 	})
 	openStackMachine.Status.Addresses = addresses
 
+	_, identityRef := r.ScopeFactory.GetIdentityRefFromObjects(openStackMachine, openStackCluster)
+	openStackMachine.Spec.IdentityRef = identityRef
+
 	result := r.reconcileMachineState(scope, openStackMachine, machine, machineServer)
 	if result != nil {
 		return *result, nil
@@ -573,13 +576,11 @@ func (r *OpenStackMachineReconciler) getOrCreateMachineServer(ctx context.Contex
 	}
 	if apierrors.IsNotFound(err) {
 		// Use credentials from the machine object by default, falling back to cluster credentials.
-		identityRef := func() infrav1.OpenStackIdentityReference {
-			if openStackMachine.Spec.IdentityRef != nil {
-				return *openStackMachine.Spec.IdentityRef
-			}
-			return openStackCluster.Spec.IdentityRef
-		}()
-		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network.ID)
+		_, identityRef := r.ScopeFactory.GetIdentityRefFromObjects(openStackMachine, openStackCluster)
+		if identityRef == nil {
+			return nil, fmt.Errorf("unable to get identityRef from provided objects")
+		}
+		machineServerSpec := openStackMachineSpecToOpenStackServerSpec(&openStackMachine.Spec, *identityRef, compute.InstanceTags(&openStackMachine.Spec, openStackCluster), failureDomain, userDataRef, getManagedSecurityGroup(openStackCluster, machine), openStackCluster.Status.Network.ID)
 		machineServer = &infrav1alpha1.OpenStackServer{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels: map[string]string{

pkg/scope/mock.go

@@ -33,6 +33,7 @@ import (
 // MockScopeFactory implements both the ScopeFactory and ClientScope interfaces. It can be used in place of the default ProviderScopeFactory
 // when we want to use mocked service clients which do not attempt to connect to a running OpenStack cloud.
 type MockScopeFactory struct {
+	*defaultScopeFactory
 	ComputeClient *mock.MockComputeClient
 	NetworkClient *mock.MockNetworkClient
 	VolumeClient  *mock.MockVolumeClient

pkg/scope/provider.go

@@ -48,19 +48,12 @@ const (
 )
 
 type providerScopeFactory struct {
+	*defaultScopeFactory
 	clientCache *cache.LRUExpireCache
 }
 
 func (f *providerScopeFactory) NewClientScopeFromObject(ctx context.Context, ctrlClient client.Client, defaultCACert []byte, logger logr.Logger, objects ...infrav1.IdentityRefProvider) (Scope, error) {
-	var namespace *string
-	var identityRef *infrav1.OpenStackIdentityReference
-
-	for _, o := range objects {
-		namespace, identityRef = o.GetIdentityRef()
-		if namespace != nil || identityRef != nil {
-			break
-		}
-	}
+	namespace, identityRef := f.GetIdentityRefFromObjects(objects...)
 
 	if namespace == nil || identityRef == nil {
 		return nil, fmt.Errorf("unable to get identityRef from provided objects")

pkg/scope/scope.go

@@ -43,6 +43,21 @@ func NewFactory(maxCacheSize int) Factory {
 type Factory interface {
 	// NewClientScopeFromObject creates a new scope from the first object which returns an OpenStackIdentityRef
 	NewClientScopeFromObject(ctx context.Context, ctrlClient client.Client, defaultCACert []byte, logger logr.Logger, objects ...infrav1.IdentityRefProvider) (Scope, error)
+	GetIdentityRefFromObjects(objects ...infrav1.IdentityRefProvider) (*string, *infrav1.OpenStackIdentityReference)
+}
+
+type defaultScopeFactory struct{}
+
+func (f *defaultScopeFactory) GetIdentityRefFromObjects(objects ...infrav1.IdentityRefProvider) (*string, *infrav1.OpenStackIdentityReference) {
+	var namespace *string
+	var identityRef *infrav1.OpenStackIdentityReference
+	for _, o := range objects {
+		namespace, identityRef = o.GetIdentityRef()
+		if identityRef != nil {
+			break
+		}
+	}
+	return namespace, identityRef
 }
 
 // Scope contains arguments common to most operations.

test/e2e/data/kustomize/providerID-with-region-default/kustomization.yaml

@@ -0,0 +1,15 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+- ../default
+
+patches:
+- path: patch-machine-template-identity-ref.yaml
+  target:
+    kind: OpenStackCluster
+- path: patch-kubeadm-config-template-provider-id.yaml
+  target:
+    kind: KubeadmConfigTemplate
+- path: patch-kubeadm-control-plane-provider-id.yaml
+  target:
+    kind: KubeadmControlPlane

test/e2e/data/kustomize/providerID-with-region-default/patch-kubeadm-config-template-provider-id.yaml

@@ -0,0 +1,4 @@
+---
+- op: replace
+  path: /spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs/provider-id
+  value: "openstack://${CAPO_REGION}/{{ instance_id }}"

test/e2e/data/kustomize/providerID-with-region-default/patch-kubeadm-control-plane-provider-id.yaml

@@ -0,0 +1,7 @@
+---
+- op: replace
+  path: /spec/kubeadmConfigSpec/initConfiguration/nodeRegistration/kubeletExtraArgs/provider-id
+  value: "openstack://${CAPO_REGION}/{{ instance_id }}"
+- op: replace
+  path: /spec/kubeadmConfigSpec/joinConfiguration/nodeRegistration/kubeletExtraArgs/provider-id
+  value: "openstack://${CAPO_REGION}/{{ instance_id }}"

test/e2e/data/kustomize/providerID-with-region-default/patch-machine-template-identity-ref.yaml

@@ -0,0 +1,4 @@
+---
+- op: add
+  path: /spec/identityRef/region
+  value: ${CAPO_REGION}

test/e2e/data/kustomize/providerID-with-region-override/kustomization.yaml

@@ -0,0 +1,15 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+resources:
+- ../default
+
+patches:
+- path: patch-kubeadm-config-template-provider-id.yaml
+  target:
+    kind: KubeadmConfigTemplate
+- path: patch-kubeadm-control-plane-provider-id.yaml
+  target:
+    kind: KubeadmControlPlane
+- path: patch-openstack-machine-template-identityRef.yaml
+  target:
+    kind: OpenStackMachineTemplate

test/e2e/data/kustomize/providerID-with-region-override/patch-kubeadm-config-template-provider-id.yaml

@@ -0,0 +1,4 @@
+---
+- op: replace
+  path: /spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs/provider-id
+  value: "openstack://${CAPO_REGION}/{{ instance_id }}"

test/e2e/data/kustomize/providerID-with-region-override/patch-kubeadm-control-plane-provider-id.yaml

@@ -0,0 +1,7 @@
+---
+- op: replace
+  path: /spec/kubeadmConfigSpec/initConfiguration/nodeRegistration/kubeletExtraArgs/provider-id
+  value: "openstack://${CAPO_REGION}/{{ instance_id }}"
+- op: replace
+  path: /spec/kubeadmConfigSpec/joinConfiguration/nodeRegistration/kubeletExtraArgs/provider-id
+  value: "openstack://${CAPO_REGION}/{{ instance_id }}"

test/e2e/data/kustomize/providerID-with-region-override/patch-openstack-machine-template-identityRef.yaml

@@ -0,0 +1,7 @@
+---
+- op: add
+  path: /spec/template/spec/identityRef
+  value: 
+    cloudName: ${OPENSTACK_CLOUD}
+    name: ${CLUSTER_NAME}-cloud-config
+    region: ${CAPO_REGION}

test/e2e/shared/common.go

@@ -174,9 +174,13 @@ func getOpenStackClusterFromMachine(ctx context.Context, client client.Client, m
 	return openStackCluster, err
 }
 
-// getIDFromProviderID returns the server ID part of a provider ID string.
-func getIDFromProviderID(providerID string) string {
-	return strings.TrimPrefix(providerID, "openstack:///")
+// GetIDFromProviderID returns the server ID part of a provider ID string.
+func GetIDFromProviderID(providerID string) string {
+	providerIDSplit := strings.SplitN(providerID, "://", 2)
+	Expect(providerIDSplit[0]).To(Equal("openstack"))
+	providerIDPathSplit := strings.SplitN(providerIDSplit[1], "/", 2)
+	// providerIDPathSplit[0] contain region name, could be empty
+	return providerIDPathSplit[1]
 }
 
 type OpenStackLogCollector struct {
@@ -201,7 +205,7 @@ func (o OpenStackLogCollector) CollectMachineLog(ctx context.Context, management
 	}
 	ip := m.Status.Addresses[0].Address
 
-	srv, err := GetOpenStackServerWithIP(o.E2EContext, getIDFromProviderID(*m.Spec.ProviderID), openStackCluster)
+	srv, err := GetOpenStackServerWithIP(o.E2EContext, GetIDFromProviderID(*m.Spec.ProviderID), openStackCluster)
 	if err != nil {
 		return fmt.Errorf("error getting OpenStack server: %w", err)
 	}

test/e2e/shared/defaults.go

@@ -34,33 +34,35 @@ import (
 )
 
 const (
-	DefaultSSHKeyPairName      = "cluster-api-provider-openstack-sigs-k8s-io"
-	KubeContext                = "KUBE_CONTEXT"
-	KubernetesVersion          = "KUBERNETES_VERSION"
-	CCMPath                    = "CCM"
-	CCMResources               = "CCM_RESOURCES"
-	OpenStackBastionFlavorAlt  = "OPENSTACK_BASTION_MACHINE_FLAVOR_ALT"
-	OpenStackCloudYAMLFile     = "OPENSTACK_CLOUD_YAML_FILE"
-	OpenStackCloud             = "OPENSTACK_CLOUD"
-	OpenStackCloudCACertB64    = "OPENSTACK_CLOUD_CACERT_B64"
-	OpenStackCloudAdmin        = "OPENSTACK_CLOUD_ADMIN"
-	OpenStackFailureDomain     = "OPENSTACK_FAILURE_DOMAIN" //nolint:gosec // Linter thinks this could be credentials...
-	OpenStackFailureDomainAlt  = "OPENSTACK_FAILURE_DOMAIN_ALT"
-	OpenStackVolumeTypeAlt     = "OPENSTACK_VOLUME_TYPE_ALT"
-	OpenStackImageName         = "OPENSTACK_IMAGE_NAME"
-	OpenStackNodeMachineFlavor = "OPENSTACK_NODE_MACHINE_FLAVOR"
-	SSHUserMachine             = "SSH_USER_MACHINE"
-	FlavorDefault              = ""
-	FlavorNoBastion            = "no-bastion"
-	FlavorWithoutLB            = "without-lb"
-	FlavorMultiNetwork         = "multi-network"
-	FlavorMultiAZ              = "multi-az"
-	FlavorV1alpha7             = "v1alpha7"
-	FlavorMDRemediation        = "md-remediation"
-	FlavorKCPRemediation       = "kcp-remediation"
-	FlavorFlatcar              = "flatcar"
-	FlavorKubernetesUpgrade    = "k8s-upgrade"
-	FlavorFlatcarSysext        = "flatcar-sysext"
+	DefaultSSHKeyPairName              = "cluster-api-provider-openstack-sigs-k8s-io"
+	KubeContext                        = "KUBE_CONTEXT"
+	KubernetesVersion                  = "KUBERNETES_VERSION"
+	CCMPath                            = "CCM"
+	CCMResources                       = "CCM_RESOURCES"
+	OpenStackBastionFlavorAlt          = "OPENSTACK_BASTION_MACHINE_FLAVOR_ALT"
+	OpenStackCloudYAMLFile             = "OPENSTACK_CLOUD_YAML_FILE"
+	OpenStackCloud                     = "OPENSTACK_CLOUD"
+	OpenStackCloudCACertB64            = "OPENSTACK_CLOUD_CACERT_B64"
+	OpenStackCloudAdmin                = "OPENSTACK_CLOUD_ADMIN"
+	OpenStackFailureDomain             = "OPENSTACK_FAILURE_DOMAIN" //nolint:gosec // Linter thinks this could be credentials...
+	OpenStackFailureDomainAlt          = "OPENSTACK_FAILURE_DOMAIN_ALT"
+	OpenStackVolumeTypeAlt             = "OPENSTACK_VOLUME_TYPE_ALT"
+	OpenStackImageName                 = "OPENSTACK_IMAGE_NAME"
+	OpenStackNodeMachineFlavor         = "OPENSTACK_NODE_MACHINE_FLAVOR"
+	SSHUserMachine                     = "SSH_USER_MACHINE"
+	FlavorDefault                      = ""
+	FlavorNoBastion                    = "no-bastion"
+	FlavorWithoutLB                    = "without-lb"
+	FlavorProviderIDWithRegionOverride = "providerID-with-region-override"
+	FlavorProviderIDWithRegionDefault  = "providerID-with-region-default"
+	FlavorMultiNetwork                 = "multi-network"
+	FlavorMultiAZ                      = "multi-az"
+	FlavorV1alpha7                     = "v1alpha7"
+	FlavorMDRemediation                = "md-remediation"
+	FlavorKCPRemediation               = "kcp-remediation"
+	FlavorFlatcar                      = "flatcar"
+	FlavorKubernetesUpgrade            = "k8s-upgrade"
+	FlavorFlatcarSysext                = "flatcar-sysext"
 )
 
 // DefaultScheme returns the default scheme to use for testing.

test/e2e/shared/suite.go

@@ -201,6 +201,10 @@ func AllNodesBeforeSuite(e2eCtx *E2EContext, data []byte) {
 	SetEnvVar("OPENSTACK_CLOUD_YAML_B64", getEncodedOpenStackCloudYAML(openStackCloudYAMLFile), true)
 	SetEnvVar("OPENSTACK_CLOUD_PROVIDER_CONF_B64", getEncodedOpenStackCloudProviderConf(openStackCloudYAMLFile, openStackCloud), true)
 	SetEnvVar("OPENSTACK_SSH_KEY_NAME", DefaultSSHKeyPairName, false)
+	clouds := getParsedOpenStackCloudYAML(openStackCloudYAMLFile)
+	cloud, ok := clouds.Clouds[openStackCloud]
+	Expect(ok).To(BeTrue())
+	SetEnvVar("CAPO_REGION", cloud.RegionName, false)
 }
 
 // AllNodesAfterSuite is cleanup that runs on all ginkgo parallel nodes after the test suite finishes.