From 21f34601eb4c62fc4fd52a26a6750dcd05cfea11 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Sat, 14 Jun 2025 09:16:06 +0200 Subject: [PATCH] artifact mount: improve single blob behavior If the artifact has a single blob then use the dst path directly as mount in case it does not exist. Signed-off-by: Paul Holzinger --- docs/source/markdown/options/mount.md | 13 +++++++++---- libpod/container_internal_common.go | 9 ++++++--- test/e2e/artifact_mount_test.go | 28 ++++++++++++++++++--------- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/docs/source/markdown/options/mount.md b/docs/source/markdown/options/mount.md index 0d298c61b4..b70ab873ca 100644 --- a/docs/source/markdown/options/mount.md +++ b/docs/source/markdown/options/mount.md @@ -34,15 +34,20 @@ Options specific to type=**artifact**: The *src* argument contains the name of the artifact, which must already exist locally. The *dst* argument contains the target path, if the path in the container is a -directory or does not exist the blob title (`org.opencontainers.image.title` -annotation) will be used as filename and joined to the path. If the annotation -does not exist the digest will be used as filename instead. This results in all blobs -of the artifact mounted into the container at the given path. +directory the blob title (`org.opencontainers.image.title` annotation) will be used as +filename and joined to the path. If the annotation does not exist the digest will be +used as filename instead. This results in all blobs of the artifact mounted into the +container at the given path. However, if the *dst* path is an existing file in the container, then the blob will be mounted directly on it. This only works when the artifact contains a single blob or when either *digest* or *title* are specified. +If the *dst* path does not already exist in the container then if the artifact contains +a single blob it behaves like existing file case and mounts directly to that path. +If the artifact has more than one blob it works like the existing directory case and +mounts each blob as file within the *dst* path. + Options specific to type=**volume**: - *ro*, *readonly*: *true* or *false* (default if unspecified: *false*). diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index b6196dba7e..f5222b7a93 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -554,9 +554,12 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc return nil, nil, err } - // Ignore the error, destIsFile will return false with errors so if the file does not exist - // we treat it as dir, the oci runtime will always create the target bind mount path. - destIsFile, _ := containerPathIsFile(c.state.Mountpoint, artifactMount.Dest) + destIsFile, err := containerPathIsFile(c.state.Mountpoint, artifactMount.Dest) + // When the file does not exists and the artifact has only a single blob to mount + // assume it is a file so we use the dest path as direct mount. + if err != nil && len(paths) == 1 && errors.Is(err, fs.ErrNotExist) { + destIsFile = true + } if destIsFile && len(paths) > 1 { return nil, nil, fmt.Errorf("artifact %q contains more than one blob and container path %q is a file", artifactMount.Source, artifactMount.Dest) } diff --git a/test/e2e/artifact_mount_test.go b/test/e2e/artifact_mount_test.go index d158dc1331..e8b5a7ca2a 100644 --- a/test/e2e/artifact_mount_test.go +++ b/test/e2e/artifact_mount_test.go @@ -29,7 +29,7 @@ var _ = Describe("Podman artifact mount", func() { { name: "single artifact mount", mountOpts: "dst=/test", - containerFile: "/test/testfile", + containerFile: "/test", }, { name: "single artifact mount on existing file", @@ -44,7 +44,7 @@ var _ = Describe("Podman artifact mount", func() { { name: "single artifact mount with digest", mountOpts: "dst=/data,digest=sha256:e9510923578af3632946ecf5ae479c1b5f08b47464e707b5cbab9819272a9752", - containerFile: "/data/sha256-e9510923578af3632946ecf5ae479c1b5f08b47464e707b5cbab9819272a9752", + containerFile: "/data", }, } @@ -104,21 +104,31 @@ var _ = Describe("Podman artifact mount", func() { }, }, { - name: "multi blob filter by title", + name: "multi blob filter by title on non existing file", mountOpts: "src=" + ARTIFACT_MULTI + ",dst=/test,title=test2", containerFiles: []expectedFiles{ { - file: "/test/test2", + file: "/test", + content: artifactContent2, + }, + }, + }, + { + name: "multi blob filter by title on existing file", + mountOpts: "src=" + ARTIFACT_MULTI + ",dst=/tmp,title=test2", + containerFiles: []expectedFiles{ + { + file: "/tmp/test2", content: artifactContent2, }, }, }, { name: "multi blob filter by digest", - mountOpts: "src=" + ARTIFACT_MULTI + ",dst=/test,digest=sha256:8257bba28b9d19ac353c4b713b470860278857767935ef7e139afd596cb1bb2d", + mountOpts: "src=" + ARTIFACT_MULTI + ",dst=/tmp,digest=sha256:8257bba28b9d19ac353c4b713b470860278857767935ef7e139afd596cb1bb2d", containerFiles: []expectedFiles{ { - file: "/test/sha256-8257bba28b9d19ac353c4b713b470860278857767935ef7e139afd596cb1bb2d", + file: "/tmp/sha256-8257bba28b9d19ac353c4b713b470860278857767935ef7e139afd596cb1bb2d", content: artifactContent1, }, }, @@ -152,12 +162,12 @@ var _ = Describe("Podman artifact mount", func() { podmanTest.PodmanExitCleanly("artifact", "add", artifactName, artifactFile) // FIXME: we need https://github.com/containers/container-selinux/pull/360 to fix the selinux access problem, until then disable it. - podmanTest.PodmanExitCleanly("run", "--security-opt=label=disable", "--name", ctrName, "-d", "--mount", "type=artifact,src="+artifactName+",dst=/test", ALPINE, "sleep", "100") + podmanTest.PodmanExitCleanly("run", "--security-opt=label=disable", "--name", ctrName, "-d", "--mount", "type=artifact,src="+artifactName+",dst=/tmp", ALPINE, "sleep", "100") podmanTest.PodmanExitCleanly("artifact", "rm", artifactName) // file must sill be readable after artifact removal - session := podmanTest.PodmanExitCleanly("exec", ctrName, "cat", "/test/"+artifactFileName) + session := podmanTest.PodmanExitCleanly("exec", ctrName, "cat", "/tmp/"+artifactFileName) Expect(session.OutputToString()).To(Equal("hello world")) // restart will fail if artifact does not exist @@ -174,7 +184,7 @@ var _ = Describe("Podman artifact mount", func() { podmanTest.PodmanExitCleanly("artifact", "add", artifactName, artifactFile, artifactFile2) podmanTest.PodmanExitCleanly("start", ctrName) - session = podmanTest.PodmanExitCleanly("exec", ctrName, "cat", "/test/"+artifactFileName, "/test/"+artifactFile2Name) + session = podmanTest.PodmanExitCleanly("exec", ctrName, "cat", "/tmp/"+artifactFileName, "/tmp/"+artifactFile2Name) Expect(session.OutputToString()).To(Equal("hello world second file")) })