From 77f30fa193228ccaa233cd97bf03cb485e5b2c6c Mon Sep 17 00:00:00 2001 From: Johannes Malsam Date: Thu, 4 May 2023 10:59:37 +0200 Subject: [PATCH] Revert "ROX-13692: create final snapshot for tenant db (#994)" This reverts commit 06e11f6d4e0c6ec0589f3d16146ee0804322b4f0. --- .github/workflows/rds.yaml | 3 +- Makefile | 2 +- .../central/cloudprovider/awsclient/rds.go | 16 ++---- .../cloudprovider/awsclient/rds_test.go | 53 +++---------------- 4 files changed, 13 insertions(+), 61 deletions(-) diff --git a/.github/workflows/rds.yaml b/.github/workflows/rds.yaml index f6101d5f5c..c16b2252f7 100644 --- a/.github/workflows/rds.yaml +++ b/.github/workflows/rds.yaml @@ -32,7 +32,6 @@ on: - 'docs/**' - 'pkg/api/openapi/docs/**' - 'pkg/api/openapi/.openapi-generator-ignore' - - 'dp-terraform/**' jobs: verify-test: @@ -68,4 +67,4 @@ jobs: AWS_AUTH_HELPER: "none" run: | ./dev/env/scripts/exec_fleetshard_sync.sh make test/rds - timeout-minutes: 50 + timeout-minutes: 35 diff --git a/Makefile b/Makefile index f43ba5b476..2bedbc3eef 100644 --- a/Makefile +++ b/Makefile @@ -326,7 +326,7 @@ test: $(GOTESTSUM_BIN) # Runs the AWS RDS integration tests. test/rds: $(GOTESTSUM_BIN) RUN_RDS_TESTS=true \ - $(GOTESTSUM_BIN) --junitfile data/results/rds-integration-tests.xml --format $(GOTESTSUM_FORMAT) -- -p 1 -v -timeout 45m -count=1 \ + $(GOTESTSUM_BIN) --junitfile data/results/rds-integration-tests.xml --format $(GOTESTSUM_FORMAT) -- -p 1 -v -timeout 30m -count=1 \ ./fleetshard/pkg/central/cloudprovider/awsclient/... .PHONY: test/rds diff --git a/fleetshard/pkg/central/cloudprovider/awsclient/rds.go b/fleetshard/pkg/central/cloudprovider/awsclient/rds.go index cb5ce9e98e..a0d232aff2 100644 --- a/fleetshard/pkg/central/cloudprovider/awsclient/rds.go +++ b/fleetshard/pkg/central/cloudprovider/awsclient/rds.go @@ -166,6 +166,7 @@ func (r *RDS) ensureInstanceDeleted(instanceID string) error { if instanceStatus != dbDeletingStatus { glog.Infof("Initiating deprovisioning of RDS database instance %s.", instanceID) + // TODO(ROX-13692): do not skip taking a final DB snapshot _, err := r.rdsClient.DeleteDBInstance(newDeleteCentralDBInstanceInput(instanceID, true)) if err != nil { return fmt.Errorf("deleting DB instance: %w", err) @@ -186,7 +187,8 @@ func (r *RDS) ensureClusterDeleted(clusterID string) error { if clusterStatus != dbDeletingStatus { glog.Infof("Initiating deprovisioning of RDS database cluster %s.", clusterID) - _, err := r.rdsClient.DeleteDBCluster(newDeleteCentralDBClusterInput(clusterID, false)) + // TODO(ROX-13692): do not skip taking a final DB snapshot + _, err := r.rdsClient.DeleteDBCluster(newDeleteCentralDBClusterInput(clusterID, true)) if err != nil { return fmt.Errorf("deleting DB cluster: %w", err) } @@ -364,16 +366,10 @@ func newDeleteCentralDBInstanceInput(instanceID string, skipFinalSnapshot bool) } func newDeleteCentralDBClusterInput(clusterID string, skipFinalSnapshot bool) *rds.DeleteDBClusterInput { - input := &rds.DeleteDBClusterInput{ + return &rds.DeleteDBClusterInput{ DBClusterIdentifier: aws.String(clusterID), SkipFinalSnapshot: aws.Bool(skipFinalSnapshot), } - - if !skipFinalSnapshot { - input.FinalDBSnapshotIdentifier = getFinalSnapshotID(clusterID) - } - - return input } func newRdsClient(awsConfig config.AWS, auth fleetmanager.Auth) (*rds.RDS, error) { @@ -399,10 +395,6 @@ func newRdsClient(awsConfig config.AWS, auth fleetmanager.Auth) (*rds.RDS, error return rds.New(sess), nil } -func getFinalSnapshotID(clusterID string) *string { - return aws.String(fmt.Sprintf("%s-%s", clusterID, "final")) -} - type tokenFetcher struct { auth fleetmanager.Auth } diff --git a/fleetshard/pkg/central/cloudprovider/awsclient/rds_test.go b/fleetshard/pkg/central/cloudprovider/awsclient/rds_test.go index 9c76300068..ecc67b6453 100644 --- a/fleetshard/pkg/central/cloudprovider/awsclient/rds_test.go +++ b/fleetshard/pkg/central/cloudprovider/awsclient/rds_test.go @@ -17,7 +17,7 @@ import ( "github.com/stretchr/testify/require" ) -const awsTimeoutMinutes = 30 +const awsTimeoutMinutes = 15 func newTestRDS() (*RDS, error) { rdsClient, err := newTestRDSClient() @@ -47,7 +47,7 @@ func newTestRDSClient() (*rds.RDS, error) { func waitForClusterToBeDeleted(ctx context.Context, rdsClient *RDS, clusterID string) (bool, error) { for { - clusterExists, _, err := rdsClient.clusterStatus(clusterID) + clusterExists, clusterStatus, err := rdsClient.clusterStatus(clusterID) if err != nil { return false, err } @@ -56,6 +56,11 @@ func waitForClusterToBeDeleted(ctx context.Context, rdsClient *RDS, clusterID st return true, nil } + // exit early if cluster is marked as deleting + if clusterStatus == dbDeletingStatus { + return true, nil + } + ticker := time.NewTicker(awsRetrySeconds * time.Second) select { case <-ticker.C: @@ -66,37 +71,6 @@ func waitForClusterToBeDeleted(ctx context.Context, rdsClient *RDS, clusterID st } } -func waitForFinalSnapshotToExist(ctx context.Context, rdsClient *RDS, clusterID string) (bool, error) { - - ticker := time.NewTicker(awsRetrySeconds * time.Second) - for { - select { - case <-ticker.C: - snapshotOut, err := rdsClient.rdsClient.DescribeDBClusterSnapshots(&rds.DescribeDBClusterSnapshotsInput{ - DBClusterSnapshotIdentifier: getFinalSnapshotID(clusterID), - }) - - if err != nil { - if awsErr, ok := err.(awserr.Error); ok { - if awsErr.Code() != rds.ErrCodeDBClusterSnapshotNotFoundFault { - return false, err - } - - continue - } - } - - if snapshotOut != nil { - return len(snapshotOut.DBClusterSnapshots) == 1, nil - } - case <-ctx.Done(): - return false, fmt.Errorf("waiting for final DB snapshot: %w", ctx.Err()) - } - - } - -} - func TestRDSProvisioning(t *testing.T) { if os.Getenv("RUN_RDS_TESTS") != "true" { t.Skip("Skip RDS tests. Set RUN_RDS_TESTS=true env variable to enable RDS tests.") @@ -162,19 +136,6 @@ func TestRDSProvisioning(t *testing.T) { clusterDeleted, err := waitForClusterToBeDeleted(deleteCtx, rdsClient, clusterID) require.NoError(t, err) assert.True(t, clusterDeleted) - - // Always attemt to delete the final snapshot if it exists - defer func() { - _, err := rdsClient.rdsClient.DeleteDBClusterSnapshot( - &rds.DeleteDBClusterSnapshotInput{DBClusterSnapshotIdentifier: getFinalSnapshotID(clusterID)}, - ) - - assert.NoError(t, err) - }() - - snapshotExists, err := waitForFinalSnapshotToExist(deleteCtx, rdsClient, clusterID) - require.NoError(t, err) - require.True(t, snapshotExists) } func TestGetDBConnection(t *testing.T) {