Skip to content

Commit

Permalink
Merge pull request #17469 from vrothberg/fix-17345
Browse files Browse the repository at this point in the history
kube play: set service container as main PID when possible
  • Loading branch information
openshift-merge-robot authored Feb 10, 2023
2 parents 15caef9 + 1541ce5 commit f099c1f
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 12 deletions.
35 changes: 29 additions & 6 deletions pkg/domain/infra/abi/play.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

Expand Down
7 changes: 4 additions & 3 deletions test/system/250-systemd.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion test/system/252-quadlet.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
10 changes: 8 additions & 2 deletions test/system/260-sdnotify.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down

0 comments on commit f099c1f

Please sign in to comment.