thousandeyes / terraform-provider-thousandeyes

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

fix: fix inconsistent plan when agent data resources run during apply… #141

Closed pedro-te closed 1 year ago

pedro-te commented 1 year ago

… always.

Addresses:

│ Error: Provider produced inconsistent final plan │ │ When expanding the plan for module.thousandeyes-tests.thousandeyes_http_server.Server-test-ire[0] to include new values learned so far during apply, provider "registry.terraform.io/thousandeyes/thousandeyes" produced an invalid new value for │ .agents: planned set element cty.ObjectVal(map[string]cty.Value{"agent_id":cty.UnknownVal(cty.Number), "agent_name":cty.StringVal(""), "agent_state":cty.StringVal(""), "agent_type":cty.UnknownVal(cty.String), │ "cluster_members":cty.ListValEmpty(cty.Object(map[string]cty.Type{"agent_state":cty.String, "ip_addresses":cty.List(cty.String), "last_seen":cty.String, "member_id":cty.Number, "name":cty.String, "network":cty.String, "prefix":cty.String, │ "public_ip_addresses":cty.List(cty.String), "target_for_tests":cty.String, "utilization":cty.Number})), "country_id":cty.StringVal(""), "created_date":cty.StringVal(""), "enabled":cty.NullVal(cty.Bool), │ "error_details":cty.ListValEmpty(cty.Object(map[string]cty.Type{"code":cty.String, "description":cty.String})), "groups":cty.SetValEmpty(cty.Object(map[string]cty.Type{"builtin":cty.Bool, "group_id":cty.Number, "name":cty.String, │ "type":cty.String})), "hostname":cty.StringVal(""), "ip_addresses":cty.UnknownVal(cty.List(cty.String)), "ipv6_policy":cty.StringVal(""), "keep_browser_cache":cty.NullVal(cty.Bool), "last_seen":cty.StringVal(""), "location":cty.StringVal(""), │ "network":cty.StringVal(""), "prefix":cty.StringVal(""), "target_for_tests":cty.StringVal(""), "utilization":cty.NullVal(cty.Number), "verify_ssl_certificate":cty.NullVal(cty.Bool)}) does not correlate with any element in actual. │ │ This is a bug in the provider, which should be reported in the provider's own issue tracker. ╵

I can provide an explanation on why this fixes the issue via slack if needed. But essentially, during the apply phase, terraform compares what it had in the plan with what was learned during apply, but it was unable to compare the agents as per the error message:

(...) does not correlate with any element in actual.

This is due to the fact that it's trying to match these two, I think: This is what is present in the test resource

              {
                "agent_id": 56299,
                "agent_name": "",
                "agent_state": "",
                "agent_type": "",
                "cluster_members": [],
                "country_id": "",
                "created_date": "",
                "enabled": false,
                "error_details": [],
                "groups": [],
                "hostname": "",
                "ip_addresses": [],
                "ipv6_policy": "",
                "keep_browser_cache": false,
                "last_seen": "",
                "location": "",
                "network": "",
                "prefix": "",
                "target_for_tests": "",
                "utilization": 0,
                "verify_ssl_certificate": false
              }

and this is what it learns during the apply for the data resource:

{
            "agent_id": 56299,
            "agent_name": "st-381-aws-thousandeyes-agent",
            "agent_state": null,
            "agent_type": null,
            "cluster_members": null,
            "country_id": null,
            "created_date": null,
            "enabled": null,
            "error_details": null,
            "groups": null,
            "hostname": null,
            "id": "0x1400071bf38",
            "ip_addresses": null,
            "ipv6_policy": null,
            "keep_browser_cache": null,
            "last_seen": null,
            "location": null,
            "network": null,
            "prefix": null,
            "target_for_tests": null,
            "utilization": null,
            "verify_ssl_certificate": null
          }

I think all those nulls being compared to the empty strings were causing the issue. Since only agent_id matters here, since everything else is removed here https://github.com/thousandeyes/terraform-provider-thousandeyes/blob/main/thousandeyes/util.go#L164 , I removed all other fields, so now only agent_id is compared, which is always set.

pedro-te commented 1 year ago

/review @sfreitas-te @brumarqu-te

brumarqu-te commented 1 year ago

@pedro-te i think ipAddress matters in agent tests (check agents.sourceIpAddress in the test metadata) but that's for a different PR i guess, since we were already stripping out all the fields and just using the agent_id.

pedro-te commented 1 year ago

@pedro-te i think ipAddress matters in agent tests (check agents.sourceIpAddress in the test metadata) but that's for a different PR i guess, since we were already stripping out all the fields and just using the agent_id.

Yeah that's the thing, it seems we never supported it. But we can add this functionality in a different PR for sure. 👍🏻