fix(sync): enable exponential HTTP retry backoff via maxRetryDelay (#4175)

Add optional maxRetryDelay to sync registry config and pass separate
init/max values to regclient WithDelay instead of using retryDelay for
both. When maxRetryDelay is unset, it defaults to retryDelay so existing
configs keep a fixed HTTP retry interval. Validate that maxRetryDelay
requires retryDelay and is not less than it.

Update examples and docs for the new option.

Signed-off-by: Andrei Aaron <andreifdaaron@gmail.com>

Author: andaaron

examples/README.md

@@ -1358,7 +1358,8 @@ Configure each registry sync:
 				"tlsVerify": true,                  # whether or not to verify tls (default is true)
 				"certDir": "/home/user/certs",      # use certificates at certDir path similar to Docker's /etc/docker/certs.d., if not specified then use the default certs dir,
 				"maxRetries": 5,                    # maxRetries in case of temporary errors (default: no retries)
-				"retryDelay": "10m",                # delay between retries, retry options are applied for both on demand and periodically sync and retryDelay is mandatory when using maxRetries.
+				"retryDelay": "1s",                 # initial HTTP retry delay; mandatory when using maxRetries
+				"maxRetryDelay": "30s",             # max HTTP retry backoff; optional, defaults to retryDelay (fixed interval). Set higher than retryDelay for exponential backoff.
 				"onlySigned": true,                 # sync only signed images (either notary or cosign)
 				"content":[                         # which content to periodically pull, also it's used for filtering ondemand images, if not set then periodically polling will not run
 					{
@@ -1405,7 +1406,8 @@ Configure each registry sync:
 				"onDemand": true,                     # doesn't have content, don't periodically pull, pull just on demand.
 				"tlsVerify": true,
 				"maxRetries": 3,                      
-				"retryDelay": "15m"
+				"retryDelay": "15m",                # initial HTTP retry delay; fixed 15m interval unless maxRetryDelay is set higher
+				"maxRetryDelay": "15m"              # optional; omit or set equal to retryDelay for fixed interval (as here)
 			}
 		]
 		}

examples/config-sync.json

@@ -74,7 +74,8 @@
 					"onDemand": true,
 					"tlsVerify": true,
 					"maxRetries": 5,
-					"retryDelay": "30s",
+					"retryDelay": "1s",
+					"maxRetryDelay": "30s",
 					"syncTimeout": "10m"
 				},
 				{

pkg/cli/server/root.go

@@ -1644,6 +1644,23 @@ func validateSync(config *config.Config, logger zlog.Logger) error {
 				return fmt.Errorf("%w: %s", zerr.ErrBadConfig, msg)
 			}
 
+			if regCfg.MaxRetryDelay != nil && regCfg.RetryDelay == nil {
+				msg := "retryDelay is required when using maxRetryDelay"
+				logger.Error().Err(zerr.ErrBadConfig).Int("id", regID).Interface("extensions.sync.registries[id]",
+					extensionsConfig.Sync.Registries[regID]).Msg(msg)
+
+				return fmt.Errorf("%w: %s", zerr.ErrBadConfig, msg)
+			}
+
+			if regCfg.MaxRetryDelay != nil && regCfg.RetryDelay != nil &&
+				*regCfg.MaxRetryDelay < *regCfg.RetryDelay {
+				msg := "maxRetryDelay must be greater than or equal to retryDelay"
+				logger.Error().Err(zerr.ErrBadConfig).Int("id", regID).Interface("extensions.sync.registries[id]",
+					extensionsConfig.Sync.Registries[regID]).Msg(msg)
+
+				return fmt.Errorf("%w: %s", zerr.ErrBadConfig, msg)
+			}
+
 			// check preserveDigest without compat
 			if regCfg.PreserveDigest && !config.IsCompatEnabled() {
 				msg := "can not use PreserveDigest option without enabling http.Compat"

pkg/cli/server/root_test.go

@@ -1850,6 +1850,49 @@ storage:
 		So(err, ShouldNotBeNil)
 	})
 
+	Convey("Test verify sync with maxRetryDelay without retryDelay", t, func(c C) {
+		content := `{"storage":{"rootDirectory":"/tmp/zot"},
+							"http":{"address":"127.0.0.1","port":"8080","realm":"zot",
+							"auth":{"htpasswd":{"path":"test/data/htpasswd"},"failDelay":1}},
+							"extensions":{"sync": {"registries": [{"urls":["localhost:9999"],
+							"maxRetryDelay": "30s"}]}}}`
+		tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+
+		os.Args = []string{"cli_test", "verify", tmpfile}
+		err := cli.NewServerRootCmd().Execute()
+		So(err, ShouldNotBeNil)
+		So(err, ShouldWrap, zerr.ErrBadConfig)
+		So(err.Error(), ShouldContainSubstring, "retryDelay is required when using maxRetryDelay")
+	})
+
+	Convey("Test verify sync with maxRetryDelay less than retryDelay", t, func(c C) {
+		content := `{"storage":{"rootDirectory":"/tmp/zot"},
+							"http":{"address":"127.0.0.1","port":"8080","realm":"zot",
+							"auth":{"htpasswd":{"path":"test/data/htpasswd"},"failDelay":1}},
+							"extensions":{"sync": {"registries": [{"urls":["localhost:9999"],
+							"retryDelay": "30s", "maxRetryDelay": "1s"}]}}}`
+		tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+
+		os.Args = []string{"cli_test", "verify", tmpfile}
+		err := cli.NewServerRootCmd().Execute()
+		So(err, ShouldNotBeNil)
+		So(err, ShouldWrap, zerr.ErrBadConfig)
+		So(err.Error(), ShouldContainSubstring, "maxRetryDelay must be greater than or equal to retryDelay")
+	})
+
+	Convey("Test verify sync with valid maxRetryDelay", t, func(c C) {
+		content := `{"storage":{"rootDirectory":"/tmp/zot"},
+							"http":{"address":"127.0.0.1","port":"8080","realm":"zot",
+							"auth":{"htpasswd":{"path":"test/data/htpasswd"},"failDelay":1}},
+							"extensions":{"sync": {"registries": [{"urls":["localhost:9999"],
+							"maxRetries": 3, "retryDelay": "1s", "maxRetryDelay": "30s"}]}}}`
+		tmpfile := MakeTempFileWithContent(t, "zot-test.json", content)
+
+		os.Args = []string{"cli_test", "verify", tmpfile}
+		err := cli.NewServerRootCmd().Execute()
+		So(err, ShouldBeNil)
+	})
+
 	Convey("Test verify config with unknown keys", t, func(c C) {
 		content := `{"distSpecVersion": "1.0.0", "storage": {"rootDirectory": "/tmp/zot"},
 							"http": {"url": "127.0.0.1", "port": "8080"},

pkg/cli/server/schema_test.go

@@ -64,6 +64,7 @@ func TestSchemaAllowsNullForPointerFields(t *testing.T) {
 						map[string]any{
 							"tlsVerify":            nil,
 							"retryDelay":           nil,
+							"maxRetryDelay":        nil,
 							"onlySigned":           nil,
 							"syncLegacyCosignTags": nil,
 						},

pkg/extensions/config/sync/config.go

@@ -31,6 +31,7 @@ type RegistryConfig struct {
 	CertDir               string
 	MaxRetries            *int
 	RetryDelay            *time.Duration
+	MaxRetryDelay         *time.Duration // max HTTP retry backoff; when unset defaults to retryDelay (fixed delay)
 	OnlySigned            *bool
 	SyncLegacyCosignTags  *bool // when unset, defaults to true
 	CredentialHelper      string

pkg/extensions/sync/service.go

@@ -806,6 +806,24 @@ func getTLSConfigOption(url *url.URL, tlsVerify *bool) config.TLSConf {
 	return tls
 }
 
+// httpRetryDelayBounds returns regclient WithDelay arguments for HTTP retry backoff.
+// When maxRetryDelay is unset, delayMax defaults to delayInit so existing configs keep a fixed retry interval.
+// Set maxRetryDelay greater than retryDelay to enable exponential backoff up to that cap.
+func httpRetryDelayBounds(opts syncconf.RegistryConfig) (time.Duration, time.Duration, bool) {
+	if opts.RetryDelay == nil {
+		return 0, 0, false
+	}
+
+	delayInit := *opts.RetryDelay
+	delayMax := delayInit
+
+	if opts.MaxRetryDelay != nil {
+		delayMax = *opts.MaxRetryDelay
+	}
+
+	return delayInit, delayMax, true
+}
+
 func newClient(opts syncconf.RegistryConfig, credentials syncconf.CredentialsFile, logger log.Logger,
 ) (*regclient.RegClient, []config.Host, error) {
 	urls, err := parseRegistryURLs(opts.URLs)
@@ -889,8 +907,8 @@ func newClient(opts syncconf.RegistryConfig, credentials syncconf.CredentialsFil
 		regOpts = append(regOpts, reg.WithRetryLimit(*opts.MaxRetries))
 	}
 
-	if opts.RetryDelay != nil {
-		regOpts = append(regOpts, reg.WithDelay(*opts.RetryDelay, *opts.RetryDelay))
+	if delayInit, delayMax, ok := httpRetryDelayBounds(opts); ok {
+		regOpts = append(regOpts, reg.WithDelay(delayInit, delayMax))
 	}
 
 	// Configure transport with timeouts to prevent indefinite hangs.

pkg/extensions/sync/sync_internal_test.go

@@ -1387,3 +1387,34 @@ func TestNewClientTimeoutBehavior(t *testing.T) {
 		})
 	})
 }
+
+func TestHTTPRetryDelayBounds(t *testing.T) {
+	Convey("httpRetryDelayBounds maps sync config to regclient delay args", t, func() {
+		retryDelay := 1 * time.Second
+		maxRetryDelay := 30 * time.Second
+
+		Convey("no retryDelay returns ok=false", func() {
+			delayInit, delayMax, ok := httpRetryDelayBounds(syncconf.RegistryConfig{})
+			So(ok, ShouldBeFalse)
+			So(delayInit, ShouldEqual, time.Duration(0))
+			So(delayMax, ShouldEqual, time.Duration(0))
+		})
+
+		Convey("retryDelay only defaults max to init (fixed interval)", func() {
+			delayInit, delayMax, ok := httpRetryDelayBounds(syncconf.RegistryConfig{RetryDelay: &retryDelay})
+			So(ok, ShouldBeTrue)
+			So(delayInit, ShouldEqual, retryDelay)
+			So(delayMax, ShouldEqual, retryDelay)
+		})
+
+		Convey("maxRetryDelay greater than retryDelay enables exponential backoff cap", func() {
+			delayInit, delayMax, ok := httpRetryDelayBounds(syncconf.RegistryConfig{
+				RetryDelay:    &retryDelay,
+				MaxRetryDelay: &maxRetryDelay,
+			})
+			So(ok, ShouldBeTrue)
+			So(delayInit, ShouldEqual, retryDelay)
+			So(delayMax, ShouldEqual, maxRetryDelay)
+		})
+	})
+}