Make libpod return error status code on failure to pull image by simonbrauner · Pull Request #28109 · containers/podman

simonbrauner · 2026-02-17T16:45:50Z

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
the sign-off email address. See CONTRIBUTING.md
for more information.
Referenced issues using Fixes: #00000 in commit message (if applicable)
Tests have been added/updated (or no tests are needed)
Documentation has been updated (or no documentation changes are needed)
All commits pass make validatepr (format/lint checks)
Release note entered in the section below (or None if no user-facing changes)

Does this PR introduce a user-facing change?

The API endpoint `/libpod/images/pull` returns an error status code on failure to pull image instead of always returning 200.

packit-as-a-service · 2026-02-17T16:49:19Z

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

mtrmac

I didn’t carefully re-read the full function, but at a glance, this looks pretty good.

(As discussed, we should probably start streaming after some timeout.)

pkg/api/handlers/libpod/images_pull.go

pkg/api/handlers/utils/images.go

Honny1

My only comment is about the specific section that I believe is causing the CI to fail. This could be solved by either adapting the client to accept multiple reports in a single response or by just sending one report per response (less breaking change). Also, it should be documented that the response contains multiple JSON objects.

pkg/api/handlers/libpod/images_pull.go

simonbrauner · 2026-02-23T13:33:01Z

My only comment is about the specific section that I believe is causing the CI to fail. This could be solved by either adapting the client to accept multiple reports in a single response or by just sending one report per response (less breaking change). Also, it should be documented that the response contains multiple JSON objects.

It should be fixed now

Honny1

LGTM

mtrmac

ACK overall, just a bunch of comments to simplify future maintenance.

But I’d like the formal LGTM to be from someone familiar with the swagger documentation / tooling.

test/apiv2/python/rest_api/test_v2_0_0_image.py

pkg/api/handlers/swagger/responses.go

mtrmac · 2026-02-25T20:24:01Z

pkg/api/handlers/libpod/images_pull.go

 		images, err := runtime.LibimageRuntime().Pull(r.Context(), query.Reference, pullPolicy, pullOptions)
-		var report entities.ImagePullReport
 		if err != nil {
-			report.Error = err.Error()


I suppose this is technically an API break. API experts, is that OK?

With 6.0, yes, I am fine with this sort of thing.

pkg/api/handlers/libpod/images_pull.go

mtrmac · 2026-02-25T20:43:45Z

pkg/api/handlers/libpod/images_pull.go

-			flush()
 		case <-runCtx.Done():
+			if !streamingStarted && pullError != nil {
+				utils.Error(w, utils.HTTPStatusFromRegistryError(pullError), pullError)


I suppose this is technically an API break. API experts, is that OK?

that is API break, marking it with label 6.0 should be it to track it, just double triple checking @Luap99 @mheon ?

I think that should be fine. AFAICT the podman-remote client should be able to handle that error at least and generally speaking all clients should handle any http status error already as well as it could have failed for other reasons so I would consider this a bug fix

simonbrauner · 2026-03-03T09:15:58Z

@mtrmac I addressed your comments, except for those about API break

mtrmac

ACK, still

I’d like the formal LGTM to be from someone familiar with the swagger documentation / tooling.

pkg/api/handlers/libpod/images_pull.go

test/apiv2/python/rest_api/test_v2_0_0_image.py

Podman part of the functionality implemented in containers/podman#28109 Fixes: containers#301 Signed-off-by: Šimon Brauner <sbrauner@redhat.com>

Honny1 · 2026-03-04T12:29:46Z

pkg/api/server/register_images.go

 	//  - images
 	// summary: Pull images
-	// description: Pull one or more images from a container registry.
+	// description: Pull one or more images from a container registry. Error status codes can come either from the API or from the registry.


I would also inform users that if an error occurs mid-pull, the HTTP status code will be 200, and the error will be detailed in the report's error field.

It is in the description of the response now, do you think it would be better to move it to the main description?

I would put that in both places, or just not mention the error codes in the description at all, since it currently only covers one potential error path.

I think it's obvious from the error codes that 401, 403, and 404 are from the repository, not errors caused by the Podman pulling logic. Maybe this could be stated more clearly in the description of each error code.

I think it's obvious from the error codes that 401, 403, and 404 are from the repository

The difficulty here is that pull registry.example/nonexistentrepo reports 404 in some registries, and 401 in others (under the theory that there might be a private repo that the user is not allowed to see, and returning 404 would disclose that information — so always returning 401 is more secure).

I think instead of listing every possible error code, we could use a wildcard to indicate that any other error codes returned by the repository will be propagated to the client.

For example:

responses: '200': description: OK '400': description: Bad Request (explicitly handled) default: description: Unexpected Error (This propagates any other repository errors) content: application/json: schema: $ref: '#/components/schemas/ErrorModel'

I think, ideally, we would add registry-specific heuristics (where possible, sometimes registries intentionally hide information) and allow ~reliable detection of “repository is missing” and other error kinds. In that sense, promising to propagate raw registry status is not ideal either.

Pragmatically, I’m not sure how much that matters, because the HTTP statuses (and even more registry error codes) are simply unreliable across the registry ecosystem; so users need to rely on integration tests anyway, regardless of what we document.

There is the version with default now, is that fine? or would it better to switch back to the explicit 401/403/404?

test/apiv2/python/rest_api/test_v2_0_0_image.py

Podman part of the functionality implemented in containers/podman#28109 Fixes: containers#301 Signed-off-by: Šimon Brauner <sbrauner@redhat.com>

Honny1

LGTM

Honny1 · 2026-03-09T13:25:17Z

LGTM,

PTAL @containers/podman-maintainers final review and merge.

Fixes: containers#22105 Signed-off-by: Šimon Brauner <sbrauner@redhat.com>

mheon · 2026-03-09T14:08:56Z

LGTM

github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Feb 17, 2026

simonbrauner force-pushed the issue-22105 branch from 2e2ab04 to 8a0af91 Compare February 17, 2026 16:48

simonbrauner force-pushed the issue-22105 branch from 8a0af91 to b6b8e68 Compare February 17, 2026 16:52

mtrmac reviewed Feb 17, 2026

View reviewed changes

simonbrauner force-pushed the issue-22105 branch 9 times, most recently from e56c699 to cb866ac Compare February 18, 2026 14:53

simonbrauner marked this pull request as ready for review February 18, 2026 15:01

inknos added the 6.0 Breaking changes for Podman 6.0 label Feb 18, 2026

Honny1 requested changes Feb 19, 2026

View reviewed changes

pkg/api/handlers/libpod/images_pull.go Outdated Show resolved Hide resolved

simonbrauner force-pushed the issue-22105 branch 9 times, most recently from bd69b6b to 44cdac1 Compare February 23, 2026 12:32

simonbrauner requested a review from Honny1 February 23, 2026 13:13

Honny1 approved these changes Feb 23, 2026

View reviewed changes

mtrmac reviewed Feb 25, 2026

View reviewed changes

inknos mentioned this pull request Feb 26, 2026

Ensure that APIError is thrown on failure to pull image containers/podman-py#631

Open

simonbrauner force-pushed the issue-22105 branch 9 times, most recently from 587451e to 6abee77 Compare March 3, 2026 09:12

mtrmac reviewed Mar 3, 2026

View reviewed changes

pkg/api/handlers/libpod/images_pull.go Outdated Show resolved Hide resolved

test/apiv2/python/rest_api/test_v2_0_0_image.py Outdated Show resolved Hide resolved

simonbrauner force-pushed the issue-22105 branch from 6abee77 to e22fbbb Compare March 4, 2026 07:03

Honny1 reviewed Mar 4, 2026

View reviewed changes

simonbrauner force-pushed the issue-22105 branch from e22fbbb to 175a0b7 Compare March 5, 2026 11:15

simonbrauner force-pushed the issue-22105 branch from 175a0b7 to 7ae3430 Compare March 5, 2026 11:53

Honny1 approved these changes Mar 5, 2026

View reviewed changes

simonbrauner force-pushed the issue-22105 branch from 7ae3430 to 26548af Compare March 9, 2026 13:08

Make libpod return error status code on failure to pull image

76095db

Fixes: containers#22105 Signed-off-by: Šimon Brauner <sbrauner@redhat.com>

simonbrauner force-pushed the issue-22105 branch from 26548af to 76095db Compare March 9, 2026 13:26

mheon merged commit 6b10bea into containers:main Mar 9, 2026
78 of 82 checks passed

Conversation

simonbrauner commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Does this PR introduce a user-facing change?

Uh oh!

packit-as-a-service bot commented Feb 17, 2026

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Honny1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

simonbrauner commented Feb 23, 2026

Uh oh!

Honny1 left a comment

Choose a reason for hiding this comment

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonbrauner commented Mar 3, 2026

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Honny1 left a comment

Choose a reason for hiding this comment

Uh oh!

Honny1 commented Mar 9, 2026

Uh oh!

mheon commented Mar 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

simonbrauner commented Feb 17, 2026 •

edited

Loading