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

api: return imageID instead of imageName, for "Image" when Podman api is queried #15360

Merged

Conversation

m0duspwnens
Copy link
Contributor

@m0duspwnens m0duspwnens commented Aug 17, 2022

[NO NEW TESTS NEEDED]

This PR is intended to resolve a discrepancy for the "Image" key when the Podman API is queried.

Query example:

curl --unix-socket /run/podman/podman.sock http://localhost/v4.0.0/containers/somecontainer/json | jq

The Docker API returns the imageID for Image when querying a container as seen below:

  "Image": "sha256:somehash",

When querying the Podman API, the imageName is currently being returned:

  "Image": "ghcr.io/someimage:latest",

Once the change is implemented, the imageID is returned for the Image key.

  "Image": "sha256:somehash",

Does this PR introduce a user-facing change?

Fixed: Podman API will now return the imageID instead of imageName for the "Image" key when the API is queried.

@mheon
Copy link
Member

mheon commented Aug 17, 2022

Tests are red, but changes LGTM otherwise

@mheon
Copy link
Member

mheon commented Aug 17, 2022

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2022
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

Here's the error:

[+0078s] not ok 414 [20-containers] GET containers/8d97d1ece10812a6e793307a813ddb4d34f0d83ecd05d1af3fbc68fffcad303b/json : .Image
[+0078s] #  expected: localhost/test/testformultitag:tag
[+0078s] #    actual: sha256:5e9e9275e4d60569c72dde5d1e31cee0795df81a386bdc325873b8ed22875bc6

@TomSweeneyRedHat
Copy link
Member

And if it's useful, the 1043 test failure:

[+0163s] not ok 1043 [70-short-names] GET containers/51a68da6c2a640fd194c0b22f1eaaef6915bb60562717ad83ab1a56e6e14ae34/json : .Image
[+0163s] #  expected: quay.io/libpod/alpine:latest
[+0163s] #    actual: sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4

@m0duspwnens
Copy link
Contributor Author

m0duspwnens commented Aug 30, 2022

Do you have any guidance on the current test failures?

@jertel
Copy link
Contributor

jertel commented Aug 30, 2022

On the initial PR request, only two tests failed CI. After correcting those two tests, which were confirmed to be affected by this PR, they are now passing. However, now the sys * ubuntu-2204 root* host tests have begun failing. This PR was then updated with master, yet still they both failed, although one has different error output from the previous CI run, which makes me think they are failing for different reasons. Is there known instability with the sys * ubuntu-2204 root host tests?

@vrothberg
Copy link
Member

Do you have any guidance on the current test failures?

That's unrelated to your changes. Apologies for the inconvenience.

I suspect that the Ubuntu tests are using a broken containers/conmon.

@lsm5 @cevich can we bump conmon to v2.1.4 in the VMs? v2.1.3 is broken.

Cc: @edsantiago

@edsantiago
Copy link
Member

There is no conmon-2.1.4 for Ubuntu. As of yesterday, it's still 2.1.3

I've restarted the flakes. I think the only solution here is to file yet another ubuntu-is-broken issue, then add skip_if_ubuntu "FIXME: #12345 conmon signal flake" to the failing tests.

@lsm5
Copy link
Member

lsm5 commented Aug 31, 2022

It's building at https://build.opensuse.org/package/show/devel:kubic:libcontainers:unstable/conmon , so this should make progress in sometime.

@lsm5
Copy link
Member

lsm5 commented Aug 31, 2022

v2.1.4 should now be available on the kubic repo. And I see the tests are green, unless I'm looking in the wrong place.

@rhatdan
Copy link
Member

rhatdan commented Aug 31, 2022

/approve
LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 31, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: m0duspwnens, mheon, rhatdan

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

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

/lgtm

Thanks!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 1, 2022
@openshift-merge-robot openshift-merge-robot merged commit 72f4c77 into containers:main Sep 1, 2022
@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 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 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. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants