cmd/build: add support for `--push`

[danishprakash]

trafficstars

What type of PR is this?

/kind feature

Which issue(s) this PR fixes:

Closes #4671

Special notes for your reviewer:

Extremely WIP draft.

Does this PR introduce a user-facing change?

- add support for `--push` flag to `bud` push image immediately after building

[rhatdan]

I was thinking this would just be added to the client and not the service side.

buildah build --push

Would internally do

buildah build followed by buildah push.

[rhatdan]

What do you think @nalind @flouthoc @vrothberg

[nalind]

I was thinking this would just be added to the client and not the service side.

buildah build --push

Would internally do

buildah build followed by buildah push.

That would be simpler; API callers already have a buildah.Push() that they can call directly, and it's less API churn if we add such a call to the tail end of cmd/buildah/buildCmd() if --push.

For the "output=registry" case, setting IsDir in a BuildOutputOption while also adding a Type field that overlaps its meaning is confusing. Making that a special case after we've extracted the output image's rootfs as an archive (an archive which doesn't get used if we're just going to push the image) looks wrong; pushing to a registry probably needs to be an entirely separate branch in generateBuildOutput().

[TomSweeneyRedHat]

And just in case, we'll need updates to the man page and additional test added for the final PR. From a quick flyby review, it LGTM other than other comments.

[vrothberg]

What do you think @nalind @flouthoc @vrothberg

I am no fan of mixing multiple commands as it breaks separation of concerns. buildah push has over a dozen command-line options. How would they be exposed in buildah build?

But I leave the decision up to the core maintainers of Buildah.

[rhatdan]

I see this for compatibility with buildx https://docs.docker.com/engine/reference/commandline/buildx_build/ --push |   | Shorthand for --output=type=registry

I don't think we have to add other options for push, and potentially could have just done this in podman build, but the --output=type calls are handled within buildah.

[flouthoc]

I concur should we just do this at podman, and only add --output=type=registry to buildah ?

[danishprakash]

I was thinking this would just be added to the client and not the service side.

buildah build --push

Would internally do

buildah build followed by buildah push.

I honestly didn't think that is how we'd want to push. On the outset, --push seems like it should be added serially to build but I felt it might not be extensible given there could be modifications/additions to the --output flag in future.

For the "output=registry" case, setting IsDir in a BuildOutputOption while also adding a Type field that overlaps its meaning is confusing. Making that a special case after we've extracted the output image's rootfs as an archive (an archive which doesn't get used if we're just going to push the image) looks wrong; pushing to a registry probably needs to be an entirely separate branch in generateBuildOutput().

You're right, that was an oversight.

[danishprakash]

I concur should we just do this at podman, and only add --output=type=registry to buildah ?

So adding the --push flag to podman which would be translated to --output within buildah? Sounds about right to me. That was the original plan as mentioned here.

[StarpTech]

Hi, I just created https://github.com/containers/podman/issues/18642 It looks related to this PR.

I am no fan of mixing multiple commands as it breaks separation of concerns. buildah push has over a dozen command-line options. How would they be exposed in buildah build?

I agree but a good reason to combine them is to enable parallelism.

[rhatdan]

@danishprakash Are you still working on this?

[danishprakash]

Yes, @rhatdan. Will try to address the remaining comments and the tests by early next week.

[openshift-ci[bot]]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: danishprakash Once this PR has been reviewed and has the lgtm label, please assign giuseppe for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

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

[rhatdan]

@flouthoc @nalind PTAL

[github-actions[bot]]

A friendly reminder that this PR had no activity for 30 days.

[rhatdan]

@danishprakash still working on this?

[danishprakash]

Yes, going to get back to this end of this week.

[packit-as-a-service[bot]]

Ephemeral COPR build failed. @containers/packit-build please check.