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

docs: Example showing how to use default settings for workflow spec. Related to ##2388 #2411

Merged
merged 14 commits into from
Mar 13, 2020

Conversation

NikeNano
Copy link
Contributor

@NikeNano NikeNano commented Mar 11, 2020

This PR includes an example on how to use the default workflow specs in order to allow users set default specs for workflows. The defaults will only be used if the spec is not set.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I have written unit and/or e2e tests for my change. PRs without these are unlike to be merged.
  • Optional. I've added My organization is added to the USERS.md.
  • I've signed the CLA and required builds are green.

@NikeNano
Copy link
Contributor Author

@JohanWork would you like to check this out?

@codecov
Copy link

codecov bot commented Mar 11, 2020

Codecov Report

Merging #2411 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2411   +/-   ##
=======================================
  Coverage   13.13%   13.13%           
=======================================
  Files          72       72           
  Lines       25084    25084           
=======================================
  Hits         3294     3294           
  Misses      21375    21375           
  Partials      415      415

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e19a398...e270132. Read the comment docs.

@alexec alexec changed the title Example showing how to use default settings for workflow spec. Related to ##2388 docs: Example showing how to use default settings for workflow spec. Related to ##2388 Mar 11, 2020
Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

Looks good! Some comments.

docs/default-workflow-specs.md Outdated Show resolved Hide resolved
docs/default-workflow-specs.md Outdated Show resolved Hide resolved
passwordSecret:
name: argo-postgres-config
key: password
workflowDefaults:
Copy link
Contributor

Choose a reason for hiding this comment

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

There is an example config map in the docs folder. Could you put your example here please?

Copy link
Contributor Author

@NikeNano NikeNano Mar 12, 2020

Choose a reason for hiding this comment

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

I am not sure if I understand exactly what you want me to add so please give feedback on the changes of the example of the config map.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you example is fine, but should be in docs/workflow-controller-configmap.yaml

docs/default-workflow-specs.md Outdated Show resolved Hide resolved
docs/default-workflow-specs.md Outdated Show resolved Hide resolved
docs/default-workflow-specs.md Outdated Show resolved Hide resolved
@NikeNano
Copy link
Contributor Author

NikeNano commented Mar 12, 2020

There seams to be some issues in terms of how the value get set. I did some extra tests and the following is the updated wf:

Kind:         Workflow
Metadata:
  Creation Timestamp:             2020-03-12T06:47:10Z
  Deletion Grace Period Seconds:  0
  Deletion Timestamp:             2020-03-12T06:47:28Z
  Finalizers:
    foregroundDeletion
  Generate Name:     coinflip-
  Generation:        10
  Resource Version:  32192
  Self Link:         /apis/argoproj.io/v1alpha1/namespaces/argo/workflows/coinflip-5d4cs
  UID:               01a3e395-92d5-41dd-93db-83ccd3c7be61
Spec:
  Arguments:
  Entrypoint:  coinflip
  Templates:
    Inputs:
    Metadata:
    Name:  coinflip
    Outputs:
    Steps:
      [map[arguments:map[] name:flip-coin template:flip-coin]]
      [map[arguments:map[] name:heads template:heads when:{{steps.flip-coin.outputs.result}} == heads] map[arguments:map[] name:tails template:tails when:{{steps.flip-coin.outputs.result}} == tails]]
    Inputs:
    Metadata:
    Name:  flip-coin
    Outputs:
    Script:
      Command:
        python
      Image:  python:alpine3.6
      Name:   
      Resources:
      Source:  import random
result = "heads" if random.randint(0,1) == 0 else "tails"
print(result)

    Container:
      Args:
        echo "it was heads"
      Command:
        sh
        -c
      Image:  alpine:3.6
      Name:   
      Resources:
    Inputs:
    Metadata:
    Name:  heads
    Outputs:
    Container:
      Args:
        echo "it was tails"
      Command:
        sh
        -c
      Image:  alpine:3.6
      Name:   
      Resources:
    Inputs:
    Metadata:
    Name:  tails
    Outputs:
  Ttl Strategy:
    Seconds After Completion:  10

I don't get why there is spacing in "Ttl Strategy: " and "Seconds After Completion: 10". Is this an issue @alexec

@alexec alexec merged commit dd3029a into argoproj:master Mar 13, 2020
@alexec
Copy link
Contributor

alexec commented Mar 13, 2020

Great stuff -- thank you!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants