Template deployment resource #33
Conversation
There was a problem hiding this comment.
hey @thetonymaster
Thanks for this PR - I've left some comments in-line - but if we can replace the ARM Template/Terraform Configurations being used in the tests which are currently being skipped, we should be able to enable these tests.
Thanks!
| name := id.Path["deployments"] | ||
| if name == "" { | ||
| name = id.Path["Deployments"] | ||
| } |
There was a problem hiding this comment.
this bug which exists as a remnant of the early-days of Azure should be fixed in Azure Stack - so I think we should be able to normalize this to the lower-case deployments?
| name := id.Path["deployments"] | ||
| if name == "" { | ||
| name = id.Path["Deployments"] | ||
| } |
There was a problem hiding this comment.
this bug which exists as a remnant of the early-days of Azure should be fixed in Azure Stack - so I think we should be able to normalize this to the lower-case deployments?
|
|
||
| func waitForTemplateDeploymentToBeDeleted(ctx context.Context, client resources.DeploymentsClient, resourceGroup, name string) error { | ||
| // we can't use the Waiter here since the API returns a 200 once it's deleted which is considered a polling status code.. | ||
| log.Printf("[DEBUG] Waiting for Template Deployment (%q in Resource Group %q) to be deleted", name, resourceGroup) |
There was a problem hiding this comment.
to confirm: is this still the case for Azure Stack?
There was a problem hiding this comment.
If I use the future returned by the SDK I get the error pollingTrackerBase#updateRawBody: failed to unmarshal response body, So my guess is this is still needed.
There was a problem hiding this comment.
that sounds like a separate bug in the SDK, which would be worth raising on the SDK repository IMO
There was a problem hiding this comment.
|
|
||
| _, err = deployClient.Delete(ctx, resourceGroup, name) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
can we wrap this error message? e.g.
return fmt.Errorf("Error deleting Template Deployment %q (Resource Group %q): %+v", name, resourceGroup, err)
|
|
||
| read, err := deployClient.Get(ctx, resourceGroup, name) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
can we wrap this error message? e.g.
return fmt.Errorf("Error retrieving Template Deployment %q (Resource Group %q): %+v", name, resourceGroup, err)
|
|
||
| ~> **Note on AzureStack Template Deployments:** Due to the way the underlying Azure API is designed, Terraform can only manage the deployment of the AzureStack Template - and not any resources which are created by it. | ||
| This means that when deleting the `azurestack_template_deployment` resource, Terraform will only remove the reference to the deployment, whilst leaving any resources created by that AzureStack Template Deployment. | ||
| One workaround for this is to use a unique Resource Group for each AzureStack Template Deployment, which means deleting the Resource Group would contain any resources created within it - however this isn't ideal. [More information](https://docs.microsoft.com/en-us/rest/api/resources/deployments#Deployments_Delete). |
There was a problem hiding this comment.
AFAIK this is still known as an ARM Template Deployment in Azure Stack?
| // these values *should* be 'true' and 'false' but, | ||
| // due to a bug in the way terraform represents bools at various times these are for now 0 and 1 | ||
| // see https://github.com/hashicorp/terraform/issues/13512#issuecomment-295389523 | ||
| // at a later date these may return the expected 'true' / 'false' and should be changed back |
There was a problem hiding this comment.
we can remove this comment as this behaviour is intentional
| // Storage account type is not supported | ||
| func TestAccAzureStackTemplateDeployment_withOutputs(t *testing.T) { | ||
|
|
||
| t.Skip() |
There was a problem hiding this comment.
rather than skip this test we can update it to use a different ARM Template?
| // Provider doesn't support resource: azurestack_key_vault_secret | ||
| func TestAccAzureStackTemplateDeployment_withParamsBody(t *testing.T) { | ||
|
|
||
| t.Skip() |
There was a problem hiding this comment.
we can update this to send a different value via the params body - so we should be able to update this test
| // Storage Account type is not supported | ||
| func TestAccAzureStackTemplateDeployment_withParams(t *testing.T) { | ||
|
|
||
| t.Skip() |
There was a problem hiding this comment.
we should be able to update the ARM Template being used for this to use something else, such that this test can be valid?
- Change storage account types to supported types - Change the expected values on tests to be consisting with the new storage account types - Remove for now the unsupported resources and adapt the template to use supported resources
|
|
||
| # azurestack_template_deployment | ||
|
|
||
| Create a template deployment of resources |
|
tests pass: |
|
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 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
No description provided.