Skip to content

Commit

Permalink
feat: Add default field in parameters.valueFrom (#2500)
Browse files Browse the repository at this point in the history
  • Loading branch information
simster7 committed Mar 23, 2020
1 parent 33cd4f2 commit 09291d9
Show file tree
Hide file tree
Showing 14 changed files with 576 additions and 370 deletions.
31 changes: 31 additions & 0 deletions api/openapi-spec/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
],
"swagger": "2.0",
"info": {
"description": "Workflow Service API performs CRUD actions against application resources",
"title": "Argo",
"version": "latest"
},
Expand Down Expand Up @@ -2722,6 +2723,10 @@
"type": "object",
"title": "ValueFrom describes a location in which to obtain the value to a parameter",
"properties": {
"default": {
"type": "string",
"title": "Default specifies a value to be used if retrieving the value from the specified source fails"
},
"jqFilter": {
"type": "string",
"title": "JQFilter expression against the resource object in resource templates"
Expand Down Expand Up @@ -5874,5 +5879,31 @@
"description": "HTTP Basic authentication",
"type": "basic"
}
},
"x-stream-definitions": {
"io.argoproj.workflow.v1alpha1.LogEntry": {
"properties": {
"error": {
"$ref": "#/definitions/grpc.gateway.runtime.StreamError"
},
"result": {
"$ref": "#/definitions/io.argoproj.workflow.v1alpha1.LogEntry"
}
},
"title": "Stream result of io.argoproj.workflow.v1alpha1.LogEntry",
"type": "object"
},
"io.argoproj.workflow.v1alpha1.WorkflowWatchEvent": {
"properties": {
"error": {
"$ref": "#/definitions/grpc.gateway.runtime.StreamError"
},
"result": {
"$ref": "#/definitions/io.argoproj.workflow.v1alpha1.WorkflowWatchEvent"
}
},
"title": "Stream result of io.argoproj.workflow.v1alpha1.WorkflowWatchEvent",
"type": "object"
}
}
}
1 change: 1 addition & 0 deletions examples/output-parameter.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ spec:
parameters:
- name: hello-param
valueFrom:
default: "Foobar" # Default value to use if retrieving valueFrom fails. If not provided workflow will fail instead
path: /tmp/hello_world.txt

- name: print-message
Expand Down
4 changes: 4 additions & 0 deletions pkg/apiclient/cronworkflow/cron-workflow.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,10 @@
"parameter": {
"type": "string",
"title": "Parameter reference to a step or dag task in which to retrieve an output parameter value from\n(e.g. '{{steps.mystep.outputs.myparam}}')"
},
"default": {
"type": "string",
"title": "Default specifies a value to be used if retrieving the value from the specified source fails"
}
},
"title": "ValueFrom describes a location in which to obtain the value to a parameter"
Expand Down
4 changes: 4 additions & 0 deletions pkg/apiclient/workflow/workflow.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1929,6 +1929,10 @@
"parameter": {
"type": "string",
"title": "Parameter reference to a step or dag task in which to retrieve an output parameter value from\n(e.g. '{{steps.mystep.outputs.myparam}}')"
},
"default": {
"type": "string",
"title": "Default specifies a value to be used if retrieving the value from the specified source fails"
}
},
"title": "ValueFrom describes a location in which to obtain the value to a parameter"
Expand Down
4 changes: 4 additions & 0 deletions pkg/apiclient/workflowarchive/workflow-archive.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,10 @@
"parameter": {
"type": "string",
"title": "Parameter reference to a step or dag task in which to retrieve an output parameter value from\n(e.g. '{{steps.mystep.outputs.myparam}}')"
},
"default": {
"type": "string",
"title": "Default specifies a value to be used if retrieving the value from the specified source fails"
}
},
"title": "ValueFrom describes a location in which to obtain the value to a parameter"
Expand Down
4 changes: 4 additions & 0 deletions pkg/apiclient/workflowtemplate/workflow-template.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1420,6 +1420,10 @@
"parameter": {
"type": "string",
"title": "Parameter reference to a step or dag task in which to retrieve an output parameter value from\n(e.g. '{{steps.mystep.outputs.myparam}}')"
},
"default": {
"type": "string",
"title": "Default specifies a value to be used if retrieving the value from the specified source fails"
}
},
"title": "ValueFrom describes a location in which to obtain the value to a parameter"
Expand Down
769 changes: 405 additions & 364 deletions pkg/apis/workflow/v1alpha1/generated.pb.go

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions pkg/apis/workflow/v1alpha1/generated.proto

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

3 changes: 3 additions & 0 deletions pkg/apis/workflow/v1alpha1/workflow_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,9 @@ type ValueFrom struct {
// Parameter reference to a step or dag task in which to retrieve an output parameter value from
// (e.g. '{{steps.mystep.outputs.myparam}}')
Parameter string `json:"parameter,omitempty" protobuf:"bytes,4,opt,name=parameter"`

// Default specifies a value to be used if retrieving the value from the specified source fails
Default string `json:"default,omitempty" protobuf:"bytes,5,opt,name=default"`
}

// Artifact indicates an artifact to place at a specified path
Expand Down
58 changes: 58 additions & 0 deletions test/e2e/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,64 @@ func (s *FunctionalSuite) TestTerminateBehavior() {
})
}

func (s *FunctionalSuite) TestDefaultParameterOutputs() {
s.Given().
Workflow(`
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
name: default-params
labels:
argo-e2e: true
spec:
entrypoint: start
templates:
- name: start
steps:
- - name: generate-1
template: generate
- - name: generate-2
when: "True == False"
template: generate
outputs:
parameters:
- name: nested-out-parameter
valueFrom:
default: "Default value"
parameter: "{{steps.generate-2.outputs.parameters.out-parameter}}"
- name: generate
container:
image: docker/whalesay:latest
command: [sh, -c]
args: ["
echo 'my-output-parameter' > /tmp/my-output-parameter.txt
"]
outputs:
parameters:
- name: out-parameter
valueFrom:
path: /tmp/my-output-parameter.txt
`).
When().
SubmitWorkflow().
WaitForWorkflow(30 * time.Second).
Then().
ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
assert.Equal(t, wfv1.NodeSucceeded, status.Phase)
assert.True(t, status.Nodes.Any(func(node wfv1.NodeStatus) bool {
if node.Outputs != nil {
for _, param := range node.Outputs.Parameters {
if param.Value != nil && *param.Value == "Default value" {
return true
}
}
}
return false
}))
})
}

func TestFunctionalSuite(t *testing.T) {
suite.Run(t, new(FunctionalSuite))
}
7 changes: 6 additions & 1 deletion workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1801,7 +1801,12 @@ func getTemplateOutputsFromScope(tmpl *wfv1.Template, scope *wfScope) (*wfv1.Out
}
val, err := scope.resolveParameter(param.ValueFrom.Parameter)
if err != nil {
return nil, err
// We have a default value to use instead of returning an error
if param.ValueFrom.Default != "" {
val = param.ValueFrom.Default
} else {
return nil, err
}
}
param.Value = &val
param.ValueFrom = nil
Expand Down
14 changes: 12 additions & 2 deletions workflow/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,14 +425,24 @@ func (we *WorkflowExecutor) SaveParameters() error {
log.Infof("Copying %s from base image layer", param.ValueFrom.Path)
output, err = we.RuntimeExecutor.GetFileContents(mainCtrID, param.ValueFrom.Path)
if err != nil {
return err
// We have a default value to use instead of returning an error
if param.ValueFrom.Default != "" {
output = param.ValueFrom.Default
} else {
return err
}
}
} else {
log.Infof("Copying %s from from volume mount", param.ValueFrom.Path)
mountedPath := filepath.Join(common.ExecutorMainFilesystemDir, param.ValueFrom.Path)
out, err := ioutil.ReadFile(mountedPath)
if err != nil {
return err
// We have a default value to use instead of returning an error
if param.ValueFrom.Default != "" {
output = param.ValueFrom.Default
} else {
return err
}
}
output = string(out)
}
Expand Down
33 changes: 33 additions & 0 deletions workflow/executor/executor_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package executor

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -104,3 +105,35 @@ func TestIsBaseImagePath(t *testing.T) {
assert.False(t, we.isBaseImagePath("/user-mount/some-path/foo"))
assert.True(t, we.isBaseImagePath("/user-mount-coincidence"))
}

func TestDefaultParameters(t *testing.T) {
fakeClientset := fake.NewSimpleClientset()
mockRuntimeExecutor := mocks.ContainerRuntimeExecutor{}
templateWithOutParam := wfv1.Template{
Outputs: wfv1.Outputs{
Parameters: []wfv1.Parameter{
{
Name: "my-out",
ValueFrom: &wfv1.ValueFrom{
Default: "Default Value",
Path: "/path",
},
},
},
},
}
we := WorkflowExecutor{
PodName: fakePodName,
Template: templateWithOutParam,
ClientSet: fakeClientset,
Namespace: fakeNamespace,
PodAnnotationsPath: fakeAnnotations,
ExecutionControl: nil,
RuntimeExecutor: &mockRuntimeExecutor,
mainContainerID: fakeContainerID,
}
mockRuntimeExecutor.On("GetFileContents", fakeContainerID, "/path").Return("", fmt.Errorf("file not found"))
err := we.SaveParameters()
assert.NoError(t, err)
assert.Equal(t, "Default Value", *we.Template.Outputs.Parameters[0].Value)
}
11 changes: 8 additions & 3 deletions workflow/executor/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,15 @@ func (we *WorkflowExecutor) SaveResourceParameters(resourceNamespace string, res
log.Info(cmd.Args)
out, err := cmd.Output()
if err != nil {
if exErr, ok := err.(*exec.ExitError); ok {
log.Errorf("`%s` stderr:\n%s", cmd.Args, string(exErr.Stderr))
// We have a default value to use instead of returning an error
if param.ValueFrom.Default != "" {
out = []byte(param.ValueFrom.Default)
} else {
if exErr, ok := err.(*exec.ExitError); ok {
log.Errorf("`%s` stderr:\n%s", cmd.Args, string(exErr.Stderr))
}
return errors.InternalWrapError(err)
}
return errors.InternalWrapError(err)
}
output := string(out)
we.Template.Outputs.Parameters[i].Value = &output
Expand Down

0 comments on commit 09291d9

Please sign in to comment.