make the maximum resolution timeout configurable through defaultconfigmap

Author: jkhelil

config/config-defaults.yaml

@@ -92,6 +92,11 @@ data:
     # possible values could be 1m, 5m, 10s, 1h, etc
     # default-imagepullbackoff-timeout: "5m"
 
+    # default-maximum-resolution-timeout specifies the default duration used by the
+    # resolution controller before timing out when exceeded.
+    # Possible values include "1m", "5m", "10s", "1h", etc.
+    # Example: default-maximum-resolution-timeout: "1m"
+
     # default-container-resource-requirements allow users to update default resource requirements
     # to a init-containers and containers of a pods create by the controller
     # Onet: All the resource requirements are applied to init-containers and containers

pkg/apis/config/default.go

@@ -51,6 +51,9 @@ const (
 
 	DefaultImagePullBackOffTimeout = 0 * time.Minute
 
+	// Default maximum resolution timeout used by the resolution controller before timing out when exceeded
+	DefaultMaximumResolutionTimeout = 1 * time.Minute
+
 	defaultTimeoutMinutesKey                = "default-timeout-minutes"
 	defaultServiceAccountKey                = "default-service-account"
 	defaultManagedByLabelValueKey           = "default-managed-by-label-value"
@@ -63,6 +66,7 @@ const (
 	defaultResolverTypeKey                  = "default-resolver-type"
 	defaultContainerResourceRequirementsKey = "default-container-resource-requirements"
 	defaultImagePullBackOffTimeout          = "default-imagepullbackoff-timeout"
+	defaultMaximumResolutionTimeout         = "default-maximum-resolution-timeout"
 )
 
 // DefaultConfig holds all the default configurations for the config.
@@ -83,6 +87,7 @@ type Defaults struct {
 	DefaultResolverType                  string
 	DefaultContainerResourceRequirements map[string]corev1.ResourceRequirements
 	DefaultImagePullBackOffTimeout       time.Duration
+	DefaultMaximumResolutionTimeout      time.Duration
 }
 
 // GetDefaultsConfigName returns the name of the configmap containing all
@@ -114,6 +119,7 @@ func (cfg *Defaults) Equals(other *Defaults) bool {
 		other.DefaultMaxMatrixCombinationsCount == cfg.DefaultMaxMatrixCombinationsCount &&
 		other.DefaultResolverType == cfg.DefaultResolverType &&
 		other.DefaultImagePullBackOffTimeout == cfg.DefaultImagePullBackOffTimeout &&
+		other.DefaultMaximumResolutionTimeout == cfg.DefaultMaximumResolutionTimeout &&
 		reflect.DeepEqual(other.DefaultForbiddenEnv, cfg.DefaultForbiddenEnv)
 }
 
@@ -127,12 +133,13 @@ func NewDefaultsFromMap(cfgMap map[string]string) (*Defaults, error) {
 		DefaultMaxMatrixCombinationsCount: DefaultMaxMatrixCombinationsCount,
 		DefaultResolverType:               DefaultResolverTypeValue,
 		DefaultImagePullBackOffTimeout:    DefaultImagePullBackOffTimeout,
+		DefaultMaximumResolutionTimeout:   DefaultMaximumResolutionTimeout,
 	}
 
 	if defaultTimeoutMin, ok := cfgMap[defaultTimeoutMinutesKey]; ok {
 		timeout, err := strconv.ParseInt(defaultTimeoutMin, 10, 0)
 		if err != nil {
-			return nil, fmt.Errorf("failed parsing tracing config %q", defaultTimeoutMinutesKey)
+			return nil, fmt.Errorf("failed parsing default config %q", defaultTimeoutMinutesKey)
 		}
 		tc.DefaultTimeoutMinutes = int(timeout)
 	}
@@ -172,7 +179,7 @@ func NewDefaultsFromMap(cfgMap map[string]string) (*Defaults, error) {
 	if defaultMaxMatrixCombinationsCount, ok := cfgMap[defaultMaxMatrixCombinationsCountKey]; ok {
 		matrixCombinationsCount, err := strconv.ParseInt(defaultMaxMatrixCombinationsCount, 10, 0)
 		if err != nil {
-			return nil, fmt.Errorf("failed parsing tracing config %q", defaultMaxMatrixCombinationsCountKey)
+			return nil, fmt.Errorf("failed parsing default config %q", defaultMaxMatrixCombinationsCountKey)
 		}
 		tc.DefaultMaxMatrixCombinationsCount = int(matrixCombinationsCount)
 	}
@@ -200,11 +207,19 @@ func NewDefaultsFromMap(cfgMap map[string]string) (*Defaults, error) {
 	if defaultImagePullBackOff, ok := cfgMap[defaultImagePullBackOffTimeout]; ok {
 		timeout, err := time.ParseDuration(defaultImagePullBackOff)
 		if err != nil {
-			return nil, fmt.Errorf("failed parsing tracing config %q", defaultImagePullBackOffTimeout)
+			return nil, fmt.Errorf("failed parsing default config %q", defaultImagePullBackOffTimeout)
 		}
 		tc.DefaultImagePullBackOffTimeout = timeout
 	}
 
+	if defaultMaximumResolutionTimeout, ok := cfgMap[defaultMaximumResolutionTimeout]; ok {
+		timeout, err := time.ParseDuration(defaultMaximumResolutionTimeout)
+		if err != nil {
+			return nil, fmt.Errorf("failed parsing default config %q", defaultMaximumResolutionTimeout)
+		}
+		tc.DefaultMaximumResolutionTimeout = timeout
+	}
+
 	return &tc, nil
 }
 

pkg/apis/config/default_test.go

@@ -45,6 +45,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 				DefaultMaxMatrixCombinationsCount: 256,
 				DefaultResolverType:               "git",
 				DefaultImagePullBackOffTimeout:    time.Duration(5) * time.Second,
+				DefaultMaximumResolutionTimeout:   1 * time.Minute,
 			},
 			fileName: config.GetDefaultsConfigName(),
 		},
@@ -65,6 +66,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 				},
 				DefaultMaxMatrixCombinationsCount: 256,
 				DefaultImagePullBackOffTimeout:    0,
+				DefaultMaximumResolutionTimeout:   1 * time.Minute,
 			},
 			fileName: "config-defaults-with-pod-template",
 		},
@@ -88,6 +90,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 				DefaultPodTemplate:                &pod.Template{},
 				DefaultMaxMatrixCombinationsCount: 256,
 				DefaultImagePullBackOffTimeout:    0,
+				DefaultMaximumResolutionTimeout:   1 * time.Minute,
 			},
 		},
 		{
@@ -100,6 +103,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 				DefaultAAPodTemplate:              &pod.AffinityAssistantTemplate{},
 				DefaultMaxMatrixCombinationsCount: 256,
 				DefaultImagePullBackOffTimeout:    0,
+				DefaultMaximumResolutionTimeout:   1 * time.Minute,
 			},
 		},
 		{
@@ -115,6 +119,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 				DefaultServiceAccount:             "default",
 				DefaultManagedByLabelValue:        config.DefaultManagedByLabelValue,
 				DefaultImagePullBackOffTimeout:    0,
+				DefaultMaximumResolutionTimeout:   1 * time.Minute,
 			},
 		},
 		{
@@ -127,6 +132,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 				DefaultManagedByLabelValue:        "tekton-pipelines",
 				DefaultForbiddenEnv:               []string{"TEKTON_POWER_MODE", "TEST_ENV", "TEST_TEKTON"},
 				DefaultImagePullBackOffTimeout:    time.Duration(15) * time.Second,
+				DefaultMaximumResolutionTimeout:   1 * time.Minute,
 			},
 		},
 		{
@@ -139,6 +145,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 				DefaultMaxMatrixCombinationsCount:    256,
 				DefaultContainerResourceRequirements: map[string]corev1.ResourceRequirements{},
 				DefaultImagePullBackOffTimeout:       0,
+				DefaultMaximumResolutionTimeout:      1 * time.Minute,
 			},
 		},
 		{
@@ -154,6 +161,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
 				DefaultManagedByLabelValue:        "tekton-pipelines",
 				DefaultMaxMatrixCombinationsCount: 256,
 				DefaultImagePullBackOffTimeout:    0,
+				DefaultMaximumResolutionTimeout:   1 * time.Minute,
 				DefaultContainerResourceRequirements: map[string]corev1.ResourceRequirements{
 					config.ResourceRequirementDefaultContainerKey: {
 						Requests: corev1.ResourceList{
@@ -210,6 +218,7 @@ func TestNewDefaultsFromEmptyConfigMap(t *testing.T) {
 		DefaultServiceAccount:             "default",
 		DefaultMaxMatrixCombinationsCount: 256,
 		DefaultImagePullBackOffTimeout:    0,
+		DefaultMaximumResolutionTimeout:   1 * time.Minute,
 	}
 	verifyConfigFileWithExpectedConfig(t, DefaultsConfigEmptyName, expectedConfig)
 }
@@ -377,6 +386,25 @@ func TestEquals(t *testing.T) {
 			},
 			expected: true,
 		},
+		{
+			name: "different default maximum resolution timeout",
+			left: &config.Defaults{
+				DefaultMaximumResolutionTimeout: 10 * time.Minute,
+			},
+			right: &config.Defaults{
+				DefaultMaximumResolutionTimeout: 20 * time.Minute,
+			},
+			expected: false,
+		}, {
+			name: "same default maximum resolution timeout",
+			left: &config.Defaults{
+				DefaultMaximumResolutionTimeout: 10 * time.Minute,
+			},
+			right: &config.Defaults{
+				DefaultMaximumResolutionTimeout: 10 * time.Minute,
+			},
+			expected: true,
+		},
 	}
 
 	for _, tc := range testCases {

pkg/reconciler/resolutionrequest/controller.go

@@ -19,6 +19,7 @@ package resolutionrequest
 import (
 	"context"
 
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	resolutionrequestinformer "github.com/tektoncd/pipeline/pkg/client/resolution/injection/informers/resolution/v1beta1/resolutionrequest"
 	resolutionrequestreconciler "github.com/tektoncd/pipeline/pkg/client/resolution/injection/reconciler/resolution/v1beta1/resolutionrequest"
 	"k8s.io/utils/clock"
@@ -31,10 +32,19 @@ import (
 // ResolutionRequest objects.
 func NewController(clock clock.PassiveClock) func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
 	return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
+		logger := logging.FromContext(ctx)
+
+		configStore := config.NewStore(logger.Named("config-store"))
+		configStore.WatchConfigs(cmw)
+
 		r := &Reconciler{
 			clock: clock,
 		}
-		impl := resolutionrequestreconciler.NewImpl(ctx, r)
+		impl := resolutionrequestreconciler.NewImpl(ctx, r, func(impl *controller.Impl) controller.Options {
+			return controller.Options{
+				ConfigStore: configStore,
+			}
+		})
 
 		reqinformer := resolutionrequestinformer.Get(ctx)
 		if _, err := reqinformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)); err != nil {

pkg/reconciler/resolutionrequest/resolutionrequest.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
 	rrreconciler "github.com/tektoncd/pipeline/pkg/client/resolution/injection/reconciler/resolution/v1beta1/resolutionrequest"
 	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
@@ -38,10 +39,6 @@ type Reconciler struct {
 
 var _ rrreconciler.Interface = (*Reconciler)(nil)
 
-// TODO(sbwsg): This should be exposed via ConfigMap using a config
-// store similarly to Tekton Pipelines'.
-const defaultMaximumResolutionDuration = 1 * time.Minute
-
 // ReconcileKind processes updates to ResolutionRequests, sets status
 // fields on it, and returns any errors experienced along the way.
 func (r *Reconciler) ReconcileKind(ctx context.Context, rr *v1beta1.ResolutionRequest) reconciler.Event {
@@ -57,14 +54,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, rr *v1beta1.ResolutionRe
 		rr.Status.InitializeConditions()
 	}
 
+	maximumResolutionDuration := config.FromContextOrDefaults(ctx).Defaults.DefaultMaximumResolutionTimeout
 	switch {
 	case rr.Status.Data != "":
 		rr.Status.MarkSucceeded()
-	case requestDuration(rr) > defaultMaximumResolutionDuration:
-		rr.Status.MarkFailed(resolutioncommon.ReasonResolutionTimedOut, timeoutMessage())
+	case requestDuration(rr) > maximumResolutionDuration:
+		rr.Status.MarkFailed(resolutioncommon.ReasonResolutionTimedOut, timeoutMessage(maximumResolutionDuration))
 	default:
 		rr.Status.MarkInProgress(resolutioncommon.MessageWaitingForResolver)
-		return controller.NewRequeueAfter(defaultMaximumResolutionDuration - requestDuration(rr))
+		return controller.NewRequeueAfter(maximumResolutionDuration - requestDuration(rr))
 	}
 
 	return nil
@@ -77,6 +75,6 @@ func requestDuration(rr *v1beta1.ResolutionRequest) time.Duration {
 	return time.Now().UTC().Sub(creationTime)
 }
 
-func timeoutMessage() string {
-	return fmt.Sprintf("resolution took longer than global timeout of %s", defaultMaximumResolutionDuration)
+func timeoutMessage(timeout time.Duration) string {
+	return fmt.Sprintf("resolution took longer than global timeout of %s", timeout)
 }

pkg/reconciler/resolutionrequest/resolutionrequest_test.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/resolution/v1beta1"
 	ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
 	resolutioncommon "github.com/tektoncd/pipeline/pkg/resolution/common"
@@ -63,6 +64,7 @@ func initializeResolutionRequestControllerAssets(t *testing.T, d test.Data) (tes
 	t.Helper()
 	ctx, _ := ttesting.SetupFakeContext(t)
 	ctx, cancel := context.WithCancel(ctx)
+	test.EnsureConfigurationConfigMapsExist(&d)
 	c, informers := test.SeedTestData(t, ctx, d)
 	configMapWatcher := cminformer.NewInformedWatcher(c.Kube, system.Namespace())
 	ctl := NewController(testClock)(ctx, configMapWatcher)
@@ -129,7 +131,7 @@ func TestReconcile(t *testing.T) {
 						Type:    apis.ConditionSucceeded,
 						Status:  corev1.ConditionFalse,
 						Reason:  resolutioncommon.ReasonResolutionTimedOut,
-						Message: timeoutMessage(),
+						Message: timeoutMessage(config.FromContextOrDefaults(context.TODO()).Defaults.DefaultMaximumResolutionTimeout),
 					}},
 				},
 				ResolutionRequestStatusFields: v1beta1.ResolutionRequestStatusFields{},
@@ -167,6 +169,7 @@ func TestReconcile(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			d := test.Data{
 				ResolutionRequests: []*v1beta1.ResolutionRequest{tc.input},
+				ConfigMaps:         []*corev1.ConfigMap{newDefaultsConfigMap()},
 			}
 
 			testAssets, cancel := getResolutionRequestController(t, d)
@@ -192,3 +195,10 @@ func TestReconcile(t *testing.T) {
 func getRequestName(rr *v1beta1.ResolutionRequest) string {
 	return strings.Join([]string{rr.Namespace, rr.Name}, "/")
 }
+
+func newDefaultsConfigMap() *corev1.ConfigMap {
+	return &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{Name: config.GetDefaultsConfigName(), Namespace: system.Namespace()},
+		Data:       make(map[string]string),
+	}
+}

pkg/resolution/resolver/bundle/config.go

@@ -14,12 +14,13 @@ limitations under the License.
 package bundle
 
 const (
-	// ConfigMapName is the bundle resolver's config map
-	ConfigMapName = "bundleresolver-config"
 	// ConfigServiceAccount is the configuration field name for controlling
 	// the Service Account name to use for bundle requests.
 	ConfigServiceAccount = "default-service-account"
 	// 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
+	// the maximum duration of a resolution request for a file from registry.
+	ConfigTimeoutKey = "fetch-timeout"
 )