diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index e92e78fc21..ecaa8cba07 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -420,6 +420,8 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc // Podman decided for --no-dereference as many // bin-utils tools (e..g, touch, chown, cp) do. options = append(options, "copy-symlink") + case "copy", "nocopy": + // no real OCI runtime bind mount options, these should already be handled by the named volume mount above default: options = append(options, o) } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 22aa76543e..1c089fbdae 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -504,6 +504,15 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai _, err := r.state.Volume(vol.Name) if err == nil { // The volume exists, we're good + // Make sure to drop all volume-opt options as they only apply to + // the volume create which we don't do again. + var volOpts []string + for _, opts := range vol.Options { + if !strings.HasPrefix(opts, "volume-opt") { + volOpts = append(volOpts, opts) + } + } + vol.Options = volOpts continue } else if !errors.Is(err, define.ErrNoSuchVolume) { return nil, fmt.Errorf("retrieving named volume %s for new container: %w", vol.Name, err) @@ -530,6 +539,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai if len(vol.Options) > 0 { isDriverOpts := false driverOpts := make(map[string]string) + var volOpts []string for _, opts := range vol.Options { if strings.HasPrefix(opts, "volume-opt") { isDriverOpts = true @@ -538,8 +548,11 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai return nil, err } driverOpts[driverOptKey] = driverOptValue + } else { + volOpts = append(volOpts, opts) } } + vol.Options = volOpts if isDriverOpts { parsedOptions := []VolumeCreateOption{WithVolumeOptions(driverOpts)} volOptions = append(volOptions, parsedOptions...) diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index b0020e1c67..75f3d874ed 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -3,17 +3,20 @@ package integration import ( + "encoding/json" "fmt" "os" "os/exec" "os/user" "path/filepath" + "strconv" "strings" . "github.com/containers/podman/v5/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "github.com/onsi/gomega/gexec" + "github.com/opencontainers/runtime-spec/specs-go" ) // in-container mount point: using a path that is definitely not present @@ -447,9 +450,27 @@ var _ = Describe("Podman run with volumes", func() { Expect(separateVolumeSession).Should(ExitCleanly()) Expect(separateVolumeSession.OutputToString()).To(Equal(baselineOutput)) - copySession := podmanTest.Podman([]string{"run", "--rm", "-v", "testvol3:/etc/apk:copy", ALPINE, "stat", "-c", "%h", "/etc/apk/arch"}) - copySession.WaitWithDefaultTimeout() - Expect(copySession).Should(ExitCleanly()) + podmanTest.PodmanExitCleanly("run", "--name", "testctr", "-v", "testvol3:/etc/apk:copy", ALPINE, "stat", "-c", "%h", "/etc/apk/arch") + + inspect := podmanTest.PodmanExitCleanly("container", "inspect", "testctr", "--format", "{{.OCIConfigPath}}") + + // Make extra check that the OCI config does not contain the copy opt, runc 1.3.0 fails on that while crun does not. + // We only test crun upstream so make sure the spec is sane: https://github.com/containers/podman/issues/26938 + f, err := os.Open(inspect.OutputToString()) + Expect(err).ToNot(HaveOccurred()) + defer f.Close() + var spec specs.Spec + err = json.NewDecoder(f).Decode(&spec) + Expect(err).ToNot(HaveOccurred()) + + found := false + for _, m := range spec.Mounts { + if m.Destination == "/etc/apk" { + found = true + Expect(m.Options).To(Equal([]string{"rprivate", "nosuid", "nodev", "rbind"})) + } + } + Expect(found).To(BeTrue(), "OCI spec contains /etc/apk mount") noCopySession := podmanTest.Podman([]string{"run", "--rm", "-v", "testvol4:/etc/apk:nocopy", ALPINE, "stat", "-c", "%h", "/etc/apk/arch"}) noCopySession.WaitWithDefaultTimeout() @@ -859,14 +880,17 @@ VOLUME /test/`, ALPINE) It("podman run with --mount and named volume with driver-opts", func() { // anonymous volume mount with driver opts vol := "type=volume,source=test_vol,dst=/test,volume-opt=type=tmpfs,volume-opt=device=tmpfs,volume-opt=o=nodev" - session := podmanTest.Podman([]string{"run", "--rm", "--mount", vol, ALPINE, "echo", "hello"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitCleanly()) + // Loop twice to cover both the initial code path that creates the volume and the ones which reuses it. + for i := range 2 { + name := "testctr" + strconv.Itoa(i) + podmanTest.PodmanExitCleanly("run", "--name", name, "--mount", vol, ALPINE, "echo", "hello") - inspectVol := podmanTest.Podman([]string{"volume", "inspect", "test_vol"}) - inspectVol.WaitWithDefaultTimeout() - Expect(inspectVol).Should(ExitCleanly()) - Expect(inspectVol.OutputToString()).To(ContainSubstring("nodev")) + inspectVol := podmanTest.PodmanExitCleanly("volume", "inspect", "test_vol") + Expect(inspectVol.OutputToString()).To(ContainSubstring("nodev")) + + inspect := podmanTest.PodmanExitCleanly("container", "inspect", name, "--format", "{{range .Mounts}}{{.Options}}{{end}}") + Expect(inspect.OutputToString()).To(ContainSubstring("[nosuid nodev rbind]")) + } }) It("volume permissions after run", func() {