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

Allow full deletion of pop_pools, country_pools, region_pools on load balancer updates #2673

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

thetwoj
Copy link
Contributor

@thetwoj thetwoj commented Aug 9, 2023

Resolves #2659, supersedes #2660

Currently pop_pools, country_pools, and region_pools are defined as Computed: true in their respective schemas. As a result, the d.GetOk() for each in resourceCloudflareLoadBalancerUpdate is returning the currently configured value when the corresponding definition is deleted from the Terraform config, effectively making it impossible to delete all entries from pop_pools, country_pools, or region_pools on a load balancer once they're initially defined. By removing Computed: true from the schemas full deletions in Terraform are respected, mirroring the behavior from calling the REST API directly with empty maps for any of the pool sets.

This MR also adds a test for this behavior - configuring a load balancer for geo steering via pop and country balancing before updating it to use region balancing instead. This test fails in the absence of the schema changes in this MR.

New test without schema changes:

$ TESTARGS='-run "^TestAccCloudflareLoadBalancer_GeoBalancedUpdate" -count 1 -parallel 1 -v' make testacc
...
=== RUN   TestAccCloudflareLoadBalancer_GeoBalancedUpdate
=== PAUSE TestAccCloudflareLoadBalancer_GeoBalancedUpdate
=== CONT  TestAccCloudflareLoadBalancer_GeoBalancedUpdate
    resource_cloudflare_load_balancer_test.go:333: Step 2/2 error: Check failed: Check 7/9 error: cloudflare_load_balancer.leessrdmop: Attribute 'pop_pools.#' expected "0", got "1"
--- FAIL: TestAccCloudflareLoadBalancer_GeoBalancedUpdate (6.49s)
...

New test with schema changes:

$ TESTARGS='-run "^TestAccCloudflareLoadBalancer_GeoBalancedUpdate" -count 1 -parallel 1 -v' make testacc
...
=== RUN   TestAccCloudflareLoadBalancer_GeoBalancedUpdate
=== PAUSE TestAccCloudflareLoadBalancer_GeoBalancedUpdate
=== CONT  TestAccCloudflareLoadBalancer_GeoBalancedUpdate
--- PASS: TestAccCloudflareLoadBalancer_GeoBalancedUpdate (7.02s)
...

When running all the load balancer tests with this MR's changes the only failure is on a pool count assertion because I'm running the tests on an account with existing infrastructure deployed in other zones

TESTARGS='-run "^TestAccCloudflareLoadBalancer.*" -count 1 -parallel 1 -v' make testacc
...
=== CONT  TestAccCloudflareLoadBalancerPools
    data_source_load_balancer_pools_test.go:18: Step 1/1 error: Check failed: Check 1/1 error: data.cloudflare_load_balancer_pools.qilephdkdz: Attribute 'pools.#' expected "2", got "304"
--- FAIL: TestAccCloudflareLoadBalancerPools (4.97s)
...

Example of referenced successful API call body to delete pop_pools from an existing load balancer:

PUT https://api.cloudflare.com/client/v4/zones/xxxx/load_balancers/xxxx
{
    "proxied": true,
    "enabled": true,
    "name": "xxxx",
    "steering_policy": "geo",
    "fallback_pool": "f72a8b39ffce25663b06137cc4a774d2",
    "default_pools": [
        "f72a8b39ffce25663b06137cc4a774d2"
    ],
    "pop_pools": {},
    "region_pools": {
        "WNAM": [
            "4cf1f50d9fbc79ce98046893971faeac"
        ]
    }
}

@thetwoj thetwoj requested a review from jacobbednarz as a code owner August 9, 2023 00:15
@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2023

changelog detected ✅

@jacobbednarz
Copy link
Member

thanks @thetwoj, this is great and thanks for including the test case.

acceptance tests all passing

TF_ACC=1 go test ./internal/sdkv2provider -v -run "^TestAccCloudflareLoadBalancer_" -count 1 -timeout 120m -parallel 1

=== RUN   TestAccCloudflareLoadBalancer_Import
=== PAUSE TestAccCloudflareLoadBalancer_Import
=== RUN   TestAccCloudflareLoadBalancer_Basic
=== PAUSE TestAccCloudflareLoadBalancer_Basic
=== RUN   TestAccCloudflareLoadBalancer_SessionAffinity
=== PAUSE TestAccCloudflareLoadBalancer_SessionAffinity
=== RUN   TestAccCloudflareLoadBalancer_SessionAffinityIPCookie
=== PAUSE TestAccCloudflareLoadBalancer_SessionAffinityIPCookie
=== RUN   TestAccCloudflareLoadBalancer_SessionAffinityHeader
=== PAUSE TestAccCloudflareLoadBalancer_SessionAffinityHeader
=== RUN   TestAccCloudflareLoadBalancer_AdaptiveRouting
=== PAUSE TestAccCloudflareLoadBalancer_AdaptiveRouting
=== RUN   TestAccCloudflareLoadBalancer_LocationStrategy
=== PAUSE TestAccCloudflareLoadBalancer_LocationStrategy
=== RUN   TestAccCloudflareLoadBalancer_RandomSteering
=== PAUSE TestAccCloudflareLoadBalancer_RandomSteering
=== RUN   TestAccCloudflareLoadBalancer_GeoBalancedUpdate
=== PAUSE TestAccCloudflareLoadBalancer_GeoBalancedUpdate
=== RUN   TestAccCloudflareLoadBalancer_GeoBalanced
=== PAUSE TestAccCloudflareLoadBalancer_GeoBalanced
=== RUN   TestAccCloudflareLoadBalancer_ProximityBalanced
=== PAUSE TestAccCloudflareLoadBalancer_ProximityBalanced
=== RUN   TestAccCloudflareLoadBalancer_LeastOutstandingRequestsBalanced
=== PAUSE TestAccCloudflareLoadBalancer_LeastOutstandingRequestsBalanced
=== 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  TestAccCloudflareLoadBalancer_Import
--- PASS: TestAccCloudflareLoadBalancer_Import (20.45s)
=== CONT  TestAccCloudflareLoadBalancer_GeoBalancedUpdate
--- PASS: TestAccCloudflareLoadBalancer_GeoBalancedUpdate (28.01s)
=== CONT  TestAccCloudflareLoadBalancer_CreateAfterManualDestroy
--- PASS: TestAccCloudflareLoadBalancer_CreateAfterManualDestroy (26.90s)
=== CONT  TestAccCloudflareLoadBalancer_Update
--- PASS: TestAccCloudflareLoadBalancer_Update (25.60s)
=== CONT  TestAccCloudflareLoadBalancer_DuplicatePool
--- PASS: TestAccCloudflareLoadBalancer_DuplicatePool (8.51s)
=== CONT  TestAccCloudflareLoadBalancer_Rules
--- PASS: TestAccCloudflareLoadBalancer_Rules (15.35s)
=== CONT  TestAccCloudflareLoadBalancer_LeastOutstandingRequestsBalanced
--- PASS: TestAccCloudflareLoadBalancer_LeastOutstandingRequestsBalanced (15.75s)
=== CONT  TestAccCloudflareLoadBalancer_ProximityBalanced
--- PASS: TestAccCloudflareLoadBalancer_ProximityBalanced (15.65s)
=== CONT  TestAccCloudflareLoadBalancer_GeoBalanced
--- PASS: TestAccCloudflareLoadBalancer_GeoBalanced (15.85s)
=== CONT  TestAccCloudflareLoadBalancer_SessionAffinityHeader
--- PASS: TestAccCloudflareLoadBalancer_SessionAffinityHeader (17.76s)
=== CONT  TestAccCloudflareLoadBalancer_RandomSteering
--- PASS: TestAccCloudflareLoadBalancer_RandomSteering (26.36s)
=== CONT  TestAccCloudflareLoadBalancer_LocationStrategy
--- PASS: TestAccCloudflareLoadBalancer_LocationStrategy (16.40s)
=== CONT  TestAccCloudflareLoadBalancer_AdaptiveRouting
--- PASS: TestAccCloudflareLoadBalancer_AdaptiveRouting (15.99s)
=== CONT  TestAccCloudflareLoadBalancer_SessionAffinity
--- PASS: TestAccCloudflareLoadBalancer_SessionAffinity (16.41s)
=== CONT  TestAccCloudflareLoadBalancer_SessionAffinityIPCookie
--- PASS: TestAccCloudflareLoadBalancer_SessionAffinityIPCookie (16.20s)
=== CONT  TestAccCloudflareLoadBalancer_Basic
--- PASS: TestAccCloudflareLoadBalancer_Basic (15.45s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/sdkv2provider	297.371s

@jacobbednarz jacobbednarz merged commit 1bd5bd8 into cloudflare:master Aug 9, 2023
@github-actions github-actions bot added this to the v4.12.0 milestone Aug 9, 2023
github-actions bot pushed a commit that referenced this pull request Aug 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 9, 2023

This functionality has been released in v4.12.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!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pop_pools, region_pools, and country_pools do not remove when last block is removed
2 participants