+
Skip to content

Fix context cancellation when image pull progress timeout is 0 #8998

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 12, 2025
Merged

Fix context cancellation when image pull progress timeout is 0 #8998

merged 1 commit into from
Feb 12, 2025

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Feb 12, 2025 •
edited
Loading

What type of PR is this?

/kind failing-test
/kind bug

What this PR does / why we need it:

Debugging the test since it seems to flake heavily.

We should never cancel the context when the pull timeout is 0, means we now add an additional check to prevent this corner case.

Deflakes the integration tests and also fixes possible issues around a disabled pull progress timeout.

Which issue(s) this PR fixes:

Fixes #8872

Special notes for your reviewer:

None

Does this PR introduce a user-facing change?

Fixed context cancellation when image pull progress timeout is `0` (`--pull-progress-timeout=0` / `pull_progress_timeout=0`)

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Feb 12, 2025
@openshift-ci openshift-ci bot requested review from hasan4791 and QiWang19 February 12, 2025 13:30
@openshift-ci openshift-ci bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 12, 2025
@saschagrunert saschagrunert force-pushed the image-test branch 2 times, most recently from c60e109 to 7f832a5 Compare February 12, 2025 13:50
@saschagrunert
Fix context cancellation when image pull progress timeout is 0
2efa5b3 
We should never cancel the context when the pull timeout is `0`, means
we now add an additional check to prevent this corner case.

Deflakes the integration tests and also fixes possible issues around a
disabled pull progress timeout.

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@saschagrunert saschagrunert changed the title WIP: Debug image timeout integration test WIP: Fix context cancellation when image pull progress timeout is 0 Feb 12, 2025
@openshift-ci openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Feb 12, 2025
@saschagrunert saschagrunert mentioned this pull request Feb 12, 2025
Closed
@saschagrunert saschagrunert self-assigned this Feb 12, 2025
@saschagrunert saschagrunert changed the title WIP: Fix context cancellation when image pull progress timeout is 0 Fix context cancellation when image pull progress timeout is 0 Feb 12, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 12, 2025
@saschagrunert saschagrunert added kind/bug Categorizes issue or PR as related to a bug. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. and removed kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. labels Feb 12, 2025
@codecov Codecov
Copy link

codecov bot commented Feb 12, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 47.49%. Comparing base (403bef8) to head (2efa5b3).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8998      +/-   ##
==========================================
- Coverage   47.49%   47.49%   -0.01%     
==========================================
  Files         154      154              
  Lines       23125    23126       +1     
==========================================
  Hits        10983    10983              
- Misses      11072    11073       +1     
  Partials     1070     1070

@saschagrunert
Copy link
Member Author

/retest

@saschagrunert
Copy link
Member Author

@cri-o/cri-o-maintainers PTAL

sohankunkerkar
sohankunkerkar approved these changes Feb 12, 2025
View reviewed changes
Copy link
Member

@sohankunkerkar sohankunkerkar left a comment

Choose a reason for hiding this comment

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

/lgtm
Nice!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 12, 2025
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Feb 12, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: saschagrunert, sohankunkerkar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [saschagrunert,sohankunkerkar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sohankunkerkar sohankunkerkar removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 12, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit bbf6daa into cri-o:main Feb 12, 2025
76 of 78 checks passed
@saschagrunert saschagrunert deleted the image-test branch February 12, 2025 17:37
@saschagrunert
Copy link
Member Author

/cherry-pick release-1.32
/cherry-pick release-1.31

@openshift-cherrypick-robot
Copy link

@saschagrunert: new pull request created: #9009

In response to this:

/cherry-pick release-1.32
/cherry-pick release-1.31

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Feb 17, 2025
Merged
@openshift-cherrypick-robot
Copy link

@saschagrunert: new pull request created: #9010

In response to this:

/cherry-pick release-1.32
/cherry-pick release-1.31

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Feb 17, 2025
Merged
@saschagrunert saschagrunert mentioned this pull request Feb 18, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sohankunkerkar sohankunkerkar sohankunkerkar approved these changes

@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner

@hasan4791 hasan4791 Awaiting requested review from hasan4791

@QiWang19 QiWang19 Awaiting requested review from QiWang19

Assignees

@saschagrunert saschagrunert

@sohankunkerkar sohankunkerkar

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pull progress timeout should not timeout when set to 0 is flaky
3 participants
@saschagrunert @openshift-cherrypick-robot @sohankunkerkar
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载