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.

Sign up for GitHub

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

Jump to bottom

fix: check if lxd snap is installed #585

Merged
merged 12 commits into from
Jun 21, 2024
Merged

fix: check if lxd snap is installed #585

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ba258a3
fix: check if lxd snap is installed
cmatsuoka Jun 19, 2024
f489fed
chore: address linter warnings
cmatsuoka Jun 20, 2024
711e060
chore: address linter warnings
cmatsuoka Jun 20, 2024
b1e8962
tests: update lxd install detector unit test
cmatsuoka Jun 21, 2024
afc91ba
chore: address linter warnings
cmatsuoka Jun 21, 2024
9af3e97
chore: address linter warnings
cmatsuoka Jun 21, 2024
48031e0
chore: pin requests to a version that works for us
cmatsuoka Jun 21, 2024
dd67505
tests: cover executable in path and arch/nixos scenarios
cmatsuoka Jun 21, 2024
03ac670
chore: capitalize lxd name
cmatsuoka Jun 21, 2024
92f6358
chore: use british english
cmatsuoka Jun 21, 2024
059cbc5
chore: address linter warnings
cmatsuoka Jun 21, 2024
24585c3
chore: put back linter pragma that ruff told us to remove
cmatsuoka Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 39 additions & 3 deletions craft_providers/lxd/installer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@

import logging
import os
import shutil
import subprocess
import sys

import requests
import requests_unixsocket

from craft_providers.errors import details_from_called_process_error

from . import errors
Expand Down Expand Up @@ -52,6 +54,7 @@ def install(sudo: bool = True) -> str:

cmd += ["snap", "install", "lxd"]

logger.debug("installing lxd")
cmatsuoka marked this conversation as resolved.
Show resolved Hide resolved
try:
subprocess.run(cmd, check=True)
except subprocess.CalledProcessError as error:
Expand All @@ -62,6 +65,8 @@ def install(sudo: bool = True) -> str:

lxd = LXD()
lxd.wait_ready(sudo=sudo)

logger.debug("initializing lxd")
cmatsuoka marked this conversation as resolved.
Show resolved Hide resolved
lxd.init(auto=True, sudo=sudo)

if not is_user_permitted():
Expand Down Expand Up @@ -96,11 +101,42 @@ def is_initialized(*, remote: str, lxc: LXC) -> bool:


def is_installed() -> bool:
"""Check if LXD is installed (and found on PATH).
"""Check if LXD is installed.

:returns: True if lxd is installed.
"""
return shutil.which("lxd") is not None
logger.debug("Checking if LXD is installed.")

# query snapd API
url = "http+unix://%2Frun%2Fsnapd.socket/v2/snaps/lxd"
try:
snap_info = requests_unixsocket.get(
url=url, params={"select": "enabled"}
)
except requests.exceptions.ConnectionError as error:
raise errors.ProviderError(
brief="Unable to connect to snapd service."
) from error

try:
snap_info.raise_for_status()
except requests.exceptions.HTTPError as error:
logger.debug(f"Could not get snap info for LXD: {error}")
return False

# the LXD snap should be installed and active but check the status
# for completeness
try:
status = snap_info.json()["result"]["status"]
except KeyError:
raise errors.ProviderError(
brief="Unexpected response from snapd service."
)

logger.debug(f"LXD snap status: {status}")
# snap status can be "installed" or "active" - "installed" revisions
# are filtered from this API call with `select: enabled`
return bool(status == "active")
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think about retaining the old check as well? It's unlikely to fail but may still be useful:

return bool(status == "active) and shutil.which("lxd") is not None

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dariuszd21 also raised the issue about lxd on arch and nixos not being snap-packaged, we may need a dual check to cover these cases too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yikes, that's a good point.

Does that stubbed LXD exec file on Ubuntu come in as an apt package? Or is that just part of the base image?

I'm wondering if craft-providers could check if LXD is installed as a snap only on Ubuntu with is_ubuntu_like.

Making an API call to LXC may be the most robust check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Covered this for now by verifying the presence of a non-snap lxd socket in the standard location, but a more robust verification would be always welcome.



def is_user_permitted() -> bool:
Expand Down
Loading