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

refactor: Refactor assesDAGPhase logic #3035

Merged
merged 28 commits into from Jun 9, 2020
Merged

Conversation

simster7
Copy link
Member

@simster7 simster7 commented May 14, 2020

Fixes #2975
Fixes #3096
Fixes #3104

This is a WIP: it is not ready for review.

Currently when assessing DAG pahses hasMoreRetries is used to determine if a node has more retries available. This is independent of processNodeRetries, which is the source of truth as to whether a node has more retries.

This has created issues where a DAG might think a node has more retries when it in fact does not, deadlocking the controller: #2975, #2966, #2656, #3096

This attempts to solve this issue by using processNodeRetries directly to assess a DAG phase.

workflow/controller/dag.go Outdated Show resolved Hide resolved
@simster7 simster7 changed the title fix: assesDAGPhase should use processNodeRetries refactor: Refactor assesDAGPhase logic May 29, 2020
@simster7 simster7 marked this pull request as ready for review June 1, 2020 14:31
@simster7 simster7 requested a review from jessesuen June 1, 2020 14:31
@sonarcloud
Copy link

sonarcloud bot commented Jun 4, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 2 Code Smells

87.0% 87.0% Coverage
0.0% 0.0% Duplication

@simster7 simster7 merged commit 591f649 into argoproj:master Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants