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.

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

[ci:docs] Add a notice to remove pods/containers before starting the systemd service #9050

Conversation

cognition9144
Copy link
Contributor

The pods/containers have to be removed before starting the corresponding systemd service. Otherwise the service will fail. That hasn't been written into the doc. This PR adds that.

@rhatdan rhatdan changed the title [Doc] Add a notice to remove pods/containers before starting the systemd service [ci:docs] Add a notice to remove pods/containers before starting the systemd service Jan 21, 2021
@rhatdan
Copy link
Member

rhatdan commented Jan 21, 2021

@vrothberg PTAL

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not needed any more. The generated unit files take care of removing. May be you're running an older version of Podman?

@cognition9144
Copy link
Contributor Author

cognition9144 commented Jan 21, 2021

That's not needed any more. The generated unit files take care of removing. May be you're running an older version of Podman?

I'm using podman version 2.2.1 which is the latest for fedora 33. Is there any way to install a newer one? Maybe you are refering to podman 3.0 which is not available to fedora 33 yet?


# The corresponding pod/container previously created must be removed before starting the pod/container through the systemd service
$ podman stop bc219740a210455fa27deacc96d50a9e20516492f1417507c13ce1533dbdcd9d
$podman rm bc219740a210455fa27deacc96d50a9e20516492f1417507c13ce1533dbdcd9d
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one must not be replaced. I just reran the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix it after we figure out the complete instruction

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@vrothberg
Copy link
Member

vrothberg commented Jan 21, 2021

@xcffl, can you elaborate why the pods/containers must be removed before starting the systemd units?

I see two cases where this would be needed:

  1. When there's a name conflict. This case is treated implicitly by podman. If a pod or container is created with a specific name via --name, then the generated unit files will be created with --replace.
  2. When the previous containers/pods are blocking certain resources (e.g., ports). But none of the examples do that.

@cognition9144
Copy link
Contributor Author

cognition9144 commented Jan 21, 2021

@xcffl, can you elaborate why the pods/containers must be removed before starting the systemd units?

Here is what I do:

❯ cat create-pod-systemd-service.sh 
#!/bin/bash

POD_NAME=${PWD##*/}

podman-compose down && podman-compose up -d
podman generate systemd --new --files --name $POD_NAME
cp *.service $HOME/.config/systemd/user
podman-compose down
systemctl --user daemon-reload
systemctl --user enable --now pod-$POD_NAME

If I remove the second podman-compose down, the problem occurs.

@vrothberg
Copy link
Member

@xcffl can you share the error you're seeing?

@cognition9144
Copy link
Contributor Author

cognition9144 commented Jan 21, 2021

Jan 22 01:47:33 localhost.localdomain systemd[3063]: pod-nginx.service: Failed with result 'exit-code'.
Jan 22 01:47:33 localhost.localdomain systemd[3063]: pod-nginx.service: Unit process 3164 (podman pause) remains running after unit stopped.
Jan 22 01:47:33 localhost.localdomain systemd[3063]: Failed to start Podman pod-nginx.service.
Jan 22 01:47:33 localhost.localdomain systemd[3063]: pod-nginx.service: Scheduled restart job, restart counter is at 5.
Jan 22 01:47:33 localhost.localdomain systemd[3063]: Stopped Podman pod-nginx.service.
Jan 22 01:47:33 localhost.localdomain systemd[3063]: pod-nginx.service: Start request repeated too quickly.
Jan 22 01:47:33 localhost.localdomain systemd[3063]: pod-nginx.service: Failed with result 'exit-code'.
Jan 22 01:47:33 localhost.localdomain systemd[3063]: pod-nginx.service: Unit process 3164 (podman pause) remains running after unit stopped.
Jan 22 01:47:33 localhost.localdomain systemd[3063]: Failed to start Podman pod-nginx.service.

And the original pod is still alive

@vrothberg
Copy link
Member

Can you check via journalctl the error message from Podman? The upper logs don't show that, unfortunately. I suspect the Pod exposes ports (pod-nginx.service).

I propose to reword the proposed changes a bit to: "If the previously created containers or pods are using shared resources, such as ports, make sure to remove them before starting the generated systemd units."

@vrothberg
Copy link
Member

Error: error adding pod to state: name "test" is in use: pod already exists

Thanks! This should not happen as the unit files should be generated with --replace. Can you share the generated unit files?

@cognition9144
Copy link
Contributor Author

# pod-nginx.service
# autogenerated by Podman 2.2.1

[Unit]
Description=Podman pod-nginx.service
Documentation=man:podman-generate-systemd(1)
Wants=network.target
After=network-online.target
Requires=container-nginx_swag_1.service
Before=container-nginx_swag_1.service

[Service]
Environment=PODMAN_SYSTEMD_UNIT=%n
Restart=on-failure
ExecStartPre=/bin/rm -f %t/pod-nginx.pid %t/pod-nginx.pod-id
ExecStartPre=/usr/bin/podman pod create --infra-conmon-pidfile %t/pod-nginx.pid --pod-id-file %t/pod-nginx.pod-id --name=nginx --share net
ExecStart=/usr/bin/podman pod start --pod-id-file %t/pod-nginx.pod-id
ExecStop=/usr/bin/podman pod stop --ignore --pod-id-file %t/pod-nginx.pod-id -t 10
ExecStopPost=/usr/bin/podman pod rm --ignore -f --pod-id-file %t/pod-nginx.pod-id
PIDFile=%t/pod-nginx.pid
KillMode=none
Type=forking

[Install]
WantedBy=multi-user.target default.target

❯ cat ~/.config/systemd/user/container-nginx_swag_1.service 
# container-nginx_swag_1.service
# autogenerated by Podman 2.2.1

[Unit]
Description=Podman container-nginx_swag_1.service
Documentation=man:podman-generate-systemd(1)
Wants=network.target
After=network-online.target

[Service]
Environment=PODMAN_SYSTEMD_UNIT=%n
Restart=on-failure
ExecStartPre=/bin/rm -f %t/container-nginx_swag_1.pid %t/container-nginx_swag_1.ctr-id
ExecStart=/usr/bin/podman run --conmon-pidfile %t/container-nginx_swag_1.pid --cidfile %t/container-nginx_swag_1.ctr-id --cgroups=no-conmon --pod-id-file %t/pod-nginx.pod-id --replace --name=nginx_swag_1 -d --label io.podman.compose.config-hash=123 --label io.podman.compose.project=nginx --label io.podman.compose.version=0.0.1 --label com.docker.compose.container-number=1 --label com.docker.compose.service=swag --network host -e PUID=1000 ** ghcr.io/linuxserver/swag
ExecStop=/usr/bin/podman stop --ignore --cidfile %t/container-nginx_swag_1.ctr-id -t 10
ExecStopPost=/usr/bin/podman rm --ignore -f --cidfile %t/container-nginx_swag_1.ctr-id
PIDFile=%t/container-nginx_swag_1.pid
KillMode=none
Type=forking

[Install]
WantedBy=multi-user.target default.target

@zhangguanzhang
Copy link
Collaborator

need rebase to one commit

@cognition9144 cognition9144 force-pushed the doc-rm-pod-before-start-systemd-service branch from 923a942 to 74a00a7 Compare January 22, 2021 10:19
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@cognition9144
Copy link
Contributor Author

need rebase to one commit

Sure. Done.

@cognition9144 cognition9144 force-pushed the doc-rm-pod-before-start-systemd-service branch from 74a00a7 to 94f96c7 Compare January 22, 2021 10:29
@vrothberg
Copy link
Member

@zhangguanzhang @rhatdan PTAL

@rhatdan
Copy link
Member

rhatdan commented Jan 22, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 22, 2021
@rhatdan rhatdan added approved Indicates a PR has been approved by an approver from all required OWNERS files. Backport to v3.0 labels Jan 22, 2021
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: xcffl

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:

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

@openshift-merge-robot openshift-merge-robot merged commit 47616fe into containers:master Jan 22, 2021
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants