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

BREAKING CHANGE: labels/groups attachment to tests/agents updates #125

Merged
merged 2 commits into from
Feb 27, 2023

Conversation

pedro-te
Copy link
Collaborator

@pedro-te pedro-te commented Jan 17, 2023

Going forward it will no longer be possible to attach a label/group to a test in any test resource. Test resources obtain the labels linked to them via the API automatically (computed fields). Labels are now linked to tests only in the label resource. This diverges from what the ThousandEyes API allows, but is necessary considering how terraform works, as it was previously possible to have a test resource cancel out a label that is being added to it in a label resource, because the test resource wasn't setting the label.

Example:

resource "thousandeyes_agent_to_server" "thousandeyes_agent_to_server_icmp_test" {
  test_name = "Pedro-test: Example Agent to Server ICMP test from Terraform provider"
  interval = 120
  alerts_enabled = false
  server = "www.thousandeyes.com"
  protocol = "ICMP"
  enabled = false

  agents {
    agent_id = X
  }
}

resource "thousandeyes_agent_to_server" "thousandeyes_agent_to_server_tcp_no_port" {
  test_name      = "Pedro-test: agent to server TCP no port from terraform provider"
  interval       = 120
  alerts_enabled = false
  use_public_bgp = false
  enabled        = false

  server = "www.thousandeyes.com"

  agents {
    agent_id = X
  }

  # BREAKING CHANGE HERE
  //groups {
  //  group_id = thousandeyes_label.pedro-label.group_id <--- # THIS IS NO LONGER POSSIBLE, THIS IS NOW A COMPUTED FIELD
  //}
}

resource "thousandeyes_label" "pedro-label" {
  name     = "Pedro - Test Label"
  type     = "tests"

  tests {
    test_id = thousandeyes_agent_to_server.thousandeyes_agent_to_server_tcp_no_port.test_id 
  }

  tests {
    test_id = thousandeyes_agent_to_server.thousandeyes_agent_to_server_icmp_test.test_id 
  }
}

Furthermore, we should merge this first: thousandeyes/thousandeyes-sdk-go#113
And then update this PR with the new version, as this will be needed in order to support removing all agents/tests from a label. Otherwise that scenario is unsupported by our provider.

@@ -1136,6 +1134,20 @@ var schemas = map[string]*schema.Schema{
},
},
},
"tests-label": {
Copy link
Collaborator Author

@pedro-te pedro-te Jan 17, 2023

Choose a reason for hiding this comment

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

There's a schema for tests right above this one, but I'm not sure what it is being used for, as I don't think it was being used for Labels. Perhaps it can be deleted. I decided to add a new one for the Labels use case, similar to the agents-label.

At some point we might want to reconsider this "shared" schemas file as we're starting to create many exceptions.

raul-te
raul-te previously approved these changes Jan 17, 2023
sfreitas-te
sfreitas-te previously approved these changes Jan 20, 2023
@sfreitas-te
Copy link
Contributor

Since this is a breaking change let's first add a mention about the breaking change on the docs and afterwards remove the feature

Going forward it will no longer be possible to attach a label/group
to a test in any test resource. Test resources obtain the labels
linked to them via the API automatically (computed fields). Labels
are now linked to tests only in the label resource. This diverges
from what the ThousandEyes API allows, but is necessary considering
how terraform works, as it was previously possible to have a test
resource cancel out a label that is being added to it in a label
resource, because the test resource wasn't setting it.
joaomper-TE
joaomper-TE previously approved these changes Feb 23, 2023
@pedro-te pedro-te merged commit 7a3db27 into main Feb 27, 2023
@pedro-te pedro-te deleted the labels_fix branch February 27, 2023 11:48
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.

6 participants