Skip to content

Conversation

@simondeziel
Copy link
Member

No description provided.

```
+ sudo -E lxc storage create default zfs
Installing LXD snap, please be patient.
Error: Required tool "zpool" is missing
```

While `lvm2` is already part of the default package list on GHA runners,
`zfsutils-linux` is not.

Signed-off-by: Simon Deziel <[email protected]>
@simondeziel simondeziel requested a review from Copilot June 13, 2025 19:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds an optional flag to the install-lxd-runtimedeps composite action and enables it in the test workflow.

  • Introduce optional boolean input with default false in the action definition
  • Pass optional: true in the tests workflow to install extra packages
  • Conditionally install lvm2 and zfsutils-linux when optional is enabled

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.github/workflows/tests.yml Enable optional: true for the runtime‐deps action in the tests
.github/actions/install-lxd-runtimedeps/action.yml Define optional input and conditionally install extra packages
Comments suppressed due to low confidence (3)

.github/workflows/tests.yml:664

  • Consider adding a parallel workflow run or a unit test for optional: false to verify the default behavior doesn’t install the optional packages.
optional: true

.github/actions/install-lxd-runtimedeps/action.yml:21

  • It’s clearer to check for the positive case (if [ "${OPTIONAL}" = "true" ]) to avoid treating any non-false value as truthy.
if [ "${OPTIONAL}" != "false" ]; then

.github/actions/install-lxd-runtimedeps/action.yml:4

  • The new optional input should be documented in the action’s README or a top‐level comment so consumers know it’s available and what it does.
inputs:

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Ta

@simondeziel simondeziel marked this pull request as ready for review June 13, 2025 22:53
@tomponline tomponline merged commit 2e22c94 into canonical:main Jun 14, 2025
50 of 51 checks passed
@simondeziel simondeziel deleted the lxd-ui-lxd-runtime-deps branch June 14, 2025 12:13
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.

2 participants