From 0b1c1ef461d26b3c373269c3a2e7085124f89eb5 Mon Sep 17 00:00:00 2001 From: Joseph Gooch Date: Fri, 19 Jun 2020 13:29:34 +0000 Subject: Implement --sdnotify cmdline option to control sd-notify behavior --sdnotify container|conmon|ignore With "conmon", we send the MAINPID, and clear the NOTIFY_SOCKET so the OCI runtime doesn't pass it into the container. We also advertise "ready" when the OCI runtime finishes to advertise the service as ready. With "container", we send the MAINPID, and leave the NOTIFY_SOCKET so the OCI runtime passes it into the container for initialization, and let the container advertise further metadata. This is the default, which is closest to the behavior podman has done in the past. The "ignore" option removes NOTIFY_SOCKET from the environment, so neither podman nor any child processes will talk to systemd. This removes the need for hardcoded CID and PID files in the command line, and the PIDFile directive, as the pid is advertised directly through sd-notify. Signed-off-by: Joseph Gooch --- libpod/container.go | 2 ++ libpod/container_internal.go | 14 ++++++++++++++ libpod/define/config.go | 7 +++++++ libpod/oci_conmon_exec_linux.go | 2 +- libpod/oci_conmon_linux.go | 30 ++++++++++++++++++++++++------ libpod/options.go | 22 ++++++++++++++++++++++ 6 files changed, 70 insertions(+), 7 deletions(-) (limited to 'libpod') diff --git a/libpod/container.go b/libpod/container.go index a71692dd8..fa90fef7a 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -414,6 +414,8 @@ type ContainerConfig struct { // sharing kernel namespaces in a pod IsInfra bool `json:"pause"` + // SdNotifyMode tells libpod what to do with a NOTIFY_SOCKET if passed + SdNotifyMode string `json:"sdnotifyMode,omitempty"` // Systemd tells libpod to setup the container in systemd mode Systemd bool `json:"systemd"` diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 7a547e565..12132cf41 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -24,6 +24,7 @@ import ( "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/idtools" "github.com/containers/storage/pkg/mount" + "github.com/coreos/go-systemd/v22/daemon" securejoin "github.com/cyphar/filepath-securejoin" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/generate" @@ -1192,6 +1193,19 @@ func (c *Container) start() error { c.state.State = define.ContainerStateRunning + if c.config.SdNotifyMode != define.SdNotifyModeIgnore { + payload := fmt.Sprintf("MAINPID=%d", c.state.ConmonPID) + if c.config.SdNotifyMode == define.SdNotifyModeConmon { + payload += "\n" + payload += daemon.SdNotifyReady + } + if sent, err := daemon.SdNotify(false, payload); err != nil { + logrus.Errorf("Error notifying systemd of Conmon PID: %s", err.Error()) + } else if sent { + logrus.Debugf("Notify sent successfully") + } + } + if c.config.HealthCheckConfig != nil { if err := c.updateHealthStatus(define.HealthCheckStarting); err != nil { logrus.Error(err) diff --git a/libpod/define/config.go b/libpod/define/config.go index 900a363d8..c43601554 100644 --- a/libpod/define/config.go +++ b/libpod/define/config.go @@ -75,3 +75,10 @@ const JSONLogging = "json-file" // NoLogging is the string conmon expects when specifying to use no log driver whatsoever const NoLogging = "none" + +// Strings used for --sdnotify option to podman +const ( + SdNotifyModeContainer = "container" + SdNotifyModeConmon = "conmon" + SdNotifyModeIgnore = "ignore" +) diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go index 1a6fe827b..4196bdcaf 100644 --- a/libpod/oci_conmon_exec_linux.go +++ b/libpod/oci_conmon_exec_linux.go @@ -444,7 +444,7 @@ func (r *ConmonOCIRuntime) startExec(c *Container, sessionID string, options *Ex // } // } - conmonEnv, extraFiles, err := r.configureConmonEnv(runtimeDir) + conmonEnv, extraFiles, err := r.configureConmonEnv(c, runtimeDir) if err != nil { return nil, nil, err } diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 98e436e78..534622382 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -30,6 +30,7 @@ import ( "github.com/containers/libpod/v2/utils" pmount "github.com/containers/storage/pkg/mount" "github.com/coreos/go-systemd/v22/activation" + "github.com/coreos/go-systemd/v22/daemon" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux" "github.com/opencontainers/selinux/go-selinux/label" @@ -365,8 +366,10 @@ func (r *ConmonOCIRuntime) StartContainer(ctr *Container) error { return err } env := []string{fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir)} - if notify, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { - env = append(env, fmt.Sprintf("NOTIFY_SOCKET=%s", notify)) + if ctr.config.SdNotifyMode == define.SdNotifyModeContainer { + if notify, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { + env = append(env, fmt.Sprintf("NOTIFY_SOCKET=%s", notify)) + } } if path, ok := os.LookupEnv("PATH"); ok { env = append(env, fmt.Sprintf("PATH=%s", path)) @@ -887,6 +890,12 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co } } + if ctr.config.SdNotifyMode == define.SdNotifyModeIgnore { + if err := os.Unsetenv("NOTIFY_SOCKET"); err != nil { + logrus.Warnf("Error unsetting NOTIFY_SOCKET %s", err.Error()) + } + } + args := r.sharedConmonArgs(ctr, ctr.ID(), ctr.bundlePath(), filepath.Join(ctr.state.RunDir, "pidfile"), ctr.LogPath(), r.exitsDir, ociLog, ctr.LogDriver(), logTag) if ctr.config.Spec.Process.Terminal { @@ -940,7 +949,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co } // 0, 1 and 2 are stdin, stdout and stderr - conmonEnv, envFiles, err := r.configureConmonEnv(runtimeDir) + conmonEnv, envFiles, err := r.configureConmonEnv(ctr, runtimeDir) if err != nil { return err } @@ -1034,6 +1043,13 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co // conmon not having a pid file is a valid state, so don't set it if we don't have it logrus.Infof("Got Conmon PID as %d", conmonPID) ctr.state.ConmonPID = conmonPID + if ctr.config.SdNotifyMode != define.SdNotifyModeIgnore { + if sent, err := daemon.SdNotify(false, fmt.Sprintf("MAINPID=%d", conmonPID)); err != nil { + logrus.Errorf("Error notifying systemd of Conmon PID: %s", err.Error()) + } else if sent { + logrus.Debugf("Notify MAINPID sent successfully") + } + } } if ctr.config.PreserveFDs > 0 { @@ -1137,7 +1153,7 @@ func prepareProcessExec(c *Container, cmd, env []string, tty bool, cwd, user, se // configureConmonEnv gets the environment values to add to conmon's exec struct // TODO this may want to be less hardcoded/more configurable in the future -func (r *ConmonOCIRuntime) configureConmonEnv(runtimeDir string) ([]string, []*os.File, error) { +func (r *ConmonOCIRuntime) configureConmonEnv(ctr *Container, runtimeDir string) ([]string, []*os.File, error) { env := make([]string, 0, 6) env = append(env, fmt.Sprintf("XDG_RUNTIME_DIR=%s", runtimeDir)) env = append(env, fmt.Sprintf("_CONTAINERS_USERNS_CONFIGURED=%s", os.Getenv("_CONTAINERS_USERNS_CONFIGURED"))) @@ -1149,8 +1165,10 @@ func (r *ConmonOCIRuntime) configureConmonEnv(runtimeDir string) ([]string, []*o env = append(env, fmt.Sprintf("HOME=%s", home)) extraFiles := make([]*os.File, 0) - if notify, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { - env = append(env, fmt.Sprintf("NOTIFY_SOCKET=%s", notify)) + if ctr.config.SdNotifyMode == define.SdNotifyModeContainer { + if notify, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { + env = append(env, fmt.Sprintf("NOTIFY_SOCKET=%s", notify)) + } } if !r.sdNotify { if listenfds, ok := os.LookupEnv("LISTEN_FDS"); ok { diff --git a/libpod/options.go b/libpod/options.go index c1a8fdbe1..61d1676f1 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -4,6 +4,7 @@ import ( "net" "os" "path/filepath" + "strings" "syscall" "github.com/containers/common/pkg/config" @@ -22,6 +23,10 @@ import ( ) // Runtime Creation Options +var ( + // SdNotifyModeValues describes the only values that SdNotifyMode can be + SdNotifyModeValues = []string{define.SdNotifyModeContainer, define.SdNotifyModeConmon, define.SdNotifyModeIgnore} +) // WithStorageConfig uses the given configuration to set up container storage. // If this is not specified, the system default configuration will be used @@ -550,6 +555,23 @@ func WithSystemd() CtrCreateOption { } } +// WithSdNotifyMode sets the sd-notify method +func WithSdNotifyMode(mode string) CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return define.ErrCtrFinalized + } + + // verify values + if len(mode) > 0 && !util.StringInSlice(strings.ToLower(mode), SdNotifyModeValues) { + return errors.Wrapf(define.ErrInvalidArg, "--sdnotify values must be one of %q", strings.Join(SdNotifyModeValues, ", ")) + } + + ctr.config.SdNotifyMode = mode + return nil + } +} + // WithShmSize sets the size of /dev/shm tmpfs mount. func WithShmSize(size int64) CtrCreateOption { return func(ctr *Container) error { -- cgit v1.2.3-54-g00ecf