From f3b795e00d9761d13f05d0f28f8b4448bac47473 Mon Sep 17 00:00:00 2001 From: Vincent Hou Date: Tue, 18 Jun 2019 11:07:07 -0400 Subject: [PATCH] Add the support of configuring the default timeout of TaskRun * Changes the default timeout from 10 mins to 60 mins * Adds a config_defaults configMap for configuring pipeline default values per tektoncd/pipeline instance --- cmd/controller/main.go | 8 +- config/config-defaults.yaml | 39 ++++++++ docs/taskruns.md | 2 +- hack/update-codegen.sh | 6 ++ pkg/apis/config/default.go | 63 +++++++++++++ pkg/apis/config/default_test.go | 49 ++++++++++ pkg/apis/config/doc.go | 17 ++++ pkg/apis/config/store.go | 91 +++++++++++++++++++ pkg/apis/config/store_test.go | 38 ++++++++ .../testdata/config-defaults-empty.yaml | 39 ++++++++ pkg/apis/config/testdata/config-defaults.yaml | 21 +++++ pkg/apis/config/zz_generated.deepcopy.go | 37 ++++++++ pkg/reconciler/timeout_handler.go | 18 ++-- pkg/reconciler/timeout_handler_test.go | 30 ++++-- .../v1alpha1/pipelinerun/pipelinerun_test.go | 14 ++- pkg/reconciler/v1alpha1/taskrun/taskrun.go | 11 ++- .../v1alpha1/taskrun/taskrun_test.go | 22 ++++- 17 files changed, 485 insertions(+), 20 deletions(-) create mode 100644 config/config-defaults.yaml create mode 100644 pkg/apis/config/default.go create mode 100644 pkg/apis/config/default_test.go create mode 100644 pkg/apis/config/doc.go create mode 100644 pkg/apis/config/store.go create mode 100644 pkg/apis/config/store_test.go create mode 100644 pkg/apis/config/testdata/config-defaults-empty.yaml create mode 100644 pkg/apis/config/testdata/config-defaults.yaml create mode 100644 pkg/apis/config/zz_generated.deepcopy.go diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 749b72a0650..eb5b60f4c75 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -23,6 +23,7 @@ import ( "github.com/knative/pkg/logging" + apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" tklogging "github.com/tektoncd/pipeline/pkg/logging" corev1 "k8s.io/api/core/v1" kubeinformers "k8s.io/client-go/informers" @@ -116,7 +117,11 @@ func main() { pipelineInformer := pipelineInformerFactory.Tekton().V1alpha1().Pipelines() pipelineRunInformer := pipelineInformerFactory.Tekton().V1alpha1().PipelineRuns() - timeoutHandler := reconciler.NewTimeoutHandler(stopCh, logger) + + store := apisconfig.NewStore(logger.Named("config-store")) + store.WatchConfigs(configMapWatcher) + + timeoutHandler := reconciler.NewTimeoutHandler(stopCh, logger, store) trc := taskrun.NewController(opt, taskRunInformer, @@ -126,6 +131,7 @@ func main() { podInformer, nil, //entrypoint cache will be initialized by controller if not provided timeoutHandler, + store, ) prc := pipelinerun.NewController(opt, pipelineRunInformer, diff --git a/config/config-defaults.yaml b/config/config-defaults.yaml new file mode 100644 index 00000000000..661246818e6 --- /dev/null +++ b/config/config-defaults.yaml @@ -0,0 +1,39 @@ +# Copyright 2019 The Tekton Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-defaults + namespace: tekton-pipelines +data: + _example: | + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ + + # This block is not actually functional configuration, + # but serves to illustrate the available configuration + # options and document them in a way that is accessible + # to users that `kubectl edit` this config map. + # + # These sample configuration options may be copied out of + # this example block and unindented to be in the data block + # to actually change the configuration. + + # default-timeout-minutes contains the default number of + # minutes to use for TaskRun, if none is specified. + default-timeout-minutes: "60" # 60 minutes diff --git a/docs/taskruns.md b/docs/taskruns.md index 8d80a0a5f3f..580a7c0bcaa 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -49,7 +49,7 @@ following fields: [input resources](#providing-resources) - [`outputs`] - Specifies [output resources](#providing-resources) - `timeout` - Specifies timeout after which the `TaskRun` will fail. Defaults - to ten minutes. + to 60 minutes. - [`nodeSelector`] - a selector which must be true for the pod to fit on a node. The selector which must match a node's labels for the pod to be scheduled on that node. More info: diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 55b35c2bd37..004a6d46879 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -31,5 +31,11 @@ ${CODEGEN_PKG}/generate-groups.sh "deepcopy,client,informer,lister" \ pipeline:v1alpha1 \ --go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt +# Depends on generate-groups.sh to install bin/deepcopy-gen +${GOPATH}/bin/deepcopy-gen \ + -O zz_generated.deepcopy \ + --go-header-file ${REPO_ROOT_DIR}/hack/boilerplate/boilerplate.go.txt \ + -i github.com/tektoncd/pipeline/pkg/apis/config + # Make sure our dependencies are up-to-date ${REPO_ROOT_DIR}/hack/update-deps.sh diff --git a/pkg/apis/config/default.go b/pkg/apis/config/default.go new file mode 100644 index 00000000000..f31ebff3a3e --- /dev/null +++ b/pkg/apis/config/default.go @@ -0,0 +1,63 @@ +/* +Copyright 2019 The Tekton Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "fmt" + "strconv" + + corev1 "k8s.io/api/core/v1" +) + +const ( + // ConfigName is the name of the configmap + DefaultsConfigName = "config-defaults" + DefaultTimeoutMinutes = 60 + defaultTimeoutMinutesKey = "default-timeout-minutes" +) + +// ConfigDefault holds the default configurations +// +k8s:deepcopy-gen=true +type ConfigDefault struct { + DefaultTimeoutMinutes int +} + +// Equals returns true if two Configs are identical +func (cfg *ConfigDefault) Equals(other *ConfigDefault) bool { + return other.DefaultTimeoutMinutes == cfg.DefaultTimeoutMinutes +} + +// NewConfigDefaultFromMap returns a Config given a map corresponding to a ConfigMap +func NewConfigDefaultFromMap(cfgMap map[string]string) (*ConfigDefault, error) { + tc := ConfigDefault{ + DefaultTimeoutMinutes: DefaultTimeoutMinutes, + } + 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) + } + tc.DefaultTimeoutMinutes = int(timeout) + } + + return &tc, nil +} + +// NewConfigDefaultFromConfigMap returns a Config for the given configmap +func NewConfigDefaultFromConfigMap(config *corev1.ConfigMap) (*ConfigDefault, error) { + return NewConfigDefaultFromMap(config.Data) +} diff --git a/pkg/apis/config/default_test.go b/pkg/apis/config/default_test.go new file mode 100644 index 00000000000..a66e4c1b8d2 --- /dev/null +++ b/pkg/apis/config/default_test.go @@ -0,0 +1,49 @@ +/* +Copyright 2019 The Tekton Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "github.com/google/go-cmp/cmp" + test "github.com/tektoncd/pipeline/pkg/reconciler/testing" + "testing" +) + +func TestNewConfigDefaultFromConfigMap(t *testing.T) { + expectedConfig := &ConfigDefault{ + DefaultTimeoutMinutes: 50, + } + verifyConfigFileWithExpectedConfig(t, DefaultsConfigName, expectedConfig) +} + +func TestNewConfigDefaultFromEmptyConfigMap(t *testing.T) { + DefaultsConfigEmptyName := "config-defaults-empty" + expectedConfig := &ConfigDefault{ + DefaultTimeoutMinutes: 60, + } + verifyConfigFileWithExpectedConfig(t, DefaultsConfigEmptyName, expectedConfig) +} + +func verifyConfigFileWithExpectedConfig(t *testing.T, fileName string, expectedConfig *ConfigDefault) { + cm := test.ConfigMapFromTestFile(t, fileName) + if configDefault, err := NewConfigDefaultFromConfigMap(cm); err == nil { + if d := cmp.Diff(configDefault, expectedConfig); d != "" { + t.Errorf("Diff:\n%s", d) + } + } else { + t.Errorf("NewConfigDefaultFromConfigMap(actual) = %v", err) + } +} diff --git a/pkg/apis/config/doc.go b/pkg/apis/config/doc.go new file mode 100644 index 00000000000..98c3e77f149 --- /dev/null +++ b/pkg/apis/config/doc.go @@ -0,0 +1,17 @@ +/* +Copyright 2019 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config diff --git a/pkg/apis/config/store.go b/pkg/apis/config/store.go new file mode 100644 index 00000000000..0144c5c8285 --- /dev/null +++ b/pkg/apis/config/store.go @@ -0,0 +1,91 @@ +/* +Copyright 2019 The Tekton Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "context" + "github.com/knative/pkg/configmap" +) + +type cfgKey struct{} + +// Config holds the collection of configurations that we attach to contexts. +// +k8s:deepcopy-gen=false +type Config struct { + ConfigDefault *ConfigDefault +} + +// FromContext extracts a Config from the provided context. +func FromContext(ctx context.Context) *Config { + x, ok := ctx.Value(cfgKey{}).(*Config) + if ok { + return x + } + return nil +} + +// FromContextOrDefaults is like FromContext, but when no Config is attached it +// returns a Config populated with the defaults for each of the Config fields. +func FromContextOrDefaults(ctx context.Context) *Config { + if cfg := FromContext(ctx); cfg != nil { + return cfg + } + defaults, _ := NewConfigDefaultFromMap(map[string]string{}) + return &Config{ + ConfigDefault: defaults, + } +} + +// ToContext attaches the provided Config to the provided context, returning the +// new context with the Config attached. +func ToContext(ctx context.Context, c *Config) context.Context { + return context.WithValue(ctx, cfgKey{}, c) +} + +// Store is a typed wrapper around configmap.Untyped store to handle our configmaps. +// +k8s:deepcopy-gen=false +type Store struct { + *configmap.UntypedStore +} + +// NewStore creates a new store of Configs and optionally calls functions when ConfigMaps are updated. +func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value interface{})) *Store { + store := &Store{ + UntypedStore: configmap.NewUntypedStore( + "defaults", + logger, + configmap.Constructors{ + DefaultsConfigName: NewConfigDefaultFromConfigMap, + }, + onAfterStore..., + ), + } + + return store +} + +// ToContext attaches the current Config state to the provided context. +func (s *Store) ToContext(ctx context.Context) context.Context { + return ToContext(ctx, s.Load()) +} + +// Load creates a Config from the current config state of the Store. +func (s *Store) Load() *Config { + return &Config{ + ConfigDefault: s.UntypedLoad(DefaultsConfigName).(*ConfigDefault).DeepCopy(), + } +} diff --git a/pkg/apis/config/store_test.go b/pkg/apis/config/store_test.go new file mode 100644 index 00000000000..6eb23809ed0 --- /dev/null +++ b/pkg/apis/config/store_test.go @@ -0,0 +1,38 @@ +/* +Copyright 2019 The Tekton Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package config + +import ( + "context" + "github.com/google/go-cmp/cmp" + logtesting "github.com/knative/pkg/logging/testing" + test "github.com/tektoncd/pipeline/pkg/reconciler/testing" + "testing" +) + +func TestStoreLoadWithContext(t *testing.T) { + store := NewStore(logtesting.TestLogger(t)) + defaultConfig := test.ConfigMapFromTestFile(t, "config-defaults") + store.OnConfigChanged(defaultConfig) + + config := FromContext(store.ToContext(context.Background())) + + expected, _ := NewConfigDefaultFromConfigMap(defaultConfig) + if diff := cmp.Diff(config.ConfigDefault, expected); diff != "" { + t.Errorf("Unexpected default config (-want, +got): %v", diff) + } +} diff --git a/pkg/apis/config/testdata/config-defaults-empty.yaml b/pkg/apis/config/testdata/config-defaults-empty.yaml new file mode 100644 index 00000000000..661246818e6 --- /dev/null +++ b/pkg/apis/config/testdata/config-defaults-empty.yaml @@ -0,0 +1,39 @@ +# Copyright 2019 The Tekton Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-defaults + namespace: tekton-pipelines +data: + _example: | + ################################ + # # + # EXAMPLE CONFIGURATION # + # # + ################################ + + # This block is not actually functional configuration, + # but serves to illustrate the available configuration + # options and document them in a way that is accessible + # to users that `kubectl edit` this config map. + # + # These sample configuration options may be copied out of + # this example block and unindented to be in the data block + # to actually change the configuration. + + # default-timeout-minutes contains the default number of + # minutes to use for TaskRun, if none is specified. + default-timeout-minutes: "60" # 60 minutes diff --git a/pkg/apis/config/testdata/config-defaults.yaml b/pkg/apis/config/testdata/config-defaults.yaml new file mode 100644 index 00000000000..005853ca523 --- /dev/null +++ b/pkg/apis/config/testdata/config-defaults.yaml @@ -0,0 +1,21 @@ +# Copyright 2019 The Tekton Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-defaults + namespace: tekton-pipelines +data: + default-timeout-minutes: "50" diff --git a/pkg/apis/config/zz_generated.deepcopy.go b/pkg/apis/config/zz_generated.deepcopy.go new file mode 100644 index 00000000000..ac915fdf832 --- /dev/null +++ b/pkg/apis/config/zz_generated.deepcopy.go @@ -0,0 +1,37 @@ +// +build !ignore_autogenerated + +/* +Copyright 2019 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// This file was autogenerated by deepcopy-gen. Do not edit it manually! + +package config + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConfigDefault) DeepCopyInto(out *ConfigDefault) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConfigDefault. +func (in *ConfigDefault) DeepCopy() *ConfigDefault { + if in == nil { + return nil + } + out := new(ConfigDefault) + in.DeepCopyInto(out) + return out +} diff --git a/pkg/reconciler/timeout_handler.go b/pkg/reconciler/timeout_handler.go index e995c69a15d..6b9891281e6 100644 --- a/pkg/reconciler/timeout_handler.go +++ b/pkg/reconciler/timeout_handler.go @@ -7,6 +7,7 @@ import ( "time" + apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" clientset "github.com/tektoncd/pipeline/pkg/client/clientset/versioned" "go.uber.org/zap" @@ -15,7 +16,6 @@ import ( ) const ( - defaultTimeout = 10 * time.Minute maxBackoffSeconds = 120 ) @@ -52,18 +52,21 @@ type TimeoutSet struct { doneMut sync.Mutex backoffs map[string]backoff backoffsMut sync.Mutex + store *apisconfig.Store } // NewTimeoutHandler returns TimeoutSet filled structure func NewTimeoutHandler( stopCh <-chan struct{}, logger *zap.SugaredLogger, + store *apisconfig.Store, ) *TimeoutSet { return &TimeoutSet{ stopCh: stopCh, done: make(map[string]chan bool), backoffs: make(map[string]backoff), logger: logger, + store: store, } } @@ -110,8 +113,8 @@ func (t *TimeoutSet) getOrCreateFinishedChan(runObj StatusKey) chan bool { // GetTimeout takes a kubernetes Duration representing the timeout period for a // resource and returns it as a time.Duration. If the provided duration is nil // then fallback behaviour is to return a default timeout period. -func GetTimeout(d *metav1.Duration) time.Duration { - timeout := defaultTimeout +func GetTimeout(d *metav1.Duration, defaultTimeout int) time.Duration { + timeout := time.Duration(defaultTimeout) * time.Minute if d != nil { timeout = d.Duration } @@ -139,7 +142,8 @@ func (t *TimeoutSet) GetBackoff(tr *v1alpha1.TaskRun) (backoff, bool) { } b.NumAttempts += 1 b.NextAttempt = time.Now().Add(backoffDuration(b.NumAttempts, rand.Intn)) - timeoutDeadline := tr.Status.StartTime.Time.Add(GetTimeout(tr.Spec.Timeout)) + cfg := t.store.Load() + timeoutDeadline := tr.Status.StartTime.Time.Add(GetTimeout(tr.Spec.Timeout, cfg.ConfigDefault.DefaultTimeoutMinutes)) if timeoutDeadline.Before(b.NextAttempt) { b.NextAttempt = timeoutDeadline } @@ -216,14 +220,16 @@ func (t *TimeoutSet) checkTaskRunTimeouts(namespace string, pipelineclientset cl // 1. Stop signal, 2. TaskRun to complete or 3. Taskrun to time out, which is // determined by checking if the tr's timeout has occurred since the startTime func (t *TimeoutSet) WaitTaskRun(tr *v1alpha1.TaskRun, startTime *metav1.Time) { - t.waitRun(tr, GetTimeout(tr.Spec.Timeout), startTime, t.taskRunCallbackFunc) + cfg := t.store.Load() + t.waitRun(tr, GetTimeout(tr.Spec.Timeout, cfg.ConfigDefault.DefaultTimeoutMinutes), startTime, t.taskRunCallbackFunc) } // WaitPipelineRun function creates a blocking function for pipelinerun to wait for // 1. Stop signal, 2. pipelinerun to complete or 3. pipelinerun to time out which is // determined by checking if the tr's timeout has occurred since the startTime func (t *TimeoutSet) WaitPipelineRun(pr *v1alpha1.PipelineRun, startTime *metav1.Time) { - t.waitRun(pr, GetTimeout(pr.Spec.Timeout), startTime, t.pipelineRunCallbackFunc) + cfg := t.store.Load() + t.waitRun(pr, GetTimeout(pr.Spec.Timeout, cfg.ConfigDefault.DefaultTimeoutMinutes), startTime, t.pipelineRunCallbackFunc) } func (t *TimeoutSet) waitRun(runObj StatusKey, timeout time.Duration, startTime *metav1.Time, callback func(interface{})) { diff --git a/pkg/reconciler/timeout_handler_test.go b/pkg/reconciler/timeout_handler_test.go index c76bc5dcbc1..b8f1af0fa6a 100644 --- a/pkg/reconciler/timeout_handler_test.go +++ b/pkg/reconciler/timeout_handler_test.go @@ -6,6 +6,8 @@ import ( "time" "github.com/knative/pkg/apis" + logtesting "github.com/knative/pkg/logging/testing" + apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/test" tb "github.com/tektoncd/pipeline/test/builder" @@ -23,6 +25,21 @@ var ( simpleTask = tb.Task("test-task", testNs, tb.TaskSpec(simpleStep)) ) +func getStore(t *testing.T) *apisconfig.Store { + store := apisconfig.NewStore(logtesting.TestLogger(t)) + defaultConfig := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tekton-pipelines", + Name: "config-defaults", + }, + Data: map[string]string{ + "default-timeout-minutes": "60", + }, + } + store.OnConfigChanged(defaultConfig) + return store +} + func TestTaskRunCheckTimeouts(t *testing.T) { taskRunTimedout := tb.TaskRun("test-taskrun-run-timedout", testNs, tb.TaskRunSpec( tb.TaskRunTaskRef(simpleTask.Name, tb.TaskRefAPIVersion("a1")), @@ -69,7 +86,8 @@ func TestTaskRunCheckTimeouts(t *testing.T) { defer close(stopCh) c, _ := test.SeedTestData(t, d) observer, _ := observer.New(zap.InfoLevel) - th := NewTimeoutHandler(stopCh, zap.New(observer).Sugar()) + + th := NewTimeoutHandler(stopCh, zap.New(observer).Sugar(), getStore(t)) gotCallback := sync.Map{} f := func(tr interface{}) { trNew := tr.(*v1alpha1.TaskRun) @@ -172,7 +190,7 @@ func TestPipelinRunCheckTimeouts(t *testing.T) { c, _ := test.SeedTestData(t, d) stopCh := make(chan struct{}) observer, _ := observer.New(zap.InfoLevel) - th := NewTimeoutHandler(stopCh, zap.New(observer).Sugar()) + th := NewTimeoutHandler(stopCh, zap.New(observer).Sugar(), getStore(t)) defer close(stopCh) gotCallback := sync.Map{} @@ -247,7 +265,7 @@ func TestWithNoFunc(t *testing.T) { stopCh := make(chan struct{}) c, _ := test.SeedTestData(t, d) observer, _ := observer.New(zap.InfoLevel) - testHandler := NewTimeoutHandler(stopCh, zap.New(observer).Sugar()) + testHandler := NewTimeoutHandler(stopCh, zap.New(observer).Sugar(), getStore(t)) defer func() { // this delay will ensure there is no race condition between stopCh/ timeout channel getting triggered time.Sleep(10 * time.Millisecond) @@ -276,12 +294,12 @@ func TestGetTimeout(t *testing.T) { { description: "returns an end time using the default timeout if a TaskRun's timeout is nil", inputDuration: nil, - expectedDuration: defaultTimeout, + expectedDuration: 60 * time.Minute, }, } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - receivedDuration := GetTimeout(tc.inputDuration) + receivedDuration := GetTimeout(tc.inputDuration, 60) if receivedDuration != tc.expectedDuration { t.Errorf("expected %q received %q", tc.expectedDuration.String(), receivedDuration.String()) } @@ -303,7 +321,7 @@ func TestSetTaskRunTimer(t *testing.T) { stopCh := make(chan struct{}) observer, _ := observer.New(zap.InfoLevel) - testHandler := NewTimeoutHandler(stopCh, zap.New(observer).Sugar()) + testHandler := NewTimeoutHandler(stopCh, zap.New(observer).Sugar(), getStore(t)) timerDuration := 50 * time.Millisecond timerFailDeadline := 100 * time.Millisecond doneCh := make(chan struct{}) diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index 29001b63cef..233e8ed9c80 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go @@ -25,6 +25,7 @@ import ( "github.com/knative/pkg/apis" duckv1beta1 "github.com/knative/pkg/apis/duck/v1beta1" "github.com/knative/pkg/configmap" + apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/reconciler" @@ -55,7 +56,18 @@ func getPipelineRunController(t *testing.T, d test.Data, recorder record.EventRe stopCh := make(chan struct{}) configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace()) logger := zap.New(observer).Sugar() - th := reconciler.NewTimeoutHandler(stopCh, logger) + store := apisconfig.NewStore(logger) + defaultConfig := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tekton-pipelines", + Name: "config-defaults", + }, + Data: map[string]string{ + "default-timeout-minutes": "60", + }, + } + store.OnConfigChanged(defaultConfig) + th := reconciler.NewTimeoutHandler(stopCh, logger, store) return test.TestAssets{ Controller: NewController( reconciler.Options{ diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun.go b/pkg/reconciler/v1alpha1/taskrun/taskrun.go index d403c4bc601..b622a59a555 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun.go @@ -26,6 +26,7 @@ import ( "github.com/knative/pkg/apis" "github.com/knative/pkg/controller" "github.com/knative/pkg/tracker" + apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" informers "github.com/tektoncd/pipeline/pkg/client/informers/externalversions/pipeline/v1alpha1" @@ -69,6 +70,7 @@ type Reconciler struct { tracker tracker.Interface cache *entrypoint.Cache timeoutHandler *reconciler.TimeoutSet + store *apisconfig.Store } // Check that our Reconciler implements controller.Reconciler @@ -84,6 +86,7 @@ func NewController( podInformer coreinformers.PodInformer, entrypointCache *entrypoint.Cache, timeoutHandler *reconciler.TimeoutSet, + store *apisconfig.Store, ) *controller.Impl { c := &Reconciler{ @@ -93,6 +96,10 @@ func NewController( clusterTaskLister: clusterTaskInformer.Lister(), resourceLister: resourceInformer.Lister(), timeoutHandler: timeoutHandler, + store: store, + } + if c.store == nil { + c.store = apisconfig.NewStore(c.Logger.Named("config-store")) } impl := controller.NewImpl(c, c.Logger, taskRunControllerName) @@ -533,9 +540,11 @@ func (c *Reconciler) checkTimeout(tr *v1alpha1.TaskRun, ts *v1alpha1.TaskSpec, d return false, nil } - timeout := reconciler.GetTimeout(tr.Spec.Timeout) + cfg := c.store.Load() + timeout := reconciler.GetTimeout(tr.Spec.Timeout, cfg.ConfigDefault.DefaultTimeoutMinutes) runtime := time.Since(tr.Status.StartTime.Time) c.Logger.Infof("Checking timeout for TaskRun %q (startTime %s, timeout %s, runtime %s)", tr.Name, tr.Status.StartTime, timeout, runtime) + if runtime > timeout { c.Logger.Infof("TaskRun %q is timeout (runtime %s over %s), deleting pod", tr.Name, runtime, timeout) // tr.Status.PodName will be empty if the pod was never successfully created. This condition diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go index 4aaa1f559ac..192c8edadc9 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -25,6 +25,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/knative/pkg/apis" "github.com/knative/pkg/configmap" + apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" "github.com/tektoncd/pipeline/pkg/logging" @@ -223,7 +224,19 @@ func getTaskRunController(t *testing.T, d test.Data) test.TestAssets { configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace()) stopCh := make(chan struct{}) logger := zap.New(observer).Sugar() - th := reconciler.NewTimeoutHandler(stopCh, logger) + store := apisconfig.NewStore(logger.Named("config-store")) + defaultConfig := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tekton-pipelines", + Name: "config-defaults", + }, + Data: map[string]string{ + "default-timeout-minutes": "60", + }, + } + store.OnConfigChanged(defaultConfig) + + th := reconciler.NewTimeoutHandler(stopCh, logger, store) return test.TestAssets{ Controller: NewController( reconciler.Options{ @@ -239,6 +252,7 @@ func getTaskRunController(t *testing.T, d test.Data) test.TestAssets { i.Pod, entrypointCache, th, + store, ), Logs: logs, Clients: c, @@ -1500,20 +1514,20 @@ func TestReconcileTimeouts(t *testing.T) { }, }, { - taskRun: tb.TaskRun("test-taskrun-default-timeout-10-minutes", "foo", + taskRun: tb.TaskRun("test-taskrun-default-timeout-60-minutes", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef(simpleTask.Name), ), tb.TaskRunStatus(tb.Condition(apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown}), - tb.TaskRunStartTime(time.Now().Add(-11*time.Minute)))), + tb.TaskRunStartTime(time.Now().Add(-61*time.Minute)))), expectedStatus: &apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, Reason: "TaskRunTimeout", - Message: `TaskRun "test-taskrun-default-timeout-10-minutes" failed to finish within "10m0s"`, + Message: `TaskRun "test-taskrun-default-timeout-60-minutes" failed to finish within "1h0m0s"`, }, }, }