Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generate systemd: warn on --restart without --new #15766

Merged
merged 1 commit into from
Sep 13, 2022
Merged

generate systemd: warn on --restart without --new #15766

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/source/markdown/podman-generate-systemd.1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ Set the systemd unit requires (`Requires=`) option. Similar to wants, but declar
#### **--restart-policy**=*policy*

Set the systemd restart policy. The restart-policy must be one of: "no", "on-success", "on-failure", "on-abnormal",
"on-watchdog", "on-abort", or "always". The default policy is *on-failure*.
"on-watchdog", "on-abort", or "always". The default policy is *on-failure* unless the container was created with a custom restart policy.

Note that generating a unit without `--new` on a container with a custom restart policy can lead to issues on shutdown; systemd will attempt to stop the unit while Podman tries to restart it. It is recommended to to create the container without `--restart` and use the `--restart-policy` option instead when generating the unit file.

#### **--restart-sec**=*time*

Expand Down
12 changes: 12 additions & 0 deletions pkg/systemd/generate/containers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,18 @@ func generateContainerInfo(ctr *libpod.Container, options entities.GenerateSyste
return nil, errors.New("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag")
}

// #15284: old units generated without --new can lead to issues on
// shutdown when the containers are created with a custom restart
// policy.
if !options.New {
switch config.RestartPolicy {
case libpodDefine.RestartPolicyNo, libpodDefine.RestartPolicyNone:
// All good
default:
logrus.Warnf("Container %s has restart policy %q which can lead to issues on shutdown: consider recreating the container without a restart policy and use systemd's restart mechanism instead", ctr.ID(), config.RestartPolicy)
}
}

createCommand := []string{}
if config.CreateCommand != nil {
createCommand = config.CreateCommand
Expand Down
7 changes: 7 additions & 0 deletions test/system/250-systemd.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ function service_cleanup() {
skip "FIXME: 2022-09-01: requires conmon-2.1.4, ubuntu has 2.1.3"
fi

# Warn when a custom restart policy is used without --new (see #15284)
run_podman create --restart=always $IMAGE
cid="$output"
run_podman generate systemd $cid
is "$output" ".*Container $cid has restart policy .*always.* which can lead to issues on shutdown.*" "generate systemd emits warning"
run_podman rm -f $cid

cname=$(random_string)
# See #7407 for --pull=always.
run_podman create --pull=always --name $cname --label "io.containers.autoupdate=registry" $IMAGE \
Expand Down