Make podman generate systemd --new flag parsing more robust

[Luap99]

First, use the pflag library to parse the flags. With this we can
handle all corner cases such as -td or --detach=false.

Second, preserve the root args with --new. They are used for all podman
commands in the unit file. (e.g. podman --root /tmp run alpine)

Fixes #8847

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Luap99]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

Is there a corner case where a user could specify something like

podman run -d --detach=false alpine sleep 100

And we end up with using the first -d?

[Luap99]

-d -detach=false alpine sleep 100 would turn into -d -d alpine sleep 100 which is valid

[rhatdan]

It is valid but not what the user requested.

podman run -d --detach=false alpine sleep 100
Is treated as
podman run --detach=false alpine sleep 100

With your change it will be treated as
podman run --detach=true alpine sleep 100

BTW I have no problem if we just make running multiple --detach commands fail.

[Luap99]

The whole point of the --new flag is that we must add detach regardless of what the user requested. If there is no detach systemctl start will fail on that unit.

[rhatdan]

Should it fail then if the user requested it not to detach?

[alitvak69]

Sorry for butting in but I don't think systemd will fail with no detach. Here is my example that runs well.

[Unit]
Description=DNS Bind container
After=multi-user.target

[Service]
EnvironmentFile=-/etc/environment
Type=simple
#LimitNOFILE=32768
WorkingDirectory=/dns/slave-internal
ExecStartPre=-/usr/bin/podman pull xxxxxxx/services/bind-host:latest
ExecStart=/usr/bin/podman run --rm --name=dns-slave-internal --no-hosts=true -t --cgroups=no-conmon
--log-driver=journald --log-opt=tag={{.Name}}
--env=CONTAINER_HOST=xxxxxxxxx
--env=DC=chi
--env=DNSZONE_TYPE=slave
--env=DNSZONE_VIEW=internal
--env=SERVICE_NAME=dns-slave-internal
--env=SERVICE_PORT=53
--env=BIND_DATA_DIR=/dns
--cap-add=CAP_SYS_PTRACE,CAP_NET_ADMIN,CAP_NET_RAW,CAP_NET_BIND_SERVICE,CAP_SETUID,CAP_SETGID
--volume=/dns/slave-internal:/dns:rw
--net=private-dns-internal-slaves

xxxxxxxxx/services/bind-host:latest
ExecStop=-/usr/bin/podman stop dns-slave-internal
SyslogIdentifier=dns-slave-internal
Restart=on-failure
KillMode=none
RestartSec=10s
TimeoutStartSec=120
TimeoutStopSec=15

[Install]
WantedBy=multi-user.target
dns-slave-internal.service (END)

[Luap99]

Yes it works when the unit type is simple but we set the type to forking.

[vrothberg]

Thanks for tackling it, really nice work!

Please make sure to split future commits into separate pieces. This commit applies too many changes at once which makes it hard to review and may hinder backporting. But we're close to a new release, so we're good.

[rhatdan]

LGTM

[vrothberg]

/lgtm