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

Update container start error so it respects MaxSlowStartDuration #169

Merged
merged 17 commits into from
Nov 27, 2023

Conversation

jennchenn
Copy link
Member

What does this PR do?

During canary, if an error occurs during pod creation within the MaxSlowStartDuration, the pod is no longer auto-paused. If MaxSlowStartDuration is not defined, the behavior is unchanged.

Motivation

Many recent deployments have been paused due to a CreateContainerConfigError - Error: failed to sync secret cache: timed out waiting for the condition. Though this error is sometimes rectified a few seconds/minutes after it is first raised, the deployment remains paused, thus slowing down releases, etc. This change is being introduced in hopes of avoiding cases where canary is paused although the issue no longer exists.

Additional Notes

Should a default value for maxSlowStartDuration be set for our clusters?

Describe your test plan

Unit tests and E2E test were included with the change. I also deployed these changes to a few staging clusters and did not see the issue of a paused canary arising in those clusters.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

@jennchenn jennchenn added this to the v0.10 milestone Oct 30, 2023
@jennchenn jennchenn force-pushed the jenn/CONTINT-3395_canary-obey-max-ss-duration branch from 9bfc4bf to c512b49 Compare October 30, 2023 19:17
github-actions[bot]

This comment was marked as duplicate.

@codecov-commenter
Copy link

codecov-commenter commented Oct 30, 2023

Codecov Report

Merging #169 (f616e2b) into main (8a74725) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
+ Coverage   63.05%   63.10%   +0.04%     
==========================================
  Files          41       41              
  Lines        3094     3098       +4     
==========================================
+ Hits         1951     1955       +4     
  Misses       1023     1023              
  Partials      120      120              
Flag Coverage Δ
unittests 63.10% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ers/extendeddaemonsetreplicaset/strategy/canary.go 92.64% <100.00%> (+0.10%) ⬆️

Continue to review full report in Codecov by Sentry.

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

@jennchenn jennchenn force-pushed the jenn/CONTINT-3395_canary-obey-max-ss-duration branch from c512b49 to 713dfa7 Compare October 30, 2023 20:04
github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

github-actions[bot]

This comment was marked as duplicate.

@jennchenn jennchenn marked this pull request as ready for review November 3, 2023 14:48
@jennchenn jennchenn requested review from a team as code owners November 3, 2023 14:48
Copy link
Contributor

@adel121 adel121 left a comment

Choose a reason for hiding this comment

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

Thanks @jennchenn , seems good.

Left 2 small questions.

github-actions[bot]

This comment was marked as duplicate.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request contains a valid label.

@jennchenn jennchenn merged commit b53e1cd into main Nov 27, 2023
20 of 24 checks passed
@jennchenn jennchenn deleted the jenn/CONTINT-3395_canary-obey-max-ss-duration branch November 27, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants