Skip to content

Cluster: Add --status-only flag to lxc cluster restore command #15267

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

gabrielmougard
Copy link
Contributor

@gabrielmougard gabrielmougard commented Mar 27, 2025

JIRA ticket: https://warthogs.atlassian.net/browse/LXD-2602?atlOrigin=eyJpIjoiOGUwYzQ5ODJmZjNkNGEyMzk4MTlmN2ZhZWY1YWM5MzciLCJwIjoiaiJ9

This adds a --status-only flag to the lxc cluster restore command, which allows a cluster member to be restored to the online state without affecting any instances.

Problem

When a node is evacuated and then restored using lxc cluster restore:

  • Instances with local storage that were stopped during evacuation are automatically restarted.
  • Instances with remote storage that were migrated to other nodes are automatically migrated back.

We may want to restore just the node's status to online without affecting the current state and location of instances.

Solution

Added a new --status-only flag to the lxc cluster restore command that:

  • Does NOT restart any local instances that were stopped.
  • Does NOT migrate back any instances that were evacuated to other nodes.

API extension

Added clustering_restore_member_only API extension

Testing

  • Restoring a node with local and remote storage instances using the --status-only flag
  • Verifying that instances remain in their evacuated state/location after restore.
  • Confirming that a standard restore without the flag exhibits the original behavior.

@github-actions github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Mar 27, 2025
@gabrielmougard gabrielmougard force-pushed the fix/cluster-healing-member-only branch 3 times, most recently from 3200c79 to 00ceedc Compare March 27, 2025 15:21
@gabrielmougard gabrielmougard force-pushed the fix/cluster-healing-member-only branch from 00ceedc to e364448 Compare March 27, 2025 15:44
@gabrielmougard gabrielmougard marked this pull request as ready for review March 27, 2025 15:57
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.

Looks pretty good, there's a few code cleanups and optimisations we can do thought whilst we looking at this code.

@tomponline
Copy link
Member

also needs a rebase please

@tomponline tomponline changed the title lxd/cluster: Add --member-only flag to lxc cluster restore command Cluster: Add --member-only flag to lxc cluster restore command Mar 28, 2025
@gabrielmougard gabrielmougard force-pushed the fix/cluster-healing-member-only branch 2 times, most recently from 041a275 to a7098b3 Compare March 28, 2025 12:30
@gabrielmougard gabrielmougard force-pushed the fix/cluster-healing-member-only branch from a7098b3 to 21526ac Compare March 28, 2025 17:04
Copy link
Contributor

@minaelee minaelee left a comment

Choose a reason for hiding this comment

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

Copyedit suggestion + suggestion about flag naming.

@gabrielmougard gabrielmougard force-pushed the fix/cluster-healing-member-only branch 2 times, most recently from cf47c17 to f7fa9ef Compare March 31, 2025 12:41
@gabrielmougard gabrielmougard changed the title Cluster: Add --member-only flag to lxc cluster restore command Cluster: Add --status-only flag to lxc cluster restore command Mar 31, 2025
Copy link
Contributor

@minaelee minaelee left a comment

Choose a reason for hiding this comment

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

Thanks for changing member-only to status-only.
I am still seeing multiple places where member_only is used, seems like those should also be updated to status_only.

@gabrielmougard gabrielmougard force-pushed the fix/cluster-healing-member-only branch from f7fa9ef to 3603495 Compare April 7, 2025 12:36
@gabrielmougard
Copy link
Contributor Author

gabrielmougard commented Apr 7, 2025

Thanks for changing member-only to status-only.
I am still seeing multiple places where member_only is used, seems like those should also be updated to status_only.

should be fixed :)

Copy link
Contributor

@minaelee minaelee left a comment

Choose a reason for hiding this comment

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

I see a couple more instances of memberOnly

@gabrielmougard gabrielmougard force-pushed the fix/cluster-healing-member-only branch from 3603495 to 36253c9 Compare April 7, 2025 14:58
@tomponline
Copy link
Member

please rebase

Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
…nly if `--status-only` is false

Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
@gabrielmougard gabrielmougard force-pushed the fix/cluster-healing-member-only branch from 36253c9 to cfbb9e8 Compare April 7, 2025 15:25
tomponline
tomponline previously approved these changes Apr 7, 2025
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.

LGTM!

minaelee
minaelee previously approved these changes Apr 7, 2025
Copy link
Contributor

@minaelee minaelee left a comment

Choose a reason for hiding this comment

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

lgtm, ty (for docs/naming anyway - I see there are test failures)

@tomponline
Copy link
Member

tests look flaky

@gabrielmougard
Copy link
Contributor Author

hmm yes. I'm trying to understand why.

Signed-off-by: Gabriel Mougard <gabriel.mougard@canonical.com>
@gabrielmougard gabrielmougard dismissed stale reviews from minaelee and tomponline via 1857a58 April 9, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes to the REST API Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants