Add EDPMServiceType, Drop EDPMServiceName

Adds a top level EDPMServiceType field and drops EDPMServiceName. EDPMServiceType is used to construct the volume mount paths for TLS certs and CA certs, and should correspond to the ansible role name used to managing the service. When not set, a defaulting webhook will default the field to the the service name instead. EDPMRoleServiceName is kept in case there is a need to override the path for a specific TLS cert. Signed-off-by: James Slagle <[email protected]>
openstack-k8s-operators · May 28, 2024 · 84ac68b · 84ac68b
1 parent 9f05094
commit 84ac68b
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 34 deletions.
diff --git a/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml
@@ -48,7 +48,7 @@ spec:
                 type: array
               deployOnAllNodeSets:
                 type: boolean
-              edpmServiceName:
+              edpmServiceType:
                 type: string
               openStackAnsibleEERunnerImage:
                 type: string

diff --git a/api/v1beta1/openstackdataplaneservice_types.go b/api/v1beta1/openstackdataplaneservice_types.go
@@ -50,8 +50,8 @@ type OpenstackDataPlaneServiceCert struct {
 	// the edpm-ansible role where this certificate is used. For example if the
 	// certificate is for edpm_ovn from edpm-ansible, EDPMRoleServiceName must be
 	// ovn, which matches the edpm_ovn_service_name variable from the role.  If
-	// not set, OpenStackDataPlaneService.Spec.EDPMServiceName is used. If
-	// OpenStackDataPlaneService.Spec.EDPMServiceName is not set, then
+	// not set, OpenStackDataPlaneService.Spec.EDPMServiceType is used. If
+	// OpenStackDataPlaneService.Spec.EDPMServiceType is not set, then
 	// OpenStackDataPlaneService.Name is used.
 	EDPMRoleServiceName string `json:"edpmRoleServiceName,omitempty"`
 }
@@ -107,9 +107,12 @@ type OpenStackDataPlaneServiceSpec struct {
 	// github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1
 	ContainerImageFields []string `json:"containerImageFields,omitempty" yaml:"containerImageFields,omitempty"`
 
-	// EDPMServiceName - name to use for edpm_service_name ansible variable
-	// +kubebuilder:validation:Optional
-	EDPMServiceName string `json:"edpmServiceName,omitempty" yaml:"edpmServiceName,omitempty"`
+	// EDPMServiceType - service type, which typically corresponds to one of
+	// the default service names (such as nova, ovn, etc). Also typically
+	// corresponds to the ansible role name (without the "edpm_" prefix) used
+	// to manage the service. If not set, will default to the
+	// OpenStackDataPlaneService name.
+	EDPMServiceType string `json:"edpmServiceType,omitempty" yaml:"edpmServiceType,omitempty"`
 }
 
 // OpenStackDataPlaneServiceStatus defines the observed state of OpenStackDataPlaneService

diff --git a/api/v1beta1/openstackdataplaneservice_webhook.go b/api/v1beta1/openstackdataplaneservice_webhook.go
@@ -44,12 +44,14 @@ var _ webhook.Defaulter = &OpenStackDataPlaneService{}
 func (r *OpenStackDataPlaneService) Default() {
 
 	openstackdataplaneservicelog.Info("default", "name", r.Name)
-	r.Spec.Default()
+	r.Spec.Default(r.Name)
 }
 
-// Default - set defaults for this OpenStackDataPlaneDeployment
-func (spec *OpenStackDataPlaneServiceSpec) Default() {
-
+// Default - set defaults for this OpenStackDataPlaneService
+func (spec *OpenStackDataPlaneServiceSpec) Default(name string) {
+	if spec.EDPMServiceType == "" {
+		spec.EDPMServiceType = name
+	}
 }
 
 // TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.

diff --git a/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml
@@ -48,7 +48,7 @@ spec:
                 type: array
               deployOnAllNodeSets:
                 type: boolean
-              edpmServiceName:
+              edpmServiceType:
                 type: string
               openStackAnsibleEERunnerImage:
                 type: string

diff --git a/docs/assemblies/custom_resources.adoc b/docs/assemblies/custom_resources.adoc
@@ -354,8 +354,8 @@ OpenStackDataPlaneServiceSpec defines the desired state of OpenStackDataPlaneSer
 | []string
 | false
 
-| edpmServiceName
-| EDPMServiceName - name to use for edpm_service_name ansible variable
+| edpmServiceType
+| EDPMServiceType - service type, which typically corresponds to one of the default service names (such as nova, ovn, etc). Also typically corresponds to the ansible role name (without the "edpm_" prefix) used to manage the service. If not set, will default to the OpenStackDataPlaneService name.
 | string
 | false
 |===
@@ -407,7 +407,7 @@ OpenstackDataPlaneServiceCert defines the property of a TLS cert issued for a da
 | false
 
 | edpmRoleServiceName
-| EDPMRoleServiceName is the value of the +++<role>+++_service_name variable from the edpm-ansible role where this certificate is used. For example if the certificate is for edpm_ovn from edpm-ansible, EDPMRoleServiceName must be ovn, which matches the edpm_ovn_service_name variable from the role. If not set, OpenStackDataPlaneService.Spec.EDPMServiceName is used. If OpenStackDataPlaneService.Spec.EDPMServiceName is not set, then OpenStackDataPlaneService.Name is used.+++</role>+++
+| EDPMRoleServiceName is the value of the +++<role>+++_service_name variable from the edpm-ansible role where this certificate is used. For example if the certificate is for edpm_ovn from edpm-ansible, EDPMRoleServiceName must be ovn, which matches the edpm_ovn_service_name variable from the role. If not set, OpenStackDataPlaneService.Spec.EDPMServiceType is used. If OpenStackDataPlaneService.Spec.EDPMServiceType is not set, then OpenStackDataPlaneService.Name is used.+++</role>+++
 | string
 | false
 |===

diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go
@@ -280,10 +280,8 @@ func (d *Deployer) addCertMounts(
 				},
 			}
 			certMountDir := service.Spec.TLSCert.EDPMRoleServiceName
-			if certMountDir == "" && service.Spec.EDPMServiceName != "" {
-				certMountDir = service.Spec.EDPMServiceName
-			} else {
-				certMountDir = service.Name
+			if certMountDir == "" {
+				certMountDir = service.Spec.EDPMServiceType
 			}
 			certVolumeMount := corev1.VolumeMount{
 				Name:      GetServiceCertsSecretName(d.NodeSet, service.Name, 0),
@@ -315,7 +313,7 @@ func (d *Deployer) addCertMounts(
 
 			cacertVolumeMount := corev1.VolumeMount{
 				Name:      fmt.Sprintf("%s-%s", service.Name, service.Spec.CACerts),
-				MountPath: path.Join(CACertPaths, service.Name),
+				MountPath: path.Join(CACertPaths, service.Spec.EDPMServiceType),
 			}
 
 			volMounts.Volumes = append(volMounts.Volumes, cacertVolume)

diff --git a/pkg/util/ansible_execution.go b/pkg/util/ansible_execution.go
@@ -122,10 +122,10 @@ func AnsibleExecution(
 			util.LogForObject(helper,
 				fmt.Sprintf("for service %s, substituting existing ansible play host with '%s'.", service.Name, nodeSet.GetName()), ansibleEE)
 		}
-		if service.Spec.EDPMServiceName != "" {
-			ansibleEE.Spec.ExtraVars["edpm_service_name"] = json.RawMessage([]byte(fmt.Sprintf("\"%s\"", service.Spec.EDPMServiceName)))
+		if service.Spec.EDPMServiceType != "" {
+			ansibleEE.Spec.ExtraVars["edpm_service_type"] = json.RawMessage([]byte(fmt.Sprintf("\"%s\"", service.Spec.EDPMServiceType)))
 		} else {
-			ansibleEE.Spec.ExtraVars["edpm_service_name"] = json.RawMessage([]byte(fmt.Sprintf("\"%s\"", service.Name)))
+			ansibleEE.Spec.ExtraVars["edpm_service_type"] = json.RawMessage([]byte(fmt.Sprintf("\"%s\"", service.Name)))
 		}
 
 		if len(deployment.Spec.ServicesOverride) > 0 {

diff --git a/tests/functional/openstackdataplanedeployment_controller_test.go b/tests/functional/openstackdataplanedeployment_controller_test.go
@@ -126,9 +126,9 @@ var _ = Describe("Dataplane Deployment Test", func() {
 			CreateDataplaneService(dataplaneServiceName, false)
 			// marked for deployment on all nodesets
 			CreateDataplaneService(dataplaneGlobalServiceName, true)
-			// with EDPMServiceName set
+			// with EDPMServiceType set
 			CreateDataPlaneServiceFromSpec(dataplaneUpdateServiceName, map[string]interface{}{
-				"EDPMServiceName": "foo-service"})
+				"EDPMServiceType": "foo-service"})
 
 			DeferCleanup(th.DeleteService, dataplaneServiceName)
 			DeferCleanup(th.DeleteService, dataplaneGlobalServiceName)
@@ -213,10 +213,10 @@ 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.EDPMServiceName != "" {
-						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_name"])).To(Equal(fmt.Sprintf("\"%s\"", service.Spec.EDPMServiceName)))
+					if service.Spec.EDPMServiceType != "" {
+						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_type"])).To(Equal(fmt.Sprintf("\"%s\"", service.Spec.EDPMServiceType)))
 					} else {
-						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_name"])).To(Equal(fmt.Sprintf("\"%s\"", serviceName)))
+						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_type"])).To(Equal(fmt.Sprintf("\"%s\"", serviceName)))
 					}
 					if service.Spec.DeployOnAllNodeSets {
 						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_override_hosts"])).To(Equal("\"all\""))
@@ -274,7 +274,7 @@ var _ = Describe("Dataplane Deployment Test", func() {
 			CreateDataplaneService(dataplaneServiceName, false)
 			CreateDataplaneService(dataplaneGlobalServiceName, true)
 			CreateDataPlaneServiceFromSpec(dataplaneUpdateServiceName, map[string]interface{}{
-				"EDPMServiceName": "foo-service"})
+				"EDPMServiceType": "foo-service"})
 
 			DeferCleanup(th.DeleteService, dataplaneServiceName)
 			DeferCleanup(th.DeleteService, dataplaneGlobalServiceName)
@@ -428,10 +428,10 @@ var _ = Describe("Dataplane Deployment Test", func() {
 					}
 					ansibleEE.Status.JobStatus = ansibleeev1.JobStatusSucceeded
 					g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed())
-					if service.Spec.EDPMServiceName != "" {
-						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_name"])).To(Equal(fmt.Sprintf("\"%s\"", service.Spec.EDPMServiceName)))
+					if service.Spec.EDPMServiceType != "" {
+						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_type"])).To(Equal(fmt.Sprintf("\"%s\"", service.Spec.EDPMServiceType)))
 					} else {
-						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_name"])).To(Equal(fmt.Sprintf("\"%s\"", serviceName)))
+						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_type"])).To(Equal(fmt.Sprintf("\"%s\"", serviceName)))
 					}
 					if service.Spec.DeployOnAllNodeSets {
 						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_override_hosts"])).To(Equal("\"all\""))
@@ -465,10 +465,10 @@ var _ = Describe("Dataplane Deployment Test", func() {
 					}
 					ansibleEE.Status.JobStatus = ansibleeev1.JobStatusSucceeded
 					g.Expect(th.K8sClient.Status().Update(th.Ctx, ansibleEE)).To(Succeed())
-					if service.Spec.EDPMServiceName != "" {
-						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_name"])).To(Equal(fmt.Sprintf("\"%s\"", service.Spec.EDPMServiceName)))
+					if service.Spec.EDPMServiceType != "" {
+						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_type"])).To(Equal(fmt.Sprintf("\"%s\"", service.Spec.EDPMServiceType)))
 					} else {
-						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_name"])).To(Equal(fmt.Sprintf("\"%s\"", serviceName)))
+						g.Expect(string(ansibleEE.Spec.ExtraVars["edpm_service_type"])).To(Equal(fmt.Sprintf("\"%s\"", serviceName)))
 					}
 				}, th.Timeout, th.Interval).Should(Succeed())
 			}