Merge pull request #14119 from openshift-cherrypick-robot/cherry-pick-14037-to-v4.1

[v4.1]  Report correct RemoteURI

10 files changed, 91 insertions, 95 deletions

diff --git a/cmd/podman/system/service_abi.go b/cmd/podman/system/service_abi.go
index f8abea3aa..9dc9de1c8 100644
--- a/cmd/podman/system/service_abi.go
+++ b/cmd/podman/system/service_abi.go
@@ -4,17 +4,18 @@
 package system
 
 import (
-	"context"
+	"fmt"
 	"net"
 	"net/url"
 	"os"
 	"path/filepath"
 
+	"github.com/containers/podman/v4/cmd/podman/registry"
 	api "github.com/containers/podman/v4/pkg/api/server"
 	"github.com/containers/podman/v4/pkg/domain/entities"
 	"github.com/containers/podman/v4/pkg/domain/infra"
 	"github.com/containers/podman/v4/pkg/servicereaper"
-	"github.com/containers/podman/v4/pkg/util"
+	"github.com/coreos/go-systemd/v22/activation"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
@@ -27,7 +28,26 @@ func restService(flags *pflag.FlagSet, cfg *entities.PodmanConfig, opts entities
 		err      error
 	)
 
-	if opts.URI != "" {
+	libpodRuntime, err := infra.GetRuntime(registry.Context(), flags, cfg)
+	if err != nil {
+		return err
+	}
+
+	if opts.URI == "" {
+		if _, found := os.LookupEnv("LISTEN_PID"); !found {
+			return errors.New("no service URI provided and socket activation protocol is not active")
+		}
+
+		listeners, err := activation.Listeners()
+		if err != nil {
+			return fmt.Errorf("cannot retrieve file descriptors from systemd: %w", err)
+		}
+		if len(listeners) != 1 {
+			return fmt.Errorf("wrong number of file descriptors for socket activation protocol (%d != 1)", len(listeners))
+		}
+		listener = listeners[0]
+		libpodRuntime.SetRemoteURI(listeners[0].Addr().String())
+	} else {
 		uri, err := url.Parse(opts.URI)
 		if err != nil {
 			return errors.Errorf("%s is an invalid socket destination", opts.URI)
@@ -39,7 +59,6 @@ func restService(flags *pflag.FlagSet, cfg *entities.PodmanConfig, opts entities
 			if err != nil {
 				return err
 			}
-			util.SetSocketPath(path)
 			if os.Getenv("LISTEN_FDS") != "" {
 				// If it is activated by systemd, use the first LISTEN_FD (3)
 				// instead of opening the socket file.
@@ -67,6 +86,7 @@ func restService(flags *pflag.FlagSet, cfg *entities.PodmanConfig, opts entities
 		default:
 			logrus.Debugf("Attempting API Service endpoint scheme %q", uri.Scheme)
 		}
+		libpodRuntime.SetRemoteURI(uri.String())
 	}
 
 	// Close stdin, so shortnames will not prompt
@@ -78,15 +98,10 @@ func restService(flags *pflag.FlagSet, cfg *entities.PodmanConfig, opts entities
 	if err := unix.Dup2(int(devNullfile.Fd()), int(os.Stdin.Fd())); err != nil {
 		return err
 	}
-	rt, err := infra.GetRuntime(context.Background(), flags, cfg)
-	if err != nil {
-		return err
-	}
 
 	servicereaper.Start()
-
-	infra.StartWatcher(rt)
-	server, err := api.NewServerWithSettings(rt, listener, opts)
+	infra.StartWatcher(libpodRuntime)
+	server, err := api.NewServerWithSettings(libpodRuntime, listener, opts)
 	if err != nil {
 		return err
 	}
diff --git a/libpod/runtime.go b/libpod/runtime.go
index 6c2323d88..78ff8ce94 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -1144,7 +1144,7 @@ func (r *Runtime) getVolumePlugin(name string) (*plugin.VolumePlugin, error) {
 	return plugin.GetVolumePlugin(name, pluginPath)
 }
 
-// GetSecretsStoreageDir returns the directory that the secrets manager should take
+// GetSecretsStorageDir returns the directory that the secrets manager should take
 func (r *Runtime) GetSecretsStorageDir() string {
 	return filepath.Join(r.store.GraphRoot(), "secrets")
 }
@@ -1192,7 +1192,17 @@ func (r *Runtime) Network() nettypes.ContainerNetwork {
 	return r.network
 }
 
-// Network returns the network interface which is used by the runtime
+// GetDefaultNetworkName returns the network interface which is used by the runtime
 func (r *Runtime) GetDefaultNetworkName() string {
 	return r.config.Network.DefaultNetwork
 }
+
+// RemoteURI returns the API server URI
+func (r *Runtime) RemoteURI() string {
+	return r.config.Engine.RemoteURI
+}
+
+// SetRemoteURI records the API server URI
+func (r *Runtime) SetRemoteURI(uri string) {
+	r.config.Engine.RemoteURI = uri
+}
diff --git a/pkg/api/server/server.go b/pkg/api/server/server.go
index a906a01f1..7f5537fb4 100644
--- a/pkg/api/server/server.go
+++ b/pkg/api/server/server.go
@@ -20,7 +20,6 @@ import (
 	"github.com/containers/podman/v4/pkg/api/server/idle"
 	"github.com/containers/podman/v4/pkg/api/types"
 	"github.com/containers/podman/v4/pkg/domain/entities"
-	"github.com/coreos/go-systemd/v22/activation"
 	"github.com/coreos/go-systemd/v22/daemon"
 	"github.com/gorilla/mux"
 	"github.com/gorilla/schema"
@@ -65,25 +64,7 @@ func NewServerWithSettings(runtime *libpod.Runtime, listener net.Listener, opts
 }
 
 func newServer(runtime *libpod.Runtime, listener net.Listener, opts entities.ServiceOptions) (*APIServer, error) {
-	// If listener not provided try socket activation protocol
-	if listener == nil {
-		if _, found := os.LookupEnv("LISTEN_PID"); !found {
-			return nil, fmt.Errorf("no service listener provided and socket activation protocol is not active")
-		}
-
-		listeners, err := activation.Listeners()
-		if err != nil {
-			return nil, fmt.Errorf("cannot retrieve file descriptors from systemd: %w", err)
-		}
-		if len(listeners) != 1 {
-			return nil, fmt.Errorf("wrong number of file descriptors for socket activation protocol (%d != 1)", len(listeners))
-		}
-		listener = listeners[0]
-		// note that activation.Listeners() return nil when it cannot listen on the fd (i.e. udp connection)
-		if listener == nil {
-			return nil, fmt.Errorf("unexpected fd received from systemd: cannot listen on it")
-		}
-	}
+	logrus.Infof("API service listening on %q. URI: %q", listener.Addr(), runtime.RemoteURI())
 	if opts.CorsHeaders == "" {
 		logrus.Debug("CORS Headers were not set")
 	} else {
diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go
index 8e96e4154..17df0e3f8 100644
--- a/pkg/domain/infra/abi/system.go
+++ b/pkg/domain/infra/abi/system.go
@@ -6,6 +6,7 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
+	"path/filepath"
 
 	"github.com/containers/common/pkg/cgroups"
 	"github.com/containers/common/pkg/config"
@@ -27,27 +28,40 @@ func (ic *ContainerEngine) Info(ctx context.Context) (*define.Info, error) {
 	if err != nil {
 		return nil, err
 	}
+	info.Host.RemoteSocket = &define.RemoteSocket{Path: ic.Libpod.RemoteURI()}
 
-	socketPath, err := util.SocketPath()
+	// `podman system connection add` invokes podman via ssh to fill in connection string. Here
+	// we are reporting the default systemd activation socket path as we cannot know if a future
+	// service may be run with another URI.
+	if ic.Libpod.RemoteURI() == "" {
+		xdg := "/run"
+		if path, err := util.GetRuntimeDir(); err != nil {
+			// Info is as good as we can guess...
+			return info, err
+		} else if path != "" {
+			xdg = path
+		}
+
+		uri := url.URL{
+			Scheme: "unix",
+			Path:   filepath.Join(xdg, "podman", "podman.sock"),
+		}
+		ic.Libpod.SetRemoteURI(uri.String())
+		info.Host.RemoteSocket.Path = uri.Path
+	}
+
+	uri, err := url.Parse(ic.Libpod.RemoteURI())
 	if err != nil {
 		return nil, err
 	}
-	rs := define.RemoteSocket{
-		Path:   socketPath,
-		Exists: false,
-	}
 
-	// Check if the socket exists
-	if fi, err := os.Stat(socketPath); err == nil {
-		if fi.Mode()&os.ModeSocket != 0 {
-			rs.Exists = true
-		}
+	if uri.Scheme == "unix" {
+		_, err := os.Stat(uri.Path)
+		info.Host.RemoteSocket.Exists = err == nil
+	} else {
+		info.Host.RemoteSocket.Exists = true
 	}
-	// TODO
-	// it was suggested future versions of this could perform
-	// a ping on the socket for greater confidence the socket is
-	// actually active.
-	info.Host.RemoteSocket = &rs
+
 	return info, err
 }
 
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 9842a0f73..a0bf8b50d 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -731,29 +731,6 @@ func IDtoolsToRuntimeSpec(idMaps []idtools.IDMap) (convertedIDMap []specs.LinuxI
 	return convertedIDMap
 }
 
-var socketPath string
-
-func SetSocketPath(path string) {
-	socketPath = path
-}
-
-func SocketPath() (string, error) {
-	if socketPath != "" {
-		return socketPath, nil
-	}
-	xdg, err := GetRuntimeDir()
-	if err != nil {
-		return "", err
-	}
-	if len(xdg) == 0 {
-		// If no xdg is returned, assume root socket
-		xdg = "/run"
-	}
-
-	// Glue the socket path together
-	return filepath.Join(xdg, "podman", "podman.sock"), nil
-}
-
 func LookupUser(name string) (*user.User, error) {
 	// Assume UID look up first, if it fails lookup by username
 	if u, err := user.LookupId(name); err == nil {
diff --git a/test/e2e/info_test.go b/test/e2e/info_test.go
index f989a9d29..2c2c82cb6 100644
--- a/test/e2e/info_test.go
+++ b/test/e2e/info_test.go
@@ -119,33 +119,31 @@ var _ = Describe("Podman Info", func() {
 		Expect(string(out)).To(Equal(expect))
 	})
 
-	It("podman info check RemoteSocket", func() {
+	It("check RemoteSocket ", func() {
 		session := podmanTest.Podman([]string{"info", "--format", "{{.Host.RemoteSocket.Path}}"})
 		session.WaitWithDefaultTimeout()
 		Expect(session).Should(Exit(0))
 		Expect(session.OutputToString()).To(MatchRegexp("/run/.*podman.*sock"))
 
-		if IsRemote() {
-			session = podmanTest.Podman([]string{"info", "--format", "{{.Host.RemoteSocket.Exists}}"})
-			session.WaitWithDefaultTimeout()
-			Expect(session).Should(Exit(0))
-			Expect(session.OutputToString()).To(ContainSubstring("true"))
+		session = podmanTest.Podman([]string{"info", "--format", "{{.Host.ServiceIsRemote}}"})
+		session.WaitWithDefaultTimeout()
+		Expect(session).Should(Exit(0))
+		if podmanTest.RemoteTest {
+			Expect(session.OutputToString()).To(Equal("true"))
+		} else {
+			Expect(session.OutputToString()).To(Equal("false"))
 		}
-	})
 
-	It("verify ServiceIsRemote", func() {
-		session := podmanTest.Podman([]string{"info", "--format", "{{.Host.ServiceIsRemote}}"})
+		session = podmanTest.Podman([]string{"info", "--format", "{{.Host.RemoteSocket.Exists}}"})
 		session.WaitWithDefaultTimeout()
-		Expect(session).To(Exit(0))
-
-		if podmanTest.RemoteTest {
+		Expect(session).Should(Exit(0))
+		if IsRemote() {
 			Expect(session.OutputToString()).To(ContainSubstring("true"))
-		} else {
-			Expect(session.OutputToString()).To(ContainSubstring("false"))
 		}
+
 	})
 
-	It("Podman info must contain cgroupControllers with ReleventControllers", func() {
+	It("Podman info must contain cgroupControllers with RelevantControllers", func() {
 		SkipIfRootless("Hard to tell which controllers are going to be enabled for rootless")
 		SkipIfRootlessCgroupsV1("Disable cgroups not supported on cgroupv1 for rootless users")
 		session := podmanTest.Podman([]string{"info", "--format", "{{.Host.CgroupControllers}}"})
diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go
index 0e91db04c..45414ec04 100644
--- a/test/e2e/play_kube_test.go
+++ b/test/e2e/play_kube_test.go
@@ -3180,8 +3180,10 @@ invalid kube kind
 		Expect(ls).Should(Exit(0))
 		Expect(ls.OutputToStringArray()).To(HaveLen(1))
 
-		containerLen := podmanTest.Podman([]string{"pod", "inspect", pod.Name, "--format", "'{{len .Containers}}'"})
-
+		containerLen := podmanTest.Podman([]string{"pod", "inspect", pod.Name, "--format", "{{len .Containers}}"})
+		containerLen.WaitWithDefaultTimeout()
+		Expect(containerLen).Should(Exit(0))
+		Expect(containerLen.OutputToString()).To(Equal("2"))
 		ctr01Name := "ctr01"
 		ctr02Name := "ctr02"
 
@@ -3199,7 +3201,7 @@ invalid kube kind
 		replace.WaitWithDefaultTimeout()
 		Expect(replace).Should(Exit(0))
 
-		newContainerLen := podmanTest.Podman([]string{"pod", "inspect", newPod.Name, "--format", "'{{len .Containers}}'"})
+		newContainerLen := podmanTest.Podman([]string{"pod", "inspect", newPod.Name, "--format", "{{len .Containers}}"})
 		newContainerLen.WaitWithDefaultTimeout()
 		Expect(newContainerLen).Should(Exit(0))
 		Expect(newContainerLen.OutputToString()).NotTo(Equal(containerLen.OutputToString()))
diff --git a/test/e2e/system_connection_test.go b/test/e2e/system_connection_test.go
index 95920136e..2228c23b2 100644
--- a/test/e2e/system_connection_test.go
+++ b/test/e2e/system_connection_test.go
@@ -247,7 +247,7 @@ var _ = Describe("podman system connection", func() {
 			// podman-remote commands will be executed by ginkgo directly.
 			SkipIfContainerized("sshd is not available when running in a container")
 			SkipIfRemote("connection heuristic requires both podman and podman-remote binaries")
-			SkipIfNotRootless("FIXME: setup ssh keys when root")
+			SkipIfNotRootless(fmt.Sprintf("FIXME: setup ssh keys when root. uid(%d) euid(%d)", os.Getuid(), os.Geteuid()))
 			SkipIfSystemdNotRunning("cannot test connection heuristic if systemd is not running")
 			SkipIfNotActive("sshd", "cannot test connection heuristic if sshd is not running")
 		})
diff --git a/test/e2e/system_service_test.go b/test/e2e/system_service_test.go
index 4c16ea788..398290426 100644
--- a/test/e2e/system_service_test.go
+++ b/test/e2e/system_service_test.go
@@ -20,7 +20,7 @@ var _ = Describe("podman system service", func() {
 
 	// The timeout used to for the service to respond. As shown in #12167,
 	// this may take some time on machines under high load.
-	var timeout = 20
+	var timeout = 30
 
 	BeforeEach(func() {
 		tempdir, err := CreateTempDirInTempDir()
diff --git a/test/system/272-system-connection.bats b/test/system/272-system-connection.bats
index 7b70f60f4..e9e9a01ea 100644
--- a/test/system/272-system-connection.bats
+++ b/test/system/272-system-connection.bats
@@ -99,10 +99,9 @@ $c2[ ]\+tcp://localhost:54321[ ]\+true" \
     _SERVICE_PID=$!
     wait_for_port localhost $_SERVICE_PORT
 
-    # FIXME: #12023, RemoteSocket is always /run/something
-#    run_podman info --format '{{.Host.RemoteSocket.Path}}'
-#    is "$output" "tcp:localhost:$_SERVICE_PORT" \
-#       "podman info works, and talks to the correct server"
+    _run_podman_remote info --format '{{.Host.RemoteSocket.Path}}'
+    is "$output" "tcp:localhost:$_SERVICE_PORT" \
+       "podman info works, and talks to the correct server"
 
     _run_podman_remote info --format '{{.Store.GraphRoot}}'
     is "$output" "${PODMAN_TMPDIR}/root" \