Drop OpenStackDataPlaneService.Spec.Label field

The field was proving to be redundant, since it always matched the name of the Service. It was only really being used to enforce a length, that logic is moved to a function to just use the first 53 chars of the name, then the field can be dropped. Signed-off-by: James Slagle <[email protected]>
openstack-k8s-operators · Feb 14, 2024 · e7ff71d · e7ff71d
1 parent 798e16b
commit e7ff71d
Show file tree

Hide file tree

Showing 11 changed files with 43 additions and 40 deletions.
diff --git a/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/api/bases/dataplane.openstack.org_openstackdataplaneservices.yaml
@@ -40,10 +40,6 @@ spec:
                 items:
                   type: string
                 type: array
-              label:
-                maxLength: 53
-                pattern: '[a-z]([-a-z0-9]*[a-z0-9])'
-                type: string
               openStackAnsibleEERunnerImage:
                 type: string
               play:

diff --git a/api/v1beta1/openstackdataplaneservice_types.go b/api/v1beta1/openstackdataplaneservice_types.go
@@ -42,15 +42,6 @@ type OpenstackDataPlaneServiceCert struct {
 
 // OpenStackDataPlaneServiceSpec defines the desired state of OpenStackDataPlaneService
 type OpenStackDataPlaneServiceSpec struct {
-	// Label to use for service.
-	// Must follow DNS952 subdomain conventions.
-	// Since we are using it to generate the pod name,
-	// we need to keep it short.
-	// +kubebuilder:validation:Optional
-	// +kubebuilder:validation:Pattern="[a-z]([-a-z0-9]*[a-z0-9])"
-	// +kubebuilder:validation:MaxLength=53
-	Label string `json:"label,omitempty"`
-
 	// Play is an inline playbook contents that ansible will run on execution.
 	Play string `json:"play,omitempty"`
 

diff --git a/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml b/config/crd/bases/dataplane.openstack.org_openstackdataplaneservices.yaml
@@ -40,10 +40,6 @@ spec:
                 items:
                   type: string
                 type: array
-              label:
-                maxLength: 53
-                pattern: '[a-z]([-a-z0-9]*[a-z0-9])'
-                type: string
               openStackAnsibleEERunnerImage:
                 type: string
               play:

diff --git a/docs/assemblies/custom_resources.adoc b/docs/assemblies/custom_resources.adoc
@@ -253,11 +253,6 @@ OpenStackDataPlaneServiceSpec defines the desired state of OpenStackDataPlaneSer
 |===
 | Field | Description | Scheme | Required
 
-| label
-| Label to use for service. Must follow DNS952 subdomain conventions. Since we are using it to generate the pod name, we need to keep it short.
-| string
-| false
-
 | play
 | Play is an inline playbook contents that ansible will run on execution.
 | string

diff --git a/pkg/deployment/deployment.go b/pkg/deployment/deployment.go
@@ -57,7 +57,6 @@ func (d *Deployer) Deploy(services []string) (*ctrl.Result, error) {
 	var readyWaitingMessage string
 	var readyErrorMessage string
 	var deployName string
-	var deployLabel string
 
 	// Save a copy of the original ExtraMounts so it can be reset after each
 	// service deployment
@@ -71,7 +70,6 @@ func (d *Deployer) Deploy(services []string) (*ctrl.Result, error) {
 			return &ctrl.Result{}, err
 		}
 		deployName = foundService.Name
-		deployLabel = foundService.Spec.Label
 		readyCondition = condition.Type(fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentReadyCondition, service))
 		readyWaitingMessage = fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentReadyWaitingMessage, deployName)
 		readyMessage = fmt.Sprintf(dataplanev1.NodeSetServiceDeploymentReadyMessage, deployName)
@@ -103,7 +101,6 @@ func (d *Deployer) Deploy(services []string) (*ctrl.Result, error) {
 			readyWaitingMessage,
 			readyErrorMessage,
 			deployName,
-			deployLabel,
 			foundService,
 		)
 
@@ -127,7 +124,6 @@ func (d *Deployer) ConditionalDeploy(
 	readyWaitingMessage string,
 	readyErrorMessage string,
 	deployName string,
-	deployLabel string,
 	foundService dataplanev1.OpenStackDataPlaneService,
 ) error {
 	var err error
@@ -151,7 +147,7 @@ func (d *Deployer) ConditionalDeploy(
 	}
 
 	if nsConditions.IsFalse(readyCondition) {
-		ansibleEE, err := dataplaneutil.GetAnsibleExecution(d.Ctx, d.Helper, d.Deployment, deployLabel)
+		ansibleEE, err := dataplaneutil.GetAnsibleExecution(d.Ctx, d.Helper, d.Deployment, foundService.Name)
 		if err != nil {
 			// Return nil if we don't have AnsibleEE available yet
 			if k8s_errors.IsNotFound(err) {

diff --git a/pkg/util/ansible_execution.go b/pkg/util/ansible_execution.go
@@ -48,24 +48,20 @@ func AnsibleExecution(
 	var err error
 	var cmdLineArguments strings.Builder
 
-	ansibleEE, err := GetAnsibleExecution(ctx, helper, obj, service.Spec.Label)
+	ansibleEE, err := GetAnsibleExecution(ctx, helper, obj, service.Name)
 	if err != nil && !k8serrors.IsNotFound(err) {
 		return err
 	}
 	if ansibleEE == nil {
-		var executionName string
-		if len(service.Spec.Label) > 0 {
-			executionName = fmt.Sprintf("%s-%s", service.Spec.Label, obj.GetName())
-		} else {
-			executionName = obj.GetName()
-		}
+
+		executionName := fmt.Sprintf("%s-%s", GetAnsibleExecutionNamePrefix(service), obj.GetName())
 		ansibleEE = &ansibleeev1.OpenStackAnsibleEE{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      executionName,
 				Namespace: obj.GetNamespace(),
 				Labels: map[string]string{
-					service.Spec.Label: string(obj.GetUID()),
-					"osdpd":            obj.GetName(),
+					service.Name: string(obj.GetUID()),
+					"osdpd":      obj.GetName(),
 				},
 			},
 		}
@@ -197,3 +193,14 @@ func GetAnsibleExecution(ctx context.Context, helper *helper.Helper, obj client.
 
 	return ansibleEE, nil
 }
+
+// GetAnsibleExecutionNamePrefix compute the name of the AnsibleEE
+func GetAnsibleExecutionNamePrefix(service *dataplanev1.OpenStackDataPlaneService) string {
+	var executionNamePrefix string
+	if len(service.Name) > AnsibleExecutionServiceNameLen {
+		executionNamePrefix = service.Name[:AnsibleExecutionServiceNameLen]
+	} else {
+		executionNamePrefix = service.Name
+	}
+	return executionNamePrefix
+}
diff --git a/pkg/util/const.go b/pkg/util/const.go
@@ -0,0 +1,22 @@
+/*
+Copyright 2023.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package util
+
+const (
+	// AnsibleExecutionServiceNameLen max length for the ansibleEE service name prefix
+	AnsibleExecutionServiceNameLen = 53
+)
diff --git a/tests/functional/openstackdataplanedeployment_controller_test.go b/tests/functional/openstackdataplanedeployment_controller_test.go
@@ -7,6 +7,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	dataplanev1 "github.com/openstack-k8s-operators/dataplane-operator/api/v1beta1"
+	dataplaneutil "github.com/openstack-k8s-operators/dataplane-operator/pkg/util"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
 	ansibleeev1 "github.com/openstack-k8s-operators/openstack-ansibleee-operator/api/v1beta1"
@@ -144,11 +145,12 @@ var _ = Describe("Dataplane Deployment Test", func() {
 					Namespace: namespace,
 				}
 				service := GetService(dataplaneServiceName)
+				executionName := dataplaneutil.GetAnsibleExecutionNamePrefix(service)
 				//Retrieve service AnsibleEE and set JobStatus to Successful
 				Eventually(func(g Gomega) {
 					// Make an AnsibleEE name for each service
 					ansibleeeName := types.NamespacedName{
-						Name:      fmt.Sprintf("%s-%s", service.Spec.Label, dataplaneDeploymentName.Name),
+						Name:      fmt.Sprintf("%s-%s", executionName, dataplaneDeploymentName.Name),
 						Namespace: dataplaneDeploymentName.Namespace,
 					}
 					ansibleEE := &ansibleeev1.OpenStackAnsibleEE{

diff --git a/tests/kuttl/tests/dataplane-service-config/00-create.yaml b/tests/kuttl/tests/dataplane-service-config/00-create.yaml
@@ -39,7 +39,6 @@ kind: OpenStackDataPlaneService
 metadata:
   name: kuttl-service
 spec:
-  label: kuttl-service
   play: |
     - hosts: localhost
       gather_facts: no

diff --git a/tests/kuttl/tests/dataplane-service-custom-image/00-assert.yaml b/tests/kuttl/tests/dataplane-service-custom-image/00-assert.yaml
@@ -37,7 +37,7 @@ status:
 apiVersion: ansibleee.openstack.org/v1beta1
 kind: OpenStackAnsibleEE
 metadata:
-  name: dp-custom-image-service-edpm-compute-no-nodes
+  name: custom-image-service-edpm-compute-no-nodes
   namespace: openstack
   ownerReferences:
   - apiVersion: dataplane.openstack.org/v1beta1

diff --git a/tests/kuttl/tests/dataplane-service-custom-image/00-dataplane-create.yaml b/tests/kuttl/tests/dataplane-service-custom-image/00-dataplane-create.yaml
@@ -3,7 +3,6 @@ kind: OpenStackDataPlaneService
 metadata:
   name: custom-image-service
 spec:
-  label: dp-custom-image-service
   openStackAnsibleEERunnerImage: example.com/repo/runner-image:latest
   role:
     name: "test role"