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

[1.23] Make storage unmount less strict #6518

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Jan 11, 2023

What type of PR is this?

/kind bug

What this PR does / why we need it:

There are cases where the container storage unmount has been already (partially) done. This would cause StopContainer() in server/container_stop.go:76 fail and therefore make containers get stuck in recreation, making their pods stuck in NotReady.

We now double check the two c/storage errors ErrContainerUnknown and ErrLayerUnknown

Somehow related to: containers/podman#11207 (comment)

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

Cherry-pick of #6517
Issue: https://issues.redhat.com/browse/OCPBUGS-6585

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jan 11, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 11, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: saschagrunert

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:

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

@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 11, 2023
@codecov
Copy link

codecov bot commented Jan 11, 2023

Codecov Report

Merging #6518 (4bb320b) into release-1.23 (d12c284) will decrease coverage by 0.03%.
The diff coverage is 25.00%.

❗ Current head 4bb320b differs from pull request most recent head 260dfc0. Consider uploading reports for the commit 260dfc0 to get more accurate results

Additional details and impacted files
@@               Coverage Diff                @@
##           release-1.23    #6518      +/-   ##
================================================
- Coverage         43.02%   42.99%   -0.03%     
================================================
  Files               123      123              
  Lines             12396    12401       +5     
================================================
- Hits               5333     5332       -1     
- Misses             6556     6560       +4     
- Partials            507      509       +2     

@saschagrunert saschagrunert force-pushed the release-1.23-unmount-container-gone branch 2 times, most recently from 50f385b to bfd88c2 Compare January 19, 2023 09:51
There are cases where the container storage unmount has been already
(partially) done. This would cause `StopContainer()`  in
`server/container_stop.go:76` fail and therefore make containers get
stuck in recreation, making their pods stuck in `NotReady`.

We now double check the two c/stroage errors `ErrContainerUnknown` and
`ErrLayerUnknown`

Somehow related to:
containers/podman#11207 (comment)

Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
@saschagrunert saschagrunert force-pushed the release-1.23-unmount-container-gone branch from bfd88c2 to 260dfc0 Compare January 19, 2023 10:00
@saschagrunert saschagrunert changed the title [1.23] WIP: Make storage unmount less strict [1.23] Make storage unmount less strict Jan 23, 2023
@saschagrunert
Copy link
Member Author

@haircommander PTAL, 4.11 got verified.

@haircommander
Copy link
Member

/test e2e-agnostic
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 10, 2023
@openshift-merge-robot openshift-merge-robot merged commit 5cc2f1e into cri-o:release-1.23 Feb 10, 2023
@saschagrunert saschagrunert deleted the release-1.23-unmount-container-gone branch February 13, 2023 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants