argo-workflows icon indicating copy to clipboard operation
argo-workflows copied to clipboard

Group UI nodes based on `templateRef`

Open JPZ13 opened this issue 1 year ago • 10 comments

Summary

I was chatting with @mshatkhin23, and he mentioned that he'd like to have the workflow graph grouped based on the workflow template ref. Given this workflow snippet (using templates instead of refs here for simplicity):

- name: dag-tier-3
  dag:
    tasks:
      - name: task-1
        template: flakey-container
      - name: task-2
        depends: 'task-1'
        template: flakey-container
- name: dag-tier-2
  dag:
    tasks:
      - name: task-1
        template: dag-tier-3
      - name: task-2
        depends: 'task-1'
        template: dag-tier-3
- name: user-workflow
  dag:
    tasks:
      - name: task-1
        template: dag-tier-2
      - name: task-2
        depends: 'task-1'
        template: dag-tier-2

It produces the following graph: image

We'd like to find a way to group or collapse things based on a given template. In the graph above, it's hard to know which task-1 is in user-workflow or dag-tier-2 or dag-tier-3. Ideally we:

  • collapse templateRefs from the start
  • adjust the display name so that the task name also includes which template is invoking it

Use Cases

This should help eliminate confusion when looking at the Argo Workflows graph. As workflows scale and orgs manage lots of templates, there will naturally be naming collisions that arise


Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.

JPZ13 avatar May 19 '23 17:05 JPZ13

Great idea

terrytangyuan avatar May 19 '23 18:05 terrytangyuan

Thanks for posting @JPZ13 !

Small addition to adjust the display name so that the task name also includes which template is invoking it - it would also be nice to make these tasks-in-dag-template (or steps-in-steps-template) relationships visually apparent, so that the developer can clearly discern between template layers. Threw together some mock-ups below to illustrate what I mean (clearly I'm no designer 😅). Definitely still some more thought needed around questions like how to discern between parallel tasks and template-layers, but I think you get the idea.

image image image

mshatkhin23 avatar May 21 '23 16:05 mshatkhin23

@mshatkhin23 I love the idea. Also any other idea/suggestion that will allow easy rendering of big or huge workflows. If it can be based on a state (like Failed or Error) or by label/template (inner template not templateRef).

dpeer6 avatar May 22 '23 15:05 dpeer6

Hey everyone! I think that the option to group the graph is amazing! I think that grouping by template is only one option but if we go to that direction we should allow a more generic way to group.

Here's an example of a "complex" workflow that can be presented a 3 groups: (imagine how more complex it can be if we had 15 test groups, and that the app update takes 5 jobs, and that we have 4 different layers of terraform)

- name: deployment
  dag:
    tasks:
     # First update infrastructure using terraform
      - name: terraform-plan
        template: terraform-plan
        group: infra

      - name: terraform-apply
        depends: 'task-1'
        template: terraform-apply
        group: infra
     
      # Then update app backend and frontend
      - name: update-backend
        depends: 'terraform-apply'
        template: argo-update
        group: app

     - name: update-cdn
        depends: update-backend
        template: update-cdn
        group: app

     # Then run validation
     - name: backend-e2e-A
        depends: update-backend
        template: backend-e2e
        arguments:
          parameters:
          - name: test_group
            value: "A"
        group: validation

     - name: backend-e2e-B
        depends: update-backend
        template: backend-e2e
        arguments:
          parameters:
          - name: test_group
            value: "B"
        group: validation

     - name: backend-e2e-C
        depends: update-backend
        template: backend-e2e
        arguments:
          parameters:
          - name: test_group
            value: "C"
        group: validation

     - name: frontend-tests
        depends: update-frontend
        template: frontend-tests
        group: validation

or-shachar avatar May 24 '23 08:05 or-shachar

@or-shachar rly like the idea! Could we accomplish this through metadata labels/annotations you think? Although exposing as a top-level task/steps attribute does make things a bit cleaner.

We could maybe even have a group-based config field to expose style options? Although this would certainly just be a nice-to-have. For example,

groupConfig:
    - name: infra
      value: '{"color": "red", "border": "blue"}'
    - name: app
      value: ...
...

Also do you imagine group to have a meaning other than UI grouping? (nothing obvious comes to mind)

mshatkhin23 avatar May 26 '23 13:05 mshatkhin23

I think is a good idea.

In this example, this should be collapsed by default: image

I'm using empty workflows to group them, but can't collapse.

eirisdg avatar Sep 14 '23 17:09 eirisdg

Great Idea.

GandophSmida avatar May 10 '24 10:05 GandophSmida

I'd be happy to handle this one

Adrien-D avatar Jul 31 '24 09:07 Adrien-D

I think that grouping by template is only one option but if we go to that direction we should allow a more generic way to group.

It would be a good bit more complicated to add something new to the spec, especially one that is UI-facing only (i.e. not used by the Controller) vs using an existing field.

  • collapse templateRefs from the start

Collapsibility I think would be easier than grouping in general, since IIRC the graph layout is entirely algorithmic, based on a Coffman-Graham Sort and not a custom layout.

Pluggable sorting/layout algorithms could potentially make sense for #6945

agilgur5 avatar Jul 31 '24 16:07 agilgur5

After taking a look on how to handle this issue, here is where I am at :

  • That could be nice to have 3 switchable distinct layouts
    • The existing one based on children props
    • A collapsing one, which would be the existing one but collapsed by templateRef

adrien-out yaml

  • A grouping/collapsing one, based on templateRef and boundaryID which would be pretty similar to one required

adrien-out yaml (1)

The only drawback I found to handle this only working on the UI, is the missing link between step depending on an other one, represented here in red arrows on this image. We don't have this information at the moment.

adrien-out yaml (1)

If you have an insight on this matter that would be really helpful, otherwise, in my opinion it might already be a good step forward and I can start working on implementing that.

Adrien-D avatar Aug 13 '24 16:08 Adrien-D