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

azurerm_shared_image & azurerm_shared_image_version - support for specialized images by setting generalized to false #7277

Merged

Conversation

ArcturusZhang
Copy link
Contributor

This PR adds support for the specialized shared image, with the following changes:

  • azurerm_shared_image - add os_state attribute to indicate this image definition is generalized or specialized.
  • azurerm_shared_image_version - add os_disk_snapshot_id attribute to support create new image version using snapshot, which also enables you create new version for a specialized image.

@ArcturusZhang
Copy link
Contributor Author

Tests are passing:

=== RUN   TestAccAzureRMSharedImageVersion_basic
=== PAUSE TestAccAzureRMSharedImageVersion_basic
=== CONT  TestAccAzureRMSharedImageVersion_basic
--- PASS: TestAccAzureRMSharedImageVersion_basic (2421.21s)
=== RUN   TestAccAzureRMSharedImageVersion_storageAccountTypeLrs
=== PAUSE TestAccAzureRMSharedImageVersion_storageAccountTypeLrs
=== CONT  TestAccAzureRMSharedImageVersion_storageAccountTypeLrs
--- PASS: TestAccAzureRMSharedImageVersion_storageAccountTypeLrs (1294.24s)
=== RUN   TestAccAzureRMSharedImageVersion_storageAccountTypeZrs
=== PAUSE TestAccAzureRMSharedImageVersion_storageAccountTypeZrs
=== CONT  TestAccAzureRMSharedImageVersion_storageAccountTypeZrs
--- PASS: TestAccAzureRMSharedImageVersion_storageAccountTypeZrs (1212.54s)
=== RUN   TestAccAzureRMSharedImageVersion_specializedImageVersion
=== PAUSE TestAccAzureRMSharedImageVersion_specializedImageVersion
=== CONT  TestAccAzureRMSharedImageVersion_specializedImageVersion
--- PASS: TestAccAzureRMSharedImageVersion_specializedImageVersion (1087.86s)
=== RUN   TestAccAzureRMSharedImageVersion_requiresImport
=== PAUSE TestAccAzureRMSharedImageVersion_requiresImport
=== CONT  TestAccAzureRMSharedImageVersion_requiresImport
--- PASS: TestAccAzureRMSharedImageVersion_requiresImport (1509.59s)
PASS

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

@ArcturusZhang - i am getting test failures on TC:


TestAccAzureRMSharedImageVersion_requiresImport
TestAccDataSourceAzureRMSharedImageVersion_basic
TestAccDataSourceAzureRMSharedImageVersion_latest
TestAccDataSourceAzureRMSharedImageVersion_recent
TestAccDataSourceAzureRMSharedImageVersions_basic
TestAccDataSourceAzureRMSharedImageVersions_tagsFilter


@ArcturusZhang
Copy link
Contributor Author

Hi @katbyte I have synced with master... and now the tests seem passing
image
Interesting that first time one test fails, and second time I only run that failed test, and it passes.

@katbyte katbyte modified the milestones: v2.15.0, v2.16.0 Jun 16, 2020
@tombuildsstuff tombuildsstuff modified the milestones: v2.16.0, v2.17.0 Jun 25, 2020
@@ -74,6 +74,8 @@ The following arguments are supported:

* `release_note_uri` - (Optional) The URI containing the Release Notes associated with this Shared Image.

* `os_state` - (Optional) The state of Operating System present in this Shared Image. Possible values are `Generalized` and `Specialized`. Defaults to `Generalized`. Changing this forces a new resource to be created.
Copy link
Collaborator

Choose a reason for hiding this comment

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

After some internal discussion could we rename this to generalized = bool ? this is at heart a boolean true/false and os_state really doesn't convey what its doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea! I will take care of this.

@ArcturusZhang
Copy link
Contributor Author

Hi @katbyte I have resolved your comments, please have a look, thanks

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @ArcturusZhang, LGTM 👍

@katbyte katbyte changed the title Update resource and data source azurerm_shared_image, azurerm_shared_image_version - Add support for specialized shared image azurerm_shared_image & azurerm_shared_image_version - support for specialized images by setting generalized to false Jun 29, 2020
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Aside from 2 minor documentation changes which i will make now this LGTM 👍

website/docs/r/shared_image.html.markdown Outdated Show resolved Hide resolved
website/docs/d/shared_image.html.markdown Outdated Show resolved Hide resolved
@katbyte katbyte merged commit 0b71db8 into hashicorp:master Jun 29, 2020
@ArcturusZhang ArcturusZhang deleted the specialized-custom-image-shared-image branch June 29, 2020 23:31
katbyte added a commit that referenced this pull request Jun 30, 2020
@ghost
Copy link

ghost commented Jul 3, 2020

This has been released in version 2.17.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.17.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Jul 30, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@hashicorp hashicorp locked and limited conversation to collaborators Jul 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants