Fix customServiceConfig bug <Jira: OSPRH-10696>

Author: mumesan

controllers/ironic_controller.go

@@ -49,6 +49,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	batchv1 "k8s.io/api/batch/v1"
@@ -218,6 +219,52 @@ func (r *IronicReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *IronicReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	// watch for configmap where the CM owner label AND the CR.Spec.ManagingCrName label matches
+	configMapFn := func(ctx context.Context, o client.Object) []reconcile.Request {
+		Log := r.GetLogger(ctx)
+
+		result := []reconcile.Request{}
+
+		// get all API CRs
+		apis := &ironicv1.IronicInspectorList{}
+		listOpts := []client.ListOption{
+			client.InNamespace(o.GetNamespace()),
+		}
+		if err := r.Client.List(
+			ctx,
+			apis,
+			listOpts...); err != nil {
+
+			Log.Error(err, "Unable to retrieve API CRs %v")
+			return nil
+		}
+
+		label := o.GetLabels()
+		// TODO: Just trying to verify that the CM is owned by this CR's managing CR
+		if l, ok := label[labels.GetOwnerNameLabelSelector(
+			labels.GetGroupLabel(ironic.ServiceName))]; ok {
+			for _, cr := range apis.Items {
+				// return reconcil event for the CR where the CM owner label
+				// AND the parentIronicName matches
+				if l == ironicv1.GetOwningIronicName(&cr) {
+					// return namespace and Name of CR
+					name := client.ObjectKey{
+						Namespace: o.GetNamespace(),
+						Name:      cr.Name,
+					}
+					Log.Info(fmt.Sprintf(
+						"ConfigMap object %s and CR %s marked with label: %s",
+						o.GetName(), cr.Name, l))
+					result = append(
+						result, reconcile.Request{NamespacedName: name})
+				}
+			}
+		}
+		if len(result) > 0 {
+			return result
+		}
+		return nil
+	}
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&ironicv1.Ironic{}).
 		Owns(&ironicv1.IronicConductor{}).
@@ -228,10 +275,13 @@ func (r *IronicReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Owns(&mariadbv1.MariaDBAccount{}).
 		Owns(&batchv1.Job{}).
 		Owns(&corev1.Secret{}).
-		Owns(&corev1.ConfigMap{}).
 		Owns(&corev1.ServiceAccount{}).
 		Owns(&rbacv1.Role{}).
 		Owns(&rbacv1.RoleBinding{}).
+		Watches(
+			&corev1.ConfigMap{},
+			handler.EnqueueRequestsFromMapFunc(configMapFn),
+			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})).
 		Watches(&keystonev1.KeystoneAPI{},
 			handler.EnqueueRequestsFromMapFunc(r.findObjectForSrc),
 			builder.WithPredicates(keystonev1.KeystoneAPIStatusChangedPredicate)).
@@ -539,6 +589,10 @@ func (r *IronicReconciler) reconcileNormal(ctx context.Context, instance *ironic
 		}
 	}
 
+	if instance.Spec.IronicAPI.CustomServiceConfig == "" {
+		instance.Spec.IronicAPI.CustomServiceConfig = instance.Spec.IronicSpecCore.CustomServiceConfig
+	}
+
 	// deploy ironic-api
 	ironicAPI, op, err := r.apiDeploymentCreateOrUpdate(instance, &keystoneEndpoints)
 	if err != nil {
@@ -935,8 +989,8 @@ func (r *IronicReconciler) generateServiceConfigMaps(
 	// all other files get placed into /etc/ironic to allow overwrite of e.g. policy.json
 	// TODO: make sure custom.conf can not be overwritten
 	customData := map[string]string{
-		common.CustomServiceConfigFileName: instance.Spec.CustomServiceConfig,
-		"my.cnf":                           db.GetDatabaseClientConfig(tlsCfg), //(mschuppert) for now just get the default my.cnf
+		"01-ironic-custom.conf": instance.Spec.CustomServiceConfig,
+		"my.cnf":                db.GetDatabaseClientConfig(tlsCfg), //(mschuppert) for now just get the default my.cnf
 
 	}
 	for key, data := range instance.Spec.DefaultConfigOverwrite {

controllers/ironicapi_controller.go

@@ -224,43 +224,6 @@ func (r *IronicAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *IronicAPIReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
-	// watch for configmap where the CM owner label AND the CR.Spec.ManagingCrName label matches
-	configMapFn := func(ctx context.Context, o client.Object) []reconcile.Request {
-		Log := r.GetLogger(ctx)
-
-		result := []reconcile.Request{}
-
-		// get all API CRs
-		apis := &ironicv1.IronicAPIList{}
-		listOpts := []client.ListOption{
-			client.InNamespace(o.GetNamespace()),
-		}
-		if err := r.Client.List(ctx, apis, listOpts...); err != nil {
-			Log.Error(err, "Unable to retrieve API CRs %v")
-			return nil
-		}
-
-		label := o.GetLabels()
-		// TODO: Just trying to verify that the CM is owned by this CR's managing CR
-		if l, ok := label[labels.GetOwnerNameLabelSelector(labels.GetGroupLabel(ironic.ServiceName))]; ok {
-			for _, cr := range apis.Items {
-				// return reconcil event for the CR where the CM owner label AND the parentIronicName matches
-				if l == ironicv1.GetOwningIronicName(&cr) {
-					// return namespace and Name of CR
-					name := client.ObjectKey{
-						Namespace: o.GetNamespace(),
-						Name:      cr.Name,
-					}
-					Log.Info(fmt.Sprintf("ConfigMap object %s and CR %s marked with label: %s", o.GetName(), cr.Name, l))
-					result = append(result, reconcile.Request{NamespacedName: name})
-				}
-			}
-		}
-		if len(result) > 0 {
-			return result
-		}
-		return nil
-	}
 
 	// index passwordSecretField
 	if err := mgr.GetFieldIndexer().IndexField(ctx, &ironicv1.IronicAPI{}, passwordSecretField, func(rawObj client.Object) []string {
@@ -332,9 +295,6 @@ func (r *IronicAPIReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Man
 		Owns(&corev1.ServiceAccount{}).
 		Owns(&rbacv1.Role{}).
 		Owns(&rbacv1.RoleBinding{}).
-		// watch the config CMs we don't own
-		Watches(&corev1.ConfigMap{},
-			handler.EnqueueRequestsFromMapFunc(configMapFn)).
 		Watches(
 			&corev1.Secret{},
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
@@ -351,8 +311,36 @@ func (r *IronicAPIReconciler) findObjectsForSrc(ctx context.Context, src client.
 
 	l := log.FromContext(ctx).WithName("Controllers").WithName("IronicAPI")
 
+	crList := &ironicv1.IronicAPIList{}
+	listOpts := []client.ListOption{client.InNamespace(src.GetNamespace())}
+
+	if err := r.List(ctx, crList, listOpts...); err != nil {
+		l.Error(err, "Unable to retrieve Conductor CRs %v")
+	} else {
+		label := src.GetLabels()
+		// TODO: Just trying to verify that the Secret is owned by this CR's managing CR
+		if lbl, ok := label[labels.GetOwnerNameLabelSelector(labels.GetGroupLabel(ironic.ServiceName))]; ok {
+			for _, item := range crList.Items {
+				// return reconcile event for the CR where the Secret owner label AND the parentIronicName matches
+				if lbl == ironicv1.GetOwningIronicName(&item) {
+					// return Namespace and Name of CR
+					l.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace()))
+
+					requests = append(requests,
+						reconcile.Request{
+							NamespacedName: types.NamespacedName{
+								Name:      item.GetName(),
+								Namespace: item.GetNamespace(),
+							},
+						},
+					)
+
+				}
+			}
+		}
+	}
+
 	for _, field := range ironicAPIWatchFields {
-		crList := &ironicv1.IronicAPIList{}
 		listOps := &client.ListOptions{
 			FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
 			Namespace:     src.GetNamespace(),
@@ -662,6 +650,31 @@ func (r *IronicAPIReconciler) reconcileNormal(ctx context.Context, instance *iro
 	//
 	// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
 	//
+
+	// Get secrets by Namespace and Label that we need to hash
+	labelSelectorMap := map[string]string{}
+	lbl := labels.GetOwnerNameLabelSelector(labels.GetGroupLabel(ironic.ServiceName))
+	labelSelectorMap[lbl] = ironicv1.GetOwningIronicName(instance)
+	secrets, err := secret.GetSecrets(ctx, helper, instance.Namespace, labelSelectorMap)
+	if err != nil {
+		Log.Info(fmt.Sprintf("No secrets with label %s found", lbl))
+	} else {
+		for _, s := range secrets.Items {
+			hash, err := secret.Hash(&s)
+			if err != nil {
+				Log.Info(fmt.Sprintf("Not able to has secret %s found", s.Name))
+				instance.Status.Conditions.Set(condition.FalseCondition(
+					condition.InputReadyCondition,
+					condition.ErrorReason,
+					condition.SeverityWarning,
+					condition.InputReadyErrorMessage,
+					err.Error()))
+				return ctrl.Result{}, err
+			}
+			configMapVars[s.Name] = env.SetValue(hash)
+		}
+	}
+
 	ospSecret, hash, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
 	if err != nil {
 		if k8s_errors.IsNotFound(err) {

controllers/ironicconductor_controller.go

@@ -207,43 +207,6 @@ func (r *IronicConductorReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *IronicConductorReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error {
-	// watch for configmap where the CM owner label AND the CR.Spec.ManagingCrName label matches
-	configMapFn := func(ctx context.Context, o client.Object) []reconcile.Request {
-		result := []reconcile.Request{}
-		Log := r.GetLogger(ctx)
-
-		// get all API CRs
-		apis := &ironicv1.IronicConductorList{}
-		listOpts := []client.ListOption{
-			client.InNamespace(o.GetNamespace()),
-		}
-		if err := r.Client.List(ctx, apis, listOpts...); err != nil {
-			Log.Error(err, "Unable to retrieve API CRs %v")
-			return nil
-		}
-
-		label := o.GetLabels()
-		// TODO: Just trying to verify that the CM is owned by this CR's managing CR
-		if l, ok := label[labels.GetOwnerNameLabelSelector(labels.GetGroupLabel(ironic.ServiceName))]; ok {
-			for _, cr := range apis.Items {
-				// return reconcil event for the CR where the CM owner label AND the parentIronicName matches
-				if l == ironicv1.GetOwningIronicName(&cr) {
-					// return namespace and Name of CR
-					name := client.ObjectKey{
-						Namespace: o.GetNamespace(),
-						Name:      cr.Name,
-					}
-					Log.Info(fmt.Sprintf("ConfigMap object %s and CR %s marked with label: %s", o.GetName(), cr.Name, l))
-					result = append(result, reconcile.Request{NamespacedName: name})
-				}
-			}
-		}
-		if len(result) > 0 {
-			return result
-		}
-		return nil
-	}
-
 	// index passwordSecretField
 	if err := mgr.GetFieldIndexer().IndexField(ctx, &ironicv1.IronicConductor{}, passwordSecretField, func(rawObj client.Object) []string {
 		// Extract the secret name from the spec, if one is provided
@@ -290,9 +253,6 @@ func (r *IronicConductorReconciler) SetupWithManager(ctx context.Context, mgr ct
 		Owns(&corev1.ServiceAccount{}).
 		Owns(&rbacv1.Role{}).
 		Owns(&rbacv1.RoleBinding{}).
-		// watch the config CMs we don't own
-		Watches(&corev1.ConfigMap{},
-			handler.EnqueueRequestsFromMapFunc(configMapFn)).
 		Watches(
 			&corev1.Secret{},
 			handler.EnqueueRequestsFromMapFunc(r.findObjectsForSrc),
@@ -309,8 +269,38 @@ func (r *IronicConductorReconciler) findObjectsForSrc(ctx context.Context, src c
 
 	l := log.FromContext(ctx).WithName("Controllers").WithName("IronicConductor")
 
+	crList := &ironicv1.IronicConductorList{}
+	namespace := src.GetNamespace()
+	listOpts := []client.ListOption{client.InNamespace(namespace)}
+
+	if err := r.List(ctx, crList, listOpts...); err != nil {
+		l.Error(err, "Unable to retrieve Conductor CRs %v")
+	} else {
+		label := src.GetLabels()
+		// TODO: Just trying to verify that the Secret is owned by this CR's managing CR
+		if lbl, ok := label[labels.GetOwnerNameLabelSelector(labels.GetGroupLabel(ironic.ServiceName))]; ok {
+			for _, item := range crList.Items {
+				// return reconcil event for the CR where the Secret owner label AND the parentIronicName matches
+				if lbl == ironicv1.GetOwningIronicName(&item) {
+					// return Namespace and Name of CR
+					l.Info(fmt.Sprintf("input source %s changed, reconcile: %s - %s", src.GetName(), item.GetName(), item.GetNamespace()))
+
+					requests = append(
+						requests,
+						reconcile.Request{
+							NamespacedName: k8s_types.NamespacedName{
+								Name:      item.GetName(),
+								Namespace: item.GetNamespace(),
+							},
+						},
+					)
+
+				}
+			}
+		}
+	}
+
 	for _, field := range ironicConductorWatchFields {
-		crList := &ironicv1.IronicConductorList{}
 		listOps := &client.ListOptions{
 			FieldSelector: fields.OneTermEqualSelector(field, src.GetName()),
 			Namespace:     src.GetNamespace(),
@@ -500,6 +490,30 @@ func (r *IronicConductorReconciler) reconcileNormal(ctx context.Context, instanc
 	//
 	// check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map
 	//
+
+	// Get secrets by Namespace and Label that we need to hash ...
+	labelSelectorMap := map[string]string{}
+	lbl := labels.GetOwnerNameLabelSelector(labels.GetGroupLabel(ironic.ServiceName))
+	labelSelectorMap[lbl] = ironicv1.GetOwningIronicName(instance)
+	secrets, err := secret.GetSecrets(ctx, helper, instance.Namespace, labelSelectorMap)
+	if err != nil {
+		Log.Info(fmt.Sprintf("No secrets with label %s found", lbl))
+	} else {
+		for _, s := range secrets.Items {
+			hash, err := secret.Hash(&s)
+			if err != nil {
+				Log.Info(fmt.Sprintf("Not able to has secret %s not found", s.Name))
+				instance.Status.Conditions.Set(condition.FalseCondition(
+					condition.InputReadyCondition,
+					condition.ErrorReason,
+					condition.SeverityWarning,
+					condition.InputReadyErrorMessage,
+					err.Error()))
+				return ctrl.Result{}, err
+			}
+			configMapVars[s.Name] = env.SetValue(hash)
+		}
+	}
 	ospSecret, hash, err := secret.GetSecret(ctx, helper, instance.Spec.Secret, instance.Namespace)
 	if err != nil {
 		if k8s_errors.IsNotFound(err) {

pkg/ironicapi/volumes.go

@@ -1,21 +1,24 @@
 package ironicapi
 
 import (
+	"fmt"
+	"strings"
+
 	"github.com/openstack-k8s-operators/ironic-operator/pkg/ironic"
 	corev1 "k8s.io/api/core/v1"
 )
 
 // GetVolumes -
 func GetVolumes(name string) []corev1.Volume {
 	var config0640AccessMode int32 = 0640
-
+	parentName := strings.Replace(name, "-api", "", 1)
 	apiVolumes := []corev1.Volume{
 		{
 			Name: "config-data-custom",
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
 					DefaultMode: &config0640AccessMode,
-					SecretName:  name + "-config-data",
+					SecretName:  fmt.Sprintf("%s-config-data", parentName),
 				},
 			},
 		},

pkg/ironicconductor/volumes.go

@@ -2,6 +2,7 @@ package ironicconductor
 
 import (
 	"fmt"
+	"strings"
 
 	ironicv1 "github.com/openstack-k8s-operators/ironic-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/ironic-operator/pkg/ironic"
@@ -11,13 +12,15 @@ import (
 // GetVolumes -
 func GetVolumes(instance *ironicv1.IronicConductor) []corev1.Volume {
 	var config0640AccessMode int32 = 0640
+	parentName := strings.Replace(instance.Name, "-conductor", "", 1)
+
 	conductorVolumes := []corev1.Volume{
 		{
 			Name: "config-data-custom",
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
 					DefaultMode: &config0640AccessMode,
-					SecretName:  fmt.Sprintf("%s-config-data", instance.Name),
+					SecretName:  fmt.Sprintf("%s-config-data", parentName),
 				},
 			},
 		},