diff options
author | Valentin Rothberg <vrothberg@redhat.com> | 2022-05-12 11:11:37 +0200 |
---|---|---|
committer | Valentin Rothberg <vrothberg@redhat.com> | 2022-05-12 11:11:37 +0200 |
commit | 03af8213cea25036788d9c9db33e748b7985d590 (patch) | |
tree | 4d71c0b0821ce0e0597454b96184d01b9d1887a5 | |
parent | f65e13eb7aa5874faad579bee2ac16170d467845 (diff) | |
download | podman-03af8213cea25036788d9c9db33e748b7985d590.tar.gz podman-03af8213cea25036788d9c9db33e748b7985d590.tar.bz2 podman-03af8213cea25036788d9c9db33e748b7985d590.zip |
sdnotify: send MAINPID only once
Send the main PID only once. Previously, `(*Container).start()` and
the conmon handler sent them ~simultaneously and went into a race.
I noticed the issue while debugging a WIP PR.
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
-rw-r--r-- | libpod/oci_conmon_linux.go | 14 | ||||
-rw-r--r-- | test/system/260-sdnotify.bats | 11 |
2 files changed, 5 insertions, 20 deletions
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index c232702e9..6aa7ce6dc 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -36,7 +36,6 @@ import ( "github.com/containers/podman/v4/utils" "github.com/containers/storage/pkg/homedir" pmount "github.com/containers/storage/pkg/mount" - "github.com/coreos/go-systemd/v22/daemon" spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/selinux/go-selinux/label" "github.com/pkg/errors" @@ -1279,19 +1278,6 @@ 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 - - // Send the MAINPID via sdnotify if needed. - switch ctr.config.SdNotifyMode { - case define.SdNotifyModeContainer, define.SdNotifyModeIgnore: - // Nothing to do or conmon takes care of it already. - - default: - if sent, err := daemon.SdNotify(false, fmt.Sprintf("MAINPID=%d", conmonPID)); err != nil { - logrus.Errorf("Notifying systemd of Conmon PID: %v", err) - } else if sent { - logrus.Debugf("Notify MAINPID sent successfully") - } - } } runtimeRestoreDuration := func() int64 { diff --git a/test/system/260-sdnotify.bats b/test/system/260-sdnotify.bats index 395e6f94f..88d84c86f 100644 --- a/test/system/260-sdnotify.bats +++ b/test/system/260-sdnotify.bats @@ -106,6 +106,9 @@ function _assert_mainpid_is_conmon() { cid="$output" wait_for_ready $cid + run_podman container inspect sdnotify_conmon_c --format "{{.State.ConmonPid}}" + mainPID="$output" + run_podman logs sdnotify_conmon_c is "$output" "READY" "\$NOTIFY_SOCKET in container" @@ -114,12 +117,8 @@ function _assert_mainpid_is_conmon() { echo "socat log:" echo "$output" - # ARGH! 'READY=1' should always be the last output line. But sometimes, - # for reasons unknown, we get an extra MAINPID=xxx after READY=1 (#8718). - # Who knows if this is a systemd bug, or conmon, or what. I don't - # even know where to begin asking. So, to eliminate the test flakes, - # we look for READY=1 _anywhere_ in the output, not just the last line. - is "$output" ".*READY=1.*" "sdnotify sent READY=1" + is "$output" "MAINPID=$mainPID +READY=1" "sdnotify sent MAINPID and READY" _assert_mainpid_is_conmon "$output" |