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

Computed value is not recognized as changing. Second plan/apply finds it. #15857

Closed
zambien opened this issue Aug 18, 2017 · 8 comments
Closed
Labels

Comments

@zambien
Copy link

zambien commented Aug 18, 2017

I'm writing a custom provider and am finding that when an output of one resource is marked as computed the dependent resource does not get marked to change. If I run an apply after the plan is applied it will find it. In other words I have to apply twice.

Terraform Version

All versions of 9 and 10.
Currently: terraform 0.10.2

Provider Code

https://github.com/zambien/terraform-provider-apigee

Terraform Configuration Files

variable "org" { default = "REDACTED" }
variable "env" { default = "test" }

provider "apigee" {
   org = "${var.org}"
   user = "REDACTED"
   password = "REDACTED"
}

resource "apigee_api_proxy" "helloworld_proxy" {
   name =  "helloworld-terraformed"
   bundle = "${data.archive_file.bundle.output_path}"
   bundle_sha = "${data.archive_file.bundle.output_sha}"
}

data "archive_file" "bundle" {
   type        = "zip"
   source_dir = "${path.module}/proxy_files"
   output_path = "${path.module}/proxy_files_bundle/apiproxy.zip"
}

resource "apigee_api_proxy_deployment" "helloworld_proxy_deployment" {
   proxy_name = "${apigee_api_proxy.helloworld_proxy.name}"
   org = "${var.org}"
   env = "${var.env}"
#********** This is where the change is not detected
   revision = "${apigee_api_proxy.helloworld_proxy.revision}" 
}

Debug Output

https://gist.github.com/zambien/9b0d147c93cd5d6cc29b28b0d9920a2a

Expected Behavior

The update to revision should be detected when the sha changes since the revision is a computed value.

Actual Behavior

Only the sha change is detected.

Steps to Reproduce

Please list the steps required to reproduce the issue, for example:

  1. terraform apply
  2. Update the sha on the folder by changing a file
  3. terraform plan # Notice that only the sha is detected
  4. terraform apply
  5. terraform plan #Now the revision change is recognized.

This could certainly be an issue with my provider/resource code. I've spent a lot of time debugging this and did could not see that was the case though. If I force the change by passing the sha into the deployment resource I see this:

Terraform Version: 0.10.2
Resource ID: apigee_api_proxy_deployment.helloworld_proxy_deployment
Mismatch reason: extra attributes: revision

with more error information. This lead me to believe this may in fact be a terraform bug.

@zambien
Copy link
Author

zambien commented Aug 18, 2017

Here is an updated log with the error I mentioned:

https://gist.github.com/zambien/b0ffa08e8a80589bc5287894d4155e64

Error applying plan:

1 error(s) occurred:

* apigee_api_proxy_deployment.helloworld_proxy_deployment: apigee_api_proxy_deployment.helloworld_proxy_deployment: diffs didn't match during apply. This is a bug with Terraform and should be reported as a GitHub Issue.

Please include the following information in your report:

    Terraform Version: 0.10.2
    Resource ID: apigee_api_proxy_deployment.helloworld_proxy_deployment
    Mismatch reason: extra attributes: revision
    Diff One (usually from plan): *terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{"sha":*terraform.ResourceAttrDiff{Old:"", New:"c9e97f4583ff804ce6a32a071fbb54a8430572cc", NewComputed:false, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, Meta:map[string]interface {}(nil)}
    Diff Two (usually from apply): *terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{"sha":*terraform.ResourceAttrDiff{Old:"", New:"c9e97f4583ff804ce6a32a071fbb54a8430572cc", NewComputed:false, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}, "revision":*terraform.ResourceAttrDiff{Old:"2", New:"3", NewComputed:false, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, Meta:map[string]interface {}(nil)}

Also include as much context as you can about your config, state, and the steps you performed to trigger this error.


Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.

@apparentlymart
Copy link
Contributor

Hi @zambien! Sorry this isn't working properly.

It's a known (and annoying!) issue that this happens. The technical reason why is that Terraform isn't able to predict during the plan phase that the update will, as a side-effect, change the revision here. (I assume revision is something that changes for each update, but its value isn't known until after the update is complete.)

The current plan to fix this is to implement something like what's in #8769, which would then allow a provider to tell Terraform programmatically when a change to a computed attribute is expected, like this:

# DRAFT API: not implemented yet, and may change before implementation
d.SetNewComputed("revision")

There is some work in this direction in #14887. Unfortunately we had to hold on that work while working on 0.10.0 but hopefully we can pick it up again before too long and get it working.

In the mean time, unfortunately there isn't a great workaround 😖. Some resources work around this by marking all of their attributes as ForceNew so that any edit implemented as a destroy and a create, but I expect this is undesirable here because I assume that this is a versioned API and so deleting this object would cause the history to be lost, or harder to access.

For a workaround specific to this situation, I wonder if for now the apigee_api_proxy_deployment resource could take as an additional argument the bundle_sha value from apigee_api_proxy; I understand that this isn't necessary for the underlying API, but including it would allow Terraform to see something change and trigger the diff you want here:

resource "apigee_api_proxy_deployment" "helloworld_proxy_deployment" {
   proxy_name = "${apigee_api_proxy.helloworld_proxy.name}"
   org = "${var.org}"
   env = "${var.env}"
   revision = "${apigee_api_proxy.helloworld_proxy.revision}"
   bundle_sha = "${apigee_api_proxy.helloworld_proxy.output_sha}"
}

This is similar to the workarounds used on resources such as aws_s3_bucket_object and aws_lambda_function to allow Terraform to detect when an external file has changed even though the file content isn't in the state. It does hurt usability a little, but in the long run when #8769 is closed this extra attribute could be phased out to reach the "ideal" user experience.

@zambien
Copy link
Author

zambien commented Aug 19, 2017

Thanks @apparentlymart so much for the detailed response. I did pass in the bundle_sha as an attempt at a workaround but it was from the archive_file, not from the api_proxy and that was causing the error in my second post in this thread. I made the change to use the bundle_sha from the api_proxy and saw the same error below. Is there a way for me to catch this somehow and continue? I'm new to writing plugins so I may be missing something obvious here.

Error applying plan:

1 error(s) occurred:

* apigee_api_proxy_deployment.helloworld_proxy_deployment: apigee_api_proxy_deployment.helloworld_proxy_deployment: diffs didn't match during apply. This is a bug with Terraform and should be reported as a GitHub Issue.

Please include the following information in your report:

    Terraform Version: 0.10.2
    Resource ID: apigee_api_proxy_deployment.helloworld_proxy_deployment
    Mismatch reason: extra attributes: revision
    Diff One (usually from plan): *terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{"bundle_sha":*terraform.ResourceAttrDiff{Old:"92502924b2a428d9b1f7223ddfe7bde96fd6a67e", New:"79160890b971a5dee24fc31d7489f1384a78745a", NewComputed:false, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, Meta:map[string]interface {}(nil)}
    Diff Two (usually from apply): *terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{"revision":*terraform.ResourceAttrDiff{Old:"4", New:"5", NewComputed:false, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}, "bundle_sha":*terraform.ResourceAttrDiff{Old:"92502924b2a428d9b1f7223ddfe7bde96fd6a67e", New:"79160890b971a5dee24fc31d7489f1384a78745a", NewComputed:false, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, Meta:map[string]interface {}(nil)}

Also include as much context as you can about your config, state, and the steps you performed to trigger this error.


Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure.

@apparentlymart
Copy link
Contributor

Hi @zambien,

Sorry I missed the connection between the error you reported and adding the bundle_sha attribute.

This error indicates that when Terraform tried to re-create the diff during the apply step it got a different result than it did during plan. This error actually makes sense since revision is updated during the apply of the apigee_api_proxy, but it's strange that we've not run into this before since we've definitely encountered your original bug before, and applied the solution I suggested.

I think the difference is that in this case we would normally set the ForceNew attribute on the bundle_sha and treat each new deployment as a "create". It looks like the apigee API makes a distinction between ReDeploy vs. Undeploy/Deploy, which is different than the scenarios we've encountered before.

I assume this difference exists because ReDeploy allows the change to be made without any downtime, whereas Undeploy would temporarily take it offline?

Unfortunately I'm not sure there's a good way to work around this here 😖 . It might be that this can't be properly implemented until #8769 is complete, at which point it can be solved "properly" by telling Terraform explicitly that revision will change on a apigee_api_proxy update.


As a side-note: I note that in your Update implementation you have a branch where you do a destroy/create when either the proxy_name or env are changed. In Terraform we'd normally express that by marking those attributes as ForceNew in the schema, which then causes Terraform to deal with this situation for you and also give the user explicit feedback in the diff that this will happen, marking it with the special -/+ "replace" indicator.

@zambien
Copy link
Author

zambien commented Aug 20, 2017

Hello @apparentlymart,

You are correct about the reason for the Deploy/ReDeploy/UnDeploy use cases. This is why a ForceNew won't work in this case. I will look at this a bit more to see if there is a good way to to implement a ForceNew. I think I may be able to code around it by doing some funny business in Delete and Create.

Thanks for the tip on the Update and ForceNew. I had meant to go back and fix that but forgot to do it.

BTW, once I get my tests written and code cleaned up, how would I go about applying to have my provider included in the terraform-providers repo?

Thanks for your help!

@stevehorsfield
Copy link
Contributor

Also looking forward to this capability being embedded in the AWS provider.

@bflad
Copy link
Contributor

bflad commented May 30, 2018

@apparentlymart was looking for a similar diffs didn't match issue and found this one which looks like it might be good to close since #14887 was merged 😄

https://godoc.org/github.com/hashicorp/terraform/helper/customdiff

@ghost
Copy link

ghost commented Sep 28, 2019

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Sep 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants