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

Add support for LB adaptive_routing, location_strategy, random_steering, and zero_downtime_failover #1941

Conversation

tc80
Copy link
Contributor

@tc80 tc80 commented Sep 30, 2022

adaptive_routing and location_strategy are new features that were recently added to cloudflare-go: cloudflare/cloudflare-go#1091

The random_steering property was added in the past and I think we just forgot to add it to Terraform. Similarly, zero_downtime_failover was added to the session_affinity_attributes object in the past and Terraform was never updated.

All properties are documented here: https://api.cloudflare.com/#load-balancers-create-load-balancer

Also, I haven't run acceptance tests for this locally. I am new to Terraform — can you double-check to make sure I am handling random_steering properly? I am casting its pool_weights directly as map[string]float64:

random_steering = {
	pool_weights = {
		de90f38ced07c2e2f4df50b1f61d4194 = 0.3
		9290f38c5d07c2e2f4df57b1f61d4196 = 0.5
	}
	default_weight = 0.2
  }

@tc80 tc80 requested a review from jacobbednarz as a code owner September 30, 2022 01:58
@tc80 tc80 force-pushed the lb-add-adaptive-routing-and-location-strategy branch from 56f99a6 to 0dfdeec Compare September 30, 2022 01:59
@github-actions
Copy link
Contributor

changelog detected ✅

@jacobbednarz
Copy link
Member

to run the acceptance tests locally, you can run make testacc. it will prompt you for all the required environment variables needed for the test. check out https://github.com/cloudflare/terraform-provider-cloudflare/blob/master/contributing/development.md#quick-start for some other helpful tips on running it locally.

i'd highly advise running this in a dedicated account since it does create an destroy real resources and you wouldn't want it rogue in a production account. i can see a couple of issues with the tests but i'll let you get the test suite running to debug those before i do a review here 😄

@tc80 tc80 force-pushed the lb-add-adaptive-routing-and-location-strategy branch from 0dfdeec to 410acd9 Compare September 30, 2022 17:04
@tc80
Copy link
Contributor Author

tc80 commented Sep 30, 2022

Acceptance tests should pass now 👍

TESTARGS='-run "^TestAccCloudflareLoadBalancer_*" -count 1 -parallel 1' make testacc
TF_ACC=1 go test $(go list ./...) -v -run "^TestAccCloudflareLoadBalancer_*" -count 1 -parallel 1 -timeout 120m -parallel 1
?       github.com/cloudflare/terraform-provider-cloudflare     [no test files]
=== RUN   TestAccCloudflareLoadBalancerMonitor_Import
=== PAUSE TestAccCloudflareLoadBalancerMonitor_Import
=== RUN   TestAccCloudflareLoadBalancerPool_Import
=== PAUSE TestAccCloudflareLoadBalancerPool_Import
=== RUN   TestAccCloudflareLoadBalancer_Import
=== PAUSE TestAccCloudflareLoadBalancer_Import
=== RUN   TestAccCloudflareLoadBalancerMonitor_Basic
--- PASS: TestAccCloudflareLoadBalancerMonitor_Basic (4.13s)
=== RUN   TestAccCloudflareLoadBalancerMonitor_FullySpecified
--- PASS: TestAccCloudflareLoadBalancerMonitor_FullySpecified (5.02s)
=== RUN   TestAccCloudflareLoadBalancerMonitor_EmptyExpectedBody
--- PASS: TestAccCloudflareLoadBalancerMonitor_EmptyExpectedBody (4.19s)
=== RUN   TestAccCloudflareLoadBalancerMonitor_TcpFullySpecified
--- PASS: TestAccCloudflareLoadBalancerMonitor_TcpFullySpecified (4.40s)
=== RUN   TestAccCloudflareLoadBalancerMonitor_PremiumTypes
--- PASS: TestAccCloudflareLoadBalancerMonitor_PremiumTypes (8.06s)
=== RUN   TestAccCloudflareLoadBalancerMonitor_NoRequired
--- PASS: TestAccCloudflareLoadBalancerMonitor_NoRequired (2.41s)
=== RUN   TestAccCloudflareLoadBalancerMonitor_Update
--- PASS: TestAccCloudflareLoadBalancerMonitor_Update (6.04s)
=== RUN   TestAccCloudflareLoadBalancerMonitor_CreateAfterManualDestroy
--- PASS: TestAccCloudflareLoadBalancerMonitor_CreateAfterManualDestroy (6.94s)
=== RUN   TestAccCloudflareLoadBalancerMonitor_ChangingHeadersCauseReplacement
--- PASS: TestAccCloudflareLoadBalancerMonitor_ChangingHeadersCauseReplacement (5.82s)
=== RUN   TestAccCloudflareLoadBalancerPool_Basic
=== PAUSE TestAccCloudflareLoadBalancerPool_Basic
=== RUN   TestAccCloudflareLoadBalancerPool_FullySpecified
=== PAUSE TestAccCloudflareLoadBalancerPool_FullySpecified
=== RUN   TestAccCloudflareLoadBalancerPool_CreateAfterManualDestroy
=== PAUSE TestAccCloudflareLoadBalancerPool_CreateAfterManualDestroy
=== RUN   TestAccCloudflareLoadBalancer_Basic
=== PAUSE TestAccCloudflareLoadBalancer_Basic
=== RUN   TestAccCloudflareLoadBalancer_SessionAffinity
=== PAUSE TestAccCloudflareLoadBalancer_SessionAffinity
=== RUN   TestAccCloudflareLoadBalancer_AdaptiveRouting
=== PAUSE TestAccCloudflareLoadBalancer_AdaptiveRouting
=== RUN   TestAccCloudflareLoadBalancer_LocationStrategy
=== PAUSE TestAccCloudflareLoadBalancer_LocationStrategy
=== RUN   TestAccCloudflareLoadBalancer_RandomSteering
=== PAUSE TestAccCloudflareLoadBalancer_RandomSteering
=== RUN   TestAccCloudflareLoadBalancer_GeoBalanced
=== PAUSE TestAccCloudflareLoadBalancer_GeoBalanced
=== RUN   TestAccCloudflareLoadBalancer_ProximityBalanced
=== PAUSE TestAccCloudflareLoadBalancer_ProximityBalanced
=== RUN   TestAccCloudflareLoadBalancer_Rules
=== PAUSE TestAccCloudflareLoadBalancer_Rules
=== RUN   TestAccCloudflareLoadBalancer_DuplicatePool
=== PAUSE TestAccCloudflareLoadBalancer_DuplicatePool
=== RUN   TestAccCloudflareLoadBalancer_Update
=== PAUSE TestAccCloudflareLoadBalancer_Update
=== RUN   TestAccCloudflareLoadBalancer_CreateAfterManualDestroy
=== PAUSE TestAccCloudflareLoadBalancer_CreateAfterManualDestroy
=== CONT  TestAccCloudflareLoadBalancerMonitor_Import
--- PASS: TestAccCloudflareLoadBalancerMonitor_Import (5.41s)
=== CONT  TestAccCloudflareLoadBalancer_LocationStrategy
--- PASS: TestAccCloudflareLoadBalancer_LocationStrategy (5.04s)
=== CONT  TestAccCloudflareLoadBalancer_CreateAfterManualDestroy
--- PASS: TestAccCloudflareLoadBalancer_CreateAfterManualDestroy (8.06s)
=== CONT  TestAccCloudflareLoadBalancer_Update
--- PASS: TestAccCloudflareLoadBalancer_Update (7.54s)
=== CONT  TestAccCloudflareLoadBalancer_DuplicatePool
--- PASS: TestAccCloudflareLoadBalancer_DuplicatePool (3.31s)
=== CONT  TestAccCloudflareLoadBalancer_Rules
--- PASS: TestAccCloudflareLoadBalancer_Rules (6.82s)
=== CONT  TestAccCloudflareLoadBalancer_ProximityBalanced
--- PASS: TestAccCloudflareLoadBalancer_ProximityBalanced (5.11s)
=== CONT  TestAccCloudflareLoadBalancer_GeoBalanced
--- PASS: TestAccCloudflareLoadBalancer_GeoBalanced (5.34s)
=== CONT  TestAccCloudflareLoadBalancer_RandomSteering
--- PASS: TestAccCloudflareLoadBalancer_RandomSteering (5.27s)
=== CONT  TestAccCloudflareLoadBalancerPool_CreateAfterManualDestroy
--- PASS: TestAccCloudflareLoadBalancerPool_CreateAfterManualDestroy (6.55s)
=== CONT  TestAccCloudflareLoadBalancer_AdaptiveRouting
--- PASS: TestAccCloudflareLoadBalancer_AdaptiveRouting (5.50s)
=== CONT  TestAccCloudflareLoadBalancer_SessionAffinity
--- PASS: TestAccCloudflareLoadBalancer_SessionAffinity (10.62s)
=== CONT  TestAccCloudflareLoadBalancer_Basic
--- PASS: TestAccCloudflareLoadBalancer_Basic (4.97s)
=== CONT  TestAccCloudflareLoadBalancerPool_Basic
=== CONT  TestAccCloudflareLoadBalancerPool_FullySpecified
--- PASS: TestAccCloudflareLoadBalancerPool_Basic (4.46s)
--- PASS: TestAccCloudflareLoadBalancerPool_FullySpecified (4.30s)
=== CONT  TestAccCloudflareLoadBalancer_Import
--- PASS: TestAccCloudflareLoadBalancer_Import (5.81s)
=== CONT  TestAccCloudflareLoadBalancerPool_Import
--- PASS: TestAccCloudflareLoadBalancerPool_Import (4.42s)
PASS
ok      github.com/cloudflare/terraform-provider-cloudflare/internal/provider   145.761s

@tc80 tc80 force-pushed the lb-add-adaptive-routing-and-location-strategy branch from 410acd9 to b6295c5 Compare September 30, 2022 17:44
@tc80 tc80 force-pushed the lb-add-adaptive-routing-and-location-strategy branch from b6295c5 to faee9c7 Compare October 2, 2022 03:15
@tc80
Copy link
Contributor Author

tc80 commented Oct 2, 2022

(Just corrected something minor in the documentation, but still ready for review 👍 )

@jacobbednarz
Copy link
Member

nice work @tc80 👏 this is great! all the acceptance tests are passing on my end too, so let's get this merged!

the next step i'd recommend with LB (when you have time of course), is migrating the to auto generated docs. it will save yourself a whole heap of headaches in the future.

@jacobbednarz jacobbednarz merged commit 013d7dc into cloudflare:master Oct 3, 2022
@github-actions github-actions bot added this to the v3.25.0 milestone Oct 3, 2022
github-actions bot pushed a commit that referenced this pull request Oct 3, 2022
@tc80 tc80 deleted the lb-add-adaptive-routing-and-location-strategy branch October 4, 2022 01:00
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2022

This functionality has been released in v3.25.0 of the Terraform Cloudflare Provider.

Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

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.

2 participants