[3.2.0 backport] CBG-4106 CBG-4105 : remove database if removeCorruptConfigIfExists fails

rest/admin_api.go

@@ -126,6 +126,9 @@ func (h *handler) handleCreateDB() error {
 		// if it used to be corrupt we need to remove it from the invalid database map on server context and remove the old corrupt config from the bucket
 		err = h.removeCorruptConfigIfExists(contextNoCancel.Ctx, bucket, h.server.Config.Bootstrap.ConfigGroupID, dbName)
 		if err != nil {
+			// we cannot continue on with database creation with possibility of the corrupt database config in the bucket for this db
+			// thus we need to unload the requested database config to prevent the cluster being in an inconsistent state
+			h.server._removeDatabase(contextNoCancel.Ctx, dbName)
 			return err
 		}
 		cas, err := h.server.BootstrapContext.InsertConfig(contextNoCancel.Ctx, bucket, h.server.Config.Bootstrap.ConfigGroupID, &persistedConfig)

rest/config_test.go

@@ -30,6 +30,7 @@ import (
 	"runtime"
 	"strings"
 	"testing"
+	"time"
 
 	"golang.org/x/crypto/bcrypt"
 	"gopkg.in/square/go-jose.v2"
@@ -2928,3 +2929,143 @@ func makeScopesConfigWithDefault(scopeName string, collections []string) *Scopes
 	scopesConfig := makeScopesConfig(scopeName, collections)
 	return &scopesConfig
 }
+
+// TestInvalidDbConfigNoLongerPresentInBucket:
+//   - Create rest tester with large config poll interval
+//   - Create valid db
+//   - Alter config in bucket to make it invalid
+//   - Force config poll, assert it is picked up as invalid db config
+//   - Delete the invalid db config form the bucket
+//   - Force config poll reload and assert the invalid db is cleared
+func TestInvalidDbConfigNoLongerPresentInBucket(t *testing.T) {
+	if base.UnitTestUrlIsWalrus() {
+		t.Skip("test only works with CBS, requires bootstrap connection")
+	}
+	rt := NewRestTester(t, &RestTesterConfig{
+		CustomTestBucket: base.GetTestBucket(t),
+		PersistentConfig: true,
+		MutateStartupConfig: func(config *StartupConfig) {
+			// configure the interval time to not run
+			config.Bootstrap.ConfigUpdateFrequency = base.NewConfigDuration(10 * time.Minute)
+		},
+		DatabaseConfig: nil,
+	})
+	defer rt.Close()
+	realBucketName := rt.CustomTestBucket.GetName()
+	ctx := base.TestCtx(t)
+	const dbName = "db1"
+
+	// create db with correct config
+	dbConfig := rt.NewDbConfig()
+	resp := rt.CreateDatabase(dbName, dbConfig)
+	RequireStatus(t, resp, http.StatusCreated)
+
+	// wait for db to come online
+	require.NoError(t, rt.WaitForDBOnline())
+
+	// grab the persisted db config from the bucket
+	databaseConfig := DatabaseConfig{}
+	_, err := rt.ServerContext().BootstrapContext.GetConfig(rt.Context(), realBucketName, rt.ServerContext().Config.Bootstrap.ConfigGroupID, "db1", &databaseConfig)
+	require.NoError(t, err)
+
+	// update the persisted config to a fake bucket name
+	newBucketName := "fakeBucket"
+	_, err = rt.UpdatePersistedBucketName(&databaseConfig, &newBucketName)
+	require.NoError(t, err)
+
+	// force reload of configs from bucket
+	rt.ServerContext().ForceDbConfigsReload(t, ctx)
+
+	// assert the config is picked as invalid db config
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		invalidDatabases := rt.ServerContext().AllInvalidDatabaseNames(t)
+		assert.Equal(c, 1, len(invalidDatabases))
+		assert.Equal(c, 0, len(rt.ServerContext().dbConfigs))
+	}, time.Second*10, time.Millisecond*100)
+
+	// remove the invalid config from the bucket
+	rt.RemoveDbConfigFromBucket(dbName, realBucketName)
+
+	// force reload of configs from bucket
+	rt.ServerContext().ForceDbConfigsReload(t, ctx)
+
+	// assert the config is removed from tracking
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		invalidDatabases := rt.ServerContext().AllInvalidDatabaseNames(t)
+		assert.Equal(c, 0, len(invalidDatabases))
+		assert.Equal(c, 0, len(rt.ServerContext().dbConfigs))
+	}, time.Second*10, time.Millisecond*100)
+
+	// create db again, should succeed
+	resp = rt.CreateDatabase(dbName, dbConfig)
+	RequireStatus(t, resp, http.StatusCreated)
+}
+
+// TestNotFoundOnInvalidDatabase:
+//   - Create rest tester with large config polling interval
+//   - Insert a bad dbConfig into the bucket
+//   - Manually fetch and load db from buckets
+//   - Assert that the bad config is tracked as invalid config
+//   - Delete the bad config manually and attempt to correct the db config through create db endpoint
+//   - Assert db is removed form invalid db's and is now a running database on server context
+func TestNotFoundOnInvalidDatabase(t *testing.T) {
+	rt := NewRestTester(t, &RestTesterConfig{
+		CustomTestBucket: base.GetTestBucket(t),
+		PersistentConfig: true,
+		MutateStartupConfig: func(config *StartupConfig) {
+			// configure the interval time to not run
+			config.Bootstrap.ConfigUpdateFrequency = base.NewConfigDuration(100 * time.Second)
+		},
+		DatabaseConfig: nil,
+	})
+	defer rt.Close()
+	realBucketName := rt.CustomTestBucket.GetName()
+
+	// create a new invalid db config and persist to bucket
+	badName := "badBucketName"
+	dbConfig := rt.NewDbConfig()
+	dbConfig.Name = "db1"
+
+	version, err := GenerateDatabaseConfigVersionID(rt.Context(), "", &dbConfig)
+	require.NoError(t, err)
+	metadataID, metadataIDError := rt.ServerContext().BootstrapContext.ComputeMetadataIDForDbConfig(base.TestCtx(t), &dbConfig)
+	require.NoError(t, metadataIDError)
+
+	// insert the db config with bad bucket name
+	dbConfig.Bucket = &badName
+	persistedConfig := DatabaseConfig{
+		Version:    version,
+		MetadataID: metadataID,
+		DbConfig:   dbConfig,
+		SGVersion:  base.ProductVersion.String(),
+	}
+	rt.InsertDbConfigToBucket(&persistedConfig, rt.CustomTestBucket.GetName())
+
+	// manually fetch and load db configs from bucket
+	_, err = rt.ServerContext().fetchAndLoadConfigs(rt.Context(), false)
+	require.NoError(t, err)
+
+	// assert the config is picked as invalid db config
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		invalidDatabases := rt.ServerContext().AllInvalidDatabaseNames(t)
+		assert.Equal(c, 1, len(invalidDatabases))
+	}, time.Second*10, time.Millisecond*100)
+
+	resp := rt.SendAdminRequest(http.MethodGet, "/db1/", "")
+	RequireStatus(t, resp, http.StatusNotFound)
+	assert.Contains(t, resp.Body.String(), "You must update database config immediately")
+
+	// delete the invalid db config to force the not found error
+	rt.RemoveDbConfigFromBucket(dbConfig.Name, realBucketName)
+
+	// fix the bucket name and try fix corrupt db through create db endpoint
+	dbConfig.Bucket = &realBucketName
+	RequireStatus(t, rt.CreateDatabase(dbConfig.Name, dbConfig), http.StatusCreated)
+
+	// assert the config is remove the invalid config and we have a running db
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		invalidDatabases := rt.ServerContext().AllInvalidDatabaseNames(t)
+		assert.Equal(c, 0, len(invalidDatabases))
+		assert.Equal(c, 1, len(rt.ServerContext().dbConfigs))
+	}, time.Second*10, time.Millisecond*100)
+}

rest/handler.go

@@ -704,7 +704,7 @@ func (h *handler) removeCorruptConfigIfExists(ctx context.Context, bucket, confi
 	}
 	// remove the bad config from the bucket
 	err := h.server.BootstrapContext.DeleteConfig(ctx, bucket, configGroupID, dbName)
-	if err != nil {
+	if err != nil && !base.IsDocNotFoundError(err) {
 		return err
 	}
 	// delete the database name form the invalid database map on server context

rest/utilities_testing_resttester.go

@@ -323,6 +323,11 @@ func (rt *RestTester) InsertDbConfigToBucket(config *DatabaseConfig, bucketName
 	require.NoError(rt.TB(), insertErr)
 }
 
+func (rt *RestTester) RemoveDbConfigFromBucket(dbName string, bucketName string) {
+	deleteErr := rt.ServerContext().BootstrapContext.DeleteConfig(base.TestCtx(rt.TB()), bucketName, rt.ServerContext().Config.Bootstrap.ConfigGroupID, dbName)
+	require.NoError(rt.TB(), deleteErr)
+}
+
 func (rt *RestTester) PersistDbConfigToBucket(dbConfig DbConfig, bucketName string) {
 	version, err := GenerateDatabaseConfigVersionID(rt.Context(), "", &dbConfig)
 	require.NoError(rt.TB(), err)