Merge pull request #18663 from Luap99/v4.5-backports

[v4.5] backport my fixes from the past few weeks

cmd/podman/networks/create.go

@@ -85,7 +85,7 @@ func networkCreateFlags(cmd *cobra.Command) {
 
 	flags.BoolVar(&networkCreateOptions.IgnoreIfExists, "ignore", false, "Don't fail if network already exists")
 	dnsserverFlagName := "dns"
-	flags.StringArrayVar(&networkCreateOptions.NetworkDNSServers, dnsserverFlagName, nil, "DNS servers this network will use")
+	flags.StringSliceVar(&networkCreateOptions.NetworkDNSServers, dnsserverFlagName, nil, "DNS servers this network will use")
 	_ = cmd.RegisterFlagCompletionFunc(dnsserverFlagName, completion.AutocompleteNone)
 }
 func init() {

cmd/podman/networks/update.go

@@ -31,9 +31,9 @@ func networkUpdateFlags(cmd *cobra.Command) {
 	flags := cmd.Flags()
 
 	addDNSServerFlagName := "dns-add"
-	flags.StringArrayVar(&networkUpdateOptions.AddDNSServers, addDNSServerFlagName, nil, "add network level nameservers")
+	flags.StringSliceVar(&networkUpdateOptions.AddDNSServers, addDNSServerFlagName, nil, "add network level nameservers")
 	removeDNSServerFlagName := "dns-drop"
-	flags.StringArrayVar(&networkUpdateOptions.RemoveDNSServers, removeDNSServerFlagName, nil, "remove network level nameservers")
+	flags.StringSliceVar(&networkUpdateOptions.RemoveDNSServers, removeDNSServerFlagName, nil, "remove network level nameservers")
 	_ = cmd.RegisterFlagCompletionFunc(addDNSServerFlagName, completion.AutocompleteNone)
 	_ = cmd.RegisterFlagCompletionFunc(removeDNSServerFlagName, completion.AutocompleteNone)
 }

libpod/container.go

@@ -882,6 +882,17 @@ func (c *Container) execSessionNoCopy(id string) (*ExecSession, error) {
 		return nil, fmt.Errorf("no exec session with ID %s found in container %s: %w", id, c.ID(), define.ErrNoSuchExecSession)
 	}
 
+	// make sure to update the exec session if needed #18424
+	alive, err := c.ociRuntime.ExecUpdateStatus(c, id)
+	if err != nil {
+		return nil, err
+	}
+	if !alive {
+		if err := retrieveAndWriteExecExitCode(c, session.ID()); err != nil {
+			return nil, err
+		}
+	}
+
 	return session, nil
 }
 

libpod/container_api.go

@@ -254,14 +254,6 @@ func (c *Container) StopWithTimeout(timeout uint) (finalErr error) {
 		}
 	}
 
-	if c.ensureState(define.ContainerStateStopped, define.ContainerStateExited) {
-		return define.ErrCtrStopped
-	}
-
-	if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopping) {
-		return fmt.Errorf("can only stop created or running containers. %s is in state %s: %w", c.ID(), c.state.State.String(), define.ErrCtrStateInvalid)
-	}
-
 	return c.stop(timeout)
 }
 

libpod/container_internal.go

@@ -238,6 +238,11 @@ func (c *Container) shouldRestart() bool {
 		}
 	}
 
+	// Explicitly stopped by user, do not restart again.
+	if c.state.StoppedByUser {
+		return false
+	}
+
 	// If we did not get a restart policy match, return false
 	// Do the same if we're not a policy that restarts.
 	if !c.state.RestartPolicyMatch ||
@@ -1307,15 +1312,38 @@ func (c *Container) stop(timeout uint) error {
 		}
 	}
 
-	// Set the container state to "stopping" and unlock the container
-	// before handing it over to conmon to unblock other commands.  #8501
-	// demonstrates nicely that a high stop timeout will block even simple
-	// commands such as `podman ps` from progressing if the container lock
-	// is held when busy-waiting for the container to be stopped.
+	// OK, the following code looks a bit weird but we have to make sure we can stop
+	// containers with the restart policy always, to do this we have to set
+	// StoppedByUser even when there is nothing to stop right now. This is due to the
+	// cleanup process waiting on the container lock and then afterwards restarts it.
+	// shouldRestart() then checks for StoppedByUser and does not restart it.
+	// https://github.com/containers/podman/issues/18259
+	var cannotStopErr error
+	if c.ensureState(define.ContainerStateStopped, define.ContainerStateExited) {
+		cannotStopErr = define.ErrCtrStopped
+	} else if !c.ensureState(define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopping) {
+		cannotStopErr = fmt.Errorf("can only stop created or running containers. %s is in state %s: %w", c.ID(), c.state.State.String(), define.ErrCtrStateInvalid)
+	}
+
 	c.state.StoppedByUser = true
-	c.state.State = define.ContainerStateStopping
+	if cannotStopErr == nil {
+		// Set the container state to "stopping" and unlock the container
+		// before handing it over to conmon to unblock other commands.  #8501
+		// demonstrates nicely that a high stop timeout will block even simple
+		// commands such as `podman ps` from progressing if the container lock
+		// is held when busy-waiting for the container to be stopped.
+		c.state.State = define.ContainerStateStopping
+	}
 	if err := c.save(); err != nil {
-		return fmt.Errorf("saving container %s state before stopping: %w", c.ID(), err)
+		rErr := fmt.Errorf("saving container %s state before stopping: %w", c.ID(), err)
+		if cannotStopErr == nil {
+			return rErr
+		}
+		// we return below with cannotStopErr
+		logrus.Error(rErr)
+	}
+	if cannotStopErr != nil {
+		return cannotStopErr
 	}
 	if !c.batched {
 		c.lock.Unlock()

libpod/networking_common.go

@@ -115,7 +115,7 @@ func (r *Runtime) teardownNetworkBackend(ns string, opts types.NetworkOptions) e
 		// execute the network setup in the rootless net ns
 		err = rootlessNetNS.Do(tearDownPod)
 		if cerr := rootlessNetNS.Cleanup(r); cerr != nil {
-			logrus.WithError(err).Error("failed to clean up rootless netns")
+			logrus.WithError(cerr).Error("failed to clean up rootless netns")
 		}
 		rootlessNetNS.Lock.Unlock()
 	} else {

libpod/networking_linux.go

@@ -592,6 +592,14 @@ func (r *Runtime) configureNetNS(ctr *Container, ctrNS string) (status map[strin
 	if err != nil {
 		return nil, err
 	}
+	defer func() {
+		// do not forget to tear down the netns when a later error happened.
+		if rerr != nil {
+			if err := r.teardownNetworkBackend(ctrNS, netOpts); err != nil {
+				logrus.Warnf("failed to teardown network after failed setup: %v", err)
+			}
+		}
+	}()
 
 	// set up rootless port forwarder when rootless with ports and the network status is empty,
 	// if this is called from network reload the network status will not be empty and we should

pkg/api/handlers/compat/containers_create.go

@@ -483,7 +483,7 @@ func cliOpts(cc handlers.CreateContainerConfig, rtc *config.Config) (*entities.C
 	// This also handles volumes duplicated between cc.HostConfig.Mounts and
 	// cc.Volumes, as seen in compose v2.0.
 	for vol := range cc.Volumes {
-		if _, ok := volDestinations[filepath.Clean(vol)]; ok {
+		if _, ok := volDestinations[vol]; ok {
 			continue
 		}
 		cliOpts.Volume = append(cliOpts.Volume, vol)

pkg/api/handlers/compat/images.go

@@ -181,7 +181,8 @@ func CreateImageFromSrc(w http.ResponseWriter, r *http.Request) {
 		FromSrc  string   `schema:"fromSrc"`
 		Message  string   `schema:"message"`
 		Platform string   `schema:"platform"`
-		Repo     string   `shchema:"repo"`
+		Repo     string   `schema:"repo"`
+		Tag      string   `schema:"tag"`
 	}{
 		// This is where you can override the golang default value for one of fields
 	}
@@ -208,7 +209,7 @@ func CreateImageFromSrc(w http.ResponseWriter, r *http.Request) {
 
 	reference := query.Repo
 	if query.Repo != "" {
-		possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, reference)
+		possiblyNormalizedName, err := utils.NormalizeToDockerHub(r, mergeNameAndTagOrDigest(reference, query.Tag))
 		if err != nil {
 			utils.Error(w, http.StatusInternalServerError, fmt.Errorf("normalizing image: %w", err))
 			return

pkg/bindings/containers/logs.go

@@ -35,6 +35,11 @@ func Logs(ctx context.Context, nameOrID string, options *LogOptions, stdoutChan,
 	}
 	defer response.Body.Close()
 
+	// if not success handle and return possible error message
+	if !(response.IsSuccess() || response.IsInformational()) {
+		return response.Process(nil)
+	}
+
 	buffer := make([]byte, 1024)
 	for {
 		fd, l, err := DemuxHeader(response.Body, buffer)

pkg/bindings/test/exec_test.go

@@ -30,7 +30,7 @@ var _ = Describe("Podman containers exec", func() {
 		bt.cleanup()
 	})
 
-	It("Podman exec create makes an exec session", func() {
+	It("Podman exec create+start makes an exec session", func() {
 		name := "testCtr"
 		cid, err := bt.RunTopContainer(&name, nil)
 		Expect(err).ToNot(HaveOccurred())
@@ -48,6 +48,15 @@ var _ = Describe("Podman containers exec", func() {
 		Expect(inspectOut.ProcessConfig.Entrypoint).To(Equal("echo"))
 		Expect(inspectOut.ProcessConfig.Arguments).To(HaveLen(1))
 		Expect(inspectOut.ProcessConfig.Arguments[0]).To(Equal("hello world"))
+
+		err = containers.ExecStart(bt.conn, sessionID, nil)
+		Expect(err).ToNot(HaveOccurred())
+
+		inspectOut, err = containers.ExecInspect(bt.conn, sessionID, nil)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(inspectOut.ContainerID).To(Equal(cid))
+		Expect(inspectOut.Running).To(BeFalse(), "session should not be running")
+		Expect(inspectOut.ExitCode).To(Equal(0), "exit code from echo")
 	})
 
 	It("Podman exec create with bad command fails", func() {

pkg/domain/infra/tunnel/images.go

@@ -279,10 +279,19 @@ func (ir *ImageEngine) Save(ctx context.Context, nameOrID string, tags []string,
 			defer func() { _ = os.Remove(f.Name()) }()
 		}
 	default:
-		// This code was added to allow for opening stdout replacing
-		// os.Create(opts.Output) which was attempting to open the file
-		// for read/write which fails on Darwin platforms
-		f, err = os.OpenFile(opts.Output, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
+		// This is ugly but I think the best we can do for now,
+		// on windows there is no /dev/stdout but the save command defaults to /dev/stdout.
+		// The proper thing to do would be to pass an io.Writer down from the cli frontend
+		// but since the local save API does not support an io.Writer this is impossible.
+		// I reported it a while ago in https://github.com/containers/common/issues/1275
+		if opts.Output == "/dev/stdout" {
+			f = os.Stdout
+		} else {
+			// This code was added to allow for opening stdout replacing
+			// os.Create(opts.Output) which was attempting to open the file
+			// for read/write which fails on Darwin platforms
+			f, err = os.OpenFile(opts.Output, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
+		}
 	}
 	if err != nil {
 		return err

pkg/machine/connection.go

@@ -10,6 +10,8 @@ import (
 	"github.com/containers/common/pkg/config"
 )
 
+const LocalhostIP = "127.0.0.1"
+
 func AddConnection(uri fmt.Stringer, name, identity string, isDefault bool) error {
 	if len(identity) < 1 {
 		return errors.New("identity must be defined")

pkg/machine/hyperv/machine.go

@@ -99,8 +99,8 @@ func (m *HyperVMachine) Init(opts machine.InitOptions) (bool, error) {
 	m.IdentityPath = filepath.Join(sshDir, m.Name)
 
 	if len(opts.IgnitionPath) < 1 {
-		uri := machine.SSHRemoteConnection.MakeSSHURL("localhost", fmt.Sprintf("/run/user/%d/podman/podman.sock", m.UID), strconv.Itoa(m.Port), m.RemoteUsername)
-		uriRoot := machine.SSHRemoteConnection.MakeSSHURL("localhost", "/run/podman/podman.sock", strconv.Itoa(m.Port), "root")
+		uri := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, fmt.Sprintf("/run/user/%d/podman/podman.sock", m.UID), strconv.Itoa(m.Port), m.RemoteUsername)
+		uriRoot := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, "/run/podman/podman.sock", strconv.Itoa(m.Port), "root")
 		identity := filepath.Join(sshDir, m.Name)
 
 		uris := []url.URL{uri, uriRoot}

pkg/machine/qemu/machine.go

@@ -328,8 +328,8 @@ func (v *MachineVM) Init(opts machine.InitOptions) (bool, error) {
 	v.CmdLine = append(v.CmdLine, "-drive", "if=virtio,file="+v.getImageFile())
 	// This kind of stinks but no other way around this r/n
 	if len(opts.IgnitionPath) < 1 {
-		uri := machine.SSHRemoteConnection.MakeSSHURL("localhost", fmt.Sprintf("/run/user/%d/podman/podman.sock", v.UID), strconv.Itoa(v.Port), v.RemoteUsername)
-		uriRoot := machine.SSHRemoteConnection.MakeSSHURL("localhost", "/run/podman/podman.sock", strconv.Itoa(v.Port), "root")
+		uri := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, fmt.Sprintf("/run/user/%d/podman/podman.sock", v.UID), strconv.Itoa(v.Port), v.RemoteUsername)
+		uriRoot := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, "/run/podman/podman.sock", strconv.Itoa(v.Port), "root")
 		identity := filepath.Join(sshDir, v.Name)
 
 		uris := []url.URL{uri, uriRoot}
@@ -940,13 +940,6 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func()
 	files = append(files, socketPath.Path)
 	files = append(files, v.archRemovalFiles()...)
 
-	if err := machine.RemoveConnection(v.Name); err != nil {
-		logrus.Error(err)
-	}
-	if err := machine.RemoveConnection(v.Name + "-root"); err != nil {
-		logrus.Error(err)
-	}
-
 	vmConfigDir, err := machine.GetConfDir(vmtype)
 	if err != nil {
 		return "", nil, err
@@ -977,6 +970,12 @@ func (v *MachineVM) Remove(_ string, opts machine.RemoveOptions) (string, func()
 				logrus.Error(err)
 			}
 		}
+		if err := machine.RemoveConnection(v.Name); err != nil {
+			logrus.Error(err)
+		}
+		if err := machine.RemoveConnection(v.Name + "-root"); err != nil {
+			logrus.Error(err)
+		}
 		return nil
 	}, nil
 }

pkg/machine/wsl/machine.go

@@ -474,8 +474,8 @@ func (v *MachineVM) writeConfig() error {
 }
 
 func setupConnections(v *MachineVM, opts machine.InitOptions, sshDir string) error {
-	uri := machine.SSHRemoteConnection.MakeSSHURL("localhost", "/run/user/1000/podman/podman.sock", strconv.Itoa(v.Port), v.RemoteUsername)
-	uriRoot := machine.SSHRemoteConnection.MakeSSHURL("localhost", "/run/podman/podman.sock", strconv.Itoa(v.Port), "root")
+	uri := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, "/run/user/1000/podman/podman.sock", strconv.Itoa(v.Port), v.RemoteUsername)
+	uriRoot := machine.SSHRemoteConnection.MakeSSHURL(machine.LocalhostIP, "/run/podman/podman.sock", strconv.Itoa(v.Port), "root")
 	identity := filepath.Join(sshDir, v.Name)
 
 	uris := []url.URL{uri, uriRoot}

test/apiv2/10-images.at

@@ -66,6 +66,13 @@ podman untag docker.io/library/alpine:latest
 
 t POST "images/create?fromImage=quay.io/libpod/alpine&tag=sha256:fa93b01658e3a5a1686dc3ae55f170d8de487006fb53a28efcd12ab0710a2e5f" 200
 
+# create image from source with tag
+# Note the "-" is used to use an empty body and not "{}" which is the default.
+t POST "images/create?fromSrc=-&repo=myimage&tag=mytag" - 200
+t GET "images/myimage:mytag/json" 200 \
+  .Id~'^sha256:[0-9a-f]\{64\}$' \
+  .RepoTags[0]="docker.io/library/myimage:mytag"
+
 # Display the image history
 t GET libpod/images/nonesuch/history 404
 

test/apiv2/20-containers.at

@@ -357,6 +357,15 @@ t GET containers/$cid/json 200 \
 
 t DELETE containers/$cid?v=true 204
 
+# Test Volumes with bind mount, for some reason docker-py sets this #18454
+t POST containers/create Image=$IMAGE Volumes='{"/test/":{}}'  HostConfig='{"Binds":["/tmp:/test/:ro"]}'  201 \
+  .Id~[0-9a-f]\\{64\\}
+cid=$(jq -r '.Id' <<<"$output")
+t GET containers/$cid/json 200 \
+  .Mounts[0].Destination="/test/"
+
+t DELETE containers/$cid?v=true 204
+
 # test port mapping
 podman run -d --rm --name bar -p 8080:9090 $IMAGE top
 

test/apiv2/test-apiv2

@@ -255,6 +255,10 @@ function t() {
 
         for arg; do
             case "$arg" in
+                # This is just some hack to avoid adding `-d {}` to curl for endpoints where we really need an empty body.
+                # --disable makes curl not lookup the curlrc file, it't should't effect the tests in any way.
+                -)                curl_args+=(--disable);
+                                  shift;;
                 *=*)              post_args+=("$arg");
                                   shift;;
                 *.json)           _add_curl_args $arg;

test/e2e/logs_test.go

@@ -47,6 +47,13 @@ var _ = Describe("Podman logs", func() {
 
 	})
 
+	It("podman logs on not existent container", func() {
+		results := podmanTest.Podman([]string{"logs", "notexist"})
+		results.WaitWithDefaultTimeout()
+		Expect(results).To(Exit(125))
+		Expect(results.ErrorToString()).To(Equal(`Error: no container with name or ID "notexist" found: no such container`))
+	})
+
 	for _, log := range []string{"k8s-file", "journald", "json-file"} {
 		// This is important to move the 'log' var to the correct scope under Ginkgo flow.
 		log := log