Merge pull request #287 from abays/OSPRH-19982

[OSPRH-19982] Improve consistency of condition severity usage

Author: openshift-merge-bot[bot]

controllers/barbican_controller.go

@@ -251,6 +251,8 @@ func (r *BarbicanReconciler) reconcileNormal(ctx context.Context, instance *barb
 	instance.Status.TransportURLSecret = transportURL.Status.SecretName
 
 	if instance.Status.TransportURLSecret == "" {
+		// Since the TransportURL secret is automatically created by the Infra operator,
+		// we treat this as an info (because the user is not responsible for manually creating it).
 		Log.Info(fmt.Sprintf("Waiting for TransportURL %s secret to be created", transportURL.Name))
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			barbicanv1beta1.BarbicanRabbitMQTransportURLReadyCondition,
@@ -328,11 +330,13 @@ func (r *BarbicanReconciler) reconcileNormal(ctx context.Context, instance *barb
 		nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace)
 		if err != nil {
 			if k8s_errors.IsNotFound(err) {
+				// Since the net-attach-def CR should have been manually created by the user and referenced in the spec,
+				// we treat this as a warning because it means that the service will not be able to start.
 				Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt))
 				instance.Status.Conditions.Set(condition.FalseCondition(
 					condition.NetworkAttachmentsReadyCondition,
-					condition.RequestedReason,
-					condition.SeverityInfo,
+					condition.ErrorReason,
+					condition.SeverityWarning,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
 				return ctrl.Result{RequeueAfter: time.Second * 10}, nil
@@ -351,7 +355,6 @@ func (r *BarbicanReconciler) reconcileNormal(ctx context.Context, instance *barb
 		}
 	}
 
-	// TODO: _ should be serviceAnnotations
 	serviceAnnotations, err := nad.EnsureNetworksAnnotation(nadList)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed create network annotation from %s: %w",
@@ -1135,11 +1138,13 @@ func (r *BarbicanReconciler) verifySecret(
 			err.Error()))
 		return ctrl.Result{}, err
 	} else if (result != ctrl.Result{}) {
+		// We treat this as a warning because it means that the service will not be able to start
+		// while we are waiting for the secret to be created manually by the user.
 		Log.Info(fmt.Sprintf("OpenStack secret %s not found", secretName))
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,
-			condition.RequestedReason,
-			condition.SeverityInfo,
+			condition.ErrorReason,
+			condition.SeverityWarning,
 			condition.InputReadyWaitingMessage))
 		return result, nil
 	}

controllers/barbicanapi_controller.go

@@ -225,11 +225,27 @@ func (r *BarbicanAPIReconciler) verifySecret(
 			err.Error()))
 		return ctrl.Result{}, err
 	} else if (result != ctrl.Result{}) {
+		// This function is used in different contexts, but only one of them, for the transport URL secret,
+		// involves a secret that is automatically created elsewhere.  For the other contexts, we treat this
+		// as a warning because it means that the service will not be able to start while we are waiting for
+		// the secret to be created manually by the user.
+
+		var reason condition.Reason
+		var severity condition.Severity
+
+		if expectedFields != nil && slices.Contains(expectedFields, TransportURL) {
+			reason = condition.RequestedReason
+			severity = condition.SeverityInfo
+		} else {
+			reason = condition.ErrorReason
+			severity = condition.SeverityWarning
+		}
+
 		Log.Info(fmt.Sprintf("OpenStack secret %s not found", secretName))
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,
-			condition.RequestedReason,
-			condition.SeverityInfo,
+			reason,
+			severity,
 			condition.InputReadyWaitingMessage))
 		return result, nil
 	}
@@ -632,10 +648,12 @@ func (r *BarbicanAPIReconciler) reconcileNormal(ctx context.Context, instance *b
 		)
 		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.RequestedReason,
-					condition.SeverityInfo,
+					condition.ErrorReason,
+					condition.SeverityWarning,
 					condition.TLSInputReadyWaitingMessage,
 					instance.Spec.TLS.CaBundleSecretName))
 				return ctrl.Result{}, nil
@@ -659,6 +677,8 @@ func (r *BarbicanAPIReconciler) reconcileNormal(ctx context.Context, instance *b
 	certsHash, err := instance.Spec.TLS.API.ValidateCertSecrets(ctx, helper, instance.Namespace)
 	if err != nil {
 		if k8s_errors.IsNotFound(err) {
+			// Since the OpenStackControlPlane creates the API service certs secrets,
+			// we treat this as an info (because the user is not responsible for manually creating them).
 			instance.Status.Conditions.Set(condition.FalseCondition(
 				condition.TLSInputReadyCondition,
 				condition.RequestedReason,
@@ -710,11 +730,13 @@ func (r *BarbicanAPIReconciler) reconcileNormal(ctx context.Context, instance *b
 		nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace)
 		if err != nil {
 			if k8s_errors.IsNotFound(err) {
+				// Since the net-attach-def CR should have been manually created by the user and referenced in the spec,
+				// we treat this as a warning because it means that the service will not be able to start.
 				Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt))
 				instance.Status.Conditions.Set(condition.FalseCondition(
 					condition.NetworkAttachmentsReadyCondition,
-					condition.RequestedReason,
-					condition.SeverityInfo,
+					condition.ErrorReason,
+					condition.SeverityWarning,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
 				return ctrl.Result{RequeueAfter: time.Second * 10}, nil

controllers/barbicankeystonelistener_controller.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+	"slices"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -203,11 +204,27 @@ func (r *BarbicanKeystoneListenerReconciler) verifySecret(
 			err.Error()))
 		return ctrl.Result{}, err
 	} else if (result != ctrl.Result{}) {
+		// This function is used in different contexts, but only one of them, for the transport URL secret,
+		// involves a secret that is automatically created elsewhere.  For the other contexts, we treat this
+		// as a warning because it means that the service will not be able to start while we are waiting for
+		// the secret to be created manually by the user.
+
+		var reason condition.Reason
+		var severity condition.Severity
+
+		if expectedFields != nil && slices.Contains(expectedFields, TransportURL) {
+			reason = condition.RequestedReason
+			severity = condition.SeverityInfo
+		} else {
+			reason = condition.ErrorReason
+			severity = condition.SeverityWarning
+		}
+
 		Log.Info(fmt.Sprintf("OpenStack secret %s not found", secretName))
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,
-			condition.RequestedReason,
-			condition.SeverityInfo,
+			reason,
+			severity,
 			condition.InputReadyWaitingMessage))
 		return result, nil
 	}
@@ -421,10 +438,12 @@ func (r *BarbicanKeystoneListenerReconciler) reconcileNormal(ctx context.Context
 		)
 		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.RequestedReason,
-					condition.SeverityInfo,
+					condition.ErrorReason,
+					condition.SeverityWarning,
 					condition.TLSInputReadyWaitingMessage,
 					instance.Spec.TLS.CaBundleSecretName))
 				return ctrl.Result{}, nil
@@ -497,11 +516,13 @@ func (r *BarbicanKeystoneListenerReconciler) reconcileNormal(ctx context.Context
 		nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace)
 		if err != nil {
 			if k8s_errors.IsNotFound(err) {
+				// Since the net-attach-def CR should have been manually created by the user and referenced in the spec,
+				// we treat this as a warning because it means that the service will not be able to start.
 				Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt))
 				instance.Status.Conditions.Set(condition.FalseCondition(
 					condition.NetworkAttachmentsReadyCondition,
-					condition.RequestedReason,
-					condition.SeverityInfo,
+					condition.ErrorReason,
+					condition.SeverityWarning,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
 				return ctrl.Result{RequeueAfter: time.Second * 10}, nil

controllers/barbicanworker_controller.go

@@ -195,11 +195,27 @@ func (r *BarbicanWorkerReconciler) verifySecret(
 			err.Error()))
 		return ctrl.Result{}, err
 	} else if (result != ctrl.Result{}) {
+		// This function is used in different contexts, but only one of them, for the transport URL secret,
+		// involves a secret that is automatically created elsewhere.  For the other contexts, we treat this
+		// as a warning because it means that the service will not be able to start while we are waiting for
+		// the secret to be created manually by the user.
+
+		var reason condition.Reason
+		var severity condition.Severity
+
+		if expectedFields != nil && slices.Contains(expectedFields, TransportURL) {
+			reason = condition.RequestedReason
+			severity = condition.SeverityInfo
+		} else {
+			reason = condition.ErrorReason
+			severity = condition.SeverityWarning
+		}
+
 		Log.Info(fmt.Sprintf("OpenStack secret %s not found", secretName))
 		instance.Status.Conditions.Set(condition.FalseCondition(
 			condition.InputReadyCondition,
-			condition.RequestedReason,
-			condition.SeverityInfo,
+			reason,
+			severity,
 			condition.InputReadyWaitingMessage))
 		return result, nil
 	}
@@ -419,10 +435,12 @@ func (r *BarbicanWorkerReconciler) reconcileNormal(ctx context.Context, instance
 		)
 		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.RequestedReason,
-					condition.SeverityInfo,
+					condition.ErrorReason,
+					condition.SeverityWarning,
 					condition.TLSInputReadyWaitingMessage,
 					instance.Spec.TLS.CaBundleSecretName))
 				return ctrl.Result{}, nil
@@ -495,11 +513,13 @@ func (r *BarbicanWorkerReconciler) reconcileNormal(ctx context.Context, instance
 		nad, err := nad.GetNADWithName(ctx, helper, netAtt, instance.Namespace)
 		if err != nil {
 			if k8s_errors.IsNotFound(err) {
+				// Since the net-attach-def CR should have been manually created by the user and referenced in the spec,
+				// we treat this as a warning because it means that the service will not be able to start.
 				Log.Info(fmt.Sprintf("network-attachment-definition %s not found", netAtt))
 				instance.Status.Conditions.Set(condition.FalseCondition(
 					condition.NetworkAttachmentsReadyCondition,
-					condition.RequestedReason,
-					condition.SeverityInfo,
+					condition.ErrorReason,
+					condition.SeverityWarning,
 					condition.NetworkAttachmentsReadyWaitingMessage,
 					netAtt))
 				return ctrl.Result{RequeueAfter: time.Second * 10}, nil