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

fix: Support --parameters-file where ARGO_SERVER specified. Fixes #8160 #8213

Merged
merged 3 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
4 changes: 0 additions & 4 deletions api/jsonschema/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -7033,10 +7033,6 @@
"$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.OwnerReference",
"description": "OwnerReference creates a metadata.ownerReference"
},
"parameterFile": {
"description": "ParameterFile holds a reference to a parameter file. This option is not supported in API",
"type": "string"
},
"parameters": {
"description": "Parameters passes input parameters to workflow",
"items": {
Expand Down
4 changes: 0 additions & 4 deletions api/openapi-spec/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -11372,10 +11372,6 @@
"description": "OwnerReference creates a metadata.ownerReference",
"$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.OwnerReference"
},
"parameterFile": {
"description": "ParameterFile holds a reference to a parameter file. This option is not supported in API",
"type": "string"
},
"parameters": {
"description": "Parameters passes input parameters to workflow",
"type": "array",
Expand Down
11 changes: 8 additions & 3 deletions cmd/argo/commands/cron/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"log"
"os"

"github.com/argoproj/pkg/errors"
"github.com/argoproj/pkg/json"
"github.com/spf13/cobra"

Expand All @@ -24,8 +25,9 @@ type cliCreateOpts struct {

func NewCreateCommand() *cobra.Command {
var (
cliCreateOpts cliCreateOpts
submitOpts wfv1.SubmitOpts
cliCreateOpts cliCreateOpts
submitOpts wfv1.SubmitOpts
parametersFile string
)
command := &cobra.Command{
Use: "create FILE1 FILE2...",
Expand All @@ -36,11 +38,14 @@ func NewCreateCommand() *cobra.Command {
os.Exit(1)
}

err := util.ReadParametersFile(parametersFile, &submitOpts)
errors.CheckError(err)

CreateCronWorkflows(cmd.Context(), args, &cliCreateOpts, &submitOpts)
},
}

util.PopulateSubmitOpts(command, &submitOpts, false)
util.PopulateSubmitOpts(command, &submitOpts, &parametersFile, false)
command.Flags().StringVarP(&cliCreateOpts.output, "output", "o", "", "Output format. One of: name|json|yaml|wide")
command.Flags().BoolVar(&cliCreateOpts.strict, "strict", true, "perform strict workflow validation")
command.Flags().StringVar(&cliCreateOpts.schedule, "schedule", "", "override cron workflow schedule")
Expand Down
14 changes: 9 additions & 5 deletions cmd/argo/commands/submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ type cliSubmitOpts struct {

func NewSubmitCommand() *cobra.Command {
var (
submitOpts wfv1.SubmitOpts
cliSubmitOpts cliSubmitOpts
priority int32
from string
submitOpts wfv1.SubmitOpts
parametersFile string
cliSubmitOpts cliSubmitOpts
priority int32
from string
)
command := &cobra.Command{
Use: "submit [FILE... | --from `kind/name]",
Expand Down Expand Up @@ -72,6 +73,9 @@ func NewSubmitCommand() *cobra.Command {
log.Warn("--status should only be used with --watch")
}

err := util.ReadParametersFile(parametersFile, &submitOpts)
errors.CheckError(err)

ctx, apiClient := client.NewAPIClient(cmd.Context())
serviceClient := apiClient.NewWorkflowServiceClient()
namespace := client.Namespace()
Expand All @@ -86,7 +90,7 @@ func NewSubmitCommand() *cobra.Command {
}
},
}
util.PopulateSubmitOpts(command, &submitOpts, true)
util.PopulateSubmitOpts(command, &submitOpts, &parametersFile, true)
command.Flags().StringVarP(&cliSubmitOpts.output, "output", "o", "", "Output format. One of: name|json|yaml|wide")
command.Flags().BoolVarP(&cliSubmitOpts.wait, "wait", "w", false, "wait for the workflow to complete")
command.Flags().BoolVar(&cliSubmitOpts.watch, "watch", false, "watch the workflow until it completes")
Expand Down
2 changes: 0 additions & 2 deletions pkg/apis/workflow/v1alpha1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ type SubmitOpts struct {
Entrypoint string `json:"entryPoint,omitempty" protobuf:"bytes,4,opt,name=entrypoint"`
// Parameters passes input parameters to workflow
Parameters []string `json:"parameters,omitempty" protobuf:"bytes,5,rep,name=parameters"`
// ParameterFile holds a reference to a parameter file. This option is not supported in API
ParameterFile string `json:"parameterFile,omitempty" protobuf:"bytes,6,opt,name=parameterFile"`
// ServiceAccount runs all pods in the workflow using specified ServiceAccount.
ServiceAccount string `json:"serviceAccount,omitempty" protobuf:"bytes,7,opt,name=serviceAccount"`
// DryRun validates the workflow on the client-side without creating it. This option is not supported in API
Expand Down
1,149 changes: 554 additions & 595 deletions pkg/apis/workflow/v1alpha1/generated.pb.go

Large diffs are not rendered by default.

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

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

7 changes: 0 additions & 7 deletions pkg/apis/workflow/v1alpha1/openapi_generated.go

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

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ Name | Type | Description | Notes
**labels** | **String** | Labels adds to metadata.labels | [optional]
**name** | **String** | Name overrides metadata.name | [optional]
**ownerReference** | [**OwnerReference**](OwnerReference.md) | | [optional]
**parameterFile** | **String** | ParameterFile holds a reference to a parameter file. This option is not supported in API | [optional]
**parameters** | **List<String>** | Parameters passes input parameters to workflow | [optional]
**podPriorityClassName** | **String** | Set the podPriorityClassName of the workflow | [optional]
**priority** | **Integer** | Priority is used if controller is configured to process limited number of workflows in parallel, higher priority workflows are processed first. | [optional]
Expand Down

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

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

1 change: 0 additions & 1 deletion sdks/python/client/docs/WorkflowServiceApi.md

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

10 changes: 10 additions & 0 deletions test/e2e/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,16 @@ func (s *CLISuite) TestLogProblems() {
})
}

func (s *CLISuite) TestParametersFile() {
err := os.WriteFile("/tmp/parameters-file.yaml", []byte("message: hello"), os.ModePerm)
assert.NoError(s.T(), err)
s.Given().
RunCli([]string{"submit", "testdata/parameters-workflow.yaml", "-l", "workflows.argoproj.io/test=true", "--parameter-file=/tmp/parameters-file.yaml"}, func(t *testing.T, output string, err error) {
assert.NoError(t, err)
assert.Contains(t, output, "message: hello")
})
}

func (s *CLISuite) TestRoot() {
s.Run("Submit", func() {
s.Given().RunCli([]string{"submit", "testdata/basic-workflow.yaml", "-l", "workflows.argoproj.io/test=true"}, func(t *testing.T, output string, err error) {
Expand Down
16 changes: 16 additions & 0 deletions test/e2e/testdata/parameters-workflow.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: parameters-
spec:
entrypoint: main
arguments:
parameters:
- name: message
templates:
- name: main
container:
image: argoproj/argosay:v2
args:
- echo
- "{{workflow.parameters.message}}"
79 changes: 36 additions & 43 deletions workflow/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,13 @@ func CreateServerDryRun(ctx context.Context, wf *wfv1.Workflow, wfClientset wfcl
return wf, err
}

func PopulateSubmitOpts(command *cobra.Command, submitOpts *wfv1.SubmitOpts, includeDryRun bool) {
func PopulateSubmitOpts(command *cobra.Command, submitOpts *wfv1.SubmitOpts, parameterFile *string, includeDryRun bool) {
command.Flags().StringVar(&submitOpts.Name, "name", "", "override metadata.name")
command.Flags().StringVar(&submitOpts.GenerateName, "generate-name", "", "override metadata.generateName")
command.Flags().StringVar(&submitOpts.Entrypoint, "entrypoint", "", "override entrypoint")
command.Flags().StringArrayVarP(&submitOpts.Parameters, "parameter", "p", []string{}, "pass an input parameter")
command.Flags().StringVar(&submitOpts.ServiceAccount, "serviceaccount", "", "run all pods in the workflow using specified serviceaccount")
command.Flags().StringVarP(&submitOpts.ParameterFile, "parameter-file", "f", "", "pass a file containing all input parameters")
command.Flags().StringVarP(parameterFile, "parameter-file", "f", "", "pass a file containing all input parameters")
command.Flags().StringVarP(&submitOpts.Labels, "labels", "l", "", "Comma separated labels to apply to the workflow. Will override previous values.")

if includeDryRun {
Expand Down Expand Up @@ -275,7 +275,7 @@ func ApplySubmitOpts(wf *wfv1.Workflow, opts *wfv1.SubmitOpts) error {
}
}
wf.SetAnnotations(wfAnnotations)
if len(opts.Parameters) > 0 || opts.ParameterFile != "" {
if len(opts.Parameters) > 0 {
newParams := make([]wfv1.Parameter, 0)
passedParams := make(map[string]bool)
for _, paramStr := range opts.Parameters {
Expand All @@ -287,46 +287,6 @@ func ApplySubmitOpts(wf *wfv1.Workflow, opts *wfv1.SubmitOpts) error {
newParams = append(newParams, param)
passedParams[param.Name] = true
}

// Add parameters from a parameter-file, if one was provided
if opts.ParameterFile != "" {
var body []byte
var err error
if cmdutil.IsURL(opts.ParameterFile) {
body, err = ReadFromUrl(opts.ParameterFile)
if err != nil {
return errors.InternalWrapError(err)
}
} else {
body, err = ioutil.ReadFile(opts.ParameterFile)
if err != nil {
return errors.InternalWrapError(err)
}
}

yamlParams := map[string]json.RawMessage{}
err = yaml.Unmarshal(body, &yamlParams)
if err != nil {
return errors.InternalWrapError(err)
}

for k, v := range yamlParams {
// We get quoted strings from the yaml file.
value, err := strconv.Unquote(string(v))
if err != nil {
// the string is already clean.
value = string(v)
}
param := wfv1.Parameter{Name: k, Value: wfv1.AnyStringPtr(value)}
if _, ok := passedParams[param.Name]; ok {
// this parameter was overridden via command line
continue
}
newParams = append(newParams, param)
passedParams[param.Name] = true
}
}

for _, param := range wf.Spec.Arguments.Parameters {
if _, ok := passedParams[param.Name]; ok {
// this parameter was overridden via command line
Expand All @@ -348,6 +308,39 @@ func ApplySubmitOpts(wf *wfv1.Workflow, opts *wfv1.SubmitOpts) error {
return nil
}

func ReadParametersFile(file string, opts *wfv1.SubmitOpts) error {
var body []byte
var err error
if cmdutil.IsURL(file) {
body, err = ReadFromUrl(file)
if err != nil {
return err
}
} else {
body, err = ioutil.ReadFile(file)
if err != nil {
return err
}
}

yamlParams := map[string]json.RawMessage{}
err = yaml.Unmarshal(body, &yamlParams)
if err != nil {
return err
}

for k, v := range yamlParams {
// We get quoted strings from the yaml file.
value, err := strconv.Unquote(string(v))
if err != nil {
// the string is already clean.
value = string(v)
}
opts.Parameters = append(opts.Parameters, fmt.Sprintf("%s=%s", k, value))
}
return nil
}

// SuspendWorkflow suspends a workflow by setting spec.suspend to true. Retries conflict errors
func SuspendWorkflow(ctx context.Context, wfIf v1alpha1.WorkflowInterface, workflowName string) error {
err := waitutil.Backoff(retry.DefaultRetry, func() (bool, error) {
Expand Down
30 changes: 15 additions & 15 deletions workflow/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,21 +588,6 @@ func TestApplySubmitOpts(t *testing.T) {
assert.Equal(t, "81861780812", parameters[0].Value.String())
}
})
t.Run("ParameterFile", func(t *testing.T) {
wf := &wfv1.Workflow{}
file, err := ioutil.TempFile("", "")
assert.NoError(t, err)
defer func() { _ = os.Remove(file.Name()) }()
err = ioutil.WriteFile(file.Name(), []byte(`a: 81861780812`), 0o600)
assert.NoError(t, err)
err = ApplySubmitOpts(wf, &wfv1.SubmitOpts{ParameterFile: file.Name()})
assert.NoError(t, err)
parameters := wf.Spec.Arguments.Parameters
if assert.Len(t, parameters, 1) {
assert.Equal(t, "a", parameters[0].Name)
assert.Equal(t, "81861780812", parameters[0].Value.String())
}
})
t.Run("PodPriorityClassName", func(t *testing.T) {
wf := &wfv1.Workflow{}
err := ApplySubmitOpts(wf, &wfv1.SubmitOpts{PodPriorityClassName: "abc"})
Expand All @@ -611,6 +596,21 @@ func TestApplySubmitOpts(t *testing.T) {
})
}

func TestReadParametersFile(t *testing.T) {
file, err := ioutil.TempFile("", "")
assert.NoError(t, err)
defer func() { _ = os.Remove(file.Name()) }()
err = ioutil.WriteFile(file.Name(), []byte(`a: 81861780812`), 0o600)
assert.NoError(t, err)
opts := &wfv1.SubmitOpts{}
err = ReadParametersFile(file.Name(), opts)
assert.NoError(t, err)
parameters := opts.Parameters
if assert.Len(t, parameters, 1) {
assert.Equal(t, "a=81861780812", parameters[0])
}
}

func TestFormulateResubmitWorkflow(t *testing.T) {
t.Run("Labels", func(t *testing.T) {
wf := &wfv1.Workflow{
Expand Down