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

provider/azurerm Fix multiple loadbalancer resource IDs #9401

Merged
merged 10 commits into from
Oct 18, 2016

Conversation

carinadigital
Copy link
Contributor

@carinadigital carinadigital commented Oct 17, 2016

Fixes GH-9311 and usage of ID property for azurerm loadbalancers including:

azurerm_lb_probe
azurerm_lb_backend_address_pool
azurerm_lb_rule
azurerm_lb_nat_rule
azurerm_lb_nat_pool

@carinadigital carinadigital force-pushed the GH-9311 branch 3 times, most recently from e686cb8 to ab95a32 Compare October 17, 2016 16:52
@@ -128,7 +128,18 @@ func resourceArmLoadBalancerProbeCreate(d *schema.ResourceData, meta interface{}
return fmt.Errorf("Cannot read LoadBalancer %s (resource group %s) ID", loadBalancerName, resGroup)
}

d.SetId(*read.ID)
var createdProbe_id string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could use retrieveLoadBalancerById() here again then findLoadBalancerProbeByName(), but it's another call to the API.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine to read the response here and avoid the additional round trip.

Andreas Kyrris added 2 commits October 18, 2016 09:26
The check for ARM_SUBSCRIPTION_ID breaks the PR testing. The PR
testing isn't trying to check acceptance tests anyway.
There will still be a correct failure for missing ARM_SUBSCRIPTION_ID
when running the acceptance test due to the later testAccPreCheck().
@carinadigital carinadigital changed the title [WIP] provider/azurerm Fix multiple loadbalancer resource IDs provider/azurerm Fix multiple loadbalancer resource IDs Oct 18, 2016
@carinadigital
Copy link
Contributor Author

This is ready for review. Passing tests listed below.

root@00a430836884:/go/src/github.com/hashicorp/terraform# make testacc TEST=./builtin/providers/azurerm TESTARGS='-run=TestAccAzureRMLoadBalancer'                                                                                                                                            
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/18 09:40:28 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMLoadBalancer -timeout 120m
=== RUN   TestAccAzureRMLoadBalancerBackEndAddressPool_basic
--- PASS: TestAccAzureRMLoadBalancerBackEndAddressPool_basic (152.14s)
=== RUN   TestAccAzureRMLoadBalancerBackEndAddressPool_removal
--- PASS: TestAccAzureRMLoadBalancerBackEndAddressPool_removal (146.24s)
=== RUN   TestAccAzureRMLoadBalancerNatPool_basic
--- PASS: TestAccAzureRMLoadBalancerNatPool_basic (147.21s)
=== RUN   TestAccAzureRMLoadBalancerNatPool_removal
--- PASS: TestAccAzureRMLoadBalancerNatPool_removal (168.26s)
=== RUN   TestAccAzureRMLoadBalancerNatRule_basic
--- PASS: TestAccAzureRMLoadBalancerNatRule_basic (152.48s)
=== RUN   TestAccAzureRMLoadBalancerNatRule_removal
--- PASS: TestAccAzureRMLoadBalancerNatRule_removal (163.14s)
=== RUN   TestAccAzureRMLoadBalancerProbe_basic
--- PASS: TestAccAzureRMLoadBalancerProbe_basic (148.39s)
=== RUN   TestAccAzureRMLoadBalancerProbe_removal
--- PASS: TestAccAzureRMLoadBalancerProbe_removal (165.07s)
=== RUN   TestAccAzureRMLoadBalancerRule_basic
--- PASS: TestAccAzureRMLoadBalancerRule_basic (147.74s)
=== RUN   TestAccAzureRMLoadBalancerRule_removal
--- PASS: TestAccAzureRMLoadBalancerRule_removal (166.23s)
=== RUN   TestAccAzureRMLoadBalancer_basic
--- PASS: TestAccAzureRMLoadBalancer_basic (101.79s)
=== RUN   TestAccAzureRMLoadBalancer_frontEndConfig
--- PASS: TestAccAzureRMLoadBalancer_frontEndConfig (182.38s)
=== RUN   TestAccAzureRMLoadBalancer_tags
--- PASS: TestAccAzureRMLoadBalancer_tags (116.12s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/azurerm        1957.206s

Copy link
Contributor

@jen20 jen20 left a comment

Choose a reason for hiding this comment

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

Hi @carinadigital! Thanks for submitting this pull request. I'm going to merge it for the imminent version of Terraform after making a few local tweaks to the error messages.

if pool_id != "" {
d.SetId(pool_id)
} else {
return fmt.Errorf("Error can not find created loadbalacner backend address pool id %s", pool_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

loadbalacner => LoadBalancer

@@ -15,6 +16,11 @@ func TestAccAzureRMLoadBalancerBackEndAddressPool_basic(t *testing.T) {
ri := acctest.RandInt()
addressPoolName := fmt.Sprintf("%d-address-pool", ri)

subscriptionID := os.Getenv("ARM_SUBSCRIPTION_ID")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we expose this via the provider?

@@ -128,7 +128,18 @@ func resourceArmLoadBalancerProbeCreate(d *schema.ResourceData, meta interface{}
return fmt.Errorf("Cannot read LoadBalancer %s (resource group %s) ID", loadBalancerName, resGroup)
}

d.SetId(*read.ID)
var createdProbe_id string
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine to read the response here and avoid the additional round trip.

@jen20 jen20 merged commit 182ebf3 into hashicorp:master Oct 18, 2016
@carinadigital carinadigital deleted the GH-9311 branch October 21, 2016 13:11
@ghost
Copy link

ghost commented Apr 21, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azurerm can't create LoadBalancer rule with probe
3 participants