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

volumes in WorkflowTemplates are not carried over to Workflows #3341

Closed
3 of 4 tasks
foobarbecue opened this issue Jun 29, 2020 · 14 comments · Fixed by #3437
Closed
3 of 4 tasks

volumes in WorkflowTemplates are not carried over to Workflows #3341

foobarbecue opened this issue Jun 29, 2020 · 14 comments · Fixed by #3437
Assignees
Labels

Comments

@foobarbecue
Copy link
Contributor

foobarbecue commented Jun 29, 2020

Checklist:

  • I've included the version.
  • I've included reproduction steps.
  • I've included the workflow YAML.
  • I've included the logs.

What happened:

This Workflow suceeds, and writes a file test.txt to the host:

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: voltest-
  namespace: default
spec:
  entrypoint: test-vols
  volumes:
    - name: nac-volume
      hostPath:
        path: /c/testdir
        type: DirectoryOrCreate

  templates:
    - name: test-vols
      container:
        image: ubuntu
        volumeMounts:
          - name: nac-volume
            mountPath: /data
        command: [sh, -c]
        args: ["echo test > /data/test.txt"]

However, if you convert it to a WorkflowTemplate and submit a Workflow based on it, then it fails with
volume 'nac-volume' not found in workflow spec

Here's the WorkflowTemplate version:

apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
  name: voltesttmpl
  generateName: voltest-
  namespace: default
spec:
  entrypoint: test-vols
  volumes:
    - name: nac-volume
      hostPath:
        path: /c/testdir
        type: DirectoryOrCreate

  templates:
    - name: test-vols
      container:
        image: ubuntu
        volumeMounts:
          - name: nac-volume
            mountPath: /data
        command: [sh, -c]
        args: ["echo test > /data/test.txt"]

What you expected to happen:

I expected the volumes to be carried over to the Template from the WorkflowTemplate.

How to reproduce it (as minimally and precisely as possible):

See "what happened" above.

Environment:

  • Argo version:
$ argo version
argo: v2.9.0-rc4
  BuildDate: 2020-06-26T21:54:40Z
  GitCommit: 5b109bcb9257653ecbae46e6315c8d65842de58a
  GitTreeState: clean
  GitTag: v2.9.0-rc4
  GoVersion: go1.13.4
  Compiler: gc
  Platform: linux/amd64
  • Kubernetes version :
$ kubectl version -o yaml
clientVersion:
  buildDate: "2019-11-13T11:23:11Z"
  compiler: gc
  gitCommit: b3cbbae08ec52a7fc73d334838e18d17e8512749
  gitTreeState: clean
  gitVersion: v1.16.3
  goVersion: go1.12.12
  major: "1"
  minor: "16"
  platform: linux/amd64
serverVersion:
  buildDate: "2020-01-15T08:18:29Z"
  compiler: gc
  gitCommit: e7f962ba86f4ce7033828210ca3556393c377bcc
  gitTreeState: clean
  gitVersion: v1.16.6-beta.0
  goVersion: go1.13.5
  major: "1"
  minor: 16+
  platform: linux/amd64

Message from the maintainers:

If you are impacted by this bug please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

@alexec
Copy link
Contributor

alexec commented Jun 30, 2020

@foobarbecue do you know if this is a new problem?

@foobarbecue
Copy link
Contributor Author

foobarbecue commented Jun 30, 2020

@alexec I don't know -- this was my first time trying volumes with WorkflowTemplates. So it's probably not new. BTW I also tried https://github.com/argoproj/argo/blob/master/examples/volumes-emptydir.yaml as a WorkflowTemplate with the same result.

Probably time for me to learn to use PVCs anyway ;-)

Edit: PVCs don't work either

@foobarbecue foobarbecue changed the title volumes in WorkflowTemplates are not carried over to Templates volumes in WorkflowTemplates are not carried over to Workflows Jun 30, 2020
@foobarbecue
Copy link
Contributor Author

I'm working on this now:

  • Adding the missing properties from WorkflowSpec to WorkflowTemplateSpec in ui/src/models
  • Correcting the getWorkflow function in ui/src/app/workflow-templates/components/workflow-template-details/workflow-template-details.tsx so that it copies those properties over

Presumably there needs to be a server-side fix as well, for when you do argo submit --from workflowtemplate/mywft

I am more and more convinced that argo shot itself in the foot when it created workflowtemplates instead of adding features to templates :-p

@alexec
Copy link
Contributor

alexec commented Jul 9, 2020

@sarabala1979 would you like to own this please?

@foobarbecue
Copy link
Contributor Author

foobarbecue commented Jul 9, 2020

Yep, can do. I expect to have a PR today. Edit: Oh lol didn't read properly. @sarabala1979 can own it, for sure. I'll submit a PR in a bit though.

@foobarbecue
Copy link
Contributor Author

Oh, sorry I was working on an old fork... there is no more WorkflowTemplateSpec in ui/src/models, it just references TemplateSpec... so that's a good thing. So it's just a matter of fixing the getWorkflow function now (and then the CLI fixes).

foobarbecue added a commit to foobarbecue/argo that referenced this issue Jul 10, 2020
First attempt to address argoproj#3341 . There's still a problem which causes "error": "resourceVersion should not be set on objects to be created" when you submit a template which has a workflowTemplateRef and a volume.

(cherry picked from commit 40735ff780787683abaeecc23d1846ac43002cd4)
foobarbecue added a commit to foobarbecue/argo that referenced this issue Jul 10, 2020
First attempt to address argoproj#3341 . There's still a problem which causes "error": "resourceVersion should not be set on objects to be created" when you submit a template which has a workflowTemplateRef and a volume.
@foobarbecue
Copy link
Contributor Author

Draft PR: #3441

@alexec alexec linked a pull request Jul 10, 2020 that will close this issue
6 tasks
@alexec
Copy link
Contributor

alexec commented Jul 10, 2020

I this #3436 fixes this too.

@alexec alexec self-assigned this Jul 10, 2020
@alexec alexec linked a pull request Jul 10, 2020 that will close this issue
6 tasks
@foobarbecue
Copy link
Contributor Author

Ah, cool, I'll try out #3437 and close this if it works

@foobarbecue
Copy link
Contributor Author

foobarbecue commented Jul 11, 2020

I just tried it, at 81cba83 and get volume 'nac-volume' not found in workflow spec . I'm just gonna wait a while since it seems workflowtemplates are still being enhanced to support all the fields that workflows support.

@alexec
Copy link
Contributor

alexec commented Jul 11, 2020

@foobarbecue - this needs two fixes. One to remove the panic that obscures the error (this was a pre-existing bug) an another to fix the actual bug #3441.

@foobarbecue
Copy link
Contributor Author

foobarbecue commented Jul 11, 2020

Ah, ok. When we make a WF from a WFT, and the WF is referring to the WFT using workflowTemplateRef, do we want the volumes and stuff to be looked up on the WFT through the reference? Or do we want to copy them to the WF? My #3441 copies them to the WF, which I'm now thinking is the wrong way of doing it.

I guess I'm going to have to study how workflowTemplateRefs are actually implemented.

@alexec
Copy link
Contributor

alexec commented Jul 14, 2020

Fixed in v2.9

@alexec alexec closed this as completed Jul 14, 2020
@foobarbecue
Copy link
Contributor Author

Just to confirm, I'm using this now and it works well!

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