Merge pull request #732 from jpodivin/boolpointerswitch

Switching deployOnAllNodeSets from *bool to bool
openstack-k8s-operators · Mar 6, 2024 · 203540b · 203540b
2 parents 7298157 + cf9b522
commit 203540b
Show file tree

Hide file tree

Showing 8 changed files with 11 additions and 16 deletions.
diff --git a/api/v1beta1/openstackdataplaneservice_types.go b/api/v1beta1/openstackdataplaneservice_types.go
@@ -78,7 +78,7 @@ type OpenStackDataPlaneServiceSpec struct {
 	// DeployOnAllNodeSets - should the service be deploy across all nodesets
 	// This will override default target of a service play, setting it to 'all'.
 	// +kubebuilder:validation:Optional
-	DeployOnAllNodeSets *bool `json:"deployOnAllNodeSets,omitempty" yaml:"deployOnAllNodeSets,omitempty"`
+	DeployOnAllNodeSets bool `json:"deployOnAllNodeSets,omitempty" yaml:"deployOnAllNodeSets,omitempty"`
 }
 
 // OpenStackDataPlaneServiceStatus defines the observed state of OpenStackDataPlaneService

diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/docs/assemblies/custom_resources.adoc b/docs/assemblies/custom_resources.adoc
@@ -300,7 +300,7 @@ OpenStackDataPlaneServiceSpec defines the desired state of OpenStackDataPlaneSer
 
 | deployOnAllNodeSets
 | DeployOnAllNodeSets - should the service be deploy across all nodesets This will override default target of a service play, setting it to 'all'.
-| *bool
+| bool
 | false
 |===
 

diff --git a/pkg/deployment/service.go b/pkg/deployment/service.go
@@ -170,7 +170,7 @@ func CheckGlobalServiceExecutionConsistency(ctx context.Context, helper *helper.
 			return err
 		}
 
-		if service.Spec.DeployOnAllNodeSets != nil && *service.Spec.DeployOnAllNodeSets {
+		if service.Spec.DeployOnAllNodeSets {
 			if serviceInList(service.Name, globalServices) {
 				return fmt.Errorf("global service %s defined multiple times", service.Name)
 			}

diff --git a/pkg/util/ansible_execution.go b/pkg/util/ansible_execution.go
@@ -114,7 +114,7 @@ func AnsibleExecution(
 		if ansibleEE.Spec.ExtraVars == nil {
 			ansibleEE.Spec.ExtraVars = make(map[string]json.RawMessage)
 		}
-		if service.Spec.DeployOnAllNodeSets != nil && *service.Spec.DeployOnAllNodeSets {
+		if service.Spec.DeployOnAllNodeSets {
 			ansibleEE.Spec.ExtraVars["edpm_override_hosts"] = json.RawMessage([]byte("\"all\""))
 			util.LogForObject(helper, fmt.Sprintf("for service %s, substituting existing ansible play host with 'all'.", service.Name), ansibleEE)
 		} else {
@@ -123,7 +123,7 @@ func AnsibleExecution(
 		}
 
 		for sshKeyNodeName, sshKeySecret := range sshKeySecrets {
-			if service.Spec.DeployOnAllNodeSets != nil && *service.Spec.DeployOnAllNodeSets {
+			if service.Spec.DeployOnAllNodeSets {
 				sshKeyName = fmt.Sprintf("ssh-key-%s", sshKeyNodeName)
 				sshKeyMountSubPath = fmt.Sprintf("ssh_key_%s", targetNodeset)
 				sshKeyMountPath = fmt.Sprintf("/runner/env/ssh_key/%s", sshKeyMountSubPath)
@@ -158,7 +158,7 @@ func AnsibleExecution(
 
 		// Mounting inventory and secrets
 		for inventoryIndex, inventorySecret := range inventorySecrets {
-			if service.Spec.DeployOnAllNodeSets != nil && *service.Spec.DeployOnAllNodeSets {
+			if service.Spec.DeployOnAllNodeSets {
 				inventoryName = fmt.Sprintf("inventory-%d", inventoryIndex)
 				inventoryMountPath = fmt.Sprintf("/runner/inventory/%s", inventoryName)
 			} else {

diff --git a/tests/functional/openstackdataplanedeployment_controller_test.go b/tests/functional/openstackdataplanedeployment_controller_test.go
@@ -186,7 +186,7 @@ var _ = Describe("Dataplane Deployment Test", func() {
 
 					g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed())
 					g.Expect(ansibleEE.Spec.ExtraVars).To(HaveKey("edpm_override_hosts"))
-					if service.Spec.DeployOnAllNodeSets != nil && *service.Spec.DeployOnAllNodeSets {
+					if service.Spec.DeployOnAllNodeSets {
 						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_override_hosts"])).To(Equal("\"all\""))
 					} else {
 						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_override_hosts"])).To(Equal(fmt.Sprintf("\"%s\"", dataplaneNodeSetName.Name)))
@@ -387,7 +387,7 @@ var _ = Describe("Dataplane Deployment Test", func() {
 					g.Expect(th.K8sClient.Get(th.Ctx, ansibleeeName, ansibleEE)).To(Succeed())
 					ansibleEE.Status.JobStatus = ansibleeev1.JobStatusSucceeded
 					g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed())
-					if service.Spec.DeployOnAllNodeSets != nil && *service.Spec.DeployOnAllNodeSets {
+					if service.Spec.DeployOnAllNodeSets {
 						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_override_hosts"])).To(Equal("\"all\""))
 					}
 				}, th.Timeout, th.Interval).Should(Succeed())

diff --git a/tests/functional/openstackdataplanenodeset_controller_test.go b/tests/functional/openstackdataplanenodeset_controller_test.go
@@ -347,7 +347,7 @@ var _ = Describe("Dataplane NodeSet Test", func() {
 
 			It("Should have service called 'global-service'", func() {
 				service := GetService(dataplaneGlobalServiceName)
-				Expect(*service.Spec.DeployOnAllNodeSets).Should(BeTrue())
+				Expect(service.Spec.DeployOnAllNodeSets).Should(BeTrue())
 			})
 		})
 

diff --git a/tests/functional/service_test.go b/tests/functional/service_test.go
@@ -44,7 +44,7 @@ var _ = Describe("OpenstackDataplaneService Test", func() {
 			Expect(service.Spec.Secrets).To(BeEmpty())
 			Expect(service.Spec.Playbook).To(BeEmpty())
 			Expect(service.Spec.ConfigMaps).To(BeEmpty())
-			Expect(service.Spec.DeployOnAllNodeSets).To(BeNil())
+			Expect(service.Spec.DeployOnAllNodeSets).To(BeFalse())
 		})
 	})
 
@@ -60,7 +60,7 @@ var _ = Describe("OpenstackDataplaneService Test", func() {
 			Expect(service.Spec.Secrets).To(BeEmpty())
 			Expect(service.Spec.Playbook).To(BeEmpty())
 			Expect(service.Spec.ConfigMaps).To(BeEmpty())
-			Expect(*service.Spec.DeployOnAllNodeSets).To(BeTrue())
+			Expect(service.Spec.DeployOnAllNodeSets).To(BeTrue())
 		})
 	})
 })
-Original file line number
+Diff line change
@@ Expand Up @@
     | deployOnAllNodeSets
     | DeployOnAllNodeSets - should the service be deploy across all nodesets This will override default target of a service play, setting it to 'all'.
-    | *bool
+    | bool
     | false
     |===
@@ Expand Down @@