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

TEP-0069: Support retries for custom task in a pipeline. #4135

Closed
wants to merge 1 commit into from
Closed
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
6 changes: 6 additions & 0 deletions config/config-defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ data:
# minutes to use for TaskRun and PipelineRun, if none is specified.
default-timeout-minutes: "60" # 60 minutes

# default-short-timeout-seconds contains the default number of
# seconds to wait for custom task to respond, on timeout it is assumed
# custom task does not support the feature. It is used to
# quickly timeout a retry in a custom-task.
default-short-timeout-seconds: "30" # 30 seconds

# default-service-account contains the default service account name
# to use for TaskRun and PipelineRun, if none is specified.
default-service-account: "default"
Expand Down
5 changes: 5 additions & 0 deletions docs/runs.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ the `example.dev` API group, with the version `v1alpha1`.
behaviour is supported then, appropriate validation error should be
updated to the `Run`'s status.

4. A custom task controller can support `retries` by watching the `/spec/status`
of `Run`. If it is `RunRetry` then start executing retry and clear its
status to let `tektoncd` controller know that the custom task has started
retrying.

### Specifying `Parameters`

If a custom task supports [`parameters`](tasks.md#parameters), you can use the
Expand Down
57 changes: 35 additions & 22 deletions pkg/apis/config/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,31 @@ import (
)

const (
DefaultTimeoutMinutes = 60
NoTimeoutDuration = 0 * time.Minute
defaultTimeoutMinutesKey = "default-timeout-minutes"
defaultServiceAccountKey = "default-service-account"
DefaultServiceAccountValue = "default"
defaultManagedByLabelValueKey = "default-managed-by-label-value"
DefaultManagedByLabelValue = "tekton-pipelines"
defaultPodTemplateKey = "default-pod-template"
defaultCloudEventsSinkKey = "default-cloud-events-sink"
DefaultCloudEventSinkValue = ""
defaultTaskRunWorkspaceBinding = "default-task-run-workspace-binding"
DefaultTimeoutMinutes = 60
NoTimeoutDuration = 0 * time.Minute
DefaultShortTimeoutSecondsValue = 30
defaultShortTimeoutSecondsKey = "default-short-timeout-seconds"
defaultTimeoutMinutesKey = "default-timeout-minutes"
defaultServiceAccountKey = "default-service-account"
DefaultServiceAccountValue = "default"
defaultManagedByLabelValueKey = "default-managed-by-label-value"
DefaultManagedByLabelValue = "tekton-pipelines"
defaultPodTemplateKey = "default-pod-template"
defaultCloudEventsSinkKey = "default-cloud-events-sink"
DefaultCloudEventSinkValue = ""
defaultTaskRunWorkspaceBinding = "default-task-run-workspace-binding"
)

// Defaults holds the default configurations
// +k8s:deepcopy-gen=true
type Defaults struct {
DefaultTimeoutMinutes int
DefaultServiceAccount string
DefaultManagedByLabelValue string
DefaultPodTemplate *pod.Template
DefaultCloudEventsSink string
DefaultTaskRunWorkspaceBinding string
DefaultShortTimeoutSecondsValue int
DefaultTimeoutMinutes int
DefaultServiceAccount string
DefaultManagedByLabelValue string
DefaultPodTemplate *pod.Template
DefaultCloudEventsSink string
DefaultTaskRunWorkspaceBinding string
}

// GetDefaultsConfigName returns the name of the configmap containing all
Expand All @@ -71,7 +74,8 @@ func (cfg *Defaults) Equals(other *Defaults) bool {
return false
}

return other.DefaultTimeoutMinutes == cfg.DefaultTimeoutMinutes &&
return other.DefaultShortTimeoutSecondsValue == cfg.DefaultShortTimeoutSecondsValue &&
other.DefaultTimeoutMinutes == cfg.DefaultTimeoutMinutes &&
other.DefaultServiceAccount == cfg.DefaultServiceAccount &&
other.DefaultManagedByLabelValue == cfg.DefaultManagedByLabelValue &&
other.DefaultPodTemplate.Equals(cfg.DefaultPodTemplate) &&
Expand All @@ -82,10 +86,11 @@ func (cfg *Defaults) Equals(other *Defaults) bool {
// NewDefaultsFromMap returns a Config given a map corresponding to a ConfigMap
func NewDefaultsFromMap(cfgMap map[string]string) (*Defaults, error) {
tc := Defaults{
DefaultTimeoutMinutes: DefaultTimeoutMinutes,
DefaultServiceAccount: DefaultServiceAccountValue,
DefaultManagedByLabelValue: DefaultManagedByLabelValue,
DefaultCloudEventsSink: DefaultCloudEventSinkValue,
DefaultShortTimeoutSecondsValue: DefaultShortTimeoutSecondsValue,
DefaultTimeoutMinutes: DefaultTimeoutMinutes,
DefaultServiceAccount: DefaultServiceAccountValue,
DefaultManagedByLabelValue: DefaultManagedByLabelValue,
DefaultCloudEventsSink: DefaultCloudEventSinkValue,
}

if defaultTimeoutMin, ok := cfgMap[defaultTimeoutMinutesKey]; ok {
Expand All @@ -96,6 +101,14 @@ func NewDefaultsFromMap(cfgMap map[string]string) (*Defaults, error) {
tc.DefaultTimeoutMinutes = int(timeout)
}

if defaultShortTimeoutSec, ok := cfgMap[defaultShortTimeoutSecondsKey]; ok {
timeout, err := strconv.ParseInt(defaultShortTimeoutSec, 10, 0)
if err != nil {
return nil, fmt.Errorf("failed parsing config %q", defaultShortTimeoutSecondsKey)
}
tc.DefaultShortTimeoutSecondsValue = int(timeout)
}

if defaultServiceAccount, ok := cfgMap[defaultServiceAccountKey]; ok {
tc.DefaultServiceAccount = defaultServiceAccount
}
Expand Down
41 changes: 32 additions & 9 deletions pkg/apis/config/default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,19 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
testCases := []testCase{
{
expectedConfig: &config.Defaults{
DefaultTimeoutMinutes: 50,
DefaultServiceAccount: "tekton",
DefaultManagedByLabelValue: "something-else",
DefaultShortTimeoutSecondsValue: 30,
DefaultTimeoutMinutes: 50,
DefaultServiceAccount: "tekton",
DefaultManagedByLabelValue: "something-else",
},
fileName: config.GetDefaultsConfigName(),
},
{
expectedConfig: &config.Defaults{
DefaultTimeoutMinutes: 50,
DefaultServiceAccount: "tekton",
DefaultManagedByLabelValue: config.DefaultManagedByLabelValue,
DefaultShortTimeoutSecondsValue: 30,
DefaultTimeoutMinutes: 50,
DefaultServiceAccount: "tekton",
DefaultManagedByLabelValue: config.DefaultManagedByLabelValue,
DefaultPodTemplate: &pod.Template{
NodeSelector: map[string]string{
"label": "value",
Expand Down Expand Up @@ -79,9 +81,10 @@ func TestNewDefaultsFromConfigMap(t *testing.T) {
func TestNewDefaultsFromEmptyConfigMap(t *testing.T) {
DefaultsConfigEmptyName := "config-defaults-empty"
expectedConfig := &config.Defaults{
DefaultTimeoutMinutes: 60,
DefaultManagedByLabelValue: "tekton-pipelines",
DefaultServiceAccount: "default",
DefaultShortTimeoutSecondsValue: 30,
DefaultTimeoutMinutes: 60,
DefaultManagedByLabelValue: "tekton-pipelines",
DefaultServiceAccount: "default",
}
verifyConfigFileWithExpectedConfig(t, DefaultsConfigEmptyName, expectedConfig)
}
Expand Down Expand Up @@ -117,6 +120,16 @@ func TestEquals(t *testing.T) {
right: &config.Defaults{},
expected: true,
},
{
name: "different default short timeout",
left: &config.Defaults{
DefaultShortTimeoutSecondsValue: 10,
},
right: &config.Defaults{
DefaultShortTimeoutSecondsValue: 20,
},
expected: false,
},
{
name: "different default timeout",
left: &config.Defaults{
Expand All @@ -137,6 +150,16 @@ func TestEquals(t *testing.T) {
},
expected: true,
},
{
name: "same default short timeout",
left: &config.Defaults{
DefaultShortTimeoutSecondsValue: 20,
},
right: &config.Defaults{
DefaultShortTimeoutSecondsValue: 20,
},
expected: true,
},
{
name: "different default pod template",
left: &config.Defaults{
Expand Down
14 changes: 13 additions & 1 deletion pkg/apis/pipeline/v1alpha1/run_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,14 @@ type RunSpec struct {
// +optional
Params []v1beta1.Param `json:"params,omitempty"`

// Used for cancelling a run (and maybe more later on)
// Used for cancelling/retrying a run (and maybe more later on)
// +optional
Status RunSpecStatus `json:"status,omitempty"`

// Used for propagating retries count to custom tasks
// +optional
Retries int `json:"retries,omitempty"`

// +optional
ServiceAccountName string `json:"serviceAccountName"`

Expand All @@ -80,6 +84,9 @@ const (
// RunSpecStatusCancelled indicates that the user wants to cancel the run,
// if not already cancelled or terminated
RunSpecStatusCancelled RunSpecStatus = "RunCancelled"

// RunSpecStatusRetry indicates that custom task needs to honor the retry request
RunSpecStatusRetry RunSpecStatus = "RunRetry"
)

// TODO(jasonhall): Move this to a Params type so other code can use it?
Expand Down Expand Up @@ -173,6 +180,11 @@ func (r *Run) IsCancelled() bool {
return r.Spec.Status == RunSpecStatusCancelled
}

// IsRetry returns true if the Run's spec status is set to Retry state
func (r *Run) IsRetry() bool {
return r.Spec.Status == RunSpecStatusRetry
}

// IsDone returns true if the Run's status indicates that it is done.
func (r *Run) IsDone() bool {
return !r.Status.GetCondition(apis.ConditionSucceeded).IsUnknown()
Expand Down
11 changes: 11 additions & 0 deletions pkg/apis/pipeline/v1alpha1/run_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,17 @@ func TestRunIsCancelled(t *testing.T) {
}
}

func TestRunIsRetry(t *testing.T) {
run := v1alpha1.Run{
Spec: v1alpha1.RunSpec{
Status: v1alpha1.RunSpecStatusRetry,
},
}
if !run.IsRetry() {
t.Fatal("Expected run status to be retry")
}
}

// TestRunStatusExtraFields tests that extraFields in a RunStatus can be parsed
// from YAML.
func TestRunStatus(t *testing.T) {
Expand Down
3 changes: 0 additions & 3 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,6 @@ func (pt PipelineTask) validateCustomTask() (errs *apis.FieldError) {
errs = errs.Also(apis.ErrInvalidValue("custom tasks do not support conditions - use when expressions instead", "conditions"))
}
// TODO(#3133): Support these features if possible.
if pt.Retries > 0 {
errs = errs.Also(apis.ErrInvalidValue("custom tasks do not support retries", "retries"))
}
if pt.Resources != nil {
errs = errs.Also(apis.ErrInvalidValue("custom tasks do not support PipelineResources", "resources"))
}
Expand Down
11 changes: 0 additions & 11 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,17 +184,6 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) {
Message: `invalid value: custom tasks do not support conditions - use when expressions instead`,
Paths: []string{"conditions"},
},
}, {
name: "custom task doesn't support retries",
task: PipelineTask{
Name: "foo",
Retries: 3,
TaskRef: &TaskRef{APIVersion: "example.dev/v0", Kind: "Example"},
},
expectedError: apis.FieldError{
Message: `invalid value: custom tasks do not support retries`,
Paths: []string{"retries"},
},
}, {
name: "custom task doesn't support pipeline resources",
task: PipelineTask{
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/run/v1alpha1/runstatus_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ type RunStatusFields struct {
// +optional
Results []RunResult `json:"results,omitempty"`

// RetriesStatus contains the history of RunStatus, in case of a retry.
// +optional
RetriesStatus []RunStatus `json:"retriesStatus,omitempty"`

// ExtraFields holds arbitrary fields provided by the custom task
// controller.
ExtraFields runtime.RawExtension `json:"extraFields,omitempty"`
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/run/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading