Skip to content

Commit

Permalink
make the maximum resolution timeout configurable through defaultconfi…
Browse files Browse the repository at this point in the history
…gmap
  • Loading branch information
jkhelil committed Nov 5, 2024
1 parent 5231241 commit 565470b
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 14 deletions.
5 changes: 5 additions & 0 deletions config/config-defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 18 additions & 3 deletions pkg/apis/config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down
28 changes: 28 additions & 0 deletions pkg/apis/config/default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
Expand All @@ -65,6 +66,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
},
DefaultMaxMatrixCombinationsCount: 256,
DefaultImagePullBackOffTimeout: 0,
DefaultMaximumResolutionTimeout: 1 * time.Minute,
},
fileName: "config-defaults-with-pod-template",
},
Expand All @@ -88,6 +90,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
DefaultPodTemplate: &pod.Template{},
DefaultMaxMatrixCombinationsCount: 256,
DefaultImagePullBackOffTimeout: 0,
DefaultMaximumResolutionTimeout: 1 * time.Minute,
},
},
{
Expand All @@ -100,6 +103,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
DefaultAAPodTemplate: &pod.AffinityAssistantTemplate{},
DefaultMaxMatrixCombinationsCount: 256,
DefaultImagePullBackOffTimeout: 0,
DefaultMaximumResolutionTimeout: 1 * time.Minute,
},
},
{
Expand All @@ -115,6 +119,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
DefaultServiceAccount: "default",
DefaultManagedByLabelValue: config.DefaultManagedByLabelValue,
DefaultImagePullBackOffTimeout: 0,
DefaultMaximumResolutionTimeout: 1 * time.Minute,
},
},
{
Expand All @@ -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,
},
},
{
Expand All @@ -139,6 +145,7 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
DefaultMaxMatrixCombinationsCount: 256,
DefaultContainerResourceRequirements: map[string]corev1.ResourceRequirements{},
DefaultImagePullBackOffTimeout: 0,
DefaultMaximumResolutionTimeout: 1 * time.Minute,
},
},
{
Expand All @@ -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{
Expand Down Expand Up @@ -210,6 +218,7 @@ func TestNewDefaultsFromEmptyConfigMap(t *testing.T) {
DefaultServiceAccount: "default",
DefaultMaxMatrixCombinationsCount: 256,
DefaultImagePullBackOffTimeout: 0,
DefaultMaximumResolutionTimeout: 1 * time.Minute,
}
verifyConfigFileWithExpectedConfig(t, DefaultsConfigEmptyName, expectedConfig)
}
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion pkg/reconciler/resolutionrequest/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down
16 changes: 7 additions & 9 deletions pkg/reconciler/resolutionrequest/resolutionrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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)
}
12 changes: 11 additions & 1 deletion pkg/reconciler/resolutionrequest/resolutionrequest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{},
Expand Down Expand Up @@ -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)
Expand All @@ -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),
}
}

0 comments on commit 565470b

Please sign in to comment.