PPBM-1645 Trigger storage resync only for critical fields

cmd/pbm/config.go

@@ -5,8 +5,6 @@ import (
 	"fmt"
 	"log"
 	"os"
-	"reflect"
-	"strings"
 	"time"
 
 	"go.mongodb.org/mongo-driver/mongo"
@@ -62,31 +60,30 @@ func runConfig(
 
 	switch {
 	case len(c.set) > 0:
+		oldCfg, err := pbm.GetConfig(ctx)
+		if err != nil {
+			if !errors.Is(err, mongo.ErrNoDocuments) {
+				return nil, errors.Wrap(err, "unable to get current config")
+			}
+			oldCfg = &config.Config{}
+		}
+
 		var o confVals
-		rsnc := false
 		for k, v := range c.set {
 			err := config.SetConfigVar(ctx, conn, k, v)
 			if err != nil {
 				return nil, errors.Wrapf(err, "set %s", k)
 			}
 			o = append(o, confKV{k, v})
+		}
 
-			path := strings.Split(k, ".")
-			if !rsnc && len(path) > 0 && path[0] == "storage" {
-				rsnc = true
-			}
+		newCfg, err := pbm.GetConfig(ctx)
+		if err != nil {
+			return nil, errors.Wrap(err, "unable to get updated config")
 		}
-		if rsnc {
-			cid, err := pbm.SyncFromStorage(ctx, false)
-			if err != nil {
-				return nil, errors.Wrap(err, "resync")
-			}
 
-			if c.wait {
-				if err := waitForResyncWithTimeout(ctx, pbm, cid, c.waitTime); err != nil {
-					return nil, err
-				}
-			}
+		if err := resyncIfNeeded(ctx, pbm, oldCfg, newCfg, c); err != nil {
+			return nil, err
 		}
 		return o, nil
 	case len(c.key) > 0:
@@ -138,18 +135,8 @@ func runConfig(
 			return nil, errors.Wrap(err, "unable to set config: write to db")
 		}
 
-		// resync storage only if Storage options have changed
-		if !reflect.DeepEqual(newCfg.Storage, oldCfg.Storage) {
-			cid, err := pbm.SyncFromStorage(ctx, false)
-			if err != nil {
-				return nil, errors.Wrap(err, "resync")
-			}
-
-			if c.wait {
-				if err := waitForResyncWithTimeout(ctx, pbm, cid, c.waitTime); err != nil {
-					return nil, err
-				}
-			}
+		if err := resyncIfNeeded(ctx, pbm, oldCfg, newCfg, c); err != nil {
+			return nil, err
 		}
 
 		return newCfg, nil
@@ -171,3 +158,20 @@ func readConfigFromFile(filename string) (*config.Config, error) {
 
 	return config.Parse(file)
 }
+
+func resyncIfNeeded(ctx context.Context, pbm *sdk.Client, oldCfg, newCfg *config.Config, c *configOpts) error {
+	if newCfg.Storage.IsSameStorage(&oldCfg.Storage) {
+		return nil
+	}
+
+	cid, err := pbm.SyncFromStorage(ctx, false)
+	if err != nil {
+		return errors.Wrap(err, "resync")
+	}
+
+	if !c.wait {
+		return nil
+	}
+
+	return waitForResyncWithTimeout(ctx, pbm, cid, c.waitTime)
+}

pbm/config/config_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/percona/percona-backup-mongodb/pbm/storage/oss"
 	"github.com/testcontainers/testcontainers-go"
 	"github.com/testcontainers/testcontainers-go/modules/mongodb"
 	"go.mongodb.org/mongo-driver/mongo"
@@ -43,9 +44,10 @@ func TestIsSameStorage(t *testing.T) {
 			InsecureSkipTLSVerify: false,
 		}
 		eq := &s3.Config{
-			Region: "eu",
-			Bucket: "b1",
-			Prefix: "p1",
+			Region:      "eu",
+			Bucket:      "b1",
+			Prefix:      "p1",
+			EndpointURL: "ep.com",
 		}
 		if !cfg.IsSameStorage(eq) {
 			t.Errorf("config storage should identify the same instance: cfg=%+v, eq=%+v", cfg, eq)
@@ -68,6 +70,12 @@ func TestIsSameStorage(t *testing.T) {
 		if cfg.IsSameStorage(neq) {
 			t.Errorf("storage instances has different prefix: cfg=%+v, eq=%+v", cfg, neq)
 		}
+
+		neq = cfg.Clone()
+		neq.EndpointURL = "ep2.com"
+		if cfg.IsSameStorage(neq) {
+			t.Errorf("storage instances has different EndpointURL: cfg=%+v, eq=%+v", cfg, neq)
+		}
 	})
 
 	t.Run("Azure", func(t *testing.T) {
@@ -82,9 +90,10 @@ func TestIsSameStorage(t *testing.T) {
 		}
 
 		eq := &azure.Config{
-			Account:   "a1",
-			Container: "c1",
-			Prefix:    "p1",
+			Account:     "a1",
+			Container:   "c1",
+			Prefix:      "p1",
+			EndpointURL: "az.com",
 		}
 		if !cfg.IsSameStorage(eq) {
 			t.Errorf("config storage should identify the same instance: cfg=%+v, eq=%+v", cfg, eq)
@@ -107,6 +116,12 @@ func TestIsSameStorage(t *testing.T) {
 		if cfg.IsSameStorage(neq) {
 			t.Errorf("storage instances has different prefix: cfg=%+v, eq=%+v", cfg, neq)
 		}
+
+		neq = cfg.Clone()
+		neq.EndpointURL = "az2.com"
+		if cfg.IsSameStorage(neq) {
+			t.Errorf("storage instances has different EndpointURL: cfg=%+v, eq=%+v", cfg, neq)
+		}
 	})
 
 	t.Run("GCS", func(t *testing.T) {
@@ -217,6 +232,59 @@ func TestIsSameStorage(t *testing.T) {
 			t.Errorf("storage instances has different prefix: cfg=%+v, eq=%+v", cfg, neq)
 		}
 	})
+
+	t.Run("oss", func(t *testing.T) {
+		cfg := &oss.Config{
+			Region:      "eu",
+			EndpointURL: "ep.com",
+			Bucket:      "b1",
+			Prefix:      "p1",
+			Credentials: oss.Credentials{
+				AccessKeyID:     "k1",
+				AccessKeySecret: "k2",
+				SecurityToken:   "sect",
+			},
+			ConnectTimeout:       10 * time.Second,
+			UploadPartSize:       6 << 20,
+			MaxObjSizeGB:         floatPtr(1.1),
+			Retryer:              &oss.Retryer{},
+			ServerSideEncryption: &oss.SSE{},
+		}
+		eq := &oss.Config{
+			Region:      "eu",
+			EndpointURL: "ep.com",
+			Bucket:      "b1",
+			Prefix:      "p1",
+		}
+		if !cfg.IsSameStorage(eq) {
+			t.Errorf("config storage should identify the same instance: cfg=%+v, eq=%+v, diff=%s",
+				cfg, eq, cmp.Diff(*cfg, *eq))
+		}
+
+		neq := cfg.Clone()
+		neq.Region = "us"
+		if cfg.IsSameStorage(neq) {
+			t.Errorf("storage instances has different region: cfg=%+v, eq=%+v", cfg, neq)
+		}
+
+		neq = cfg.Clone()
+		neq.EndpointURL = "ep2.com"
+		if cfg.IsSameStorage(neq) {
+			t.Errorf("storage instances has different EndpointURL: cfg=%+v, eq=%+v", cfg, neq)
+		}
+
+		neq = cfg.Clone()
+		neq.Bucket = "b2"
+		if cfg.IsSameStorage(neq) {
+			t.Errorf("storage instances has different bucket: cfg=%+v, eq=%+v", cfg, neq)
+		}
+
+		neq = cfg.Clone()
+		neq.Prefix = "p2"
+		if cfg.IsSameStorage(neq) {
+			t.Errorf("storage instances has different prefix: cfg=%+v, eq=%+v", cfg, neq)
+		}
+	})
 }
 
 func TestCastError(t *testing.T) {

pbm/storage/azure/config.go

@@ -70,6 +70,13 @@ func (cfg *Config) IsSameStorage(other *Config) bool {
 	if cfg.Prefix != other.Prefix {
 		return false
 	}
+	if cfg.EndpointURL != other.EndpointURL {
+		return false
+	}
+	if !maps.Equal(cfg.EndpointURLMap, other.EndpointURLMap) {
+		return false
+	}
+
 	return true
 }
 

pbm/storage/oss/client.go

@@ -44,9 +44,9 @@
 }
 
 type SSE struct {
 	EncryptionMethod    string `bson:"encryptionMethod,omitempty" json:"encryptionMethod,omitempty" yaml:"encryptionMethod,omitempty"`
 	EncryptionAlgorithm string `bson:"encryptionAlgorithm,omitempty" json:"encryptionAlgorithm,omitempty" yaml:"encryptionAlgorithm,omitempty"`
 	EncryptionKeyID     string `bson:"encryptionKeyId,omitempty" json:"encryptionKeyId,omitempty" yaml:"encryptionKeyId,omitempty"`
 }
 
 type Retryer struct {
@@ -78,6 +78,10 @@
 	if cfg.Prefix != other.Prefix {
 		return false
 	}
+	if cfg.EndpointURL != other.EndpointURL {
+		return false
+	}
+
 	return true
 }
 

pbm/storage/s3/s3.go

@@ -196,6 +196,12 @@ func (cfg *Config) IsSameStorage(other *Config) bool {
 	if cfg.Prefix != other.Prefix {
 		return false
 	}
+	if cfg.EndpointURL != other.EndpointURL {
+		return false
+	}
+	if !maps.Equal(cfg.EndpointURLMap, other.EndpointURLMap) {
+		return false
+	}
 	return true
 }