diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 9ab690ffabfc..f7df4162e6cb 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -2954,7 +2954,7 @@ "format": "int32" }, "ttlStrategy": { - "title": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values\nUpdate", + "description": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values.", "$ref": "#/definitions/io.argoproj.workflow.v1alpha1.TTLStrategy" }, "volumeClaimTemplates": { diff --git a/config/config.go b/config/config.go index c91763bb7cb0..0f33f290628e 100644 --- a/config/config.go +++ b/config/config.go @@ -51,11 +51,13 @@ type Config struct { // controller watches workflows and pods that *are not* labeled with an instance id. InstanceID string `json:"instanceID,omitempty"` + // MetricsConfig specifies configuration for metrics emission MetricsConfig PrometheusConfig `json:"metricsConfig,omitempty"` // FeatureFlags for general/experimental features FeatureFlags FeatureFlags `json:"featureFlags,omitempty"` + // TelemetryConfig specifies configuration for telemetry emission TelemetryConfig PrometheusConfig `json:"telemetryConfig,omitempty"` // Parallelism limits the max total parallel workflows that can execute at the same time @@ -70,8 +72,8 @@ type Config struct { // Config customized Docker Sock path DockerSockPath string `json:"dockerSockPath,omitempty"` - // Default workflow spec, will be adde to workflow if the parameters are not set in the workflow - DefautWorkflowSpec *wfv1.WorkflowSpec `json:"workflowDefaults,omitempty"` + // WorkflowDefaults are values that will apply to all Workflows from this controller, unless overridden on the Workflow-level + WorkflowDefaults *wfv1.Workflow `json:"workflowDefaults,omitempty"` // PodSpecLogStrategy enable the logging of podspec on controller log. PodSpecLogStrategy PodSpecLogStrategy `json:"podSpecLogStrategy,omitempty"` diff --git a/docs/default-workflow-specs.md b/docs/default-workflow-specs.md index 742661d73384..0dce932655e5 100644 --- a/docs/default-workflow-specs.md +++ b/docs/default-workflow-specs.md @@ -4,34 +4,54 @@ > v2.7 and after -It's possible to set default workflow specs which will be written to all workflows if the spec of interest is not set. This can be configurated through the -workflow controller config [map](../workflow/config/config.go#L11) and the field [DefaultWorkflowSpec](../workflow/config/config.go#L69). +## Introduction +Default Workflow spec values can be set at the controller config map that will apply to all Workflows executed from said controller. +If a Workflow has a value that also has a default value set in the config map, thw Workflow's value will take precedence. -In order to edit the Default workflow spec for a controller, edit the workflow config map: +## Setting Default Workflow Values +Default Workflow values can be specified by adding them under the `workflowDefaults` key in the [`workflow-controller-configmap`](./workflow-controller-configmap.yaml). +Values can be added as the would under the `Workflow.spec` tag. -```bash -kubectl edit cm/workflow-controller-configmap -``` - - -As an example the time for a argo workflow to live after finish can be set, in the spec this field is known as ```secondsAfterCompletion``` in the ```ttlStrategy```. Example of how the config map could look with this filed can be found [here](./workflow-controller-configmap.yaml). - -In order to test it a example workflow can be submited, in this case the [coinflip example](../examples/coinflip.yaml), the following can be run: +For example, to specify default values that would partially produce the following `Workflow`: -```bash -argo submit ./examples/coinflip.yaml +```yaml +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: gc-ttl- + annotations: + argo: workflows + labels: + foo: bar +spec: + ttlStrategy: + secondsAfterSuccess: 5 # Time to live after workflow is successful + parallelism: 3 ``` -to verify that the the defaultd are set run - -```bash -argo get [YOUR_ARGO_WORKFLOW_NAME] -``` +The following would be specified in the Config Map: -You should then see the field, Ttl Strategy populated ```yaml -Ttl Strategy: - Seconds After Completion: 10 -``` +# This file describes the config settings available in the workflow controller configmap +apiVersion: v1 +kind: ConfigMap +metadata: + name: workflow-controller-configmap +data: + config: | + + # Default values that will apply to all Workflows from this controller, unless overridden on the Workflow-level + workflowDefaults: + metadata: + annotations: + argo: workflows + labels: + foo: bar + spec: + ttlStrategy: + secondsAfterSuccess: 5 + parallelism: 3 + +``` \ No newline at end of file diff --git a/docs/workflow-controller-configmap.yaml b/docs/workflow-controller-configmap.yaml index e85e0cdb4d90..7c4a939452d0 100644 --- a/docs/workflow-controller-configmap.yaml +++ b/docs/workflow-controller-configmap.yaml @@ -171,7 +171,15 @@ data: # name: argo-mysql-config # key: password - # default workflow spec, follow the workflow spec, more info: https://github.com/argoproj/argo/blob/master/examples/README.md#the-structure-of-workflow-specs + # Default values that will apply to all Workflows from this controller, unless overridden on the Workflow-level + # See more: docs/default-workflow-specs.yaml workflowDefaults: - ttlStrategy: - secondsAfterCompletion: 10 + metadata: + annotations: + argo: workflows + labels: + foo: bar + spec: + ttlStrategy: + secondsAfterSuccess: 5 + parallelism: 3 diff --git a/go.mod b/go.mod index 4176846998b6..ddde3037256e 100644 --- a/go.mod +++ b/go.mod @@ -76,7 +76,7 @@ require ( golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d golang.org/x/net v0.0.0-20200301022130-244492dfa37a golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d - golang.org/x/tools v0.0.0-20200323210725-ef1313dc6d0a // indirect + golang.org/x/tools v0.0.0-20200325010219-a49f79bcc224 // indirect google.golang.org/api v0.20.0 google.golang.org/genproto v0.0.0-20200317114155-1f3552e48f24 google.golang.org/grpc v1.28.0 diff --git a/go.sum b/go.sum index 7642b5fcec37..909f588f7646 100644 --- a/go.sum +++ b/go.sum @@ -607,8 +607,8 @@ golang.org/x/tools v0.0.0-20200207183749-b753a1ba74fa/go.mod h1:TB2adYChydJhpapK golang.org/x/tools v0.0.0-20200212150539-ea181f53ac56/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200224181240-023911ca70b2/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200317043434-63da46f3035e/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8= -golang.org/x/tools v0.0.0-20200323210725-ef1313dc6d0a h1:QpsrlnR31DN2tCItRzamUXN4oAe3U00ru8f0ik9HSYI= -golang.org/x/tools v0.0.0-20200323210725-ef1313dc6d0a/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8= +golang.org/x/tools v0.0.0-20200325010219-a49f79bcc224 h1:azwY/v0y0K4mFHVsg5+UrTgchqALYWpqVo6vL5OmkmI= +golang.org/x/tools v0.0.0-20200325010219-a49f79bcc224/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/pkg/apiclient/cronworkflow/cron-workflow.swagger.json b/pkg/apiclient/cronworkflow/cron-workflow.swagger.json index e49ffd9016bc..649298fb90f6 100644 --- a/pkg/apiclient/cronworkflow/cron-workflow.swagger.json +++ b/pkg/apiclient/cronworkflow/cron-workflow.swagger.json @@ -1699,7 +1699,7 @@ }, "ttlStrategy": { "$ref": "#/definitions/github.com.argoproj.argo.pkg.apis.workflow.v1alpha1.TTLStrategy", - "title": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values\nUpdate" + "description": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values." }, "activeDeadlineSeconds": { "type": "string", diff --git a/pkg/apiclient/workflow/workflow.swagger.json b/pkg/apiclient/workflow/workflow.swagger.json index f57bc51dedfc..5e01e27e199e 100644 --- a/pkg/apiclient/workflow/workflow.swagger.json +++ b/pkg/apiclient/workflow/workflow.swagger.json @@ -2106,7 +2106,7 @@ }, "ttlStrategy": { "$ref": "#/definitions/github.com.argoproj.argo.pkg.apis.workflow.v1alpha1.TTLStrategy", - "title": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values\nUpdate" + "description": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values." }, "activeDeadlineSeconds": { "type": "string", diff --git a/pkg/apiclient/workflowarchive/workflow-archive.swagger.json b/pkg/apiclient/workflowarchive/workflow-archive.swagger.json index c0a75a3f0b40..8024c9686704 100644 --- a/pkg/apiclient/workflowarchive/workflow-archive.swagger.json +++ b/pkg/apiclient/workflowarchive/workflow-archive.swagger.json @@ -1537,7 +1537,7 @@ }, "ttlStrategy": { "$ref": "#/definitions/github.com.argoproj.argo.pkg.apis.workflow.v1alpha1.TTLStrategy", - "title": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values\nUpdate" + "description": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values." }, "activeDeadlineSeconds": { "type": "string", diff --git a/pkg/apiclient/workflowtemplate/workflow-template.swagger.json b/pkg/apiclient/workflowtemplate/workflow-template.swagger.json index 8a61ccf2c6ba..186d0295b2b2 100644 --- a/pkg/apiclient/workflowtemplate/workflow-template.swagger.json +++ b/pkg/apiclient/workflowtemplate/workflow-template.swagger.json @@ -1567,7 +1567,7 @@ }, "ttlStrategy": { "$ref": "#/definitions/github.com.argoproj.argo.pkg.apis.workflow.v1alpha1.TTLStrategy", - "title": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values\nUpdate" + "description": "TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it\nSucceeded or Failed. If this struct is set, once the Workflow finishes, it will be\ndeleted after the time to live expires. If this field is unset,\nthe controller config map will hold the default values." }, "activeDeadlineSeconds": { "type": "string", diff --git a/pkg/apis/workflow/v1alpha1/generated.proto b/pkg/apis/workflow/v1alpha1/generated.proto index 85a3aaaba262..b639a1ce013c 100644 --- a/pkg/apis/workflow/v1alpha1/generated.proto +++ b/pkg/apis/workflow/v1alpha1/generated.proto @@ -1077,8 +1077,7 @@ message WorkflowSpec { // TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it // Succeeded or Failed. If this struct is set, once the Workflow finishes, it will be // deleted after the time to live expires. If this field is unset, - // the controller config map will hold the default values - // Update + // the controller config map will hold the default values. optional TTLStrategy ttlStrategy = 30; // Optional duration in seconds relative to the workflow start time which the workflow is diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go index 1534230800f4..0a40895e019b 100644 --- a/pkg/apis/workflow/v1alpha1/workflow_types.go +++ b/pkg/apis/workflow/v1alpha1/workflow_types.go @@ -239,8 +239,7 @@ type WorkflowSpec struct { // TTLStrategy limits the lifetime of a Workflow that has finished execution depending on if it // Succeeded or Failed. If this struct is set, once the Workflow finishes, it will be // deleted after the time to live expires. If this field is unset, - // the controller config map will hold the default values - // Update + // the controller config map will hold the default values. TTLStrategy *TTLStrategy `json:"ttlStrategy,omitempty" protobuf:"bytes,30,opt,name=ttlStrategy"` // Optional duration in seconds relative to the workflow start time which the workflow is diff --git a/workflow/controller/controller.go b/workflow/controller/controller.go index 264ac3cc73e4..eb94f6a80860 100644 --- a/workflow/controller/controller.go +++ b/workflow/controller/controller.go @@ -366,13 +366,15 @@ func (wfc *WorkflowController) processNextItem() bool { wfc.throttler.Remove(key) return true } - err = wfc.addingWorkflowDefaultValueIfValueNotExist(wf) + + err = wfc.setWorkflowDefaults(wf) if err != nil { - log.Warnf("Failed to unmarshal key '%s' to workflow object: %v", key, err) + log.Warnf("Failed to apply default workflow values to '%s': %v", wf.Name, err) woc := newWorkflowOperationCtx(wf, wfc) woc.markWorkflowFailed(fmt.Sprintf("invalid spec: %s", err.Error())) woc.persistUpdates() wfc.throttler.Remove(key) + return true } if wf.ObjectMeta.Labels[common.LabelKeyCompleted] == "true" { @@ -381,7 +383,9 @@ func (wfc *WorkflowController) processNextItem() bool { // but we are still draining the controller's workflow workqueue return true } + woc := newWorkflowOperationCtx(wf, wfc) + // Loading running workflow from persistence storage if nodeStatusOffload enabled if wf.Status.IsOffloadNodeStatus() { nodes, err := wfc.offloadNodeStatusRepo.Get(string(wf.UID), wf.GetOffloadNodeStatusVersion()) @@ -434,32 +438,6 @@ func (wfc *WorkflowController) processNextItem() bool { return true } -// addingWorkflowDefaultValueIfValueNotExist sets values in the workflow.Spec with defaults from the -// workflowController. Values in the workflow will be given the upper hand over the defaults. -// The defaults for the workflow controller is set in the WorkflowController.Config.DefautWorkflowSpec -func (wfc *WorkflowController) addingWorkflowDefaultValueIfValueNotExist(wf *wfv1.Workflow) error { - if wfc.Config.DefautWorkflowSpec != nil { - defaultsSpec, err := json.Marshal(*wfc.Config.DefautWorkflowSpec) - if err != nil { - return err - } - workflowSpec, err := json.Marshal(wf.Spec) - if err != nil { - return err - } - // https: //godoc.org/k8s.io/apimachinery/pkg/util/strategicpatch#StrategicMergePatch - new, err := strategicpatch.StrategicMergePatch(defaultsSpec, workflowSpec, wfv1.WorkflowSpec{}) - if err != nil { - return err - } - err = json.Unmarshal(new, &wf.Spec) - if err != nil { - return err - } - } - return nil -} - func (wfc *WorkflowController) podWorker() { for wfc.processNextPodItem() { } @@ -642,6 +620,31 @@ func (wfc *WorkflowController) newPodInformer() cache.SharedIndexInformer { return informer } +// setWorkflowDefaults sets values in the workflow.Spec with defaults from the +// workflowController. Values in the workflow will be given the upper hand over the defaults. +// The defaults for the workflow controller are set in the workflow-controller config map +func (wfc *WorkflowController) setWorkflowDefaults(wf *wfv1.Workflow) error { + if wfc.Config.WorkflowDefaults != nil { + defaultsSpec, err := json.Marshal(*wfc.Config.WorkflowDefaults) + if err != nil { + return err + } + workflowBytes, err := json.Marshal(wf) + if err != nil { + return err + } + mergedWf, err := strategicpatch.StrategicMergePatch(defaultsSpec, workflowBytes, wfv1.Workflow{}) + if err != nil { + return err + } + err = json.Unmarshal(mergedWf, &wf) + if err != nil { + return err + } + } + return nil +} + func (wfc *WorkflowController) newWorkflowTemplateInformer() wfextvv1alpha1.WorkflowTemplateInformer { return wfextv.NewSharedInformerFactoryWithOptions(wfc.wfclientset, workflowTemplateResyncPeriod, wfextv.WithNamespace(wfc.GetManagedNamespace())).Argoproj().V1alpha1().WorkflowTemplates() } diff --git a/workflow/controller/controller_test.go b/workflow/controller/controller_test.go index 99588870f8bf..18fd4290d1a5 100644 --- a/workflow/controller/controller_test.go +++ b/workflow/controller/controller_test.go @@ -50,6 +50,8 @@ apiVersion: argoproj.io/v1alpha1 kind: Workflow metadata: name: hello-world + labels: + foo: bar spec: entrypoint: whalesay serviceAccountName: whalesay @@ -130,8 +132,10 @@ func newControllerWithDefaults() *WorkflowController { return &WorkflowController{ Config: config.Config{ ExecutorImage: "executor:latest", - DefautWorkflowSpec: &wfv1.WorkflowSpec{ - HostNetwork: &myBool, + WorkflowDefaults: &wfv1.Workflow{ + Spec: wfv1.WorkflowSpec{ + HostNetwork: &myBool, + }, }, }, kubeclientset: fake.NewSimpleClientset(), @@ -158,16 +162,26 @@ func newControllerWithComplexDefaults() *WorkflowController { return &WorkflowController{ Config: config.Config{ ExecutorImage: "executor:latest", - DefautWorkflowSpec: &wfv1.WorkflowSpec{ - HostNetwork: &myBool, - Entrypoint: "good_entrypoint", - ServiceAccountName: "my_service_account", - TTLStrategy: &wfv1.TTLStrategy{ - SecondsAfterCompletion: &ten, - SecondsAfterSuccess: &ten, - SecondsAfterFailure: &ten, + WorkflowDefaults: &wfv1.Workflow{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "annotation": "value", + }, + Labels: map[string]string{ + "label": "value", + }, + }, + Spec: wfv1.WorkflowSpec{ + HostNetwork: &myBool, + Entrypoint: "good_entrypoint", + ServiceAccountName: "my_service_account", + TTLStrategy: &wfv1.TTLStrategy{ + SecondsAfterCompletion: &ten, + SecondsAfterSuccess: &ten, + SecondsAfterFailure: &ten, + }, + TTLSecondsAfterFinished: &seven, }, - TTLSecondsAfterFinished: &seven, }, }, kubeclientset: fake.NewSimpleClientset(), @@ -228,16 +242,15 @@ func makePodsPhaseAll(t *testing.T, phase apiv1.PodPhase, kubeclientset kubernet } func TestAddingWorkflowDefaultValueIfValueNotExist(t *testing.T) { - assert.Equal(t, "hello", "hello") ans := true controller := newController() workflow := unmarshalWF(helloWorldWf) - err := controller.addingWorkflowDefaultValueIfValueNotExist(workflow) + err := controller.setWorkflowDefaults(workflow) assert.NoError(t, err) assert.Equal(t, workflow, unmarshalWF(helloWorldWf)) controllerDefaults := newControllerWithDefaults() defautWorkflowSpec := unmarshalWF(helloWorldWf) - err = controllerDefaults.addingWorkflowDefaultValueIfValueNotExist(defautWorkflowSpec) + err = controllerDefaults.setWorkflowDefaults(defautWorkflowSpec) assert.NoError(t, err) assert.Equal(t, defautWorkflowSpec.Spec.HostNetwork, &ans) assert.NotEqual(t, defautWorkflowSpec, unmarshalWF(helloWorldWf)) @@ -245,28 +258,30 @@ func TestAddingWorkflowDefaultValueIfValueNotExist(t *testing.T) { } func TestAddingWorkflowDefaultComplex(t *testing.T) { - assert.Equal(t, "hello", "hello") controller := newControllerWithComplexDefaults() workflow := unmarshalWF(testDefaultWf) var ten int32 = 10 assert.Equal(t, workflow.Spec.Entrypoint, "whalesay") assert.Nil(t, workflow.Spec.TTLStrategy) - err := controller.addingWorkflowDefaultValueIfValueNotExist(workflow) + assert.Contains(t, workflow.Labels, "foo") + err := controller.setWorkflowDefaults(workflow) assert.NoError(t, err) assert.NotEqual(t, workflow, unmarshalWF(testDefaultWf)) assert.Equal(t, workflow.Spec.Entrypoint, "whalesay") assert.Equal(t, workflow.Spec.ServiceAccountName, "whalesay") assert.Equal(t, *workflow.Spec.TTLStrategy.SecondsAfterFailure, ten) + assert.Contains(t, workflow.Labels, "foo") + assert.Contains(t, workflow.Labels, "label") + assert.Contains(t, workflow.Annotations, "annotation") } func TestAddingWorkflowDefaultComplexTwo(t *testing.T) { - assert.Equal(t, "hello", "hello") controller := newControllerWithComplexDefaults() workflow := unmarshalWF(testDefaultWfTTL) var ten int32 = 10 var seven int32 = 7 var five int32 = 5 - err := controller.addingWorkflowDefaultValueIfValueNotExist(workflow) + err := controller.setWorkflowDefaults(workflow) assert.NoError(t, err) assert.NotEqual(t, workflow, unmarshalWF(testDefaultWfTTL)) assert.Equal(t, workflow.Spec.Entrypoint, "whalesay") @@ -274,4 +289,7 @@ func TestAddingWorkflowDefaultComplexTwo(t *testing.T) { assert.Equal(t, *workflow.Spec.TTLStrategy.SecondsAfterCompletion, five) assert.Equal(t, *workflow.Spec.TTLStrategy.SecondsAfterFailure, ten) assert.Equal(t, *workflow.Spec.TTLSecondsAfterFinished, seven) + assert.NotContains(t, workflow.Labels, "foo") + assert.Contains(t, workflow.Labels, "label") + assert.Contains(t, workflow.Annotations, "annotation") }