Skip to content

[v4.4.1-rhel] do not pass volume-opt as bind mounts options to runtime #28450

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.

Sign up for GitHub

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

Open
TomSweeneyRedHat wants to merge 4 commits into
base: v4.4.1-rhel
Choose a base branch
from
Open

[v4.4.1-rhel] do not pass volume-opt as bind mounts options to runtime #28450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0ae642f
[v4.4.1-rhel] do not pass volume-opt as bind mounts options to runtime
Luap99 Sep 1, 2025
1ea0979
[v4.4.1-rhel] do not pass [no]copy as bind mounts options to runtime
Luap99 Sep 1, 2025
a9ac512
[v4.4.1-rhel] CI: e2e: new ginkgo matcher, ExitCleanly()
edsantiago Aug 29, 2023
955e7f5
[v4.4.1-rhel] Introduce PodmanTestIntegration.PodmanExitCleanly
mtrmac Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion libpod/container_internal_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,13 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
if err := c.relabel(m.Source, c.MountLabel(), label.IsShared(o)); err != nil {
return nil, err
}

case "no-dereference":
// crun calls the option `copy-symlink`.
// 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)
}
Expand Down
13 changes: 13 additions & 0 deletions libpod/runtime_ctr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,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)
Expand All @@ -504,6 +513,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 opts == "idmap" {
needsChown = false
Expand All @@ -515,8 +525,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...)
Expand Down
21 changes: 19 additions & 2 deletions test/e2e/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,16 @@ func (s *PodmanSessionIntegration) InspectImageJSON() []inspect.ImageData {
return i
}

// PodmanExitCleanly runs a podman command with args, and expects it to ExitCleanly within the default timeout.
// It returns the session (to allow consuming output if desired).
func (p *PodmanTestIntegration) PodmanExitCleanly(args ...string) *PodmanSessionIntegration {
GinkgoHelper()
Copy link
Copy Markdown
Member

@lsm5 lsm5 Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomSweeneyRedHat remove this line too. This is not present on ginkgo v1. Sorry should've pointed that out earlier.

session := p.Podman(args)
session.WaitWithDefaultTimeout()
Expect(session).Should(ExitCleanly())
return session
}

// InspectContainer returns a container's inspect data in JSON format
func (p *PodmanTestIntegration) InspectContainer(name string) []define.InspectContainerData {
cmd := []string{"inspect", name}
Expand Down Expand Up @@ -475,8 +485,15 @@ func (p *PodmanTestIntegration) RunTopContainerWithArgs(name string, args []stri
podmanArgs = append(podmanArgs, "--name", name)
}
podmanArgs = append(podmanArgs, args...)
podmanArgs = append(podmanArgs, "-d", ALPINE, "top")
return p.Podman(podmanArgs)
podmanArgs = append(podmanArgs, "-d", ALPINE, "top", "-b")
session := p.PodmanExitCleanly(podmanArgs...)
cid := session.OutputToString()
// Output indicates that top is running, which means it's safe
// for our caller to invoke `podman stop`
if !WaitContainerReady(p, cid, "Mem:", 20, 1) {
Fail("Could not start a top container")
}
return session
}

// RunLsContainer runs a simple container in the background that
Expand Down
45 changes: 35 additions & 10 deletions test/e2e/run_volume_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
package integration

import (
"encoding/json"
"fmt"
"os"
"os/exec"
"os/user"
"path/filepath"
"strconv"
"strings"

. "github.com/containers/podman/v4/test/utils"
. "github.com/onsi/ginkgo"
. "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
Expand Down Expand Up @@ -451,9 +454,27 @@ var _ = Describe("Podman run with volumes", func() {
Expect(separateVolumeSession).Should(Exit(0))
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(Exit(0))
podmanTest.PodmanExitCleanly("run", "--name", "testctr", "-v", "testvol3:/etc/apk:copy", ALPINE, "stat", "-c", "%h", "/etc/apk/arch")
Copy link
Copy Markdown
Member

@lsm5 lsm5 Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PodmanExitCleanly doesn't exist on this branch yet, so we gotta revert to the old way or backport PodmanExitCleanly changes too.

Copy link
Copy Markdown
Member Author

@TomSweeneyRedHat TomSweeneyRedHat Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a look, thanks for the pointer.


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()
Expand Down Expand Up @@ -875,14 +896,18 @@ 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(Exit(0))

inspectVol := podmanTest.Podman([]string{"volume", "inspect", "test_vol"})
inspectVol.WaitWithDefaultTimeout()
Expect(inspectVol).Should(Exit(0))
Expect(inspectVol.OutputToString()).To(ContainSubstring("nodev"))
// 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.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() {
Expand Down
52 changes: 52 additions & 0 deletions test/utils/matchers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,58 @@ func (matcher *ExitMatcher) MatchMayChangeInTheFuture(actual interface{}) bool {
return true
}

// ExitCleanly asserts that a PodmanSession exits 0 and with no stderr
// Consider using PodmanTestIntegration.PodmanExitCleanly instead of directly using this matcher.
func ExitCleanly() types.GomegaMatcher {
return &exitCleanlyMatcher{}
}

type exitCleanlyMatcher struct {
msg string
}

type podmanSession interface {
ExitCode() int
ErrorToString() string
}

func (matcher *exitCleanlyMatcher) Match(actual interface{}) (success bool, err error) {
session, ok := actual.(podmanSession)
if !ok {
return false, fmt.Errorf("ExitCleanly must be passed a PodmanSession; Got:\n %+v\n%q", actual, format.Object(actual, 1))
}

exitcode := session.ExitCode()
stderr := session.ErrorToString()
if exitcode != 0 {
matcher.msg = fmt.Sprintf("Command failed with exit status %d", exitcode)
if stderr != "" {
matcher.msg += ". See above for error message."
}
return false, nil
}

// FIXME: #19809, "failed to connect to syslog" warnings on f38
// FIXME: so, until that is fixed, don't check stderr if containerized
if !Containerized() {
if stderr != "" {
matcher.msg = fmt.Sprintf("Unexpected warnings seen on stderr: %q", stderr)
return false, nil
}
}

return true, nil
}

func (matcher *exitCleanlyMatcher) FailureMessage(_ interface{}) (message string) {
return matcher.msg
}

func (matcher *exitCleanlyMatcher) NegatedFailureMessage(_ interface{}) (message string) {
// FIXME - I see no situation in which we could ever want this?
return matcher.msg + " (NOT!)"
}

type ValidJSONMatcher struct {
types.GomegaMatcher
}
Expand Down