Add services boot overview #19455
Add services boot overview #19455
Conversation
QazCetelic
changed the title
|
👋 I'd really appreciate a code review on my PR whenever you get a chance. If you're busy, just a quick ETA would be awesome. Thanks a bunch! |
|
Hi, any update on the review? @garrett @martinpitt |
There was a problem hiding this comment.
Thanks @QazCetelic , this is a cool feature! Sorry for taking so long to review, holidays and all. I have a few initial comments. Most importantly, this feature needs to add some integration test in test/verify/check-system-services. This should cover at least system/session mode, validate that there is something sensible printed in the card/legend, and contains at least one well-known running service (or grab one from calling systemd-analyze and make sure it appears). I'm happy to assist with that, of course. But do you want to give it an initial shot? Please see https://github.com/cockpit-project/cockpit/blob/main/test/README.md
For the next iteration, can you please git fetch origin; git rebase origin/main instead of merge commits? Updating this branch should fix the RPM build failures.
pkg/systemd/bootInfo.jsx
Outdated
| const [plot, legend] = doc.querySelectorAll("g"); | ||
| legend.remove(); | ||
| [...plot.querySelectorAll("text.left"), ...plot.querySelectorAll("text.right")].forEach((text) => { | ||
| const match = text.innerHTML.match(/^(?<service>.+\.[a-z._-]+)(\s+\((?<time>\d+(\.\d+)?)\w+\))?$/); |
There was a problem hiding this comment.
In my version of systemd-analyze (Fedora 39) the output SVG doesn't contain <service> tags. The services look like
<text class="left" x="264.191" y="2674.000">initrd-parse-etc.service (52ms)</text>
<rect class="activating" x="264.614" y="2680.000" width="0.000" height="19.000" />
<rect class="active" x="264.614" y="2680.000" width="84.109" height="19.000" />There was a problem hiding this comment.
This code doesn't look for a <service> tag. Are you perhaps confusing the named capture group with an HTML tag?
There was a problem hiding this comment.
Ah right, I did. But this looks even more brittle then -- processing XML with line-based REs sounds very brittle.
There was a problem hiding this comment.
It doesn't process any XML.
I'll use your example from above to explain
Here's a text element that is processed.
<text class="left" x="264.191" y="2674.000">initrd-parse-etc.service (52ms)</text>The code gets the inner HTML, which is initrd-parse-etc.service (52ms) (innerText would return undefined). Then parses that using the regex.
The 3 named regex capturing groups then contain the following values:
| Group | Value |
|---|---|
| service | initrd-parse-etc.service |
| time | 52 |
| time_unit | ms |
Note: I changed the regex to split time and the unit with the latest commit
It won't matter if other text elements are added by systemd-analyze later with the same class because it won't do anything unless the format matches.
pkg/systemd/services.jsx
Outdated
| { activeTab == "boot" | ||
| ? <BootInfo /> | ||
| : <ServicesPageBody |
There was a problem hiding this comment.
This feels bolted on -- all other tabs are unit types, this one is completely different. I could live with it, though.
Perhaps we should instead have a separate "Boot chart" button? @garrett any opinion/idea?
There was a problem hiding this comment.
I understand what you mean, but I wouldn't know where else to put it. @garrett do you have a suggestion?
libssh 0.10.6 made host name parsing stricter. `some_host` is not a valid general host name, and is rejected with the latest version.
Bumps the esbuild group with 2 updates: [esbuild](https://github.com/evanw/esbuild) and [esbuild-wasm](https://github.com/evanw/esbuild). Updates `esbuild` from 0.19.10 to 0.19.11 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.19.10...v0.19.11) Updates `esbuild-wasm` from 0.19.10 to 0.19.11 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.19.10...v0.19.11) --- updated-dependencies: - dependency-name: esbuild dependency-type: direct:development update-type: version-update:semver-patch dependency-group: esbuild - dependency-name: esbuild-wasm dependency-type: direct:development update-type: version-update:semver-patch dependency-group: esbuild ... Signed-off-by: dependabot[bot] <[email protected]>
The most recent RHEL 9.4 cloud images added the "rhc" package. This currently contains a hack to locally modify the SELinux policy [1], so we need to ignore this. At least we can drop the insights_client hack, it was fixed properly in https://bugzilla.redhat.com/show_bug.cgi?id=2226684 [1] https://issues.redhat.com/browse/RHEL-20352
This unbreaks the Shell's SSH support with the latest round of OpenSSH 0.9.6 security updates.
… 255 output format Commit 800c89f fixed this for check-static-login already. Debian testing now got systemd 255 and ran into this changed output format as well.
This reverts commit 3956fac.
There is a race between the kernel and lvconvert which might cause lvconvert to hang when the kernel is done repairing before lvconvert has resumed all device mapper devices. Let's try to slow down the repair process by making the disks bigger. See https://bugzilla.redhat.com/show_bug.cgi?id=2256433
Bumps [sass](https://github.com/sass/dart-sass) from 1.69.5 to 1.69.6. - [Release notes](https://github.com/sass/dart-sass/releases) - [Changelog](https://github.com/sass/dart-sass/blob/main/CHANGELOG.md) - [Commits](sass/dart-sass@1.69.5...1.69.6) --- updated-dependencies: - dependency-name: sass dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
When no files exist in the directory the existing code would try to use the * as a file: kkoukiou@o2:~/k$ for f in *; do echo $f; done * Instead let's list the files with `ls` to avoid this corner case as this breaks anaconda tests.
Bumps [sass](https://github.com/sass/dart-sass) from 1.69.6 to 1.69.7. - [Release notes](https://github.com/sass/dart-sass/releases) - [Changelog](https://github.com/sass/dart-sass/blob/main/CHANGELOG.md) - [Commits](sass/dart-sass@1.69.6...1.69.7) --- updated-dependencies: - dependency-name: sass dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]>
This didn't help after all. This reverts commit 4b8d7e7.
It still fails way too often. Bring back the skip, and add the bug reference.
On loading the page, the "Disallow interactive password" checkbox is initially disabled, with a `null` value which shows up as [-]. Wait until it is initialized before taking the pixel test. Fixes cockpit-project#19787
Avoid returning a -1 exec ID in getFrameExecId(), as it is an invalid/unknown context. This got triggered by the changes on the latest Grafana's login page, which creates a short-lived frame with two execution contexts which immediately go away again. Also simplify the code a bit -- it's perfectly legit to look up an `undefined` key in a dictionary and get `undefined` as result.
Our manual addScriptToEvaluateOnNewDocument() is asynchronous, so it can happen that a `Browser.open(); Browser.wait_*()` command sequence tries to call `ph_wait_cond()` which doesn't actually exist in the page JS environment just yet. Catch this exception and retry the condition, similar to the various handled `RuntimeError`s.
The latest Grafana's login page creates two short-lived "about:blank" and "about:srcdoc" frames with empty `""` names, and two execution contexts which immediately go away again. This confuses our frame ID → frameIdToContextId so that they try to run our CDP commands in these wrong frames. Ignore them.
…ilableUpdates This test often fails because the "Reboot scheduled" message does not reliably appear in dnf-automatic-install.service's journal. The scheduled reboot still works, though. Robustify this by replacing the journal scraping with a direct check for a scheduled reboot. Newer systemd versions offer `shutdown --show` for this, which exits with 1 and prints "No scheduled shutdown", or exits with 0 and prints "Reboot scheduled for <date>...". On RHEL8 we already have the `/run/nologin` check, which tests this enough. (Also, RHEL 8 support will drop in less than a month, so not important any more). Drop the obsolete "Shutdown" absence check from the case where no shutdown should be scheduled. Recent systemd says "Reboot" (so this was a no-op), and checking for an absent /run/nologin is sufficient. To avoid destroying the testbed, ensure that the test always cancels the scheduled shutdown even if the check fails. Fixes cockpit-project#19812
Bumps [date-fns](https://github.com/date-fns/date-fns) from 3.0.6 to 3.1.0. - [Release notes](https://github.com/date-fns/date-fns/releases) - [Changelog](https://github.com/date-fns/date-fns/blob/main/CHANGELOG.md) - [Commits](date-fns/date-fns@v3.0.6...v3.1.0) --- updated-dependencies: - dependency-name: date-fns dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
In the btrfs PR this logic was copied over again and then refactored in a function.
|
@martinpitt thanks for the response. I'm a bit busy myself right now, so I was only able to partially process the feedback. I will follow up another time. |
|
Created new branch and pr at #20085 |
Page that shows the time spent on each service during the boot process using
systemd-analyze. It can be used to debug a slow boot process.The service detail page is shown when a service is clicked in the chart.
Loading state
Failure state
This should never occur, but has been added just in case.
