debug flatpak test failure #20
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This reliably reproduces in an interactive tmate session, but not in a local
I can take out the first part of the test (before "Check if we can start via pure DBus activation"), this keeps the behaviour (both local success and GH hang/timeout). This removes some noise and speeds up iteration. I added the following debug patch: --- a/src/client/cockpit-client
+++ b/src/client/cockpit-client
@@ -232,9 +232,11 @@ class CockpitClient(Gtk.Application):
self.activate()
def quit_action(self, *args):
+ print("XXXX quit", args)
self.quit()
def open_path(self, action, parameter, *args):
+ print("XXXX open_path", action, parameter, args)
CockpitClientWindow(self, self.uri + parameter.get_string()).present()
def do_activate(self):Locally, with and when it's slow, it looks like this: it always times out on GH, without even reaching the callbacks: So that "XXX open_path" callback is confusing -- that shouldn't happen after "quit", and the previous I don't see a way to change the 25s default D-Bus timeout from outside (env variable or |
martinpitt
force-pushed
the
debug-flatpak
branch
3 times, most recently
from
bcbf97a to
be2761d
Compare
|
Meh, now it changed to time out even earlier, in the first part of the test. |
martinpitt
force-pushed
the
debug-flatpak
branch
from
be2761d to
6011868
Compare
|
In my interactive machine, at least the -ssh test works again if I revert cockpit-project#19689 -- oddly, that PR did not have a flatpak-test run ❓ Reverting it definitively changes the failure mode, so it is related. But it's not hunky-dory: The tests still hang a lot, and just fails differently. |
martinpitt
force-pushed
the
debug-flatpak
branch
2 times, most recently
from
ddb9a4e to
3768fed
Compare
Bumps the patternfly group with 5 updates: | Package | From | To | | --- | --- | --- | | [@patternfly/react-core](https://github.com/patternfly/patternfly-react) | `5.1.1` | `5.1.2` | | [@patternfly/react-icons](https://github.com/patternfly/patternfly-react) | `5.1.1` | `5.1.2` | | @patternfly/react-styles | `5.1.1` | `5.1.2` | | [@patternfly/react-table](https://github.com/patternfly/patternfly-react) | `5.1.1` | `5.1.2` | | [@patternfly/react-tokens](https://github.com/patternfly/patternfly-react) | `5.1.1` | `5.1.2` | Updates `@patternfly/react-core` from 5.1.1 to 5.1.2 - [Release notes](https://github.com/patternfly/patternfly-react/releases) - [Commits](https://github.com/patternfly/patternfly-react/compare/@patternfly/[email protected]...@patternfly/[email protected]) Updates `@patternfly/react-icons` from 5.1.1 to 5.1.2 - [Release notes](https://github.com/patternfly/patternfly-react/releases) - [Commits](https://github.com/patternfly/patternfly-react/compare/@patternfly/[email protected]...@patternfly/[email protected]) Updates `@patternfly/react-styles` from 5.1.1 to 5.1.2 Updates `@patternfly/react-table` from 5.1.1 to 5.1.2 - [Release notes](https://github.com/patternfly/patternfly-react/releases) - [Commits](https://github.com/patternfly/patternfly-react/compare/@patternfly/[email protected]...@patternfly/[email protected]) Updates `@patternfly/react-tokens` from 5.1.1 to 5.1.2 - [Release notes](https://github.com/patternfly/patternfly-react/releases) - [Commits](https://github.com/patternfly/patternfly-react/compare/@patternfly/[email protected]...@patternfly/[email protected]) --- updated-dependencies: - dependency-name: "@patternfly/react-core" dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patternfly - dependency-name: "@patternfly/react-icons" dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patternfly - dependency-name: "@patternfly/react-styles" dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patternfly - dependency-name: "@patternfly/react-table" dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patternfly - dependency-name: "@patternfly/react-tokens" dependency-type: direct:production update-type: version-update:semver-patch dependency-group: patternfly ... Signed-off-by: dependabot[bot] <[email protected]>
In the case that we have a filesystem mounted by a user without an fstab entry, Cockpit warns us that this won't be re-mounted after a restart. The option is given to unmount the filesystem which leads to an error as it uses the `old_dir` the fstab mountpoint which is undefined. As the `mounted-no-config` condition can happen without an fstab entry and the `no-mount-on-boot` only with an fstab entry explicitly provide the directory to unmount to do_unmount. With this behaviour we don't fall back to other when we shouldn't but raise an error which should never happen (if everything is well covered).
Commit 747e7c2 renamed send_message() to send_json(). Relax the pattern to get along with either (so that this also works for older cockpit versions in e.g. cockpit-machines tests).
flake8 6.0 (current in Fedora 39 and our tasks container) complains about these. Our unit tests still run version 5.0.4 which does not yet see this.
Add a new Browser.get_pf_progress_value() utility method for this.
When the CPU is busy, the "Usage" card shows a warning icon next to the progress bar meter, which shifts the layout. Fixes cockpit-project#19614
When the bridge shuts down in an orderly fashion from an EOF on stdin, we signal the endpoints to exit, wait for them to do so, and only close the connection after that's done. When the connection closes suddenly, such as from a BrokenPipeError, we don't do this waiting around. The result is that the endpoints are never signalled to clean themselves up, and (crucially) we don't wait for them to do so. Let's move the communicate() method from the protocol level to the router so that we can make it aware of endpoints. Then we make two changes: - if we didn't already call eof_received() then make sure it happens on disconnect of the transport. This is what is responsible for asking the endpoints to close. - from the end of communicate(), unconditionally wait for all endpoints to have closed down. This way we'll never get endpoints still running after the event loop has closed. We accomplish that by adding a bit of redundant state: an asyncio.Event which can be awaited, and tracks if we have no endpoints left. We also drop calling .shutdown() on all of the routing rules. This is no longer necessary, as we're signalling all of the endpoints directly. A minor tweak is needed to a test which was using communicate() on a subclass of CockpitProtocolServer — it can derive from Router now. This should remove all of the warnings we've been seeing about "Event loop is closed" and "Task was destroyed but it is pending". Closes cockpit-project#18355
|
With during that time there are no actual callback calls in cockpit-client, and it also times out waiting for it to make the SSH connection. The journal also doesn't say anything for that time. When adding This really feels like a flatpak bug at this point.. |
Commit c7c54d4 fixed that. Fixes cockpit-project#18386
The latter is always present, even for rows that have no real usage displayed, just to get the spacing right.
This call was introduced in commit 483027a, but this API only exists since glib 2.72, while current RHEL 9.3 still has 2.68. This causes client to crash with an AttributeError. Provide a fallback implementation. https://issues.redhat.com/browse/RHEL-15020
…g a host This ought to happen somewhere between the shell and the bridge, but is difficult to fix. In the meantime, explicitly kill existing ssh connetions to the target host when removing a host from the shell. See cockpit-project#19672 This is going to be important for testing more remote host specification variants, which will happen in the next commit.
…" field When both are given, the explicitly given "User name:" field should win, and the `user@` part of the connection string should get ignored. This matches what ssh does with `ssh -l someone user@host`, which will connect as `someone`. generate_connection_string() gets along fine with an undefined `user` argument, so we can always call it to remove the special case.
This fixes adding a remote host with address `ssh://hostname` and a non-empty "User name" field. Fixes cockpit-project#17635 Co-Authored-By: Martin Pitt <[email protected]>
Otherwise the restart of kdump.service migth sometimes not regenerate the initrd since the old initrd has the same timestamp as the new kdump.conf. Fixes cockpit-project#19714
We removed support for `.min.` filenames because we thought that nobody was using it anymore, but there are still external packages that make use of it. Add it back again, but only for the case where the non-minified version isn't present. The minified version can always be explicitly requested via its full filename (minus `.gz`, if applicable). Add a bit of debugging output and some test cases to make sure we handle the combinations properly. Fixes https://issues.redhat.com/browse/RHEL-18861
Make testMountingHelp more re-usable by using one variable for the filesystem card header identifier.
Running just "make distcheck" somehow misses regressions in test/static-code.
It is broken in testing.
Ubuntu ≥ 23.04 (mantic) integrates netplan into NetworkManager much more tightly. The primary config is now in netplan. That particular check just concerns our internal error handling of the networking page, and it's enough to run it on one OS. Skip this on all Ubuntus, so that we won't trip over this again once we move ubuntu-2204 to -2404.
While device major number 9 is mdraid, device mapper is *not* defined to have major number 253. That was just happenstance, but it's wrong both ways: On Ubuntu 23.04's kernel, devmapper devices sometimes have 252, and virtio_blk devices sometimes have 253. That lead to "real" disk drives to get ignored and thus the metrics page does not show any disk I/O for them. However, all device-mapper devices are always called "dm-*" in the kernel, and naming happens through symlinks. So we can identify them by name instead of having to rummage arounnd in /sys (which is expensive for every sample). [1] https://www.kernel.org/doc/Documentation/admin-guide/devices.txt
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.8 to 0.19.9 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.19.8...v0.19.9) Updates `esbuild-wasm` from 0.19.8 to 0.19.9 - [Release notes](https://github.com/evanw/esbuild/releases) - [Changelog](https://github.com/evanw/esbuild/blob/main/CHANGELOG.md) - [Commits](evanw/esbuild@v0.19.8...v0.19.9) --- 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]>
For visual consistency, and to avoid a table layout bug when the first column of all rows is empty. Only the "other" entries didn't have one yet. Fixes: cockpit-project#19736
|
Interesting! The flatpak test does still work on the rhel-9.3 branch, e.g. in cockpit-project#19740 - log; so it's not a spurious env change in github, so (1) we can bisect, and (2) have a comparison how broken it was before (not at all, the whole test runs in 10s and there are no hangs). In a local ubuntu-22.04 VM I also get some hangs, though, so it's not very well suited to investigate this 🤯 |
Also move the "show the RUN label" command further up where it belongs. Fixes cockpit-project#19701
At no point pages should have requested an explicit locale specific file like `test.de.js`. This has always been handled internally by the C bridge's packages loader, depending on the currently set locale. Since commit 1f34f76, the Python bridge handles `po.js` specially, and the locale specific files only happen for them. This doesn't belong into the "generic package file" lookup documentation, so just drop that. Also the ordering of `.min` lookups was wrong: older Python bridge versions didn't do this at all, and commit 5d00faa pinned down the ordering to prefer `test.js{,.gz}` over `test.min.js{,.gz}`. Finally, there is no defined preference between `.gz` and plain. Fixes cockpit-project#19720
This test started to fail yesterday and corrupt the VM. The dialog fails (for an unknown reason), `vgroup0` stays busy and becomes unremovable. This doesn't happen in a local `tmt` run, in TF runs against cockpit itself (just in cross-project tests), nor in our fedora-rawhide CI image, and we currently don't get enough debug/journal information to say what's going on. Skip the test on rawhide for the time being, so that we can improve our test machinery and investigate this without causing failure and noise to stratis, selinux, and udisks.
Most UI elements are already disabled/not editable, just these two were left. This creates a bit of noise in the pixel test due to the extra `<Privilege>` wrapper, adjust the reference. Thanks to @g-pape for the initial patch! Fixes cockpit-project#19242
This fixes running a test multiple times on current ubuntu-stable with NM's new netplan backend.
This reverts commit 0d76020.
This reverts commit 1ea2ddc.
This reverts commit c5aa7f6.
martinpitt
force-pushed
the
debug-flatpak
branch
from
3768fed to
55327ee
Compare
|
As this is so difficult to reproduce locally, I am now doing some bisecting. The test started to fail around Dec 5, so I reverted these:
|
martinpitt
force-pushed
the
debug-flatpak
branch
3 times, most recently
from
1e1f4ed to
5f1b3c2
Compare
martinpitt
force-pushed
the
debug-flatpak
branch
from
5f1b3c2 to
197f175
Compare
|
Curiously this has fixed itself. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://github.com/cockpit-project/cockpit/actions/runs/7101988137/job/19335024307?pr=19698