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

Allow overriding workflow labels in 'argo submit' #1475

Merged
merged 3 commits into from
Aug 2, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions cmd/argo/commands/submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func NewSubmitCommand() *cobra.Command {
command.Flags().BoolVar(&cliSubmitOpts.strict, "strict", true, "perform strict workflow validation")
command.Flags().Int32Var(&priority, "priority", 0, "workflow priority")
command.Flags().StringVarP(&submitOpts.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.")
// Only complete files with appropriate extension.
err := command.Flags().SetAnnotation("parameter-file", cobra.BashCompFilenameExt, []string{"json", "yaml", "yml"})
if err != nil {
Expand Down
24 changes: 24 additions & 0 deletions util/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,27 @@ func SetLogLevel(level string) {
log.Fatalf("Unknown level: %s", level)
}
}

// ParseLabels turns a string representation of a label set into a map[string]string
func ParseLabels(labelSpec interface{}) (map[string]string, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you write tests for this method?

labelString, isString := labelSpec.(string)
if !isString {
return nil, fmt.Errorf("expected string, found %v", labelSpec)
}
if len(labelString) == 0 {
return nil, fmt.Errorf("no label spec passed")
}
labels := map[string]string{}
labelSpecs := strings.Split(labelString, ",")
for ix := range labelSpecs {
labelSpec := strings.Split(labelSpecs[ix], "=")
if len(labelSpec) != 2 {
return nil, fmt.Errorf("unexpected label spec: %s", labelSpecs[ix])
}
if len(labelSpec[0]) == 0 {
return nil, fmt.Errorf("unexpected empty label key")
}
labels[labelSpec[0]] = labelSpec[1]
}
return labels, nil
}
14 changes: 9 additions & 5 deletions workflow/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ type SubmitOpts struct {
Parameters []string // --parameter
ParameterFile string // --parameter-file
ServiceAccount string // --serviceaccount
Labels string // --labels
OwnerReference *metav1.OwnerReference // useful if your custom controller creates argo workflow resources
}

Expand All @@ -155,14 +156,17 @@ func SubmitWorkflow(wfIf v1alpha1.WorkflowInterface, wf *wfv1.Workflow, opts *Su
if opts.ServiceAccount != "" {
wf.Spec.ServiceAccountName = opts.ServiceAccount
}
labels := wf.GetLabels()
if opts.Labels != "" {
labels, _ = cmdutil.ParseLabels(opts.Labels)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we silently ignore an error from this method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Labels from a workflow are completely ignored if labels are set by the command line flag. I think we should merge the labels.

}
if labels == nil {
labels = make(map[string]string)
}
if opts.InstanceID != "" {
labels := wf.GetLabels()
if labels == nil {
labels = make(map[string]string)
}
labels[common.LabelKeyControllerInstanceID] = opts.InstanceID
wf.SetLabels(labels)
}
wf.SetLabels(labels)
if len(opts.Parameters) > 0 || opts.ParameterFile != "" {
newParams := make([]wfv1.Parameter, 0)
passedParams := make(map[string]bool)
Expand Down