Automatically start podman.socket #1976
Conversation
martinpitt
force-pushed
the
auto-start-socket
branch
from
e714b98 to
58cd691
Compare
martinpitt
force-pushed
the
auto-start-socket
branch
from
58cd691 to
8075969
Compare
|
This failure is completely independent, some internal xterm bug:
Not sure why it triggers so robustly now, changed timing or 3x affected? It rings a bell, need to check when I'm more awake. https://stackoverflow.com/questions/78116360/xtermjs-cannot-read-properties-of-undefined-reading-dimensions-v5-3-on-next |
martinpitt
force-pushed
the
auto-start-socket
branch
from
8075969 to
f8565d1
Compare
martinpitt
force-pushed
the
auto-start-socket
branch
from
f8565d1 to
607991d
Compare
src/ContainerLogs.jsx
Outdated
| @@ -80,11 +80,14 @@ class ContainerLogs extends React.Component { | |||
| } | |||
|
|
|||
| resize(width) { | |||
| if (!this.term?._core?._renderService?.dimensions) | |||
There was a problem hiding this comment.
This is a bit akward as this now depends on internals. Which I see, it already did...
martinpitt
force-pushed
the
auto-start-socket
branch
from
607991d to
d7de01a
Compare
src/app.jsx
Outdated
| }); | ||
|
|
||
| // Listen if podman is still running |
There was a problem hiding this comment.
Unrelated to code review, but why does this exist? I suppose to check if streaming events works as subscribing to streamEvents does nothing until an event comes along
There was a problem hiding this comment.
No, not at all. This is meant to update the state if/when podman.service/.socket gets stopped, i.e. the "close" method. I updated the comment, is it clearer now?
There was a problem hiding this comment.
Ah, but then we can in theory re-use podmanMonitor for this? Except it doesn't expose the channel so we can't easily add an eventlistener to close()
There was a problem hiding this comment.
Seems there are some complications however, this is unrelated to your work.
We will soon add another piece of initialization at the top, and this already has too many indentation levels.
Always start the system and user `podman.socket` unit on initialization. There really is no reason to explicitly ask the user about it -- we can treat it as "extended cockpit" to just access podman. There also isn't a reason to enable the socket unit -- starting it on demand is fine from cockpit-podman's perspective. Now the user will only see the empty state if the service fails, which should be very rare. So turn the Troubleshoot button into a primary one. We also don't need the "System/User podman is also available" alerts any more -- gaining root privileges auto-starts the system service.
martinpitt
force-pushed
the
auto-start-socket
branch
from
d7de01a to
f4a4865
Compare
| this.updatePods(system); | ||
|
|
||
| client.streamEvents(system, message => this.handleEvent(message, system)) | ||
| .catch(e => console.error("init", system ? "system" : "user", "streamEvents failed:", e.toString())) |
There was a problem hiding this comment.
This added line is not executed by any test.
| client.streamEvents(system, message => this.handleEvent(message, system)) | ||
| .catch(e => console.error("init", system ? "system" : "user", "streamEvents failed:", e.toString())) | ||
| .finally(() => { | ||
| this.setState({ [system ? "systemServiceAvailable" : "userServiceAvailable"]: false }); |
There was a problem hiding this comment.
This added line is not executed by any test.
| console.log("init", system ? "system" : "user", "podman service closed"); | ||
| this.setState({ [system ? "systemServiceAvailable" : "userServiceAvailable"]: false }); |
There was a problem hiding this comment.
These 2 added lines are not executed by any test.
Always start the system and user
podman.socketunit on initialization. There really is no reason to explicitly ask the user about it -- we can treat it as "extended cockpit" to just access podman.There also isn't a reason to enable the socket unit -- starting it on demand is fine from cockpit-podman's perspective.
Now the user will only see the empty state if the service fails, which should be very rare. So turn the Troubleshoot button into a primary one.
We also don't need the "System/User podman is also available" alerts any more -- gaining root privileges auto-starts the system service.
This is a necessary prerequisite for https://issues.redhat.com/browse/COCKPIT-1086 , but useful in its own right -- it removes a big hurdle/annoyance at startup.
Podman: Automatically start podman.socket
If the system or user's
podman.socketunit isn't running, cockpit-podman now silently starts it on its own, instead of asking the user about it. There is no need to have the service enabled (i.e. always start it on boot or user login) for cockpit-podman any more.