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

fix: constraint containerType outboundnode boundary. Fixes #12997 #13048

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

tczhao
Copy link
Member

@tczhao tczhao commented May 14, 2024

Fixes #12997

Motivation

Root cause:
say we have dag

task: A(containerset)
task: B(containerset), depends: A
task: C(containerset), depends: A

if task B is processed and started before the controller can process C,
C will find the deepest node in A without considering boundaries and mistakenly attach to B

Modifications

if a node(containerType) child node do not have the same boundyID, we return the current node as the deepest outbound node.

Verification

Tested:
Before code change

  • ran the workflow from the issue ticket, failed: incorrect dependency and controller panic
  • ran new unit test without change, failed as expected

After code change

  • ran the workflow from the issue ticket, succeeded with the correct dependencies
  • ran unit test with the code change, succeed

@tczhao tczhao changed the title DRAFT fix: constraint containerType outboundnode boundary fix: constraint containerType outboundnode boundary May 14, 2024
@tczhao tczhao changed the title fix: constraint containerType outboundnode boundary fix: constraint containerType outboundnode boundary. Fixes #12997 May 14, 2024
@tczhao tczhao added area/controller Controller issues, panics area/templates/dag labels May 14, 2024
@tczhao tczhao marked this pull request as ready for review May 14, 2024 03:15
Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
@tczhao tczhao requested a review from Joibel June 11, 2024 14:07
@agilgur5 agilgur5 merged commit a154a93 into argoproj:main Jul 19, 2024
28 checks passed
@agilgur5
Copy link

Merging as this has been approved and has no conflicts & passes tests.

@agilgur5 agilgur5 added this to the v3.5.x patches milestone Jul 30, 2024
agilgur5 pushed a commit that referenced this pull request Jul 30, 2024
…3048)

Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
(cherry picked from commit a154a93)
Joibel pushed a commit to pipekit/argo-workflows that referenced this pull request Sep 19, 2024
Joibel pushed a commit that referenced this pull request Sep 20, 2024
…3048)

Signed-off-by: Tianchu Zhao <evantczhao@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complex DAG of container set nodes always failed
3 participants