diff --git a/Jenkinsfile b/Jenkinsfile index ab8ede84..ff6e05c9 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -18,6 +18,13 @@ pipeline { sudo apt-get update if ! which psql > /dev/null; then + + + timeout 300 bash -c -- 'while sudo fuser /var/lib/dpkg/lock-frontend > /dev/null 2>&1 + do + echo "Waiting to get lock /var/lib/dpkg/lock-frontend..." + sleep 5 + done' sudo apt-get install -y postgresql-client-14 fi diff --git a/Makefile b/Makefile index eb9c59b4..9c2882fa 100644 --- a/Makefile +++ b/Makefile @@ -70,7 +70,7 @@ test: manifests generate fmt vet envtest ## Run tests. .PHONY: test-e2e # Run the e2e tests against a Kind k8s instance that is spun up. test-e2e: NS?=$(shell cat .id) test-e2e: - NAMESPACE=$(NS) go test ./test/e2e/ -v -ginkgo.v + NAMESPACE=$(NS) ENV=box-3 go test ./test/e2e/ -v -ginkgo.v .PHONY: lint lint: golangci-lint ## Run golangci-lint linter diff --git a/Makefile.infoblox b/Makefile.infoblox index 74a844c5..52ece672 100644 --- a/Makefile.infoblox +++ b/Makefile.infoblox @@ -93,7 +93,7 @@ push-images: docker-push-db-controller docker-push-dbproxy docker-push-dsnexec ${HELM_SETFLAGS} # Consider removing this, since we dont actually no # helm upgrade is applied in the cluster - @touch $@ + #@touch $@ deploy: package-chart-db-controller .deploy-$(GIT_COMMIT) diff --git a/cmd/config/config.yaml b/cmd/config/config.yaml index febeca3e..b595ceb0 100644 --- a/cmd/config/config.yaml +++ b/cmd/config/config.yaml @@ -31,13 +31,12 @@ passwordConfig: minPasswordLength: 15 passwordRotationPeriod: 60 sample-connection: - masterUsername: root + masterUsername: postgres username: postgres host: localhost port: 5432 sslMode: disable passwordSecretRef: postgres-postgresql - passwordSecretKey: postgresql-password # host omitted, allocates database dynamically dynamic-connection: masterUsername: root diff --git a/cmd/main.go b/cmd/main.go index 6eb9b1f2..4944cfed 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -189,12 +189,13 @@ func main() { } dbClaimConfig := &databaseclaim.DatabaseClaimConfig{ - Viper: ctlConfig, - + Viper: ctlConfig, + Namespace: os.Getenv("SERVICE_NAMESPACE"), Class: class, DbIdentifierPrefix: dbIdentifierPrefix, // Log: ctrl.Log.WithName("controllers").WithName("DatabaseClaim").V(controllers.InfoLevel), MasterAuth: rdsauth.NewMasterAuth(), + MetricsEnabled: true, MetricsDepYamlPath: metricsDepYamlPath, MetricsConfigYamlPath: metricsConfigYamlPath, } diff --git a/internal/controller/databaseclaim_controller.go b/internal/controller/databaseclaim_controller.go index 38f28f4e..62069f24 100644 --- a/internal/controller/databaseclaim_controller.go +++ b/internal/controller/databaseclaim_controller.go @@ -62,13 +62,17 @@ func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reques return r.reconciler.Reconcile(ctx, req) } -// SetupWithManager sets up the controller with the Manager. -func (r *DatabaseClaimReconciler) SetupWithManager(mgr ctrl.Manager) error { - +func (r *DatabaseClaimReconciler) Setup() { r.reconciler = &databaseclaim.DatabaseClaimReconciler{ Client: r.Client, Config: r.Config, } +} + +// SetupWithManager sets up the controller with the Manager. +func (r *DatabaseClaimReconciler) SetupWithManager(mgr ctrl.Manager) error { + + r.Setup() return ctrl.NewControllerManagedBy(mgr). For(&persistancev1.DatabaseClaim{}). diff --git a/internal/controller/databaseclaim_controller_test.go b/internal/controller/databaseclaim_controller_test.go index d6736bc5..d5eaa673 100644 --- a/internal/controller/databaseclaim_controller_test.go +++ b/internal/controller/databaseclaim_controller_test.go @@ -17,8 +17,25 @@ limitations under the License. package controller import ( + "context" + "net/url" + "path/filepath" + "testing" + "time" + . "github.com/onsi/ginkgo/v2" - //. "github.com/onsi/gomega" + . "github.com/onsi/gomega" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + persistancev1 "github.com/infobloxopen/db-controller/api/v1" + "github.com/infobloxopen/db-controller/pkg/config" + "github.com/infobloxopen/db-controller/pkg/databaseclaim" ) var _ = Describe("DatabaseClaim Controller", func() { @@ -78,3 +95,148 @@ var _ = Describe("DatabaseClaim Controller", func() { }) }) + +var _ = Describe("db-controller", func() { + + // Define utility constants for object names and testing timeouts/durations and intervals. + + Context("When updating DB Claim Status", func() { + + const resourceName = "test-dbclaim" + const secretName = "postgres-postgresql" + + ctx := context.Background() + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: "default", // TODO(user):Modify as needed + } + typeNamespacedSecretName := types.NamespacedName{ + Name: secretName, + Namespace: "default", // TODO(user):Modify as needed + } + claim := &persistancev1.DatabaseClaim{} + + BeforeEach(func() { + parsedDSN, err := url.Parse(testDSN) + Expect(err).NotTo(HaveOccurred()) + password, ok := parsedDSN.User.Password() + Expect(ok).To(BeTrue()) + + By("creating the custom resource for the Kind DatabaseClaim") + err = k8sClient.Get(ctx, typeNamespacedName, claim) + if err != nil && errors.IsNotFound(err) { + resource := &persistancev1.DatabaseClaim{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "persistance.atlas.infoblox.com/v1", + Kind: "DatabaseClaim", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: "default", + }, + Spec: persistancev1.DatabaseClaimSpec{ + Class: ptr.To(""), + AppID: "sample-app", + DatabaseName: "sample_app", + InstanceLabel: "sample-connection", + SecretName: secretName, + Username: parsedDSN.User.Username(), + EnableSuperUser: ptr.To(false), + EnableReplicationRole: ptr.To(false), + UseExistingSource: ptr.To(false), + Type: "postgres", + + Port: parsedDSN.Port(), + Host: parsedDSN.Hostname(), + }, + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + + secret := &corev1.Secret{} + err = k8sClient.Get(ctx, typeNamespacedSecretName, secret) + if err != nil && errors.IsNotFound(err) { + resource := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: "default", + }, + StringData: map[string]string{ + "password": password, + }, + Type: "Opaque", + } + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + + } + + }) + + AfterEach(func() { + // TODO(user): Cleanup logic after each test, like removing the resource instance. + resource := &persistancev1.DatabaseClaim{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + By("Cleanup the specific resource instance DatabaseClaim") + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + + secret := &corev1.Secret{} + err = k8sClient.Get(ctx, typeNamespacedSecretName, secret) + Expect(err).NotTo(HaveOccurred()) + + By("Cleanup the database secret") + + Expect(k8sClient.Delete(ctx, secret)).To(Succeed()) + + }) + + It("Should update DB Claim status", func() { + + By("Reconciling the created resource") + + configPath, err := filepath.Abs(filepath.Join("..", "..", "cmd", "config", "config.yaml")) + Expect(err).NotTo(HaveOccurred()) + + controllerReconciler := &DatabaseClaimReconciler{ + Config: &databaseclaim.DatabaseClaimConfig{ + Viper: config.NewConfig(logger, configPath), + Namespace: "default", + }, + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + controllerReconciler.Setup() + + // FIXME: make these actual properties on the reconciler struct + controllerReconciler.Config.Viper.Set("defaultMasterusername", "postgres") + controllerReconciler.Config.Viper.Set("defaultSslMode", "require") + + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + Eventually(func() bool { + resource := &persistancev1.DatabaseClaim{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + if err != nil { + return false + } + return resource.Status.Error == "" + + }, 10*time.Second, 100*time.Millisecond).Should(BeTrue()) + + }) + }) +}) + +func TestDB(t *testing.T) { + db, _, cleanup := RunDB() + defer cleanup() + defer db.Close() + _, err := db.Exec("CREATE TABLE test (id SERIAL PRIMARY KEY, name TEXT)") + if err != nil { + panic(err) + } +} diff --git a/internal/controller/dbroleclaim_controller_test.go b/internal/controller/dbroleclaim_controller_test.go index 2d8fab0f..674db60f 100644 --- a/internal/controller/dbroleclaim_controller_test.go +++ b/internal/controller/dbroleclaim_controller_test.go @@ -53,6 +53,15 @@ func TestReconcileDbRoleClaim_CopyExistingSecret(t *testing.T) { Name: resourceName, Namespace: "default", } + typeNamespacedClaimName := types.NamespacedName{ + Name: "testdbclaim", + Namespace: "default", + } + typeNamespacedSecretName := types.NamespacedName{ + Name: "master-secret", + Namespace: "default", + } + dbroleclaim := &persistancev1.DbRoleClaim{} viperObj := viper.New() viperObj.Set("passwordconfig::passwordRotationPeriod", 60) @@ -77,7 +86,11 @@ func TestReconcileDbRoleClaim_CopyExistingSecret(t *testing.T) { Status: persistancev1.DbRoleClaimStatus{}, } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + dbclaim := persistancev1.DatabaseClaim{} + err = k8sClient.Get(ctx, typeNamespacedClaimName, &dbclaim) + if err != nil && errors.IsNotFound(err) { dbClaim := &persistancev1.DatabaseClaim{ ObjectMeta: metav1.ObjectMeta{ Name: "testdbclaim", @@ -91,14 +104,22 @@ func TestReconcileDbRoleClaim_CopyExistingSecret(t *testing.T) { Status: persistancev1.DatabaseClaimStatus{}, } Expect(k8sClient.Create(ctx, dbClaim)).To(Succeed()) + } + + secret := corev1.Secret{} + err = k8sClient.Get(ctx, typeNamespacedSecretName, &secret) + if err != nil && errors.IsNotFound(err) { - sec := &corev1.Secret{} - sec.Data = map[string][]byte{ - "password": []byte("masterpassword"), - "username": []byte("user_a"), + sec := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "master-secret", + Namespace: "default", + }, + Data: map[string][]byte{ + "password": []byte("masterpassword"), + "username": []byte("user_a"), + }, } - sec.Name = "master-secret" - sec.Namespace = "default" Expect(k8sClient.Create(ctx, sec)).To(Succeed()) } }) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index b5ce10b2..cb70d89c 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -17,11 +17,15 @@ limitations under the License. package controller import ( + "database/sql" "fmt" "path/filepath" "runtime" "testing" + "time" + "github.com/go-logr/logr" + "github.com/go-logr/logr/funcr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -42,17 +46,26 @@ import ( var cfg *rest.Config var k8sClient client.Client var testEnv *envtest.Environment +var namespace string +var logger logr.Logger func TestControllers(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Controller Suite") + + logger = funcr.New(func(prefix, args string) { + t.Log(prefix, args) + }, funcr.Options{ + Verbosity: 1, + }) } var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") + namespace = "default" testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")}, ErrorIfCRDPathMissing: true, @@ -81,9 +94,21 @@ var _ = BeforeSuite(func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient).NotTo(BeNil()) + now := time.Now() + testdb, testDSN, cleanupTestDB = RunDB() + logger.Info("postgres_setup_took", "duration", time.Since(now)) + }) +// Stand up postgres in a container +var ( + testdb *sql.DB + testDSN string + cleanupTestDB func() +) + var _ = AfterSuite(func() { + cleanupTestDB() By("tearing down the test environment") err := testEnv.Stop() Expect(err).NotTo(HaveOccurred()) diff --git a/internal/controller/utils.go b/internal/controller/utils.go new file mode 100644 index 00000000..2b6d8c21 --- /dev/null +++ b/internal/controller/utils.go @@ -0,0 +1,90 @@ +package controller + +import ( + "context" + "database/sql" + "fmt" + "log" + "net" + "os" + "os/exec" + "time" + + _ "github.com/lib/pq" +) + +func getEphemeralPort() int { + l, err := net.Listen("tcp", "localhost:0") + if err != nil { + panic(err) + } + defer l.Close() // nolint:errcheck + return l.Addr().(*net.TCPAddr).Port +} + +func RunDB() (*sql.DB, string, func()) { + port := getEphemeralPort() + + // Run PostgreSQL in Docker + cmd := exec.Command("docker", "run", "-d", "-p", fmt.Sprintf("%d:5432", port), "-e", "POSTGRES_PASSWORD=postgres", "postgres:15") + cmd.Stderr = os.Stderr + out, err := cmd.Output() + if err != nil { + panic(err) + } + container := string(out[:len(out)-1]) // remove newline + + // Exercise hotload + //hotload.RegisterSQLDriver("pgx", stdlib.GetDefaultDriver()) + dsn := fmt.Sprintf("postgres://postgres:postgres@localhost:%d/postgres?sslmode=disable", port) + f, err := os.CreateTemp("", "dsn.txt") + if err != nil { + panic(err) + } + if _, err := f.WriteString(dsn); err != nil { + panic(err) + } + if err := f.Close(); err != nil { + panic(err) + } + + // TODO: read from file + conn, err := sql.Open("postgres", dsn) + if err != nil { + panic(err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + // try to connect to the database for 10 seconds + for i := 0; i < 10; i++ { + err = conn.PingContext(ctx) + if err == nil { + break + } + cancel() + ctx, cancel = context.WithTimeout(context.Background(), 1*time.Second) + time.Sleep(time.Second) + } + cancel() + if err != nil { + + cmd := exec.Command("docker", "logs", container) + cmd.Stderr = os.Stderr + out, err := cmd.Output() + if err != nil { + log.Println("docker logs failed:", err) + } else { + log.Println("docker logs:", string(out)) + } + + panic(err) + } + + return conn, dsn, func() { + _ = os.Remove(f.Name()) + cmd := exec.Command("docker", "rm", "-f", container) + cmd.Stderr = os.Stderr + // cmd.Stdout = os.Stdout + _ = cmd.Run() + } +} diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go index b938a926..fa00a165 100644 --- a/pkg/databaseclaim/databaseclaim.go +++ b/pkg/databaseclaim/databaseclaim.go @@ -3,7 +3,6 @@ package databaseclaim import ( "context" "fmt" - "os" "strings" "time" @@ -71,9 +70,10 @@ type input struct { // FIXME: this is type DatabaseType, not string DbType string - FragmentKey string - ManageCloudDB bool - SharedDBHost bool + FragmentKey string + ManageCloudDB bool + SharedDBHost bool + // FIXME: remove this, it's being logged as well. Remove those MasterConnInfo v1.DatabaseClaimConnectionInfo TempSecret string DbHostIdentifier string @@ -104,6 +104,8 @@ type DatabaseClaimConfig struct { MasterAuth *rdsauth.MasterAuth DbIdentifierPrefix string Class string + Namespace string + MetricsEnabled bool MetricsDepYamlPath string MetricsConfigYamlPath string } @@ -273,6 +275,7 @@ func (r *DatabaseClaimReconciler) setReqInfo(ctx context.Context, dbClaim *v1.Da } sharedDBHost = true } + r.Input.FragmentKey = fragmentKey connInfo := r.getClientConn(dbClaim) if connInfo.Port == "" { @@ -335,6 +338,14 @@ func Reconcile(r *DatabaseClaimReconciler, ctx context.Context, req ctrl.Request func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logr := log.FromContext(ctx).WithValues("databaseclaim", req.NamespacedName) + if r.Config == nil { + return ctrl.Result{}, fmt.Errorf("DatabaseClaimConfig is not set") + } + + if r.Client == nil { + return ctrl.Result{}, fmt.Errorf("client is not set") + } + var dbClaim v1.DatabaseClaim if err := r.Get(ctx, req.NamespacedName, &dbClaim); err != nil { if client.IgnoreNotFound(err) != nil { @@ -411,9 +422,10 @@ func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reques } } - // FIXME: turn on metrics deployments later when testing on box-2 is available - if err := r.createMetricsDeployment(ctx, dbClaim); err != nil { - return ctrl.Result{}, err + if r.Config.MetricsEnabled { + if err := r.createMetricsDeployment(ctx, dbClaim); err != nil { + return ctrl.Result{}, err + } } return r.executeDbClaimRequest(ctx, &dbClaim) } @@ -659,8 +671,7 @@ func (r *DatabaseClaimReconciler) reconcileUseExistingDB(ctx context.Context, db return nil } -func (r *DatabaseClaimReconciler) reconcileNewDB(ctx context.Context, - dbClaim *v1.DatabaseClaim) (ctrl.Result, error) { +func (r *DatabaseClaimReconciler) reconcileNewDB(ctx context.Context, dbClaim *v1.DatabaseClaim) (ctrl.Result, error) { logr := log.FromContext(ctx).WithValues("databaseclaim", dbClaim.Namespace+"/"+dbClaim.Name, "func", "reconcileNewDB") logr.Info("reconcileNewDB", "r.Input", r.Input) @@ -943,7 +954,7 @@ func ReplaceOrAddTag(tags []*crossplanerds.Tag, key string, value string) []*cro return tags } -func (r *DatabaseClaimReconciler) operationalTaggingForDbParamGroup(ctx context.Context, logr logr.Logger, dbParamGroupName string) { +func (r *DatabaseClaimReconciler) operationalTaggingForDbParamGroup(ctx context.Context, logr logr.Logger, dbParamGroupName string) error { dbParameterGroup := &crossplanerds.DBParameterGroup{} err := r.Client.Get(ctx, client.ObjectKey{ @@ -951,31 +962,29 @@ func (r *DatabaseClaimReconciler) operationalTaggingForDbParamGroup(ctx context. }, dbParameterGroup) if err != nil { - if errors.IsNotFound(err) { - return // nothing to delete - } - logr.Error(err, "Error getting crossplane db param group for old DB ") - } else { - operationalTagForProviderPresent := false - for _, tag := range dbParameterGroup.Spec.ForProvider.Tags { - if *tag.Key == operationalStatusTagKey && *tag.Value == operationalStatusInactiveValue { - operationalTagForProviderPresent = true - } + return err + } + operationalTagForProviderPresent := false + for _, tag := range dbParameterGroup.Spec.ForProvider.Tags { + if *tag.Key == operationalStatusTagKey && *tag.Value == operationalStatusInactiveValue { + operationalTagForProviderPresent = true } - if !operationalTagForProviderPresent { - patchDBParameterGroup := client.MergeFrom(dbParameterGroup.DeepCopy()) + } + if !operationalTagForProviderPresent { + patchDBParameterGroup := client.MergeFrom(dbParameterGroup.DeepCopy()) - dbParameterGroup.Spec.ForProvider.Tags = ReplaceOrAddTag(dbParameterGroup.Spec.ForProvider.Tags, operationalStatusTagKey, operationalStatusInactiveValue) + dbParameterGroup.Spec.ForProvider.Tags = ReplaceOrAddTag(dbParameterGroup.Spec.ForProvider.Tags, operationalStatusTagKey, operationalStatusInactiveValue) - err := r.Client.Patch(ctx, dbParameterGroup, patchDBParameterGroup) - if err != nil { - logr.Error(err, "Error updating operational tags for crossplane db param group ") - } + err := r.Client.Patch(ctx, dbParameterGroup, patchDBParameterGroup) + if err != nil { + logr.Error(err, "Error updating operational tags for crossplane db param group ") + return err } } + return nil } -func (r *DatabaseClaimReconciler) operationalTaggingForDbClusterParamGroup(ctx context.Context, logr logr.Logger, dbParamGroupName string) { +func (r *DatabaseClaimReconciler) operationalTaggingForDbClusterParamGroup(ctx context.Context, logr logr.Logger, dbParamGroupName string) error { dbClusterParamGroup := &crossplanerds.DBClusterParameterGroup{} err := r.Client.Get(ctx, client.ObjectKey{ @@ -983,32 +992,32 @@ func (r *DatabaseClaimReconciler) operationalTaggingForDbClusterParamGroup(ctx c }, dbClusterParamGroup) if err != nil { - if errors.IsNotFound(err) { - return // nothing to delete - } logr.Error(err, "Error getting crossplane db cluster param group for old DB ") - } else { - operationalTagForProviderPresent := false - for _, tag := range dbClusterParamGroup.Spec.ForProvider.Tags { - if *tag.Key == operationalStatusTagKey && *tag.Value == operationalStatusInactiveValue { - operationalTagForProviderPresent = true - } + return err + } + + operationalTagForProviderPresent := false + for _, tag := range dbClusterParamGroup.Spec.ForProvider.Tags { + if *tag.Key == operationalStatusTagKey && *tag.Value == operationalStatusInactiveValue { + operationalTagForProviderPresent = true } - if !operationalTagForProviderPresent { - patchDBClusterParameterGroup := client.MergeFrom(dbClusterParamGroup.DeepCopy()) + } - dbClusterParamGroup.Spec.ForProvider.Tags = ReplaceOrAddTag(dbClusterParamGroup.Spec.ForProvider.Tags, operationalStatusTagKey, operationalStatusInactiveValue) + if !operationalTagForProviderPresent { + patchDBClusterParameterGroup := client.MergeFrom(dbClusterParamGroup.DeepCopy()) - err := r.Client.Patch(ctx, dbClusterParamGroup, patchDBClusterParameterGroup) - if err != nil { - logr.Error(err, "Error updating operational tags for crossplane db cluster param group ") - } + dbClusterParamGroup.Spec.ForProvider.Tags = ReplaceOrAddTag(dbClusterParamGroup.Spec.ForProvider.Tags, operationalStatusTagKey, operationalStatusInactiveValue) + + err := r.Client.Patch(ctx, dbClusterParamGroup, patchDBClusterParameterGroup) + if err != nil { + logr.Error(err, "Error updating operational tags for crossplane db cluster param group ") + return err } } - + return nil } -func (r *DatabaseClaimReconciler) operationalTaggingForDbCluster(ctx context.Context, logr logr.Logger, dbHostName string) { +func (r *DatabaseClaimReconciler) operationalTaggingForDbCluster(ctx context.Context, logr logr.Logger, dbHostName string) error { dbCluster := &crossplanerds.DBCluster{} err := r.Client.Get(ctx, client.ObjectKey{ @@ -1016,28 +1025,27 @@ func (r *DatabaseClaimReconciler) operationalTaggingForDbCluster(ctx context.Con }, dbCluster) if err != nil { - if errors.IsNotFound(err) { - return // nothing to delete - } - logr.Error(err, "Error getting crossplane DBCluster for old DB") - } else { - operationalTagForProviderPresent := false - for _, tag := range dbCluster.Spec.ForProvider.Tags { - if *tag.Key == operationalStatusTagKey && *tag.Value == operationalStatusInactiveValue { - operationalTagForProviderPresent = true - } + return err + } + operationalTagForProviderPresent := false + for _, tag := range dbCluster.Spec.ForProvider.Tags { + if *tag.Key == operationalStatusTagKey && *tag.Value == operationalStatusInactiveValue { + operationalTagForProviderPresent = true } - if !operationalTagForProviderPresent { - patchDBClusterParameterGroup := client.MergeFrom(dbCluster.DeepCopy()) + } - dbCluster.Spec.ForProvider.Tags = ReplaceOrAddTag(dbCluster.Spec.ForProvider.Tags, operationalStatusTagKey, operationalStatusInactiveValue) + if !operationalTagForProviderPresent { + patchDBClusterParameterGroup := client.MergeFrom(dbCluster.DeepCopy()) - err := r.Client.Patch(ctx, dbCluster, patchDBClusterParameterGroup) - if err != nil { - logr.Error(err, "Error updating operational tags for crossplane db cluster ") - } + dbCluster.Spec.ForProvider.Tags = ReplaceOrAddTag(dbCluster.Spec.ForProvider.Tags, operationalStatusTagKey, operationalStatusInactiveValue) + + err := r.Client.Patch(ctx, dbCluster, patchDBClusterParameterGroup) + if err != nil { + logr.Error(err, "Error updating operational tags for crossplane db cluster ") + return err } } + return nil } @@ -1050,7 +1058,6 @@ func (r *DatabaseClaimReconciler) operationalTaggingForDbInstance(ctx context.Co }, dbInstance) if err != nil { - logr.Error(err, "Error getting crossplane dbInstance for old DB") return false, err } else { operationalTagForProviderPresent := false @@ -1106,9 +1113,18 @@ func (r *DatabaseClaimReconciler) ManageOperationalTagging(ctx context.Context, func (r *DatabaseClaimReconciler) manageOperationalTagging(ctx context.Context, logr logr.Logger, dbInstanceName, dbParamGroupName string) (bool, error) { - r.operationalTaggingForDbClusterParamGroup(ctx, logr, dbParamGroupName) - r.operationalTaggingForDbParamGroup(ctx, logr, dbParamGroupName) - r.operationalTaggingForDbCluster(ctx, logr, dbInstanceName) + err := r.operationalTaggingForDbClusterParamGroup(ctx, logr, dbParamGroupName) + if err != nil { + return false, err + } + err = r.operationalTaggingForDbParamGroup(ctx, logr, dbParamGroupName) + if err != nil { + return false, err + } + err = r.operationalTaggingForDbCluster(ctx, logr, dbInstanceName) + if err != nil { + return false, err + } // unlike other resources above, verifying tags updation and handling errors if any just for "DBInstance" resource isVerfied, err := r.operationalTaggingForDbInstance(ctx, logr, dbInstanceName) @@ -1152,8 +1168,8 @@ func (r *DatabaseClaimReconciler) getMasterPasswordForExistingDB(ctx context.Con if err != nil { return "", err } - password := string(gs.Data[secretKey]) + password := string(gs.Data[secretKey]) if password == "" { return "", fmt.Errorf("invalid credentials (password)") } @@ -1427,6 +1443,7 @@ func (r *DatabaseClaimReconciler) getMinPasswordLength() int { } func (r *DatabaseClaimReconciler) getSecretRef(fragmentKey string) string { + return r.Config.Viper.GetString(fmt.Sprintf("%s::PasswordSecretRef", fragmentKey)) } @@ -1513,7 +1530,7 @@ func (r *DatabaseClaimReconciler) readResourceSecret(ctx context.Context, secret rs := &corev1.Secret{} connInfo := v1.DatabaseClaimConnectionInfo{} - serviceNS, _ := getServiceNamespace() + serviceNS, _ := r.getServiceNamespace() err := r.Client.Get(ctx, client.ObjectKey{ Namespace: serviceNS, @@ -1787,7 +1804,7 @@ func (r *DatabaseClaimReconciler) manageDBCluster(ctx context.Context, dbHostNam return false, err } - serviceNS, err := getServiceNamespace() + serviceNS, err := r.getServiceNamespace() if err != nil { return false, err } @@ -1913,7 +1930,7 @@ func (r *DatabaseClaimReconciler) manageDBCluster(ctx context.Context, dbHostNam func (r *DatabaseClaimReconciler) managePostgresDBInstance(ctx context.Context, dbHostName string, dbClaim *v1.DatabaseClaim) (bool, error) { logr := log.FromContext(ctx) - serviceNS, err := getServiceNamespace() + serviceNS, err := r.getServiceNamespace() if err != nil { return false, err } @@ -2648,18 +2665,14 @@ func (r *DatabaseClaimReconciler) createOrUpdateSecret(ctx context.Context, dbCl Name: secretName, }, gs) - if err != nil { - if !errors.IsNotFound(err) { - return err - } + if err != nil && errors.IsNotFound(err) { if err := r.createSecret(ctx, dbClaim, dsn, dbURI, connInfo); err != nil { return err } - } else if err := r.updateSecret(ctx, dbClaim.Spec.DSNName, dsn, dbURI, connInfo, gs); err != nil { - return err + return nil } - return nil + return r.updateSecret(ctx, dbClaim.Spec.DSNName, dsn, dbURI, connInfo, gs) } func (r *DatabaseClaimReconciler) createSecret(ctx context.Context, dbClaim *v1.DatabaseClaim, dsn, dbURI string, connInfo *v1.DatabaseClaimConnectionInfo) error { @@ -2704,6 +2717,11 @@ func (r *DatabaseClaimReconciler) updateSecret(ctx context.Context, dsnName, dsn logr := log.FromContext(ctx) + // FIXME: move this to validation logic + if dsnName == "" { + dsnName = "dsn.txt" + } + exSecret.Data[dsnName] = []byte(dsn) exSecret.Data["uri_"+dsnName] = []byte(dbURI) exSecret.Data["hostname"] = []byte(connInfo.Host) @@ -2720,6 +2738,7 @@ func (r *DatabaseClaimReconciler) updateSecret(ctx context.Context, dsnName, dsn func (r *DatabaseClaimReconciler) readMasterPassword(ctx context.Context) (string, error) { gs := &corev1.Secret{} secretName := r.getSecretRef(r.Input.FragmentKey) + // FIXME: remove this nonsense, only unit tests were using a special secret key. This is pointless secretKey := r.getSecretKey(r.Input.FragmentKey) if secretKey == "" { secretKey = "password" @@ -2727,10 +2746,11 @@ func (r *DatabaseClaimReconciler) readMasterPassword(ctx context.Context) (strin if secretName == "" { return "", fmt.Errorf("an empty password secret reference") } - namespace, err := getServiceNamespace() + namespace, err := r.getServiceNamespace() if err != nil { return "", err } + err = r.Client.Get(ctx, client.ObjectKey{ Namespace: namespace, Name: secretName, @@ -2738,6 +2758,7 @@ func (r *DatabaseClaimReconciler) readMasterPassword(ctx context.Context) (strin if err != nil { return "", err } + return string(gs.Data[secretKey]), nil } @@ -2751,6 +2772,7 @@ func (r *DatabaseClaimReconciler) matchInstanceLabel(dbClaim *v1.DatabaseClaim) rTree.Insert(k, true) } } + // Find the longest prefix match m, _, ok := rTree.LongestPrefix(dbClaim.Spec.InstanceLabel) if !ok { @@ -2855,12 +2877,12 @@ func updateClusterStatus(status *v1.Status, hostParams *hostparams.HostParams) { } } -func getServiceNamespace() (string, error) { - ns, found := os.LookupEnv(serviceNamespaceEnvVar) - if !found { +func (r *DatabaseClaimReconciler) getServiceNamespace() (string, error) { + if r.Config.Namespace == "" { return "", fmt.Errorf("service namespace env %s must be set", serviceNamespaceEnvVar) } - return ns, nil + + return r.Config.Namespace, nil } func (r *DatabaseClaimReconciler) getSrcAdminPasswdFromSecret(ctx context.Context, dbClaim *v1.DatabaseClaim) (string, error) { diff --git a/pkg/dbclient/client.go b/pkg/dbclient/client.go index 044ae340..40c9942c 100644 --- a/pkg/dbclient/client.go +++ b/pkg/dbclient/client.go @@ -149,6 +149,17 @@ func (pc *client) CreateDatabase(dbName string) (bool, error) { return created, err } +func (pc *client) CheckExtension(extName string) (bool, error) { + var exists bool + db := pc.DB + err := db.QueryRow("SELECT EXISTS(SELECT * FROM pg_extension WHERE extname = $1)", extName).Scan(&exists) + if err != nil { + pc.log.Error(err, "could not query for extension") + return exists, err + } + return exists, nil +} + func (pc *client) CreateDefaultExtensions(dbName string) error { db, err := pc.getDB(dbName) if err != nil { @@ -158,8 +169,15 @@ func (pc *client) CreateDefaultExtensions(dbName string) error { pc.log.Info("connected to " + dbName) defer db.Close() for _, s := range getDefaulExtensions() { + ok, err := pc.CheckExtension(s) + if err != nil { + return fmt.Errorf("psql_extension_query %s: %w", s, err) + } + if !ok { + continue + } + if _, err = db.Exec(fmt.Sprintf("CREATE EXTENSION IF NOT EXISTS %s", pq.QuoteIdentifier(s))); err != nil { - pc.log.Error(err, "could not create extension", "database_name", dbName) return fmt.Errorf("could not create extension %s: %s", s, err) } pc.log.Info("created extension " + s) diff --git a/test/e2e/databaseclaim_controller_integ_test.go b/test/e2e/databaseclaim_controller_integ_test.go index 3405dfa5..4f9b9ac0 100644 --- a/test/e2e/databaseclaim_controller_integ_test.go +++ b/test/e2e/databaseclaim_controller_integ_test.go @@ -2,7 +2,7 @@ package e2e import ( "context" - "log" + "fmt" "testing" "time" @@ -12,8 +12,8 @@ import ( . "github.com/onsi/gomega" "github.com/spf13/viper" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/rand" "k8s.io/utils/ptr" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" persistancev1 "github.com/infobloxopen/db-controller/api/v1" @@ -74,142 +74,152 @@ var _ = Describe("db-controller", func() { }) }) -var _ = Describe("manageOperationalTagging", Ordered, func() { +// FIXME: these are not integration tests, move to databaseclaim_controller_test.go +var _ = Describe("ManageTagging", Ordered, func() { - return // define and create objects in the test cluster - dbCluster := &crossplanerds.DBCluster{} - dbClusterParam := &crossplanerds.DBClusterParameterGroup{} - dbParam := &crossplanerds.DBParameterGroup{} - dnInstance1 := &crossplanerds.DBInstance{} - dnInstance2 := &crossplanerds.DBInstance{} - dnInstance3 := &crossplanerds.DBInstance{} - - BeforeAll(func() { - if testing.Short() { - Skip("skipping k8s based tests") - } - log.Println("FIXME: move integration tests to a separate package kubebuilder uses testing/e2e") - - By("Creating objects beforehand of DBClsuerParameterGroup, DBCluser, DBParameterGroup and DBInstance") - testString := "test" - ctx := context.Background() - dbCluster = &crossplanerds.DBCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "rds.aws.crossplane.io/v1alpha1", - Kind: "DBCluster", + name := fmt.Sprintf("test-%s-%s", namespace, rand.String(5)) + + dbCluster := &crossplanerds.DBCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "rds.aws.crossplane.io/v1alpha1", + Kind: "DBCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: crossplanerds.DBClusterSpec{ + ForProvider: crossplanerds.DBClusterParameters{ + Engine: ptr.To("test"), }, - ObjectMeta: metav1.ObjectMeta{ - Name: "db", - Namespace: "default", + }, + } + dbClusterParam := &crossplanerds.DBClusterParameterGroup{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "rds.aws.crossplane.io/v1alpha1", + Kind: "DBClusterParameterGroup", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: crossplanerds.DBClusterParameterGroupSpec{ + ForProvider: crossplanerds.DBClusterParameterGroupParameters{ + Description: ptr.To("test"), }, - Spec: crossplanerds.DBClusterSpec{ - ForProvider: crossplanerds.DBClusterParameters{ - Engine: &testString, - }, + }, + } + + dbParam := &crossplanerds.DBParameterGroup{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "rds.aws.crossplane.io/v1alpha1", + Kind: "DBParameterGroup", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: crossplanerds.DBParameterGroupSpec{ + ForProvider: crossplanerds.DBParameterGroupParameters{ + Description: ptr.To("test"), }, - } - Expect(e2e_k8sClient.Create(ctx, dbCluster)).Should(Succeed()) - ctx = context.Background() - dbClusterParam = &crossplanerds.DBClusterParameterGroup{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "rds.aws.crossplane.io/v1alpha1", - Kind: "DBClusterParameterGroup", + }, + } + + dnInstance1 := &crossplanerds.DBInstance{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "rds.aws.crossplane.io/v1alpha1", + Kind: "DBInstance", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: crossplanerds.DBInstanceSpec{ + ForProvider: crossplanerds.DBInstanceParameters{ + Engine: ptr.To("test"), + DBInstanceClass: ptr.To("test"), }, - ObjectMeta: metav1.ObjectMeta{ - Name: "dbparam", - Namespace: "default", + }, + } + name2 := fmt.Sprintf("%s-2", name) + dnInstance2 := &crossplanerds.DBInstance{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "rds.aws.crossplane.io/v1alpha1", + Kind: "DBInstance", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name2, + Namespace: namespace, + }, + Spec: crossplanerds.DBInstanceSpec{ + ForProvider: crossplanerds.DBInstanceParameters{ + Engine: ptr.To("test"), + DBInstanceClass: ptr.To("test"), }, - Spec: crossplanerds.DBClusterParameterGroupSpec{ - ForProvider: crossplanerds.DBClusterParameterGroupParameters{ - Description: &testString, - }, + }, + } + name3 := fmt.Sprintf("%s-3", name) + dnInstance3 := &crossplanerds.DBInstance{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "rds.aws.crossplane.io/v1alpha1", + Kind: "DBInstance", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name3, + Namespace: namespace, + }, + Spec: crossplanerds.DBInstanceSpec{ + ForProvider: crossplanerds.DBInstanceParameters{ + Engine: ptr.To("test"), + DBInstanceClass: ptr.To("test"), }, - } - Expect(e2e_k8sClient.Create(ctx, dbClusterParam)).Should(Succeed()) + }, + } - dbParam = &crossplanerds.DBParameterGroup{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "rds.aws.crossplane.io/v1alpha1", - Kind: "DBParameterGroup", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "dbparam", - Namespace: "default", - }, - Spec: crossplanerds.DBParameterGroupSpec{ - ForProvider: crossplanerds.DBParameterGroupParameters{ - Description: &testString, - }, - }, - } - Expect(e2e_k8sClient.Create(ctx, dbParam)).Should(Succeed()) + BeforeAll(func() { - ctx = context.Background() - dnInstance1 = &crossplanerds.DBInstance{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "rds.aws.crossplane.io/v1alpha1", - Kind: "DBInstance", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "db", - Namespace: "default", - }, - Spec: crossplanerds.DBInstanceSpec{ - ForProvider: crossplanerds.DBInstanceParameters{ - Engine: &testString, - DBInstanceClass: &testString, - }, - }, - } - Expect(e2e_k8sClient.Create(ctx, dnInstance1)).Should(Succeed()) + By("Creating objects beforehand of DBClsuerParameterGroup, DBCluster, DBParameterGroup and DBInstance") + ctx := context.Background() - ctx = context.Background() - dnInstance2 = &crossplanerds.DBInstance{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "rds.aws.crossplane.io/v1alpha1", - Kind: "DBInstance", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "db-2", - Namespace: "default", - }, - Spec: crossplanerds.DBInstanceSpec{ - ForProvider: crossplanerds.DBInstanceParameters{ - Engine: &testString, - DBInstanceClass: &testString, - }, - }, - } - Expect(e2e_k8sClient.Create(ctx, dnInstance2)).Should(Succeed()) + Expect(k8sClient.Create(ctx, dbCluster)).Should(Succeed()) + Expect(k8sClient.Create(ctx, dbClusterParam)).Should(Succeed()) + Expect(k8sClient.Create(ctx, dbParam)).Should(Succeed()) + + Expect(k8sClient.Create(ctx, dnInstance1)).Should(Succeed()) + + Expect(k8sClient.Create(ctx, dnInstance2)).Should(Succeed()) + Expect(k8sClient.Create(ctx, dnInstance3)).Should(Succeed()) + }) + + AfterAll(func() { + + By("Deleting objects of DBClsuerParameterGroup, DBCluster, DBParameterGroup and DBInstance") + Expect(k8sClient.Delete(ctx, dbCluster)).Should(Succeed()) + Expect(k8sClient.Delete(ctx, dbClusterParam)).Should(Succeed()) + Expect(k8sClient.Delete(ctx, dbParam)).Should(Succeed()) + + Expect(k8sClient.Delete(ctx, dnInstance1)).Should(Succeed()) + + Expect(k8sClient.Delete(ctx, dnInstance2)).Should(Succeed()) + Expect(k8sClient.Delete(ctx, dnInstance3)).Should(Succeed()) - ctx = context.Background() - dnInstance3 = &crossplanerds.DBInstance{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "rds.aws.crossplane.io/v1alpha1", - Kind: "DBInstance", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "db3", - Namespace: "default", - }, - Spec: crossplanerds.DBInstanceSpec{ - ForProvider: crossplanerds.DBInstanceParameters{ - Engine: &testString, - DBInstanceClass: &testString, - }, - }, - } - Expect(e2e_k8sClient.Create(ctx, dnInstance3)).Should(Succeed()) }) Context("Now, try adding tags to resources which does not exists, while multiAZ is enabled", func() { It("Should not add tags to any other already existing resources", func() { - mockReconciler := &controller.DatabaseClaimReconciler{} - mockReconciler.Client = e2e_k8sClient - mockReconciler.Config.Viper = viper.New() + + mockReconciler := &controller.DatabaseClaimReconciler{ + Client: k8sClient, + Config: &databaseclaim.DatabaseClaimConfig{ + Viper: viper.New(), + }, + } mockReconciler.Config.Viper.Set("dbMultiAZEnabled", true) + mockReconciler.Setup() + // providing names of non-existing resources below check, err := mockReconciler.Reconciler().ManageOperationalTagging(context.Background(), logr.Discard(), "dbb", "dbparamm") Expect(err).Should(HaveOccurred()) // This should create error @@ -218,38 +228,38 @@ var _ = Describe("manageOperationalTagging", Ordered, func() { By("Lets get all objects again to check whether tags have not been added to any resource, as we provied wrong names above") dbCluster := &crossplanerds.DBCluster{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db", + Expect(k8sClient.Get(context.Background(), client.ObjectKey{ + Name: name, }, dbCluster)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dbCluster.Spec.ForProvider.Tags)).To(Equal(false)) dbClusterParam := &crossplanerds.DBClusterParameterGroup{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "dbparam", + Expect(k8sClient.Get(context.Background(), client.ObjectKey{ + Name: name, }, dbClusterParam)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dbClusterParam.Spec.ForProvider.Tags)).To(Equal(false)) dbParam := &crossplanerds.DBParameterGroup{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "dbparam", + Expect(k8sClient.Get(context.Background(), client.ObjectKey{ + Name: name, }, dbParam)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dbParam.Spec.ForProvider.Tags)).To(Equal(false)) dnInstance1 = &crossplanerds.DBInstance{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db", + Expect(k8sClient.Get(context.Background(), client.ObjectKey{ + Name: name, }, dnInstance1)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance1.Spec.ForProvider.Tags)).To(Equal(false)) dnInstance2 = &crossplanerds.DBInstance{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db-2", + Expect(k8sClient.Get(context.Background(), client.ObjectKey{ + Name: name2, }, dnInstance2)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance2.Spec.ForProvider.Tags)).To(Equal(false)) dnInstance3 = &crossplanerds.DBInstance{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db3", + Expect(k8sClient.Get(context.Background(), client.ObjectKey{ + Name: name3, }, dnInstance3)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance3.Spec.ForProvider.Tags)).To(Equal(false)) }) @@ -257,51 +267,56 @@ var _ = Describe("manageOperationalTagging", Ordered, func() { Context("Now, try Adding tags to resources, with multiAZ disabled", func() { It("Should add tags to all valid resources. Should skip instance-2 as multiAZ is disabled", func() { - mockReconciler := &controller.DatabaseClaimReconciler{} - mockReconciler.Client = e2e_k8sClient - mockReconciler.Config.Viper = viper.New() + + mockReconciler := &controller.DatabaseClaimReconciler{ + Client: k8sClient, + Config: &databaseclaim.DatabaseClaimConfig{ + Viper: viper.New(), + }, + } mockReconciler.Config.Viper.Set("dbMultiAZEnabled", false) - check, err := mockReconciler.Reconciler().ManageOperationalTagging(context.Background(), logr.Logger{}, "db", "dbparam") - Expect(err).ShouldNot(HaveOccurred()) + mockReconciler.Setup() + + check, err := mockReconciler.Reconciler().ManageOperationalTagging(context.Background(), logger, name, name) + Expect(err).To(BeNil()) Expect(check).To(BeFalse()) By("Lets get all objects again to check whether tags can be found at .spec.ForProvider") dbCluster = &crossplanerds.DBCluster{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db", - }, dbCluster)).ShouldNot(HaveOccurred()) + Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{Name: name}, dbCluster)).ShouldNot(HaveOccurred()) + Expect(hasOperationalTag(dbCluster.Spec.ForProvider.Tags)).To(Equal(true)) dbClusterParam = &crossplanerds.DBClusterParameterGroup{} Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "dbparam", + Name: name, }, dbClusterParam)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dbClusterParam.Spec.ForProvider.Tags)).To(Equal(true)) dbParam = &crossplanerds.DBParameterGroup{} Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "dbparam", + Name: name, }, dbParam)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dbParam.Spec.ForProvider.Tags)).To(Equal(true)) dnInstance1 = &crossplanerds.DBInstance{} Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db", + Name: name, }, dnInstance1)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance1.Spec.ForProvider.Tags)).To(Equal(true)) // tag should not be found at spec for dbInstance2 as multiAZ is disabled dnInstance2 = &crossplanerds.DBInstance{} Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db-2", + Name: name2, }, dnInstance2)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance2.Spec.ForProvider.Tags)).To(Equal(false)) // tag should not be found at spec for dbInstance3 as we had not requested this resource to be tagged dnInstance3 = &crossplanerds.DBInstance{} Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db3", + Name: name3, }, dnInstance3)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance3.Spec.ForProvider.Tags)).To(Equal(false)) }) @@ -309,33 +324,39 @@ var _ = Describe("manageOperationalTagging", Ordered, func() { Context("Adding tags to resources, while multiAZ is enabled", func() { It("Should add tags to all valid resources if exists. Should NOT skip instance-2 as multiAZ is enabled", func() { - mockReconciler := &controller.DatabaseClaimReconciler{} - mockReconciler.Client = e2e_k8sClient - mockReconciler.Config.Viper = viper.New() + + mockReconciler := &controller.DatabaseClaimReconciler{ + Client: k8sClient, + Config: &databaseclaim.DatabaseClaimConfig{ + Viper: viper.New(), + }, + } mockReconciler.Config.Viper.Set("dbMultiAZEnabled", true) - check, err := mockReconciler.Reconciler().ManageOperationalTagging(context.Background(), ctrl.Log.WithName("controllers"), "db", "dbparam") + mockReconciler.Setup() + + check, err := mockReconciler.Reconciler().ManageOperationalTagging(context.Background(), logger, name, name) Expect(err).ShouldNot(HaveOccurred()) Expect(check).To(BeFalse()) By("Lets get all DBinstance objects again to check whether tags can be found at .spec.ForProvider for all instances in multiAZ") - dnInstance1 = &crossplanerds.DBInstance{} + dnInstance1 := &crossplanerds.DBInstance{} Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db", + Name: name, }, dnInstance1)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance1.Spec.ForProvider.Tags)).To(Equal(true)) // tag should be found at spec for dbInstancw2 as multiAZ is enabled now - dnInstance2 = &crossplanerds.DBInstance{} + dnInstance2 := &crossplanerds.DBInstance{} Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db-2", + Name: name2, }, dnInstance2)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance2.Spec.ForProvider.Tags)).To(Equal(true)) // tag should not be found at spec for dbInstancr3 as we had not requested this resource to be tagged - dnInstance3 = &crossplanerds.DBInstance{} + dnInstance3 := &crossplanerds.DBInstance{} Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db3", + Name: name3, }, dnInstance3)).ShouldNot(HaveOccurred()) Expect(hasOperationalTag(dnInstance3.Spec.ForProvider.Tags)).To(Equal(false)) }) @@ -343,10 +364,15 @@ var _ = Describe("manageOperationalTagging", Ordered, func() { Context("When tags get successfully updated, They are reflected at .status.AtProvider for DBInstance", func() { It("manageOperationalTagging() Should return true without any error", func() { - mockReconciler := &controller.DatabaseClaimReconciler{} - mockReconciler.Client = e2e_k8sClient - mockReconciler.Config.Viper = viper.New() + + mockReconciler := &controller.DatabaseClaimReconciler{ + Client: k8sClient, + Config: &databaseclaim.DatabaseClaimConfig{ + Viper: viper.New(), + }, + } mockReconciler.Config.Viper.Set("dbMultiAZEnabled", true) + mockReconciler.Setup() By("adding tags beforehand to .status.AtProvier.TagList. As in reality, if tags gets successfully added. It will reflect at the said path") @@ -354,44 +380,47 @@ var _ = Describe("manageOperationalTagging", Ordered, func() { operationalStatusInactiveValuePtr := databaseclaim.OperationalStatusInactiveValue ctx := context.Background() - dnInstance1.Status.AtProvider.TagList = []*crossplanerds.Tag{ - { - Key: &operationalStatusTagKeyPtr, - Value: &operationalStatusInactiveValuePtr, - }, - } - dnInstance2.Status.AtProvider.TagList = []*crossplanerds.Tag{ - { - Key: &operationalStatusTagKeyPtr, - Value: &operationalStatusInactiveValuePtr, - }, + var updateStatus = func(dbName string) { + dnInstance := crossplanerds.DBInstance{} + Expect(k8sClient.Get(context.Background(), client.ObjectKey{ + Name: dbName, + }, &dnInstance)).ShouldNot(HaveOccurred()) + + dnInstance.Status.AtProvider.TagList = []*crossplanerds.Tag{ + { + Key: &operationalStatusTagKeyPtr, + Value: &operationalStatusInactiveValuePtr, + }, + } + Expect(k8sClient.Status().Update(ctx, &dnInstance)).Should(Succeed()) } - Expect(e2e_k8sClient.Status().Update(ctx, dnInstance1)).Should(Succeed()) - Expect(e2e_k8sClient.Status().Update(ctx, dnInstance2)).Should(Succeed()) + updateStatus(name) + updateStatus(name2) - check, err := mockReconciler.Reconciler().ManageOperationalTagging(context.Background(), ctrl.Log.WithName("controllers"), "db", "dbparam") + check, err := mockReconciler.Reconciler().ManageOperationalTagging(ctx, logger, name, name) Expect(err).ShouldNot(HaveOccurred()) Expect(check).To(BeTrue()) // Lets also check the tags at status - dnInstance1 = &crossplanerds.DBInstance{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db", - }, dnInstance1)).ShouldNot(HaveOccurred()) - Expect(hasOperationalTag(dnInstance1.Status.AtProvider.TagList)).To(Equal(true)) - - dnInstance2 = &crossplanerds.DBInstance{} - Expect(mockReconciler.Client.Get(context.Background(), client.ObjectKey{ - Name: "db-2", - }, dnInstance2)).ShouldNot(HaveOccurred()) - Expect(hasOperationalTag(dnInstance2.Status.AtProvider.TagList)).To(Equal(true)) + checkInstanceStatus(k8sClient, name, true) + checkInstanceStatus(k8sClient, name2, true) }) }) }) +func checkInstanceStatus(k8sClient client.Client, name string, expected bool) { + + dnInstance1 := &crossplanerds.DBInstance{} + Expect(k8sClient.Get(context.Background(), client.ObjectKey{ + Name: name, + }, dnInstance1)).ShouldNot(HaveOccurred()) + Expect(hasOperationalTag(dnInstance1.Status.AtProvider.TagList)).To(Equal(expected)) + +} + var _ = Describe("canTagResources", Ordered, func() { return // Creating resources required to do tests beforehand diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index b6f5eef5..c759d7bd 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -21,8 +21,11 @@ import ( "os" "os/exec" "testing" + "time" crossplanerdsv1alpha1 "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" + "github.com/go-logr/logr" + "github.com/go-logr/logr/funcr" persistancev1 "github.com/infobloxopen/db-controller/api/v1" "github.com/infobloxopen/db-controller/test/utils" "k8s.io/client-go/kubernetes/scheme" @@ -46,12 +49,23 @@ func init() { // FIXME: remove this and use namespace instead var class = "" +var logger logr.Logger // Run e2e tests using the Ginkgo runner. func TestE2E(t *testing.T) { RegisterFailHandler(Fail) fmt.Fprintf(GinkgoWriter, "Starting E2E suite\n") RunSpecs(t, "e2e suite") + logger = NewGinkgoLogger(t) +} + +func NewGinkgoLogger(t *testing.T) logr.Logger { + // Create a new logger with the formatter and a test writer + return funcr.New(func(prefix, args string) { + t.Log(prefix, args) + }, funcr.Options{ + Verbosity: 1, + }) } var _ = BeforeSuite(func() { @@ -89,12 +103,24 @@ var _ = BeforeSuite(func() { _, err = utils.Run(cmd) ExpectWithOffset(1, err).NotTo(HaveOccurred()) + By("validating that the controller-manager pod is running as expected") + verifyControllerUp := func() error { + // Get pod name + + cmd := exec.Command("make", "helm-test") + _, err = utils.Run(cmd) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + return nil + } + EventuallyWithOffset(1, verifyControllerUp, time.Minute, time.Second).Should(Succeed()) + }) var _ = AfterSuite(func() { - By("Helm upgrading the manager") - cmd := exec.Command("make", "undeploy") - _, err := utils.Run(cmd) - Expect(err).NotTo(HaveOccurred()) + // By("Helm upgrading the manager") + // cmd := exec.Command("make", "undeploy") + // _, err := utils.Run(cmd) + // Expect(err).NotTo(HaveOccurred()) })