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

[3.0.1-rhel] handle corrupted images #10637

Merged

Conversation

vrothberg
Copy link
Member

While various execution paths in Podman already handle corrupted
images, podman-{create,image exists,run} did not.

Some corruptions can only be detected when accessing the individual
data. A reliable way of accessing such data is accessing its layers.
Hence, an image will only be listed to exist if a) it has been found
and b) can be inspected. If the inspection fails, the image will be
reported to not exists but without an error; the error will only be
logged. This allows for properly recovering and pull the image, even
in podman-{create,run}.

Podman will now behave as follows:

$ ./bin/podman run --rm nginx echo "it works!"
ERRO[0000] Image nginx exists in local storage but may be corrupted: layer not known
Resolved "nginx" as an alias (/home/vrothberg/.cache/containers/short-name-aliases.conf)
Trying to pull docker.io/library/nginx:latest...
Getting image source signatures
Copying blob 351ad75a6cfa skipped: already exists
Copying blob febe5bd23e98 skipped: already exists
Copying blob 30afc0b18f67 skipped: already exists
Copying blob 596b1d696923 skipped: already exists
Copying blob 8283eee92e2f skipped: already exists
Copying blob 69692152171a done
Copying config d1a364dc54 done
Writing manifest to image destination
Storing signatures
it works!

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1966872
Signed-off-by: Valentin Rothberg rothberg@redhat.com

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 10, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vrothberg

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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 10, 2021
@vrothberg
Copy link
Member Author

/hold
Still want to add a test.

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2021
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 10, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 10, 2021
@vrothberg
Copy link
Member Author

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2021
@vrothberg
Copy link
Member Author

@edsantiago PTAL at the system test.

@mheon
Copy link
Member

mheon commented Jun 10, 2021

Hold this one until we clarify if this needs a 8.4.0 zstream

@vrothberg
Copy link
Member Author

I think we're ready to go, are we?

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2021
While various execution paths in Podman already handle corrupted
images, `podman-{create,image exists,run}` did not.

Some corruptions can only be detected when accessing the individual
data.  A reliable way of accessing such data is accessing its layers.
Hence, an image will only be listed to exist if a) it has been found
and b) can be inspected.  If the inspection fails, the image will be
reported to not exists but without an error; the error will only be
logged.  This allows for properly recovering and pull the image, even
in `podman-{create,run}`.

Podman will now behave as follows:
```
$ ./bin/podman run --rm nginx echo "it works!"
ERRO[0000] Image nginx exists in local storage but may be corrupted: layer not known
Resolved "nginx" as an alias (/home/vrothberg/.cache/containers/short-name-aliases.conf)
Trying to pull docker.io/library/nginx:latest...
Getting image source signatures
Copying blob 351ad75a6cfa skipped: already exists
Copying blob febe5bd23e98 skipped: already exists
Copying blob 30afc0b18f67 skipped: already exists
Copying blob 596b1d696923 skipped: already exists
Copying blob 8283eee92e2f skipped: already exists
Copying blob 69692152171a done
Copying config d1a364dc54 done
Writing manifest to image destination
Storing signatures
it works!
```

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1966872
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
@edsantiago
Copy link
Collaborator

Thanks @vrothberg . Two things still bother me: podman pull (which is going to cause flakes), and the fact that layers.json is empty so we don't test the one-good-image, one-bad-one case. But I can't think of an easy way to fix either one right now, so we can defer that.

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2021
@edsantiago
Copy link
Collaborator

LGTM but I'm placing a hold again due to @mheon's earlier z-stream comment. Release if/when ready.

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 10, 2021
@TomSweeneyRedHat
Copy link
Member

LGTM
and I think the hold should be canceled, but I'll leave that to @mheon

@mheon
Copy link
Member

mheon commented Jun 16, 2021

/hold cancel
/lgtm

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 16, 2021
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2021
@openshift-merge-robot openshift-merge-robot merged commit 1ee48c4 into containers:v3.0.1-rhel Jun 16, 2021
@vrothberg vrothberg deleted the 3.0-bz-1966872 branch June 17, 2021 06:47
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants