diff options
30 files changed, 315 insertions, 237 deletions
diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index d28becc8a..886c10cb5 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -255,9 +255,8 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions, _ = cmd.RegisterFlagCompletionFunc(hostUserFlagName, completion.AutocompleteNone) imageVolumeFlagName := "image-volume" - createFlags.StringVar( - &cf.ImageVolume, - imageVolumeFlagName, DefaultImageVolume, + createFlags.String( + imageVolumeFlagName, containerConfig.Engine.ImageVolumeMode, `Tells podman how to handle the builtin image volumes ("bind"|"tmpfs"|"ignore")`, ) _ = cmd.RegisterFlagCompletionFunc(imageVolumeFlagName, AutocompleteImageVolume) diff --git a/cmd/podman/common/default.go b/cmd/podman/common/default.go index 7caec50ff..6f78d3d29 100644 --- a/cmd/podman/common/default.go +++ b/cmd/podman/common/default.go @@ -5,9 +5,6 @@ import ( ) var ( - - // DefaultImageVolume default value - DefaultImageVolume = "bind" // Pull in configured json library json = registry.JSONLibrary() ) diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index c62ddd6eb..0a513c606 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -102,16 +102,25 @@ func init() { createFlags(containerCreateCommand) } -func create(cmd *cobra.Command, args []string) error { - var ( - err error - ) +func commonFlags(cmd *cobra.Command) error { + var err error flags := cmd.Flags() cliVals.Net, err = common.NetFlagsToNetOptions(nil, *flags) if err != nil { return err } + if cmd.Flags().Changed("image-volume") { + cliVals.ImageVolume = cmd.Flag("image-volume").Value.String() + } + return nil +} + +func create(cmd *cobra.Command, args []string) error { + if err := commonFlags(cmd); err != nil { + return err + } + // Check if initctr is used with --pod and the value is correct if initctr := InitContainerType; cmd.Flags().Changed("init-ctr") { if !cmd.Flags().Changed("pod") { @@ -123,7 +132,7 @@ func create(cmd *cobra.Command, args []string) error { cliVals.InitContainerType = initctr } - cliVals, err = CreateInit(cmd, cliVals, false) + cliVals, err := CreateInit(cmd, cliVals, false) if err != nil { return err } diff --git a/cmd/podman/containers/run.go b/cmd/podman/containers/run.go index 951981293..a6c500afa 100644 --- a/cmd/podman/containers/run.go +++ b/cmd/podman/containers/run.go @@ -109,7 +109,9 @@ func init() { } func run(cmd *cobra.Command, args []string) error { - var err error + if err := commonFlags(cmd); err != nil { + return err + } // TODO: Breaking change should be made fatal in next major Release if cliVals.TTY && cliVals.Interactive && !term.IsTerminal(int(os.Stdin.Fd())) { @@ -122,14 +124,10 @@ func run(cmd *cobra.Command, args []string) error { } } - flags := cmd.Flags() - cliVals.Net, err = common.NetFlagsToNetOptions(nil, *flags) - if err != nil { - return err - } runOpts.CIDFile = cliVals.CIDFile runOpts.Rm = cliVals.Rm - if cliVals, err = CreateInit(cmd, cliVals, false); err != nil { + cliVals, err := CreateInit(cmd, cliVals, false) + if err != nil { return err } diff --git a/cmd/podman/machine/ssh.go b/cmd/podman/machine/ssh.go index 4a86da67a..8261f3607 100644 --- a/cmd/podman/machine/ssh.go +++ b/cmd/podman/machine/ssh.go @@ -9,6 +9,7 @@ import ( "github.com/containers/common/pkg/completion" "github.com/containers/common/pkg/config" "github.com/containers/podman/v4/cmd/podman/registry" + "github.com/containers/podman/v4/cmd/podman/utils" "github.com/containers/podman/v4/pkg/machine" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -89,7 +90,8 @@ func ssh(cmd *cobra.Command, args []string) error { if err != nil { return errors.Wrapf(err, "vm %s not found", vmName) } - return vm.SSH(vmName, sshOpts) + err = vm.SSH(vmName, sshOpts) + return utils.HandleOSExecError(err) } func remoteConnectionUsername() (string, error) { diff --git a/cmd/podman/system/unshare.go b/cmd/podman/system/unshare.go index 0ae5b81ad..1ed08eac3 100644 --- a/cmd/podman/system/unshare.go +++ b/cmd/podman/system/unshare.go @@ -2,10 +2,10 @@ package system import ( "os" - "os/exec" "github.com/containers/common/pkg/completion" "github.com/containers/podman/v4/cmd/podman/registry" + "github.com/containers/podman/v4/cmd/podman/utils" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/rootless" "github.com/pkg/errors" @@ -60,22 +60,5 @@ func unshare(cmd *cobra.Command, args []string) error { } err := registry.ContainerEngine().Unshare(registry.Context(), args, unshareOptions) - if err != nil { - if exitError, ok := err.(*exec.ExitError); ok { - // the user command inside the unshare env has failed - // we set the exit code, do not return the error to the user - // otherwise "exit status X" will be printed - registry.SetExitCode(exitError.ExitCode()) - return nil - } - // cmd.Run() can return fs.ErrNotExist, fs.ErrPermission or exec.ErrNotFound - // follow podman run/exec standard with the exit codes - if errors.Is(err, os.ErrNotExist) || errors.Is(err, exec.ErrNotFound) { - registry.SetExitCode(127) - } else if errors.Is(err, os.ErrPermission) { - registry.SetExitCode(126) - } - return err - } - return nil + return utils.HandleOSExecError(err) } diff --git a/cmd/podman/utils/error.go b/cmd/podman/utils/error.go index 2aaa71373..3efff0301 100644 --- a/cmd/podman/utils/error.go +++ b/cmd/podman/utils/error.go @@ -4,10 +4,12 @@ import ( "errors" "fmt" "os" + "os/exec" "strconv" "strings" buildahCLI "github.com/containers/buildah/pkg/cli" + "github.com/containers/podman/v4/cmd/podman/registry" ) type OutputErrors []error @@ -43,3 +45,33 @@ func ExitCodeFromBuildError(errorMsg string) (int, error) { } return buildahCLI.ExecErrorCodeGeneric, errors.New("message does not contains a valid exit code") } + +// HandleOSExecError checks the given error for an exec.ExitError error and +// sets the same podman exit code as the error. +// No error will be returned in this case to make sure things like podman +// unshare false work correctly without extra output. +// When the exec file does not exists we set the exit code to 127, for +// permission errors 126 is used as exit code. In this case we still return +// the error so the user gets an error message. +// If the error is nil it returns nil. +func HandleOSExecError(err error) error { + if err == nil { + return nil + } + var exitError *exec.ExitError + if errors.As(err, &exitError) { + // the user command inside the unshare/ssh env has failed + // we set the exit code, do not return the error to the user + // otherwise "exit status X" will be printed + registry.SetExitCode(exitError.ExitCode()) + return nil + } + // cmd.Run() can return fs.ErrNotExist, fs.ErrPermission or exec.ErrNotFound + // follow podman run/exec standard with the exit codes + if errors.Is(err, os.ErrNotExist) || errors.Is(err, exec.ErrNotFound) { + registry.SetExitCode(127) + } else if errors.Is(err, os.ErrPermission) { + registry.SetExitCode(126) + } + return err +} diff --git a/cmd/podman/utils/signals_linux.go b/cmd/podman/utils/signals_linux.go deleted file mode 100644 index dd0507c0e..000000000 --- a/cmd/podman/utils/signals_linux.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build !windows -// +build !windows - -package utils - -import ( - "os" - - "golang.org/x/sys/unix" -) - -// Platform specific signal synonyms -var ( - SIGHUP os.Signal = unix.SIGHUP -) diff --git a/cmd/podman/utils/signals_windows.go b/cmd/podman/utils/signals_windows.go deleted file mode 100644 index e6fcc1b32..000000000 --- a/cmd/podman/utils/signals_windows.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build windows -// +build windows - -package utils - -import ( - "os" - - "golang.org/x/sys/windows" -) - -// Platform specific signal synonyms -var ( - SIGHUP os.Signal = windows.SIGHUP -) diff --git a/contrib/systemd/system/podman-restart.service.in b/contrib/systemd/system/podman-restart.service.in index 1f13e57e1..de0249381 100644 --- a/contrib/systemd/system/podman-restart.service.in +++ b/contrib/systemd/system/podman-restart.service.in @@ -2,12 +2,15 @@ Description=Podman Start All Containers With Restart Policy Set To Always Documentation=man:podman-start(1) StartLimitIntervalSec=0 +Wants=network-online.target +After=network-online.target [Service] Type=oneshot RemainAfterExit=true Environment=LOGGING="--log-level=info" ExecStart=@@PODMAN@@ $LOGGING start --all --filter restart-policy=always +ExecStop=/bin/sh -c '@@PODMAN@@ $LOGGING stop $(@@PODMAN@@ container ls --filter restart-policy=always -q)' [Install] WantedBy=default.target diff --git a/docs/source/markdown/podman-machine-ssh.1.md b/docs/source/markdown/podman-machine-ssh.1.md index db0350961..6a1455df1 100644 --- a/docs/source/markdown/podman-machine-ssh.1.md +++ b/docs/source/markdown/podman-machine-ssh.1.md @@ -14,6 +14,7 @@ first argument must be the virtual machine name. The optional command to execute can then follow. If no command is provided, an interactive session with the virtual machine is established. +The exit code from ssh command will be forwarded to the podman machine ssh caller, see [Exit Codes](#Exit-Codes). ## OPTIONS @@ -25,6 +26,35 @@ Print usage statement. Username to use when SSH-ing into the VM. +## Exit Codes + +The exit code from `podman machine ssh` gives information about why the command failed. +When `podman machine ssh` commands exit with a non-zero code, +the exit codes follow the `chroot` standard, see below: + + **125** The error is with podman **_itself_** + + $ podman machine ssh --foo; echo $? + Error: unknown flag: --foo + 125 + + **126** Executing a _contained command_ and the _command_ cannot be invoked + + $ podman machine ssh /etc; echo $? + Error: fork/exec /etc: permission denied + 126 + + **127** Executing a _contained command_ and the _command_ cannot be found + + $ podman machine ssh foo; echo $? + Error: fork/exec /usr/bin/bogus: no such file or directory + 127 + + **Exit code** _contained command_ exit code + + $ podman machine ssh /bin/sh -c 'exit 3'; echo $? + 3 + ## EXAMPLES To get an interactive session with the default virtual machine: diff --git a/libpod/healthcheck_linux.go b/libpod/healthcheck_linux.go index 45b3a0e41..1e03db542 100644 --- a/libpod/healthcheck_linux.go +++ b/libpod/healthcheck_linux.go @@ -7,6 +7,7 @@ import ( "os/exec" "strings" + "github.com/containers/podman/v4/pkg/errorhandling" "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/systemd" "github.com/pkg/errors" @@ -46,6 +47,17 @@ func (c *Container) createTimer() error { return nil } +// Wait for a message on the channel. Throw an error if the message is not "done". +func systemdOpSuccessful(c chan string) error { + msg := <-c + switch msg { + case "done": + return nil + default: + return fmt.Errorf("expected %q but received %q", "done", msg) + } +} + // startTimer starts a systemd timer for the healthchecks func (c *Container) startTimer() error { if c.disableHealthCheckSystemd() { @@ -56,8 +68,17 @@ func (c *Container) startTimer() error { return errors.Wrapf(err, "unable to get systemd connection to start healthchecks") } defer conn.Close() - _, err = conn.StartUnitContext(context.Background(), fmt.Sprintf("%s.service", c.ID()), "fail", nil) - return err + + startFile := fmt.Sprintf("%s.service", c.ID()) + startChan := make(chan string) + if _, err := conn.StartUnitContext(context.Background(), startFile, "fail", startChan); err != nil { + return err + } + if err := systemdOpSuccessful(startChan); err != nil { + return fmt.Errorf("starting systemd health-check timer %q: %w", startFile, err) + } + + return nil } // removeTransientFiles removes the systemd timer and unit files @@ -71,30 +92,37 @@ func (c *Container) removeTransientFiles(ctx context.Context) error { return errors.Wrapf(err, "unable to get systemd connection to remove healthchecks") } defer conn.Close() + + // Errors are returned at the very end. Let's make sure to stop and + // clean up as much as possible. + stopErrors := []error{} + + // Stop the timer before the service to make sure the timer does not + // fire after the service is stopped. + timerChan := make(chan string) timerFile := fmt.Sprintf("%s.timer", c.ID()) - serviceFile := fmt.Sprintf("%s.service", c.ID()) + if _, err := conn.StopUnitContext(ctx, timerFile, "fail", timerChan); err != nil { + if !strings.HasSuffix(err.Error(), ".timer not loaded.") { + stopErrors = append(stopErrors, fmt.Errorf("removing health-check timer %q: %w", timerFile, err)) + } + } else if err := systemdOpSuccessful(timerChan); err != nil { + stopErrors = append(stopErrors, fmt.Errorf("stopping systemd health-check timer %q: %w", timerFile, err)) + } - // If the service has failed (the healthcheck has failed), then - // the .service file is not removed on stopping the unit file. If - // we check the properties of the service, it will automatically - // reset the state. But checking the state takes msecs vs usecs to - // blindly call reset. + // Reset the service before stopping it to make sure it's being removed + // on stop. + serviceChan := make(chan string) + serviceFile := fmt.Sprintf("%s.service", c.ID()) if err := conn.ResetFailedUnitContext(ctx, serviceFile); err != nil { - logrus.Debugf("failed to reset unit file: %q", err) + logrus.Debugf("Failed to reset unit file: %q", err) } - - // We want to ignore errors where the timer unit and/or service unit has already - // been removed. The error return is generic so we have to check against the - // string in the error - if _, err = conn.StopUnitContext(ctx, serviceFile, "fail", nil); err != nil { + if _, err := conn.StopUnitContext(ctx, serviceFile, "fail", serviceChan); err != nil { if !strings.HasSuffix(err.Error(), ".service not loaded.") { - return errors.Wrapf(err, "unable to remove service file") - } - } - if _, err = conn.StopUnitContext(ctx, timerFile, "fail", nil); err != nil { - if strings.HasSuffix(err.Error(), ".timer not loaded.") { - return nil + stopErrors = append(stopErrors, fmt.Errorf("removing health-check service %q: %w", serviceFile, err)) } + } else if err := systemdOpSuccessful(serviceChan); err != nil { + stopErrors = append(stopErrors, fmt.Errorf("stopping systemd health-check service %q: %w", serviceFile, err)) } - return err + + return errorhandling.JoinErrors(stopErrors) } diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go index 73e64530e..37fa9b5f5 100644 --- a/libpod/networking_linux.go +++ b/libpod/networking_linux.go @@ -930,6 +930,8 @@ func (r *Runtime) reloadContainerNetwork(ctr *Container) (map[string]types.Statu return r.configureNetNS(ctr, ctr.state.NetNS) } +// TODO (5.0): return the statistics per network interface +// This would allow better compat with docker. func getContainerNetIO(ctr *Container) (*netlink.LinkStatistics, error) { var netStats *netlink.LinkStatistics @@ -943,21 +945,39 @@ func getContainerNetIO(ctr *Container) (*netlink.LinkStatistics, error) { return nil, nil } - // FIXME get the interface from the container netstatus - dev := "eth0" netMode := ctr.config.NetMode + netStatus := ctr.getNetworkStatus() if otherCtr != nil { netMode = otherCtr.config.NetMode + netStatus = otherCtr.getNetworkStatus() } if netMode.IsSlirp4netns() { - dev = "tap0" + // create a fake status with correct interface name for the logic below + netStatus = map[string]types.StatusBlock{ + "slirp4netns": { + Interfaces: map[string]types.NetInterface{"tap0": {}}, + }, + } } err := ns.WithNetNSPath(netNSPath, func(_ ns.NetNS) error { - link, err := netlink.LinkByName(dev) - if err != nil { - return err + for _, status := range netStatus { + for dev := range status.Interfaces { + link, err := netlink.LinkByName(dev) + if err != nil { + return err + } + if netStats == nil { + netStats = link.Attrs().Statistics + continue + } + // Currently only Tx/RxBytes are used. + // In the future we should return all stats per interface so that + // api users have a better options. + stats := link.Attrs().Statistics + netStats.TxBytes += stats.TxBytes + netStats.RxBytes += stats.RxBytes + } } - netStats = link.Attrs().Statistics return nil }) return netStats, err diff --git a/pkg/domain/infra/runtime_libpod.go b/pkg/domain/infra/runtime_libpod.go index ac557e9de..daa6f0cbf 100644 --- a/pkg/domain/infra/runtime_libpod.go +++ b/pkg/domain/infra/runtime_libpod.go @@ -9,9 +9,9 @@ import ( "os" "os/signal" "sync" + "syscall" "github.com/containers/common/pkg/cgroups" - "github.com/containers/podman/v4/cmd/podman/utils" "github.com/containers/podman/v4/libpod" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/namespaces" @@ -375,7 +375,7 @@ func ParseIDMapping(mode namespaces.UsernsMode, uidMapSlice, gidMapSlice []strin func StartWatcher(rt *libpod.Runtime) { // Setup the signal notifier ch := make(chan os.Signal, 1) - signal.Notify(ch, utils.SIGHUP) + signal.Notify(ch, syscall.SIGHUP) go func() { for { diff --git a/pkg/machine/config.go b/pkg/machine/config.go index d34776714..abbebc9f9 100644 --- a/pkg/machine/config.go +++ b/pkg/machine/config.go @@ -138,14 +138,15 @@ type DistributionDownload interface { Get() *Download } type InspectInfo struct { - ConfigPath VMFile - Created time.Time - Image ImageConfig - LastUp time.Time - Name string - Resources ResourceConfig - SSHConfig SSHConfig - State Status + ConfigPath VMFile + ConnectionInfo ConnectionConfig + Created time.Time + Image ImageConfig + LastUp time.Time + Name string + Resources ResourceConfig + SSHConfig SSHConfig + State Status } func (rc RemoteConnectionType) MakeSSHURL(host, path, port, userName string) url.URL { @@ -286,11 +287,11 @@ func NewMachineFile(path string, symlink *string) (*VMFile, error) { // makeSymlink for macOS creates a symlink in $HOME/.podman/ // for a machinefile like a socket func (m *VMFile) makeSymlink(symlink *string) error { - homedir, err := os.UserHomeDir() + homeDir, err := os.UserHomeDir() if err != nil { return err } - sl := filepath.Join(homedir, ".podman", *symlink) + sl := filepath.Join(homeDir, ".podman", *symlink) // make the symlink dir and throw away if it already exists if err := os.MkdirAll(filepath.Dir(sl), 0700); err != nil && !errors2.Is(err, os.ErrNotExist) { return err @@ -335,3 +336,9 @@ type SSHConfig struct { // RemoteUsername of the vm user RemoteUsername string } + +// ConnectionConfig contains connections like sockets, etc. +type ConnectionConfig struct { + // PodmanSocket is the exported podman service socket + PodmanSocket *VMFile `json:"PodmanSocket"` +} diff --git a/pkg/machine/e2e/config.go b/pkg/machine/e2e/config.go index c17b840d3..248a2f0ad 100644 --- a/pkg/machine/e2e/config.go +++ b/pkg/machine/e2e/config.go @@ -85,6 +85,14 @@ func (ms *machineSession) outputToString() string { return strings.Join(fields, " ") } +// errorToString returns the error output from a session in string form +func (ms *machineSession) errorToString() string { + if ms == nil || ms.Err == nil || ms.Err.Contents() == nil { + return "" + } + return string(ms.Err.Contents()) +} + // newMB constructor for machine test builders func newMB() (*machineTestBuilder, error) { mb := machineTestBuilder{ diff --git a/pkg/machine/e2e/inspect_test.go b/pkg/machine/e2e/inspect_test.go index 2c9de5664..cdf13bb1a 100644 --- a/pkg/machine/e2e/inspect_test.go +++ b/pkg/machine/e2e/inspect_test.go @@ -2,6 +2,7 @@ package e2e import ( "encoding/json" + "strings" "github.com/containers/podman/v4/pkg/machine" "github.com/containers/podman/v4/pkg/machine/qemu" @@ -86,6 +87,7 @@ var _ = Describe("podman machine stop", func() { var inspectInfo []machine.InspectInfo err = jsoniter.Unmarshal(inspectSession.Bytes(), &inspectInfo) Expect(err).To(BeNil()) + Expect(strings.HasSuffix(inspectInfo[0].ConnectionInfo.PodmanSocket.GetPath(), "podman.sock")) inspect := new(inspectMachine) inspect = inspect.withFormat("{{.Name}}") diff --git a/pkg/machine/e2e/ssh_test.go b/pkg/machine/e2e/ssh_test.go index 155d39a64..9ee31ac26 100644 --- a/pkg/machine/e2e/ssh_test.go +++ b/pkg/machine/e2e/ssh_test.go @@ -56,5 +56,12 @@ var _ = Describe("podman machine ssh", func() { Expect(err).To(BeNil()) Expect(sshSession).To(Exit(0)) Expect(sshSession.outputToString()).To(ContainSubstring("Fedora CoreOS")) + + // keep exit code + sshSession, err = mb.setName(name).setCmd(ssh.withSSHComand([]string{"false"})).run() + Expect(err).To(BeNil()) + Expect(sshSession).To(Exit(1)) + Expect(sshSession.outputToString()).To(Equal("")) + Expect(sshSession.errorToString()).To(Equal("")) }) }) diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index e3fb3b970..b9f23662e 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -952,7 +952,8 @@ func (v *MachineVM) SSH(_ string, opts machine.SSHOptions) error { sshDestination := username + "@localhost" port := strconv.Itoa(v.Port) - args := []string{"-i", v.IdentityPath, "-p", port, sshDestination, "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=no"} + args := []string{"-i", v.IdentityPath, "-p", port, sshDestination, "-o", "UserKnownHostsFile=/dev/null", + "-o", "StrictHostKeyChecking=no", "-o", "LogLevel=ERROR"} if len(opts.Args) > 0 { args = append(args, opts.Args...) } else { @@ -1471,16 +1472,22 @@ func (v *MachineVM) Inspect() (*machine.InspectInfo, error) { if err != nil { return nil, err } - + connInfo := new(machine.ConnectionConfig) + podmanSocket, err := v.forwardSocketPath() + if err != nil { + return nil, err + } + connInfo.PodmanSocket = podmanSocket return &machine.InspectInfo{ - ConfigPath: v.ConfigPath, - Created: v.Created, - Image: v.ImageConfig, - LastUp: v.LastUp, - Name: v.Name, - Resources: v.ResourceConfig, - SSHConfig: v.SSHConfig, - State: state, + ConfigPath: v.ConfigPath, + ConnectionInfo: *connInfo, + Created: v.Created, + Image: v.ImageConfig, + LastUp: v.LastUp, + Name: v.Name, + Resources: v.ResourceConfig, + SSHConfig: v.SSHConfig, + State: state, }, nil } diff --git a/pkg/specgen/container_validate.go b/pkg/specgen/container_validate.go index 532a2094f..5616a4511 100644 --- a/pkg/specgen/container_validate.go +++ b/pkg/specgen/container_validate.go @@ -183,10 +183,12 @@ func (s *SpecGenerator) Validate() error { } // Set defaults if network info is not provided - if s.NetNS.NSMode == "" { - s.NetNS.NSMode = Bridge + // when we are rootless we default to slirp4netns + if s.NetNS.IsPrivate() || s.NetNS.IsDefault() { if rootless.IsRootless() { s.NetNS.NSMode = Slirp + } else { + s.NetNS.NSMode = Bridge } } if err := validateNetNS(&s.NetNS); err != nil { diff --git a/pkg/specgen/generate/namespaces.go b/pkg/specgen/generate/namespaces.go index 37d561ec2..4dd6b3eaf 100644 --- a/pkg/specgen/generate/namespaces.go +++ b/pkg/specgen/generate/namespaces.go @@ -236,10 +236,12 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod. toReturn = append(toReturn, libpod.WithCgroupsMode(s.CgroupsMode)) } - // Net - // TODO validate CNINetworks, StaticIP, StaticIPv6 are only set if we - // are in bridge mode. postConfigureNetNS := !s.UserNS.IsHost() + // when we are rootless we default to slirp4netns + if rootless.IsRootless() && (s.NetNS.IsPrivate() || s.NetNS.IsDefault()) { + s.NetNS.NSMode = specgen.Slirp + } + switch s.NetNS.NSMode { case specgen.FromPod: if pod == nil || infraCtr == nil { @@ -262,9 +264,7 @@ func namespaceOptions(s *specgen.SpecGenerator, rt *libpod.Runtime, pod *libpod. val = fmt.Sprintf("slirp4netns:%s", s.NetNS.Value) } toReturn = append(toReturn, libpod.WithNetNS(portMappings, expose, postConfigureNetNS, val, nil)) - case specgen.Private: - fallthrough - case specgen.Bridge: + case specgen.Bridge, specgen.Private, specgen.Default: portMappings, expose, err := createPortMappings(s, imageData) if err != nil { return nil, err diff --git a/pkg/specgen/generate/pod_create.go b/pkg/specgen/generate/pod_create.go index 5b7bb2b57..d4f281a11 100644 --- a/pkg/specgen/generate/pod_create.go +++ b/pkg/specgen/generate/pod_create.go @@ -141,6 +141,9 @@ func MapSpec(p *specgen.PodSpecGenerator) (*specgen.SpecGenerator, error) { case specgen.Bridge: p.InfraContainerSpec.NetNS.NSMode = specgen.Bridge logrus.Debugf("Pod using bridge network mode") + case specgen.Private: + p.InfraContainerSpec.NetNS.NSMode = specgen.Private + logrus.Debugf("Pod will use default network mode") case specgen.Host: logrus.Debugf("Pod will use host networking") if len(p.InfraContainerSpec.PortMappings) > 0 || @@ -151,15 +154,15 @@ func MapSpec(p *specgen.PodSpecGenerator) (*specgen.SpecGenerator, error) { p.InfraContainerSpec.NetNS.NSMode = specgen.Host case specgen.Slirp: logrus.Debugf("Pod will use slirp4netns") - if p.InfraContainerSpec.NetNS.NSMode != "host" { + if p.InfraContainerSpec.NetNS.NSMode != specgen.Host { p.InfraContainerSpec.NetworkOptions = p.NetworkOptions - p.InfraContainerSpec.NetNS.NSMode = specgen.NamespaceMode("slirp4netns") + p.InfraContainerSpec.NetNS.NSMode = specgen.Slirp } case specgen.NoNetwork: logrus.Debugf("Pod will not use networking") if len(p.InfraContainerSpec.PortMappings) > 0 || len(p.InfraContainerSpec.Networks) > 0 || - p.InfraContainerSpec.NetNS.NSMode == "host" { + p.InfraContainerSpec.NetNS.NSMode == specgen.Host { return nil, errors.Wrapf(define.ErrInvalidArg, "cannot disable pod network if network-related configuration is specified") } p.InfraContainerSpec.NetNS.NSMode = specgen.NoNetwork diff --git a/pkg/specgen/namespaces.go b/pkg/specgen/namespaces.go index 5a3b94ca4..f1343f6e2 100644 --- a/pkg/specgen/namespaces.go +++ b/pkg/specgen/namespaces.go @@ -10,7 +10,6 @@ import ( "github.com/containers/common/pkg/cgroups" cutil "github.com/containers/common/pkg/util" "github.com/containers/podman/v4/libpod/define" - "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/util" "github.com/containers/storage" spec "github.com/opencontainers/runtime-spec/specs-go" @@ -319,62 +318,6 @@ func ParseUserNamespace(ns string) (Namespace, error) { return ParseNamespace(ns) } -// ParseNetworkNamespace parses a network namespace specification in string -// form. -// Returns a namespace and (optionally) a list of CNI networks to join. -func ParseNetworkNamespace(ns string, rootlessDefaultCNI bool) (Namespace, map[string]types.PerNetworkOptions, error) { - toReturn := Namespace{} - networks := make(map[string]types.PerNetworkOptions) - // Net defaults to Slirp on rootless - switch { - case ns == string(Slirp), strings.HasPrefix(ns, string(Slirp)+":"): - toReturn.NSMode = Slirp - case ns == string(FromPod): - toReturn.NSMode = FromPod - case ns == "" || ns == string(Default) || ns == string(Private): - if rootless.IsRootless() { - if rootlessDefaultCNI { - toReturn.NSMode = Bridge - } else { - toReturn.NSMode = Slirp - } - } else { - toReturn.NSMode = Bridge - } - case ns == string(Bridge): - toReturn.NSMode = Bridge - case ns == string(NoNetwork): - toReturn.NSMode = NoNetwork - case ns == string(Host): - toReturn.NSMode = Host - case strings.HasPrefix(ns, "ns:"): - split := strings.SplitN(ns, ":", 2) - if len(split) != 2 { - return toReturn, nil, errors.Errorf("must provide a path to a namespace when specifying \"ns:\"") - } - toReturn.NSMode = Path - toReturn.Value = split[1] - case strings.HasPrefix(ns, string(FromContainer)+":"): - split := strings.SplitN(ns, ":", 2) - if len(split) != 2 { - return toReturn, nil, errors.Errorf("must provide name or ID or a container when specifying \"container:\"") - } - toReturn.NSMode = FromContainer - toReturn.Value = split[1] - default: - // Assume we have been given a list of CNI networks. - // Which only works in bridge mode, so set that. - networkList := strings.Split(ns, ",") - for _, net := range networkList { - networks[net] = types.PerNetworkOptions{} - } - - toReturn.NSMode = Bridge - } - - return toReturn, networks, nil -} - // ParseNetworkFlag parses a network string slice into the network options // If the input is nil or empty it will use the default setting from containers.conf func ParseNetworkFlag(networks []string) (Namespace, map[string]types.PerNetworkOptions, map[string][]string, error) { @@ -400,13 +343,7 @@ func ParseNetworkFlag(networks []string) (Namespace, map[string]types.PerNetwork case ns == string(FromPod): toReturn.NSMode = FromPod case ns == "" || ns == string(Default) || ns == string(Private): - // Net defaults to Slirp on rootless - if rootless.IsRootless() { - toReturn.NSMode = Slirp - break - } - // if root we use bridge - fallthrough + toReturn.NSMode = Private case ns == string(Bridge), strings.HasPrefix(ns, string(Bridge)+":"): toReturn.NSMode = Bridge parts := strings.SplitN(ns, ":", 2) diff --git a/pkg/specgen/namespaces_test.go b/pkg/specgen/namespaces_test.go index 368c92bd5..d03a6d032 100644 --- a/pkg/specgen/namespaces_test.go +++ b/pkg/specgen/namespaces_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/containers/common/libnetwork/types" - "github.com/containers/podman/v4/pkg/rootless" "github.com/stretchr/testify/assert" ) @@ -17,14 +16,6 @@ func parsMacNoErr(mac string) types.HardwareAddr { func TestParseNetworkFlag(t *testing.T) { // root and rootless have different defaults defaultNetName := "default" - defaultNetworks := map[string]types.PerNetworkOptions{ - defaultNetName: {}, - } - defaultNsMode := Namespace{NSMode: Bridge} - if rootless.IsRootless() { - defaultNsMode = Namespace{NSMode: Slirp} - defaultNetworks = map[string]types.PerNetworkOptions{} - } tests := []struct { name string @@ -37,26 +28,26 @@ func TestParseNetworkFlag(t *testing.T) { { name: "empty input", args: nil, - nsmode: defaultNsMode, - networks: defaultNetworks, + nsmode: Namespace{NSMode: Private}, + networks: map[string]types.PerNetworkOptions{}, }, { name: "empty string as input", args: []string{}, - nsmode: defaultNsMode, - networks: defaultNetworks, + nsmode: Namespace{NSMode: Private}, + networks: map[string]types.PerNetworkOptions{}, }, { name: "default mode", args: []string{"default"}, - nsmode: defaultNsMode, - networks: defaultNetworks, + nsmode: Namespace{NSMode: Private}, + networks: map[string]types.PerNetworkOptions{}, }, { name: "private mode", args: []string{"private"}, - nsmode: defaultNsMode, - networks: defaultNetworks, + nsmode: Namespace{NSMode: Private}, + networks: map[string]types.PerNetworkOptions{}, }, { name: "bridge mode", diff --git a/pkg/specgen/podspecgen.go b/pkg/specgen/podspecgen.go index 603506241..777097ac5 100644 --- a/pkg/specgen/podspecgen.go +++ b/pkg/specgen/podspecgen.go @@ -4,6 +4,7 @@ import ( "net" "github.com/containers/common/libnetwork/types" + storageTypes "github.com/containers/storage/types" spec "github.com/opencontainers/runtime-spec/specs-go" ) @@ -222,6 +223,10 @@ type PodResourceConfig struct { type PodSecurityConfig struct { SecurityOpt []string `json:"security_opt,omitempty"` + // IDMappings are UID and GID mappings that will be used by user + // namespaces. + // Required if UserNS is private. + IDMappings *storageTypes.IDMappingOptions `json:"idmappings,omitempty"` } // NewPodSpecGenerator creates a new pod spec diff --git a/pkg/specgenutil/createparse.go b/pkg/specgenutil/createparse.go index fb5f9c351..132f93771 100644 --- a/pkg/specgenutil/createparse.go +++ b/pkg/specgenutil/createparse.go @@ -18,20 +18,5 @@ func validate(c *entities.ContainerCreateOptions) error { return err } - var imageVolType = map[string]string{ - "bind": "", - "tmpfs": "", - "ignore": "", - } - if _, ok := imageVolType[c.ImageVolume]; !ok { - switch { - case c.IsInfra: - c.ImageVolume = "bind" - case c.IsClone: // the image volume type will be deduced later from the container we are cloning - return nil - default: - return errors.Errorf("invalid image-volume type %q. Pick one of bind, tmpfs, or ignore", c.ImageVolume) - } - } - return nil + return config.ValidateImageVolumeMode(c.ImageVolume) } diff --git a/pkg/specgenutil/specgen.go b/pkg/specgenutil/specgen.go index efaade9cd..6d70af106 100644 --- a/pkg/specgenutil/specgen.go +++ b/pkg/specgenutil/specgen.go @@ -229,9 +229,11 @@ func setNamespaces(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions) } func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions, args []string) error { - var ( - err error - ) + rtc, err := config.Default() + if err != nil { + return err + } + // validate flags as needed if err := validate(c); err != nil { return err @@ -479,8 +481,13 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions if len(s.HostUsers) == 0 || len(c.HostUsers) != 0 { s.HostUsers = c.HostUsers } - if len(s.ImageVolumeMode) == 0 || len(c.ImageVolume) != 0 { - s.ImageVolumeMode = c.ImageVolume + if len(c.ImageVolume) != 0 { + if len(s.ImageVolumeMode) == 0 { + s.ImageVolumeMode = c.ImageVolume + } + } + if len(s.ImageVolumeMode) == 0 { + s.ImageVolumeMode = rtc.Engine.ImageVolumeMode } if s.ImageVolumeMode == "bind" { s.ImageVolumeMode = "anonymous" @@ -550,11 +557,6 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions s.CgroupsMode = c.CgroupsMode } if s.CgroupsMode == "" { - rtc, err := config.Default() - if err != nil { - return err - } - s.CgroupsMode = rtc.Cgroups() } diff --git a/test/e2e/network_connect_disconnect_test.go b/test/e2e/network_connect_disconnect_test.go index a0716c84d..019bb4617 100644 --- a/test/e2e/network_connect_disconnect_test.go +++ b/test/e2e/network_connect_disconnect_test.go @@ -114,6 +114,11 @@ var _ = Describe("Podman network connect and disconnect", func() { exec3.WaitWithDefaultTimeout() Expect(exec3).Should(Exit(0)) Expect(strings.Contains(exec3.OutputToString(), ns)).To(BeFalse()) + + // make sure stats still works https://github.com/containers/podman/issues/13824 + stats := podmanTest.Podman([]string{"stats", "test", "--no-stream"}) + stats.WaitWithDefaultTimeout() + Expect(stats).Should(Exit(0)) }) It("bad network name in connect should result in error", func() { @@ -237,6 +242,11 @@ var _ = Describe("Podman network connect and disconnect", func() { Expect(exec3).Should(Exit(0)) Expect(strings.Contains(exec3.OutputToString(), ns)).To(BeTrue()) + // make sure stats works https://github.com/containers/podman/issues/13824 + stats := podmanTest.Podman([]string{"stats", "test", "--no-stream"}) + stats.WaitWithDefaultTimeout() + Expect(stats).Should(Exit(0)) + // make sure no logrus errors are shown https://github.com/containers/podman/issues/9602 rm := podmanTest.Podman([]string{"rm", "--time=0", "-f", "test"}) rm.WaitWithDefaultTimeout() diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats index 5b0460723..797883ec6 100644 --- a/test/system/160-volumes.bats +++ b/test/system/160-volumes.bats @@ -411,4 +411,43 @@ NeedsChown | true fi } +@test "podman --image-volume" { + tmpdir=$PODMAN_TMPDIR/volume-test + mkdir -p $tmpdir + containerfile=$tmpdir/Containerfile + cat >$containerfile <<EOF +FROM $IMAGE +VOLUME /data +EOF + fs=$(stat -f -c %T .) + run_podman build -t volume_image $tmpdir + + containersconf=$tmpdir/containers.conf + cat >$containersconf <<EOF +[engine] +image_volume_mode="tmpfs" +EOF + + run_podman run --image-volume tmpfs --rm volume_image stat -f -c %T /data + is "$output" "tmpfs" "Should be tmpfs" + + run_podman 1 run --image-volume ignore --rm volume_image stat -f -c %T /data + is "$output" "stat: can't read file system information for '/data': No such file or directory" "Should fail with /data does not exists" + + CONTAINERS_CONF="$containersconf" run_podman run --rm volume_image stat -f -c %T /data + is "$output" "tmpfs" "Should be tmpfs" + + CONTAINERS_CONF="$containersconf" run_podman run --image-volume bind --rm volume_image stat -f -c %T /data + assert "$output" != "tmpfs" "Should match hosts $fs" + + CONTAINERS_CONF="$containersconf" run_podman run --image-volume tmpfs --rm volume_image stat -f -c %T /data + is "$output" "tmpfs" "Should be tmpfs" + + CONTAINERS_CONF="$containersconf" run_podman 1 run --image-volume ignore --rm volume_image stat -f -c %T /data + is "$output" "stat: can't read file system information for '/data': No such file or directory" "Should fail with /data does not exists" + + run_podman rm --all --force -t 0 + run_podman image rm --force localhost/volume_image +} + # vim: filetype=sh diff --git a/test/system/170-run-userns.bats b/test/system/170-run-userns.bats index b80351902..46cb37b9d 100644 --- a/test/system/170-run-userns.bats +++ b/test/system/170-run-userns.bats @@ -38,10 +38,12 @@ function _require_crun() { @test "rootful pod with custom ID mapping" { skip_if_rootless "does not work rootless - rootful feature" - skip_if_remote "remote --uidmap is broken (see #14233)" random_pod_name=$(random_string 30) run_podman pod create --uidmap 0:200000:5000 --name=$random_pod_name run_podman pod start $random_pod_name + run_podman pod inspect --format '{{.InfraContainerID}}' $random_pod_name + run podman inspect --format '{{.HostConfig.IDMappings.UIDMap}}' $output + is "$output" ".*0:200000:5000" "UID Map Successful" # Remove the pod and the pause image run_podman pod rm $random_pod_name |