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

[frontend] incorrect DAG with argo v3.1.0 #5944

Closed
Bobgy opened this issue Jun 29, 2021 · 6 comments · Fixed by #5971
Closed

[frontend] incorrect DAG with argo v3.1.0 #5944

Bobgy opened this issue Jun 29, 2021 · 6 comments · Fixed by #5971

Comments

@Bobgy
Copy link
Contributor

Bobgy commented Jun 29, 2021

Environment

  • How did you deploy Kubeflow Pipelines (KFP)?

kfp standalone

  • KFP version: 1.7.0-alpha.1

Steps to reproduce

  1. run v2 sample test
  2. note that, two steps are running at the same time, but there's a dependency edge between them:
    image

Expected result

the two build image steps should have no dependency to each other

Materials and Reference


Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

@Bobgy
Copy link
Contributor Author

Bobgy commented Jun 29, 2021

/assign @zijianjoy

@zijianjoy
Copy link
Collaborator

zijianjoy commented Jun 29, 2021

A quick walkthrough of the workflow changes in https://github.com/argoproj/argo-workflows/blame/v3.1.0/ui/src/models/workflows.ts:

There are 3 areas which have changed since 2.12.9:

  1. ContainerNode interface: https://github.com/argoproj/argo-workflows/blame/v3.1.0/ui/src/models/workflows.ts#L296
  2. interface Template added fields
containerSet?: {
        containers: ContainerNode[];
    };
  1. NodeType enum add new type Container.

Reference PRs:
argoproj/argo-workflows#5099
argoproj/argo-workflows#5348

@zijianjoy
Copy link
Collaborator

See a sample runtime node info about build_kfp_launcher:

"v2-sample-test-lmx4x-2942687590": {
        "id": "v2-sample-test-lmx4x-2942687590",
        "name": "v2-sample-test-lmx4x.kaniko",
        "displayName": "kaniko",
        "type": "Pod",
        "templateName": "kaniko",
        "templateScope": "local/v2-sample-test-lmx4x",
        "phase": "Running",
        "boundaryID": "v2-sample-test-lmx4x",
        "startedAt": "2021-06-29T18:41:14Z",
        "finishedAt": null,
        "progress": "0/1",
        "inputs": {
          "parameters": [
            {
              "name": "launcher_destination",
              "value": "gcr.io/jamxl-kfp-dev/kfp-launcher/kfp-launcher"
            }
          ],
          "artifacts": [
            {
              "name": "download-folder-from-tarball-on-gcs-Folder",
              "path": "/tmp/inputs/context_artifact/data",
              "s3": {
                "key": "artifacts/v2-sample-test-lmx4x/2021/06/29/v2-sample-test-lmx4x-2365399356/download-folder-from-tarball-on-gcs-Folder.tgz"
              }
            }
          ]
        },
        "children": [
          "v2-sample-test-lmx4x-1625946649"
        ],
        "hostNodeName": "gke-kfp-v2-compatible-default-pool-1d50a9c4-3t50"
      }
    }

Note that children has v2-sample-test-lmx4x-1625946649, which is build_samples_image. This caused the frontend thinks that there are dependencies between two steps.

@Bobgy
Copy link
Contributor Author

Bobgy commented Jun 30, 2021

I think containerSet and the first change you mentioned are related to argo's new introduction of container DAG in one single Pod. KFP does not plan to use them, so we can safely ignore the changes.

@Bobgy
Copy link
Contributor Author

Bobgy commented Jun 30, 2021

Note that children has v2-sample-test-lmx4x-1625946649, which is build_samples_image. This caused the frontend thinks that there are dependencies between two steps.

this is strange, can it be a bug in argo?
or did our workflow spec need any changes upgrading to argo v3.1?

@zijianjoy
Copy link
Collaborator

Validated offline that there is a misrepresentation of diamond workflow in argo v3.1. Created argoproj/argo-workflows#6267 for reporting and tracking this bug.

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

Successfully merging a pull request may close this issue.

2 participants