From 3fc126e152d5ebe4bfef980dea04192762628773 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Aug 2022 14:22:54 +0200 Subject: [PATCH] libpod: allow the notify socket to be passed programatically The notify socket can now either be specified via an environment variable or programatically (where the env is ignored). The notify mode and the socket are now also displayed in `container inspect` which comes in handy for debugging and allows for propper testing. Signed-off-by: Valentin Rothberg --- libpod/container.go | 4 ---- libpod/container_config.go | 2 ++ libpod/container_inspect.go | 2 ++ libpod/container_internal.go | 5 +++-- libpod/container_internal_linux.go | 5 +---- libpod/define/container_inspect.go | 4 ++++ libpod/oci_conmon_linux.go | 11 +++++------ libpod/options.go | 11 +++++++++++ pkg/specgen/generate/container_create.go | 5 +++++ test/system/260-sdnotify.bats | 14 +++++++++++++- 10 files changed, 46 insertions(+), 17 deletions(-) diff --git a/libpod/container.go b/libpod/container.go index 4e2d938603..6c05b10848 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -124,10 +124,6 @@ type Container struct { // This is true if a container is restored from a checkpoint. restoreFromCheckpoint bool - // Used to query the NOTIFY_SOCKET once along with setting up - // mounts etc. - notifySocket string - slirp4netnsSubnet *net.IPNet } diff --git a/libpod/container_config.go b/libpod/container_config.go index 544c45a8cc..bd98166517 100644 --- a/libpod/container_config.go +++ b/libpod/container_config.go @@ -386,6 +386,8 @@ type ContainerMiscConfig struct { IsService bool `json:"isService"` // SdNotifyMode tells libpod what to do with a NOTIFY_SOCKET if passed SdNotifyMode string `json:"sdnotifyMode,omitempty"` + // SdNotifySocket stores NOTIFY_SOCKET in use by the container + SdNotifySocket string `json:"sdnotifySocket,omitempty"` // Systemd tells libpod to set up the container in systemd mode, a value of nil denotes false Systemd *bool `json:"systemd,omitempty"` // HealthCheckConfig has the health check command and related timings diff --git a/libpod/container_inspect.go b/libpod/container_inspect.go index fa2130a283..5e2ab28180 100644 --- a/libpod/container_inspect.go +++ b/libpod/container_inspect.go @@ -414,6 +414,8 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) *define.Insp ctrConfig.Passwd = c.config.Passwd ctrConfig.ChrootDirs = append(ctrConfig.ChrootDirs, c.config.ChrootDirs...) + ctrConfig.SdNotifyMode = c.config.SdNotifyMode + ctrConfig.SdNotifySocket = c.config.SdNotifySocket return ctrConfig } diff --git a/libpod/container_internal.go b/libpod/container_internal.go index bad68991bb..7cef067b08 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -31,6 +31,7 @@ import ( "github.com/containers/podman/v4/pkg/lookup" "github.com/containers/podman/v4/pkg/rootless" "github.com/containers/podman/v4/pkg/selinux" + "github.com/containers/podman/v4/pkg/systemd/notifyproxy" "github.com/containers/podman/v4/pkg/util" "github.com/containers/storage" "github.com/containers/storage/pkg/archive" @@ -1224,9 +1225,9 @@ func (c *Container) start() error { payload += "\n" payload += daemon.SdNotifyReady } - if sent, err := daemon.SdNotify(false, payload); err != nil { + if err := notifyproxy.SendMessage(c.config.SdNotifySocket, payload); err != nil { logrus.Errorf("Notifying systemd of Conmon PID: %s", err.Error()) - } else if sent { + } else { logrus.Debugf("Notify sent successfully") } } diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index a131ab3674..c4f83b5716 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -969,12 +969,9 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { // and if the sdnotify mode is set to container. It also sets c.notifySocket // to avoid redundantly looking up the env variable. func (c *Container) mountNotifySocket(g generate.Generator) error { - notify, ok := os.LookupEnv("NOTIFY_SOCKET") - if !ok { + if c.config.SdNotifySocket == "" { return nil } - c.notifySocket = notify - if c.config.SdNotifyMode != define.SdNotifyModeContainer { return nil } diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go index e6a34ba616..5982d684c5 100644 --- a/libpod/define/container_inspect.go +++ b/libpod/define/container_inspect.go @@ -79,6 +79,10 @@ type InspectContainerConfig struct { // treated as root directories. Standard bind mounts will be mounted // into paths relative to these directories. ChrootDirs []string `json:"ChrootDirs,omitempty"` + // SdNotifyMode is the sd-notify mode of the container. + SdNotifyMode string `json:"sdNotifyMode,omitempty"` + // SdNotifySocket is the NOTIFY_SOCKET in use by/configured for the container. + SdNotifySocket string `json:"sdNotifySocket,omitempty"` } // InspectRestartPolicy holds information about the container's restart policy. diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index cb76de72c2..1b654ed33c 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -1062,8 +1062,8 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co args := r.sharedConmonArgs(ctr, ctr.ID(), ctr.bundlePath(), pidfile, ctr.LogPath(), r.exitsDir, ociLog, ctr.LogDriver(), logTag) - if ctr.config.SdNotifyMode == define.SdNotifyModeContainer && ctr.notifySocket != "" { - args = append(args, fmt.Sprintf("--sdnotify-socket=%s", ctr.notifySocket)) + if ctr.config.SdNotifyMode == define.SdNotifyModeContainer && ctr.config.SdNotifySocket != "" { + args = append(args, fmt.Sprintf("--sdnotify-socket=%s", ctr.config.SdNotifySocket)) } if ctr.config.Spec.Process.Terminal { @@ -1391,14 +1391,13 @@ func startCommand(cmd *exec.Cmd, ctr *Container) error { // Make sure to unset the NOTIFY_SOCKET and reset it afterwards if needed. switch ctr.config.SdNotifyMode { case define.SdNotifyModeContainer, define.SdNotifyModeIgnore: - if ctr.notifySocket != "" { + if prev := os.Getenv("NOTIFY_SOCKET"); prev != "" { if err := os.Unsetenv("NOTIFY_SOCKET"); err != nil { logrus.Warnf("Error unsetting NOTIFY_SOCKET %v", err) } - defer func() { - if err := os.Setenv("NOTIFY_SOCKET", ctr.notifySocket); err != nil { - logrus.Errorf("Resetting NOTIFY_SOCKET=%s", ctr.notifySocket) + if err := os.Setenv("NOTIFY_SOCKET", prev); err != nil { + logrus.Errorf("Resetting NOTIFY_SOCKET=%s", prev) } }() } diff --git a/libpod/options.go b/libpod/options.go index b31cb4ab24..933c9a1c37 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -613,6 +613,17 @@ func WithSystemd() CtrCreateOption { } } +// WithSdNotifySocket sets the sd-notify of the container +func WithSdNotifySocket(socketPath string) CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return define.ErrCtrFinalized + } + ctr.config.SdNotifySocket = socketPath + return nil + } +} + // WithSdNotifyMode sets the sd-notify method func WithSdNotifyMode(mode string) CtrCreateOption { return func(ctr *Container) error { diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index 8334d386f5..f4c67e534e 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "os" "path/filepath" "strings" @@ -353,6 +354,10 @@ func createContainerOptions(rt *libpod.Runtime, s *specgen.SpecGenerator, pod *l if len(s.SdNotifyMode) > 0 { options = append(options, libpod.WithSdNotifyMode(s.SdNotifyMode)) } + if notify, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { + options = append(options, libpod.WithSdNotifySocket(notify)) + } + if pod != nil { logrus.Debugf("adding container to pod %s", pod.Name()) options = append(options, rt.WithPod(pod)) diff --git a/test/system/260-sdnotify.bats b/test/system/260-sdnotify.bats index cd7b1262a8..fe442f57d3 100644 --- a/test/system/260-sdnotify.bats +++ b/test/system/260-sdnotify.bats @@ -88,7 +88,13 @@ function _assert_mainpid_is_conmon() { export NOTIFY_SOCKET=$PODMAN_TMPDIR/ignore.sock _start_socat - run_podman 1 run --rm --sdnotify=ignore $IMAGE printenv NOTIFY_SOCKET + run_podman create --rm --sdnotify=ignore $IMAGE printenv NOTIFY_SOCKET + cid="$output" + + run_podman container inspect $cid --format "{{.Config.SdNotifyMode}} {{.Config.SdNotifySocket}}" + is "$output" "ignore $NOTIFY_SOCKET" + + run_podman 1 start --attach $cid is "$output" "" "\$NOTIFY_SOCKET in container" is "$(< $_SOCAT_LOG)" "" "nothing received on socket" @@ -106,6 +112,9 @@ function _assert_mainpid_is_conmon() { cid="$output" wait_for_ready $cid + run_podman container inspect $cid --format "{{.Config.SdNotifyMode}} {{.Config.SdNotifySocket}}" + is "$output" "conmon $NOTIFY_SOCKET" + run_podman container inspect sdnotify_conmon_c --format "{{.State.ConmonPid}}" mainPID="$output" @@ -151,6 +160,9 @@ READY=1" "sdnotify sent MAINPID and READY" cid="$output" wait_for_ready $cid + run_podman container inspect $cid --format "{{.Config.SdNotifyMode}} {{.Config.SdNotifySocket}}" + is "$output" "container $NOTIFY_SOCKET" + run_podman logs $cid is "${lines[0]}" "/run/notify/notify.sock" "NOTIFY_SOCKET is passed to container"