Follow-up changes for WatcherAPI from PR 13

Following the review on PR 13, there were a few changes suggested. This
PR does 2 of them:

1. Make the Secret field on the Spec required, and remove the default
   from WatcherAPI. This means that the field is no longer shared
   between the Watcher and WatcherAPI kinds.

2. Add more functional tests to test scenarios where the secret or
   database are not present.

To test the change 1, this commit also changes the watcherapi kuttl test
to explicitely create a Secret, mimicking what we'll later do via the
Watcher controller.

api/bases/watcher.openstack.org_watcherapis.yaml

@@ -62,11 +62,11 @@ spec:
                     type: string
                 type: object
               secret:
-                default: osp-secret
                 description: Secret containing all passwords / keys needed
                 type: string
             required:
             - databaseInstance
+            - secret
             type: object
           status:
             description: WatcherAPIStatus defines the observed state of WatcherAPI

api/v1beta1/common_types.go

@@ -18,11 +18,6 @@ package v1beta1
 
 // WatcherCommon defines a spec based reusable for all the CRDs
 type WatcherCommon struct {
-	// +kubebuilder:validation:Optional
-	// +kubebuilder:default=osp-secret
-	// Secret containing all passwords / keys needed
-	Secret string `json:"secret"`
-
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default={service: WatcherPassword,}
 	// PasswordSelectors - Selectors to identify the ServiceUser password from the Secret
@@ -50,6 +45,11 @@ type WatcherTemplate struct {
 	// RabbitMQ instance name
 	// Needed to request a transportURL that is created and used in Barbican
 	RabbitMqClusterName string `json:"rabbitMqClusterName"`
+
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:default=osp-secret
+	// Secret containing all passwords / keys needed
+	Secret string `json:"secret"`
 }
 
 // PasswordSelector to identify the DB and AdminUser password from the Secret

api/v1beta1/watcherapi_types.go

@@ -27,6 +27,9 @@ type WatcherAPISpec struct {
 	// Important: Run "make" to regenerate code after modifying this file
 
 	WatcherCommon `json:",inline"`
+	// +kubebuilder:validation:Required
+	// Secret containing all passwords / keys needed
+	Secret string `json:"secret"`
 }
 
 // WatcherAPIStatus defines the observed state of WatcherAPI

config/crd/bases/watcher.openstack.org_watcherapis.yaml

@@ -62,11 +62,11 @@ spec:
                     type: string
                 type: object
               secret:
-                default: osp-secret
                 description: Secret containing all passwords / keys needed
                 type: string
             required:
             - databaseInstance
+            - secret
             type: object
           status:
             description: WatcherAPIStatus defines the observed state of WatcherAPI

config/samples/watcher_v1beta1_watcherapi.yaml

@@ -7,3 +7,4 @@ metadata:
   name: watcherapi-sample
 spec:
   databaseInstance: "openstack"
+  secret: "osp-secret"

controllers/watcherapi_controller.go

@@ -152,11 +152,11 @@ func (r *WatcherAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	db, err := mariadbv1.GetDatabaseByNameAndAccount(ctx, helper, watcher.DatabaseCRName, instance.Spec.DatabaseAccount, instance.Namespace)
 	if err != nil {
 		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.ServiceConfigReadyCondition,
+			condition.InputReadyCondition,
 			condition.ErrorReason,
 			condition.SeverityWarning,
-			condition.ServiceConfigReadyErrorMessage,
-			err.Error()))
+			condition.InputReadyErrorMessage,
+			fmt.Sprintf("couldn't get database %s and account %s", watcher.DatabaseCRName, instance.Spec.DatabaseAccount)))
 		return ctrl.Result{}, err
 	}
 

tests/functional/watcherapi_controller_test.go

@@ -11,13 +11,21 @@ import (
 	. "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers"
 	mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
 	watcherv1beta1 "github.com/openstack-k8s-operators/watcher-operator/api/v1beta1"
+	"github.com/openstack-k8s-operators/watcher-operator/pkg/watcher"
 	corev1 "k8s.io/api/core/v1"
 )
 
+var (
+	MinimalWatcherAPISpec = map[string]interface{}{
+		"secret":           "osp-secret",
+		"databaseInstance": "openstack",
+	}
+)
+
 var _ = Describe("WatcherAPI controller with minimal spec values", func() {
 	When("A Watcher instance is created from minimal spec", func() {
 		BeforeEach(func() {
-			DeferCleanup(th.DeleteInstance, CreateWatcherAPI(watcherTest.Instance, MinimalWatcherSpec))
+			DeferCleanup(th.DeleteInstance, CreateWatcherAPI(watcherTest.Instance, MinimalWatcherAPISpec))
 		})
 
 		It("should have the Spec fields defaulted", func() {
@@ -173,4 +181,59 @@ var _ = Describe("WatcherAPI controller", func() {
 			)
 		})
 	})
+	When("A WatcherAPI instance without secret is created", func() {
+		BeforeEach(func() {
+			DeferCleanup(th.DeleteInstance, CreateWatcherAPI(watcherTest.Instance, GetDefaultWatcherAPISpec()))
+		})
+		It("is missing the secret", func() {
+			th.ExpectConditionWithDetails(
+				watcherTest.Instance,
+				ConditionGetterFunc(WatcherAPIConditionGetter),
+				condition.InputReadyCondition,
+				corev1.ConditionFalse,
+				condition.RequestedReason,
+				condition.InputReadyWaitingMessage,
+			)
+		})
+	})
+	When("A WatcherAPI instance without a database but with a secret is created", func() {
+		BeforeEach(func() {
+			secret := th.CreateSecret(
+				watcherTest.InternalTopLevelSecretName,
+				map[string][]byte{
+					"WatcherPassword": []byte("service-password"),
+				},
+			)
+			DeferCleanup(k8sClient.Delete, ctx, secret)
+			DeferCleanup(th.DeleteInstance, CreateWatcherAPI(watcherTest.Instance, GetDefaultWatcherAPISpec()))
+		})
+		It("should have input not ready", func() {
+			WatcherAPI := GetWatcherAPI(watcherTest.Instance)
+			customErrorString := fmt.Sprintf(
+				"couldn't get database %s and account %s",
+				watcher.DatabaseCRName,
+				WatcherAPI.Spec.DatabaseAccount,
+			)
+			errorString := fmt.Sprintf(
+				condition.InputReadyErrorMessage,
+				customErrorString,
+			)
+			th.ExpectConditionWithDetails(
+				watcherTest.Instance,
+				ConditionGetterFunc(WatcherAPIConditionGetter),
+				condition.InputReadyCondition,
+				corev1.ConditionFalse,
+				condition.ErrorReason,
+				errorString,
+			)
+		})
+		It("should have config service unknown", func() {
+			th.ExpectCondition(
+				watcherTest.Instance,
+				ConditionGetterFunc(WatcherAPIConditionGetter),
+				condition.ServiceConfigReadyCondition,
+				corev1.ConditionUnknown,
+			)
+		})
+	})
 })

tests/kuttl/test-suites/default/watcher-api/01-cleanup-watcherapi.yaml

@@ -4,3 +4,6 @@ delete:
 - apiVersion: watcher.openstack.org/v1beta1
   kind: WatcherAPI
   name: watcherapi-kuttl
+- apiVersion: v1
+  kind: Secret
+  name: watcherapi-secret

tests/kuttl/test-suites/default/watcher-api/03-assert.yaml

@@ -9,7 +9,7 @@ spec:
   databaseInstance: openstack
   passwordSelectors:
     service: WatcherPassword
-  secret: osp-secret
+  secret: watcherapi-secret
 status:
   conditions:
   - message: Setup complete

tests/kuttl/test-suites/default/watcher-api/03-deploy-watcher-api.yaml

@@ -1,6 +1,15 @@
+apiVersion: v1
+kind: Secret
+metadata:
+  name: watcherapi-secret
+type: Opaque
+stringData:
+  WatcherPassword: password
+---
 apiVersion: watcher.openstack.org/v1beta1
 kind: WatcherAPI
 metadata:
   name: watcherapi-kuttl
 spec:
-  databaseInstance: "openstack"
+  databaseInstance: openstack
+  secret: watcherapi-secret