Skip to content

Commit

Permalink
Introduce source_pif_device (replacement for pif_id) to better ma…
Browse files Browse the repository at this point in the history 
…tch TF's mental model fix import issues (#260)

* Replace problematic pif_id argument with source_pif_device to address network resource import issues

Signed-off-by: Dom Del Nano <[email protected]>

* Update network resource docs

* Fix pif filtering

Signed-off-by: Dom Del Nano <[email protected]>

---------

Signed-off-by: Dom Del Nano <[email protected]>
  • Loading branch information
@ddelnano
ddelnano authored Sep 26, 2023
1 parent 8a0b0cd commit 7a13658
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 41 deletions.
15 changes: 12 additions & 3 deletions client/pif.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,24 @@ type PIF struct {
func (p PIF) Compare(obj interface{}) bool {
otherPif := obj.(PIF)

if p.Id != "" && otherPif.Id == p.Id {
return true
if p.Id != "" {
if otherPif.Id == p.Id {
return true
} else {
return false
}
}
hostIdExists := p.Host != ""
if hostIdExists && p.Host != otherPif.Host {
return false
}

if p.Vlan == otherPif.Vlan && p.Device == otherPif.Device {
networkIdExists := p.Network != ""
if networkIdExists && p.Network != otherPif.Network {
return false
}

if p.Vlan == otherPif.Vlan && (p.Device == "" || (p.Device == otherPif.Device)) {
return true
}
return false
Expand Down
15 changes: 8 additions & 7 deletions docs/resources/network.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,17 @@ data "xenorchestra_host" "host1" {
name_label = "Your host"
}
data "xenorchestra_pif" "pif" {
device = "eth0"
vlan = -1
host_id = data.xenorchestra_host.host1.id
# Create a single server network private network
resource "xenorchestra_network" "private_network" {
name_label = "new network name"
pool_id = data.xenorchestra_host.host1.pool_id
}
resource "xenorchestra_network" "network" {
# Create a network with a 22 VLAN tag from the eth0 device
resource "xenorchestra_network" "vlan_network" {
name_label = "new network name"
pool_id = data.xenorchestra_host.host1.pool_id
pif_id = data.xenorchestra_pif.pif.id
source_pif_device = "eth0"
vlan = 22
}
```
Expand All @@ -46,7 +47,7 @@ resource "xenorchestra_network" "network" {
- `mtu` (Number) The MTU of the network. Defaults to `1500` if unspecified.
- `name_description` (String)
- `nbd` (Boolean) Whether the network should use a network block device. Defaults to `false` if unspecified.
- `pif_id` (String) The pif (uuid) that should be used for this network.
- `source_pif_device` (String) The PIF device (eth0, eth1, etc) that will be used as an input during network creation. This parameter is required if a vlan is specified.
- `vlan` (Number) The vlan to use for the network. Defaults to `0` meaning no VLAN.

### Read-Only
Expand Down
13 changes: 7 additions & 6 deletions examples/resources/xenorchestra_network/resource.tf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,16 @@ data "xenorchestra_host" "host1" {
name_label = "Your host"
}

data "xenorchestra_pif" "pif" {
device = "eth0"
vlan = -1
host_id = data.xenorchestra_host.host1.id
# Create a single server network private network
resource "xenorchestra_network" "private_network" {
name_label = "new network name"
pool_id = data.xenorchestra_host.host1.pool_id
}

resource "xenorchestra_network" "network" {
# Create a network with a 22 VLAN tag from the eth0 device
resource "xenorchestra_network" "vlan_network" {
name_label = "new network name"
pool_id = data.xenorchestra_host.host1.pool_id
pif_id = data.xenorchestra_pif.pif.id
source_pif_device = "eth0"
vlan = 22
}
76 changes: 62 additions & 14 deletions xoa/resource_xenorchestra_network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package xoa

import (
"errors"
"fmt"
"log"

"github.com/ddelnano/terraform-provider-xenorchestra/client"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
Expand Down Expand Up @@ -40,21 +42,21 @@ func resourceXoaNetwork() *schema.Resource {
Optional: true,
Default: netDefaultDesc,
},
"pif_id": &schema.Schema{
"source_pif_device": &schema.Schema{
Type: schema.TypeString,
Optional: true,
RequiredWith: []string{
"vlan",
},
ForceNew: true,
Description: "The pif (uuid) that should be used for this network.",
Description: "The PIF device (eth0, eth1, etc) that will be used as an input during network creation. This parameter is required if a vlan is specified.",
},
"vlan": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
Default: 0,
RequiredWith: []string{
"pif_id",
"source_pif_device",
},
ForceNew: true,
Description: "The vlan to use for the network. Defaults to `0` meaning no VLAN.",
Expand Down Expand Up @@ -85,6 +87,16 @@ func resourceXoaNetwork() *schema.Resource {
func resourceNetworkCreate(d *schema.ResourceData, m interface{}) error {
c := m.(client.XOClient)

var pifId string
if sourcePIFDevice := d.Get("source_pif_device").(string); sourcePIFDevice != "" {
pif, err := getNetworkCreationSourcePIF(c, sourcePIFDevice, d.Get("pool_id").(string))

if err != nil {
return err
}
pifId = pif.Id
}

network, err := c.CreateNetwork(client.CreateNetworkRequest{
Automatic: d.Get("automatic").(bool),
DefaultIsLocked: d.Get("default_is_locked").(bool),
Expand All @@ -94,31 +106,63 @@ func resourceNetworkCreate(d *schema.ResourceData, m interface{}) error {
Mtu: d.Get("mtu").(int),
Nbd: d.Get("nbd").(bool),
Vlan: d.Get("vlan").(int),
PIF: d.Get("pif_id").(string),
PIF: pifId,
})
if err != nil {
return err
}
vlan, err := getVlanForNetwork(c, network)
vlan, pifDevice, err := getVlanForNetwork(c, network)
if err != nil {
return err
}
return networkToData(network, vlan, d)
return networkToData(network, vlan, pifDevice, d)
}

func getVlanForNetwork(c client.XOClient, net *client.Network) (int, error) {
// This function returns the PIF specified the given device name on the pool's primary host. In order to create
// networks with a VLAN, a PIF for the given device must be provided. Xen Orchestra uses the primary host's PIF
// for this and so we emulate that behavior.
func getNetworkCreationSourcePIF(c client.XOClient, pifDevice, poolId string) (*client.PIF, error) {
pools, err := c.GetPools(client.Pool{Id: poolId})
if err != nil {
return nil, err
}

if len(pools) != 1 {
return nil, errors.New(fmt.Sprintf("expected to find a single pool, instead found %d", len(pools)))
}

pool := pools[0]
pifs, err := c.GetPIF(client.PIF{
Host: pool.Master,
Vlan: -1,
Device: pifDevice,
})

if err != nil {
return nil, err
}

if len(pifs) != 1 {
return nil, errors.New(fmt.Sprintf("expected to find a single PIF, instead found %d. %+v", len(pifs), pifs))
}

return &pifs[0], nil
}

// Returns the VLAN and device name for the given network.
func getVlanForNetwork(c client.XOClient, net *client.Network) (int, string, error) {
if len(net.PIFs) > 0 {
pifs, err := c.GetPIF(client.PIF{Id: net.PIFs[0]})
if err != nil {
return -1, err
return -1, "", err
}

if len(pifs) != 1 {
return -1, errors.New("expected to find single PIF")
return -1, "", errors.New("expected to find single PIF")
}
return pifs[0].Vlan, nil
return pifs[0].Vlan, pifs[0].Device, nil
}
return 0, nil
return 0, "", nil
}

func resourceNetworkRead(d *schema.ResourceData, m interface{}) error {
Expand All @@ -135,11 +179,11 @@ func resourceNetworkRead(d *schema.ResourceData, m interface{}) error {
return err
}

vlan, err := getVlanForNetwork(c, net)
vlan, pifDevice, err := getVlanForNetwork(c, net)
if err != nil {
return err
}
return networkToData(net, vlan, d)
return networkToData(net, vlan, pifDevice, d)
}

func resourceNetworkUpdate(d *schema.ResourceData, m interface{}) error {
Expand Down Expand Up @@ -178,11 +222,12 @@ func resourceNetworkDelete(d *schema.ResourceData, m interface{}) error {
return nil
}

func networkToData(network *client.Network, vlan int, d *schema.ResourceData) error {
func networkToData(network *client.Network, vlan int, pifDevice string, d *schema.ResourceData) error {
d.SetId(network.Id)
if err := d.Set("name_label", network.NameLabel); err != nil {
return err
}
log.Printf("This is being called\n")
if err := d.Set("name_description", network.NameDescription); err != nil {
return err
}
Expand All @@ -204,5 +249,8 @@ func networkToData(network *client.Network, vlan int, d *schema.ResourceData) er
if err := d.Set("vlan", vlan); err != nil {
return err
}
if err := d.Set("source_pif_device", pifDevice); err != nil {
return err
}
return nil
}
53 changes: 42 additions & 11 deletions xoa/resource_xenorchestra_network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,42 @@ func TestAccXONetwork_create(t *testing.T) {
)
}

func TestAccXONetwork_import(t *testing.T) {
if accTestPIF.Id == "" {
t.Skip()
}
resourceName := "xenorchestra_network.network"
nameLabel := fmt.Sprintf("%s - %s", accTestPrefix, t.Name())
desc := "Non default description"
nbd := "false"
mtu := "1500"
vlan := "23"
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccXenorchestraNetworkConfigNonDefaultsWithVlan(nameLabel, desc, mtu, nbd, vlan),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
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"),
resource.TestCheckResourceAttrSet(resourceName, "source_pif_device"),
resource.TestCheckResourceAttrSet(resourceName, "vlan")),
},
},
},
)
}

func TestAccXONetwork_createWithVlanRequiresPIF(t *testing.T) {
if accTestPIF.Id == "" {
t.Skip()
Expand All @@ -46,11 +82,11 @@ func TestAccXONetwork_createWithVlanRequiresPIF(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccXenorchestraNetworkConfigWithoutVlan(nameLabel),
ExpectError: regexp.MustCompile("all of `pif_id,vlan` must be specified"),
ExpectError: regexp.MustCompile("all of `source_pif_device,vlan` must be specified"),
},
{
Config: testAccXenorchestraNetworkConfigWithoutPIF(nameLabel),
ExpectError: regexp.MustCompile("all of `pif_id,vlan` must be specified"),
ExpectError: regexp.MustCompile("all of `source_pif_device,vlan` must be specified"),
},
},
},
Expand Down Expand Up @@ -80,6 +116,7 @@ func TestAccXONetwork_createWithNonDefaults(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "name_description", desc),
resource.TestCheckResourceAttr(resourceName, "pool_id", accTestPIF.PoolId),
resource.TestCheckResourceAttr(resourceName, "mtu", mtu),
resource.TestCheckResourceAttrSet(resourceName, "source_pif_device"),
resource.TestCheckResourceAttr(resourceName, "vlan", vlan),
resource.TestCheckResourceAttr(resourceName, "nbd", nbd)),
},
Expand Down Expand Up @@ -205,7 +242,7 @@ var testAccXenorchestraNetworkConfigWithoutVlan = func(name string) string {
resource "xenorchestra_network" "network" {
name_label = "%s"
pool_id = "%s"
pif_id = data.xenorchestra_pif.pif.id
source_pif_device = "eth1"
}
`, name, accTestPIF.PoolId)
}
Expand Down Expand Up @@ -234,22 +271,16 @@ resource "xenorchestra_network" "network" {

var testAccXenorchestraNetworkConfigNonDefaultsWithVlan = func(name, desc, mtu, nbd, vlan string) string {
return fmt.Sprintf(`
data "xenorchestra_pif" "pif" {
device = "eth0"
vlan = -1
host_id = "%s"
}
resource "xenorchestra_network" "network" {
name_label = "%s"
name_description = "%s"
pool_id = "%s"
mtu = %s
nbd = %s
pif_id = data.xenorchestra_pif.pif.id
source_pif_device = "eth0"
vlan = %s
}
`, accTestPIF.Host, name, desc, accTestPIF.PoolId, mtu, nbd, vlan)
`, name, desc, accTestPIF.PoolId, mtu, nbd, vlan)
}

var testAccXenorchestraNetworkConfigInPlaceUpdates = func(name, desc, nbd, automatic, isLocked string) string {
Expand Down

0 comments on commit 7a13658

Please sign in to comment.