From 6ea122f6e07f8b49f7a07884556ea697827a5052 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 27 Apr 2022 15:48:04 -0400 Subject: [PATCH] Report correct RemoteURI Rather than assuming a filesystem path, the API service URI is recorded in the libpod runtime configuration and then reported as requested. Note: All schemes other than "unix" are hard-coded to report URI exists. Fixes #12023 Signed-off-by: Jhon Honce Signed-off-by: Daniel J Walsh --- cmd/podman/system/service_abi.go | 37 +++++++++++++++------- libpod/runtime.go | 14 ++++++-- pkg/api/server/server.go | 21 +----------- pkg/domain/infra/abi/system.go | 44 +++++++++++++++++--------- pkg/util/utils.go | 23 -------------- test/e2e/info_test.go | 28 ++++++++-------- test/e2e/play_kube_test.go | 8 +++-- test/e2e/system_connection_test.go | 2 +- test/e2e/system_service_test.go | 2 +- test/system/272-system-connection.bats | 7 ++-- 10 files changed, 91 insertions(+), 95 deletions(-) diff --git a/cmd/podman/system/service_abi.go b/cmd/podman/system/service_abi.go index f8abea3aae..9dc9de1c85 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 6c2323d889..78ff8ce944 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 a906a01f14..7f5537fb4f 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 8e96e41549..17df0e3f8b 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 9842a0f734..a0bf8b50d1 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 f989a9d299..2c2c82cb6d 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 0e91db04c8..45414ec04d 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 95920136e4..2228c23b2c 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 4c16ea7880..3982904265 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 7b70f60f47..e9e9a01ea5 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" \