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.

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

Allow instance rename #526

Merged
merged 2 commits into from
Sep 20, 2024
Merged

Conversation

MusicDin
Copy link
Member

Instead of replacing an instance, allow instance to be renamed. This is only possible if instance is either currently stopped or the running field is set to false.

@MusicDin MusicDin force-pushed the feat/instance-rename branch from 9beb9e2 to fd6cc54 Compare September 20, 2024 09:57
@MusicDin MusicDin marked this pull request as ready for review September 20, 2024 15:23
{
// Ensure instance rename fails when instance is running.
Config: testAccInstance_rename(instanceNameA, true),
ExpectError: regexp.MustCompile("Renaming of running instance not allowed"),
Copy link
Member

Choose a reason for hiding this comment

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

Should we also have the resource.TestCheckResourceAttr("lxd_instance.instance1", "name", instanceNameB), checked here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if state is testable when error is expected, but worth trying.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to be working. Thanks :)

},
{
// Stop and rename the instance.
Config: testAccInstance_rename(instanceNameB, false),
Copy link
Member

Choose a reason for hiding this comment

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

The provided name could be a bit more indicative of the intention. Say the instance started with A, then move to A2B -> B2C -> C2D -(fail)-> D2E.

Just an idea as I think what you have is fine, it just required a bit more mental tracking.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hehe, will fix :)

@MusicDin MusicDin force-pushed the feat/instance-rename branch from fd6cc54 to 4e5f92b Compare September 20, 2024 18:45
@MusicDin MusicDin merged commit eeff26a into terraform-lxd:main Sep 20, 2024
9 checks passed
@MusicDin MusicDin deleted the feat/instance-rename branch September 20, 2024 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants