Skip to content

CLI: Fix profile replacement for storage pools - #863

Merged
roosterfish merged 5 commits into
canonical:mainfrom
roosterfish:fix_profile_replacement
Jul 14, 2025
Merged

CLI: Fix profile replacement for storage pools#863
roosterfish merged 5 commits into
canonical:mainfrom
roosterfish:fix_profile_replacement

Conversation

@roosterfish

@roosterfish roosterfish commented Jul 10, 2025

Copy link
Copy Markdown
Contributor

Preceded by #864

The question to ask for profile device replacement was only applicable for networks because the storage section was missed when migrating the code into a helper in 29764bc.
This had the effect that only in case distributed network got configured the question got asked to replace the FAN with OVN network.
However when configuring distributed storage the profile's root device got replaced in any case without asking the user.

Another check is added in this PR to explicitly check the behavior which went unnoticed when refactoring the code.

@roosterfish
roosterfish force-pushed the fix_profile_replacement branch 4 times, most recently from 4e79516 to 23ae623 Compare July 10, 2025 15:33
@roosterfish
roosterfish force-pushed the fix_profile_replacement branch 6 times, most recently from bf9e60c to abad31f Compare July 14, 2025 10:41
…or profile replacement

Usually the askProfileUpdate helper should ask in case either the default profile's network or storage pool (root vol) is about to be changed.
However when moving the logic in the the askProfileUpdate helper, only the loop which was populating the profile's networks was moved
(See commit canonical@29764bc).

The storage pools where not moved which caused the helper to only ask for confirmation if the network got changed.
But the storage pool was overwritten in any case.
If a cluster was formed only using local storage and MicroCeph got added at a later point in time, the profile's storage pool
was overwritten to use the new remote storage pool without asking for confirmation.

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
…system_lxd

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
…alidator func

Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
Signed-off-by: Julian Pelizäus <julian.pelizaeus@canonical.com>
@roosterfish
roosterfish force-pushed the fix_profile_replacement branch from abad31f to 8b18da6 Compare July 14, 2025 11:08
@roosterfish
roosterfish marked this pull request as ready for review July 14, 2025 12:14
@roosterfish
roosterfish requested a review from markylaing July 14, 2025 12:14
Comment thread test/suites/basic.sh
services_validator
# The initial cluster got setup with only local storage.
# Due to REPLACE_PROFILE="no" the default profile's device doesn't get replaced.
services_validator local

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

local is a bash builtin, so it might be a good idea to wrap it in quotes to prevent syntax highlighting or confusing the reader :)

lxc profile device get default eth0 network | grep -q "default"
else
lxc profile device get default eth0 network | grep -q "lxdfan0"
lxc profile device get default eth0 network | grep -q "lxdfan0"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar to previous reviews, it would be good to try and update these so that we are testing for an exact match. Not urgent though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ack, will address it in the follow up PR mentioned on the other PR.

@markylaing markylaing left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just some test nits but I think these can all be lumped together in a later PR :)

@roosterfish
roosterfish merged commit a3dc008 into canonical:main Jul 14, 2025
31 checks passed
@roosterfish
roosterfish deleted the fix_profile_replacement branch July 14, 2025 12:51
roosterfish added a commit that referenced this pull request Jul 22, 2025
)

Preceded by:
* #864
* #863

Fixes #812.

The PR ensures that an existing (configured) MicroCeph cluster can be
reused by MicroCloud without the need of having to add additional disks
during the process.

In a scenario with multiple nodes running MicroCeph, a user might decide
to leverage the MicroCeph cluster which is already configured with OSDs
for MicroCloud. MicroCloud allows this re-use and asks during the
interactive questionnaire if the user wants to reuse this existing
cluster.
However until now MicroCloud required to add additional disks (OSDs) to
be able to include this existing cluster.
This limitation is now removed and the re-use of disks is made more
transparent to the user by printing the already configured disks if the
user decides to re-use the existing MicroCeph cluster.
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