-
Notifications
You must be signed in to change notification settings - Fork 788
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
buildkit: add from=
field to bind
and cache
mounts so images can be used as source
#3590
buildkit: add from=
field to bind
and cache
mounts so images can be used as source
#3590
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc 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 |
2446d90
to
4d7076d
Compare
a2f101b
to
86a1548
Compare
@nalind @TomSweeneyRedHat Made requested corrections. Could you please take a look again. |
LGTM |
The tests need to also exercise |
@nalind Thanks i'll get these tests in. |
86a1548
to
f1e0075
Compare
f1e0075
to
ed42c9a
Compare
c683b49
to
3941014
Compare
@nalind Added requested tests. I would also request you to take a look at commit 3941014 which makes sure we use common selinux label across all containers/builders spawned under a common Executor session. Its needed to mount different stages. |
a4579c2
to
b934085
Compare
b934085
to
f93ac81
Compare
73a8402
to
91b47f1
Compare
I'm not entirely sure we've got the behavior right for writable bind mounts, otherwise LGTM. |
Me and @nalind discussed this, so our design has some limitation on @containers/build-maintainers @rhatdan @TomSweeneyRedHat PTAL |
|
||
setDest := false | ||
|
||
for _, val := range args { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no way to consolidate this code together, this is the third time you have code to walk the arg list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rhatdan I agree we can reduce redundancy. Some of the options are not supported in other mounts so switch
case is slightly different for each one.
However parsing
is legacy code this is just a move around. So to prevent huge diff would it work if i do that in a follow up
PR where I will also fix selinux
issue.
@flouthoc needs rebase. |
6a65fa8
to
fb163aa
Compare
Sadly @flouthoc needs a rebase. |
…ed as source Following commit adds buildkit like support for `from` field to `--mount=type=bind` and `--mount=type=cache` so images and stage can be used as mount source. Usage looks like ```dockerfile RUN --mount=type=bind,source=.,from=<your-image>,target=/path ls /path ``` and ```dockerfile RUN --mount=type=cache,from=<your-image>,target=/path ls /path ``` Signed-off-by: Aditya Rajan <[email protected]>
fb163aa
to
719b660
Compare
/lgtm |
Following commit adds buildkit like support for
from
field to--mount=type=bind
and
--mount=type=cache
so images and stage can be used as mount source.Usage looks like
RUN --mount=type=bind,source=.,from=<your-image>,target=/path ls /path
and
RUN --mount=type=cache,from=<your-image>,target=/path ls /path
Closes: #3501