diff --git a/config/config-defaults.yaml b/config/config-defaults.yaml index fa34c191729..275ab5760fb 100644 --- a/config/config-defaults.yaml +++ b/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 diff --git a/pkg/apis/config/default.go b/pkg/apis/config/default.go index c52da0ba27f..3bb5e02ab55 100644 --- a/pkg/apis/config/default.go +++ b/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 } diff --git a/pkg/apis/config/default_test.go b/pkg/apis/config/default_test.go index de873f2f103..fdc8ba92378 100644 --- a/pkg/apis/config/default_test.go +++ b/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 { diff --git a/pkg/reconciler/resolutionrequest/controller.go b/pkg/reconciler/resolutionrequest/controller.go index 44cd6e828d7..563c0ea2365 100644 --- a/pkg/reconciler/resolutionrequest/controller.go +++ b/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,11 +32,16 @@ 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) reqinformer := resolutionrequestinformer.Get(ctx) if _, err := reqinformer.Informer().AddEventHandler(controller.HandleAll(impl.Enqueue)); err != nil { logging.FromContext(ctx).Panicf("Couldn't register ResolutionRequest informer event handler: %w", err) diff --git a/pkg/reconciler/resolutionrequest/resolutionrequest.go b/pkg/reconciler/resolutionrequest/resolutionrequest.go index 38da3fea352..b5ea6331e0c 100644 --- a/pkg/reconciler/resolutionrequest/resolutionrequest.go +++ b/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) } diff --git a/pkg/reconciler/resolutionrequest/resolutionrequest_test.go b/pkg/reconciler/resolutionrequest/resolutionrequest_test.go index 4b0c2791228..6a4581c4132 100644 --- a/pkg/reconciler/resolutionrequest/resolutionrequest_test.go +++ b/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), + } +}