From 4bdd8e8fd0518228f31020cb142e50795fcbeab2 Mon Sep 17 00:00:00 2001 From: MatthieuFin Date: Mon, 28 Oct 2024 21:44:37 +0100 Subject: [PATCH 1/2] test(e2e): cover region in providerID clusterv1 machine object Signed-off-by: MatthieuFin --- Makefile | 6 +- controllers/openstackmachine_controller.go | 3 + .../kustomization.yaml | 15 +++ ...h-kubeadm-config-template-provider-id.yaml | 4 + ...tch-kubeadm-control-plane-provider-id.yaml | 7 ++ .../patch-machine-template-identity-ref.yaml | 4 + .../kustomization.yaml | 15 +++ ...h-kubeadm-config-template-provider-id.yaml | 4 + ...tch-kubeadm-control-plane-provider-id.yaml | 7 ++ ...penstack-machine-template-identityRef.yaml | 7 ++ test/e2e/shared/common.go | 12 +- test/e2e/shared/defaults.go | 56 ++++----- test/e2e/shared/suite.go | 4 + test/e2e/suites/e2e/e2e_test.go | 110 +++++++++++++++++- 14 files changed, 216 insertions(+), 38 deletions(-) create mode 100644 test/e2e/data/kustomize/providerID-with-region-default/kustomization.yaml create mode 100644 test/e2e/data/kustomize/providerID-with-region-default/patch-kubeadm-config-template-provider-id.yaml create mode 100644 test/e2e/data/kustomize/providerID-with-region-default/patch-kubeadm-control-plane-provider-id.yaml create mode 100644 test/e2e/data/kustomize/providerID-with-region-default/patch-machine-template-identity-ref.yaml create mode 100644 test/e2e/data/kustomize/providerID-with-region-override/kustomization.yaml create mode 100644 test/e2e/data/kustomize/providerID-with-region-override/patch-kubeadm-config-template-provider-id.yaml create mode 100644 test/e2e/data/kustomize/providerID-with-region-override/patch-kubeadm-control-plane-provider-id.yaml create mode 100644 test/e2e/data/kustomize/providerID-with-region-override/patch-openstack-machine-template-identityRef.yaml diff --git a/Makefile b/Makefile index 3690fe8999..ae1f8e456d 100644 --- a/Makefile +++ b/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) \ diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index a07b5144b0..10a8a92d19 100644 --- 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 new file mode 100644 index 0000000000..9aa8f07292 --- /dev/null +++ b/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 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 new file mode 100644 index 0000000000..d9e7f06cae --- /dev/null +++ b/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 }}" 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 new file mode 100644 index 0000000000..21d5e611bd --- /dev/null +++ b/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 }}" 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 new file mode 100644 index 0000000000..5e19892bab --- /dev/null +++ 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-override/kustomization.yaml b/test/e2e/data/kustomize/providerID-with-region-override/kustomization.yaml new file mode 100644 index 0000000000..bd643078cf --- /dev/null +++ b/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 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 new file mode 100644 index 0000000000..d9e7f06cae --- /dev/null +++ b/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 }}" 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 new file mode 100644 index 0000000000..21d5e611bd --- /dev/null +++ b/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 }}" 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 new file mode 100644 index 0000000000..2298518467 --- /dev/null +++ b/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} diff --git a/test/e2e/shared/common.go b/test/e2e/shared/common.go index 8bef9e9964..68f266cb89 100644 --- 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 index 0710302184..0bd3b08734 100644 --- 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 index a3edc638a1..07a5ad6112 100644 --- 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 index cb55b1b028..cb860044b4 100644 --- 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 { From e1edef952f1907a756fd743519c4e0fe69523a7e Mon Sep 17 00:00:00 2001 From: MatthieuFin Date: Wed, 30 Oct 2024 22:42:47 +0100 Subject: [PATCH 2/2] refactor(scope): GetIdentityRefFromObjects factorize identityRef lookup from multiple object to implement logic of identityRef's inheritage in one place (scope factory) Signed-off-by: MatthieuFin --- controllers/openstackmachine_controller.go | 18 ++++++++---------- pkg/scope/mock.go | 1 + pkg/scope/provider.go | 11 ++--------- pkg/scope/scope.go | 15 +++++++++++++++ 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index 10a8a92d19..91fd4062b6 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -391,9 +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 - } + + _, identityRef := r.ScopeFactory.GetIdentityRefFromObjects(openStackMachine, openStackCluster) + openStackMachine.Spec.IdentityRef = identityRef result := r.reconcileMachineState(scope, openStackMachine, machine, machineServer) if result != nil { @@ -576,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{ diff --git a/pkg/scope/mock.go b/pkg/scope/mock.go index d9568d6446..235c0aa6af 100644 --- a/pkg/scope/mock.go +++ b/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 diff --git a/pkg/scope/provider.go b/pkg/scope/provider.go index d23e3323a4..9f5c102acc 100644 --- a/pkg/scope/provider.go +++ b/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") diff --git a/pkg/scope/scope.go b/pkg/scope/scope.go index 93f070fe73..8ccdada229 100644 --- a/pkg/scope/scope.go +++ b/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.