Merge pull request #812 from vyzigold/fix-autoscaling-watching

[OSPRH-22602] Fix resource watching in autoscaling

Author: openshift-merge-bot[bot]

internal/controller/aodh_controller.go

@@ -24,7 +24,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -33,14 +32,12 @@ import (
 	common "github.com/openstack-k8s-operators/lib-common/modules/common"
 	condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	endpoint "github.com/openstack-k8s-operators/lib-common/modules/common/endpoint"
-	env "github.com/openstack-k8s-operators/lib-common/modules/common/env"
 	helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	job "github.com/openstack-k8s-operators/lib-common/modules/common/job"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/labels"
 	secret "github.com/openstack-k8s-operators/lib-common/modules/common/secret"
 	service "github.com/openstack-k8s-operators/lib-common/modules/common/service"
 	statefulset "github.com/openstack-k8s-operators/lib-common/modules/common/statefulset"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/tls"
 	util "github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
 
@@ -304,9 +301,6 @@ func (r *AutoscalingReconciler) reconcileNormalAodh(
 		common.AppSelector: autoscaling.ServiceName,
 	}
 
-	// ConfigVars
-	configVars := make(map[string]env.Setter)
-
 	//
 	// Handle Topology
 	//
@@ -521,66 +515,6 @@ func (r *AutoscalingReconciler) reconcileNormalAodh(
 		return ctrlResult, nil
 	}
 
-	//
-	// TLS input validation
-	//
-	// Validate the CA cert secret if provided
-	if instance.Spec.Aodh.TLS.CaBundleSecretName != "" {
-		hash, err := tls.ValidateCACertSecret(
-			ctx,
-			helper.GetClient(),
-			types.NamespacedName{
-				Name:      instance.Spec.Aodh.TLS.CaBundleSecretName,
-				Namespace: instance.Namespace,
-			},
-		)
-		if err != nil {
-			if k8s_errors.IsNotFound(err) {
-				// Since the CA cert secret should have been manually created by the user and provided in the spec,
-				// we treat this as a warning because it means that the service will not be able to start.
-				instance.Status.Conditions.Set(condition.FalseCondition(
-					condition.TLSInputReadyCondition,
-					condition.ErrorReason,
-					condition.SeverityWarning,
-					condition.TLSInputReadyWaitingMessage, instance.Spec.Aodh.TLS.CaBundleSecretName))
-				return ctrl.Result{}, nil
-			}
-			instance.Status.Conditions.Set(condition.FalseCondition(
-				condition.TLSInputReadyCondition,
-				condition.ErrorReason,
-				condition.SeverityWarning,
-				condition.TLSInputErrorMessage,
-				err.Error()))
-			return ctrl.Result{}, err
-		}
-
-		if hash != "" {
-			configVars[tls.CABundleKey] = env.SetValue(hash)
-		}
-
-		// Validate API service certs secrets
-		certsHash, err := instance.Spec.Aodh.TLS.API.ValidateCertSecrets(ctx, helper, instance.Namespace)
-		if err != nil {
-			if k8s_errors.IsNotFound(err) {
-				instance.Status.Conditions.Set(condition.FalseCondition(
-					condition.TLSInputReadyCondition,
-					condition.RequestedReason,
-					condition.SeverityInfo,
-					condition.TLSInputReadyWaitingMessage, err.Error()))
-				return ctrl.Result{}, nil
-			}
-			instance.Status.Conditions.Set(condition.FalseCondition(
-				condition.TLSInputReadyCondition,
-				condition.ErrorReason,
-				condition.SeverityWarning,
-				condition.TLSInputErrorMessage,
-				err.Error()))
-			return ctrl.Result{}, err
-		}
-
-		configVars[tls.TLSHashName] = env.SetValue(certsHash)
-	}
-
 	// remove finalizers from unused MariaDBAccount records
 	err = mariadbv1.DeleteUnusedMariaDBAccountFinalizers(
 		ctx, helper, autoscaling.DatabaseCRName,
@@ -589,9 +523,6 @@ func (r *AutoscalingReconciler) reconcileNormalAodh(
 		return ctrl.Result{}, err
 	}
 
-	// all cert input checks out so report InputReady
-	instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage)
-
 	Log.Info("Reconciled Service Aodh successfully")
 	return ctrl.Result{}, nil
 }

internal/controller/autoscaling_controller.go

@@ -221,12 +221,11 @@ func (r *AutoscalingReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 // fields to index to reconcile when change
 const (
-	autoscalingPasswordSecretField     = ".spec.secret"
-	autoscalingCaBundleSecretNameField = ".spec.tls.caBundleSecretName" //nolint:gosec // G101: Not actual credentials, just field path
-	autoscalingTLSAPIInternalField     = ".spec.tls.api.internal.secretName"
-	autoscalingTLSAPIPublicField       = ".spec.tls.api.public.secretName"
-	autoscalingTLSField                = ".spec.tls.secretName"
-	topologyField                      = ".spec.topologyRef.Name"
+	autoscalingPasswordSecretField     = ".spec.aodh.secret"                 //nolint:gosec // G101: Not actual credentials, just field path
+	autoscalingCaBundleSecretNameField = ".spec.aodh.tls.caBundleSecretName" //nolint:gosec // G101: Not actual credentials, just field path
+	autoscalingTLSAPIInternalField     = ".spec.aodh.tls.api.internal.secretName"
+	autoscalingTLSAPIPublicField       = ".spec.aodh.tls.api.public.secretName"
+	topologyField                      = ".spec.aodh.topologyRef.Name"
 )
 
 var (
@@ -235,7 +234,6 @@ var (
 		autoscalingCaBundleSecretNameField,
 		autoscalingTLSAPIInternalField,
 		autoscalingTLSAPIPublicField,
-		autoscalingTLSField,
 		topologyField,
 	}
 )
@@ -538,7 +536,70 @@ func (r *AutoscalingReconciler) reconcileNormal(
 		return ctrl.Result{}, err
 	}
 
+	//
+	// TLS input validation
+	//
+	// Validate the CA cert secret if provided
+	if instance.Spec.Aodh.TLS.CaBundleSecretName != "" {
+		hash, err := tls.ValidateCACertSecret(
+			ctx,
+			helper.GetClient(),
+			types.NamespacedName{
+				Name:      instance.Spec.Aodh.TLS.CaBundleSecretName,
+				Namespace: instance.Namespace,
+			},
+		)
+		if err != nil {
+			if k8s_errors.IsNotFound(err) {
+				// Since the CA cert secret should have been manually created by the user and provided in the spec,
+				// we treat this as a warning because it means that the service will not be able to start.
+				instance.Status.Conditions.Set(condition.FalseCondition(
+					condition.TLSInputReadyCondition,
+					condition.ErrorReason,
+					condition.SeverityWarning,
+					condition.TLSInputReadyWaitingMessage, instance.Spec.Aodh.TLS.CaBundleSecretName))
+				return ctrl.Result{}, nil
+			}
+			instance.Status.Conditions.Set(condition.FalseCondition(
+				condition.TLSInputReadyCondition,
+				condition.ErrorReason,
+				condition.SeverityWarning,
+				condition.TLSInputErrorMessage,
+				err.Error()))
+			return ctrl.Result{}, err
+		}
+
+		if hash != "" {
+			configMapVars[tls.CABundleKey] = env.SetValue(hash)
+		}
+		// Validate API service certs secrets
+		certsHash, err := instance.Spec.Aodh.TLS.API.ValidateCertSecrets(ctx, helper, instance.Namespace)
+		if err != nil {
+			if k8s_errors.IsNotFound(err) {
+				instance.Status.Conditions.Set(condition.FalseCondition(
+					condition.TLSInputReadyCondition,
+					condition.RequestedReason,
+					condition.SeverityInfo,
+					condition.TLSInputReadyWaitingMessage, err.Error()))
+				return ctrl.Result{}, nil
+			}
+			instance.Status.Conditions.Set(condition.FalseCondition(
+				condition.TLSInputReadyCondition,
+				condition.ErrorReason,
+				condition.SeverityWarning,
+				condition.TLSInputErrorMessage,
+				err.Error()))
+			return ctrl.Result{}, err
+		}
+
+		configMapVars[tls.TLSHashName] = env.SetValue(certsHash)
+	}
+
+	// all cert input checks out so report InputReady
+	instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage)
+
 	inputHash, hashChanged, err := r.createHashOfInputHashes(ctx, instance, configMapVars)
+
 	if err != nil {
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.ServiceConfigReadyCondition,
@@ -886,6 +947,19 @@ func (r *AutoscalingReconciler) SetupWithManager(ctx context.Context, mgr ctrl.M
 		}
 		return nil
 	}
+
+	// index autoscalingPasswordSecretField
+	if err := mgr.GetFieldIndexer().IndexField(context.Background(), &telemetryv1.Autoscaling{}, autoscalingPasswordSecretField, func(rawObj client.Object) []string {
+		// Extract the secret name from the spec, if one is provided
+		cr := rawObj.(*telemetryv1.Autoscaling)
+		if cr.Spec.Aodh.Secret == "" {
+			return nil
+		}
+		return []string{cr.Spec.Aodh.Secret}
+	}); err != nil {
+		return err
+	}
+
 	// index autoscalingCaBundleSecretNameField
 	if err := mgr.GetFieldIndexer().IndexField(context.Background(), &telemetryv1.Autoscaling{}, autoscalingCaBundleSecretNameField, func(rawObj client.Object) []string {
 		// Extract the secret name from the spec, if one is provided
@@ -981,6 +1055,7 @@ func (r *AutoscalingReconciler) findObjectsForSrc(ctx context.Context, src clien
 		}
 		err := r.List(ctx, crList, listOps)
 		if err != nil {
+			Log.Error(err, fmt.Sprintf("listing %s for field: %s - %s", crList.GroupVersionKind().Kind, field, src.GetNamespace()))
 			return []reconcile.Request{}
 		}