terraform-provider-azurerm icon indicating copy to clipboard operation
terraform-provider-azurerm copied to clipboard

Test Fix PR [WIP] New Resource: `azurerm_custom_ip_prefix`

Open sinbai opened this issue 3 years ago • 1 comments

Support for new resource azurerm_custom_ip_prefix. A few things need to be clarified.

  1. Due to an IPv6 issue in the API, currently only IPv4 is supported and confirmed with the service team that it cannot be fixed before GA (2022/03/31). The next service API release will support IPv6.

  2. The test cases need to be run under the "eastus2euap" region, as the test IP range (provided by the service team for terraform testing) is now only available in that region.

  3. Test cases are executed sequentially because the test IP range can only create one resource at a time. The total time for the IPv4 test is approximately 7 hours.

Test results: PASS: TestAccCustomIpPrefix_withIpv4 (2604.35s) PASS: TestAccCustomIpPrefix_ipv4Update (8695.65s) PASS: TestAccCustomIpPrefix_ipv4Complete (2533.77s) PASS: TestAccCustomIpPrefix_ipv4RemoveCommissioned (6237.24s) PASS: TestAccCustomIpPrefix_requiresImport (2560.32s)

sinbai avatar Mar 23 '22 09:03 sinbai

Due to an IPv6 issue in the API, currently only IPv4 is supported and confirmed with the service team that it cannot be fixed before GA (2022/03/31). The next service API release will support IPv6.

this GA date4 has passed so presumably we can implement ipv6?

The test cases need to be run under the "eastus2euap" region, as the test IP range (provided by the service team for terraform testing) is now only available in that region.

Can we run the tests in a real region now?

Test cases are executed sequentially because the test IP range can only create one resource at a time. The total time for the IPv4 test is approximately 7 hours.

can we use different IP ranges to speed this up now?

katbyte avatar Jul 26 '22 15:07 katbyte

@katbyte answers inline.

Due to an IPv6 issue in the API, currently only IPv4 is supported and confirmed with the service team that it cannot be fixed before GA (2022/03/31). The next service API release will support IPv6.

this GA date4 has passed so presumably we can implement ipv6?

Per doc and reconfirmed by the service team, custom IP prefix does not currently support IPv6 for some reason.

The test cases need to be run under the "eastus2euap" region, as the test IP range (provided by the service team for terraform testing) is now only available in that region.

Can we run the tests in a real region now?

yes, the tests could now be run in a real region, the code has been updated to remove the eastus2euap region restriction.

Test cases are executed sequentially because the test IP range can only create one resource at a time. The total time for the IPv4 test is approximately 7 hours.

can we use different IP ranges to speed this up now?

After confirming with the service team, I am sorry to say it is not possible as they don't have more IP ranges could be used.

sinbai avatar Sep 28 '22 03:09 sinbai

@sinbai - 7 hours for all tests or each test?

and we have test failures:


------- Stdout: -------
=== RUN   TestAccCustomIpPrefix_ipv4Update_from_deprovisioned_todo_decommission
    testcase.go:117: Step 1/6 error: Error running apply: exit status 1
        
        Error: waiting for Custom Ip Prefix: (Custom Ip Prefixe Name "acctest-CustomIpPrefix-220928174322198272" / Resource Group "acctestRG-auto-220928174322198272") to become ready: unexpected state 'ValidationFailed', wanted target 'Provisioned'. last error: %!s(<nil>)
        
          with azurerm_custom_ip_prefix.test,
          on terraform_plugin_test.tf line 11, in resource "azurerm_custom_ip_prefix" "test":
          11: resource "azurerm_custom_ip_prefix" "test" {
        
    testing_new.go:84: Error running post-test destroy, there may be dangling resources: exit status 1
        
        Error: deleting Resource Group "acctestRG-auto-220928174322198272": the Resource Group still contains Resources.
        
        Terraform is configured to check for Resources within the Resource Group when deleting the Resource Group - and
        raise an error if nested Resources still exist to avoid unintentionally deleting these Resources.
        
        Terraform has detected that the following Resources still exist within the Resource Group:
        
        * `/subscriptions/*******/resourceGroups/acctestRG-auto-220928174322198272/providers/Microsoft.Network/customIpPrefixes/acctest-CustomIpPrefix-220928174322198272`
        
        This feature is intended to avoid the unintentional destruction of nested Resources provisioned through some
        other means (for example, an ARM Template Deployment) - as such you must either remove these Resources, or
        disable this behaviour using the feature flag `prevent_deletion_if_contains_resources` within the `features`
        block when configuring the Provider, for example:
        
        provider "azurerm" {
          features {
            resource_group {
              prevent_deletion_if_contains_resources = false
            }
          }
        }
        
        When that feature flag is set, Terraform will skip checking for any Resources within the Resource Group and
        delete this using the Azure API directly (which will clear up any nested resources).
        
        More information on the `features` block can be found in the documentation:
        https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs#features
        
        
--- FAIL: TestAccCustomIpPrefix_ipv4Update_from_deprovisioned_todo_decommission (780.79s)
FAIL

katbyte avatar Sep 29 '22 03:09 katbyte

@katbyte It takes about 18 hours for all the test cases to run through. In my tests, the time spent in each case is as follows.

image image image image

I updated the description of this PR as follows, the reason for failure of TestAccCustomIpPrefix_ipv4Update_from_deprovisioned_todo_decommission should be that Cidr is used by an existing custom ip prefix. This situation is what I mentioned in "If the test case fails in some reason..." below.

Support for new resource azurerm_custom_ip_prefix. A few things need to be clarified.

Test cases are executed sequentially because the test IP range can only create one resource at a time. The total time for all the IPv4 tests is approximately 18 hours.

If the test case fails in some reason (e.g. ResourceGroupBeingDeleted), and the commissionstate of custom ip prefix in not in Deprovisioned or ValidationFailed, the custom ip prefix could not be deleted. It should be updated to Deprovisioned(either in Azure portal by clicking deprovision or use powershell). Otherwise, the CIDR used in the custom ip prefix will not be able to be used to recreate custom ip prefix resource. The following is the relevant information about commissionstate confirmed with service team:

-  `Provisioning`:    can do nothing
-  `ValidatedFailed`: can retry provision(backend start from the beginning, do WAN validation again) or delete
-  `Provisioned`:     can do commission,or deprovision
-  `ProvisionFailed`: can retry provision(backend skip WAN validation)
-  `Commissioning`:  can do nothing
-  `Commissioned`:   can do decommission
-  `CommissionFailed`: can do commission
-  `Decommissioning`:  can donothing (back to provisioned)
-  `DecommissionFailed`:can do decommission
-  `Deprovisioning`:  can do nothing
-  `Deprovisioned`:  can do delete or provision
-  `DeprovisionFailed`: can do deprovisioning

sinbai avatar Sep 29 '22 03:09 sinbai

Hi team, how close are we to being complete with this?

brianlehr avatar Oct 31 '22 21:10 brianlehr

Pushing a rebase of this

tombuildsstuff avatar Dec 07 '22 17:12 tombuildsstuff

Pushing a rebase of this

@tombuildsstuff Are we almost complete with this? It has been open for 9 months at this point.

brianlehr avatar Dec 07 '22 18:12 brianlehr

hey @sinbai

Thanks for this PR - apologies for the delayed review of this one, I thought I'd left an update before the Christmas break.

As mentioned previously I've been working to try and review this PR, unfortunately I think there's an issue with the Service which I've been trying to diagnose where a Custom IP Prefix doesn't appear to be fully released until sometime after it's deleted.

Since the behaviour of the API has also changed considerably since this PR was first opened, supporting both specifying a CIDR block and (appearing to) provision one for you - I believe there maybe some other behavioural changes needed to this resource too, namely removing the action field in favour of taking a different approach. Whilst I've got local changes to do that - unfortunately I'm blocked on testing this at this point in time, since whilst we're running the tests for Custom IP Prefix, it appears that the CIDR block used by the Custom IP Prefix isn't fully reusable until sometime after the Custom IP Prefix is deleted.

Whilst I've been trying to diagnose that (by deleting any spurious Custom IP Prefixes, and waiting a couple of days [since an hour didn't make much difference] before re-running the test to give us enough time to be sure there's an issue or not) - unfortunately an automated process keeps kicking these tests off, which is causing the need to start over - as has happened again this morning.

To workaround this, whilst I've got some experimental changes to this resource locally (as outlined above) - I'm going to push a commit to this branch to force-fail the tests for the moment, which should stop these creating Custom IP Prefixes temporarily until we can diagnose this issue. I've also reached out to ask that the automated process is stopped, however pushing this failure mode should be sufficient to catch it even if it is.

I've also removed the remaining Custom IP Prefixes within our account, so I'm hopeful that later in the week we should be finally able to diagnose this issue and determine how to proceed here.

As such, whilst I'd like to apologies for the delay reviewing this PR (and that I've forgotten to post this update before the Christmas break) - I'm hopeful that we should be able to diagnose this issue later in the week and then determine what's needed to proceed here.

Thanks, Tom

tombuildsstuff avatar Jan 17 '23 13:01 tombuildsstuff

(Adding waiting-response to avoid the automated tooling too)

tombuildsstuff avatar Jan 17 '23 13:01 tombuildsstuff

hey @sinbai

Thanks for this PR - apologies for the delayed review of this one, I thought I'd left an update before the Christmas break.

As mentioned previously I've been working to try and review this PR, unfortunately I think there's an issue with the Service which I've been trying to diagnose where a Custom IP Prefix doesn't appear to be fully released until sometime after it's deleted.

Since the behaviour of the API has also changed considerably since this PR was first opened, supporting both specifying a CIDR block and (appearing to) provision one for you - I believe there maybe some other behavioural changes needed to this resource too, namely removing the action field in favour of taking a different approach. Whilst I've got local changes to do that - unfortunately I'm blocked on testing this at this point in time, since whilst we're running the tests for Custom IP Prefix, it appears that the CIDR block used by the Custom IP Prefix isn't fully reusable until sometime after the Custom IP Prefix is deleted.

Whilst I've been trying to diagnose that (by deleting any spurious Custom IP Prefixes, and waiting a couple of days [since an hour didn't make much difference] before re-running the test to give us enough time to be sure there's an issue or not) - unfortunately an automated process keeps kicking these tests off, which is causing the need to start over - as has happened again this morning.

To workaround this, whilst I've got some experimental changes to this resource locally (as outlined above) - I'm going to push a commit to this branch to force-fail the tests for the moment, which should stop these creating Custom IP Prefixes temporarily until we can diagnose this issue. I've also reached out to ask that the automated process is stopped, however pushing this failure mode should be sufficient to catch it even if it is.

I've also removed the remaining Custom IP Prefixes within our account, so I'm hopeful that later in the week we should be finally able to diagnose this issue and determine how to proceed here.

As such, whilst I'd like to apologies for the delay reviewing this PR (and that I've forgotten to post this update before the Christmas break) - I'm hopeful that we should be able to diagnose this issue later in the week and then determine what's needed to proceed here.

Thanks, Tom

FYI @gitlwh

@tombuildsstuff - Curious to know what Custom IP Prefixes you are testing? I ask as the PM for the product - we gave @sinbai access to a single working Custom IP Prefix but above it references multiple?

brianlehr avatar Jan 17 '23 17:01 brianlehr

@brianlehr

@tombuildsstuff - Curious to know what Custom IP Prefixes you are testing? I ask as the PM for the product - we gave @sinbai access to a single working Custom IP Prefix but above it references multiple?

We're using 194.41.20.0/24 (as per the acctests in this branch), with each test running sequentially - however as mentioned above we're unable to successfully provision this at the moment as outlined above (we get a Validation Failed: CIDR in use - with no Custom IP Prefixes present in our Subscription), which is why I'm wondering if there's something kicking around here?

tombuildsstuff avatar Jan 18 '23 17:01 tombuildsstuff

@brianlehr

@tombuildsstuff - Curious to know what Custom IP Prefixes you are testing? I ask as the PM for the product - we gave @sinbai access to a single working Custom IP Prefix but above it references multiple?

We're using 194.41.20.0/24 (as per the acctests in this branch), with each test running sequentially - however as mentioned above we're unable to successfully provision this at the moment as outlined above (we get a Validation Failed: CIDR in use - with no Custom IP Prefixes present in our Subscription), which is why I'm wondering if there's something kicking around here?

The CIDR was in use by another MSFT subscription. I've gone ahead and removed it now, so you should be able to onboard.

brianlehr avatar Jan 18 '23 17:01 brianlehr

@tombuildsstuff I have manually tested the IP range 194.41.20.0/24 and it could be successfully provisioned, deprovisioned and deleted completely. Could you take another look?

sinbai avatar Jan 19 '23 03:01 sinbai

Closing in favour of #21322

manicminer avatar Apr 06 '23 17:04 manicminer