machines: detect if libvirt is running #8493

atiratree · 2018-01-25T19:50:32Z

show error with link to libvirt service if it is not running

this error is not dismissable
fixes machines: detect libvirt stopped #8214

add infrastructure for adding errors to the ui

they are dismissable
will be used in machines: installing vms missing in the UI #8441

atiratree · 2018-01-25T19:52:47Z

pkg/machines/libvirt.es6

@@ -812,6 +850,7 @@ function startEventMonitor(dispatch, connectionName) {
        .fail(ex => {
            // this usually happens if libvirtd gets stopped or isn't running; retry connecting every 10s
            console.log("virsh event failed:", ex);


virsh process can fail or be killed here, but is this console.log still needed?

I'm fine with dropping it; I wanted to keep it in after landing the virsh event stuff, but I figure it's stable enough now.

ok, I will remove it then

martinpitt · 2018-01-25T21:22:35Z

This seems to break all VM tests?

atiratree · 2018-01-26T10:47:36Z

This seems to break all VM tests?

It is because of the layout change (there is no empty vms screen first)
This is fixed in #7820 13b1876

mareklibra · 2018-01-26T11:02:29Z

pkg/machines/components/notificationArea.jsx

+    actionMessage: PropTypes.string.isRequired,
+};
+
+const NotificationArea = ({ errors, dismissable, onDismiss, id }) => {


Can we reuse Alert from inlineNotification.jsx?

Will be resolved after the final design decision

mareklibra · 2018-01-26T11:22:08Z

pkg/machines/hostvmslist.jsx

+            });
+        }
+
+        let notificationArea = errors.length > 0 ? (


Can we move this into the NotificationArea component, so it can properly handle even empty list?

mareklibra · 2018-01-26T11:22:40Z

pkg/machines/hostvmslist.jsx

+
+        if (!isLibvirtActive) {
+            errors.push({
+                message: "Libvirt is not active.",


consider I18N: use _("...")

mareklibra · 2018-01-26T11:25:41Z

pkg/machines/hostvmslist.jsx

+        const errors = [...ui.errors];
+        const isLibvirtActive = libvirtState.status === 'active';
+
+        if (!isLibvirtActive) {


I would make the NotificationArea component responsible for producing this message, just pass state-info to props.

mareklibra · 2018-01-26T11:29:08Z

pkg/machines/libvirt.es6

@@ -56,6 +58,18 @@ if (!String.prototype.startsWith) {
    };
 }

+if (typeof String.prototype.includes === 'undefined') {


Where is this required? Can't we find a way to do it via babel? Or avoid it?

We could use https://babeljs.io/docs/usage/polyfill/

These are required here 3546515#diff-de184ca652af74b4142eac404126b77fR767 and in #7820

String: startsWith, includes

Array: includes

mareklibra · 2018-01-26T11:29:45Z

pkg/machines/libvirt.es6

+    };
+}
+
+if (typeof Array.prototype.startsWith === 'undefined') {


this one was added by mistake

mareklibra · 2018-01-26T11:31:03Z

pkg/machines/libvirt.es6

+    CHECK_LIBVIRT_STATUS() {
+        logDebug(`${this.name}.CHECK_LIBVIRT_STATUS`);
+        return dispatch => {
+            return cockpit.spawn(['systemctl', '--property=ActiveState', 'show', 'libvirtd'])


Is systemctl available on all supported systems?

should be (on their supported versions)
http://cockpit-project.org/running.html

systemctl itself is, but less sure about --property. But systemctl is-active --quiet myunit has been supported for a long time, and is even easier to use as you just need to look at the exit code. Alternatively, use the D-Bus API (GetUnit() and then its ActiveState property).

I looked at the history of systemd man pages and it should be ok for our versions. I will leave it like it is for now. + we are using the string state in the ui so it is not a big difference

mareklibra · 2018-01-26T11:35:34Z

pkg/machines/libvirt.es6

    // set up event monitor for that connection; force PTY as otherwise the buffering
    // will not show every line immediately
    cockpit.spawn(['virsh'].concat(VMS_CONFIG.Virsh.connections[connectionName].params).concat(['-r', 'event', '--all', '--loop']), {'err': 'message', 'pty': true})
        .stream(data => {
+            if (data.startsWith("error: Disconnected from") || data.startsWith("error: internal error: client socket is closed")) {
+                // libvirt failed


can we log here? Just to be sure we do not swallow status info which might be useful when debugging

added debug log to all the places where event monitor fail is expected

atiratree · 2018-01-30T14:21:42Z

New version of design

There is one notification area which consists of:

libvirt warning which is not dismissable and will disappear when fixed
dismissable errors
dismissable info messages

Reasons

There can be multiple errors and messages so I put them in one closable notification

the ui will not be cluttered by many messages
you don't have to click on the cross of each message to clear them all

Design

@garrett @andreasn
can you give me feedback on this pls?

garrett · 2018-01-30T16:12:34Z

Warning: libvirt

"libvirt is not active": Cockpit has a pattern in place, based on PatternFly's empty state pattern to handle things like this:

A customized version for libvirt would look something like this:

This does look a little odd, as Sentence case is used on the header and Title Case is used on the button — but, from what I can tell, libvirt is one of those projects named completely lowercase.

However, in Cockpit, we generally try to use phrases of higher-level concepts instead of specific technology (where appropriate). In this case, the resulting empty state screen might look like the following:

I'm not a fan of jargony phrases like "subsystems", however. @andreasn (or anyone else): Have any better ideas on what to call this?

Dismissable errors & notices

Could you provide some concrete examples of errors and notices that would be used in the inline notifications, please? (Good design requires example content and intent.) Thanks!

atiratree · 2018-01-30T16:34:55Z

I guess empty state makes more sense.

Just one note:

I am not sure this will be the right choice for the ovirt usecase though. Because you can choose to connet to ovirt instead.

Could you provide some concrete examples of errors and notices that would be used in the inline notifications, please? (Good design requires example content and intent.) Thanks!

For now error about vm creation failure. It can take some time and there can be multiple failures at the same time.

there is no usage for notices yet (I just put it into the example), but it could be useful in the future

garrett · 2018-01-30T16:57:39Z

It could use an empty state pattern with a secondary button (btn-standard) for connecting to oVirt. When you click on the button, it would ask for the details in a modal popup

mareklibra · 2018-01-31T12:15:58Z

IMO, we should name libvirt within the empty-state title and not rephrase it to virtualization susbsystem or so. The first one is more clear, the second one is like definition of new terminology.

It's both nice and consistent to have the Start button there causing sort of systemctl start libvirtd.
Anyway, it would be useful to provide a link to the /services view for the case the user needs to inspect logs or even enable the service (means start at boot time).

I am not sure this will be the right choice for the ovirt usecase though. Because you can choose to connet to ovirt instead.

No matter what the application allows now, the cockpit-machines-ovirt is being described as an extension on top of cockpit-machines functionality. Having that on mind, the empty state shall be rendered even here if the libvirt is down.

atiratree · 2018-01-31T14:49:55Z

current state

libvirt status

before libvirt status is known:

when libvirt is down:

error messages

are left the same as before from design perspective
code was refactored to reuse functionality in inlineNotification

garrett

The concept should be expressed in the language. However, I think it's a good compromise to merge our mockups to also mention libvirt in the docs link. (I like your idea to add the link there for further details.)

I've added inline comments in the code.

garrett · 2018-02-05T16:23:01Z

pkg/machines/components/libvirtSlate.jsx

+            icon = (<div className="spinner spinner-lg"/>);
+            break;
+        default:
+            message = _("libvirt is not active on the system");


This should say "Virtualization service is not active", which is more descriptive of what's happening. Not everyone will know what "libvirt" is. (It even sounds confusingly like a library.)

garrett · 2018-02-05T16:26:52Z

pkg/machines/components/libvirtSlate.jsx

+            icon = (<span className="fa fa-exclamation-circle"/>);
+            detail = (
+                <p>
+                    {_("See libvirt service for ")}<a onClick={mouseClick(goToLink)}>{_("more configuration.")}</a>


It's fine to say libvirt here and, in fact, would help to reinforce that libvirt is a virtualization service (and vice versa) based on context.

Perhaps the code should look something like the following:

detail = ( <p> { cockpit.format(_("Please refer to $0 for more information."), `<a href="${goToLink}" target="_blank">libvirt documentation</a>`) } </p> );

(It might not be quite right -- I didn't test it. It's based on the file pkg/ovirt/components/ConsoleClientResources.jsx which has a the link as an actual link (instead of a JavaScript-only action) and the method in which it is written enables the strings to be translatable more easily.)

Docs on cockpit.format() are located @ http://cockpit-project.org/guide/latest/cockpit-util

The reference lines of ConsoleClientResources.jsx are @ https://github.com/cockpit-project/cockpit/blob/master/pkg/ovirt/components/ConsoleClientResources.jsx#L34-L35

I get the reason to do it, but I do not think it is a good idea.
That example is much easier because it is just a link (outside of cockpit).

passing function like that breaks things:
Refused to execute inline event handler because it violates the following Content Security Policy directive: "default-src 'self' http://localhost:9090". Either the 'unsafe-inline' keyword, a hash ('sha256-...'), or a nonce ('nonce-...') is required to enable inline execution.

it is not a react like

translation is problematic; the link can be at various position in other languages

I know; this way it is also not translation friendly

Yeah, I don't know React*. Please listen to someone else on the team with React experience. Treat any React code I write anytime soon to basically be pseudo-code. 😉

garrett · 2018-02-05T16:29:11Z

pkg/machines/components/libvirtSlate.jsx

+            );
+            action = (
+                <div className="blank-slate-pf-main-action">
+                    <button className="btn btn-primary btn-lg" onClick={mouseClick(start)}>{_("Start libvirt")}</button>


This button name should also be descriptive, similar to the header; that is, it should state: "Start Virtualization Service".

The docs link directly above explains that the "virtualization service" is libvirt.

atiratree · 2018-02-06T14:42:29Z

although now, the word "service" is occurring 3 time

atiratree · 2018-02-06T15:29:35Z

Errors: expected usage

atiratree · 2018-02-06T15:38:03Z

Errors: expected usage

for create vm dialog that results in this flow:

wait for few sec and allow user to fix the mistake 1)
close the dialog
error occurs with some delay
show the error in Notification area machines: detect if libvirt is running #8493 (comment)

1)

garrett · 2018-02-07T08:10:56Z

although now, the word "service" is occurring 3 time

Yeah, you're right.

The secondary line should be something more like "Please refer to libvirt documentation for more information."

(The link should be a link that opens in a new window with a target, such as: <a target="_blank" href="libvirt-docs-link-goes-here">link text</a>.)

atiratree · 2018-02-07T11:55:20Z

The secondary line should be something more like "Please refer to libvirt documentation for more information."

I really think there should be link to the service in cockpit. The main reason for that is to enable the service. Clicking the button only makes the service start once.

We could also add the documentation part, but I am not sure if it isn't too much?

mareklibra · 2018-02-07T12:34:41Z

The main reason for that is to enable the service.

I agree, there's already functionality in Cockpit allowing fixing that in most of the cases.
From UX perspective, there might be Troubleshoot secondary action leading the user to the service view.

garrett · 2018-02-07T17:07:40Z

link to the service in cockpit

What do you mean? (I thought it was supposed to be a documentation link.)

The main reason for that is to enable the service. Clicking the button only makes the service start once.

Why not both enable and start the service at the same time?

leading the user to the service view

It wouldn't be helpful to just drop someone in the service view to find something (most people probably wouldn't remember they're looking for "libvirt") and then do something with it. (We know they'd need to enable the service, but would they? Most likely not.)

atiratree · 2018-02-13T12:20:43Z

Why not both enable and start the service at the same time?

I do not think that is good default behaviour, because it might not be what the user wants.

It wouldn't be helpful to just drop someone in the service view to find something (most people probably wouldn't remember they're looking for "libvirt") and then do something with it. (We know they'd need to enable the service, but would they? Most likely not.)

no, it goes exactly to the service detail /system/services#/libvirtd.service
I think linux user should be familiar with the concept of services and what is expected of them to do if they click more configuration -> lands on the libvirt service page

there can be also link to the documentation but I do not think it is necessary

atiratree · 2018-02-20T14:15:12Z

Can we let it in the current state?

I will start writing tests for this.

garrett · 2018-02-20T15:02:23Z

There seem to be multiple things covered in this one PR. Can we split apart this pull request into 3 different items?

This PR: Keep the general error message that has an empty pattern
New, separate PR: Create new dialog error
A design issue where it is discussed how best to handle enable versus start for services (as this impacts other parts of Cockpit as well, such as the Containers page)

I'll open the issue (item # 3).

@suomiy, can you move the dialog part of this PR to a separate one (# 2)?

By splitting it apart, I think it can help to focus on getting this PR in (and then the next too). Thanks!

atiratree · 2018-03-15T16:10:12Z

ubuntu needs to be fixed
name of the service is libvirt-bin instead of libvirtd there

atiratree · 2018-03-20T11:16:18Z

@mareklibra
Can you please rereview? I had to get name of a libvirt service in initialization phase because of ubuntus

mareklibra · 2018-03-20T13:19:12Z

pkg/machines/libvirt.es6

@@ -170,29 +174,47 @@ LIBVIRT_PROVIDER = {
        };
    },

+    INITIALIZE () {


Why not reuse the init method above?
Please note, the init() can return both boolean or Promise.

I think we can fire reading of libvirt service name in init(), store result into redux and reuse it in GET_ALL_VMS

Please align with similar initialization flow in pkg/ovirt

We decided it is better not to mix init() in libvirt and ovirt

mareklibra · 2018-03-20T13:26:34Z

pkg/machines/actions.es6

 */
-export function getAllVms(connectionName) {
-    return virt('GET_ALL_VMS', { connectionName });
+export function getAllVms(connectionName, libvirtServiceName) {


I don't like adding the param here unless really necessarily.
It has nothing to do with the actual read of VMs.

We decided it is fine for now

mareklibra · 2018-03-20T13:32:51Z

pkg/machines/components/libvirtSlate.jsx

+    render() {
+        const service = this.props.libvirtService;
+        const goToLink = () => cockpit.jump("/system/services#/" + service.name);
+        const start = () => {


Can be moved outside the render() function to sort of onStartClicked()

mareklibra · 2018-03-20T13:43:09Z

pkg/machines/index.es6

@@ -50,5 +50,5 @@ export function appMain() {
    render();

    // initiate data retrieval
-    store.dispatch(getAllVms());
+    store.dispatch(initialize());


Important: Please do update pkg/ovirt accordingly as well.

mareklibra · 2018-03-20T13:48:20Z

test/verify/check-machines

    def startLibvirt(self):
        m = self.machine
        # Ensure everything has started correctly
-        m.execute("systemctl start libvirtd")
+        m.execute("systemctl start {0}".format(self.getLibvirtName()))


The tests were passing prior this PR. What has changed that this is needed?

it works, but disabling libvirt will make this stop working in ubuntus where libvirt-bin.service can have a priority. libvirtd.service is an alias when libvirt-bin.service is enabled

atiratree · 2018-03-20T20:11:30Z

fixed review

mareklibra

LGTM

martinpitt

Thanks for working on this! This looks very nice indeed. I found some things to clean up, but the general structure looks fine.

martinpitt · 2018-03-21T07:43:06Z

pkg/machines/libvirt.es6

+        logDebug(`${this.name}.CHECK_LIBVIRT_STATUS`);
+        return dispatch => {
+            return cockpit.spawn(['systemctl', '--property=ActiveState', '--property=UnitFileState', 'show', serviceName])
+                .done(data => {


Please use the systemd D-Bus API for querying and controlling the libvirt service. Have a look at pkg/lib/service.js which provides a nice API for that. It's e. g. being used in pkg/tuned/dialog.js.

martinpitt · 2018-03-21T07:50:56Z

pkg/machines/libvirt.es6

+        if (line.toLowerCase().includes("failed to connect")) {
+            // known error: virsh process fails anyway
+            logDebug(error, line);
+        }else{


(nitpick: missing spaces around else)

martinpitt · 2018-03-21T07:53:54Z

pkg/machines/reducers.es6

+            unitState: 'unknown',
+        },
+        osInfoList: [],
+    };
    state = state ? state : [];


This line is a no-op now and should go away. After the previous hunk state is now always non-empty.

martinpitt · 2018-03-21T08:03:04Z

pkg/machines/scripts/get_libvirt_service_name.sh

+#!/bin/sh
+
+# libvirt-bin is primary in ubuntu 1604
+NAME="`systemctl list-unit-files | grep -o libvirt-bin.service || echo libvirtd.service`"


You can save the grep and the extra systemctl call below with

NAME=`systemctl --no-legend list-unit-files libvirtd.service libvirt-bin.service | head -n1 | cut -f1 -d' '`

This will prefer libvirtd.service over libvirt-bin.service even if the latter is an alias on upgrades.

martinpitt · 2018-03-21T08:07:07Z

test/verify/check-machines

+        name = self.machine.execute(
+            "systemctl list-unit-files | grep -o libvirt-bin.service || echo libvirtd.service").replace('\n', '')
+        return self.machine.execute(
+            "systemctl  --property=Id show {0} | cut -c 4-".format(name)).replace('\n', '')


This should not repeat the same logic. Just special-case if self.machine.image == "ubuntu-1604":

martinpitt · 2018-03-21T08:13:55Z

test/verify/check-machines

+        m.execute("systemctl disable {0}".format(libvirtServiceName))
+        print(self.getLibvirtName())
+        m.execute("systemctl stop {0}".format(libvirtServiceName))
+        b.wait(lambda: not checkLibvirtEnabled())


Definitively no wait needed here -- systemctl stop and disable are synchronous.

martinpitt · 2018-03-21T08:14:14Z

test/verify/check-machines

+        b.click("#start-libvirt")
+
+        b.wait_in_text("body", "Virtual Machines")
+        b.wait(lambda: checkLibvirtEnabled())


Again, once the UI updates (which you already wait for in the previous line), this should succeed right away.

Ui shows up when the service starts. We should not expect the Ui to enable the service atomicly with the start

martinpitt · 2018-03-21T08:14:42Z

test/verify/check-machines

+        b.wait_present("#enable-libvirt:checked")
+        b.set_checked("#enable-libvirt", False)
+        b.click("#start-libvirt")
+        b.wait(lambda: not checkLibvirtEnabled())


If you move this down one line (i. e. wait for the UI to update first), it should again not require waiting.

martinpitt · 2018-03-21T08:16:56Z

test/verify/check-machines

+        b.wait(lambda: url_location in b.eval_js("window.location.href"))
+
+        # HACK: restarting libvirtd causes this completely unrelated SELinux issue
+        self.allow_journal_messages('.*denied.*search.*"systemd-machine".*dev="proc".*')


We generally report them to bugzilla, create a known issue, and add a naughty override for these, so that these issues get fixed and these hacks clean up themselves over time.

martinpitt · 2018-03-21T08:17:53Z

test/verify/check-machines

+        self.allow_journal_messages('.*denied.*search.*"systemd-machine".*dev="proc".*')
+        # enabling/disabling libvirt causes unwanted logs
+        self.allow_journal_messages('.*libvirt.*')
+        self.allow_journal_messages('.*virt.*socket.*')


Sorry, this looks too broad and can easily hide errors. What are the complete messages here, and how do they break the test? We only check for unexpected log messages on cockpit.service and type=14?? audit messages.

I am pretty sure it was complaining about creating/removing symlinks, but now it is fine on my setup.

It only throws this on ubuntu-1604

audit: type=1400 audit(1521636424.736:15): apparmor="DENIED" operation="open" profile="libvirt-a73c87e3-37a7-43c0-8671-b477f5783d95" name="/etc/gss/mech.d/" pid=2069 comm="qemu-system-x86" requested_mask="r" denied_mask="r" fsuid=64055 ouid=0

debian-testing

audit: type=1400 audit(1521637589.708:41): apparmor="DENIED" operation="signal" profile="/usr/sbin/libvirtd" pid=1146 comm="libvirtd" requested_mask="send" denied_mask="send" signal=hup peer="unconfined"

@suomiy : The gss failure is issue #6901, we already have a naughty override for this. We don't yet have one for the "signal" thing.

Removed #6901. Anyway I think --sit should not fail without --thorough option on known issue. This got me confused.

atiratree · 2018-03-21T13:17:28Z

fixed review

martinpitt

Thanks for the updates! Next round of review.

martinpitt · 2018-03-22T11:21:24Z

pkg/machines/scripts/get_libvirt_service_name.sh

+if [ -n "$NAME" ]; then
+    # get id name because libvirt-bin is primary in ubuntu 1604
+    systemctl  --property=Id show "$NAME" | cut -c 4-
+fi


Again, this isn't necessary. On Ubuntu-16.04, the above command already gives you libvirt-bin.service. (But see comment above, I think this file could be eliminated completely).

It can also give libvirtd.service on Ubuntu 16.04. libvirt-bin.service creates alias to libvirtd.service when enabled. To make things more interesting we even cannot prefer libvirt-bin.service because on ubuntu 17.10 libvirtd.service is primary and creates symlink to libvirt-bin.service.

We really need the id of the service, because that is the only service visible when disabled and stopped.

"We really need the id of the service" → What does that mean? How is it in any way different than the $NAME you already determined? (It's demonstrably not on fedora-27 and ubuntu-1604; I didn't try other cases)

I am sorry. It works on ubuntu 16.04 (didn't try this exact command and assumed wrong sort order of those two names).

But it still breaks on ubuntu 17.10:

systemctl enable libvirtd.service systemctl --no-legend list-unit-files libvirtd.service libvirt-bin.service | head -n1 | cut -f1 -d' ' # libvirt-bin.service systemctl disable libvirtd.service systemctl --no-legend list-unit-files libvirtd.service libvirt-bin.service | head -n1 | cut -f1 -d' ' # libvirtd.service

appending to the last comment:

systemctl status libvirt-bin # Unit libvirt-bin.service could not be found.

martinpitt · 2018-03-22T11:23:15Z

pkg/machines/libvirt.es6

+        logDebug(`${this.name}.INIT_DATA_RETRIEVAL():`);
+        return dispatch => {
+            dispatch(getOsInfoList());
+            return cockpit.script(getLibvirtServiceNameScript, null, { err: "message", environ: ['LC_ALL=en_US.UTF-8'] })


As this script is now an one-liner, and you already do some regexp post-processing below, you could just simply call cockpit.spawn() here on systemctl --no-legend list-unit-files libvirtd.service libvirt-bin.service and ditch the script. This makes the code easier to read as everything that happens here is at the same place.

I think we still need the script.

martinpitt · 2018-03-22T11:24:37Z

pkg/machines/libvirt.es6

+            return cockpit.script(getLibvirtServiceNameScript, null, { err: "message", environ: ['LC_ALL=en_US.UTF-8'] })
+                .then(serviceName => {
+                    const match = serviceName.match(/(.+)\n$/);
+                    const name = getRegexGroup(match, 1, 'libvirtd.service'); // fallback to libvirtd to be able to show link in libvirtSlate


I don't think this should fall back in this way. If libvirtd isn't installed (i. e. the above command does not have any result), then this should be treated properly and not ignored. If libvirt isn't installed, then getting its state and VMs is doomed to fail anyway.

Ok I will move this handling to libvirtSlate and leave serviceName empty in the store

martinpitt · 2018-03-22T11:33:42Z

pkg/machines/libvirt.es6

+            dispatch(getOsInfoList());
+            return cockpit.script(getLibvirtServiceNameScript, null, { err: "message", environ: ['LC_ALL=en_US.UTF-8'] })
+                .then(serviceName => {
+                    const match = serviceName.match(/(.+)\n$/);


Change this to get the first word only, this saves the head and cut processing in shell, and drop the unnecessary helper function:

.then(output => { const match = output.match(/(\w+)/); const serviceName = match ? match[0] : null; if (serviceName) { ... get libvirt state } else { console.error (...); // handle faillure } );

martinpitt · 2018-03-22T11:39:11Z

test/verify/check-machines

+        # HACK: restarting libvirtd causes this completely unrelated SELinux issue
+        self.allow_journal_messages('.*denied.*search.*"systemd-machine".*dev="proc".*')
+
+        self.allow_journal_messages('.*audit.*apparmor="DENIED".*operation="signal".*libvirt.*denied_mask="send".*')


These two still need to be replaced with known issues.

should I create these issues and put these messages into naughty overrides in this PR?

Yes, please.

maybe we could reopen this issue #8467 ? It is exactly the same except the signal is hup instead of kill

I'm quite sure the policy is not so specific as to limit particular signals, it's just not allowed to send signals in general. I. e. it's most likely the same bug. But are you sure that it still actually applies? This was fixed recently (certainly way after you started working on this PR), and in the recent test run there is no such apparmor denial. So I think this can just be dropped and should be fine as-is.

I still see it in the logs, but it is fine with me if it is not causing any problems anymore.

atiratree · 2018-03-22T14:43:20Z

fixed except the journal messages (the first one is present in testBasic as well)

- Show spinner before the status is known - Show slate with action if libvirt is down Closes cockpit-project#8493

martinpitt

I did a force-push with some small changes:

Drop the AppArmor expected message (should work just fine now)
Squash code change and test together
Add a Closes # for button mergeability

Thank you, great work!

- Show spinner before the status is known - Show slate with action if libvirt is down Closes #8493

martinpitt · 2018-04-04T07:32:01Z

Current screenshot for release notes:

atiratree commented Jan 25, 2018

View reviewed changes

atiratree force-pushed the libvirt-status-msgs branch from bf0b86c to 62d6045 Compare January 25, 2018 19:55

mareklibra suggested changes Jan 26, 2018

View reviewed changes

atiratree force-pushed the libvirt-status-msgs branch 4 times, most recently from 0f5e0c3 to 07c25ff Compare January 30, 2018 13:52

atiratree force-pushed the libvirt-status-msgs branch from 07c25ff to 7dc6270 Compare January 31, 2018 14:41

atiratree force-pushed the libvirt-status-msgs branch from 7dc6270 to c6f7dbe Compare January 31, 2018 14:50

garrett requested changes Feb 5, 2018

View reviewed changes

atiratree force-pushed the libvirt-status-msgs branch from c6f7dbe to f908378 Compare February 6, 2018 14:40

garrett mentioned this pull request Feb 20, 2018

service not running: enable versus start #8661

Closed

atiratree force-pushed the libvirt-status-msgs branch 4 times, most recently from e16e2bf to fdb4366 Compare March 19, 2018 23:49

mareklibra suggested changes Mar 20, 2018

View reviewed changes

atiratree force-pushed the libvirt-status-msgs branch 2 times, most recently from 3c35182 to 4fe8dca Compare March 20, 2018 20:10

mareklibra approved these changes Mar 21, 2018

View reviewed changes

martinpitt requested changes Mar 21, 2018

View reviewed changes

atiratree force-pushed the libvirt-status-msgs branch from 4fe8dca to f1e3618 Compare March 21, 2018 12:54

atiratree force-pushed the libvirt-status-msgs branch 2 times, most recently from 2378fd7 to 1a0f514 Compare March 21, 2018 14:13

martinpitt requested changes Mar 22, 2018

View reviewed changes

atiratree force-pushed the libvirt-status-msgs branch from 1a0f514 to 9aaef4e Compare March 22, 2018 14:38

atiratree force-pushed the libvirt-status-msgs branch from 9aaef4e to 50a9695 Compare March 22, 2018 16:37

suomiy added 2 commits March 23, 2018 11:13

machines: Detect if libvirt is running

373d772

- Show spinner before the status is known - Show slate with action if libvirt is down Closes cockpit-project#8493

bots: Add naughty override for SELinux systemd-machine search

f7a7b96

martinpitt force-pushed the libvirt-status-msgs branch from 2e89d3c to f7a7b96 Compare March 23, 2018 10:14

martinpitt approved these changes Mar 23, 2018

View reviewed changes

martinpitt merged commit 94cc54e into cockpit-project:master Mar 24, 2018

martinpitt pushed a commit that referenced this pull request Mar 24, 2018

machines: Detect if libvirt is running

0e78787

- Show spinner before the status is known - Show slate with action if libvirt is down Closes #8493

garrett mentioned this pull request Jun 4, 2018

networking: Hide actions and show notification if NM is not running #9264

Merged

4 tasks

andreasn mentioned this pull request Nov 27, 2018

No good error state if the lorax-composer.service is not running osbuild/cockpit-composer#460

Closed

atiratree mentioned this pull request Oct 1, 2019

rhel-7-9, Centos-7: SELinux is preventing /usr/lib/systemd/systemd-machined from 'search' accesses cockpit-project/bots#61

Open

machines: detect if libvirt is running #8493

machines: detect if libvirt is running #8493

Conversation

atiratree commented Jan 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinpitt commented Jan 25, 2018

atiratree commented Jan 26, 2018

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

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

atiratree commented Jan 30, 2018

New version of design

Reasons

Design

garrett commented Jan 30, 2018

Warning: libvirt

Dismissable errors & notices

atiratree commented Jan 30, 2018

garrett commented Jan 30, 2018

mareklibra commented Jan 31, 2018

atiratree commented Jan 31, 2018

current state

libvirt status

error messages

garrett left a 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

atiratree commented Feb 6, 2018

atiratree commented Feb 6, 2018 • edited Loading

Errors: expected usage

atiratree commented Feb 6, 2018 • edited Loading

Errors: expected usage

1)

garrett commented Feb 7, 2018

atiratree commented Feb 7, 2018

mareklibra commented Feb 7, 2018 • edited Loading

garrett commented Feb 7, 2018

atiratree commented Feb 13, 2018

atiratree commented Feb 20, 2018

garrett commented Feb 20, 2018

atiratree commented Mar 15, 2018

atiratree commented Mar 20, 2018

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

atiratree commented Mar 20, 2018

mareklibra left a comment

Choose a reason for hiding this comment

martinpitt left a 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

atiratree commented Feb 6, 2018 •

edited

Loading

atiratree commented Feb 6, 2018 •

edited

Loading

mareklibra commented Feb 7, 2018 •

edited

Loading