Repository navigation
feat: show Docker and Podman together, plus KVM/libvirt domains - #2
Conversation
|
Follow-up on this PR:
Still one read-only SSH round trip. 52 tests pass. |
|
Thanks for this — and for the follow-up commit. Reviewed against current The short version: the Podman work is good and I would like to take it. Could you split the KVM half into its own PR? Podman: yesThe parser work is the part that makes this real, and it is correct. Swapping the binary alone would have left CPU and memory cards empty, because Podman gives One regression to fix before it landsChanging container warning ids from Not hypothetical: the maintainer's live selection file holds 13 container mutes, ten of them a I have a fix on KVM: separate PR pleaseTwo reasons, neither of them "no". It is a different feature. VMs modelled as It costs more than it looks. Both That matters here specifically because of your own earlier find. The observer effect you diagnosed in #1 is exactly this — the monitor's own connection spiking the remote run queue. The load5 change absorbs it, but this makes the burst it has to absorb about four times larger. A batched call would mostly remove the concern. Minor
Nothing here is merged or closed yet — that is the maintainer's call once the split lands. |
|
Understood. I will consider KVM into a separate PR . I am using agents to most of the work here . Will update as per your suggestions |
WHAT: bumps manifest.json, package.json and pluginVersion to 0.9.0 for the Podman support landed from PR #2 (kanthi's commit a63bfbe, cherry-picked with authorship intact as 77f56c3). WHY: on a host that runs only rootless Podman, the collector probed Docker, found no daemon, and emitted empty container sections, so the panel showed no containers at all. Omarchy ships the Docker client without dockerd, which makes that the common case rather than an edge. SCOPE: Podman-only, as agreed with kanthi on PR #2. The probe is still one first-working engine (sudo docker, docker, sudo podman, podman). The parser handles Podman's differing JSON: Names as an array, CPU as a number, memory as separate byte fields, blank Status. podman0 is excluded from host net counters. Container warning ids are unchanged, so every existing mute still applies. The KVM/libvirt commit (dc13784) stays out pending its own PR, because it adds two virsh spawns per domain per refresh. MEASURED: instrumented shims, rootless-Podman host with six libvirt domains, per refresh: main 1 spawn and no containers shown, this release 6 spawns with containers shown, the full PR 22 spawns of which 16 are virsh. CHECKS: 63 tests pass, 27 of 27 Text elements pinned to PlainText, qmllint clean, omarchy plugin validate clean. LR-T: omarchy-server-status, podman, containers, release, v0.9.0 LR-D: desktop, omarchy, containers LR-K: podman-rootless-collection, pr2-podman-only-split LR-P: release Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thank you, kanthi — the Podman support is merged and released. Your commit landed on Measured before merging, with instrumented shims on a rootless-Podman host running six libvirt domains, per refresh:
It passed 63 tests on current This PR can become the KVM PR. Since
No rush on any of it. Thanks again — between the observer-effect diagnosis in #1 and this, you have made the plugin meaningfully better. |
- Collect Docker and Podman independently so hosts running both show both - Query libvirt domains in a single batched virsh domstats call - Query qemu:///system first and fall back to qemu:///session only when empty - Deduplicate domains by name across URIs - Preserve backwards compatibility for container warning mutes - Bump plugin and manifest version to 0.10.0
dc13784 to
bb3e5bd
Compare
|
Updated as suggested:
|
…ions - Filter containers and KVM domains into appContainers and virtualMachines - Display distinct CONTAINERS and VIRTUAL MACHINES section headers with item counts - Add VmTile component with [KVM] badge, vmDetail formatter, and alert mute toggle - Ensure VM name text in VmTile elides properly with Layout.fillWidth - Add unit test for section separation; passes all 73 tests
|
Added a follow-up commit to visually separate Containers and Virtual Machines in the panel UI:
|
|
Thank you, and thank you for basing it on current main including Fileri's bounded-read hardening. Podman-only hosts showing empty container sections is exactly the kind of gap worth closing, and separating VMs from containers reads better than mixing them. I audited the elevated paths before merging, since a bar plugin reaching for sudo deserves it. Every command that can run under sudo -n is read-only: docker/podman info, ps --all, ps -aq, inspect --format, stats --no-stream, and virsh domstats/dominfo. Nothing that starts, stops, removes or defines anything. The sudo -n count is unchanged from main, so this adds no new escalation, only new read paths. Checked out the branch and ran the suite: 73 pass, 0 fail. Merging. |
Why
The collector already shows Docker containers. Hosts that only run Podman (typical on Omarchy: Docker client present, no
dockerd, rootless Podman socket) produced empty container sections, even whenpodman pslisted running workloads.This is based on current
main(33c8e17, including Fileri's bounded-read / PlainText hardening). Docker-first behaviour is unchanged.Core collector (
backend/collect.ts)This is the agentless SSH script plus the parsers that turn its stdout into the snapshot the panel renders. Nothing is installed on the remote.
1. Runtime probe: Docker first, then Podman
Before:
docker versionfails closed when/var/run/docker.sockis missing (Omarchy ships the Docker client without a daemon). The script then emitted empty@@DOCKER_PS@@/STATS/INSPECTblocks.After, the same four-way probe, still Docker-preferring:
sudo -n docker version— operator accounts outside the docker groupdocker version— docker-group hostssudo -n podman version— rootful Podmanpodman version— rootless Podman for the SSH userThe selected binary is stored in
CTRand used forps --all,stats --no-stream, and the existing narrowinspect --format(no env vars). Section markers stay@@DOCKER_*@@sosplitSections, the snapshot cache, andMAX_CONTAINERScap do not change.readBounded/ SSH byte budgets from the security work are untouched.2. Why a parser change is required (not just swapping the binary)
Podman
--format "{{json .}}"is not Docker's JSON:psNames"web-1"["openclaw"]psStatus"Up 6 days (healthy)"""(useState)statsCPUCPUPerc: "9.39%"CPU: 1.95(number)statsmemoryMemUsage: "1.254GiB / 4GiB"MemUsage+MemLimitas bytesstatsMemPerc/PIDsinspectName"/web-1""openclaw"(no slash; already stripped)If we only switched the CLI, names would mostly work (
String(["openclaw"])happens to stringify), but CPU/memory cards would stay empty becauseCPUPercand"a / b"MemUsageare missing.parseContainersnow:psName()— first element ifNamesis an arraycpuPercentFromStats()—CPUPerc(strip%) or numericCPUmemFromStats()—"used / limit"viaparseSize, or numericMemUsage/MemLimit/MemPercstatusfalls back toStatewhen Podman leavesStatusblankPIDsaccepts string or numberDocker sample fixtures still parse the same (existing
parseContainerstest).ps.slice(0, MAX_CONTAINERS)is unchanged.3. Host net counters
podman0is ignored the same way asdocker0, so the bridge is not counted as physical traffic.Tests
sudo -n docker versionand thepodmanfallback.Names: ["openclaw"], numeric CPU/mem) joins ps/stats/inspect.bun run check: 50 tests).Docs / copy
README,
server-status.tsusage, and the Panel observer-effect comment now say Docker or Podman. Requirements:sudo -n docker/ docker group or a workingpodmanfor the SSH account (rootless is enough).Out of scope
DOCKER_HOSTrewrite.Verified against a live Omarchy host over Tailscale SSH: five rootless Podman containers (
openclaw,hermes,beszel,ntfy,beszel-agent) appear with CPU/mem.