From 3fc126e152d5ebe4bfef980dea04192762628773 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 5 Aug 2022 14:22:54 +0200 Subject: 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 +++++++++++ 8 files changed, 28 insertions(+), 16 deletions(-) (limited to 'libpod') diff --git a/libpod/container.go b/libpod/container.go index 4e2d93860..6c05b1084 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 544c45a8c..bd9816651 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 fa2130a28..5e2ab2818 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 bad68991b..7cef067b0 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 a131ab367..c4f83b571 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 e6a34ba61..5982d684c 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 cb76de72c..1b654ed33 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 b31cb4ab2..933c9a1c3 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 { -- cgit v1.2.3-54-g00ecf From 79e21b5b162d3c2d3fb274b20bfe180c15284893 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 8 Aug 2022 11:44:52 +0200 Subject: kube play: sd-notify integration Integrate sd-notify policies into `kube play`. The policies can be configured for all contianers via the `io.containers.sdnotify` annotation or for indidivual containers via the `io.containers.sdnotify/$name` annotation. The `kube play` process will wait for all containers to be ready by waiting for the individual `READY=1` messages which are received via the `pkg/systemd/notifyproxy` proxy mechanism. Also update the simple "container" sd-notify test as it did not fully test the expected behavior which became obvious when adding the new tests. Signed-off-by: Valentin Rothberg --- cmd/podman/kube/play.go | 4 + libpod/container_validate.go | 4 + libpod/define/config.go | 7 -- libpod/define/sdnotify.go | 20 +++++ libpod/options.go | 13 +-- pkg/domain/infra/abi/play.go | 79 ++++++++++++++++- pkg/domain/infra/abi/play_utils.go | 16 ++++ pkg/domain/infra/abi/play_utils_test.go | 38 +++++++++ pkg/specgen/container_validate.go | 6 +- pkg/specgen/generate/container_create.go | 8 +- pkg/systemd/notifyproxy/notifyproxy.go | 8 ++ test/system/260-sdnotify.bats | 141 +++++++++++++++++++++++++++++-- test/system/helpers.bash | 17 ++++ 13 files changed, 324 insertions(+), 37 deletions(-) create mode 100644 libpod/define/sdnotify.go create mode 100644 pkg/domain/infra/abi/play_utils.go create mode 100644 pkg/domain/infra/abi/play_utils_test.go (limited to 'libpod') diff --git a/cmd/podman/kube/play.go b/cmd/podman/kube/play.go index 4811bcf4b..07c4b59b9 100644 --- a/cmd/podman/kube/play.go +++ b/cmd/podman/kube/play.go @@ -182,6 +182,10 @@ func playFlags(cmd *cobra.Command) { } func Play(cmd *cobra.Command, args []string) error { + if playOptions.ServiceContainer && !playOptions.StartCLI { // Sanity check to be future proof + return fmt.Errorf("--service-container does not work with --start=stop") + } + // TLS verification in c/image is controlled via a `types.OptionalBool` // which allows for distinguishing among set-true, set-false, unspecified // which is important to implement a sane way of dealing with defaults of diff --git a/libpod/container_validate.go b/libpod/container_validate.go index e280c60d2..da33f6db7 100644 --- a/libpod/container_validate.go +++ b/libpod/container_validate.go @@ -133,5 +133,9 @@ func (c *Container) validate() error { if len(c.config.InitContainerType) > 0 && len(c.config.Pod) < 1 { return fmt.Errorf("init containers must be created in a pod: %w", define.ErrInvalidArg) } + + if c.config.SdNotifyMode == define.SdNotifyModeIgnore && len(c.config.SdNotifySocket) > 0 { + return fmt.Errorf("cannot set sd-notify socket %q with sd-notify mode %q", c.config.SdNotifySocket, c.config.SdNotifyMode) + } return nil } diff --git a/libpod/define/config.go b/libpod/define/config.go index 0181bd31c..34c1a675d 100644 --- a/libpod/define/config.go +++ b/libpod/define/config.go @@ -81,13 +81,6 @@ const NoLogging = "none" // PassthroughLogging is the string conmon expects when specifying to use the passthrough driver const PassthroughLogging = "passthrough" -// Strings used for --sdnotify option to podman -const ( - SdNotifyModeContainer = "container" - SdNotifyModeConmon = "conmon" - SdNotifyModeIgnore = "ignore" -) - // DefaultRlimitValue is the value set by default for nofile and nproc const RLimitDefaultValue = uint64(1048576) diff --git a/libpod/define/sdnotify.go b/libpod/define/sdnotify.go new file mode 100644 index 000000000..1d548c764 --- /dev/null +++ b/libpod/define/sdnotify.go @@ -0,0 +1,20 @@ +package define + +import "fmt" + +// Strings used for --sdnotify option to podman +const ( + SdNotifyModeContainer = "container" + SdNotifyModeConmon = "conmon" + SdNotifyModeIgnore = "ignore" +) + +// ValidateSdNotifyMode validates the specified mode. +func ValidateSdNotifyMode(mode string) error { + switch mode { + case "", SdNotifyModeContainer, SdNotifyModeConmon, SdNotifyModeIgnore: + return nil + default: + return fmt.Errorf("%w: invalid sdnotify value %q: must be %s, %s or %s", ErrInvalidArg, mode, SdNotifyModeContainer, SdNotifyModeConmon, SdNotifyModeIgnore) + } +} diff --git a/libpod/options.go b/libpod/options.go index 933c9a1c3..43ed1ff78 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -6,14 +6,12 @@ import ( "net" "os" "path/filepath" - "strings" "syscall" "github.com/containers/buildah/pkg/parse" nettypes "github.com/containers/common/libnetwork/types" "github.com/containers/common/pkg/config" "github.com/containers/common/pkg/secrets" - cutil "github.com/containers/common/pkg/util" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/types" "github.com/containers/podman/v4/libpod/define" @@ -29,12 +27,6 @@ import ( "github.com/sirupsen/logrus" ) -// 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 // instead. @@ -631,9 +623,8 @@ func WithSdNotifyMode(mode string) CtrCreateOption { return define.ErrCtrFinalized } - // verify values - if len(mode) > 0 && !cutil.StringInSlice(strings.ToLower(mode), SdNotifyModeValues) { - return fmt.Errorf("--sdnotify values must be one of %q: %w", strings.Join(SdNotifyModeValues, ", "), define.ErrInvalidArg) + if err := define.ValidateSdNotifyMode(mode); err != nil { + return err } ctr.config.SdNotifyMode = mode diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index 3f2fd5f92..faa89cc26 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -27,13 +27,19 @@ import ( "github.com/containers/podman/v4/pkg/specgen/generate" "github.com/containers/podman/v4/pkg/specgen/generate/kube" "github.com/containers/podman/v4/pkg/specgenutil" + "github.com/containers/podman/v4/pkg/systemd/notifyproxy" "github.com/containers/podman/v4/pkg/util" + "github.com/coreos/go-systemd/v22/daemon" "github.com/ghodss/yaml" "github.com/opencontainers/go-digest" "github.com/sirupsen/logrus" yamlv3 "gopkg.in/yaml.v3" ) +// sdNotifyAnnotation allows for configuring service-global and +// container-specific sd-notify modes. +const sdNotifyAnnotation = "io.containers.sdnotify" + // createServiceContainer creates a container that can later on // be associated with the pods of a K8s yaml. It will be started along with // the first pod. @@ -73,7 +79,12 @@ func (ic *ContainerEngine) createServiceContainer(ctx context.Context, name stri return nil, fmt.Errorf("creating runtime spec for service container: %w", err) } opts = append(opts, libpod.WithIsService()) - opts = append(opts, libpod.WithSdNotifyMode(define.SdNotifyModeConmon)) + + // Set the sd-notify mode to "ignore". Podman is responsible for + // sending the notify messages when all containers are ready. + // The mode for individual containers or entire pods can be configured + // via the `sdNotifyAnnotation` annotation in the K8s YAML. + opts = append(opts, libpod.WithSdNotifyMode(define.SdNotifyModeIgnore)) // Create a new libpod container based on the spec. ctr, err := ic.Libpod.NewContainer(ctx, runtimeSpec, spec, false, opts...) @@ -96,6 +107,10 @@ func k8sName(content []byte, suffix string) string { } func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options entities.PlayKubeOptions) (_ *entities.PlayKubeReport, finalErr error) { + if options.ServiceContainer && options.Start == types.OptionalBoolFalse { // Sanity check to be future proof + return nil, fmt.Errorf("running a service container requires starting the pod(s)") + } + report := &entities.PlayKubeReport{} validKinds := 0 @@ -121,6 +136,8 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options var configMaps []v1.ConfigMap + ranContainers := false + var serviceContainer *libpod.Container // create pod on each document if it is a pod or deployment // any other kube kind will be skipped for _, document := range documentList { @@ -130,8 +147,7 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options } // TODO: create constants for the various "kinds" of yaml files. - var serviceContainer *libpod.Container - if options.ServiceContainer && (kind == "Pod" || kind == "Deployment") { + if options.ServiceContainer && serviceContainer == nil && (kind == "Pod" || kind == "Deployment") { ctr, err := ic.createServiceContainer(ctx, k8sName(content, "service"), options) if err != nil { return nil, err @@ -178,6 +194,7 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options report.Pods = append(report.Pods, r.Pods...) validKinds++ + ranContainers = true case "Deployment": var deploymentYAML v1apps.Deployment @@ -192,6 +209,7 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options report.Pods = append(report.Pods, r.Pods...) validKinds++ + ranContainers = true case "PersistentVolumeClaim": var pvcYAML v1.PersistentVolumeClaim @@ -239,6 +257,20 @@ func (ic *ContainerEngine) PlayKube(ctx context.Context, body io.Reader, options return nil, fmt.Errorf("YAML document does not contain any supported kube kind") } + if options.ServiceContainer && ranContainers { + // We can consider the service to be up and running now. + // Send the sd-notify messages pointing systemd to the + // service container. + data, err := serviceContainer.Inspect(false) + if err != nil { + return nil, err + } + message := fmt.Sprintf("MAINPID=%d\n%s", data.State.ConmonPid, daemon.SdNotifyReady) + if err := notifyproxy.SendMessage("", message); err != nil { + return nil, err + } + } + return report, nil } @@ -280,6 +312,11 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY report entities.PlayKubeReport ) + mainSdNotifyMode, err := getSdNotifyMode(annotations, "") + if err != nil { + return nil, err + } + // Create the secret manager before hand secretsManager, err := ic.Libpod.SecretsManager() if err != nil { @@ -562,6 +599,9 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY initContainers = append(initContainers, ctr) } + + var sdNotifyProxies []*notifyproxy.NotifyProxy // containers' sd-notify proxies + for _, container := range podYAML.Spec.Containers { // Error out if the same name is used for more than one container if _, ok := ctrNames[container.Name]; ok { @@ -606,7 +646,31 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY if err != nil { return nil, err } - opts = append(opts, libpod.WithSdNotifyMode(define.SdNotifyModeIgnore)) + + sdNotifyMode := mainSdNotifyMode + ctrNotifyMode, err := getSdNotifyMode(annotations, container.Name) + if err != nil { + return nil, err + } + if ctrNotifyMode != "" { + sdNotifyMode = ctrNotifyMode + } + if sdNotifyMode == "" { // Default to "ignore" + sdNotifyMode = define.SdNotifyModeIgnore + } + + opts = append(opts, libpod.WithSdNotifyMode(sdNotifyMode)) + + // Create a notify proxy for the container. + if sdNotifyMode != "" && sdNotifyMode != define.SdNotifyModeIgnore { + proxy, err := notifyproxy.New("") + if err != nil { + return nil, err + } + sdNotifyProxies = append(sdNotifyProxies, proxy) + opts = append(opts, libpod.WithSdNotifySocket(proxy.SocketPath())) + } + ctr, err := generate.ExecuteCreate(ctx, ic.Libpod, rtSpec, spec, false, opts...) if err != nil { return nil, err @@ -624,6 +688,13 @@ func (ic *ContainerEngine) playKubePod(ctx context.Context, podName string, podY playKubePod.ContainerErrors = append(playKubePod.ContainerErrors, fmt.Errorf("error starting container %s: %w", id, err).Error()) fmt.Println(playKubePod.ContainerErrors) } + + // Wait for each proxy to receive a READY message. + for _, proxy := range sdNotifyProxies { + if err := proxy.WaitAndClose(); err != nil { + return nil, err + } + } } playKubePod.ID = pod.ID() diff --git a/pkg/domain/infra/abi/play_utils.go b/pkg/domain/infra/abi/play_utils.go new file mode 100644 index 000000000..482a158e6 --- /dev/null +++ b/pkg/domain/infra/abi/play_utils.go @@ -0,0 +1,16 @@ +package abi + +import "github.com/containers/podman/v4/libpod/define" + +// getSdNotifyMode returns the `sdNotifyAnnotation/$name` for the specified +// name. If name is empty, it'll only look for `sdNotifyAnnotation`. +func getSdNotifyMode(annotations map[string]string, name string) (string, error) { + var mode string + switch len(name) { + case 0: + mode = annotations[sdNotifyAnnotation] + default: + mode = annotations[sdNotifyAnnotation+"/"+name] + } + return mode, define.ValidateSdNotifyMode(mode) +} diff --git a/pkg/domain/infra/abi/play_utils_test.go b/pkg/domain/infra/abi/play_utils_test.go new file mode 100644 index 000000000..80a9fe543 --- /dev/null +++ b/pkg/domain/infra/abi/play_utils_test.go @@ -0,0 +1,38 @@ +package abi + +import ( + "testing" + + "github.com/containers/podman/v4/libpod/define" + "github.com/stretchr/testify/require" +) + +func TestGetSdNotifyMode(t *testing.T) { + tests := []struct { + key, value, name, result string + mustError bool + }{ + {sdNotifyAnnotation, define.SdNotifyModeConmon, "", define.SdNotifyModeConmon, false}, + {sdNotifyAnnotation + "/container-a", define.SdNotifyModeContainer, "container-a", define.SdNotifyModeContainer, false}, + {sdNotifyAnnotation + "/container-b", define.SdNotifyModeIgnore, "container-b", define.SdNotifyModeIgnore, false}, + {sdNotifyAnnotation + "/container-c", "", "container-c", "", false}, + {sdNotifyAnnotation + "-/wrong-key", "xxx", "wrong-key", "", false}, + {sdNotifyAnnotation + "/container-error", "invalid", "container-error", "", true}, + } + + annotations := make(map[string]string) + // Populate the annotations + for _, test := range tests { + annotations[test.key] = test.value + } + // Run the tests + for _, test := range tests { + result, err := getSdNotifyMode(annotations, test.name) + if test.mustError { + require.Error(t, err, "%v", test) + continue + } + require.NoError(t, err, "%v", test) + require.Equal(t, test.result, result, "%v", test) + } +} diff --git a/pkg/specgen/container_validate.go b/pkg/specgen/container_validate.go index 63d94b6b3..064245602 100644 --- a/pkg/specgen/container_validate.go +++ b/pkg/specgen/container_validate.go @@ -67,9 +67,9 @@ func (s *SpecGenerator) Validate() error { if len(s.ContainerBasicConfig.Systemd) > 0 && !util.StringInSlice(strings.ToLower(s.ContainerBasicConfig.Systemd), SystemDValues) { return fmt.Errorf("--systemd values must be one of %q: %w", strings.Join(SystemDValues, ", "), ErrInvalidSpecConfig) } - // sdnotify values must be container, conmon, or ignore - if len(s.ContainerBasicConfig.SdNotifyMode) > 0 && !util.StringInSlice(strings.ToLower(s.ContainerBasicConfig.SdNotifyMode), SdNotifyModeValues) { - return fmt.Errorf("--sdnotify values must be one of %q: %w", strings.Join(SdNotifyModeValues, ", "), ErrInvalidSpecConfig) + + if err := define.ValidateSdNotifyMode(s.ContainerBasicConfig.SdNotifyMode); err != nil { + return err } // diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go index f4c67e534..e9cec2873 100644 --- a/pkg/specgen/generate/container_create.go +++ b/pkg/specgen/generate/container_create.go @@ -353,9 +353,11 @@ 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 s.SdNotifyMode != define.SdNotifyModeIgnore { + if notify, ok := os.LookupEnv("NOTIFY_SOCKET"); ok { + options = append(options, libpod.WithSdNotifySocket(notify)) + } + } } if pod != nil { diff --git a/pkg/systemd/notifyproxy/notifyproxy.go b/pkg/systemd/notifyproxy/notifyproxy.go index b9f5737f7..9e6eb4cf0 100644 --- a/pkg/systemd/notifyproxy/notifyproxy.go +++ b/pkg/systemd/notifyproxy/notifyproxy.go @@ -12,7 +12,15 @@ import ( ) // SendMessage sends the specified message to the specified socket. +// No message is sent if no socketPath is provided and the NOTIFY_SOCKET +// variable is not set either. func SendMessage(socketPath string, message string) error { + if socketPath == "" { + socketPath, _ = os.LookupEnv("NOTIFY_SOCKET") + if socketPath == "" { + return nil + } + } socketAddr := &net.UnixAddr{ Name: socketPath, Net: "unixgram", diff --git a/test/system/260-sdnotify.bats b/test/system/260-sdnotify.bats index fe442f57d..6c3ef7f3f 100644 --- a/test/system/260-sdnotify.bats +++ b/test/system/260-sdnotify.bats @@ -92,7 +92,7 @@ function _assert_mainpid_is_conmon() { cid="$output" run_podman container inspect $cid --format "{{.Config.SdNotifyMode}} {{.Config.SdNotifySocket}}" - is "$output" "ignore $NOTIFY_SOCKET" + is "$output" "ignore " "NOTIFY_SOCKET is not set with 'ignore' mode" run_podman 1 start --attach $cid is "$output" "" "\$NOTIFY_SOCKET in container" @@ -122,6 +122,7 @@ function _assert_mainpid_is_conmon() { is "$output" "READY" "\$NOTIFY_SOCKET in container" # The 'echo's help us debug failed runs + wait_for_file $_SOCAT_LOG run cat $_SOCAT_LOG echo "socat log:" echo "$output" @@ -141,7 +142,7 @@ READY=1" "sdnotify sent MAINPID and READY" # These tests can fail in dev. environment because of SELinux. # quick fix: chcon -t container_runtime_exec_t ./bin/podman @test "sdnotify : container" { - skip_if_aarch64 "FIXME: #15074 - fails on aarch64 non-remote" + skip_if_aarch64 "FIXME: #15277 sdnotify doesn't work on aarch64" # Sigh... we need to pull a humongous image because it has systemd-notify. # (IMPORTANT: fedora:32 and above silently removed systemd-notify; this # caused CI to hang. That's why we explicitly require fedora:31) @@ -156,7 +157,7 @@ READY=1" "sdnotify sent MAINPID and READY" _start_socat run_podman run -d --sdnotify=container $_FEDORA \ - sh -c 'printenv NOTIFY_SOCKET;echo READY;systemd-notify --ready;while ! test -f /stop;do sleep 0.1;done' + sh -c 'printenv NOTIFY_SOCKET; echo READY; while ! test -f /stop;do sleep 0.1;done;systemd-notify --ready' cid="$output" wait_for_ready $cid @@ -166,6 +167,8 @@ READY=1" "sdnotify sent MAINPID and READY" run_podman logs $cid is "${lines[0]}" "/run/notify/notify.sock" "NOTIFY_SOCKET is passed to container" + run_podman container inspect $cid --format "{{.State.ConmonPid}}" + mainPID="$output" # With container, READY=1 isn't necessarily the last message received; # just look for it anywhere in received messages run cat $_SOCAT_LOG @@ -173,19 +176,25 @@ READY=1" "sdnotify sent MAINPID and READY" echo "socat log:" echo "$output" - is "$output" ".*READY=1" "received READY=1 through notify socket" - - _assert_mainpid_is_conmon "$output" + is "$output" "MAINPID=$mainPID" "Container is not ready yet, so we only know the main PID" # Done. Stop container, clean up. run_podman exec $cid touch /stop run_podman wait $cid + + wait_for_file $_SOCAT_LOG + run cat $_SOCAT_LOG + echo "socat log:" + echo "$output" + is "$output" "MAINPID=$mainPID +READY=1" + run_podman rm $cid run_podman rmi $_FEDORA _stop_socat } -@test "sdnotify : play kube" { +@test "sdnotify : play kube - no policies" { # Create the YAMl file yaml_source="$PODMAN_TMPDIR/test.yaml" cat >$yaml_source <$yaml_source </dev/null & + + # Wait for both containers to be running + for i in $(seq 1 20); do + run_podman "?" container wait $container_a $container_b --condition="running" + if [[ $status == 0 ]]; then + break + fi + sleep 0.5 + # Just for debugging + run_podman ps -a + done + if [[ $status != 0 ]]; then + die "container $container_a and/or $container_b did not start" + fi + + # Make sure the containers have the correct policy + run_podman container inspect $container_a $container_b $service_container --format "{{.Config.SdNotifyMode}}" + is "$output" "container +conmon +ignore" + + is "$(< $_SOCAT_LOG)" "" "nothing received on socket" + + # Make sure the container received a "proxy" socket and is not using the + # one of `kube play` + run_podman container inspect $container_a --format "{{.Config.SdNotifySocket}}" + assert "$output" != $NOTIFY_SOCKET + + run_podman logs $container_a + is "${lines[0]}" "/run/notify/notify.sock" "NOTIFY_SOCKET is passed to container" + + # Instruct the container to send the READY + run_podman exec $container_a /bin/touch /stop + + run_podman container inspect $service_container --format "{{.State.ConmonPid}}" + main_pid="$output" + + run_podman container wait $container_a + wait_for_file $_SOCAT_LOG + # The 'echo's help us debug failed runs + run cat $_SOCAT_LOG + echo "socat log:" + echo "$output" + + is "$output" "MAINPID=$main_pid +READY=1" "sdnotify sent MAINPID and READY" + + _stop_socat + + # Clean up pod and pause image + run_podman play kube --down $yaml_source + run_podman rmi $_FEDORA $(pause_image) +} # vim: filetype=sh diff --git a/test/system/helpers.bash b/test/system/helpers.bash index b821175bb..5ff3fae6d 100644 --- a/test/system/helpers.bash +++ b/test/system/helpers.bash @@ -342,6 +342,23 @@ function wait_for_port() { die "Timed out waiting for $host:$port" } +################### +# wait_for_file # Returns once file is available on host +################### +function wait_for_file() { + local file=$1 # The path to the file + local _timeout=${2:-5} # Optional; default 5 seconds + + # Wait + while [ $_timeout -gt 0 ]; do + test -e $file && return + sleep 1 + _timeout=$(( $_timeout - 1 )) + done + + die "Timed out waiting for $file" +} + # END podman helpers ############################################################################### # BEGIN miscellaneous tools -- cgit v1.2.3-54-g00ecf