Skip to content

Conversation

@MisterOwlPT
Copy link
Contributor

Closes #592.

With the proposed changes it is now possible to create manifests and automatically add images to them as part of the build process (equivalent to podman build --manifest ...).

# ...
with PodmanClient(base_url=uri) as client:
  image, _ = client.images.build(
    path=".",
    dockerfile="Containerfile",
    manifest="example:v1.2.3",  # <-- this field is now supported 
    platform=["linux/amd64", "linux/arm64"],
  )

Running the previous code, all built images are automatically added to the provided manifest (which is also created if it does not exist already).

Changes:

  • Mapped the --manifest argument into the expected manifest query parameter when making requests;
  • Updated documentation for the client.images.build function as to refer to the newly support field. Description text extracted from the Podman CLI itself;
  • Updated existing unit tests to ensure that --manifest argument is mapped into the expected query parameter;
  • Implemented new integration test to ensure that a manifest is automatically created if required. A complementary cleanup mechanism was also implemented to ensure that manifests created during tests are deleted after their execution.

Looking forward to your feedback.

@jwhonce
Copy link
Member

jwhonce commented Oct 6, 2025

/approve

LGTM

@MisterOwlPT
Copy link
Contributor Author

MisterOwlPT commented Oct 6, 2025

Hi @jwhonce,

Some more points concerning this PR.

  • I have fixed the existing linting errors;
  • I've shortened the two long commit messages (to fit the 72 character limit).
  • Removed accidental signature lines from commits I didn’t author and only the commits that are actually mine are now signed by me.

Thanks for the review, and really really sorry for the extra noise!

Edit: typos

@jwhonce
Copy link
Member

jwhonce commented Oct 7, 2025

@MisterOwlPT It appears the commit linter is still not happy with your work.

@MisterOwlPT
Copy link
Contributor Author

@jwhonce,
I've amended the troublesome commit messages.
Everything should be fine now.

Let me know if everything looks good to you.

@inknos
Copy link
Contributor

inknos commented Oct 7, 2025

@MisterOwlPT your code is clean and linter is happy, nice job.

I believe you need to rebase to main and fix the commits, I see the commit history is mixed with merged history, that's not clean and can't be merged.

I tried to rebase locally, and this is what imho looks nicer in a single patch. what do you think?

0001-support-flag-manifest-when-building-container-images.patch

@MisterOwlPT MisterOwlPT force-pushed the feat/592-build-support-manifest branch from 6a83c35 to 64e82c2 Compare October 8, 2025 22:42
@MisterOwlPT
Copy link
Contributor Author

@inknos,

I believe you need to rebase to main and fix the commits, I see the commit history is mixed with merged history, that's not clean and can't be merged.

Yes, sincere apologies for that!
As suggested, I've git rebase to main and fixed the commits.
I believe this branch is now clean as intended.

Let me know if something else is required.
Thanks once again for your patience!

Copy link
Contributor

@inknos inknos left a comment

Choose a reason for hiding this comment

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

great job with the commits, I found a thing that could break the expected behavior, please take a look and let me know

@inknos
Copy link
Contributor

inknos commented Oct 9, 2025

@MisterOwlPT all is good. let me just nitpick on the commits. we don't squash on merge, and the change looks nice on a single commit since it's all part of one added feature. would you do squash them, please? then I'll merge :D

Signed-off-by: Pedro Pereira <augmented64chord@gmail.com>
@MisterOwlPT MisterOwlPT force-pushed the feat/592-build-support-manifest branch from 64e82c2 to cf1a777 Compare October 9, 2025 09:41
@MisterOwlPT
Copy link
Contributor Author

@inknos,

would you do squash them, please?

Done!
I got a bit carried away with "little, incremental commits" 😅

Copy link
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

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

At first glance, the code LGTM. Let's see what CI has to say.

Edit: approved CI passed

@inknos
Copy link
Contributor

inknos commented Oct 9, 2025

/approve
/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 9, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: inknos, jwhonce, MisterOwlPT

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-merge-bot openshift-merge-bot bot merged commit c9ca28d into containers:main Oct 9, 2025
22 checks passed
@MisterOwlPT MisterOwlPT deleted the feat/592-build-support-manifest branch October 9, 2025 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add direct support to --manifest for building multi-architecture container images.

4 participants