[OSPRH-11235] Do not fail when clouds CM exists

The Ensure[Horizon|Tobiko]CloudsYAML function was failing when the CM containing the modified clouds.yaml already existed: Object openstack/horizontest-clouds-config is already owned by another HorizonTest controller horizontest-tests This patch does two things: - Ensures that we do not create the modified clouds.yaml when it already exists. - Removes the duplicate implementation of of the Ensure*CloudsYAML function and replaces it with EnsureCloudsConfigMapExists.
openstack-k8s-operators · Nov 5, 2024 · 779f50c · 779f50c
1 parent 06ff0ea
commit 779f50c
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 103 deletions.
diff --git a/controllers/common.go b/controllers/common.go
@@ -16,6 +16,7 @@ import (
 	"github.com/openstack-k8s-operators/lib-common/modules/common/pvc"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	v1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
+	"gopkg.in/yaml.v3"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -550,3 +551,71 @@ func GetCommonRbacRules(privileged bool) []rbacv1.PolicyRule {
 
 	return []rbacv1.PolicyRule{rbacPolicyRule}
 }
+
+// Some frameworks like (e.g., Tobiko and Horizon) require password value to be
+// present in clouds.yaml. This code ensures that we set a default value of
+// 12345678 when password value is missing in the clouds.yaml
+func EnsureCloudsConfigMapExists(
+	ctx context.Context,
+	instance client.Object,
+	helper *helper.Helper,
+	labels map[string]string,
+) (ctrl.Result, error) {
+	const openstackConfigMapName = "openstack-config"
+	const testOperatorCloudsConfigMapName = "test-operator-clouds-config"
+
+	cm, _, _ := configmap.GetConfigMap(
+		ctx,
+		helper,
+		instance,
+		openstackConfigMapName,
+		time.Second*10,
+	)
+	if cm.Name == testOperatorCloudsConfigMapName {
+		return ctrl.Result{}, nil
+	}
+
+	cm, _, _ = configmap.GetConfigMap(
+		ctx,
+		helper,
+		instance,
+		openstackConfigMapName,
+		time.Second*10,
+	)
+	result := make(map[string]interface{})
+
+	err := yaml.Unmarshal([]byte(cm.Data["clouds.yaml"]), &result)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	clouds := result["clouds"].(map[string]interface{})
+	defaultValue := clouds["default"].(map[string]interface{})
+	auth := defaultValue["auth"].(map[string]interface{})
+
+	if _, ok := auth["password"].(string); !ok {
+		auth["password"] = "12345678"
+	}
+
+	yamlString, err := yaml.Marshal(result)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	cms := []util.Template{
+		{
+			Name:      testOperatorCloudsConfigMapName,
+			Namespace: instance.GetNamespace(),
+			Labels:    labels,
+			CustomData: map[string]string{
+				"clouds.yaml": string(yamlString),
+			},
+		},
+	}
+	err = configmap.EnsureConfigMaps(ctx, helper, instance, cms, nil)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
+	return ctrl.Result{}, nil
+}
diff --git a/controllers/horizontest_controller.go b/controllers/horizontest_controller.go
@@ -23,20 +23,16 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/openstack-k8s-operators/lib-common/modules/common"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/configmap"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/job"
 	common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/test-operator/pkg/horizontest"
-	"gopkg.in/yaml.v3"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
@@ -155,7 +151,7 @@ func (r *HorizonTestReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		"workflowStep": "0",
 	}
 
-	yamlResult, err := r.EnsureHorizonTestCloudsYAML(ctx, instance, helper, serviceLabels)
+	yamlResult, err := EnsureCloudsConfigMapExists(ctx, instance, helper, serviceLabels)
 
 	if err != nil {
 		return yamlResult, err
@@ -272,49 +268,6 @@ func (r *HorizonTestReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-// Horizon requires password value to be present in clouds.yaml
-// This code ensures that we set a default value of 12345678 when
-// password value is missing in the clouds.yaml
-func (r *HorizonTestReconciler) EnsureHorizonTestCloudsYAML(ctx context.Context, instance client.Object, helper *helper.Helper, labels map[string]string) (ctrl.Result, error) {
-	cm, _, _ := configmap.GetConfigMap(ctx, helper, instance, "openstack-config", time.Second*10)
-	result := make(map[string]interface{})
-
-	err := yaml.Unmarshal([]byte(cm.Data["clouds.yaml"]), &result)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
-	clouds := result["clouds"].(map[string]interface{})
-	defaultValue := clouds["default"].(map[string]interface{})
-	auth := defaultValue["auth"].(map[string]interface{})
-
-	if _, ok := auth["password"].(string); !ok {
-		auth["password"] = "12345678"
-	}
-
-	yamlString, err := yaml.Marshal(result)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
-	cms := []util.Template{
-		{
-			Name:      "horizontest-clouds-config",
-			Namespace: instance.GetNamespace(),
-			Labels:    labels,
-			CustomData: map[string]string{
-				"clouds.yaml": string(yamlString),
-			},
-		},
-	}
-	err = configmap.EnsureConfigMaps(ctx, helper, instance, cms, nil)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
-	return ctrl.Result{}, nil
-}
-
 func (r *HorizonTestReconciler) PrepareHorizonTestEnvVars(
 	instance *testv1beta1.HorizonTest,
 ) map[string]env.Setter {

diff --git a/controllers/tobiko_controller.go b/controllers/tobiko_controller.go
@@ -34,7 +34,6 @@ import (
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/test-operator/pkg/tobiko"
-	"gopkg.in/yaml.v3"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
@@ -182,7 +181,7 @@ func (r *TobikoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 		"operator":         "test-operator",
 	}
 
-	yamlResult, err := r.EnsureTobikoCloudsYAML(ctx, instance, helper, serviceLabels)
+	yamlResult, err := EnsureCloudsConfigMapExists(ctx, instance, helper, serviceLabels)
 
 	if err != nil {
 		return yamlResult, err
@@ -359,49 +358,6 @@ func (r *TobikoReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-// Tobiko requires password value to be present in clouds.yaml
-// This code ensures that we set a default value of 12345678 when
-// password value is missing in the clouds.yaml
-func (r *TobikoReconciler) EnsureTobikoCloudsYAML(ctx context.Context, instance client.Object, helper *helper.Helper, labels map[string]string) (ctrl.Result, error) {
-	cm, _, _ := configmap.GetConfigMap(ctx, helper, instance, "openstack-config", time.Second*10)
-	result := make(map[string]interface{})
-
-	err := yaml.Unmarshal([]byte(cm.Data["clouds.yaml"]), &result)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
-	clouds := result["clouds"].(map[string]interface{})
-	defaultValue := clouds["default"].(map[string]interface{})
-	auth := defaultValue["auth"].(map[string]interface{})
-
-	if _, ok := auth["password"].(string); !ok {
-		auth["password"] = "12345678"
-	}
-
-	yamlString, err := yaml.Marshal(result)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
-	cms := []util.Template{
-		{
-			Name:      "tobiko-clouds-config",
-			Namespace: instance.GetNamespace(),
-			Labels:    labels,
-			CustomData: map[string]string{
-				"clouds.yaml": string(yamlString),
-			},
-		},
-	}
-	err = configmap.EnsureConfigMaps(ctx, helper, instance, cms, nil)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
-	return ctrl.Result{}, nil
-}
-
 // This function prepares env variables for a single workflow step.
 func (r *TobikoReconciler) PrepareTobikoEnvVars(
 	ctx context.Context,

diff --git a/pkg/horizontest/volumes.go b/pkg/horizontest/volumes.go
@@ -2,6 +2,8 @@ package horizontest
 
 import (
 	testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
+	util "github.com/openstack-k8s-operators/test-operator/pkg/util"
+
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -15,8 +17,6 @@ func GetVolumes(
 
 	var scriptsVolumeDefaultMode int32 = 0755
 	var scriptsVolumeConfidentialMode int32 = 0420
-	//var privateKeyMode int32 = 0600
-	//var publicKeyMode int32 = 0644
 	var tlsCertificateMode int32 = 0444
 	var publicInfoMode int32 = 0744
 
@@ -33,12 +33,12 @@ func GetVolumes(
 			},
 		},
 		{
-			Name: "horizontest-clouds-config",
+			Name: util.TestOperatorCloudsConfigMapName,
 			VolumeSource: corev1.VolumeSource{
 				ConfigMap: &corev1.ConfigMapVolumeSource{
 					DefaultMode: &scriptsVolumeConfidentialMode,
 					LocalObjectReference: corev1.LocalObjectReference{
-						Name: "horizontest-clouds-config",
+						Name: util.TestOperatorCloudsConfigMapName,
 					},
 				},
 			},
@@ -123,13 +123,13 @@ func GetVolumeMounts(mountCerts bool, mountKeys bool, mountKubeconfig bool, inst
 			ReadOnly:  false,
 		},
 		{
-			Name:      "horizontest-clouds-config",
+			Name:      util.TestOperatorCloudsConfigMapName,
 			MountPath: "/var/lib/horizontest/.config/openstack/clouds.yaml",
 			SubPath:   "clouds.yaml",
 			ReadOnly:  true,
 		},
 		{
-			Name:      "horizontest-clouds-config",
+			Name:      util.TestOperatorCloudsConfigMapName,
 			MountPath: "/etc/openstack/clouds.yaml",
 			SubPath:   "clouds.yaml",
 			ReadOnly:  true,

diff --git a/pkg/tobiko/volumes.go b/pkg/tobiko/volumes.go
@@ -2,6 +2,7 @@ package tobiko
 
 import (
 	testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
+	"github.com/openstack-k8s-operators/test-operator/pkg/util"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -34,12 +35,12 @@ func GetVolumes(
 			},
 		},
 		{
-			Name: "tobiko-clouds-config",
+			Name: util.TestOperatorCloudsConfigMapName,
 			VolumeSource: corev1.VolumeSource{
 				ConfigMap: &corev1.ConfigMapVolumeSource{
 					DefaultMode: &scriptsVolumeConfidentialMode,
 					LocalObjectReference: corev1.LocalObjectReference{
-						Name: "tobiko-clouds-config",
+						Name: util.TestOperatorCloudsConfigMapName,
 					},
 				},
 			},
@@ -155,13 +156,13 @@ func GetVolumeMounts(mountCerts bool, mountKeys bool, mountKubeconfig bool, inst
 			ReadOnly:  false,
 		},
 		{
-			Name:      "tobiko-clouds-config",
+			Name:      util.TestOperatorCloudsConfigMapName,
 			MountPath: "/var/lib/tobiko/.config/openstack/clouds.yaml",
 			SubPath:   "clouds.yaml",
 			ReadOnly:  true,
 		},
 		{
-			Name:      "tobiko-clouds-config",
+			Name:      util.TestOperatorCloudsConfigMapName,
 			MountPath: "/etc/openstack/clouds.yaml",
 			SubPath:   "clouds.yaml",
 			ReadOnly:  true,

diff --git a/pkg/util/common.go b/pkg/util/common.go
@@ -4,6 +4,13 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
+const (
+	// TestOperatorCloudsConfigMapName is name of the ConfigMap which contains
+	// modified clouds.yaml obtained from openstack-config ConfigMap. The modified
+	// CM is needed by some test frameworks (e.g., HorizonTest and Tobiko)
+	TestOperatorCloudsConfigMapName = "test-operator-clouds-config"
+)
+
 func GetSecurityContext(
 	runAsUser int64,
 	addCapabilities []corev1.Capability,