Use conmon pidfile in generated systemd unit as PIDFile.

By default, podman points PIDFile in generated unit file to non-existent
location. As a result, the unit file, generated by podman, is broken:
an attempt to start this unit without prior modification results in a crash,
because systemd can not find the pidfile of service's main process.

Fix the value of "PIDFile" and add a system test for this case.

Signed-off-by: Danila Kiver <danila.kiver@mail.ru>
This commit is contained in:
Danila Kiver
2019-07-04 03:58:37 +03:00
parent f5593d305f
commit a54429cf87
6 changed files with 72 additions and 13 deletions

View File

@ -39,7 +39,7 @@ ExecStart=/usr/bin/podman start c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f14
ExecStop=/usr/bin/podman stop -t 10 c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f145e3b05607f31cffc
KillMode=none
Type=forking
PIDFile=/var/lib/containers/storage/overlay-containers/c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f145e3b05607f31cffc/userdata/c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f145e3b05607f31cffc.pid
PIDFile=/var/run/containers/storage/overlay-containers/c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f145e3b05607f31cffc/userdata/conmon.pid
[Install]
WantedBy=multi-user.target
```
@ -55,7 +55,7 @@ ExecStart=/usr/bin/podman start c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f14
ExecStop=/usr/bin/podman stop -t 1 c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f145e3b05607f31cffc
KillMode=none
Type=forking
PIDFile=/var/lib/containers/storage/overlay-containers/c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f145e3b05607f31cffc/userdata/c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f145e3b05607f31cffc.pid
PIDFile=/var/run/containers/storage/overlay-containers/c21da63c4783be2ac2cd3487ef8d2ec15ee2a28f63dd8f145e3b05607f31cffc/userdata/conmon.pid
[Install]
WantedBy=multi-user.target
```

View File

@ -1058,7 +1058,14 @@ func (r *LocalRuntime) GenerateSystemd(c *cliconfig.GenerateSystemdValues) (stri
if c.Name {
name = ctr.Name()
}
return systemdgen.CreateSystemdUnitAsString(name, ctr.ID(), c.RestartPolicy, ctr.Config().StaticDir, timeout)
config := ctr.Config()
conmonPidFile := config.ConmonPidFile
if conmonPidFile == "" {
return "", errors.Errorf("conmon PID file path is empty, try to recreate the container with --conmon-pidfile flag")
}
return systemdgen.CreateSystemdUnitAsString(name, ctr.ID(), c.RestartPolicy, conmonPidFile, timeout)
}
// GetNamespaces returns namespace information about a container for PS

View File

@ -2,7 +2,6 @@ package systemdgen
import (
"fmt"
"path/filepath"
"github.com/pkg/errors"
)
@ -33,11 +32,11 @@ func ValidateRestartPolicy(restart string) error {
// CreateSystemdUnitAsString takes variables to create a systemd unit file used to control
// a libpod container
func CreateSystemdUnitAsString(name, cid, restart, pidPath string, stopTimeout int) (string, error) {
func CreateSystemdUnitAsString(name, cid, restart, pidFile string, stopTimeout int) (string, error) {
if err := ValidateRestartPolicy(restart); err != nil {
return "", err
}
pidFile := filepath.Join(pidPath, fmt.Sprintf("%s.pid", cid))
unit := fmt.Sprintf(template, name, restart, name, stopTimeout, name, pidFile)
return unit, nil
}

View File

@ -41,7 +41,7 @@ ExecStart=/usr/bin/podman start 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4
ExecStop=/usr/bin/podman stop -t 10 639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401
KillMode=none
Type=forking
PIDFile=/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid
PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
[Install]
WantedBy=multi-user.target`
@ -53,7 +53,7 @@ ExecStart=/usr/bin/podman start foobar
ExecStop=/usr/bin/podman stop -t 10 foobar
KillMode=none
Type=forking
PIDFile=/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401.pid
PIDFile=/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid
[Install]
WantedBy=multi-user.target`
@ -61,7 +61,7 @@ WantedBy=multi-user.target`
name string
cid string
restart string
pidPath string
pidFile string
stopTimeout int
}
tests := []struct {
@ -76,7 +76,7 @@ WantedBy=multi-user.target`
"639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
"639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
"always",
"/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/",
"/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
10,
},
goodID,
@ -87,7 +87,7 @@ WantedBy=multi-user.target`
"foobar",
"639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
"always",
"/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/",
"/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
10,
},
goodName,
@ -98,7 +98,7 @@ WantedBy=multi-user.target`
"639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
"639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401",
"never",
"/var/lib/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/",
"/var/run/containers/storage/overlay-containers/639c53578af4d84b8800b4635fa4e680ee80fd67e0e6a2d4eea48d1e3230f401/userdata/conmon.pid",
10,
},
"",
@ -107,7 +107,7 @@ WantedBy=multi-user.target`
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := CreateSystemdUnitAsString(tt.args.name, tt.args.cid, tt.args.restart, tt.args.pidPath, tt.args.stopTimeout)
got, err := CreateSystemdUnitAsString(tt.args.name, tt.args.cid, tt.args.restart, tt.args.pidFile, tt.args.stopTimeout)
if (err != nil) != tt.wantErr {
t.Errorf("CreateSystemdUnitAsString() error = %v, wantErr %v", err, tt.wantErr)
return

View File

@ -0,0 +1,42 @@
#!/usr/bin/env bats -*- bats -*-
#
# Tests generated configurations for systemd.
#
load helpers
# Be extra paranoid in naming to avoid collisions.
SERVICE_NAME="podman_test_$(random_string)"
UNIT_DIR="$HOME/.config/systemd/user"
UNIT_FILE="$UNIT_DIR/$SERVICE_NAME.service"
function setup() {
basic_setup
mkdir -p "$UNIT_DIR"
}
function teardown() {
rm -f "$UNIT_FILE"
systemctl --user stop "$SERVICE_NAME"
basic_teardown
}
@test "podman generate - systemd - basic" {
skip_if_not_systemd
skip_if_remote
run_podman create $IMAGE echo "I'm alive!"
cid="$output"
run_podman generate systemd $cid > "$UNIT_FILE"
run systemctl --user start "$SERVICE_NAME"
if [ $status -ne 0 ]; then
die "The systemd service $SERVICE_NAME did not start correctly!"
fi
run_podman logs $cid
is "$output" "I'm alive!" "Container output"
}
# vim: filetype=sh

View File

@ -236,6 +236,17 @@ function skip_if_remote() {
skip "${1:-test does not work with podman-remote}"
}
#########################
# skip_if_not_systemd # ...with an optional message
#########################
function skip_if_not_systemd() {
if systemctl --user >/dev/null 2>&1; then
return
fi
skip "${1:-no systemd or daemon does not respond}"
}
#########
# die # Abort with helpful message
#########