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

Diffs are not getting suppressed when lifecycle.ignore_changes is ignoring a change to a different field #18209

Closed
danawillow opened this issue Jun 7, 2018 · 7 comments

Comments

@danawillow
Copy link
Contributor

FYI before you start- I've been debugging this for a while and know why the problem is occurring, just not how to fix it without breaking other things :)
Happy to give debug logs if necessary, but I don't suspect there will be anything useful in them besides what I'm telling you here (given that I've spent all day staring at them myself).

Terraform Version

v0.11.7
Also repros with v0.11.8-dev (build from source)

Terraform Configuration Files

resource "google_container_cluster" "primary" {
  name = "is-1566"

  network    = "test-network"
  subnetwork = "test-subnet"

  node_pool = {
    name = "default-pool"
  }
  lifecycle = {
    ignore_changes = ["node_pool"]
  }
}

resource "google_container_node_pool" "np" {
  name = "second-pool"
  cluster = "${google_container_cluster.primary.name}"
  node_count = 1
}

Expected Behavior

No diff because there's a DiffSuppressFunc on network and subnetwork

Actual Behavior

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ google_container_cluster.primary
      network:    "projects/graphite-test-danahoffman-tf/global/networks/test-network" => "test-network"
      subnetwork: "projects/graphite-test-danahoffman-tf/regions/us-central1/subnetworks/test-subnet" => "test-subnet"

Steps to Reproduce

  1. terraform init
  2. terraform apply
  3. terraform plan

Additional Context

Here's what's happening:

Terraform enters into Eval, and calls diff on the provider:

// Diff!
diff, err := provider.Diff(n.Info, diffState, config)
if err != nil {
return nil, err
}

which then calls diff on each of the attributes:

for k, schema := range m {
err := m.diff(k, schema, result, d, false)
if err != nil {
return nil, err
}
}

In this particular example, the diffs for network and subnetwork are suppressed (as expected), and node_pool.# shows a diff because the cluster has two node pools but only one is in the config.

Because node_pool is a ForceNew field, it sets RequiresNew on the diff, and because it does that, we hit this block where it empties the state and computes a new diff:

if result.RequiresNew() {
// Create the new diff
result2 := new(terraform.InstanceDiff)
result2.Attributes = make(map[string]*terraform.ResourceAttrDiff)
// Preserve the DestroyTainted flag
result2.DestroyTainted = result.DestroyTainted
// Reset the data to not contain state. We have to call init()
// again in order to reset the FieldReaders.
d.state = nil
d.init()
// Perform the diff again
for k, schema := range m {
err := m.diff(k, schema, result2, d, false)
if err != nil {
return nil, err
}
}

Now, because the state is empty, our DiffSuppressFunc returns false for network and subnetwork because it's comparing our config against the empty string.

Then, we return from our initial Diff() call back into Eval(), and filter out ignored attributes:

// filter out ignored resources
if err := n.processIgnoreChanges(diff); err != nil {
return nil, err
}

This removes the diff on node_pool, but leaves in the diff for network and subnetwork, thus producing the plan output seen above.

References

hashicorp/terraform-provider-google#988
hashicorp/terraform-provider-google#1566 (where this repro comes from)
hashicorp/terraform-provider-google#1610

@jbardin
Copy link
Member

jbardin commented Jun 8, 2018

Hi @danawillow,

Thanks for the excellent detective work here!

We're going to be making some significant changes to the helper/schema package soon, which will provide us with a chance to better handle the issue. As shown here, because it's based on the interaction between core and the provider this may not be something that can be fixed for existing terraform releases, but should be something covered in 0.12.

@andronux
Copy link

Hi!

any updates on this issue?

Also, thanks for the investigation!

@fproulx-dfuse
Copy link

This is very very annoying. I hope this can be addressed soon.... sigh

@apparentlymart apparentlymart added config and removed core labels Nov 8, 2018
jbardin added a commit that referenced this issue Nov 16, 2018
Terraform used to provide empty diffs to the provider when calculating
`ignore_changes`, which would cause some DiffSuppressFunc to fail, as
can be seen in #18209.

Verify that this is no longer the case in 0.12
jbardin added a commit that referenced this issue Nov 16, 2018
Terraform used to provide empty diffs to the provider when calculating
`ignore_changes`, which would cause some DiffSuppressFunc to fail, as
can be seen in #18209.

Verify that this is no longer the case in 0.12
@jbardin
Copy link
Member

jbardin commented Dec 4, 2018

This is fixed in master, with a test to verify added in the commit linked above.

@red8888
Copy link

red8888 commented Feb 19, 2019

was this actually fixed? im still seeing the behavior in google_container_cluster in tf v0.11.10

I have to do this in order to prevent tf from seeing changes every time:

  network = "projects/${var.project}/global/networks/${var.network}"
  subnetwork = "projects/${var.project}/regions/${var.region}/subnetworks/${var.subnet}"

@ctavan
Copy link
Contributor

ctavan commented Feb 19, 2019

It should be part of upcoming terraform v0.12, it doesn’t seem to be in the v0.11 branch.

@ghost
Copy link

ghost commented Mar 29, 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 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 Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants