test(e2e): cover region in providerID clusterv1 machine object

MatthieuFin · MatthieuFin · commit b5fa8d38b8fc · 2024-10-29T17:52:17.000+01:00
Signed-off-by: MatthieuFin &lt;matthieu2717@gmail.com&gt;
diff --git a/Makefile b/Makefile
@@ -176,7 +176,9 @@ e2e-templates: $(addprefix $(E2E_NO_ARTIFACT_TEMPLATES_DIR)/, \
 		 cluster-template-flatcar.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) \
 
diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go
@@ -391,6 +391,9 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 		Address: instanceStatus.Name(),
 	})
 	openStackMachine.Status.Addresses = addresses
+	if openStackMachine.Spec.IdentityRef == nil {
+		openStackMachine.Spec.IdentityRef = &openStackCluster.Spec.IdentityRef
+	}
 
 	result := r.reconcileMachineState(scope, openStackMachine, machine, machineServer)
 	if result != nil {
diff --git a/test/e2e/data/kustomize/providerID-with-region-default/kustomization.yaml b/test/e2e/data/kustomize/providerID-with-region-default/kustomization.yaml
@@ -0,0 +1,18 @@
+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
+#- path: patch-openstack-machine-template-identityRef.yaml
+#  target:
+#    kind: OpenStackMachineTemplate
diff --git a/test/e2e/data/kustomize/providerID-with-region-default/patch-kubeadm-config-template-provider-id.yaml b/test/e2e/data/kustomize/providerID-with-region-default/patch-kubeadm-config-template-provider-id.yaml
@@ -0,0 +1,5 @@
+---
+- op: replace
+  path: /spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs/provider-id
+  value: "openstack://${CAPO_REGION}/{{ instance_id }}"
+
diff --git a/test/e2e/data/kustomize/providerID-with-region-default/patch-kubeadm-control-plane-provider-id.yaml b/test/e2e/data/kustomize/providerID-with-region-default/patch-kubeadm-control-plane-provider-id.yaml
@@ -0,0 +1,9 @@
+---
+- 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 }}"
+
+
diff --git a/test/e2e/data/kustomize/providerID-with-region-default/patch-machine-template-identity-ref.yaml b/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}
diff --git a/test/e2e/data/kustomize/providerID-with-region-default/patch-openstack-machine-template-identityRef.yaml b/test/e2e/data/kustomize/providerID-with-region-default/patch-openstack-machine-template-identityRef.yaml
@@ -0,0 +1,8 @@
+---
+- op: add
+  path: /spec/template/spec/identityRef
+  value: 
+    cloudName: ${OPENSTACK_CLOUD}
+    name: ${CLUSTER_NAME}-cloud-config
+    region: ${CAPO_REGION}
+
diff --git a/test/e2e/data/kustomize/providerID-with-region-override/kustomization.yaml b/test/e2e/data/kustomize/providerID-with-region-override/kustomization.yaml
@@ -0,0 +1,21 @@
+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
+- path: patch-openstack-machine-template-identityRef.yaml
+  target:
+    kind: OpenStackMachineTemplate
+#- path: patch-no-bastion.yaml
+#  target:
+#    kind: OpenStackCluster
diff --git a/test/e2e/data/kustomize/providerID-with-region-override/patch-kubeadm-config-template-provider-id.yaml b/test/e2e/data/kustomize/providerID-with-region-override/patch-kubeadm-config-template-provider-id.yaml
@@ -0,0 +1,5 @@
+---
+- op: replace
+  path: /spec/template/spec/joinConfiguration/nodeRegistration/kubeletExtraArgs/provider-id
+  value: "openstack://${CAPO_REGION}/{{ instance_id }}"
+
diff --git a/test/e2e/data/kustomize/providerID-with-region-override/patch-kubeadm-control-plane-provider-id.yaml b/test/e2e/data/kustomize/providerID-with-region-override/patch-kubeadm-control-plane-provider-id.yaml
@@ -0,0 +1,9 @@
+---
+- 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 }}"
+
+
diff --git a/test/e2e/data/kustomize/providerID-with-region-override/patch-machine-template-identity-ref.yaml b/test/e2e/data/kustomize/providerID-with-region-override/patch-machine-template-identity-ref.yaml
@@ -0,0 +1,4 @@
+---
+- op: add
+  path: /spec/identityRef/region
+  value: ${CAPO_REGION}
diff --git a/test/e2e/data/kustomize/providerID-with-region-override/patch-no-bastion.yaml b/test/e2e/data/kustomize/providerID-with-region-override/patch-no-bastion.yaml
@@ -0,0 +1,5 @@
+---
+- op: replace
+  path: /spec/bastion/enabled
+  value: false
+
diff --git a/test/e2e/data/kustomize/providerID-with-region-override/patch-openstack-machine-template-identityRef.yaml b/test/e2e/data/kustomize/providerID-with-region-override/patch-openstack-machine-template-identityRef.yaml
@@ -0,0 +1,8 @@
+---
+- op: add
+  path: /spec/template/spec/identityRef
+  value: 
+    cloudName: ${OPENSTACK_CLOUD}
+    name: ${CLUSTER_NAME}-cloud-config
+    region: ${CAPO_REGION}
+
diff --git a/test/e2e/shared/common.go b/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)
 	}
diff --git a/test/e2e/shared/defaults.go b/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.
diff --git a/test/e2e/shared/suite.go b/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.
diff --git a/test/e2e/suites/e2e/e2e_test.go b/test/e2e/suites/e2e/e2e_test.go
@@ -59,7 +59,10 @@ import (
 	"sigs.k8s.io/cluster-api-provider-openstack/test/e2e/shared"
 )
 
-const specName = "e2e"
+const (
+	specName              = "e2e"
+	testSecurityGroupName = "testSecGroup"
+)
 
 // Additional images required for flatcar tests.
 func flatcarImages(e2eCtx *shared.E2EContext) []shared.DownloadImage {
@@ -510,7 +513,6 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 
 			shared.Logf("Creating MachineDeployment with custom port options")
 			md3Name := clusterName + "-md-3"
-			testSecurityGroupName := "testSecGroup"
 			// create required test security group
 			var securityGroupCleanup func(ctx context.Context)
 			securityGroupCleanup, err = shared.CreateOpenStackSecurityGroup(ctx, e2eCtx, testSecurityGroupName, "Test security group")
@@ -679,6 +681,106 @@ var _ = Describe("e2e tests [PR-Blocking]", func() {
 		})
 	})
 
+	Describe("Workload cluster providerID identityRef.Region field per OpenStackMachineTemplate override OpenStackCluster identityRef.Region field", func() {
+		It("should create machines with identityRef.Region field which override CLuster one", func(ctx context.Context) {
+			shared.Logf("Creating a cluster")
+			clusterName := fmt.Sprintf("cluster-%s", namespace.Name)
+			configCluster := defaultConfigCluster(clusterName, namespace.Name)
+			configCluster.ControlPlaneMachineCount = ptr.To(int64(1))
+			configCluster.WorkerMachineCount = ptr.To(int64(0))
+			configCluster.Flavor = shared.FlavorProviderIDWithRegionOverride
+			createCluster(ctx, configCluster, clusterResources)
+			md := clusterResources.MachineDeployments
+
+			workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{
+				Lister:            e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
+				ClusterName:       clusterName,
+				Namespace:         namespace.Name,
+				MachineDeployment: *md[0],
+			})
+			controlPlaneMachines := framework.GetControlPlaneMachinesByCluster(ctx, framework.GetControlPlaneMachinesByClusterInput{
+				Lister:      e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
+				ClusterName: clusterName,
+				Namespace:   namespace.Name,
+			})
+			Expect(workerMachines).To(HaveLen(0))
+			Expect(controlPlaneMachines).To(HaveLen(1))
+
+			shared.Logf("Creating MachineDeployment with identityRef Region field defined")
+			// create required test security group
+			var securityGroupCleanup func(ctx context.Context)
+			securityGroupCleanup, err = shared.CreateOpenStackSecurityGroup(ctx, e2eCtx, testSecurityGroupName, "Test security group")
+			Expect(err).To(BeNil())
+			postClusterCleanup = append(postClusterCleanup, securityGroupCleanup)
+
+			_, clientOpts, _, err := shared.GetTenantProviderClient(e2eCtx)
+			Expect(err).To(BeNil(), "Cannot create providerClient")
+
+			controlPlaneMachine := controlPlaneMachines[0]
+
+			shared.Logf("Fetching serverID control plane")
+			allControlPlaneServers, err := shared.DumpOpenStackServers(e2eCtx, servers.ListOpts{Name: controlPlaneMachine.Name})
+			Expect(err).To(BeNil())
+			Expect(allControlPlaneServers).To(HaveLen(1))
+			controlPlaneServerID := allControlPlaneServers[0].ID
+			Expect(err).To(BeNil())
+
+			Expect(*(controlPlaneMachine.Spec.ProviderID)).To(
+				Equal(fmt.Sprintf("openstack://%s/%s", clientOpts.RegionName, controlPlaneServerID)),
+				fmt.Sprintf("ControlPlane machine should have providerID string `openstack://%s/uuid`", clientOpts.RegionName))
+		})
+	})
+
+	Describe("Workload cluster providerID identityRef.Region field inherited from OpenStackCluster identityRef.Region field", func() {
+		It("should create machines without identityRef.Region field and inherit from Cluster object", func(ctx context.Context) {
+			shared.Logf("Creating a cluster")
+			clusterName := fmt.Sprintf("cluster-%s", namespace.Name)
+			configCluster := defaultConfigCluster(clusterName, namespace.Name)
+			configCluster.ControlPlaneMachineCount = ptr.To(int64(1))
+			configCluster.WorkerMachineCount = ptr.To(int64(0))
+			configCluster.Flavor = shared.FlavorProviderIDWithRegionDefault
+			createCluster(ctx, configCluster, clusterResources)
+			md := clusterResources.MachineDeployments
+
+			workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{
+				Lister:            e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
+				ClusterName:       clusterName,
+				Namespace:         namespace.Name,
+				MachineDeployment: *md[0],
+			})
+			controlPlaneMachines := framework.GetControlPlaneMachinesByCluster(ctx, framework.GetControlPlaneMachinesByClusterInput{
+				Lister:      e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
+				ClusterName: clusterName,
+				Namespace:   namespace.Name,
+			})
+			Expect(workerMachines).To(HaveLen(0))
+			Expect(controlPlaneMachines).To(HaveLen(1))
+
+			shared.Logf("Creating MachineDeployment with identityRef Region field defined")
+			// create required test security group
+			var securityGroupCleanup func(ctx context.Context)
+			securityGroupCleanup, err = shared.CreateOpenStackSecurityGroup(ctx, e2eCtx, testSecurityGroupName, "Test security group")
+			Expect(err).To(BeNil())
+			postClusterCleanup = append(postClusterCleanup, securityGroupCleanup)
+
+			_, clientOpts, _, err := shared.GetTenantProviderClient(e2eCtx)
+			Expect(err).To(BeNil(), "Cannot create providerClient")
+
+			controlPlaneMachine := controlPlaneMachines[0]
+
+			shared.Logf("Fetching serverID control plane")
+			allControlPlaneServers, err := shared.DumpOpenStackServers(e2eCtx, servers.ListOpts{Name: controlPlaneMachine.Name})
+			Expect(err).To(BeNil())
+			Expect(allControlPlaneServers).To(HaveLen(1))
+			controlPlaneServerID := allControlPlaneServers[0].ID
+			Expect(err).To(BeNil())
+
+			Expect(*(controlPlaneMachine.Spec.ProviderID)).To(
+				Equal(fmt.Sprintf("openstack://%s/%s", clientOpts.RegionName, controlPlaneServerID)),
+				fmt.Sprintf("ControlPlane machine should have providerID string `openstack://%s/uuid`", clientOpts.RegionName))
+		})
+	})
+
 	Describe("Workload cluster (multiple attached networks)", func() {
 		var (
 			clusterName   string
@@ -1035,9 +1137,7 @@ func getInstanceIDForMachine(machine *clusterv1.Machine) string {
 	providerID := machine.Spec.ProviderID
 	Expect(providerID).NotTo(BeNil())
 
-	providerIDSplit := strings.SplitN(*providerID, ":///", 2)
-	Expect(providerIDSplit[0]).To(Equal("openstack"))
-	return providerIDSplit[1]
+	return shared.GetIDFromProviderID(*machine.Spec.ProviderID)
 }
 
 func isErrorEventExists(namespace, machineDeploymentName, eventReason, errorMsg string, eList *corev1.EventList) bool {

Original file line number	Diff line number	Diff line change
`@@ -174,9 +174,13 @@ func getOpenStackClusterFromMachine(ctx context.Context, client client.Client, m`
`174`	`174`	`return openStackCluster, err`
`175`	`175`	`}`
`176`	`176`
`177`		`-// getIDFromProviderID returns the server ID part of a provider ID string.`
`178`		`-func getIDFromProviderID(providerID string) string {`
`179`		`- return strings.TrimPrefix(providerID, "openstack:///")`
	`177`	`+// GetIDFromProviderID returns the server ID part of a provider ID string.`
	`178`	`+func GetIDFromProviderID(providerID string) string {`
	`179`	`+ providerIDSplit := strings.SplitN(providerID, "://", 2)`
	`180`	`+ Expect(providerIDSplit[0]).To(Equal("openstack"))`
	`181`	`+ providerIDPathSplit := strings.SplitN(providerIDSplit[1], "/", 2)`
	`182`	`+ // providerIDPathSplit[0] contain region name, could be empty`
	`183`	`+ return providerIDPathSplit[1]`
`180`	`184`	`}`
`181`	`185`
`182`	`186`	`type OpenStackLogCollector struct {`
`@@ -201,7 +205,7 @@ func (o OpenStackLogCollector) CollectMachineLog(ctx context.Context, management`
`201`	`205`	`}`
`202`	`206`	`ip := m.Status.Addresses[0].Address`
`203`	`207`
`204`		`- srv, err := GetOpenStackServerWithIP(o.E2EContext, getIDFromProviderID(*m.Spec.ProviderID), openStackCluster)`
	`208`	`+ srv, err := GetOpenStackServerWithIP(o.E2EContext, GetIDFromProviderID(*m.Spec.ProviderID), openStackCluster)`
`205`	`209`	`if err != nil {`
`206`	`210`	`return fmt.Errorf("error getting OpenStack server: %w", err)`
`207`	`211`	`}`