diff --git a/cmd/config/config.yaml b/cmd/config/config.yaml index cdecb1ce..869fc9a8 100644 --- a/cmd/config/config.yaml +++ b/cmd/config/config.yaml @@ -33,6 +33,7 @@ project: gcp-eng-ddiaas-dev network: "ddiaas-dev-use1-vpc" subnetwork: private-service-connect numbackupstoretain: 3 +defaultMajorVersion: 15 passwordConfig: passwordComplexity: enabled diff --git a/helm/db-controller/values.yaml b/helm/db-controller/values.yaml index fe734b6d..7d2f9b40 100644 --- a/helm/db-controller/values.yaml +++ b/helm/db-controller/values.yaml @@ -170,6 +170,7 @@ controllerConfig: passwordRotationPeriod: 60m pgTemp: "/pg-temp/" storageType: gp3 + defaultMajorVersion: 15 supportSuperUserElevation: true # system funtions are created as functions in the database. The prefix is used as the schema name. # only "ib_" prefixed functions are supported at this time. diff --git a/internal/controller/databaseclaim_controller_test.go b/internal/controller/databaseclaim_controller_test.go index e2f278fa..7394f2de 100644 --- a/internal/controller/databaseclaim_controller_test.go +++ b/internal/controller/databaseclaim_controller_test.go @@ -131,11 +131,11 @@ var _ = Describe("DatabaseClaim Controller", func() { }) - It("Should fail to reconcile DB Claim missing dbVersion", func() { + It("Should succeed to reconcile DB Claim missing dbVersion", func() { By("Reconciling the created resource") _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) - Expect(err).To(MatchError(".spec.dbVersion is a mandatory field and cannot be empty")) + Expect(err).NotTo(HaveOccurred()) Expect(claim.Status.Error).To(Equal("")) }) diff --git a/pkg/basefunctions/basefunctions.go b/pkg/basefunctions/basefunctions.go index ad9ecd41..d9d22217 100644 --- a/pkg/basefunctions/basefunctions.go +++ b/pkg/basefunctions/basefunctions.go @@ -239,6 +239,10 @@ func GetSystemFunctions(viperConfig *viper.Viper) map[string]string { return viperConfig.GetStringMapString("systemFunctions") } +func GetDefaultMajorVersion(viperConfig *viper.Viper) string { + return viperConfig.GetString("defaultMajorVersion") +} + func GetDynamicHostWaitTime(viperConfig *viper.Viper) time.Duration { t := time.Duration(viperConfig.GetInt("dynamicHostWaitTimeMin")) * time.Minute diff --git a/pkg/databaseclaim/awsprovider.go b/pkg/databaseclaim/awsprovider.go index f89c54f6..aa0f6c7e 100644 --- a/pkg/databaseclaim/awsprovider.go +++ b/pkg/databaseclaim/awsprovider.go @@ -9,9 +9,11 @@ import ( "github.com/go-logr/logr" v1 "github.com/infobloxopen/db-controller/api/v1" basefun "github.com/infobloxopen/db-controller/pkg/basefunctions" + "github.com/infobloxopen/db-controller/pkg/hostparams" _ "github.com/lib/pq" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -109,11 +111,6 @@ func (r *DatabaseClaimReconciler) manageDBClusterAWS(ctx context.Context, dbHost } logr.Info("creating_crossplane_dbcluster", "name", dbHostName) - validationError := params.CheckEngineVersion() - if validationError != nil { - logr.Error(validationError, "invalid_db_version") - return false, validationError - } dbCluster = &crossplaneaws.DBCluster{ ObjectMeta: metav1.ObjectMeta{ Name: dbHostName, @@ -137,9 +134,9 @@ func (r *DatabaseClaimReconciler) manageDBClusterAWS(ctx context.Context, dbHost DBClusterParameterGroupNameRef: &xpv1.Reference{ Name: pgName, }, - EngineVersion: ¶ms.EngineVersion, + EngineVersion: ptr.To(getEngineVersion(params, r)), }, - Engine: ¶ms.Engine, + Engine: ¶ms.Type, Tags: DBClaimTags(dbClaim.Spec.Tags).DBTags(), // Items from Config MasterUsername: ¶ms.MasterUsername, @@ -249,10 +246,6 @@ func (r *DatabaseClaimReconciler) managePostgresDBInstanceAWS(ctx context.Contex }, dbInstance) if err != nil { if errors.IsNotFound(err) { - validationError := params.CheckEngineVersion() - if validationError != nil { - return false, validationError - } dbInstance = &crossplaneaws.DBInstance{ ObjectMeta: metav1.ObjectMeta{ Name: dbHostName, @@ -277,9 +270,9 @@ func (r *DatabaseClaimReconciler) managePostgresDBInstanceAWS(ctx context.Contex }, AutogeneratePassword: true, MasterUserPasswordSecretRef: &dbMasterSecretInstance, - EngineVersion: ¶ms.EngineVersion, + EngineVersion: ptr.To(getEngineVersion(params, r)), }, - Engine: ¶ms.Engine, + Engine: ¶ms.Type, MultiAZ: &multiAZ, DBInstanceClass: ¶ms.InstanceClass, AllocatedStorage: &ms64, @@ -407,10 +400,6 @@ func (r *DatabaseClaimReconciler) manageAuroraDBInstance(ctx context.Context, re if err != nil { if errors.IsNotFound(err) { logr.Info("aurora db instance not found. creating now") - validationError := params.CheckEngineVersion() - if validationError != nil { - return false, validationError - } dbInstance = &crossplaneaws.DBInstance{ ObjectMeta: metav1.ObjectMeta{ Name: dbHostName, @@ -424,10 +413,10 @@ func (r *DatabaseClaimReconciler) manageAuroraDBInstance(ctx context.Context, re CustomDBInstanceParameters: crossplaneaws.CustomDBInstanceParameters{ ApplyImmediately: &trueVal, SkipFinalSnapshot: params.SkipFinalSnapshotBeforeDeletion, - EngineVersion: ¶ms.EngineVersion, + EngineVersion: ptr.To(getEngineVersion(params, r)), }, DBParameterGroupName: &pgName, - Engine: ¶ms.Engine, + Engine: ¶ms.Type, DBInstanceClass: ¶ms.InstanceClass, Tags: ReplaceOrAddTag(DBClaimTags(dbClaim.Spec.Tags).DBTags(), OperationalStatusTagKey, OperationalStatusActiveValue), // Items from Config @@ -502,10 +491,6 @@ func (r *DatabaseClaimReconciler) managePostgresParamGroup(ctx context.Context, }, dbParamGroup) if err != nil { if errors.IsNotFound(err) { - validationError := params.CheckEngineVersion() - if validationError != nil { - return pgName, validationError - } dbParamGroup = &crossplaneaws.DBParameterGroup{ ObjectMeta: metav1.ObjectMeta{ Name: pgName, @@ -516,8 +501,8 @@ func (r *DatabaseClaimReconciler) managePostgresParamGroup(ctx context.Context, Description: &desc, CustomDBParameterGroupParameters: crossplaneaws.CustomDBParameterGroupParameters{ DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ - Engine: params.Engine, - EngineVersion: ¶ms.EngineVersion, + Engine: params.Type, + EngineVersion: ptr.To(getEngineVersion(params, r)), }, Parameters: []crossplaneaws.CustomParameter{ {ParameterName: &logical, @@ -563,6 +548,7 @@ func (r *DatabaseClaimReconciler) managePostgresParamGroup(ctx context.Context, } return pgName, nil } + func (r *DatabaseClaimReconciler) manageAuroraPostgresParamGroup(ctx context.Context, reqInfo *requestInfo, dbClaim *v1.DatabaseClaim) (string, error) { logr := log.FromContext(ctx) @@ -590,10 +576,6 @@ func (r *DatabaseClaimReconciler) manageAuroraPostgresParamGroup(ctx context.Con }, dbParamGroup) if err != nil { if errors.IsNotFound(err) { - validationError := params.CheckEngineVersion() - if validationError != nil { - return pgName, validationError - } dbParamGroup = &crossplaneaws.DBParameterGroup{ ObjectMeta: metav1.ObjectMeta{ Name: pgName, @@ -604,8 +586,8 @@ func (r *DatabaseClaimReconciler) manageAuroraPostgresParamGroup(ctx context.Con Description: &desc, CustomDBParameterGroupParameters: crossplaneaws.CustomDBParameterGroupParameters{ DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ - Engine: params.Engine, - EngineVersion: ¶ms.EngineVersion, + Engine: params.Type, + EngineVersion: ptr.To(getEngineVersion(params, r)), }, Parameters: []crossplaneaws.CustomParameter{ {ParameterName: &transactionTimeout, @@ -674,10 +656,6 @@ func (r *DatabaseClaimReconciler) manageClusterParamGroup(ctx context.Context, r }, dbParamGroup) if err != nil { if errors.IsNotFound(err) { - validationError := params.CheckEngineVersion() - if validationError != nil { - return pgName, validationError - } dbParamGroup = &crossplaneaws.DBClusterParameterGroup{ ObjectMeta: metav1.ObjectMeta{ Name: pgName, @@ -688,8 +666,8 @@ func (r *DatabaseClaimReconciler) manageClusterParamGroup(ctx context.Context, r Description: &desc, CustomDBClusterParameterGroupParameters: crossplaneaws.CustomDBClusterParameterGroupParameters{ DBParameterGroupFamilySelector: &crossplaneaws.DBParameterGroupFamilyNameSelector{ - Engine: params.Engine, - EngineVersion: ¶ms.EngineVersion, + Engine: params.Type, + EngineVersion: ptr.To(getEngineVersion(params, r)), }, Parameters: []crossplaneaws.CustomParameter{ {ParameterName: &logical, @@ -1095,3 +1073,13 @@ func HasOperationalTag(tags []*crossplaneaws.Tag) bool { } return false } + +func getEngineVersion(params *hostparams.HostParams, r *DatabaseClaimReconciler) string { + defaultMajorVersion := "" + if params.IsDefaultVersion { + defaultMajorVersion = basefun.GetDefaultMajorVersion(r.Config.Viper) + } else { + defaultMajorVersion = params.DBVersion + } + return defaultMajorVersion +} diff --git a/pkg/databaseclaim/databaseclaim.go b/pkg/databaseclaim/databaseclaim.go index 2662872b..f2e03689 100644 --- a/pkg/databaseclaim/databaseclaim.go +++ b/pkg/databaseclaim/databaseclaim.go @@ -629,7 +629,7 @@ func (r *DatabaseClaimReconciler) reconcileNewDB(ctx context.Context, reqInfo *r if dbClaim.Status.NewDB.MinStorageGB != reqInfo.HostParams.MinStorageGB { dbClaim.Status.NewDB.MinStorageGB = reqInfo.HostParams.MinStorageGB } - if reqInfo.HostParams.Engine == string(v1.Postgres) && int(dbClaim.Status.NewDB.MaxStorageGB) != int(reqInfo.HostParams.MaxStorageGB) { + if reqInfo.HostParams.Type == string(v1.Postgres) && int(dbClaim.Status.NewDB.MaxStorageGB) != int(reqInfo.HostParams.MaxStorageGB) { dbClaim.Status.NewDB.MaxStorageGB = reqInfo.HostParams.MaxStorageGB } } else { @@ -1116,11 +1116,11 @@ func (r *DatabaseClaimReconciler) getParameterGroupName(hostParams *hostparams.H switch dbType { case v1.Postgres: - return hostName + "-" + (strings.Split(hostParams.EngineVersion, "."))[0] + return hostName + "-" + (strings.Split(hostParams.DBVersion, "."))[0] case v1.AuroraPostgres: - return hostName + "-a-" + (strings.Split(hostParams.EngineVersion, "."))[0] + return hostName + "-a-" + (strings.Split(hostParams.DBVersion, "."))[0] default: - return hostName + "-" + (strings.Split(hostParams.EngineVersion, "."))[0] + return hostName + "-" + (strings.Split(hostParams.DBVersion, "."))[0] } } @@ -1340,11 +1340,11 @@ func updateHostPortStatus(status *v1.Status, host, port, sslMode string) { } func updateClusterStatus(status *v1.Status, hostParams *hostparams.HostParams) { - status.DBVersion = hostParams.EngineVersion - status.Type = v1.DatabaseType(hostParams.Engine) + status.DBVersion = hostParams.DBVersion + status.Type = v1.DatabaseType(hostParams.Type) status.Shape = hostParams.Shape status.MinStorageGB = hostParams.MinStorageGB - if hostParams.Engine == string(v1.Postgres) { + if hostParams.Type == string(v1.Postgres) { status.MaxStorageGB = hostParams.MaxStorageGB } } diff --git a/pkg/databaseclaim/gcpprovider.go b/pkg/databaseclaim/gcpprovider.go index 0f30c670..72cb1fd8 100644 --- a/pkg/databaseclaim/gcpprovider.go +++ b/pkg/databaseclaim/gcpprovider.go @@ -186,11 +186,6 @@ func (r *DatabaseClaimReconciler) manageDBClusterGCP(ctx context.Context, reqInf } logr.Info("creating_crossplane_dbcluster", "name", dbHostName) - validationError := params.CheckEngineVersion() - if validationError != nil { - logr.Error(validationError, "invalid_db_version") - return false, validationError - } dbCluster = &crossplanegcp.Cluster{ ObjectMeta: metav1.ObjectMeta{ Name: dbHostName, @@ -202,7 +197,7 @@ func (r *DatabaseClaimReconciler) manageDBClusterGCP(ctx context.Context, reqInf Enabled: ptr.To(true), }, - DatabaseVersion: getAlloyDBVersion(¶ms.EngineVersion), + DatabaseVersion: getAlloyDBVersion(¶ms.DBVersion), DeletionPolicy: ptr.To(string(params.DeletionPolicy)), @@ -312,10 +307,6 @@ func (r *DatabaseClaimReconciler) managePostgresDBInstanceGCP(ctx context.Contex }, dbInstance) if err != nil { if errors.IsNotFound(err) { - validationError := params.CheckEngineVersion() - if validationError != nil { - return false, validationError - } dbInstance = &crossplanegcp.Instance{ ObjectMeta: metav1.ObjectMeta{ Name: dbHostName, diff --git a/pkg/hostparams/hostparams.go b/pkg/hostparams/hostparams.go index 3b1b9a2a..2d028596 100644 --- a/pkg/hostparams/hostparams.go +++ b/pkg/hostparams/hostparams.go @@ -25,6 +25,7 @@ var ( // These values are purposely moved from the config file to the code to avoid accidental changes. defaultShape = "db.t4g.medium" defaultEngineVersion = "15.3" + defaultMajorVersion = "15" defaultEngine = v1.Postgres ) @@ -36,12 +37,12 @@ var ( type HostParams struct { // FIXME: this should be DatabaseType, not string - Engine string + Type string Shape string MinStorageGB int MaxStorageGB int64 - EngineVersion string + DBVersion string MasterUsername string InstanceClass string StorageType string @@ -54,11 +55,11 @@ type HostParams struct { isDefaultShape bool isDefaultInstanceClass bool isDefaultStorage bool - isDefaultVersion bool + IsDefaultVersion bool } func (p *HostParams) String() string { - return fmt.Sprintf("%s-%s-%s", p.Engine, p.InstanceClass, p.EngineVersion) + return fmt.Sprintf("%s-%s-%s", p.Type, p.InstanceClass, p.DBVersion) } func (p *HostParams) Hash() string { @@ -86,7 +87,7 @@ func (p *HostParams) HasInstanceClassChanged(activeInstanceClass string) bool { func (p *HostParams) HasStorageChanged(activeStorage int) bool { // storage is not applicable to aurora postgres - if p.Engine == defaultAuroraPostgresStr { + if p.Type == defaultAuroraPostgresStr { return false } if p.isDefaultStorage { @@ -99,27 +100,20 @@ func (p *HostParams) HasEngineChanged(activeEngine string) bool { if p.isDefaultEngine { return false } - return activeEngine != p.Engine + return activeEngine != p.Type } func (p *HostParams) HasVersionChanged(activeVersion string) bool { - if p.isDefaultVersion { + if p.IsDefaultVersion { return false } - return activeVersion != p.EngineVersion + return strings.Split(activeVersion, ".")[0] != strings.Split(p.DBVersion, ".")[0] } -func (p *HostParams) IsUpgradeRequested(np *HostParams) bool { - return p.HasEngineChanged(np.Engine) || - p.HasInstanceClassChanged(np.InstanceClass) || - p.HasVersionChanged(np.EngineVersion) -} - -func (p *HostParams) CheckEngineVersion() error { - if p.isDefaultVersion { - return ErrEngineVersionNotSpecified - } - return nil +func (p *HostParams) IsUpgradeRequested(active *HostParams) bool { + return p.HasEngineChanged(active.Type) || + p.HasInstanceClassChanged(active.InstanceClass) || + p.HasVersionChanged(active.DBVersion) } func New(config *viper.Viper, dbClaim *v1.DatabaseClaim) (*HostParams, error) { @@ -132,8 +126,8 @@ func New(config *viper.Viper, dbClaim *v1.DatabaseClaim) (*HostParams, error) { hostParams.DeletionPolicy = xpv1.DeletionPolicy( cases.Title(language.English, cases.Compact).String(string(dbClaim.Spec.DeletionPolicy))) - hostParams.Engine = string(dbClaim.Spec.Type) - hostParams.EngineVersion = dbClaim.Spec.DBVersion + hostParams.Type = string(dbClaim.Spec.Type) + hostParams.DBVersion = dbClaim.Spec.DBVersion hostParams.Shape = dbClaim.Spec.Shape hostParams.MinStorageGB = dbClaim.Spec.MinStorageGB hostParams.MaxStorageGB = dbClaim.Spec.MaxStorageGB @@ -149,12 +143,14 @@ func New(config *viper.Viper, dbClaim *v1.DatabaseClaim) (*HostParams, error) { hostParams.MasterUsername = config.GetString("defaultMasterUsername") } - if hostParams.EngineVersion == "" { + if hostParams.DBVersion == "" { if dbClaim.Status.ActiveDB.DBVersion != "" { - hostParams.EngineVersion = dbClaim.Status.ActiveDB.DBVersion + //for an existing (Status.ActiveDB NOT empty) it picks up the Status DBVersion, so no update is triggered. + hostParams.DBVersion = dbClaim.Status.ActiveDB.DBVersion } else { - hostParams.isDefaultVersion = true - hostParams.EngineVersion = defaultEngineVersion + //for a new Claim (Status.ActiveDB empty) it assumes 15 only + hostParams.IsDefaultVersion = true + hostParams.DBVersion = defaultMajorVersion } } @@ -164,9 +160,9 @@ func New(config *viper.Viper, dbClaim *v1.DatabaseClaim) (*HostParams, error) { hostParams.Shape = defaultShape } - if hostParams.Engine == "" { + if hostParams.Type == "" { hostParams.isDefaultEngine = true - hostParams.Engine = string(defaultEngine) + hostParams.Type = string(defaultEngine) } if hostParams.MinStorageGB == 0 { @@ -188,12 +184,12 @@ func New(config *viper.Viper, dbClaim *v1.DatabaseClaim) (*HostParams, error) { hostParams.EnableIAMDatabaseAuthentication = false hostParams.InstanceClass = getInstanceClass(hostParams.Shape) - hostParams.StorageType, err = getStorageType(config, hostParams.Engine, hostParams.Shape) + hostParams.StorageType, err = getStorageType(config, hostParams.Type, hostParams.Shape) if err != nil { return &HostParams{}, err } - if hostParams.Engine == string(v1.Postgres) { + if hostParams.Type == string(v1.Postgres) { if hostParams.MaxStorageGB == 0 { if dbClaim.Status.ActiveDB.MaxStorageGB != 0 { return &HostParams{}, ErrMaxStorageReduced @@ -213,8 +209,8 @@ func GetActiveHostParams(dbClaim *v1.DatabaseClaim) *HostParams { hostParams := HostParams{} - hostParams.Engine = string(dbClaim.Status.ActiveDB.Type) - hostParams.EngineVersion = dbClaim.Status.ActiveDB.DBVersion + hostParams.Type = string(dbClaim.Status.ActiveDB.Type) + hostParams.DBVersion = dbClaim.Status.ActiveDB.DBVersion hostParams.Shape = dbClaim.Status.ActiveDB.Shape hostParams.InstanceClass = getInstanceClass(hostParams.Shape) hostParams.MinStorageGB = dbClaim.Status.ActiveDB.MinStorageGB diff --git a/pkg/hostparams/hostparams_test.go b/pkg/hostparams/hostparams_test.go index 1f49b88d..c8df3079 100644 --- a/pkg/hostparams/hostparams_test.go +++ b/pkg/hostparams/hostparams_test.go @@ -3,8 +3,12 @@ package hostparams import ( "bytes" "reflect" + "strings" "testing" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" persistancev1 "github.com/infobloxopen/db-controller/api/v1" "github.com/spf13/viper" @@ -112,16 +116,23 @@ func TestHostParams_Hash(t *testing.T) { MinStorageGB: 20000, EngineVersion: "15.3", }, + }, {name: "test_Execute_postgres_15_tg4.medium", want: "416e183c", + fields: fields{Engine: "postgres", + Shape: "db.t4g.medium", + InstanceClass: "db.t4g.medium", + MinStorageGB: 20000, + EngineVersion: "15", + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { p := &HostParams{ - Engine: tt.fields.Engine, + Type: tt.fields.Engine, Shape: tt.fields.Shape, InstanceClass: tt.fields.InstanceClass, MinStorageGB: tt.fields.MinStorageGB, - EngineVersion: tt.fields.EngineVersion, + DBVersion: tt.fields.EngineVersion, MasterUsername: tt.fields.MasterUsername, SkipFinalSnapshotBeforeDeletion: tt.fields.SkipFinalSnapshotBeforeDeletion, PubliclyAccessible: tt.fields.PubliclyAccessible, @@ -131,7 +142,7 @@ func TestHostParams_Hash(t *testing.T) { isDefaultEngine: tt.fields.isDefaultEngine, isDefaultShape: tt.fields.isDefaultShape, isDefaultStorage: tt.fields.isDefaultStorage, - isDefaultVersion: tt.fields.isDefaultVersion, + IsDefaultVersion: tt.fields.isDefaultVersion, } if got := p.Hash(); got != tt.want { t.Errorf("HostParams.Hash() = %v, want %v", got, tt.want) @@ -156,10 +167,10 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { args: args{ config: NewConfig(testConfig), dbClaim: &persistancev1.DatabaseClaim{Spec: persistancev1.DatabaseClaimSpec{}}, - activeHostParams: &HostParams{Engine: "postgres", - Shape: "db.t4g.medium", - MinStorageGB: 20, - EngineVersion: "12.11", + activeHostParams: &HostParams{Type: "postgres", + Shape: "db.t4g.medium", + MinStorageGB: 20, + DBVersion: "12.11", }, }, }, @@ -173,11 +184,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { MinStorageGB: 20, DBVersion: "12.11", }}, - activeHostParams: &HostParams{Engine: "postgres", + activeHostParams: &HostParams{Type: "postgres", Shape: "db.t2.small", InstanceClass: "db.t2.small", MinStorageGB: 200, - EngineVersion: "12.11", + DBVersion: "12.11", }, }, }, @@ -190,11 +201,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { MinStorageGB: 20, DBVersion: "12.11", }}, - activeHostParams: &HostParams{Engine: "postgres", + activeHostParams: &HostParams{Type: "postgres", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "12.11", + DBVersion: "12.11", }, }, }, @@ -208,11 +219,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { MinStorageGB: 20, DBVersion: "12.11", }}, - activeHostParams: &HostParams{Engine: "postgres", + activeHostParams: &HostParams{Type: "postgres", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "13.11", + DBVersion: "13.11", }, }, }, @@ -221,11 +232,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { args: args{ config: NewConfig(testConfig), dbClaim: &persistancev1.DatabaseClaim{Spec: persistancev1.DatabaseClaimSpec{}}, - activeHostParams: &HostParams{Engine: "postgres", + activeHostParams: &HostParams{Type: "postgres", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "13.11", + DBVersion: "13.11", }, }, }, @@ -237,11 +248,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { Type: "aurora-postgresql", Shape: "db.t4g.different", }}, - activeHostParams: &HostParams{Engine: "postgres", + activeHostParams: &HostParams{Type: "postgres", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "13.11", + DBVersion: "13.11", }, }, }, @@ -254,11 +265,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { Shape: "db.t4g.medium", MinStorageGB: 20, }}, - activeHostParams: &HostParams{Engine: "postgres", + activeHostParams: &HostParams{Type: "postgres", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "13.11", + DBVersion: "13.11", }, }, }, @@ -271,11 +282,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { Shape: "db.t4g.medium", MinStorageGB: 20, }}, - activeHostParams: &HostParams{Engine: "aurora-postgresql", + activeHostParams: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "13.11", + DBVersion: "13.11", }, }, }, @@ -288,11 +299,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { Shape: "db.t4g.medium!io1", MinStorageGB: 20, }}, - activeHostParams: &HostParams{Engine: "aurora-postgresql", + activeHostParams: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium!io1", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "13.11", + DBVersion: "13.11", }, }, }, @@ -312,11 +323,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { }, }, }, - activeHostParams: &HostParams{Engine: "aurora-postgresql", + activeHostParams: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium!io1", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "15.7", + DBVersion: "15.7", }, }, }, @@ -336,11 +347,11 @@ func TestHostParams_IsUpgradeRequested(t *testing.T) { }, }, }, - activeHostParams: &HostParams{Engine: "aurora-postgresql", + activeHostParams: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium!io1", InstanceClass: "db.t4g.medium", MinStorageGB: 200, - EngineVersion: "15.3", + DBVersion: "15.3", }, }, }, @@ -381,11 +392,11 @@ func TestGetActiveHostParams(t *testing.T) { }, }, }, - want: &HostParams{Engine: "aurora-postgresql", + want: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 20, - EngineVersion: "12.11", + DBVersion: "12.11", }, }, {name: "ok - with instance class", @@ -405,11 +416,11 @@ func TestGetActiveHostParams(t *testing.T) { }, }, }, - want: &HostParams{Engine: "aurora-postgresql", + want: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 20, - EngineVersion: "12.11", + DBVersion: "12.11", }, }, {name: "ok - with instance class and storage type", @@ -429,11 +440,11 @@ func TestGetActiveHostParams(t *testing.T) { }, }, }, - want: &HostParams{Engine: "aurora-postgresql", + want: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium!io1", InstanceClass: "db.t4g.medium", MinStorageGB: 20, - EngineVersion: "12.11", + DBVersion: "12.11", }, }, } @@ -477,10 +488,10 @@ func TestNew(t *testing.T) { MinStorageGB: 20, }}, }, - want: &HostParams{Engine: "aurora-postgresql", + want: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium", MinStorageGB: 20, - EngineVersion: "12.11", + DBVersion: "12.11", InstanceClass: "db.t4g.medium", StorageType: "aurora", }, @@ -497,10 +508,10 @@ func TestNew(t *testing.T) { MinStorageGB: 20, }}, }, - want: &HostParams{Engine: "aurora-postgresql", + want: &HostParams{Type: "aurora-postgresql", Shape: "db.t4g.medium!io1", MinStorageGB: 20, - EngineVersion: "12.11", + DBVersion: "12.11", InstanceClass: "db.t4g.medium", StorageType: "aurora-iopt1", }, @@ -517,10 +528,10 @@ func TestNew(t *testing.T) { MinStorageGB: 20, }}, }, - want: &HostParams{Engine: "postgres", + want: &HostParams{Type: "postgres", Shape: "db.t4g.large", MinStorageGB: 20, - EngineVersion: "12.11", + DBVersion: "12.11", InstanceClass: "db.t4g.large", StorageType: "gp3", }, @@ -547,11 +558,11 @@ func TestNew(t *testing.T) { config: NewConfig(testConfig), dbClaim: &persistancev1.DatabaseClaim{Spec: persistancev1.DatabaseClaimSpec{}}, }, - want: &HostParams{Engine: "postgres", + want: &HostParams{Type: "postgres", Shape: "db.t4g.medium", InstanceClass: "db.t4g.medium", MinStorageGB: 42, - EngineVersion: "15.3", + DBVersion: "15", }, wantErr: false, }, @@ -605,11 +616,11 @@ func TestNew(t *testing.T) { }, }}, }, - want: &HostParams{Engine: "postgres", + want: &HostParams{Type: "postgres", Shape: "db.t4g.medium", MinStorageGB: 20, MaxStorageGB: 40, - EngineVersion: "12.11", + DBVersion: "12.11", InstanceClass: "db.t4g.medium", StorageType: "gp3", Port: 5432, @@ -653,11 +664,11 @@ func TestNew(t *testing.T) { }, }, }, - want: &HostParams{Engine: "postgres", + want: &HostParams{Type: "postgres", Shape: "db.t4g.medium", MinStorageGB: 20, MaxStorageGB: 0, - EngineVersion: "12.11", + DBVersion: "12.11", InstanceClass: "db.t4g.medium", StorageType: "gp3", Port: 5432, @@ -759,50 +770,26 @@ func TestDeletionPolicy(t *testing.T) { }) } } -func TestCheckEngineVersion(t *testing.T) { - type args struct { - config *viper.Viper - dbClaim *persistancev1.DatabaseClaim - } - tests := []struct { - name string - args args - want error - }{ - { - name: "test_default_EngineVersion", - args: args{ - config: NewConfig(testConfig), - dbClaim: &persistancev1.DatabaseClaim{Spec: persistancev1.DatabaseClaimSpec{ - Type: "aurora-postgresql", - Shape: "db.t4g.medium", - MinStorageGB: 20, - }}, - }, - want: ErrEngineVersionNotSpecified, - }, - { - name: "test_specific_EngineVersion", - args: args{ - config: NewConfig(testConfig), - dbClaim: &persistancev1.DatabaseClaim{Spec: persistancev1.DatabaseClaimSpec{ - Type: "aurora-postgresql", - DBVersion: "12.11", - Shape: "db.t4g.medium", - MinStorageGB: 20, - }}, - }, - want: nil, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - hp, _ := New(tt.args.config, tt.args.dbClaim) - if got := hp.CheckEngineVersion(); got != tt.want { - t.Errorf("CheckEngineVersion() = %v, want %v", got, tt.want) - } - }) - } + +func TestDBVersions(t *testing.T) { + RegisterFailHandler(Fail) + + dbVersion1 := "15" + + Expect(strings.Split(dbVersion1, ".")[0]).To(Equal("15")) + + dbVersion2 := "15.2" + + Expect(strings.Split(dbVersion2, ".")[0]).To(Equal("15")) + + dbVersion3 := "15.1.2" + + Expect(strings.Split(dbVersion3, ".")[0]).To(Equal("15")) + + dbVersion4 := "" + + Expect(strings.Split(dbVersion4, ".")[0]).To(Equal("")) + } func NewConfig(in []byte) *viper.Viper { diff --git a/pkg/pgctl/helper_test.go b/pkg/pgctl/helper_test.go deleted file mode 100644 index 6ff9c5e2..00000000 --- a/pkg/pgctl/helper_test.go +++ /dev/null @@ -1 +0,0 @@ -package pgctl diff --git a/pkg/roleclaim/mockclient.go b/pkg/roleclaim/mockclient.go index b0947a12..ad84686e 100644 --- a/pkg/roleclaim/mockclient.go +++ b/pkg/roleclaim/mockclient.go @@ -43,7 +43,7 @@ func (m *MockClient) Get(ctx context.Context, key client.ObjectKey, obj client.O _ = ctx var time8DaysAgo = metav1.Time{Time: time.Now().Add(-8 * 24 * time.Hour)} if (key.Namespace == "testNamespace" || key.Namespace == "testNamespaceWithDbIdentifierPrefix" || key.Namespace == "unitest" || key.Namespace == "schema-user-test") && - (key.Name == "sample-master-secret" || key.Name == "dbc-sample-connection" || key.Name == "dbc-sample-claim" || key.Name == "dbc-box-sample-claim" || key.Name == "test" || key.Name == "testclaim-1ec9b27c") { + (key.Name == "sample-master-secret" || key.Name == "dbc-sample-connection" || key.Name == "dbc-sample-claim" || key.Name == "dbc-box-sample-claim" || key.Name == "test" || key.Name == "testclaim-1ec9b27c" || key.Name == "testclaim-416e183c") { sec, ok := obj.(*corev1.Secret) if !ok { return fmt.Errorf("can't assert type") diff --git a/test/e2e/dbc_end2end_test.go b/test/e2e/dbc_end2end_test.go index 80df49ff..ad44821c 100644 --- a/test/e2e/dbc_end2end_test.go +++ b/test/e2e/dbc_end2end_test.go @@ -22,21 +22,21 @@ import ( "strings" "time" - crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" - persistanceinfobloxcomv1alpha1 "github.com/infobloxopen/db-controller/api/persistance.infoblox.com/v1alpha1" v1 "github.com/infobloxopen/db-controller/api/v1" "github.com/infobloxopen/db-controller/pkg/config" "github.com/infobloxopen/db-controller/pkg/hostparams" "github.com/infobloxopen/db-controller/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - crossplanegcp "github.com/upbound/provider-gcp/apis/alloydb/v1beta2" 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" + crossplaneaws "github.com/crossplane-contrib/provider-aws/apis/rds/v1alpha1" + persistanceinfobloxcomv1alpha1 "github.com/infobloxopen/db-controller/api/persistance.infoblox.com/v1alpha1" + crossplanegcp "github.com/upbound/provider-gcp/apis/alloydb/v1beta2" "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -54,12 +54,6 @@ var ( var _ = Describe("dbc-end2end", Ordered, func() { - db1 = namespace + "-db-1" - db2 = namespace + "-db-2" - - dbroleclaim1 = namespace + "-dbrc-1" - dbroleclaim2 = namespace + "-dbrc-2" - var ( newdbcMasterSecretName string rds1 string @@ -68,19 +62,27 @@ var _ = Describe("dbc-end2end", Ordered, func() { _, _ = dbinstance1update, dbinstance2 - rds1 = env + "-" + db1 + "-1ec9b27c" - newdbcMasterSecretName = rds1 + "-master" - logf.Log.Info("Starting test", "timeout", timeout_e2e, "interval", interval_e2e) //creates db_1 Context("Creating a DB", func() { - It("Creating a DBClaim", func() { + newdbcMasterSecretName = rds1 + "-master" + It("Creating a DBClaim with invalid version", func() { + + Expect(namespace).NotTo(BeEmpty(), "Namespace is empty") + Expect(env).NotTo(BeEmpty(), "Env is empty") + // TODO:check why this is not being initialized correctly + db1 = namespace + "-db-1" + db2 = namespace + "-db-2" + + dbroleclaim1 = namespace + "-dbrc-1" + dbroleclaim2 = namespace + "-dbrc-2" + rds1 = env + "-" + db1 + "-1ec9b27c" Expect(db1).NotTo(BeEmpty()) key := types.NamespacedName{ - Name: db1, + Name: env + "-" + db1, Namespace: namespace, } @@ -127,64 +129,46 @@ var _ = Describe("dbc-end2end", Ordered, func() { err := k8sClient.Get(ctx, types.NamespacedName{Name: dbinstance1}, dbInst) Expect(errors.IsNotFound(err)).To(BeTrue(), "crossplane cr: %s exists, please delete", dbinstance1) - Expect(k8sClient.Create(ctx, dbClaim)).Should(Succeed()) - - By("status error includes engine version not specified error") - Eventually(func() (string, error) { - Expect(k8sClient.Get(ctx, key, dbClaim)).ToNot(HaveOccurred()) - Expect(dbClaim.Spec.DBVersion).To(BeEmpty()) - return dbClaim.Status.Error, nil - }, 2*time.Minute, 250*time.Millisecond).Should(Equal(hostparams.ErrEngineVersionNotSpecified.Error())) - }) - - It("Updating a databaseclaim to have an invalid dbVersion", func() { - By("erroring out when AWS/GCP does not support dbVersion") - key := types.NamespacedName{ - Name: db1, - Namespace: namespace, - } invalidVersion := "15.3" if cloud == "gcp" { invalidVersion = "15.1" } - prevDbClaim := &v1.DatabaseClaim{} - By("Getting the prev dbclaim") - Expect(k8sClient.Get(ctx, key, prevDbClaim)).Should(Succeed()) - By(fmt.Sprintf("Updating with version dbVersion: %s", invalidVersion)) - prevDbClaim.Spec.DBVersion = invalidVersion - Expect(k8sClient.Update(ctx, prevDbClaim)).Should(Succeed()) - updatedDbClaim := &v1.DatabaseClaim{} - By("Check that .spec.dbVersion is set") - Expect(k8sClient.Get(ctx, key, updatedDbClaim)).Should(Succeed()) - Expect(updatedDbClaim.Spec.DBVersion).To(Equal(invalidVersion)) + dbClaim.Spec.DBVersion = invalidVersion + + Expect(k8sClient.Create(ctx, dbClaim)).Should(Succeed()) By(fmt.Sprintf("checking %s status.error message is not empty", key.String())) Eventually(func() string { - Expect(k8sClient.Get(ctx, key, updatedDbClaim)).Should(Succeed()) - return updatedDbClaim.Status.Error + Expect(k8sClient.Get(ctx, key, dbClaim)).Should(Succeed()) + return dbClaim.Status.Error }, time.Minute*3, time.Second*3).ShouldNot(ContainSubstring(v1.ErrInvalidDBVersion.Error())) + }) - It("Updating dbclaim to valid db version", func() { + It("Updating a databaseclaim", func() { + By("setting DBVersion to empty should use value 15") key := types.NamespacedName{ - Name: db1, + Name: env + "-" + db1, Namespace: namespace, } - prevDbClaim := &v1.DatabaseClaim{} - By("Update DBC CR") - Eventually(func() error { - Expect(k8sClient.Get(ctx, key, prevDbClaim)).Should(Succeed()) - prevDbClaim.Spec.DBVersion = "15" - if cloud == "aws" { - prevDbClaim.Spec.DBVersion = "15.5" - } - return k8sClient.Update(ctx, prevDbClaim) - }, 30*time.Second, 100*time.Millisecond).Should(Succeed()) + prevDbClaim := &v1.DatabaseClaim{} + By("Getting the prev dbclaim") + Expect(k8sClient.Get(ctx, key, prevDbClaim)).Should(Succeed()) + By("Updating with version dbVersion: ") + newDeploy := prevDbClaim.DeepCopy() + newDeploy.Spec.DBVersion = "" + Expect(k8sClient.Patch(ctx, newDeploy, client.MergeFrom(prevDbClaim))).Should(Succeed()) updatedDbClaim := &v1.DatabaseClaim{} - By(fmt.Sprintf("checking dbclaim %s status is ready", db1)) + + By("should create an instance using default version 15: " + dbinstance1) + Eventually(func() (v1.DbState, error) { + Expect(k8sClient.Get(ctx, key, updatedDbClaim)).ToNot(HaveOccurred()) + Expect(updatedDbClaim.Spec.DBVersion).To(Equal("")) + return updatedDbClaim.Status.ActiveDB.DbState, nil + }, timeout_e2e, interval_e2e).Should(Equal(v1.Ready)) + Expect(k8sClient.Get(ctx, key, updatedDbClaim)).Should(Succeed()) - // FIXME: replace this hash naming scheme { wd, err := utils.GetProjectDir() Expect(err).ToNot(HaveOccurred()) @@ -200,6 +184,7 @@ var _ = Describe("dbc-end2end", Ordered, func() { dbInst := utils.DBInstanceType(cloud) return k8sClient.Get(ctx, types.NamespacedName{Name: dbinstance1update}, dbInst) }, timeout_e2e, interval_e2e).Should(Succeed()) + By("Check Crossplane CR is ready: " + dbinstance1update) Eventually(func() bool { dbInst := utils.DBInstanceType(cloud) @@ -226,6 +211,7 @@ var _ = Describe("dbc-end2end", Ordered, func() { Eventually(func() error { return k8sClient.Get(ctx, types.NamespacedName{Name: dbinstance1update, Namespace: namespace}, &creds) }, 30*time.Second, 500*time.Millisecond).Should(BeNil()) + // TODO: this secret is empty on aws // Eventually(func() string { // Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dbinstance1, Namespace: namespace}, &creds)).Should(Succeed()) @@ -298,7 +284,7 @@ var _ = Describe("dbc-end2end", Ordered, func() { SecretName: secretName, SourceDatabaseClaim: &v1.SourceDatabaseClaim{ Namespace: namespace, - Name: db1, + Name: env + "-" + db1, }, SchemaRoleMap: map[string]v1.RoleType{ "schemaapp111": v1.ReadOnly, @@ -355,7 +341,7 @@ var _ = Describe("dbc-end2end", Ordered, func() { SecretName: secretName, SourceDatabaseClaim: &v1.SourceDatabaseClaim{ Namespace: namespace, - Name: db1, + Name: env + "-" + db1, }, SchemaRoleMap: map[string]v1.RoleType{ "schemaapp444": v1.ReadOnly, @@ -420,7 +406,7 @@ var _ = Describe("dbc-end2end", Ordered, func() { k8sClient.Create(ctx, existingDBMasterSecret) By("successfully processing an useExisting dbClaim") key = types.NamespacedName{ - Name: db2, + Name: env + "-" + db2, Namespace: namespace, } dbClaim := &v1.DatabaseClaim{ @@ -493,7 +479,7 @@ var _ = Describe("dbc-end2end", Ordered, func() { }) By("Removing the useExistingFlag in dbclaim and forcing the use of cached secret") key := types.NamespacedName{ - Name: db2, + Name: env + "-" + db2, Namespace: namespace, } existingDbClaim := &v1.DatabaseClaim{} @@ -541,7 +527,7 @@ var _ = Describe("dbc-end2end", Ordered, func() { } key := types.NamespacedName{ - Name: db2, + Name: env + "-" + db2, Namespace: namespace, } type testState struct { @@ -749,24 +735,14 @@ var _ = Describe("dbc-end2end", Ordered, func() { } // delete Secrets within this namespace - secrets := &corev1.SecretList{} - if err := k8sClient.List(ctx, secrets, client.InNamespace(namespace)); err != nil { - Expect(err).To(BeNil()) - } - for _, secret := range secrets.Items { - By("Deleting Secret: " + secret.Name) - k8sClient.Delete(ctx, &secret) - } - - //delete the namespace - // Don't delete a namespace in a test - // By("deleting the namespace") - // k8sClient.Delete(ctx, &corev1.Namespace{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: namespace, - // }, - // }) - + // secrets := &corev1.SecretList{} + // if err := k8sClient.List(ctx, secrets, client.InNamespace(namespace)); err != nil { + // Expect(err).To(BeNil()) + // } + // for _, secret := range secrets.Items { + // By("Deleting Secret: " + secret.Name) + // k8sClient.Delete(ctx, &secret) + // } } _ = afterall }) diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 1f229586..44532eb8 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -98,6 +98,7 @@ var _ = BeforeSuite(func() { // read kubectl context from the k8sClient env, err = utils.GetKubeContext() + Expect(env).NotTo(BeEmpty()) Expect(err).NotTo(HaveOccurred()) // Check if current context is box-3, kind or gcp-ddi-dev-use1