thousandeyes / terraform-provider-thousandeyes

ThousandEyes Terraform Provider
Apache License 2.0
21 stars 26 forks source link

[Bug]: default alert is set to null with each apply even after applying change #165

Closed Sraza11 closed 7 months ago

Sraza11 commented 7 months ago

What versions are you using?

2.0.8

What did you expect to happen?

once I apply a resource it should NOT show that an alert is getting set from a value to null

What actually happened?

once you apply the following code and your run terraform apply or plan you will see that alert is getting set from default to null.

terraform plan

thousandeyes_agent_to_server.TE_TEST will be updated in-place
  ~ resource "thousandeyes_agent_to_server" "TE_TEST" {
      ~ alerts_enabled         = false -> true
        id                     = "4634087"
        # (24 unchanged attributes hidden)

      - alert_rules {
          - rule_id = 448256 -> null
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

--- Actual resource

resource "thousandeyes_agent_to_server" "TE_TEST" {
  test_name              = "NO ACTION NEEDD TEST ONLY"
  interval               = 120
  alerts_enabled         = true
  description            = "test"
  server                 = "1.1.1.1"
  protocol               = "ICMP"
  enabled                = false
#  mtu_measurements       = false
 # bandwidth_measurements = false
  agents {
    agent_id = 3
  }
}

Terraform code to reproduce the bug

resource "thousandeyes_agent_to_server" "TE_TEST" {
  test_name              = "NO ACTION NEEDD TEST ONLY"
  interval               = 120
  alerts_enabled         = true
  description            = "test"
  server                 = "1.1.1.1"
  protocol               = "ICMP"
  enabled                = false
#  mtu_measurements       = false
 # bandwidth_measurements = false
  agents {
    agent_id = 3
  }
}

Any additional comments or code?

without fixing this issue I can not move from 2.0.5 to 2.0.8 version. and I need the functionality of 2.0.8 version

Steps to reproduce the bug

apply the resource and re run the plan you will see the bug

resource "thousandeyes_agent_to_server" "TE_TEST" {
  test_name              = "NO ACTION NEEDD TEST ONLY"
  interval               = 120
  alerts_enabled         = true
  description            = "test"
  server                 = "1.1.1.1"
  protocol               = "ICMP"
  enabled                = false
 mtu_measurements       = false
 bandwidth_measurements = false
  agents {
    agent_id = 3
  }
}
joaomper-TE commented 7 months ago

Hey @Sraza11, thanks for reporting. Can you confirm that this happens in 2.0.8 only but not in 2.0.5?

joaomper-TE commented 7 months ago

@Sraza11 after looking a bit more into this, it looks like this was introduced by the fix in https://github.com/thousandeyes/terraform-provider-thousandeyes/pull/151.

However, we can't do much about it at this point, since it's the way Terraform works as this is a resource created outside of Terraform - and in this case, since these are default Alert Rules we don't want to have them managed from Terraform.

Also, there's a super easy workaround for this, which is to simply add the default alert_rule id to the test:

 resource "thousandeyes_agent_to_server" "TE_TEST" {
  test_name              = "NO ACTION NEEDD TEST ONLY"
  interval               = 120
  alerts_enabled         = true
  description            = "test"
  server                 = "1.1.1.1"
  protocol               = "ICMP"
  enabled                = true

  alert_rules {
          rule_id = 921610 
        }

  agents {
    agent_id = 3
   }
 }

Is this an acceptable solution for you?

Sraza11 commented 7 months ago

Hey @Sraza11, thanks for reporting. Can you confirm that this happens in 2.0.8 only but not in 2.0.5?

yes, this only happens with version > 2.0.5

Sraza11 commented 7 months ago

@Sraza11 after looking a bit more into this, it looks like this was introduced by the fix in #151.

However, we can't do much about it at this point, since it's the way Terraform works as this is a resource created outside of Terraform - and in this case, since these are default Alert Rules which we don't want to have managed from Terraform.

Also, there's a super easy workaround for this, which is to simply add the default alert_rule id to the test:

 resource "thousandeyes_agent_to_server" "TE_TEST" {
  test_name              = "NO ACTION NEEDD TEST ONLY"
  interval               = 120
  alerts_enabled         = true
  description            = "test"
  server                 = "1.1.1.1"
  protocol               = "ICMP"
  enabled                = true

  alert_rules {
          rule_id = 921610 
        }

  agents {
    agent_id = 3
   }
 }

Is this an acceptable solution for you?

that is not going to work for me since I have many hundreds of tests that are defined without alert rule and default alert takes over. if there is no way around I suppose adding an alert rule to each test will be one way to fix this issue.

this was not happening until version 2.0.5

joaomper-TE commented 7 months ago

Yeah, like I said this happens because of https://github.com/thousandeyes/terraform-provider-thousandeyes/pull/151 which fixes the fact that default alert rules were not showing up on the ids. And after 2.0.5 the behavior was corrected.

Now, the fact that a default alert rule is assigned to a test that has none when defined, I guess it should be dealt in one of 3 ways:

  1. Assign the default alert rule to the tests when defining them (you have the id, so should be straightforward)
  2. Try to somehow import the state with the default alert rules (haven't tested this one myself) - see Import State section this link
  3. Create a new alert rule as you mentioned

Sorry if the response is not what you wanted, but in this case it does seem that we are battling a bit with the way Terraform works.

Sraza11 commented 7 months ago

Yeah, like I said this happens because of #151 which fixes the fact that default alert rules were not showing up on the ids. And after 2.0.5 the behavior was corrected.

Now, the fact that a default alert rule is assigned to a test that has none when defined, I guess it should be dealt in one of 3 ways:

1. Assign the default alert rule to the tests when defining them (you have the id, so should be straightforward)

2. Try to somehow import the state with the default alert rules (haven't tested this one myself) - see `Import State` section this [link](https://leonardo-loch.medium.com/using-resources-created-outside-of-the-current-terraform-state-a0cc68cd606b)

3. Create a new alert rule as you mentioned

Sorry if the response is not what you wanted, but in this case it does seem that we are battling a bit with the way Terraform works.

no worries, I can work on this as long as it is a feature and not a bug.