-
Notifications
You must be signed in to change notification settings - Fork 75
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
charmcraft: perform LXD checks before use (CRAFT-420) #474
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,16 +27,10 @@ | |
|
||
from craft_providers import Executor, bases, lxd | ||
from craft_providers.actions import snap_installer | ||
from craft_providers.lxd import LXDInstallationError | ||
from craft_providers.lxd import installer as lxd_installer | ||
from craft_providers.lxd.remotes import configure_buildd_image_remote | ||
|
||
from charmcraft.cmdbase import CommandError | ||
from charmcraft.config import Base | ||
from charmcraft.env import ( | ||
get_managed_environment_log_path, | ||
get_managed_environment_project_path, | ||
) | ||
from charmcraft.env import get_managed_environment_log_path, get_managed_environment_project_path | ||
from charmcraft.utils import confirm_with_user, get_host_architecture | ||
|
||
logger = logging.getLogger(__name__) | ||
|
@@ -112,26 +106,29 @@ def ensure_provider_is_available() -> None: | |
|
||
:raises CommandError: if provider is not available. | ||
""" | ||
if is_provider_available(): | ||
return | ||
|
||
if confirm_with_user( | ||
"LXD is required, but not installed. Do you wish to install LXD " | ||
"and configure it with the defaults?", | ||
default=False, | ||
): | ||
try: | ||
lxd_installer.install() | ||
except LXDInstallationError as error: | ||
if not is_provider_available(): | ||
if confirm_with_user( | ||
"LXD is required, but not installed. Do you wish to install LXD " | ||
"and configure it with the defaults?", | ||
default=False, | ||
): | ||
try: | ||
lxd.install() | ||
except lxd.LXDInstallationError as error: | ||
raise CommandError( | ||
"Failed to install LXD. Please visit https://snapcraft.io/lxd for " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove Please, start with Visit -> https://discourse.ubuntu.com/t/tone-of-voice/18883 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, I questioned it when I wrote it way back when and never did reconsider. 🤔 I think I just feel compelled to be polite 🤣 will fix (but note that this was just an indent change, not new text). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done! |
||
"instructions on how to install the LXD snap for your distribution" | ||
) from error | ||
else: | ||
raise CommandError( | ||
"Failed to install LXD. Please visit https://snapcraft.io/lxd for " | ||
"LXD is required, but not installed. Please visit https://snapcraft.io/lxd for " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done! |
||
"instructions on how to install the LXD snap for your distribution" | ||
) from error | ||
else: | ||
raise CommandError( | ||
"LXD is required, but not installed. Please visit https://snapcraft.io/lxd for " | ||
"instructions on how to install the LXD snap for your distribution" | ||
) | ||
) | ||
|
||
try: | ||
lxd.ensure_lxd_is_ready() | ||
except lxd.LXDError as error: | ||
raise CommandError(str(error)) | ||
|
||
|
||
def is_base_providable(base: Base) -> Tuple[bool, Union[str, None]]: | ||
|
@@ -216,7 +213,7 @@ def is_provider_available() -> bool: | |
|
||
:returns: True if installed. | ||
""" | ||
return lxd_installer.is_installed() | ||
return lxd.is_installed() | ||
|
||
|
||
@contextlib.contextmanager | ||
|
@@ -250,7 +247,7 @@ def launched_environment( | |
) | ||
|
||
environment = get_command_environment() | ||
image_remote = configure_buildd_image_remote() | ||
image_remote = lxd.configure_buildd_image_remote() | ||
base_configuration = CharmcraftBuilddBaseConfiguration( | ||
alias=alias, environment=environment, hostname=instance_name | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not worth for this branch, but for the future: for readability in these cases please write the long message outside the
if
. Thanks!