From 0f94f03ec11e84e40f3c02aec45734476a54a63c Mon Sep 17 00:00:00 2001
From: arewm <arewm@users.noreply.github.com>
Date: Mon, 17 Feb 2025 15:29:18 -0500
Subject: [PATCH] Add configuration for custom bundle resolver backoff
A bundle resolver is susceptible to flakes in network communication. In
order to increase reliability, we can add backoff retries to mitigate
transient issues.
While it may be possible to add a generic retry functionality around
resolvers, implementation will likely be simpler if the retry
functionality is isolated to the individual resolvers. Future work is
needed to add retries to git resolver requests.
Partially addresses #8571
Signed-off-by: arewm <arewm@users.noreply.github.com>
---
docs/bundle-resolver.md | 11 ++-
pkg/resolution/resolver/bundle/bundle.go | 12 ++-
pkg/resolution/resolver/bundle/config.go | 91 ++++++++++++++++++-
.../resolver/bundle/resolver_test.go | 39 ++++++++
4 files changed, 147 insertions(+), 6 deletions(-)
diff --git a/docs/bundle-resolver.md b/docs/bundle-resolver.md
index 262963f48d6..a3321fab634 100644
--- a/docs/bundle-resolver.md
+++ b/docs/bundle-resolver.md
@@ -36,9 +36,14 @@ for the name, namespace and defaults that the resolver ships with.
### Options
-| Option Name | Description | Example Values |
-|---------------------------|--------------------------------------------------------------|-----------------------|
-| `default-kind` | The default layer kind in the bundle image. | `task`, `pipeline` |
+| Option Name | Description | Example Values |
+|----------------------|-------------------------------------------------------------------|-----------------------|
+| `backoff-duration` | The initial duration for a backoff. | `500ms`, `2s` |
+| `backoff-factor` | The factor by which the sleep duration increases every step. | `2.5`, `4.0` |
+| `backoff-jitter` | A random amount of additioan sleep between 0 andduration * jitter.| `0.1`, `0.5` |
+| `backoff-steps` | The number of backoffs to attempt. | `3`, `7` |
+| `backoff-cap` | The maxumum backoff duration. If reached, remaining steps are zeroed.| `10s`, `20s` |
+| `default-kind` | The default layer kind in the bundle image. | `task`, `pipeline` |
## Usage
diff --git a/pkg/resolution/resolver/bundle/bundle.go b/pkg/resolution/resolver/bundle/bundle.go
index bc6a7fed07f..8174db2fbfd 100644
--- a/pkg/resolution/resolver/bundle/bundle.go
+++ b/pkg/resolution/resolver/bundle/bundle.go
@@ -142,9 +142,17 @@ func retrieveImage(ctx context.Context, keychain authn.Keychain, ref string) (st
if err != nil {
return "", nil, fmt.Errorf("%s is an unparseable image reference: %w", ref, err)
}
+ customRetryBackoff, err := GetBundleResolverBackoff(ctx)
+ if err == nil {
+ img, err := remote.Image(imgRef, remote.WithAuthFromKeychain(keychain), remote.WithContext(ctx),
+ remote.WithRetryBackoff(customRetryBackoff))
- img, err := remote.Image(imgRef, remote.WithAuthFromKeychain(keychain), remote.WithContext(ctx))
- return imgRef.Context().Name(), img, err
+ return imgRef.Context().Name(), img, err
+ } else {
+ img, err := remote.Image(imgRef, remote.WithAuthFromKeychain(keychain), remote.WithContext(ctx))
+
+ return imgRef.Context().Name(), img, err
+ }
}
// checkImageCompliance will perform common checks to ensure the Tekton Bundle is compliant to our spec.
diff --git a/pkg/resolution/resolver/bundle/config.go b/pkg/resolution/resolver/bundle/config.go
index da672db62e8..f37671198b5 100644
--- a/pkg/resolution/resolver/bundle/config.go
+++ b/pkg/resolution/resolver/bundle/config.go
@@ -13,6 +13,16 @@ limitations under the License.
package bundle
+import (
+ "context"
+ "fmt"
+ "strconv"
+ "time"
+
+ "github.com/google/go-containerregistry/pkg/v1/remote"
+ "github.com/tektoncd/pipeline/pkg/resolution/resolver/framework"
+)
+
const (
// ConfigServiceAccount is the configuration field name for controlling
// the Service Account name to use for bundle requests.
@@ -20,7 +30,86 @@ const (
// ConfigKind is the configuration field name for controlling
// what the layer name in the bundle image is.
ConfigKind = "default-kind"
- // DefaultTimeoutKey is the configuration field name for controlling
+ // ConfigTimeoutKey is the configuration field name for controlling
// the maximum duration of a resolution request for a file from registry.
ConfigTimeoutKey = "fetch-timeout"
+ // ConfigBackoffDuration is the configuration field name for controlling
+ // the initial duration of a backoff when a bundle resolution fails
+ ConfigBackoffDuration = "backoff-duration"
+ DefaultBackoffDuration = 2.0 * time.Second
+ // ConfigBackoffFactor is the configuration field name for controlling
+ // the factor by which successive backoffs will increase when a bundle
+ // resolution fails
+ ConfigBackoffFactor = "backoff-factor"
+ DefaultBackoffFactor = 2.0
+ // ConfigBackoffJitter is the configuration field name for controlling
+ // the randomness applied to backoff durations when a bundle resolution fails
+ ConfigBackoffJitter = "backoff-jitter"
+ DefaultBackoffJitter = 0.1
+ // ConfigBackoffSteps is the configuration field name for controlling
+ // the number of attempted backoffs to retry when a bundle resolution fails
+ ConfigBackoffSteps = "backoff-steps"
+ DefaultBackoffSteps = 2
+ // ConfigBackoffCap is the configuration field name for controlling
+ // the maximum duration to try when backing off
+ ConfigBackoffCap = "backoff-cap"
+ DefaultBackoffCap = 10 * time.Second
)
+
+// GetBundleResolverBackoff returns a remote.Backoff to
+// be passed when resolving remote images. This can be configured with the
+// backoff-duration, backoff-factor, backoff-jitter, backoff-steps, and backoff-cap
+// fields in the bundle-resolver-config ConfigMap.
+func GetBundleResolverBackoff(ctx context.Context) (remote.Backoff, error) {
+ conf := framework.GetResolverConfigFromContext(ctx)
+
+ customRetryBackoff := remote.Backoff{
+ Duration: DefaultBackoffDuration,
+ Factor: DefaultBackoffFactor,
+ Jitter: DefaultBackoffJitter,
+ Steps: DefaultBackoffSteps,
+ Cap: DefaultBackoffCap,
+ }
+ if v, ok := conf[ConfigBackoffDuration]; ok {
+ var err error
+ duration, err := time.ParseDuration(v)
+ if err != nil {
+ return customRetryBackoff, fmt.Errorf("error parsing backoff duration value %s: %w", v, err)
+ }
+ customRetryBackoff.Duration = duration
+ }
+ if v, ok := conf[ConfigBackoffFactor]; ok {
+ var err error
+ factor, err := strconv.ParseFloat(v, 64)
+ if err != nil {
+ return customRetryBackoff, fmt.Errorf("error parsing backoff factor value %s: %w", v, err)
+ }
+ customRetryBackoff.Factor = factor
+ }
+ if v, ok := conf[ConfigBackoffJitter]; ok {
+ var err error
+ jitter, err := strconv.ParseFloat(v, 64)
+ if err != nil {
+ return customRetryBackoff, fmt.Errorf("error parsing backoff jitter value %s: %w", v, err)
+ }
+ customRetryBackoff.Jitter = jitter
+ }
+ if v, ok := conf[ConfigBackoffSteps]; ok {
+ var err error
+ steps, err := strconv.Atoi(v)
+ if err != nil {
+ return customRetryBackoff, fmt.Errorf("error parsing backoff steps value %s: %w", v, err)
+ }
+ customRetryBackoff.Steps = steps
+ }
+ if v, ok := conf[ConfigBackoffCap]; ok {
+ var err error
+ cap, err := time.ParseDuration(v)
+ if err != nil {
+ return customRetryBackoff, fmt.Errorf("error parsing backoff steps value %s: %w", v, err)
+ }
+ customRetryBackoff.Cap = cap
+ }
+
+ return customRetryBackoff, nil
+}
diff --git a/pkg/resolution/resolver/bundle/resolver_test.go b/pkg/resolution/resolver/bundle/resolver_test.go
index 1984c868cb3..4c37878a4a9 100644
--- a/pkg/resolution/resolver/bundle/resolver_test.go
+++ b/pkg/resolution/resolver/bundle/resolver_test.go
@@ -22,6 +22,7 @@ import (
"fmt"
"net/http/httptest"
"net/url"
+ "strconv"
"strings"
"testing"
"time"
@@ -740,3 +741,41 @@ func TestGetResolutionTimeoutCustom(t *testing.T) {
t.Fatalf("expected timeout from config to be returned")
}
}
+
+func TestGetResolutionBackoffCustom(t *testing.T) {
+ // resolver := bundle.Resolver{}
+ // defaultTimeout := 30 * time.Minute
+ configBackoffDuration := 7.0 * time.Second
+ configBackoffFactor := 7.0
+ configBackoffJitter := 0.5
+ configBackoffSteps := 3
+ configBackoffCap := 20 * time.Second
+ config := map[string]string{
+ bundle.ConfigBackoffDuration: configBackoffDuration.String(),
+ bundle.ConfigBackoffFactor: strconv.FormatFloat(configBackoffFactor, 'f', -1, 64),
+ bundle.ConfigBackoffJitter: strconv.FormatFloat(configBackoffJitter, 'f', -1, 64),
+ bundle.ConfigBackoffSteps: strconv.Itoa(configBackoffSteps),
+ bundle.ConfigBackoffCap: configBackoffCap.String(),
+ }
+ ctx := framework.InjectResolverConfigToContext(context.Background(), config)
+ backoffConfig, err := bundle.GetBundleResolverBackoff(ctx)
+ // timeout, err := resolver.GetResolutionTimeout(ctx, defaultTimeout, map[string]string{})
+ if err != nil {
+ t.Fatalf("couldn't get backoff config: %v", err)
+ }
+ if backoffConfig.Duration != configBackoffDuration {
+ t.Fatalf("expected duration from config to be returned")
+ }
+ if backoffConfig.Factor != configBackoffFactor {
+ t.Fatalf("expected backoff from config to be returned")
+ }
+ if backoffConfig.Jitter != configBackoffJitter {
+ t.Fatalf("expected jitter from config to be returned")
+ }
+ if backoffConfig.Steps != configBackoffSteps {
+ t.Fatalf("expected steps from config to be returned")
+ }
+ if backoffConfig.Cap != configBackoffCap {
+ t.Fatalf("expected steps from config to be returned")
+ }
+}