manifest list inspect single image #8100

QiWang19 · 2020-10-21T16:31:51Z

If the image name not a manifest list type, enable manifest inspect to return manifest of single image with manifest type application/vnd.docker.distribution.manifest.v2+json
close #8023

Signed-off-by: Qi Wang [email protected]

TomSweeneyRedHat · 2020-10-21T18:51:37Z

@QiWang19 can you touch up the title please? "insepct" -> "inspect"

pkg/domain/infra/abi/manifest.go

TomSweeneyRedHat · 2020-10-21T19:26:18Z

I'd like to hear @edsantiago 's thought on the friendliness. Personally I don't think a query that only happens sometimes is not friendly at all, I think it's better to get the info with a warning.

edsantiago · 2020-10-21T19:33:42Z

I'm trying to focus every minute I have today on Chris's big PR, and can't give this the attention it deserves. (And I'm out tomorrow and Friday). Sorry. My first reaction, though, is that I really dislike being prompted. I like error messages that indicate what went wrong and how to fix it (e.g. by a special command-line flag), and I like command-line flags that are forgiving or at least offer a best-effort attempt to Do The Right Thing so that scripts can be written robustly. HTH.

rhatdan · 2020-10-21T19:35:54Z

I agree on avoiding the query, unless you are going to add a -y option so I can script it.

I also think this is not a mainstream command, so having it fail or print a warning would be better. If someone is playing with manifests they will quickly learn how to deal with them. Most users will never touch this code.

QiWang19 · 2020-10-21T20:19:51Z

If having it fail is expected, #8023 may not be treated as a podman bug. The code used to report the manifest type error. But just tried docker manifest inspect, it returns the single manifest directly without any errors.

jwhonce · 2020-10-22T20:56:39Z

pkg/domain/infra/abi/manifest.go

+
+	switch manType {
+	case manifest.DockerV2Schema2MediaType:
+		fmt.Printf("Warning! The manifest type %s is not a manifest list.  Displaying the the manifest of the single image.\n", manType)


Suggested change

fmt.Printf("Warning! The manifest type %s is not a manifest list. Displaying the the manifest of the single image.\n", manType)

fmt.Fprintf(os.Stderr, "Warning! The manifest type %s is not a manifest list. Displaying the the manifest of the single image.\n", manType)

I know this has been bashed around a bit but errors should go to stderr IMHO

Thanks. Fixed.

rhatdan · 2020-10-24T09:53:13Z

pkg/domain/infra/abi/manifest.go

+
+	switch manType {
+	case manifest.DockerV2Schema2MediaType:
+		fmt.Fprintf(os.Stderr, "Warning! The manifest type %s is not a manifest list.  Displaying the the manifest of the single image.\n", manType)


Once we start displaying at the Warning level, this should be logrus.Warnf.

rhatdan · 2020-10-24T09:54:50Z

Could you cleanup you commit message to match latest reality and fix spelling mistakes.

 manifest list insepct single image

Use prompt to enable `manifest inspect` to return manifest of single image if it's not a manfiest list type.

rhatdan · 2020-10-26T21:24:24Z

/approve
LGTM
@jwhonce PTAL

openshift-ci-robot · 2020-10-26T21:24:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: QiWang19, 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:

~~OWNERS~~ [rhatdan]

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

QiWang19 · 2020-10-29T19:18:56Z

@containers/podman-maintainers PTAL.

vrothberg

Just a wording nit. @QiWang19 could you add some tests? I always want a test for fixes to make sure we're not re-regressing in the future.

vrothberg · 2020-10-30T08:59:18Z

pkg/domain/infra/abi/manifest.go

+
+	switch manType {
+	case manifest.DockerV2Schema2MediaType:
+		logrus.Warnf("Warning! The manifest type %s is not a manifest list.  Displaying the the manifest of the single image.\n", manType)


Suggested change

logrus.Warnf("Warning! The manifest type %s is not a manifest list. Displaying the the manifest of the single image.\n", manType)

logrus.Warnf("Warning! The manifest type %s is not a manifest list but a single image.\n", manType)

vrothberg

One tiny non-blocking format suggestion but LGTM

vrothberg · 2020-10-30T15:08:15Z

pkg/domain/infra/abi/manifest.go

+
+	switch manType {
+	case manifest.DockerV2Schema2MediaType:
+		logrus.Warnf("Warning! The manifest type %s is not a manifest list but a single image.\n", manType)


Suggested change

logrus.Warnf("Warning! The manifest type %s is not a manifest list but a single image.\n", manType)

logrus.Warnf("Warning! The manifest type %s is not a manifest list but a single image", manType)

If the image name not a manifest list type, enable manifest inspect to return manifest of single image manifest type vnd.docker.distribution.manifest.v2+json. Signed-off-by: Qi Wang <[email protected]>

QiWang19 · 2020-10-30T19:12:48Z

@containers/podman-maintainers PTAL

TomSweeneyRedHat · 2020-10-30T21:14:17Z

LGTM
and happy green test buttons. Nice work @QiWang19

jwhonce · 2020-10-30T23:17:10Z

/lgtm

QiWang19 linked an issue Oct 21, 2020 that may be closed by this pull request

unable to load manifest list: unsupported format #8023

Closed

QiWang19 changed the title ~~manifest list insepct single image~~ manifest list inspect single image Oct 21, 2020

TomSweeneyRedHat reviewed Oct 21, 2020

View reviewed changes

pkg/domain/infra/abi/manifest.go Show resolved Hide resolved

QiWang19 force-pushed the mirror-manifest branch from 38480f3 to f924476 Compare October 21, 2020 20:04

QiWang19 force-pushed the mirror-manifest branch from f924476 to b1d671f Compare October 22, 2020 20:06

jwhonce reviewed Oct 22, 2020

View reviewed changes

QiWang19 force-pushed the mirror-manifest branch from b1d671f to 2494324 Compare October 22, 2020 22:44

rhatdan reviewed Oct 24, 2020

View reviewed changes

QiWang19 force-pushed the mirror-manifest branch 3 times, most recently from ae2b842 to 71ceaeb Compare October 26, 2020 16:58

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2020

vrothberg reviewed Oct 30, 2020

View reviewed changes

QiWang19 force-pushed the mirror-manifest branch from 71ceaeb to 71b0e0e Compare October 30, 2020 14:13

vrothberg reviewed Oct 30, 2020

View reviewed changes

manifest list inspect single image

57650aa

If the image name not a manifest list type, enable manifest inspect to return manifest of single image manifest type vnd.docker.distribution.manifest.v2+json. Signed-off-by: Qi Wang <[email protected]>

QiWang19 force-pushed the mirror-manifest branch from 71b0e0e to 57650aa Compare October 30, 2020 15:14

openshift-ci-robot assigned jwhonce Oct 30, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2020

openshift-merge-robot merged commit b6ab2df into containers:master Oct 30, 2020

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 24, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

manifest list inspect single image #8100

manifest list inspect single image #8100

QiWang19 commented Oct 21, 2020 •

edited

Loading

TomSweeneyRedHat commented Oct 21, 2020

TomSweeneyRedHat commented Oct 21, 2020

edsantiago commented Oct 21, 2020

rhatdan commented Oct 21, 2020

QiWang19 commented Oct 21, 2020

jwhonce Oct 22, 2020

QiWang19 Oct 22, 2020

rhatdan Oct 24, 2020

rhatdan commented Oct 24, 2020

rhatdan commented Oct 26, 2020

openshift-ci-robot commented Oct 26, 2020

QiWang19 commented Oct 29, 2020

vrothberg left a comment

vrothberg Oct 30, 2020

vrothberg left a comment

vrothberg Oct 30, 2020

QiWang19 commented Oct 30, 2020

TomSweeneyRedHat commented Oct 30, 2020

jwhonce commented Oct 30, 2020

	fmt.Printf("Warning! The manifest type %s is not a manifest list. Displaying the the manifest of the single image.\n", manType)
	fmt.Fprintf(os.Stderr, "Warning! The manifest type %s is not a manifest list. Displaying the the manifest of the single image.\n", manType)

	logrus.Warnf("Warning! The manifest type %s is not a manifest list. Displaying the the manifest of the single image.\n", manType)
	logrus.Warnf("Warning! The manifest type %s is not a manifest list but a single image.\n", manType)

manifest list inspect single image #8100

manifest list inspect single image #8100

Conversation

QiWang19 commented Oct 21, 2020 • edited Loading

TomSweeneyRedHat commented Oct 21, 2020

TomSweeneyRedHat commented Oct 21, 2020

edsantiago commented Oct 21, 2020

rhatdan commented Oct 21, 2020

QiWang19 commented Oct 21, 2020

jwhonce Oct 22, 2020

Choose a reason for hiding this comment

QiWang19 Oct 22, 2020

Choose a reason for hiding this comment

rhatdan Oct 24, 2020

Choose a reason for hiding this comment

rhatdan commented Oct 24, 2020

rhatdan commented Oct 26, 2020

openshift-ci-robot commented Oct 26, 2020

QiWang19 commented Oct 29, 2020

vrothberg left a comment

Choose a reason for hiding this comment

vrothberg Oct 30, 2020

Choose a reason for hiding this comment

vrothberg left a comment

Choose a reason for hiding this comment

vrothberg Oct 30, 2020

Choose a reason for hiding this comment

QiWang19 commented Oct 30, 2020

TomSweeneyRedHat commented Oct 30, 2020

jwhonce commented Oct 30, 2020

QiWang19 commented Oct 21, 2020 •

edited

Loading