From 826d7d0e54d46a853f4bc6f4ef6f986aaf97a3c9 Mon Sep 17 00:00:00 2001 From: James Slagle Date: Thu, 23 May 2024 17:20:51 -0400 Subject: [PATCH] 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, the service name will be used instead. EDPMRoleServiceName is kept in case there is a need to override the path for a specific TLS cert. Signed-off-by: James Slagle --- ...nstack.org_openstackdataplaneservices.yaml | 2 +- .../openstackdataplaneservice_types.go | 12 ++++++---- ...nstack.org_openstackdataplaneservices.yaml | 2 +- docs/assemblies/custom_resources.adoc | 6 ++--- pkg/deployment/deployment.go | 12 +++++++--- pkg/util/ansible_execution.go | 6 ++--- ...tackdataplanedeployment_controller_test.go | 24 +++++++++---------- 7 files changed, 36 insertions(+), 28 deletions(-) diff --git a/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml index c55f2ff8d..e48e77f2c 100644 --- 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 index 53b95437a..452a8634d 100644 --- 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,11 @@ 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. + EDPMServiceType string `json:"edpmServiceType,omitempty" yaml:"edpmServiceType,omitempty"` } // OpenStackDataPlaneServiceStatus defines the observed state of OpenStackDataPlaneService diff --git a/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml index c55f2ff8d..e48e77f2c 100644 --- 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 index 52404ffcf..4d59a9202 100644 --- 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. | 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 ++++++_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.++++++ +| EDPMRoleServiceName is the value of the ++++++_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.++++++ | string | false |=== diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go index 6a85f9cdb..ae840607c 100644 --- a/pkg/deployment/deployment.go +++ b/pkg/deployment/deployment.go @@ -280,8 +280,8 @@ func (d *Deployer) addCertMounts( }, } certMountDir := service.Spec.TLSCert.EDPMRoleServiceName - if certMountDir == "" && service.Spec.EDPMServiceName != "" { - certMountDir = service.Spec.EDPMServiceName + if certMountDir == "" && service.Spec.EDPMServiceType != "" { + certMountDir = service.Spec.EDPMServiceType } else { certMountDir = service.Name } @@ -313,9 +313,15 @@ func (d *Deployer) addCertMounts( }, } + var cacertMountDir string + if service.Spec.EDPMServiceType != "" { + cacertMountDir = service.Spec.EDPMServiceType + } else { + cacertMountDir = service.Name + } cacertVolumeMount := corev1.VolumeMount{ Name: fmt.Sprintf("%s-%s", service.Name, service.Spec.CACerts), - MountPath: path.Join(CACertPaths, service.Name), + MountPath: path.Join(CACertPaths, cacertMountDir), } volMounts.Volumes = append(volMounts.Volumes, cacertVolume) diff --git a/pkg/util/ansible_execution.go b/pkg/util/ansible_execution.go index 4fd8902df..4c3dd0dda 100644 --- 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 index 359058c7a..fe948c7f0 100644 --- 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()) }