From d7cb677915e5c959cedd49dcb6d51c07edbc45e5 Mon Sep 17 00:00:00 2001 From: Abhishek Bodhekar <108131161+abodhekar@users.noreply.github.com> Date: Tue, 21 Nov 2023 15:50:03 +0530 Subject: [PATCH] Revert "adding auto-scalling feature to rds postgres instance" This reverts commit 9d0a687abe99270e0a4dee4ca4344d08c4866d92. --- controllers/databaseclaim_controller.go | 43 +-- controllers/databaseclaim_controller_test.go | 284 +------------------ pkg/hostparams/hostparams.go | 2 - 3 files changed, 10 insertions(+), 319 deletions(-) diff --git a/controllers/databaseclaim_controller.go b/controllers/databaseclaim_controller.go index b2f1527d..73e72565 100644 --- a/controllers/databaseclaim_controller.go +++ b/controllers/databaseclaim_controller.go @@ -74,8 +74,6 @@ const ( masterSecretSuffix = "-master" masterPasswordKey = "password" ErrMaxNameLen = Error("dbclaim name is too long. max length is 44 characters") - ErrMaxStorageReduced = Error("reducing .spec.MaxStorageGB value is not allowed") - ErrMaxStorageLesser = Error(".spec.MaxStorageGB should always be greater or equel to spec.minStorageGB") // InfoLevel is used to set V level to 0 as suggested by official docs // https://github.com/kubernetes-sigs/controller-runtime/blob/main/TMP-LOGGING.md InfoLevel = 0 @@ -245,12 +243,6 @@ func (r *DatabaseClaimReconciler) getMode(dbClaim *persistancev1.DatabaseClaim) return M_UseNewDB } -// checkIfMaxStorageReduced checks if the MaxStorageGB has been reduced or disabled compared to earlier state. -// This can be done by comparing it with MaxStorageGB in the status. -func (r *DatabaseClaimReconciler) checkIfMaxStorageReduced(dbClaim *persistancev1.DatabaseClaim) bool { - return r.Input.HostParams.MaxStorageGB < dbClaim.Status.ActiveDB.MaxStorageGB || r.Input.HostParams.MaxStorageGB < dbClaim.Status.NewDB.MaxStorageGB -} - func (r *DatabaseClaimReconciler) setReqInfo(dbClaim *persistancev1.DatabaseClaim) error { logr := r.Log.WithValues("databaseclaim", dbClaim.Namespace+"/"+dbClaim.Name, "func", "setReqInfo") @@ -336,22 +328,6 @@ func (r *DatabaseClaimReconciler) setReqInfo(dbClaim *persistancev1.DatabaseClai r.Input.EnableReplicationRole = *dbClaim.Spec.EnableReplicationRole } - if hostParams.Engine == defaultPostgresStr { - if r.checkIfMaxStorageReduced(dbClaim) { - // if MaxStorageGB was present in earlier state, and now is being reduced or removed. This Should throw an error - return ErrMaxStorageReduced - } else if r.Input.HostParams.MaxStorageGB == 0 { - // If MaxStorageGB is currently not present or zero, and it was not present earlier as well, then we assign MinStorageGB value to it - // marking autoStorageScalling as disabled - r.Input.HostParams.MaxStorageGB = int64(r.Input.HostParams.MinStorageGB) - } else if r.Input.HostParams.MaxStorageGB != 0 { - if r.Input.HostParams.MaxStorageGB < int64(r.Input.HostParams.MinStorageGB) { - // If MaxStorageGB is being provided, but it is lasser than minStorage, This should not be allowed. - return ErrMaxStorageLesser - } - } - } - logr.V(DebugLevel).Info("setup values of ", "DatabaseClaimReconciler", r) return nil } @@ -387,7 +363,6 @@ func (r *DatabaseClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err := r.setReqInfo(&dbClaim); err != nil { return r.manageError(ctx, &dbClaim, err) } - // name of our custom finalizer dbFinalizerName := "databaseclaims.persistance.atlas.infoblox.com/finalizer" @@ -693,9 +668,6 @@ func (r *DatabaseClaimReconciler) reconcileNewDB(ctx context.Context, dbClaim.Status.NewDB = *dbClaim.Status.ActiveDB.DeepCopy() if dbClaim.Status.NewDB.MinStorageGB != r.Input.HostParams.MinStorageGB { dbClaim.Status.NewDB.MinStorageGB = r.Input.HostParams.MinStorageGB - if r.Input.HostParams.Engine == defaultPostgresStr { - dbClaim.Status.NewDB.MaxStorageGB = r.Input.HostParams.MaxStorageGB - } } } else { updateClusterStatus(&dbClaim.Status.NewDB, &r.Input.HostParams) @@ -1905,12 +1877,11 @@ func (r *DatabaseClaimReconciler) managePostgresDBInstance(ctx context.Context, EngineVersion: ¶ms.EngineVersion, }, // Items from Claim and fragmentKey - Engine: ¶ms.Engine, - MultiAZ: &multiAZ, - DBInstanceClass: ¶ms.Shape, - AllocatedStorage: &ms64, - MaxAllocatedStorage: ¶ms.MaxStorageGB, - Tags: DBClaimTags(dbClaim.Spec.Tags).DBTags(), + Engine: ¶ms.Engine, + MultiAZ: &multiAZ, + DBInstanceClass: ¶ms.Shape, + AllocatedStorage: &ms64, + Tags: DBClaimTags(dbClaim.Spec.Tags).DBTags(), // Items from Config MasterUsername: ¶ms.MasterUsername, PubliclyAccessible: ¶ms.PubliclyAccessible, @@ -2421,7 +2392,6 @@ func (r *DatabaseClaimReconciler) updateDBInstance(ctx context.Context, dbClaim params := &r.Input.HostParams ms64 := int64(params.MinStorageGB) dbInstance.Spec.ForProvider.AllocatedStorage = &ms64 - dbInstance.Spec.ForProvider.MaxAllocatedStorage = ¶ms.MaxStorageGB dbInstance.Spec.ForProvider.EnableCloudwatchLogsExports = r.Input.EnableCloudwatchLogsExport dbInstance.Spec.ForProvider.MultiAZ = &multiAZ } @@ -2713,9 +2683,6 @@ func updateClusterStatus(status *persistancev1.Status, hostParams *hostparams.Ho status.Type = persistancev1.DatabaseType(hostParams.Engine) status.Shape = hostParams.Shape status.MinStorageGB = hostParams.MinStorageGB - if hostParams.Engine == defaultPostgresStr { - status.MaxStorageGB = hostParams.MaxStorageGB - } } func getServiceNamespace() (string, error) { diff --git a/controllers/databaseclaim_controller_test.go b/controllers/databaseclaim_controller_test.go index 2f3ffd3d..4cb4f90d 100644 --- a/controllers/databaseclaim_controller_test.go +++ b/controllers/databaseclaim_controller_test.go @@ -1191,97 +1191,6 @@ func TestDatabaseClaimReconciler_getDynamicHostName(t *testing.T) { }) } } - -func TestCheckIfMaxStorageReduced(t *testing.T) { - testSuite := []struct { - testName string - r *DatabaseClaimReconciler - dbc *persistancev1.DatabaseClaim - expected bool - }{ - { - testName: "OK", - r: &DatabaseClaimReconciler{ - Input: &input{ - HostParams: hostparams.HostParams{ - MaxStorageGB: 100, - }, - }, - }, - dbc: &persistancev1.DatabaseClaim{ - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{ - MaxStorageGB: 100, - }, - NewDB: persistancev1.Status{ - MaxStorageGB: 100, - }, - }, - }, - expected: false, - }, - { - testName: "OK", - r: &DatabaseClaimReconciler{ - Input: &input{ - HostParams: hostparams.HostParams{ - MaxStorageGB: 200, - }, - }, - }, - dbc: &persistancev1.DatabaseClaim{ - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{ - MaxStorageGB: 100, - }, - NewDB: persistancev1.Status{ - MaxStorageGB: 100, - }, - }, - }, - expected: false, - }, - { - testName: "MaxStorageGB reduced", - r: &DatabaseClaimReconciler{ - Input: &input{ - HostParams: hostparams.HostParams{ - MaxStorageGB: 200, - }, - }, - }, - dbc: &persistancev1.DatabaseClaim{ - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{ - MaxStorageGB: 300, - }, - }, - }, - expected: true, - }, - { - testName: "fresh dbc, no values provided", - r: &DatabaseClaimReconciler{ - Input: &input{ - HostParams: hostparams.HostParams{}, - }, - }, - dbc: &persistancev1.DatabaseClaim{ - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{}, - }, - }, - expected: false, - }, - } - - for _, test := range testSuite { - if got := test.r.checkIfMaxStorageReduced(test.dbc); got != test.expected { - t.Errorf("checkIfMaxStorageReduced failed. test name = %v, expected = %v , got = %v ", test.testName, test.expected, got) - } - } -} - func TestDatabaseClaimReconciler_setReqInfo(t *testing.T) { opts := zap.Options{ Development: true, @@ -1301,11 +1210,10 @@ func TestDatabaseClaimReconciler_setReqInfo(t *testing.T) { dbClaim *persistancev1.DatabaseClaim } tests := []struct { - name string - fields fields - args args - want error - wantMaxStorageGB int + name string + fields fields + args args + want error }{ { "OK", @@ -1329,7 +1237,6 @@ func TestDatabaseClaimReconciler_setReqInfo(t *testing.T) { }, }, nil, - 20, }, { "Dbname too long", @@ -1353,183 +1260,6 @@ func TestDatabaseClaimReconciler_setReqInfo(t *testing.T) { }, }, ErrMaxNameLen, - 20, - }, - { - "MaxStorageGB reduced", - fields{ - Config: NewConfig(multiConfig), - Log: zap.New(zap.UseFlagOptions(&opts)), - DbIdentifierPrefix: "boxing-x", - Input: &input{FragmentKey: "", - HostParams: hostparams.HostParams{Engine: "postgres", - EngineVersion: "12.11", - Shape: "db.t4g.medium", - MinStorageGB: 20}}, - }, - args{ - dbClaim: &persistancev1.DatabaseClaim{ - ObjectMeta: v1.ObjectMeta{Name: "44characterlengthname23456789012345678901234"}, - Spec: persistancev1.DatabaseClaimSpec{ - AppID: "identity", - DatabaseName: "identity", - EnableReplicationRole: &flse, - MaxStorageGB: 60}, - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{ - MaxStorageGB: 100, - }, - }, - }, - }, - ErrMaxStorageReduced, - 20, - }, - { - "MaxStorageGB removed", - fields{ - Config: NewConfig(multiConfig), - Log: zap.New(zap.UseFlagOptions(&opts)), - DbIdentifierPrefix: "boxing-x", - Input: &input{FragmentKey: "", - HostParams: hostparams.HostParams{Engine: "postgres", - EngineVersion: "12.11", - Shape: "db.t4g.medium", - MinStorageGB: 20}}, - }, - args{ - dbClaim: &persistancev1.DatabaseClaim{ - ObjectMeta: v1.ObjectMeta{Name: "44characterlengthname23456789012345678901234"}, - Spec: persistancev1.DatabaseClaimSpec{ - AppID: "identity", - DatabaseName: "identity", - EnableReplicationRole: &flse, - }, - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{ - MaxStorageGB: 100, - }, - }, - }, - }, - ErrMaxStorageReduced, - 100, - }, - { - "ok, MaxStorageGB stays same", - fields{ - Config: NewConfig(multiConfig), - Log: zap.New(zap.UseFlagOptions(&opts)), - DbIdentifierPrefix: "boxing-x", - Input: &input{FragmentKey: "", - HostParams: hostparams.HostParams{Engine: "postgres", - EngineVersion: "12.11", - Shape: "db.t4g.medium", - MinStorageGB: 20}}, - }, - args{ - dbClaim: &persistancev1.DatabaseClaim{ - ObjectMeta: v1.ObjectMeta{Name: "44characterlengthname23456789012345678901234"}, - Spec: persistancev1.DatabaseClaimSpec{ - AppID: "identity", - DatabaseName: "identity", - EnableReplicationRole: &flse, - MaxStorageGB: 60}, - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{ - MaxStorageGB: 60, - }, - }, - }, - }, - nil, - 60, - }, - { - "ok, MaxStorageGB increased ", - fields{ - Config: NewConfig(multiConfig), - Log: zap.New(zap.UseFlagOptions(&opts)), - DbIdentifierPrefix: "boxing-x", - Input: &input{FragmentKey: "", - HostParams: hostparams.HostParams{Engine: "postgres", - EngineVersion: "12.11", - Shape: "db.t4g.medium", - MinStorageGB: 20}}, - }, - args{ - dbClaim: &persistancev1.DatabaseClaim{ - ObjectMeta: v1.ObjectMeta{Name: "44characterlengthname23456789012345678901234"}, - Spec: persistancev1.DatabaseClaimSpec{ - AppID: "identity", - DatabaseName: "identity", - EnableReplicationRole: &flse, - MaxStorageGB: 100}, - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{ - MaxStorageGB: 60, - }, - }, - }, - }, - nil, - 100, - }, - { - "MaxStorageGB lesser", - fields{ - Config: NewConfig(multiConfig), - Log: zap.New(zap.UseFlagOptions(&opts)), - DbIdentifierPrefix: "boxing-x", - Input: &input{FragmentKey: "", - HostParams: hostparams.HostParams{Engine: "postgres", - EngineVersion: "12.11", - Shape: "db.t4g.medium", - }}, - }, - args{ - dbClaim: &persistancev1.DatabaseClaim{ - ObjectMeta: v1.ObjectMeta{Name: "44characterlengthname23456789012345678901234"}, - Spec: persistancev1.DatabaseClaimSpec{ - AppID: "identity", - DatabaseName: "identity", - EnableReplicationRole: &flse, - MaxStorageGB: 10}, - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{}, - }, - }, - }, - ErrMaxStorageLesser, - 10, - }, - { - "ok", - fields{ - Config: NewConfig(multiConfig), - Log: zap.New(zap.UseFlagOptions(&opts)), - DbIdentifierPrefix: "boxing-x", - Input: &input{FragmentKey: "", - HostParams: hostparams.HostParams{Engine: "postgres", - EngineVersion: "12.11", - Shape: "db.t4g.medium", - }}, - }, - args{ - dbClaim: &persistancev1.DatabaseClaim{ - ObjectMeta: v1.ObjectMeta{Name: "44characterlengthname23456789012345678901234"}, - Spec: persistancev1.DatabaseClaimSpec{ - AppID: "identity", - DatabaseName: "identity", - EnableReplicationRole: &flse, - }, - Status: persistancev1.DatabaseClaimStatus{ - ActiveDB: persistancev1.Status{}, - }, - }, - }, - nil, - 20, }, } for _, tt := range tests { @@ -1544,13 +1274,9 @@ func TestDatabaseClaimReconciler_setReqInfo(t *testing.T) { Mode: tt.fields.Mode, Input: tt.fields.Input, } - got := r.setReqInfo(tt.args.dbClaim) - if got != tt.want { + if got := r.setReqInfo(tt.args.dbClaim); got != tt.want { t.Errorf("DatabaseClaimReconciler.setReqInfo() = %v, want %v", got, tt.want) } - if got == nil && tt.wantMaxStorageGB != int(r.Input.HostParams.MaxStorageGB) { - t.Errorf("hostParams.MaxStorageGB = %v, want %v ", r.Input.HostParams.MaxStorageGB, tt.wantMaxStorageGB) - } }) } } diff --git a/pkg/hostparams/hostparams.go b/pkg/hostparams/hostparams.go index 9362d1ca..fc723962 100644 --- a/pkg/hostparams/hostparams.go +++ b/pkg/hostparams/hostparams.go @@ -19,7 +19,6 @@ type HostParams struct { Engine string Shape string MinStorageGB int - MaxStorageGB int64 EngineVersion string MasterUsername string SkipFinalSnapshotBeforeDeletion bool @@ -97,7 +96,6 @@ func New(config *viper.Viper, fragmentKey string, dbClaim *persistancev1.Databas hostParams.Shape = dbClaim.Spec.Shape hostParams.MinStorageGB = dbClaim.Spec.MinStorageGB port = dbClaim.Spec.Port - hostParams.MaxStorageGB = dbClaim.Spec.MaxStorageGB } else { hostParams.MasterUsername = config.GetString(fmt.Sprintf("%s::masterUsername", fragmentKey)) hostParams.Engine = config.GetString(fmt.Sprintf("%s::Engine", fragmentKey))