Merge pull request #389 from raptorsun/status-secret-missing

OLS-881  CRD status points out missing secrets for LLM provider and service TLS certs

internal/controller/errors.go

@@ -36,6 +36,8 @@ const (
 	ErrGetConsolePluginConfigMap       = "failed to get Console Plugin configmap"
 	ErrGetConsolePluginDeployment      = "failed to get Console Plugin deployment"
 	ErrGetConsolePluginService         = "failed to get Console Plugin service"
+	ErrGetLLMSecret                    = "failed to get LLM provider secret" // #nosec G101
+	ErrGetTLSSecret                    = "failed to get TLS secret"          // #nosec G101
 	ErrGetSARClusterRole               = "failed to get SAR cluster role"
 	ErrGetSARClusterRoleBinding        = "failed to get SAR cluster role binding"
 	ErrGetServiceMonitor               = "failed to get ServiceMonitor"

internal/controller/helpers_test.go

@@ -0,0 +1,19 @@
+package controller
+
+import (
+	olsv1alpha1 "github.com/openshift/lightspeed-operator/api/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func statusHasCondition(status olsv1alpha1.OLSConfigStatus, condition metav1.Condition) bool {
+	// ignore ObservedGeneration and LastTransitionTime
+	for _, c := range status.Conditions {
+		if c.Type == condition.Type &&
+			c.Status == condition.Status &&
+			c.Reason == condition.Reason &&
+			c.Message == condition.Message {
+			return true
+		}
+	}
+	return false
+}

internal/controller/ols_app_server_reconciliator.go

@@ -277,6 +277,7 @@ func (r *OLSConfigReconciler) reconcileLLMSecrets(ctx context.Context, cr *olsv1
 		foundSecret := &corev1.Secret{}
 		secretValues, err := getAllSecretContent(r.Client, provider.CredentialsSecretRef.Name, r.Options.Namespace, foundSecret)
 		if err != nil {
+			r.updateStatusCondition(ctx, cr, typeApiReady, false, ErrGetLLMSecret, err)
 			return fmt.Errorf("Secret token not found for provider: %s. error: %w", provider.Name, err)
 		}
 		for key, value := range secretValues {
@@ -376,7 +377,8 @@ func (r *OLSConfigReconciler) reconcileTLSSecret(ctx context.Context, cr *olsv1a
 		return true, nil
 	})
 	if err != nil {
-		return fmt.Errorf("failed to get TLS key and cert - wait err %w; last error: %w", err, lastErr)
+		r.updateStatusCondition(ctx, cr, typeApiReady, false, fmt.Sprintf("%s - %s", ErrGetTLSSecret, OLSCertsSecretName), err)
+		return fmt.Errorf("%s -%s - wait err %w; last error: %w", ErrGetTLSSecret, OLSCertsSecretName, err, lastErr)
 	}
 
 	annotateSecretWatcher(foundSecret)

internal/controller/ols_app_server_reconciliator_test.go

@@ -10,6 +10,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -364,7 +365,7 @@ var _ = Describe("App server reconciliator", Ordered, func() {
 
 	})
 
-	Context("Creation logic", Ordered, func() {
+	Context("Referred Secrets", Ordered, func() {
 		var secret *corev1.Secret
 		var tlsSecret *corev1.Secret
 		BeforeEach(func() {
@@ -401,7 +402,11 @@ var _ = Describe("App server reconciliator", Ordered, func() {
 			Expect(secretDeletionErr).NotTo(HaveOccurred())
 			By("Delete the tls secret")
 			secretDeletionErr = reconciler.Delete(ctx, tlsSecret)
-			Expect(secretDeletionErr).NotTo(HaveOccurred())
+			if secretDeletionErr != nil {
+				Expect(errors.IsNotFound(secretDeletionErr)).To(BeTrue())
+			} else {
+				Expect(secretDeletionErr).NotTo(HaveOccurred())
+			}
 		})
 
 		It("should reconcile from OLSConfig custom resource", func() {
@@ -449,5 +454,41 @@ var _ = Describe("App server reconciliator", Ordered, func() {
 			secretDeletionErr := reconciler.Delete(ctx, secret)
 			Expect(secretDeletionErr).NotTo(HaveOccurred())
 		})
+
+		It("should return error when the LLM provider token secret is not found", func() {
+			By("Reconcile after modifying the token secret")
+			originalSecretName := cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name
+			cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef = corev1.LocalObjectReference{Name: "non-existing-secret"}
+			err := reconciler.reconcileLLMSecrets(ctx, cr)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("secret not found: non-existing-secret"))
+			Expect(statusHasCondition(cr.Status, metav1.Condition{
+				Type:    typeApiReady,
+				Status:  metav1.ConditionFalse,
+				Reason:  "Reconciling",
+				Message: "failed to get LLM provider secret: secret not found: non-existing-secret. error: secrets \"non-existing-secret\" not found",
+			})).To(BeTrue())
+			cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef = corev1.LocalObjectReference{Name: originalSecretName}
+		})
+
+		It("should return error when the TLS secret is not found", func() {
+			By("reconcile TLS secret")
+			err := reconciler.reconcileTLSSecret(ctx, cr)
+			Expect(err).NotTo(HaveOccurred())
+
+			By("Delete the tls secret and reconcile again")
+			err = reconciler.Delete(ctx, tlsSecret)
+			Expect(err).NotTo(HaveOccurred())
+			err = reconciler.reconcileTLSSecret(ctx, cr)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("failed to get TLS secret"))
+			Expect(statusHasCondition(cr.Status, metav1.Condition{
+				Type:    typeApiReady,
+				Status:  metav1.ConditionFalse,
+				Reason:  "Reconciling",
+				Message: "failed to get TLS secret - lightspeed-tls: context deadline exceeded",
+			})).To(BeTrue())
+		})
+
 	})
 })