Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop trying to guess the name of the libvirt service/socket (fedora-35 necessary changes) #387

Merged
merged 5 commits into from Sep 23, 2021

Conversation

KKoukiou
Copy link
Collaborator

And leave the service activation to libvirt itself.

In all our supported distros the libvirt service is socket activated.
The socket is running by default, from the very first moment the user
installs the package. A stopped libvirt socket means that the user
manually changed the default state, and should be left alone to do so.

In addition to that, the libvirt daemon changed recently to a modular
achitecture, so instead of a single monolithic libvirtd daemon we now
get one for each driver (domain, storage, network, interfaces etc). Some distros already
opted in the new modular daemons, (see https://www.mail-archive.com/libvir-list@redhat.com/msg219931.html)
but many keep using the old monolithic libvirtd daemon.

As now we will have many sockets to check for their state, we decided to get
rid of any libvirt service/socket name detection logic from
cockpit-machines.
This logic was previously used to detect if the service was running
and if not we showed the user an empty state screen.
That also helped us to listen to socket/service changes and update
the UI accordingly.

This commit replaces this logic will a simple API call in the initial
render of this app which allows us to check if a connection type is
available; if that fails for any of the system/session connections, that means that the libvirt service is not available for the connection.

For the same reason of the name detection logic being problematic, let's
stop providing to the cockpit-machines users the ability to start/enable
the libvirt service/socket from the cockpit-machines UI directly.

@github-actions
Copy link
Contributor

Pixel test references have changed in 69df1bb. You can review them here.

@github-actions
Copy link
Contributor

Pixel test references have changed in d4f9985. You can review them here.

@KKoukiou KKoukiou force-pushed the no-detect-libvirt-socket branch 3 times, most recently from 518f650 to 7d88784 Compare September 21, 2021 13:34
@KKoukiou KKoukiou changed the title Stop trying to guess the name of the libvirt service/socket Stop trying to guess the name of the libvirt service/socket (fedora-35 necessary changes) Sep 21, 2021
@KKoukiou KKoukiou force-pushed the no-detect-libvirt-socket branch 3 times, most recently from adf52f4 to 7e4c94e Compare September 21, 2021 16:30
@KKoukiou KKoukiou marked this pull request as ready for review September 22, 2021 07:04
@KKoukiou
Copy link
Collaborator Author

/packit build

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! That's a nice cleanup as well. One question (doesn't affect the code). Can you please rebase this, to run this through packit? That's quite important in this case IMHO.

test/machineslib.py Outdated Show resolved Hide resolved
@martinpitt
Copy link
Member

Still lots of failures on fedora-35, capturing log URL. Looks like you need to start some libvirt-network service as well perhaps?

@KKoukiou KKoukiou force-pushed the no-detect-libvirt-socket branch 5 times, most recently from fc422e4 to a83dc18 Compare September 22, 2021 09:33
And leave the service activation to libvirt itself.

In all our supported distros the libvirt service is socket activated.
The socket is running by default, from the very first moment the user
installs the package. A stopped libvirt socket means that the user
manually changed the default state, and should be left alone to do so.

In addition to that, the libvirt daemon changed recently to a modular
achitecture, so instead of a single monolithic libvirtd daemon we now
get one for each driver (domain, storage, network, interfaces etc). Some distros already
opted in the new modular daemons, (see https://www.mail-archive.com/libvir-list@redhat.com/msg219931.html)
but many keep using the old monolithic libvirtd daemon.

As now we will have many sockets to check for their state, we decided to get
rid of any libvirt service/socket name detection logic from
cockpit-machines.
This logic was previously used to detect if the service was running
and if not we showed the user an empty state screen.
That also helped us to listen to socket/service changes and update
the UI accordingly.

This commit replaces this logic will a simple API call in the initial
render of this app which allows us to check if a connection type is
available; if that fails for any of the system/session connections, that means that the libvirt service is not available for the connection.

For the same reason of the name detection logic being problematic, let's
stop providing to the cockpit-machines users the ability to start/enable
the libvirt service/socket from the cockpit-machines UI directly.

This now allows us to make cockpit-machines work on fedora-35.
…ardown

Same as we do for libvirtd / virtqemud.

We do remove state in /var/run, without restarting the services libvirt
get's somehow confused.
Starting is done automatically as these are socket activated.
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Still rather ad-hoc, but we'll clean this up over time when more OSes require this. Great work!

test/machineslib.py Show resolved Hide resolved
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fails all over on the testing farm: 😢

error: Failed to connect socket to '/var/run/libvirt/virtnetworkd-sock': No such file or directory
error: failed to get network 'default'

As packit/f35 currently succeeds, this is a regression I'm afraid.

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks great, good to land on green. I also commented on the libvirt issue, thanks for filing!

test/check-machines-migrate Show resolved Hide resolved
test/browser/browser.sh Outdated Show resolved Hide resolved
@martinpitt
Copy link
Member

At least the failure looks different now..

error: Failed to define network from /etc/libvirt/qemu/networks/default.xml
error: operation failed: network 'default' already exists with uuid 5e0fbeda-3731-45be-8fd2-7a8098f92da6
error: Failed to start network default
error: Requested operation is not valid: network is already active

Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Niiice! (But no cookies for the Fedora package policy.. this will hit users in real-life as well..)

@KKoukiou KKoukiou merged commit e029fb5 into cockpit-project:main Sep 23, 2021
@KKoukiou KKoukiou deleted the no-detect-libvirt-socket branch September 23, 2021 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants