From 92b825fbd2aba620528ab94373b13d41e0e28ce8 Mon Sep 17 00:00:00 2001 From: Dom Del Nano Date: Sun, 20 Aug 2023 19:50:04 -0700 Subject: [PATCH] Change implementation approach to use a separate resource --- client/client.go | 1 + client/network.go | 87 ++++++-- xoa/provider.go | 13 +- xoa/resource_xenorchestra_bonded_network.go | 188 ++++++++++++++++ ...source_xenorchestra_bonded_network_test.go | 202 ++++++++++++++++++ xoa/resource_xenorchestra_network.go | 108 +++------- xoa/resource_xenorchestra_network_test.go | 57 ----- 7 files changed, 492 insertions(+), 164 deletions(-) create mode 100644 xoa/resource_xenorchestra_bonded_network.go create mode 100644 xoa/resource_xenorchestra_bonded_network_test.go diff --git a/client/client.go b/client/client.go index a24f6434..203b807c 100644 --- a/client/client.go +++ b/client/client.go @@ -73,6 +73,7 @@ type XOClient interface { CreateNetwork(netReq CreateNetworkRequest) (*Network, error) GetNetwork(netReq Network) (*Network, error) UpdateNetwork(netReq UpdateNetworkRequest) (*Network, error) + CreateBondedNetwork(netReq CreateBondedNetworkRequest) (*Network, error) GetNetworks() ([]Network, error) DeleteNetwork(id string) error diff --git a/client/network.go b/client/network.go index a3b8b323..214b72a0 100644 --- a/client/network.go +++ b/client/network.go @@ -46,18 +46,24 @@ func (net Network) Compare(obj interface{}) bool { return false } +// type sharedNetworkRequestParams struct { +// Pool string `mapstructure:"pool"` +// Name string `mapstructure:"name"` +// Description string `mapstructure:"description,omitempty"` +// Mtu int `mapstructure:"mtu,omitempty"` +// } + type CreateNetworkRequest struct { - BondMode string `mapstructure:"bondMode,omitempty"` - Pool string `mapstructure:"pool"` - Name string `mapstructure:"name"` - Nbd bool `mapstructure:"nbd,omitempty"` - Description string `mapstructure:"description,omitempty"` - Mtu int `mapstructure:"mtu,omitempty"` - PIF string `mapstructure:"pif,omitempty"` - PIFs []string `mapstructure:"pifs,omitempty"` - Vlan int `mapstructure:"vlan,omitempty"` - Automatic bool `mapstructure:"automatic"` - DefaultIsLocked bool `mapstructure:"defaultIsLocked"` + Pool string `mapstructure:"pool"` + Name string `mapstructure:"name"` + Description string `mapstructure:"description,omitempty"` + Mtu int `mapstructure:"mtu,omitempty"` + + Nbd bool `mapstructure:"nbd,omitempty"` + PIF string `mapstructure:"pif,omitempty"` + Vlan int `mapstructure:"vlan,omitempty"` + Automatic bool `mapstructure:"automatic"` + DefaultIsLocked bool `mapstructure:"defaultIsLocked"` } // Nbd and Automatic are eventually consistent. This ensures that waitForModifyNetwork will @@ -72,6 +78,25 @@ func (c CreateNetworkRequest) Propagated(obj interface{}) bool { return false } +// Does network.set apply for bonded networks? It uses the automatic and defaultIsLocked parameters. +// That API call also allows for nbd, which is not supported for bonded networks to my knowledge +type CreateBondedNetworkRequest struct { + Pool string `mapstructure:"pool"` + Name string `mapstructure:"name"` + Description string `mapstructure:"description,omitempty"` + Mtu int `mapstructure:"mtu,omitempty"` + + BondMode string `mapstructure:"bondMode,omitempty"` + PIFs []string `mapstructure:"pifs,omitempty"` + + Automatic bool `mapstructure:"automatic"` + DefaultIsLocked bool `mapstructure:"defaultIsLocked"` +} + +func (c CreateBondedNetworkRequest) Propagated(obj interface{}) bool { + return true +} + type UpdateNetworkRequest struct { Id string `mapstructure:"id"` Automatic bool `mapstructure:"automatic"` @@ -101,25 +126,42 @@ func (c *Client) CreateNetwork(netReq CreateNetworkRequest) (*Network, error) { delete(params, "automatic") delete(params, "defaultIsLocked") - var err error - if len(netReq.PIFs) > 0 { - log.Printf("[DEBUG] params for network.createBonded: %#v", params) + log.Printf("[DEBUG] params for network.create: %#v", params) + err := c.Call("network.create", params, &id) - var result map[string]interface{} - err = c.Call("network.createBonded", params, &result) - if err != nil { - return nil, err - } - return c.waitForModifyNetwork(result["uuid"].(string), netReq, 10*time.Second) + if err != nil { + return nil, err } - log.Printf("[DEBUG] params for network.create: %#v", params) - err = c.Call("network.create", params, &id) + // Neither automatic nor defaultIsLocked can be specified in the network.create RPC. + // Update them afterwards if the user requested it during creation. + if netReq.Automatic || netReq.DefaultIsLocked { + _, err = c.UpdateNetwork(UpdateNetworkRequest{ + Id: id, + Automatic: netReq.Automatic, + DefaultIsLocked: netReq.DefaultIsLocked, + }) + } + + return c.waitForModifyNetwork(id, netReq, 10*time.Second) +} +func (c *Client) CreateBondedNetwork(netReq CreateBondedNetworkRequest) (*Network, error) { + var params map[string]interface{} + mapstructure.Decode(netReq, ¶ms) + + delete(params, "automatic") + delete(params, "defaultIsLocked") + + log.Printf("[DEBUG] params for network.createBonded: %#v", params) + + var result map[string]interface{} + err := c.Call("network.createBonded", params, &result) if err != nil { return nil, err } + id := result["uuid"].(string) // Neither automatic nor defaultIsLocked can be specified in the network.create RPC. // Update them afterwards if the user requested it during creation. if netReq.Automatic || netReq.DefaultIsLocked { @@ -129,7 +171,6 @@ func (c *Client) CreateNetwork(netReq CreateNetworkRequest) (*Network, error) { DefaultIsLocked: netReq.DefaultIsLocked, }) } - return c.waitForModifyNetwork(id, netReq, 10*time.Second) } diff --git a/xoa/provider.go b/xoa/provider.go index a456f3be..393b9f4d 100644 --- a/xoa/provider.go +++ b/xoa/provider.go @@ -35,12 +35,13 @@ func Provider() *schema.Provider { }, }, ResourcesMap: map[string]*schema.Resource{ - "xenorchestra_acl": resourceAcl(), - "xenorchestra_cloud_config": resourceCloudConfigRecord(), - "xenorchestra_network": resourceXoaNetwork(), - "xenorchestra_vm": resourceRecord(), - "xenorchestra_resource_set": resourceResourceSet(), - "xenorchestra_vdi": resourceVDIRecord(), + "xenorchestra_acl": resourceAcl(), + "xenorchestra_bonded_network": resourceXoaBondedNetwork(), + "xenorchestra_cloud_config": resourceCloudConfigRecord(), + "xenorchestra_network": resourceXoaNetwork(), + "xenorchestra_vm": resourceRecord(), + "xenorchestra_resource_set": resourceResourceSet(), + "xenorchestra_vdi": resourceVDIRecord(), }, DataSourcesMap: map[string]*schema.Resource{ "xenorchestra_cloud_config": dataSourceXoaCloudConfig(), diff --git a/xoa/resource_xenorchestra_bonded_network.go b/xoa/resource_xenorchestra_bonded_network.go new file mode 100644 index 00000000..a22ebfcc --- /dev/null +++ b/xoa/resource_xenorchestra_bonded_network.go @@ -0,0 +1,188 @@ +package xoa + +import ( + "errors" + "fmt" + + "github.com/ddelnano/terraform-provider-xenorchestra/client" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" +) + +var validBondModes []string = []string{"balance-slb", "active-backup", "lacp"} + +func resourceXoaBondedNetwork() *schema.Resource { + return &schema.Resource{ + Create: resourceBondedNetworkCreate, + Delete: resourceBondedNetworkDelete, + Read: resourceBondedNetworkRead, + Update: resourceBondedNetworkUpdate, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, + Schema: map[string]*schema.Schema{ + // Verify if network.set applies for bonded networks unconditionally or if it + // only works with a subset of the parameters + + "automatic": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "default_is_locked": &schema.Schema{ + Type: schema.TypeBool, + Optional: true, + Default: false, + Description: "This argument controls whether the network should enforce VIF locking. This defaults to `false` which means that no filtering rules are applied.", + }, + "name_label": &schema.Schema{ + Type: schema.TypeString, + Required: true, + Description: "The name label of the network.", + }, + "name_description": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Default: netDefaultDesc, + }, + "pif_ids": &schema.Schema{ + Type: schema.TypeList, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + Optional: true, + ForceNew: true, + Description: "The pifs (uuid) that should be used for this network.", + }, + "bond_mode": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Description: "The bond mode that should be used for this network.", + ValidateFunc: validation.StringInSlice(validBondModes, false), + }, + "pool_id": &schema.Schema{ + Type: schema.TypeString, + Required: true, + ForceNew: true, + Description: "The pool id that this network should belong to.", + }, + "mtu": &schema.Schema{ + Type: schema.TypeInt, + Optional: true, + Default: 1500, + ForceNew: true, + Description: "The MTU of the network. Defaults to `1500` if unspecified.", + }, + }, + } +} + +func resourceBondedNetworkCreate(d *schema.ResourceData, m interface{}) error { + c := m.(client.XOClient) + + pifsReq := []string{} + for _, pif := range d.Get("pif_ids").([]interface{}) { + pifsReq = append(pifsReq, pif.(string)) + } + network, err := c.CreateBondedNetwork(client.CreateBondedNetworkRequest{ + Automatic: d.Get("automatic").(bool), + DefaultIsLocked: d.Get("default_is_locked").(bool), + BondMode: d.Get("bond_mode").(string), + Name: d.Get("name_label").(string), + Description: d.Get("name_description").(string), + Pool: d.Get("pool_id").(string), + Mtu: d.Get("mtu").(int), + PIFs: pifsReq, + }) + if err != nil { + return err + } + if len(network.PIFs) < 1 { + return errors.New("network should contain more than one PIF") + } + fmt.Printf("[WARNING] attempting to set pif_ids\n") + if err := d.Set("pif_ids", pifsReq); err != nil { + return errors.New("failed to set pif_ids attribute.") + } + return bondedNetworkToData(network, d) +} + +func resourceBondedNetworkRead(d *schema.ResourceData, m interface{}) error { + c := m.(client.XOClient) + network, err := c.GetNetwork( + client.Network{Id: d.Id()}) + + if _, ok := err.(client.NotFound); ok { + d.SetId("") + return nil + } + + if err != nil { + return err + } + + if len(network.PIFs) < 1 { + return errors.New("network should contain more than one PIF") + } + return bondedNetworkToData(network, d) +} + +func resourceBondedNetworkUpdate(d *schema.ResourceData, m interface{}) error { + c := m.(client.XOClient) + + netUpdateReq := client.UpdateNetworkRequest{ + Id: d.Id(), + Automatic: d.Get("automatic").(bool), + DefaultIsLocked: d.Get("default_is_locked").(bool), + } + if d.HasChange("name_label") { + nameLabel := d.Get("name_label").(string) + netUpdateReq.NameLabel = &nameLabel + } + if d.HasChange("name_description") { + nameDescription := d.Get("name_description").(string) + netUpdateReq.NameDescription = &nameDescription + } + _, err := c.UpdateNetwork(netUpdateReq) + if err != nil { + return err + } + return resourceBondedNetworkRead(d, m) +} + +func resourceBondedNetworkDelete(d *schema.ResourceData, m interface{}) error { + c := m.(client.XOClient) + + err := c.DeleteNetwork(d.Id()) + + if err != nil { + return err + } + d.SetId("") + return nil +} + +func bondedNetworkToData(network *client.Network, d *schema.ResourceData) error { + d.SetId(network.Id) + if err := d.Set("name_label", network.NameLabel); err != nil { + return err + } + if err := d.Set("name_description", network.NameDescription); err != nil { + return err + } + if err := d.Set("pool_id", network.PoolId); err != nil { + return err + } + if err := d.Set("mtu", network.MTU); err != nil { + return err + } + + if err := d.Set("automatic", network.Automatic); err != nil { + return err + } + if err := d.Set("default_is_locked", network.DefaultIsLocked); err != nil { + return err + } + return nil +} diff --git a/xoa/resource_xenorchestra_bonded_network_test.go b/xoa/resource_xenorchestra_bonded_network_test.go new file mode 100644 index 00000000..e347ea70 --- /dev/null +++ b/xoa/resource_xenorchestra_bonded_network_test.go @@ -0,0 +1,202 @@ +package xoa + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" +) + +func TestAccXOBondedNetwork_createAndPlanWithOmittedPIF(t *testing.T) { + if accTestPIF.Id == "" { + t.Skip() + } + resourceName := "xenorchestra_bonded_network.network" + nameLabel := fmt.Sprintf("%s - %s", accTestPrefix, t.Name()) + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccXenorchestraNetworkConfigBonded(nameLabel), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckXenorchestraNetwork(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "id"), + resource.TestCheckResourceAttrSet(resourceName, "name_label"), + resource.TestCheckResourceAttrSet(resourceName, "name_description"), + resource.TestCheckResourceAttrSet(resourceName, "pool_id"), + resource.TestCheckResourceAttrSet(resourceName, "mtu")), + }, + { + // Asserting that pif_ids was set in the previous set failed. This + // second step is to verify that even though it's not persisted in the + // state that it still correctly detects changes to the resource when + // the pif_ids argument is modified. + Config: testAccXenorchestraNetworkConfigBondedMissingPIF(nameLabel), + PlanOnly: true, + ExpectNonEmptyPlan: true, + }, + }, + }, + ) +} + +func TestAccXOBondedNetwork_updateInPlace(t *testing.T) { + if accTestPIF.Id == "" { + t.Skip() + } + resourceName := "xenorchestra_bonded_network.network" + nameLabel := fmt.Sprintf("%s - %s", accTestPrefix, t.Name()) + isLocked := "false" + automatic := "true" + desc := netDefaultDesc + + updatedNameLabel := nameLabel + " updated" + updatedDesc := "Non default description" + updatedAutomatic := "false" + updatedIsLocked := "true" + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccXenorchestraBondedNetworkConfigInPlaceUpdates(nameLabel, desc, automatic, isLocked), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckXenorchestraNetwork(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "id"), + resource.TestCheckResourceAttrSet(resourceName, "name_label"), + resource.TestCheckResourceAttr(resourceName, "name_description", desc), + resource.TestCheckResourceAttr(resourceName, "pool_id", accTestPIF.PoolId), + resource.TestCheckResourceAttr(resourceName, "automatic", automatic), + resource.TestCheckResourceAttr(resourceName, "default_is_locked", isLocked)), + }, + { + Config: testAccXenorchestraBondedNetworkConfigInPlaceUpdates(updatedNameLabel, updatedDesc, updatedAutomatic, updatedIsLocked), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckXenorchestraNetwork(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "id"), + resource.TestCheckResourceAttr(resourceName, "name_label", updatedNameLabel), + resource.TestCheckResourceAttr(resourceName, "name_description", updatedDesc), + resource.TestCheckResourceAttr(resourceName, "pool_id", accTestPIF.PoolId), + resource.TestCheckResourceAttr(resourceName, "automatic", updatedAutomatic), + resource.TestCheckResourceAttr(resourceName, "default_is_locked", updatedIsLocked)), + }, + }, + }, + ) +} + +func TestAccXOBondedNetwork_updateForceNew(t *testing.T) { + if accTestPIF.Id == "" { + t.Skip() + } + resourceName := "xenorchestra_bonded_network.network" + nameLabel := fmt.Sprintf("%s - %s", accTestPrefix, t.Name()) + desc := "Non default description" + origMtu := "950" + updatedMtu := "1000" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccXenorchestraBondedNetworkConfigNonDefaults(nameLabel, desc, origMtu), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckXenorchestraNetwork(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "id"), + resource.TestCheckResourceAttrSet(resourceName, "name_label"), + resource.TestCheckResourceAttr(resourceName, "name_description", desc), + resource.TestCheckResourceAttr(resourceName, "pool_id", accTestPIF.PoolId), + resource.TestCheckResourceAttr(resourceName, "mtu", origMtu)), + }, + { + Config: testAccXenorchestraBondedNetworkConfigNonDefaults(nameLabel, desc, updatedMtu), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckXenorchestraNetwork(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "id"), + resource.TestCheckResourceAttrSet(resourceName, "name_label"), + resource.TestCheckResourceAttr(resourceName, "name_description", desc), + resource.TestCheckResourceAttr(resourceName, "pool_id", accTestPIF.PoolId), + resource.TestCheckResourceAttr(resourceName, "mtu", updatedMtu)), + }, + }, + }, + ) +} + +var testAccXenorchestraBondedNetworkPIFs = func() string { + return fmt.Sprintf(` +data "xenorchestra_pif" "eth1" { + device = "eth1" + vlan = -1 + host_id = "%s" +} +data "xenorchestra_pif" "eth2" { + device = "eth2" + vlan = -1 + host_id = "%s" +} + +`, accTestPIF.Host, accTestPIF.Host) +} + +var testAccXenorchestraNetworkConfigBonded = func(name string) string { + return testAccXenorchestraBondedNetworkPIFs() + fmt.Sprintf(` + +resource "xenorchestra_bonded_network" "network" { + name_label = "%s" + pool_id = "%s" + pif_ids = [ + data.xenorchestra_pif.eth1.id, + data.xenorchestra_pif.eth2.id, + ] + bond_mode = "active-backup" +}`, name, accTestPIF.PoolId) +} + +var testAccXenorchestraNetworkConfigBondedMissingPIF = func(name string) string { + return testAccXenorchestraBondedNetworkPIFs() + fmt.Sprintf(` + +resource "xenorchestra_bonded_network" "network" { + name_label = "%s" + pool_id = "%s" + pif_ids = [ + data.xenorchestra_pif.eth1.id, + ] + bond_mode = "active-backup" +}`, name, accTestPIF.PoolId) +} + +var testAccXenorchestraBondedNetworkConfigInPlaceUpdates = func(name, desc, automatic, isLocked string) string { + return testAccXenorchestraBondedNetworkPIFs() + fmt.Sprintf(` + +resource "xenorchestra_bonded_network" "network" { + name_label = "%s" + name_description = "%s" + pool_id = "%s" + pif_ids = [ + data.xenorchestra_pif.eth1.id, + data.xenorchestra_pif.eth2.id, + ] + bond_mode = "active-backup" + automatic = %s + default_is_locked = %s +}`, name, desc, accTestPIF.PoolId, automatic, isLocked) +} + +var testAccXenorchestraBondedNetworkConfigNonDefaults = func(name, desc, mtu string) string { + return testAccXenorchestraBondedNetworkPIFs() + fmt.Sprintf(` +resource "xenorchestra_bonded_network" "network" { + bond_mode = "active-backup" + name_label = "%s" + name_description = "%s" + pool_id = "%s" + mtu = %s + pif_ids = [ + data.xenorchestra_pif.eth1.id, + data.xenorchestra_pif.eth2.id, + ] +} +`, name, desc, accTestPIF.PoolId, mtu) +} diff --git a/xoa/resource_xenorchestra_network.go b/xoa/resource_xenorchestra_network.go index 8abec618..645a5550 100644 --- a/xoa/resource_xenorchestra_network.go +++ b/xoa/resource_xenorchestra_network.go @@ -1,13 +1,13 @@ package xoa import ( + "errors" + "github.com/ddelnano/terraform-provider-xenorchestra/client" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) var netDefaultDesc string = "Created with Xen Orchestra" -var validBondModes []string = []string{"balance-slb", "active-backup", "lacp"} func resourceXoaNetwork() *schema.Resource { return &schema.Resource{ @@ -46,44 +46,18 @@ func resourceXoaNetwork() *schema.Resource { RequiredWith: []string{ "vlan", }, - ConflictsWith: []string{ - "pif_ids", - }, ForceNew: true, Description: "The pif (uuid) that should be used for this network.", }, - "pif_ids": &schema.Schema{ - Type: schema.TypeList, - Elem: &schema.Schema{ - Type: schema.TypeString, - }, - Optional: true, - ConflictsWith: []string{ - "pif_id", - "nbd", - }, - RequiredWith: []string{ - "bond_mode", - }, - ForceNew: true, - Description: "The pifs (uuid) that should be used for this network.", - }, - "bond_mode": &schema.Schema{ - Type: schema.TypeString, - Optional: true, - ForceNew: true, - Description: "The bond mode that should be used for this network.", - ValidateFunc: validation.StringInSlice(validBondModes, false), - }, "vlan": &schema.Schema{ Type: schema.TypeInt, Optional: true, - Default: -1, + Default: 0, RequiredWith: []string{ "pif_id", }, ForceNew: true, - Description: "The vlan to use for the network. Defaults to `-1` meaning no VLAN.", + Description: "The vlan to use for the network. Defaults to `0` meaning no VLAN.", }, "pool_id": &schema.Schema{ Type: schema.TypeString, @@ -111,18 +85,7 @@ func resourceXoaNetwork() *schema.Resource { func resourceNetworkCreate(d *schema.ResourceData, m interface{}) error { c := m.(client.XOClient) - pifsReq := []string{} - for _, pif := range d.Get("pif_ids").([]interface{}) { - pifsReq = append(pifsReq, pif.(string)) - } - // The Xen Orchestra API treats the VLAN default as 0, however, - // the xapi will return a -1 in those cases. Terraform must treat - vlan := 0 - if vlanParam, _ := d.Get("vlan").(int); vlanParam != -1 { - vlan = vlanParam - } network, err := c.CreateNetwork(client.CreateNetworkRequest{ - BondMode: d.Get("bond_mode").(string), Automatic: d.Get("automatic").(bool), DefaultIsLocked: d.Get("default_is_locked").(bool), Name: d.Get("name_label").(string), @@ -130,42 +93,37 @@ func resourceNetworkCreate(d *schema.ResourceData, m interface{}) error { Pool: d.Get("pool_id").(string), Mtu: d.Get("mtu").(int), Nbd: d.Get("nbd").(bool), - Vlan: vlan, + Vlan: d.Get("vlan").(int), PIF: d.Get("pif_id").(string), - PIFs: pifsReq, }) if err != nil { return err } - var pif *client.PIF - if len(network.PIFs) > 0 { - pifs, err := c.GetPIF(client.PIF{Id: network.PIFs[0]}) - if err != nil { - return err - } - pif = &pifs[0] + vlan, err := getVlanForNetwork(c, network) + if err != nil { + return err } - return networkToData(network, pif, d) + return networkToData(network, vlan, d) } -// func getVlanForNetwork(c client.XOClient, net *client.Network) (int, error) { -// if len(net.PIFs) > 0 { -// pifs, err := c.GetPIF(client.PIF{Id: net.PIFs[0]}) -// if err != nil { -// return -1, err -// } +func getVlanForNetwork(c client.XOClient, net *client.Network) (int, error) { + if len(net.PIFs) > 0 { + pifs, err := c.GetPIF(client.PIF{Id: net.PIFs[0]}) + if err != nil { + return -1, err + } -// if len(pifs) != 1 { -// return -1, errors.New("expected to find single PIF") -// } -// return pifs[0].Vlan, nil -// } -// return 0, nil -// } + if len(pifs) != 1 { + return -1, errors.New("expected to find single PIF") + } + return pifs[0].Vlan, nil + } + return 0, nil +} func resourceNetworkRead(d *schema.ResourceData, m interface{}) error { c := m.(client.XOClient) - network, err := c.GetNetwork( + net, err := c.GetNetwork( client.Network{Id: d.Id()}) if _, ok := err.(client.NotFound); ok { @@ -177,15 +135,11 @@ func resourceNetworkRead(d *schema.ResourceData, m interface{}) error { return err } - var pif *client.PIF - if len(network.PIFs) > 0 { - pifs, err := c.GetPIF(client.PIF{Id: network.PIFs[0]}) - if err != nil { - return err - } - pif = &pifs[0] + vlan, err := getVlanForNetwork(c, net) + if err != nil { + return err } - return networkToData(network, pif, d) + return networkToData(net, vlan, d) } func resourceNetworkUpdate(d *schema.ResourceData, m interface{}) error { @@ -224,7 +178,7 @@ func resourceNetworkDelete(d *schema.ResourceData, m interface{}) error { return nil } -func networkToData(network *client.Network, pif *client.PIF, d *schema.ResourceData) error { +func networkToData(network *client.Network, vlan int, d *schema.ResourceData) error { d.SetId(network.Id) if err := d.Set("name_label", network.NameLabel); err != nil { return err @@ -247,10 +201,8 @@ func networkToData(network *client.Network, pif *client.PIF, d *schema.ResourceD if err := d.Set("default_is_locked", network.DefaultIsLocked); err != nil { return err } - if pif != nil { - if err := d.Set("vlan", pif.Vlan); err != nil { - return err - } + if err := d.Set("vlan", vlan); err != nil { + return err } return nil } diff --git a/xoa/resource_xenorchestra_network_test.go b/xoa/resource_xenorchestra_network_test.go index ca57887f..10d95833 100644 --- a/xoa/resource_xenorchestra_network_test.go +++ b/xoa/resource_xenorchestra_network_test.go @@ -35,39 +35,6 @@ func TestAccXONetwork_create(t *testing.T) { ) } -func TestAccXONetwork_createBonded(t *testing.T) { - if accTestPIF.Id == "" { - t.Skip() - } - resourceName := "xenorchestra_network.network" - nameLabel := fmt.Sprintf("%s - %s", accTestPrefix, t.Name()) - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProviders, - Steps: []resource.TestStep{ - { - Config: testAccXenorchestraNetworkConfigBonded(nameLabel), - Check: resource.ComposeAggregateTestCheckFunc( - testAccCheckXenorchestraNetwork(resourceName), - resource.TestCheckResourceAttrSet(resourceName, "id"), - resource.TestCheckResourceAttrSet(resourceName, "name_label"), - resource.TestCheckResourceAttrSet(resourceName, "name_description"), - resource.TestCheckResourceAttrSet(resourceName, "pool_id"), - // resource.TestCheckResourceAttrSet(resourceName, "pif_ids"), - resource.TestCheckResourceAttrSet(resourceName, "mtu"), - resource.TestCheckNoResourceAttr(resourceName, "pif_id"), - resource.TestCheckResourceAttr(resourceName, "vlan", "-1")), - }, - // { - // Config: testAccXenorchestraNetworkConfigBonded(nameLabel), - // PlanOnly: true, - // // ExpectNonEmptyPlan: true, - // }, - }, - }, - ) -} - func TestAccXONetwork_createWithVlanRequiresPIF(t *testing.T) { if accTestPIF.Id == "" { t.Skip() @@ -297,27 +264,3 @@ resource "xenorchestra_network" "network" { } `, name, desc, accTestPIF.PoolId, nbd, automatic, isLocked) } - -var testAccXenorchestraNetworkConfigBonded = func(name string) string { - return fmt.Sprintf(` -data "xenorchestra_pif" "eth1" { - device = "eth1" - vlan = -1 - host_id = "%s" -} -data "xenorchestra_pif" "eth2" { - device = "eth2" - vlan = -1 - host_id = "%s" -} - -resource "xenorchestra_network" "network" { - name_label = "%s" - pool_id = "%s" - pif_ids = [ - data.xenorchestra_pif.eth1.id, - data.xenorchestra_pif.eth2.id, - ] - bond_mode = "active-backup" -}`, accTestPIF.Host, accTestPIF.Host, name, accTestPIF.PoolId) -}