Restart policy users #1039

jelly · 2022-07-15T18:08:43Z

Restart policy available for users with lingering enabled

Although lingering is not recommended, cockpit-podman now supports setting a restart policy when a user has enabled lingering.

jelly · 2022-07-15T19:23:50Z

src/ImageRunModal.jsx

@@ -353,6 +354,8 @@ export class ImageRunModal extends React.Component {
    componentDidMount() {
        this._isMounted = true;
        this.onSearchTriggered(this.state.searchText);
+        cockpit.file(`/var/lib/systemd/linger/${this.props.user}`).read()
+                .then((content, tag) => this.setState({ userLingeringEnabled: content !== null && tag !== '-' }));


Alternative if we don't want to depend on this file existing, call

[jelle@t14s][~/projects/systemd/build]%loginctl show-user jelle | grep Linger Linger=yes

Or use the DBUS interface:

But we need to get an uid then.

[jelle@t14s][~/projects/systemd/build]%sudo busctl call org.freedesktop.login1 /org/freedesktop/login1 org.freedesktop.login1.Manager GetUser u 1000 o "/org/freedesktop/login1/user/_1000" [jelle@t14s][~/projects/systemd/build]%busctl get-property org.freedesktop.login1 /org/freedesktop/login1/user/_1000 org.freedesktop.login1.User Linger b true

Although, maybe this should watch? As now a user has to reload the browser to notice changes.

Yeah, with very few exceptions we don't require a user to reload the browser for any other changes in Cockpit, really. (The only one I can think of is rebooting in Cockpit, and we should really fix that with a little bit of "PWA" magic. But that's completely out of scope for Cockpit-Podman. 😉 )

But we need to get an uid then

what do you mean? you need the uid for building the path as well, and that path isn't a public API. D-Bus property would really be better, and you should also get a PropertyChanged message for it?

Looking at https://freedesktop.org/wiki/Software/systemd/logind/ I am not sure if we even get a PropertyChanged event as the page only mentions PropertyChanged where it seems to emit it?

Tried:

┃ const loginmanager = client.proxy(); ┃ loginmanager.addEventListener("changed", (changes) => console.log(changes)); ┃ client.subscribe({ E interface: "org.freedesktop.DBus.Properties", ■ Expected indentation of 12 spaces but found 13. E member: "PropertiesChanged" ■ Expected indentation of 12 spaces but found 13. E }, (changes) => console.log(changes)); ■ Block must not be padded by blank lines.

But does not seem to work

OK, meh. If you want to implement reacting to changes, can you please file a systemd issue about the missing PropertiesChanged signal, and describe our workaround? Then we can add a file watch to that directory with a # HACK coomment. This then needs a test to tell us when it stops working (like, the file moves someplace else).

FTR, https://www.freedesktop.org/wiki/Software/systemd/logind/ explicitly documents the signals that do generate a PropertiesChanged. So it's "intended" in some sense. We can still ask for this if it's useful, of course.

I added a new test, but maybe that's unrequired and we can just check if the file exists? It feels a bit expensive to keep adding tests.

Not sure what you mean by this, but indeed this whole PR is a lot of effort. Honestly I don't quite like that the linger file hack is now at two different places. This shouldn't be necessary -- .watch() always fires its callback immediately with the current file contents.

(FWIW, I still consider this whole feature "unrequired" -- I guess you better land this while I'm on PTO and not watching 😁 ))

martinpitt · 2022-08-09T13:33:19Z

test/check-application

@@ -24,6 +24,9 @@ registries = ['localhost:5000', 'localhost:6000']

 NOT_RUNNING = ["Exited", "Stopped"]

+# Ubuntu 2204 lacks user systemd units https://github.com/containers/podman/commit/9312d458b4254b48e331d1ae40cb2f6d0fec9bd0


Weird .. I remember that bug, and the fix landed two years ago. That isn't a good reference, if it still doesn't work there, it broke again.

This is what I get on our image:

admin@ubuntu:~$ cat /etc/os-release | grep VERSION VERSION_ID="22.04" VERSION="22.04.1 LTS (Jammy Jellyfish)" VERSION_CODENAME=jammy admin@ubuntu:~$ systemctl --user start podman-restart Failed to start podman-restart.service: Unit podman-restart.service not found.

admin@ubuntu:~$ podman --version podman version 3.4.4

I can't find a bug report here https://launchpad.net/ubuntu/+source/libpod/+bugs should I report a new one?

Yes please. From the changelog it doesn't look intended.

martinpitt · 2022-08-10T16:45:18Z

test/check-application

@@ -2028,15 +2031,28 @@ class TestApplication(testlib.MachineCase):
    def testHealthcheckUser(self):
        self._testHealthcheck(False)

-    def testPodmanRestartEnabled(self):
+    @testlib.skipImage("podman-restart not available in debian/ubuntu", *DISTROS_WITHOUT_PODMAN_USER_RESTART)


It does work fine in Debian, right? so somehow the package gets misbuilt in Ubuntu.

martinpitt · 2022-08-10T16:47:18Z

src/app.jsx

+                    [uid], { type: "u" })
+                .then(([userpath]) => {
+                    client.call(userpath, "org.freedesktop.DBus.Properties", "Get",
+                                ["org.freedesktop.login1.User", "Linger"], { type: "ss" }).then(([result]) => {


Please always break the line before .then, so that the .catch aligns and this becomes readable.

martinpitt · 2022-08-10T16:48:24Z

test/check-application

+        b.wait_visible('#containers-images td[data-label="Image"]:contains("busybox:latest")')
+        b.click('#containers-images tbody tr:contains("busybox:latest") .ct-container-create')
+        b.wait_visible('div.pf-c-modal-box header:contains("Create container")')
+        b.wait_visible("#run-image-dialog-restart-policy")


Hmm, this is visible even if podman-restart.service isn't available?

Good point, we do check for it for the system service, but not the user service yet. Also a good point is to check when we can drop that..

src/app.jsx

This allows users who have lingering enabled to set a restart policy to automatically restart a container on boot for example. We don't want to enable lingering automatically as this is a security concern. Systemd's logind dbus API has no subscription mechanism so we depend on systemd's implementation detail. systemd/systemd#22244 Closes cockpit-project#921

marusak

Thank you!

follow up

jelly commented Jul 15, 2022

View reviewed changes

jelly force-pushed the restart-policy-users branch 3 times, most recently from 2b123ab to 17708f5 Compare July 18, 2022 14:56

jelly marked this pull request as ready for review July 18, 2022 14:57

jelly force-pushed the restart-policy-users branch from 17708f5 to 2673894 Compare July 19, 2022 15:03

jelly requested a review from marusak July 20, 2022 07:32

marusak force-pushed the restart-policy-users branch from 2673894 to 6766810 Compare July 25, 2022 07:23

martinpitt reviewed Aug 9, 2022

View reviewed changes

jelly force-pushed the restart-policy-users branch 2 times, most recently from 2ab42f0 to 76844e5 Compare August 9, 2022 18:07

jelly mentioned this pull request Aug 10, 2022

logind: Add PropertiesChanged mechanism systemd/systemd#22244

Open

jelly force-pushed the restart-policy-users branch from 76844e5 to f50b84c Compare August 10, 2022 09:43

martinpitt reviewed Aug 10, 2022

View reviewed changes

martinpitt previously requested changes Aug 10, 2022

View reviewed changes

src/app.jsx Show resolved Hide resolved

jelly force-pushed the restart-policy-users branch from 4ac5275 to 16e54ee Compare August 11, 2022 14:26

martinpitt reviewed Aug 11, 2022

View reviewed changes

src/app.jsx Outdated Show resolved Hide resolved

src/app.jsx Outdated Show resolved Hide resolved

jelly force-pushed the restart-policy-users branch from 16e54ee to fe3082b Compare August 15, 2022 09:29

jelly force-pushed the restart-policy-users branch from fe3082b to c9691ba Compare August 16, 2022 16:37

marusak approved these changes Aug 18, 2022

View reviewed changes

jelly merged commit 6d5770c into cockpit-project:main Aug 19, 2022

jelly deleted the restart-policy-users branch August 19, 2022 07:47

marusak added the release-note label Aug 23, 2022

skobyda removed the release-note label Sep 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restart policy users #1039

Restart policy users #1039

jelly commented Jul 15, 2022 •

edited

jelly Jul 15, 2022

jelly Jul 18, 2022

garrett Jul 25, 2022 •

edited

martinpitt Aug 9, 2022

jelly Aug 9, 2022

jelly Aug 9, 2022

martinpitt Aug 10, 2022

martinpitt Aug 10, 2022

jelly Aug 10, 2022

martinpitt Aug 10, 2022

martinpitt Aug 9, 2022

jelly Aug 10, 2022 •

edited

jelly Aug 10, 2022

martinpitt Aug 10, 2022

martinpitt Aug 10, 2022

martinpitt Aug 10, 2022

martinpitt Aug 10, 2022

jelly Aug 11, 2022

marusak left a comment

		@@ -24,6 +24,9 @@ registries = ['localhost:5000', 'localhost:6000']

		NOT_RUNNING = ["Exited", "Stopped"]

		# Ubuntu 2204 lacks user systemd units https://github.com/containers/podman/commit/9312d458b4254b48e331d1ae40cb2f6d0fec9bd0

Restart policy users #1039

Restart policy users #1039

Conversation

jelly commented Jul 15, 2022 • edited

Restart policy available for users with lingering enabled

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garrett Jul 25, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelly Aug 10, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marusak left a comment

Choose a reason for hiding this comment

jelly commented Jul 15, 2022 •

edited

garrett Jul 25, 2022 •

edited

jelly Aug 10, 2022 •

edited