Skip to content

Implement incus image alias nested block #250

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 3 commits into
base: main
Choose a base branch
from

Conversation

SpiffyEight77
Copy link
Contributor

@SpiffyEight77 SpiffyEight77 commented May 16, 2025

Description

This pull request implements the incus_image_alias resource.

close #221

  • source code
  • document
  • test case

Example

Terraform Configuration

resource "incus_image" "img" {
  source_image = {
    remote = "images"
    name   = "alpine/edge"
  }

  alias {
    name        = "alpine"
    description = "Alpine Linux"
  }

  alias {
    name        = "alpine-edge"
    description = "Alpine Linux Edge"
  }
}

@SpiffyEight77 SpiffyEight77 force-pushed the feat/incus-image-alias branch 6 times, most recently from d8aaeb1 to 6015eeb Compare May 19, 2025 09:54
@SpiffyEight77 SpiffyEight77 marked this pull request as ready for review May 19, 2025 10:34
@SpiffyEight77 SpiffyEight77 marked this pull request as draft May 19, 2025 10:43
@SpiffyEight77 SpiffyEight77 force-pushed the feat/incus-image-alias branch from 6015eeb to 1811a53 Compare May 20, 2025 15:19
@stgraber
Copy link
Member

@SpiffyEight77 how's this one going?

@SpiffyEight77
Copy link
Contributor Author

Hi @stgraber !
I'll need two more days for testing. I modified the sync state logic for image resources.

@SpiffyEight77 SpiffyEight77 force-pushed the feat/incus-image-alias branch 2 times, most recently from 56c6a7f to c6aa483 Compare May 26, 2025 15:14
@SpiffyEight77 SpiffyEight77 changed the title Implement incus_image_alias resource Implement incus image alias nested block May 28, 2025
@SpiffyEight77 SpiffyEight77 force-pushed the feat/incus-image-alias branch 5 times, most recently from 96adaf6 to 1fba806 Compare May 30, 2025 15:54
@SpiffyEight77 SpiffyEight77 force-pushed the feat/incus-image-alias branch from 1fba806 to e8c0967 Compare May 30, 2025 16:26
@SpiffyEight77 SpiffyEight77 force-pushed the feat/incus-image-alias branch from e8c0967 to 9eb4e1a Compare May 30, 2025 16:35
@SpiffyEight77 SpiffyEight77 marked this pull request as ready for review May 30, 2025 16:44
@stgraber
Copy link
Member

@maveonair want to review this one?

Copy link
Member

@maveonair maveonair left a comment

Choose a reason for hiding this comment

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

@SpiffyEight77 very well done! Please have a look at my comments.

@@ -42,6 +42,7 @@ type ImageModel struct {
SourceImage types.Object `tfsdk:"source_image"`
SourceInstance types.Object `tfsdk:"source_instance"`
Aliases types.Set `tfsdk:"aliases"`
Copy link
Member

Choose a reason for hiding this comment

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

With the new syntax, we probably should introduce a breaking change and remove aliases as we now have the alias blocks. @stgraber what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Yep, agreed, we don't want the redundancy

@@ -583,6 +670,26 @@ func (r ImageResource) createImageFromSourceFile(ctx context.Context, resp *reso

imageAliases = append(imageAliases, ia)
}

for _, aliasModel := range aliasModels {
Copy link
Member

Choose a reason for hiding this comment

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

We have the same code block in both createImageFromSourceImage and createImageFromSourceInstance. Could you extract this into a function that can be used in all other places?

@SpiffyEight77
Copy link
Contributor Author

Thanks @maveonair and @stgraber for confirming! I'll refactor to remove the aliases later this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Implement an incus_image_alias resource
3 participants