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

Added additional fcc example #75

Closed
wants to merge 1 commit into from
Closed

Conversation

zethsqx
Copy link

@zethsqx zethsqx commented May 21, 2020

  • added examples to cover container processes that requires conmon process to keep port listening on host

- added examples to cover container processes that requires conmon process to keep port listening on host
@lucab
Copy link
Contributor

lucab commented May 21, 2020

Thanks for the PR! However this template looks very suspicious to me (in particular, KillMode=none, the detached/forking mode, and no After dependencies). Where are you taking this from?

@zethsqx
Copy link
Author

zethsqx commented May 21, 2020

credit to Valentin Rothberg - https://www.redhat.com/sysadmin/podman-shareable-systemd-services

for current example, the processes will get killed after coming up

@lucab
Copy link
Contributor

lucab commented May 21, 2020

@zethsqx thanks, let's keep this a bit on hold while we sort out a bunch of doubts.

Hey @vrothberg, several doubts on the template being referenced here:

  • KillMode=none seems a bad idea, as a buggy container will leak behind stale processes/cgroups, eventually killing a machine if the service goes into a restart-loop (yes, we already had this in the past, see e.g. Fix journalctl leak google/cadvisor#1729)
  • the detaching/forking setup is somehow suboptimal. Among the other things, the journal for this service does not contain anymore any application log entries
  • the ExecStop looks like it is missing rm --cidfile

@vrothberg
Copy link

Hey @vrothberg, several doubts on the template being referenced here:

Thanks for the ping!

* `KillMode=none` seems a bad idea, as a buggy container will leak behind stale processes/cgroups, eventually killing a machine if the service goes into a restart-loop (yes, we already had this in the past, see e.g. [google/cadvisor#1729](https://github.com/google/cadvisor/pull/1729))

The reasoning for that setting is to avoid a race between systemd and Podman sending signals. We really want Podman to take care of container stop/remove as Podman will make sure to clean up all cgroups, namespaces, dependencies and account for the container's settings.

* the detaching/forking setup is somehow suboptimal. Among the other things, the journal for this service does not contain anymore any application log entries

You can use podman-logs or set Type=simple (plus changing all fork-specific settings). However, we encourage using the fork type to make sure that systemd is monitoring the correct process (via PIDFile).

* the ExecStop looks like it is missing `rm --cidfile`

Thanks! That's fixed in the latest Podman which now removes both files (https://github.com/containers/libpod/blob/master/pkg/systemd/generate/systemdgen.go#L109).

@lucab
Copy link
Contributor

lucab commented May 25, 2020

@vrothberg thanks for the feedback. We could probably move this to the podman bug tracker, but I'll followup here for the moment.

The reasoning for that setting is to avoid a race between systemd and Podman sending signals. We really want Podman to take care of container stop/remove as Podman will make sure to clean up all cgroups, namespaces, dependencies and account for the container's settings.

That's fair, but to the best of my knowledge KillMode=none is not for that.
Systemd synchronously waits for the command in ExecStop to finish. That means, by the time podman returns it should have cleaned up all resources. See the very explicit contract details in https://www.freedesktop.org/software/systemd/man/systemd.service.html#ExecStop=.

If you see any meaningful behavior changed by adding KillMode=none here, it likely means you have a race-y or non-synchronous cleanup logic in podman, which should probably be investigated/fixed.

You can use podman-logs or set Type=simple (plus changing all fork-specific settings). However, we encourage using the fork type to make sure that systemd is monitoring the correct process (via PIDFile).

Well yes, that was the question :)
If this becomes a simple or notify service then logging works, but that's not what you recommend. So there is still a gap between "this is the recommend service unit" and "logs are present in the journal". In which direction do you want to solve that?

@vrothberg
Copy link

If you see any meaningful behavior changed by adding KillMode=none here, it likely means you have a race-y or non-synchronous cleanup logic, which should probably be investigated/fixed.

There was a BZ for that case where systemd and Podman were racing sending signals, which may be related to different timeouts. @mheon, do you recall which? Anyhow, I think it's reasonable to let only Podman take care of that and silence systemd.

If this becomes a simple or notify service then logging works, but that's not what you recommend. So there is still a gap between "this is the recommend service unit" and "logs are present in the journal". In which direction do you want to solve that?

It's the first time we get that request. If you want to see it happen, would you open an issue against libpod? I assume that we could get it to work via --log-driver=journald and --log-opt tag=... but I have not experimented with it.

@jlebon
Copy link
Member

jlebon commented Jul 20, 2022

Where do we stand on this? There were some discussions above about aspects of the unit. Two years have passed now so I suspect podman generate systemd output has changed. There's also quadlet now which might be the end goal interface for this.

@vrothberg
Copy link

Where do we stand on this? There were some discussions above about aspects of the unit. Two years have passed now so I suspect podman generate systemd output has changed.

Yes, the output has changed since then.

There's also quadlet now which might be the end goal interface for this.

There are plans to integrate quadlet into containers/podman so they can be shipped and maintained together. Starting with Podman v4.2, we can also run Kubernetes YAML in systemd via Podman: https://github.com/containers/podman/blob/main/docs/source/markdown/podman-generate-systemd.1.md#kubernetes-integration

@travier
Copy link
Member

travier commented Jul 25, 2022

There are plans to integrate quadlet into containers/podman so they can be shipped and maintained together.

This would be great!

@jlebon
Copy link
Member

jlebon commented Jul 25, 2022

Thanks @vrothberg for the update.

OK, so this PR is stale. I'm going to close it given that it's been open for a while. But @zethsqx (or anyone else) feel free to re-open this with an updated service that we can document for now.

@jlebon jlebon closed this Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants