Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make resolvers' maximum resolution timeout configurable #8366

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
12 changes: 11 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,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 {
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),
}
}
5 changes: 3 additions & 2 deletions pkg/resolution/resolver/bundle/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Loading