From 1c981d6f30fe1fb742cecbcb7fb6b58e93bb1f8e Mon Sep 17 00:00:00 2001 From: Guillaume Giamarchi Date: Wed, 18 Feb 2015 00:12:04 +0100 Subject: [PATCH] Fix race conditions on firewall state transition --- .../resource_openstack_fw_firewall_v2.go | 38 +++++++++++-------- .../resource_openstack_fw_firewall_v2_test.go | 12 ------ 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/builtin/providers/openstack/resource_openstack_fw_firewall_v2.go b/builtin/providers/openstack/resource_openstack_fw_firewall_v2.go index cab83ce63..a216b506b 100644 --- a/builtin/providers/openstack/resource_openstack_fw_firewall_v2.go +++ b/builtin/providers/openstack/resource_openstack_fw_firewall_v2.go @@ -37,7 +37,6 @@ func resourceFWFirewallV2() *schema.Resource { "policy_id": &schema.Schema{ Type: schema.TypeString, Required: true, - ForceNew: true, }, "admin_state_up": &schema.Schema{ Type: schema.TypeBool, @@ -89,16 +88,10 @@ func resourceFirewallCreate(d *schema.ResourceData, meta interface{}) error { MinTimeout: 2 * time.Second, } - d.SetId(firewall.ID) - - d.Set("name", firewall.Name) - d.Set("description", firewall.Description) - d.Set("policy_id", firewall.PolicyID) - d.Set("admin_state_up", firewall.AdminStateUp) - d.Set("tenant_id", firewall.TenantID) - _, err = stateConf.WaitForState() + d.SetId(firewall.ID) + return nil } @@ -129,6 +122,7 @@ func resourceFirewallRead(d *schema.ResourceData, meta interface{}) error { d.Set("description", firewall.Description) d.Set("policy_id", firewall.PolicyID) d.Set("admin_state_up", firewall.AdminStateUp) + d.Set("tenant_id", firewall.TenantID) return nil } @@ -155,14 +149,15 @@ func resourceFirewallUpdate(d *schema.ResourceData, meta interface{}) error { opts.PolicyID = d.Get("policy_id").(string) } - log.Printf("[DEBUG] Updating firewall with id %s: %#v", d.Id(), opts) - - if err := firewalls.Update(networkingClient, d.Id(), opts).Err; err != nil { - return err + if d.HasChange("admin_state_up") { + adminStateUp := d.Get("admin_state_up").(bool) + opts.AdminStateUp = &adminStateUp } + log.Printf("[DEBUG] Updating firewall with id %s: %#v", d.Id(), opts) + stateConf := &resource.StateChangeConf{ - Pending: []string{"PENDING_CREATE"}, + Pending: []string{"PENDING_CREATE", "PENDING_UPDATE"}, Target: "ACTIVE", Refresh: WaitForFirewallActive(networkingClient, d.Id()), Timeout: 30 * time.Second, @@ -172,7 +167,7 @@ func resourceFirewallUpdate(d *schema.ResourceData, meta interface{}) error { _, err = stateConf.WaitForState() - return err + return firewalls.Update(networkingClient, d.Id(), opts).Err } func resourceFirewallDelete(d *schema.ResourceData, meta interface{}) error { @@ -184,13 +179,24 @@ func resourceFirewallDelete(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("Error creating OpenStack networking client: %s", err) } + stateConf := &resource.StateChangeConf{ + Pending: []string{"PENDING_CREATE", "PENDING_UPDATE"}, + Target: "ACTIVE", + Refresh: WaitForFirewallActive(networkingClient, d.Id()), + Timeout: 30 * time.Second, + Delay: 0, + MinTimeout: 2 * time.Second, + } + + _, err = stateConf.WaitForState() + err = firewalls.Delete(networkingClient, d.Id()).Err if err != nil { return err } - stateConf := &resource.StateChangeConf{ + stateConf = &resource.StateChangeConf{ Pending: []string{"DELETING"}, Target: "DELETED", Refresh: WaitForFirewallDeletion(networkingClient, d.Id()), diff --git a/builtin/providers/openstack/resource_openstack_fw_firewall_v2_test.go b/builtin/providers/openstack/resource_openstack_fw_firewall_v2_test.go index a0adecdc4..cc5343660 100644 --- a/builtin/providers/openstack/resource_openstack_fw_firewall_v2_test.go +++ b/builtin/providers/openstack/resource_openstack_fw_firewall_v2_test.go @@ -127,18 +127,6 @@ resource "openstack_fw_policy_v2" "accept_test_policy_1" { ` const testFirewallConfigUpdated = ` -resource "openstack_fw_firewall_v2" "accept_test" { - name = "accept_test" - description = "terraform acceptance test" - policy_id = "${openstack_fw_policy_v2.accept_test_policy_1.id}" -} - -resource "openstack_fw_policy_v2" "accept_test_policy_1" { - name = "policy-1" -} -` - -const testFirewallConfigForceNew = ` resource "openstack_fw_firewall_v2" "accept_test" { name = "accept_test" description = "terraform acceptance test"