buildah icon indicating copy to clipboard operation
buildah copied to clipboard

Inherit arguments between stages

Open onitake opened this issue 1 year ago • 10 comments

What type of PR is this?

/kind feature

What this PR does / why we need it:

Docker supports argument inheritance between build stages, but this feature is missing from buildah.

How to verify it

See the linked issue below for a basic example.

Which issue(s) this PR fixes:

Fixes #5762

Special notes for your reviewer:

To do:

  • [ ] Document the changes (see the Docker documentation about variable scoping: https://docs.docker.com/build/building/variables/#scoping )
  • [x] Add unit/functional tests for inherited arguments
  • [ ] Verify if stages with inherited arguments need to be rebuilt (args aren't persisted into container images?)
  • [ ] Other semantic changes to bring buildah in line with Docker (TBD)

Does this PR introduce a user-facing change?

ARG statements will now be inherited between build stages, like docker-build does.

onitake avatar Nov 22 '24 19:11 onitake

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: onitake Once this PR has been reviewed and has the lgtm label, please assign rhatdan 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:

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

openshift-ci[bot] avatar Nov 22 '24 19:11 openshift-ci[bot]

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

~~maps.Insert and maps.All for merging one map into another require Go 1.23. Is there something comparable in the buildah source that I could use?~~ Scratch that, maps.Copy does the job and is available since Go 1.21. :see_no_evil:

onitake avatar Nov 22 '24 19:11 onitake

Solving this here doesn't solve it for imagebuilder's CLI tool. I'm not sure what I think about that yet.

nalind avatar Dec 02 '24 20:12 nalind

@nalind It's still incomplete and doesn't seem to work how I imagined it yet anyway. But, what's imagebuilder? I though podman's image builder is buildah?

onitake avatar Dec 02 '24 21:12 onitake

Ah, sorry, that reference was buried somewhere in the linked issue. When I talk about "imagebuilder", I mean https://github.com/openshift/imagebuilder.

The Dockerfile parsing logic mostly lives in github.com/openshift/imagebuilder/dockerfile. The github.com/openshift/imagebuilder.Builder object "drives" a build using an implementation of its Executor interface. imagebuilder provides a CLI tool that combines its main library with github.com/openshift/imagebuilder/dockerclient.ClientExecutor, which implements the interface using github.com/fsouza/go-dockerclient.

Buildah originally didn't handle Dockerfiles at all, and when we needed to add the ability to do so, implementing a github.com/openshift/imagebuilder.Executor using github.com/containers/buildah.Builder and using a github.com/openshift/imagebuilder.Builder to "drive" builds seemed relatively straightforward.

At times, this has meant that we had to make changes there in order to enable features here, but I could say that about other dependencies we have as well.

nalind avatar Dec 02 '24 21:12 nalind

Ah sorry, now I get what you meant. Yes, I had originally thought I could/should place all argument handling logic there. But then I realized that arguments are only properly resolved and tracked when build stages are "executed", which all happens here in the buildah source.

Does that mean you would prefer that argument inheritance rules in Dockerfiles is handled by openshift/imagebuilder instead? I'm not sure if that is feasible at all, at least I think it would duplicate a lot of things.

onitake avatar Dec 02 '24 23:12 onitake

@onitake any update on this?

rhatdan avatar Dec 17 '24 18:12 rhatdan

@onitake any update on this?

I was struggling a bit to set up a proper test environment, so I haven't been able to debug why it's not working as expected yet. Will try to submit an update later this week.

onitake avatar Dec 18 '24 00:12 onitake

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

github-actions[bot] avatar Jan 18 '25 00:01 github-actions[bot]