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

Use filtering to improve performance of agent pool data source #508

Merged
merged 2 commits into from Jul 15, 2022

Conversation

JarrettSpiker
Copy link
Contributor

@JarrettSpiker JarrettSpiker commented Jun 1, 2022

Description

Once agent pool filtering is enabled (currently off in production behind the remove-agent-pool-limit feature flag) we can filter agent pool list requests by agent pool name. This improves performance of the agent pool data source when the organization has >20 agent pools by reducing the number of API calls we need to make in order to loop through all of the pages of agent pools

Testing plan

  1. Set up a TFC organization with >20 agent pools, and a terraform config with a data source matching one of those agent pools
  2. Verify that with the remove-agent-pool-limit feature flag off, the data source resolves the agent pool
  3. Verify that with the remove-agent-pool-limit feature flag on, the data source resolves the agent pool even faster
    • The difference for me, against an org with ~1100 agent pools, is 6s w/ the feature flag vs. 1m12s w/o the feature flag

External links

  • API documentation: In progress...
  • go-tfe PR

Output from acceptance tests

 TESTARGS="-run  TestAccTFEAgentPoolDataSource_" make testacc
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run  TestAccTFEAgentPoolDataSource_ -timeout 15m
?   	github.com/hashicorp/terraform-provider-tfe	[no test files]
=== RUN   TestAccTFEAgentPoolDataSource_basic
--- PASS: TestAccTFEAgentPoolDataSource_basic (8.57s)
PASS
ok  	github.com/hashicorp/terraform-provider-tfe/tfe	8.798s
?   	github.com/hashicorp/terraform-provider-tfe/version	[no test files]

...

@JarrettSpiker JarrettSpiker requested a review from a team June 1, 2022 15:03
JarrettSpiker added a commit that referenced this pull request Jun 1, 2022
@sebasslash sebasslash added the feature-flag Features that sit behind a feature flag label Jun 1, 2022
Copy link
Contributor

@sebasslash sebasslash left a comment

Choose a reason for hiding this comment

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

Looks 🔥 (won't approve until GA though)! Do you think we can improve the import of this resource with this new functionality?

@JarrettSpiker
Copy link
Contributor Author

Looks 🔥 (won't approve until GA though)! Do you think we can improve the import of this resource with this new functionality?

Im not sure exactly what you mean? Is there inefficiencies in how resources are imported by default?

@sebasslash
Copy link
Contributor

Not an inefficiency per-se, but if we look at the current import function:

Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

It just supports importing by ID. We can potentially extend this to support importing using an org-name/agent-pool-name pair. This might be way out of the scope for this PR come to think of it. Just something to consider!

@JarrettSpiker JarrettSpiker removed the feature-flag Features that sit behind a feature flag label Jul 15, 2022
Copy link
Contributor

@sebasslash sebasslash left a comment

Choose a reason for hiding this comment

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

Approved 👍

@JarrettSpiker JarrettSpiker merged commit de887b4 into main Jul 15, 2022
@JarrettSpiker JarrettSpiker deleted the jspiker/agent-pool-search branch July 15, 2022 19:23
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.

None yet

2 participants