Fix terraform state in case of deployment error

[itavy]

This solves the problem when you are trying to deploy a template and encounter an error related to validation;
The state in terraform is persisted with new values provided and subsequent runs will not detect a change

Signed-off-by: Octavian Ionescu itavyg@gmail.com

[tombuildsstuff]

hi @itavy

Thanks for this PR.

Terraform internally uses the Resource ID to be able to determine when resources have changed/deleted - in this instance we use the Resource ID to manage the Deployment assuming a valid ARM Template, as such any errors should be bubbled up to the user.

If you're using Terraform to validate the ARM Templates (by deploying them) then you'll need to use Terraform's Taint functionality to force a recreation of this resource should the deployment fail. Whilst Terraform could automatically Taint the resource on error (which would Delete & then re-create it) - we intentionally don't do this within the Template Deployment resource since "Deleting" a Template Deployment doesn't do anything to the underlying resources.

Unfortunately the solution proposed in this PR (untracking the resource in the case of a deployment error) will break when #2807 comes into effect in the near-future; as redeploying the resource (as if it were new) will show an error telling users to import the current Deployment into Terraform, unless it's deleted prior to the redeployment.

Terraform currently makes the assumption that the ARM Template is valid at deployment time; assuming that users have run something like the Azure CLI's az deployment validate method for example:

$ az deployment validate --template-file local-file.json -l westeurope

We could instead change this to call the .Validate() API prior to deploying the ARM Template in the Create/Update method, which should mean a failed deployment is less likely - WDYT? However as this behaviour is intentional whilst I'd like to thank you for this contribution I'm going to close this PR for the moment.

Thanks!

[itavy]

@tombuildsstuff
i tried first to add a validate in createUpdate method but this method cannot be used atm (see azure sdk issue)

another approach will be to use partialUpdate in createUpdate method and update id inside that block, WDYT?

[tombuildsstuff]

@itavy taking a look into the SDK - it appears the Validate method returns this object:

// DeploymentValidateResult information from validate template deployment response.
type DeploymentValidateResult struct {
	autorest.Response `json:"-"`
	// Error - Validation error.
	Error *ManagementErrorWithDetails `json:"error,omitempty"`
	// Properties - The template deployment properties.
	Properties *DeploymentPropertiesExtended `json:"properties,omitempty"`
}

As such it should be possible to differentiate a Service Error (e.g. 500) and a Template Error (e.g. err being nil) like so:

validationResult, err := client.Validate(ctx, resourceGroup, name, deployment)
// e.g. 500 error
if err != nil {
	return fmt.Errorf("Error validating Template for Deployment: %+v", err)
}

// e.g. the template isn't valid
if validationResult.Error != nil && validationResult.Error.Message != nil {
	return fmt.Errorf("Error validating Template for Deployment: %+v", validationResult.Error.Message)
}

Which I think means that Azure/azure-sdk-for-go#6125 is working as intended / it should be possible to do that approach - WDYT?

Thanks!

[itavy]

@tombuildsstuff ty for clarification. i didn't understood very well how they intend to use the sdk.

i made another pull request taking in consideration your suggestions: #4715

[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!