kube play: set service container as main PID when possible

Commit 4fa307f fixed a number of issues in the sdnotify proxies.
Whenever a container runs with a custom sdnotify policy, the proxies
need to keep running which in turn required Podman to run and wait for
the service container to stop.  Improve on that behavior and set the
service container as the main PID (instead of Podman) when no container
needs sdnotify.

Fixes: #17345
Signed-off-by: Valentin Rothberg <[email protected]>

Author: vrothberg

pkg/domain/infra/abi/play.go

@@ -314,14 +314,37 @@ 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 we started containers along with a service container, we are
+	// running inside a systemd unit and need to set the main PID.
 	if options.ServiceContainer && ranContainers {
-		message := fmt.Sprintf("MAINPID=%d\n%s", os.Getpid(), daemon.SdNotifyReady)
-		if err := notifyproxy.SendMessage("", message); err != nil {
-			return nil, err
-		}
+		switch len(notifyProxies) {
+		case 0: // Optimization for containers/podman/issues/17345
+			// No container needs sdnotify, so we can mark the
+			// service container's conmon as the main PID and
+			// return early.
+			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
+			}
+		default:
+			// At least one container has a custom sdnotify policy,
+			// so we need to let the sdnotify proxies run for the
+			// lifetime of the service container.  That means, we
+			// need to wait for the service container to stop.
+			// Podman will hence be marked as the main PID.  That
+			// comes at the cost of keeping Podman running.
+			message := fmt.Sprintf("MAINPID=%d\n%s", os.Getpid(), daemon.SdNotifyReady)
+			if err := notifyproxy.SendMessage("", message); err != nil {
+				return nil, err
+			}
 
-		if _, err := serviceContainer.Wait(ctx); err != nil {
-			return nil, fmt.Errorf("waiting for service container: %w", err)
+			if _, err := serviceContainer.Wait(ctx); err != nil {
+				return nil, fmt.Errorf("waiting for service container: %w", err)
+			}
 		}
 	}
 

test/system/250-systemd.bats

@@ -399,7 +399,7 @@ EOF
 
     # Make sure that Podman is the service's MainPID
     run systemctl show --property=MainPID --value $service_name
-    is "$(</proc/$output/comm)" "podman" "podman is the service mainPID"
+    is "$(</proc/$output/comm)" "conmon" "podman is the service mainPID"
 
     # The name of the service container is predictable: the first 12 characters
     # of the hash of the YAML file followed by the "-service" suffix
@@ -433,12 +433,13 @@ EOF
     run_podman pod kill test_pod
     for i in {0..20}; do
         run systemctl is-active $service_name
-        if [[ $output == "inactive" ]]; then
+        if [[ $output == "failed" ]]; then
             break
         fi
         sleep 0.5
     done
-    is "$output" "inactive" "systemd service transitioned to 'inactive' state: $service_name"
+    # The service is marked as failed as the service container exits non-zero.
+    is "$output" "failed" "systemd service transitioned to 'inactive' state: $service_name"
 
     # Now stop and start the service again.
     systemctl stop $service_name

test/system/252-quadlet.bats

@@ -420,7 +420,8 @@ EOF
     run_podman container inspect  --format "{{.State.Status}}" test_pod-test
     is "$output" "running" "container should be started by systemd and hence be running"
 
-    service_cleanup $QUADLET_SERVICE_NAME inactive
+    # The service is marked as failed as the service container exits non-zero.
+    service_cleanup $QUADLET_SERVICE_NAME failed
     run_podman rmi $(pause_image)
 }
 

test/system/260-sdnotify.bats

@@ -218,8 +218,14 @@ EOF
     _start_socat
     wait_for_file $_SOCAT_LOG
 
-    # Will run until all containers have stopped.
     run_podman play kube --service-container=true --log-driver journald $yaml_source
+
+    # The service container is the main PID since no container has a custom
+    # sdnotify policy.
+    run_podman container inspect $service_container --format "{{.State.ConmonPid}}"
+    main_pid="$output"
+
+    # Will run until all containers have stopped.
     run_podman container wait $service_container test_pod-test
 
     # Make sure the containers have the correct policy.
@@ -233,7 +239,7 @@ ignore"
     echo "$output"
 
     # The "with policies" test below checks the MAINPID.
-    is "$output" "MAINPID=.*
+    is "$output" "MAINPID=$main_pid
 READY=1" "sdnotify sent MAINPID and READY"
 
     _stop_socat