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: Fix workflow level timeouts #1369

Merged
merged 4 commits into from
Jan 17, 2020

Conversation

zhujl1991
Copy link
Contributor

issue: #848

mimic 133a23c . Set the pod ttl to template or wf ttl whichever is smaller.

@CLAassistant
Copy link

CLAassistant commented May 15, 2019

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Jialu Zhu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@dtaniwaki dtaniwaki self-requested a review June 27, 2019 05:58
@sarabala1979
Copy link
Member

#1357

@jessesuen
Copy link
Member

I agree with @dtaniwaki comments. @zhujl1991 - If those are addressed we can merge it in.

@Ark-kun
Copy link
Member

Ark-kun commented Sep 25, 2019

Is it possible that the same issue breaks termination when retry is used? #1620

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I please request the following?

  1. Please sync with master and make sure all checks are green.
  2. Address any review comments.

@zhujl1991 zhujl1991 force-pushed the fix_resource_timeout branch from 23356d7 to 7298afd Compare January 16, 2020 19:41
@zhujl1991 zhujl1991 changed the title [RFC] Fix workflow level timeouts Fix workflow level timeouts Jan 16, 2020
@zhujl1991 zhujl1991 changed the title Fix workflow level timeouts [Bug Fixes] Fix workflow level timeouts Jan 16, 2020
@zhujl1991 zhujl1991 changed the title [Bug Fixes] Fix workflow level timeouts fix: Fix workflow level timeouts Jan 16, 2020
@codecov
Copy link

codecov bot commented Jan 16, 2020

Codecov Report

Merging #1369 into master will increase coverage by <.01%.
The diff coverage is 38.46%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1369      +/-   ##
=========================================
+ Coverage    8.87%   8.88%   +<.01%     
=========================================
  Files          53      53              
  Lines       33650   33662      +12     
=========================================
+ Hits         2986    2990       +4     
- Misses      30273   30280       +7     
- Partials      391     392       +1
Impacted Files Coverage Δ
workflow/controller/workflowpod.go 72.01% <38.46%> (-0.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1965c9...0097b7b. Read the comment docs.

Gopkg.lock Outdated Show resolved Hide resolved
@zhujl1991 zhujl1991 requested a review from alexec January 17, 2020 04:12
@zhujl1991
Copy link
Contributor Author

@alexec Is the e2e test flaky? https://github.com/argoproj/argo/pull/1369/commits we can see from here my commit 6f1e00f passed the e2e test while my latest one failed. But the only diff between the two commits is the revert of Gopkg.lock file.

@zhujl1991
Copy link
Contributor Author

Hmm... Looks like the e2e test passed somehow. It was failed just now.

@alexec alexec merged commit 5eb15bb into argoproj:master Jan 17, 2020
@alexec
Copy link
Contributor

alexec commented Jan 17, 2020

e2e tests are not 100% stable yet

@zhujl1991 zhujl1991 deleted the fix_resource_timeout branch January 17, 2020 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants