[azurerm_batch_pool] - support for custom images with the storage_image_reference property

[jcorioland]

This PR brings support to custom image for Azure Batch Pool.
It fixes #3352.

• Implementation in resource
• Implementation in data source
• Acceptance tests
• Documentation + Example

[jcorioland]

The custom image id needs to be set in the id property of the image reference in the API. I am wondering if it's better to do it this way (like it is currently):

storage_image_reference {
    id = "${data.azurerm_image.test.id}"
}

or add an extra property to the pool, which is not nested to the image reference (like proposed in the issue) and do the assignation in the code.

@tombuildsstuff @katbyte any thoughts?

[WodansSon]

@jcorioland thanks for this PR, this look really good just want a quick change to the docs if you don't mind. Thanks. :)

[WodansSon]

Shouldn't this code sample be moved to a new section under the Example Usage section? Say something like ## Example Usage with Custom Image?

[jcorioland]

Yes, totally agree.

[jcorioland]

@jeffreyCline thank you for the review! I've made some updates according to your comment.

[jcorioland]

Hey @jeffreyCline @tombuildsstuff @katbyte - any more change required on this one?

[katbyte]

Seeing test errors for this PR:

------- Stdout: -------
=== RUN   TestAccAzureRMBatchPool_customImage
=== PAUSE TestAccAzureRMBatchPool_customImage
=== CONT  TestAccAzureRMBatchPool_customImage
--- FAIL: TestAccAzureRMBatchPool_customImage (5.30s)
    testing.go:568: Step 0 error: errors during refresh:
        
        Error: Error: Image "ubuntu1604base-img" (Resource Group "batch-custom-img-rg") was not found
        
          on /opt/teamcity-agent/temp/buildTmp/tf-test282980552/main.tf line 7:
          (source code not available)
        
        
FAIL

[jcorioland]

@katbyte - the tests are now fixed :) the travis ci fails because of spell checking failing on hdinsight, but nothing that seems related to this PR.

[jcorioland]

Hi @katbyte - is it anything that you're waiting from me on this PR?

[katbyte]

Hi @jcorioland,

thanks for the updates, overall this looks good to me. I've left some comments inline mostly around with documentation and the examples. I am curious about changing ID to force new, it appears this property was ignored before?

[jcorioland]

@katbyte thank you for the review! I've fixed the issue in the documentation and also split the example into a basic and custom-img folder to make sure we cover both use cases separately. I also made sure that the custom image example is not dependant from an existing custom image but creates it itself.
All the changes you've requested should be covered now. LMK if there is anything else I need to improve.
Thanks!

[katbyte]

Thanks for the revisions @jcorioland! LGTM now 🙂

Running acc tests and will merge once they pass

[ghost]

This has been released in version 1.32.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 = "~> 1.32.0"
}
# ... other configuration ...

[ghost]

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!