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

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

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

pedro-te
Copy link
Collaborator

@pedro-te pedro-te commented Sep 26, 2023

… 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.

Type: schema.TypeString,
Computed: true,
Description: "The type of agent.",
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -54,224 +54,6 @@ var schemas = map[string]*schema.Schema{
"agent_id": {
Type: schema.TypeInt,
Description: "The unique ID for the ThousandEyes agent.",
Optional: true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields are not needed, all of them, as they get removed here: https://github.com/thousandeyes/terraform-provider-thousandeyes/blob/main/thousandeyes/util.go#L164

@pedro-te
Copy link
Collaborator Author

/review @sfreitas-te @brumarqu-te

@brumarqu-te
Copy link
Contributor

@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
Copy link
Collaborator Author

pedro-te commented Sep 26, 2023

@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. 👍🏻

@pedro-te pedro-te merged commit 417b10f into thousandeyes:main Sep 27, 2023
@pedro-te pedro-te deleted the agents_inconsistent_plan branch September 27, 2023 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants