Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFE] Add support for --link in COPY/ADD #4325

Open
lucacome opened this issue Oct 7, 2022 · 41 comments
Open

[RFE] Add support for --link in COPY/ADD #4325

lucacome opened this issue Oct 7, 2022 · 41 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. podman-desktop

Comments

@lucacome
Copy link
Contributor

lucacome commented Oct 7, 2022

Description

Buildkit added support for the --link flag. Quoting from the docs:

Enabling this flag in COPY or ADD commands allows you to copy files with enhanced semantics where your files remain independent on their own layer and don't get invalidated when commands on previous layers are changed.

When --link is used your source files are copied into an empty destination directory. That directory is turned into a layer that is linked on top of your previous state.

This is a feature request to add this feature in buildah 🙂

@TomSweeneyRedHat TomSweeneyRedHat added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 11, 2022
@TomSweeneyRedHat TomSweeneyRedHat changed the title Add support for --link in COPY/ADD [RFE] Add support for --link in COPY/ADD Oct 11, 2022
@CypherpunkSamurai
Copy link

Hello @lucacome and @TomSweeneyRedHat,

I want to contribute to this issue but am new to the project, How can I get started? :)

@rhatdan
Copy link
Member

rhatdan commented Oct 15, 2022

Well first you would need to add support for --link in
https://github.com/openshift/imagebuilder I believe.
@flouthoc Could you help @CypherpunkSamurai contribute.

@flouthoc
Copy link
Collaborator

Hi,

Yes implementation must start from imagebuilder and introduce a new attribute link bool to Copy struct and must be set true when COPY or ADD contains --link and on buildah side if Copy.link == truethen layer must be created on a scratch base, in theory COPY --link must be replaced with a functionality similar to stage where scratch image is base and file is just copied to target path.

COPY --link /src /target

must create a intermediate image equivalent to

FROM scatch
COPY /src /target

@lucacome
Copy link
Contributor Author

Hi @CypherpunkSamurai

I was wondering if you've had a chance to start working on this? Or do you need any help? 🙂

@CypherpunkSamurai
Copy link

Really sorry about this, I had gotten a little busy with college work.

I'll start work asap by this Saturday.

@rhatdan
Copy link
Member

rhatdan commented Nov 18, 2022

No problem college work should come first.

@CypherpunkSamurai
Copy link

Hello everyone, really sorry again for stalling this issue.

Ok Here's what I understand from the documentation of the --link tag for COPY and ADD build commands:

  • It creates a cached layer that can be reused
  • This cached layer is a layer from scratch. (i.e. a simple rootfs with busybox ? doubts here)

Here's how I understand how I need to implement it:

  • introduce a new attribute link bool to Copy struct and must be set true when COPY or ADD contains --link
  • on buildah side if Copy.link == truethen layer must be created on a scratch base, in theory COPY --link must be replaced with a functionality similar to stage where scratch image is base and file is just copied to target path.

Please check if this is correct

@flouthoc
Copy link
Collaborator

flouthoc commented Nov 19, 2022

Yes SGTM. Pretty much what's written here #4325 (comment) but I'd suggest if you have time to take a look and play with buildkit a bit before implementing this. My explanation in above comment is more conceptual and in past we have seen that buildkit's feature is not completely described in the documentation so its worth verifying edge cases before implementing this.

Note: Buildah mostly tries to match parity with buildkit as much as possible.

@CypherpunkSamurai
Copy link

Sure. I need to try both the tools first i guess. Would be interesting. I'll try my best to help this issue :)

@github-actions
Copy link

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

@CypherpunkSamurai
Copy link

Alright I researched quite a bit about rootfs and build root for the few weeks. I feel comfortable with linux rootfs and how it works.

Um, if you don't mind guys (@flouthoc and @rhatdan) can you please help me point out how buildah handles creating scratch rootfs? (in addition line number would be very cool)

@flouthoc
Copy link
Collaborator

@CypherpunkSamurai Its hard to pin the exact line since I'll have to skim the code but I can tell that builder creates a container for FROM scratch instruction you can actually find the part where builder actually creates container.

@github-actions
Copy link

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

@rhatdan
Copy link
Member

rhatdan commented Jan 23, 2023

Since we have not heard any more feedback, closing. Reopen if you want to continue looking into this one.

@lucacome
Copy link
Contributor Author

@rhatdan I'm still interested in this and I'd like to keep it open. Maybe somebody else can take over from @CypherpunkSamurai ?

@flouthoc
Copy link
Collaborator

@CypherpunkSamurai If you are running short on time, I'd request to assign this issue to me :)

@CypherpunkSamurai
Copy link

CypherpunkSamurai commented Jan 24, 2023 via email

fat-tire added a commit to fat-tire/InvokeAI that referenced this issue Feb 19, 2023
This removes some docker/buildkit-specific lines that break on
Podman... for now.

See:  containers/buildah#4325
      containers/buildah#3815

This is a painful patch, but without using another Dockerfile for Podman,
I couldnt find a non-convoluted alternative.
@github-actions
Copy link

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

@huww98
Copy link

huww98 commented Mar 19, 2024

I’m currently running buildkitd in podman, then use buildctl to build images.

@danishprakash
Copy link
Contributor

@flouthoc If you haven't started, I can take a shot at this.

dtrifiro added a commit to dtrifiro/vllm that referenced this issue May 8, 2024
- get rid of --link flags for COPY operations:
  not supported by openshift CI (buildah)
  See containers/buildah#4325
- get rid of conda
- install ccache to leverage caching
dtrifiro added a commit to opendatahub-io/vllm that referenced this issue May 8, 2024
- get rid of --link flags for COPY operations:
  not supported by openshift CI (buildah)
  See containers/buildah#4325
- get rid of conda
- install ccache to leverage caching
z103cb pushed a commit to z103cb/opendatahub_vllm that referenced this issue May 9, 2024
z103cb pushed a commit to opendatahub-io/vllm that referenced this issue May 9, 2024
@rhatdan
Copy link
Member

rhatdan commented May 21, 2024

@danishprakash go for it. @flouthoc has gone back to school and has much less time to work on this.

@edwbuck
Copy link

edwbuck commented Jun 24, 2024

Any updates on this one?

@apollo13
Copy link

apollo13 commented Aug 18, 2024

I'd love to see this as well. But I am kinda wondering if it makes sense to reimplement what buildkit does instead of using/embedding buildkit in podman? FWIW, aside from being daemonless (though you kind of can run buildkit like that as well: https://github.com/moby/buildkit?tab=readme-ov-file#daemonless ) podman build imo no longer has any features over docker build(x). The performance gains with buildkits are so big that I am currently reworking a few of our internal pipelines to use buildkit.

@flouthoc
Copy link
Collaborator

flouthoc commented Aug 18, 2024

But I am kinda wondering if it makes sense to reimplement what buildkit does instead of using/embedding buildkit in podman?

@apollo13 Could you please elaborate on this, podman does not uses or embed buildkit as-is most of the features are implemented in buildah as per buildah's design and podman uses buildah not buildkit.

@apollo13
Copy link

@flouthoc I was mainly wondering if it makes sense (or were possible) for podman to directly use buildkit instead of buildah. This would allow keeping feature parity more easily and also pick up all performance improvements. Especially with multi stage builds podman is really really much slower since buildkit is able to parallelize many things (unless I missed a flag for podman build)

@flouthoc
Copy link
Collaborator

Podman multi-stage build is parallel but you have to use --jobs flag. See https://docs.podman.io/en/latest/markdown/podman-build.1.html#jobs-number

@apollo13
Copy link

apollo13 commented Aug 19, 2024 via email

@rgov
Copy link

rgov commented Nov 21, 2024

Would it be sensible to silently ignore --link until full support is added? This would make Podman / Buildah compatible with Dockerfiles that use the flag, whereas currently the builds just fail.

From the flag description, it sounds like it is an optimization for cache reuse, but missing a cache reuse opportunity is not a big deal.

On the other hand, if there is a requirement that the layer checksums are exactly the same between Buildah and BuildKit, I don't know for certain if ignoring --link would result in the same checksums?

@danishprakash
Copy link
Contributor

From the flag description, it sounds like it is an optimization for cache reuse, but missing a cache reuse opportunity is not a big deal.

It's not but it's also not good design when we support an option for the sake of docker compat, and it might be misleading to users and might impact workflow.

That being said, I'll start working on this in a week or two, but in the meantime, @rhatdan thoughts on adding dummy flags? A similar sentiment was shared regarding --follow-link in containers/podman#16585 ftr.

@nathanweeks
Copy link

If COPY/ADD --link will be initially supported only syntactically, could a warning be emitted to stderr notifying users that --link is currently unsupported and is being ignored?

@rhatdan
Copy link
Member

rhatdan commented Nov 22, 2024

I would be fine with adding a warning, but it would be best if someone would step up and add the functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. podman-desktop
Projects
Status: No status
Development

No branches or pull requests