Merge branch 'main' into PTEUDO-2363

internal/controller/databaseclaim_controller_test.go

@@ -19,10 +19,9 @@ package controller
 import (
 	"context"
 	"fmt"
-	"net/url"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"net/url"
 
 	crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1"
 	corev1 "k8s.io/api/core/v1"
@@ -125,7 +124,7 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			secret := &corev1.Secret{}
 			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
 			// this secret is created without resource owner, so it does not get deleted after dbclain is deleted
-			Expect(k8sClient.Delete(ctx, secret)).To(Succeed())
+			k8sClient.Delete(ctx, secret)
 		})
 
 		It("Should succeed to reconcile DB Claim missing dbVersion", func() {
@@ -137,6 +136,17 @@ var _ = Describe("DatabaseClaim Controller", func() {
 
 			By("Reconciling the created resource")
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Mocking crossplane instance readiness")
+			hostParams, err := hostparams.New(controllerReconciler.Config.Viper, resource)
+			Expect(err).ToNot(HaveOccurred())
+			awsRes := &crossplaneaws.DBInstance{}
+			awsRes.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctx, k8sClient, awsRes)).NotTo(HaveOccurred())
+
+			By("Reconciling the created resource")
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).NotTo(HaveOccurred())
 
 			By("Verify that the DB Claim has active DB with default Version")
@@ -169,6 +179,16 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(claim.Status.Error).To(Equal(""))
 
+			By("Mocking crossplane instance readiness")
+			hostParams, err := hostparams.New(controllerReconciler.Config.Viper, claim)
+			Expect(err).ToNot(HaveOccurred())
+			awsRes := &crossplaneaws.DBInstance{}
+			awsRes.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctx, k8sClient, awsRes)).NotTo(HaveOccurred())
+
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
 			By("Checking the user credentials secret")
 			secret := &corev1.Secret{}
 			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
@@ -203,8 +223,17 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(claim.Status.Error).To(Equal(""))
 
-			By("Checking the user credentials secret")
+			By("Mocking crossplane instance readiness")
+			hostParams, err := hostparams.New(controllerReconciler.Config.Viper, &claim)
+			Expect(err).ToNot(HaveOccurred())
+			awsRes := &crossplaneaws.DBInstance{}
+			awsRes.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctx, k8sClient, awsRes)).NotTo(HaveOccurred())
 
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("Checking the user credentials secret")
 			secret := &corev1.Secret{}
 			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
 			Expect(err).NotTo(HaveOccurred())
@@ -239,8 +268,17 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(claim.Status.Error).To(Equal(""))
 
-			By("Checking the user credentials secret")
+			By("Mocking crossplane instance readiness")
+			hostParams, err := hostparams.New(controllerReconciler.Config.Viper, &claim)
+			Expect(err).ToNot(HaveOccurred())
+			awsRes := &crossplaneaws.DBInstance{}
+			awsRes.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctx, k8sClient, awsRes)).NotTo(HaveOccurred())
 
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
+
+			By("Checking the user credentials secret")
 			secret := &corev1.Secret{}
 			err = k8sClient.Get(ctx, typeNamespacedSecretName, secret)
 			Expect(err).NotTo(HaveOccurred())
@@ -287,6 +325,13 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			Expect(err).ToNot(HaveOccurred())
 			Expect(resource.Status.Error).To(BeEmpty())
 
+			By("Mocking crossplane instance readiness")
+			awsRes := &crossplaneaws.DBInstance{}
+			awsRes.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctx, k8sClient, awsRes)).NotTo(HaveOccurred())
+
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+
 			By(fmt.Sprintf("Verifying that the DBInstance is created: %s", credSecretName))
 			var instance crossplaneaws.DBInstance
 			Eventually(func() error {
@@ -339,9 +384,14 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).NotTo(HaveOccurred())
 
+			By("Mocking crossplane instance readiness")
+			hostParams, err := hostparams.New(controllerReconciler.Config.Viper, resource)
+			Expect(err).ToNot(HaveOccurred())
+			awsRes := &crossplaneaws.DBInstance{}
+			awsRes.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctx, k8sClient, awsRes)).NotTo(HaveOccurred())
+
 			var instance crossplaneaws.DBInstance
-			viper := controllerReconciler.Config.Viper
-			hostParams, err := hostparams.New(viper, resource)
 			Expect(err).ToNot(HaveOccurred())
 			instanceName := fmt.Sprintf("%s-%s-%s", env, resourceName, hostParams.Hash())
 
@@ -355,19 +405,6 @@ var _ = Describe("DatabaseClaim Controller", func() {
 
 		})
 
-		It("Should fail to reconcile a newDB if secret is present", func() {
-			By(fmt.Sprintf("creating secret: %s", secretName))
-			secret := &corev1.Secret{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      secretName,
-					Namespace: "default",
-				},
-			}
-			Expect(k8sClient.Create(ctx, secret)).To(Succeed())
-			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
-			Expect(err).To(HaveOccurred())
-		})
-
 		It("Reconcile rotates the username", func() {
 			By("Updating CR with a DB Version")
 
@@ -378,9 +415,19 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).NotTo(HaveOccurred())
 			Expect(resource.Spec.DBVersion).To(Equal(""))
 
-			By("Rotating to UserSuffixA")
+			By("Mocking crossplane instance readiness")
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).NotTo(HaveOccurred())
+
+			hostParams, err := hostparams.New(controllerReconciler.Config.Viper, resource)
+			Expect(err).ToNot(HaveOccurred())
+			awsRes := &crossplaneaws.DBInstance{}
+			awsRes.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctx, k8sClient, awsRes)).NotTo(HaveOccurred())
+
+			By("Rotating to UserSuffixA")
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
 			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).NotTo(HaveOccurred())
 			Expect(resource.Status.Error).To(Equal(""))
 			Expect(resource.Status.ActiveDB.ConnectionInfo.Username).To(Equal("postgres_a"))
@@ -408,9 +455,19 @@ var _ = Describe("DatabaseClaim Controller", func() {
 			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).NotTo(HaveOccurred())
 			Expect(resource.Spec.DBVersion).To(Equal(""))
 
-			By("Rotating to UserSuffixA")
+			By("Mocking crossplane instance readiness")
 			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).NotTo(HaveOccurred())
+
+			hostParams, err := hostparams.New(controllerReconciler.Config.Viper, resource)
+			Expect(err).ToNot(HaveOccurred())
+			awsRes := &crossplaneaws.DBInstance{}
+			awsRes.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctx, k8sClient, awsRes)).NotTo(HaveOccurred())
+
+			By("Rotating to UserSuffixA")
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName})
+			Expect(err).NotTo(HaveOccurred())
 			Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).NotTo(HaveOccurred())
 			Expect(resource.Status.Error).To(Equal(""))
 			Expect(resource.Status.ActiveDB.ConnectionInfo.Username).To(Equal("postgres_a"))

internal/controller/databasecontroller_migrate_test.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1"
 	"net/url"
 	"strings"
 	"time"
@@ -258,14 +259,21 @@ var _ = Describe("claim migrate", func() {
 			dbc.Spec.UseExistingSource = ptr.To(false)
 			Expect(k8sClient.Update(ctxLogger, &dbc)).NotTo(HaveOccurred())
 			res, err := controllerReconciler.Reconcile(ctxLogger, reconcile.Request{NamespacedName: typeNamespacedName})
+
+			hostParams, err = hostparams.New(controllerReconciler.Config.Viper, &dbc)
+			Expect(err).ToNot(HaveOccurred())
+			resAws := &crossplaneaws.DBInstance{}
+			resAws.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctxLogger, k8sClient, resAws)).NotTo(HaveOccurred())
+			res, err = controllerReconciler.Reconcile(ctxLogger, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).To(BeNil())
 			Expect(dbc.Status.Error).To(Equal(""))
 			Expect(res.Requeue).To(BeTrue())
 			Expect(k8sClient.Get(ctxLogger, typeNamespacedName, &dbc)).NotTo(HaveOccurred())
 			By("Check source DSN looks roughly correct")
 			activeDB := dbc.Status.ActiveDB
 			Expect(activeDB.ConnectionInfo).NotTo(BeNil())
-			compareDSN := strings.Replace(testDSN, "//postgres:postgres", fmt.Sprintf("//%s_b:", migratedowner), 1)
+			compareDSN := strings.Replace(testDSN, "//postgres:postgres", fmt.Sprintf("//%s_a:", migratedowner), 1)
 			Expect(activeDB.ConnectionInfo.Uri()).To(Equal(compareDSN))
 
 			By("Check target DSN looks roughly correct")
@@ -346,6 +354,14 @@ var _ = Describe("claim migrate", func() {
 			dbc.Spec.UseExistingSource = ptr.To(false)
 			Expect(k8sClient.Update(ctxLogger, &dbc)).NotTo(HaveOccurred())
 
+			By("Reconciling once for transitioning the db instance to ready")
+			_, err = controllerReconciler.Reconcile(ctxLogger, reconcile.Request{NamespacedName: typeNamespacedName})
+			hostParams, err = hostparams.New(controllerReconciler.Config.Viper, &dbc)
+			Expect(err).ToNot(HaveOccurred())
+			resAws := &crossplaneaws.DBInstance{}
+			resAws.SetName(env + "-" + resourceName + "-" + hostParams.Hash())
+			Expect(patchCrossplaneCRReadiness(ctxLogger, k8sClient, resAws)).NotTo(HaveOccurred())
+
 			By("Reconciling again to trigger migration failure")
 			_, err = controllerReconciler.Reconcile(ctxLogger, reconcile.Request{NamespacedName: typeNamespacedName})
 			Expect(err).To(HaveOccurred())

internal/controller/suite_test.go

@@ -17,8 +17,12 @@ limitations under the License.
 package controller
 
 import (
+	"context"
 	"database/sql"
+	"encoding/json"
 	"fmt"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"path/filepath"
 	"runtime"
 	"testing"
@@ -177,3 +181,27 @@ var _ = AfterSuite(func() {
 	err := testEnv.Stop()
 	Expect(err).NotTo(HaveOccurred())
 })
+
+func patchCrossplaneCRReadiness(ctx context.Context, k8sClient client.Client, resource client.Object) error {
+	now := metav1.Now()
+	patchData := map[string]interface{}{
+		"status": map[string]interface{}{
+			"conditions": []map[string]interface{}{
+				{
+					"type":               "Ready",
+					"status":             "True",
+					"reason":             "Reconciled",
+					"message":            "Resource is ready",
+					"lastTransitionTime": now.Format(time.RFC3339),
+				},
+			},
+		},
+	}
+
+	patchBytes, err := json.Marshal(patchData)
+	if err != nil {
+		return err
+	}
+
+	return k8sClient.Status().Patch(ctx, resource, client.RawPatch(types.MergePatchType, patchBytes))
+}

pkg/databaseclaim/databaseclaim.go

@@ -9,11 +9,13 @@ import (
 	"strings"
 	"time"
 
+	crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1"
 	xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1"
 	"github.com/go-logr/logr"
 	_ "github.com/lib/pq"
 	gopassword "github.com/sethvargo/go-password/password"
 	"github.com/spf13/viper"
+	crossplanegcp "github.com/upbound/provider-gcp/apis/alloydb/v1beta2"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/ptr"
@@ -246,7 +248,7 @@ func (r *DatabaseClaimReconciler) createMetricsDeployment(ctx context.Context, d
 	cfg.DepYamlPath = r.Config.MetricsDepYamlPath
 	cfg.ConfigYamlPath = r.Config.MetricsConfigYamlPath
 	cfg.DatasourceSecretName = dbClaim.Spec.SecretName
-	cfg.DatasourceFileName = v1.DSNKey
+	cfg.DatasourceFileName = v1.DSNURIKey
 	return exporter.Apply(ctx, r.Client, cfg)
 }
 
@@ -825,8 +827,6 @@ func (r *DatabaseClaimReconciler) reconcileMigrationInProgress(ctx context.Conte
 		ExportFilePath:   basefun.GetPgTempFolder(r.Config.Viper),
 	}
 
-	logr.V(debugLevel).Info("pctl_dsn", "config", config)
-
 	s, err := pgctl.GetReplicatorState(migrationState, config)
 	if err != nil {
 		return r.statusManager.SetError(ctx, dbClaim, err)
@@ -1102,10 +1102,11 @@ func (r *DatabaseClaimReconciler) getPasswordRotationTime() time.Duration {
 
 // FindStatusCondition finds the conditionType in conditions.
 func (r *DatabaseClaimReconciler) isResourceReady(typ, name string, resourceStatus xpv1.ResourceStatus) (bool, error) {
-	if ok, err := isResourceReady(resourceStatus); err != nil {
-		return ok, fmt.Errorf("%s %s: %w", typ, name, err)
+	ok, err := isResourceReady(resourceStatus)
+	if err != nil {
+		return false, fmt.Errorf("%s %s: %w", typ, name, err)
 	}
-	return true, nil
+	return ok, nil
 }
 
 func isResourceReady(resourceStatus xpv1.ResourceStatus) (bool, error) {