From 516842d718d7f8765a6cb517894aac894f87c508 Mon Sep 17 00:00:00 2001 From: Dom Delnano Date: Wed, 20 Mar 2024 16:14:57 -0700 Subject: [PATCH] Replace VM resource's `wait_for_ip` functionality to validate it whether the ip(s) match a given prefix (#305) * Remove wait_for_ip top level argument in favor of network block scoped expected_ip_cidr Signed-off-by: Dom Del Nano * Implement logic for matching against multiple network interface cidr ranges Signed-off-by: Dom Del Nano * Implement cidr matching for multiple network interfaces and start test for verifying the lack of a match * Implement wait for ip test for the case where a network does not converge to the cidr match Signed-off-by: Dom Del Nano * Changes to fix V0 state migration test Signed-off-by: Dom Del Nano * Implement test that verifies migrating from wait_for_ip to expected_cidr_range works as expected Signed-off-by: Dom Del Nano * Fix issue with failing test and update docs Signed-off-by: Dom Del Nano --------- Signed-off-by: Dom Del Nano --- client/vm.go | 162 ++++++++++++------ docs/data-sources/vms.md | 2 +- docs/resources/vm.md | 11 +- .../resources/xenorchestra_vm/resource.tf | 5 +- xoa/data_source_vms.go | 1 - xoa/internal/state/migrate.go | 80 ++++++++- xoa/resource_xenorchestra_vm.go | 78 ++++++--- xoa/resource_xenorchestra_vm_test.go | 138 +++++++++------ 8 files changed, 339 insertions(+), 138 deletions(-) diff --git a/client/vm.go b/client/vm.go index a6a53c6..6591967 100644 --- a/client/vm.go +++ b/client/vm.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log" + "net" "net/http" "os" "strconv" @@ -135,15 +136,17 @@ type Vm struct { // These fields are used for passing in disk inputs when // creating Vms, however, this is not a real field as far // as the XO api or XAPI is concerned - Disks []Disk `json:"-"` - CloudNetworkConfig string `json:"-"` - VIFsMap []map[string]string `json:"-"` - WaitForIps bool `json:"-"` - Installation Installation `json:"-"` - ManagementAgentDetected bool `json:"managementAgentDetected"` - PVDriversDetected bool `json:"pvDriversDetected"` - DestroyCloudConfigVdiAfterBoot bool `json:"-"` - CloneType string `json:"-"` + Disks []Disk `json:"-"` + CloudNetworkConfig string `json:"-"` + VIFsMap []map[string]string `json:"-"` + // Map where the key is the network interface index and the + // value is a cidr range parsable by net.ParseCIDR + WaitForIps map[string]string `json:"-"` + Installation Installation `json:"-"` + ManagementAgentDetected bool `json:"managementAgentDetected"` + PVDriversDetected bool `json:"pvDriversDetected"` + DestroyCloudConfigVdiAfterBoot bool `json:"-"` + CloneType string `json:"-"` } type Installation struct { @@ -603,58 +606,117 @@ func (c *Client) waitForVmState(id string, stateConf StateChangeConf) error { return err } -func (c *Client) waitForModifyVm(id string, desiredPowerState string, waitForIp bool, timeout time.Duration) error { - if !waitForIp { - var pending []string - target := desiredPowerState - switch desiredPowerState { - case RunningPowerState: - pending = []string{HaltedPowerState} - case HaltedPowerState: - pending = []string{RunningPowerState} - default: - return errors.New(fmt.Sprintf("Invalid VM power state requested: %s\n", desiredPowerState)) +func waitForPowerStateReached(c *Client, vmId, desiredPowerState string, timeout time.Duration) error { + var pending []string + target := desiredPowerState + switch desiredPowerState { + case RunningPowerState: + pending = []string{HaltedPowerState} + case HaltedPowerState: + pending = []string{RunningPowerState} + default: + return errors.New(fmt.Sprintf("Invalid VM power state requested: %s\n", desiredPowerState)) + } + refreshFn := func() (result interface{}, state string, err error) { + vm, err := c.GetVm(Vm{Id: vmId}) + + if err != nil { + return vm, "", err } - refreshFn := func() (result interface{}, state string, err error) { - vm, err := c.GetVm(Vm{Id: id}) - if err != nil { - return vm, "", err - } + return vm, vm.PowerState, nil + } + stateConf := &StateChangeConf{ + Pending: pending, + Refresh: refreshFn, + Target: []string{target}, + Timeout: timeout, + } + _, err := stateConf.WaitForState() + return err +} + +type ifaceMatchCheck struct { + cidrRange string + ifaceIdx string + ifaceAddrs []string +} - return vm, vm.PowerState, nil +func waitForIPAssignment(c *Client, vmId string, waitForIps map[string]string, timeout time.Duration) error { + var lastResult ifaceMatchCheck + refreshFn := func() (result interface{}, state string, err error) { + vm, err := c.GetVm(Vm{Id: vmId}) + + if err != nil { + return vm, "", err } - stateConf := &StateChangeConf{ - Pending: pending, - Refresh: refreshFn, - Target: []string{target}, - Timeout: timeout, + + addrs := vm.Addresses + if len(addrs) == 0 || vm.PowerState != RunningPowerState { + return addrs, "Waiting", nil } - _, err := stateConf.WaitForState() - return err - } else { - refreshFn := func() (result interface{}, state string, err error) { - vm, err := c.GetVm(Vm{Id: id}) - if err != nil { - return vm, "", err + netIfaces := map[string][]string{} + for key, addr := range vm.Addresses { + + // key has the following format "{iface_id}/(ipv4|ipv6)/{iface_ip_id}" + ifaceIdx, _, _ := strings.Cut(key, "/") + if _, ok := netIfaces[ifaceIdx]; !ok { + netIfaces[ifaceIdx] = []string{} } + netIfaces[ifaceIdx] = append(netIfaces[ifaceIdx], addr) + } - l := len(vm.Addresses) - if l == 0 || vm.PowerState != RunningPowerState { - return vm, "Waiting", nil + for ifaceIdx, cidrRange := range waitForIps { + // VM's Addresses member does not contain this network interface yet + if _, ok := netIfaces[ifaceIdx]; !ok { + return addrs, "Waiting", nil } - return vm, "Ready", nil - } - stateConf := &StateChangeConf{ - Pending: []string{"Waiting"}, - Refresh: refreshFn, - Target: []string{"Ready"}, - Timeout: timeout, + found := false + for _, ipAddr := range netIfaces[ifaceIdx] { + _, ipNet, err := net.ParseCIDR(cidrRange) + + if err != nil { + return addrs, "Waiting", err + } + + if ipNet.Contains(net.ParseIP(ipAddr)) { + found = true + } + } + + if !found { + lastResult = ifaceMatchCheck{ + cidrRange: cidrRange, + ifaceIdx: ifaceIdx, + ifaceAddrs: netIfaces[ifaceIdx], + } + + return addrs, "Waiting", nil + } } - _, err := stateConf.WaitForState() - return err + + return addrs, "Ready", nil + } + stateConf := &StateChangeConf{ + Pending: []string{"Waiting"}, + Refresh: refreshFn, + Target: []string{"Ready"}, + Timeout: timeout, + } + _, err := stateConf.WaitForState() + if _, ok := err.(*TimeoutError); ok { + return errors.New(fmt.Sprintf("network[%s] never converged to the following cidr: %s, addresses: %s failed to match", lastResult.ifaceIdx, lastResult.cidrRange, lastResult.ifaceAddrs)) + } + return err +} + +func (c *Client) waitForModifyVm(id string, desiredPowerState string, waitForIps map[string]string, timeout time.Duration) error { + if len(waitForIps) == 0 { + return waitForPowerStateReached(c, id, desiredPowerState, timeout) + } else { + return waitForIPAssignment(c, id, waitForIps, timeout) } } diff --git a/docs/data-sources/vms.md b/docs/data-sources/vms.md index 88f57ba..7f5e17e 100644 --- a/docs/data-sources/vms.md +++ b/docs/data-sources/vms.md @@ -84,7 +84,6 @@ Read-Only: - `template` (String) - `vga` (String) - `videoram` (Number) -- `wait_for_ip` (Boolean) - `xenstore` (Map of String) @@ -109,6 +108,7 @@ Read-Only: - `attached` (Boolean) - `device` (String) +- `expected_ip_cidr` (String) - `ipv4_addresses` (List of String) - `ipv6_addresses` (List of String) - `mac_address` (String) diff --git a/docs/resources/vm.md b/docs/resources/vm.md index 7972c31..842fb18 100644 --- a/docs/resources/vm.md +++ b/docs/resources/vm.md @@ -98,13 +98,14 @@ resource "xenorchestra_vm" "bar" { } } -# vm resource that uses wait_for_ip +# vm resource that waits until its first network interface +# is assigned an IP via DHCP resource "xenorchestra_vm" "vm" { ... - wait_for_ip = true # Specify VM with two network interfaces network { ... + expected_ip_cidr = "10.0.0.0/16" } network { ... @@ -176,14 +177,13 @@ $ xo-cli xo.getAllObjects filter='json:{"id": "cf7b5d7d-3cd5-6b7c-5025-5c935c8cd - `timeouts` (Block, Optional) (see [below for nested schema](#nestedblock--timeouts)) - `vga` (String) The video adapter the VM should use. Possible values include std and cirrus. - `videoram` (Number) The videoram option the VM should use. Possible values include 1, 2, 4, 8, 16 -- `wait_for_ip` (Boolean) Whether terraform should wait until IP addresses are present on the VM's network interfaces before considering it created. This only works if guest-tools are installed in the VM. Defaults to false. - `xenstore` (Map of String) The key value pairs to be populated in xenstore. ### Read-Only - `id` (String) The ID of this resource. -- `ipv4_addresses` (List of String) This is only accessible if guest-tools is installed in the VM and if `wait_for_ip` is set to true. This will contain a list of the ipv4 addresses across all network interfaces in order. See the example terraform code for more details. -- `ipv6_addresses` (List of String) This is only accessible if guest-tools is installed in the VM and if `wait_for_ip` is set to true. This will contain a list of the ipv6 addresses across all network interfaces in order. +- `ipv4_addresses` (List of String) This is only accessible if guest-tools is installed in the VM and if `expected_ip_cidr` is set on any network interfaces. This will contain a list of the ipv4 addresses across all network interfaces in order. See the example terraform code for more details. +- `ipv6_addresses` (List of String) This is only accessible if guest-tools is installed in the VM and if `expected_ip_cidr` is set on any network interfaces. This will contain a list of the ipv6 addresses across all network interfaces in order. ### Nested Schema for `disk` @@ -216,6 +216,7 @@ Required: Optional: - `attached` (Boolean) Whether the device should be attached to the VM. +- `expected_ip_cidr` (String) Determines the IP cidr range terraform should watch for on this network interface. Resource creation is not complete until the IP address converges to the specified range. This only works if guest-tools are installed in the VM. Defaults to "", which skips IP address matching. - `mac_address` (String) The mac address of the network interface. This must be parsable by go's [net.ParseMAC function](https://golang.org/pkg/net/#ParseMAC). All mac addresses are stored in Terraform's state with [HardwareAddr's string representation](https://golang.org/pkg/net/#HardwareAddr.String) i.e. 00:00:5e:00:53:01 Read-Only: diff --git a/examples/resources/xenorchestra_vm/resource.tf b/examples/resources/xenorchestra_vm/resource.tf index 5ce62e7..82fbc8a 100644 --- a/examples/resources/xenorchestra_vm/resource.tf +++ b/examples/resources/xenorchestra_vm/resource.tf @@ -67,13 +67,14 @@ resource "xenorchestra_vm" "bar" { } } -# vm resource that uses wait_for_ip +# vm resource that waits until its first network interface +# is assigned an IP via DHCP resource "xenorchestra_vm" "vm" { ... - wait_for_ip = true # Specify VM with two network interfaces network { ... + expected_ip_cidr = "10.0.0.0/16" } network { ... diff --git a/xoa/data_source_vms.go b/xoa/data_source_vms.go index 5138275..49f0365 100644 --- a/xoa/data_source_vms.go +++ b/xoa/data_source_vms.go @@ -84,7 +84,6 @@ func vmToMapList(vms []client.Vm) []map[string]interface{} { "memory_max": vm.Memory.Static[1], "affinity_host": vm.AffinityHost, "template": vm.Template, - "wait_for_ip": vm.WaitForIps, "high_availability": vm.HA, "ipv4_addresses": ipv4, "ipv6_addresses": ipv6, diff --git a/xoa/internal/state/migrate.go b/xoa/internal/state/migrate.go index 75029df..24b2907 100644 --- a/xoa/internal/state/migrate.go +++ b/xoa/internal/state/migrate.go @@ -6,9 +6,9 @@ import ( "log" "net" - "github.com/vatesfr/terraform-provider-xenorchestra/client" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/vatesfr/terraform-provider-xenorchestra/client" ) var validVga = []string{ @@ -362,3 +362,81 @@ func suppressAttachedDiffWhenHalted(k, old, new string, d *schema.ResourceData) log.Printf("[DEBUG] VM '%s' attribute has transitioned from '%s' to '%s' when PowerState '%s'. Suppress diff: %t", k, old, new, powerState, suppress) return } + +// Used for tests that require the schema of the provider <= v0.28.1 +func V1TestAccVmConfigWithWaitForIp(testPrefix, vmName, templateName, netName, poolId, srId string) string { + return fmt.Sprintf(` +resource "xenorchestra_cloud_config" "bar" { + name = "%s-vm-template-%s" + template = "template" # should this be templated? +} + +data "xenorchestra_template" "template" { + name_label = "%s" + pool_id = "%s" +} + +data "xenorchestra_network" "network" { + name_label = "%s" + pool_id = "%s" +} +resource "xenorchestra_vm" "bar" { + memory_max = 4295000000 + cpus = 1 + cloud_config = xenorchestra_cloud_config.bar.template + name_label = "%s" + name_description = "description" + template = data.xenorchestra_template.template.id + network { + network_id = data.xenorchestra_network.network.id + } + disk { + sr_id = "%s" + name_label = "disk 1" + size = 10001317888 + } + wait_for_ip = true +} +`, testPrefix, vmName, templateName, poolId, netName, poolId, vmName, srId) +} + +// terraform configuration that can be used to block changes that should not destroy a VM. +// While this doesn't integrate nicely with the sdk's test helpers (failure is vague), there +// are some cases were options are limited (testing pinned provider versions). +func TestAccV1VmConfigWithDeletionBlocked(testPrefix, vmName, templateName, netName, poolId, srId, waitForIp string) string { + return fmt.Sprintf(` +resource "xenorchestra_cloud_config" "bar" { + name = "%s-vm-template-%s" + template = "template" # should this be templated? +} + +data "xenorchestra_template" "template" { + name_label = "%s" + pool_id = "%s" +} +data "xenorchestra_network" "network" { + name_label = "%s" + pool_id = "%s" +} + +resource "xenorchestra_vm" "bar" { + memory_max = 4295000000 + cpus = 1 + cloud_config = xenorchestra_cloud_config.bar.template + name_label = "%s" + name_description = "description" + template = data.xenorchestra_template.template.id + network { + network_id = data.xenorchestra_network.network.id + } + + disk { + sr_id = "%s" + name_label = "disk 1" + size = 10001317888 + } + wait_for_ip = %s + blocked_operations = ["destroy"] +} + `, testPrefix, vmName, templateName, poolId, netName, poolId, vmName, srId, waitForIp) +} diff --git a/xoa/resource_xenorchestra_vm.go b/xoa/resource_xenorchestra_vm.go index 2beae2b..f64d3b0 100644 --- a/xoa/resource_xenorchestra_vm.go +++ b/xoa/resource_xenorchestra_vm.go @@ -12,11 +12,11 @@ import ( "strings" "time" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/vatesfr/terraform-provider-xenorchestra/client" "github.com/vatesfr/terraform-provider-xenorchestra/xoa/internal" "github.com/vatesfr/terraform-provider-xenorchestra/xoa/internal/state" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) var validVga = []string{ @@ -239,14 +239,14 @@ $ xo-cli xo.getAllObjects filter='json:{"id": "cf7b5d7d-3cd5-6b7c-5025-5c935c8cd "ipv4_addresses": &schema.Schema{ Type: schema.TypeList, Computed: true, - Description: "This is only accessible if guest-tools is installed in the VM and if `wait_for_ip` is set to true. This will contain a list of the ipv4 addresses across all network interfaces in order. See the example terraform code for more details.", + Description: "This is only accessible if guest-tools is installed in the VM and if `expected_ip_cidr` is set on any network interfaces. This will contain a list of the ipv4 addresses across all network interfaces in order. See the example terraform code for more details.", Elem: &schema.Schema{ Type: schema.TypeString, }, }, "ipv6_addresses": &schema.Schema{ Type: schema.TypeList, - Description: "This is only accessible if guest-tools is installed in the VM and if `wait_for_ip` is set to true. This will contain a list of the ipv6 addresses across all network interfaces in order.", + Description: "This is only accessible if guest-tools is installed in the VM and if `expected_ip_cidr` is set on any network interfaces. This will contain a list of the ipv6 addresses across all network interfaces in order.", Computed: true, Elem: &schema.Schema{ Type: schema.TypeString, @@ -281,12 +281,6 @@ $ xo-cli xo.getAllObjects filter='json:{"id": "cf7b5d7d-3cd5-6b7c-5025-5c935c8cd Type: schema.TypeString, Optional: true, }, - "wait_for_ip": &schema.Schema{ - Type: schema.TypeBool, - Default: false, - Description: "Whether terraform should wait until IP addresses are present on the VM's network interfaces before considering it created. This only works if guest-tools are installed in the VM. Defaults to false.", - Optional: true, - }, "cdrom": &schema.Schema{ Type: schema.TypeList, Optional: true, @@ -357,6 +351,12 @@ $ xo-cli xo.getAllObjects filter='json:{"id": "cf7b5d7d-3cd5-6b7c-5025-5c935c8cd Type: schema.TypeString, }, }, + "expected_ip_cidr": &schema.Schema{ + Type: schema.TypeString, + Default: "", + Description: "Determines the IP cidr range terraform should watch for on this network interface. Resource creation is not complete until the IP address converges to the specified range. This only works if guest-tools are installed in the VM. Defaults to \"\", which skips IP address matching.", + Optional: true, + }, }, }, }, @@ -465,10 +465,11 @@ This does not work in terraform since that is applied on Xen Orchestra's client func resourceVmCreate(d *schema.ResourceData, m interface{}) error { c := m.(client.XOClient) - network_maps := []map[string]string{} + vifsMap := []map[string]string{} + waitForIpsMap := map[string]string{} networks := d.Get("network").([]interface{}) - for _, network := range networks { + for index, network := range networks { netMap, _ := network.(map[string]interface{}) netID := netMap["network_id"].(string) @@ -477,12 +478,17 @@ func resourceVmCreate(d *schema.ResourceData, m interface{}) error { netMapToAdd := map[string]string{ "network": netID, } - // We only add the mac address if it contains a value. + // Only add the mac address if it contains a value. if macAddr != "" { netMapToAdd["mac"] = getFormattedMac(macAddr) } - network_maps = append(network_maps, netMapToAdd) + expectedCidr := netMap["expected_ip_cidr"].(string) + if expectedCidr != "" { + waitForIpsMap[strconv.Itoa(index)] = expectedCidr + } + + vifsMap = append(vifsMap, netMapToAdd) } ds := []client.Disk{} @@ -524,7 +530,7 @@ func resourceVmCreate(d *schema.ResourceData, m interface{}) error { if installMethod := d.Get("installation_method").(string); installMethod != "" { installation = client.Installation{ - Method: "network", + Method: "network", Repository: "pxe", } } @@ -570,9 +576,9 @@ func resourceVmCreate(d *schema.ResourceData, m interface{}) error { Installation: installation, // TODO: (#145) Uncomment this once issues with secure_boot have been figured out // SecureBoot: d.Get("secure_boot").(bool), - VIFsMap: network_maps, + VIFsMap: vifsMap, StartDelay: d.Get("start_delay").(int), - WaitForIps: d.Get("wait_for_ip").(bool), + WaitForIps: waitForIpsMap, Videoram: client.Videoram{ Value: d.Get("videoram").(int), }, @@ -682,9 +688,21 @@ func cdromsToMapList(disks []client.Disk) []map[string]interface{} { return result } -func vifsToMapList(vifs []client.VIF, guestNets []guestNetwork) []map[string]interface{} { +func vifsToMapList(vifs []client.VIF, guestNets []guestNetwork, d *schema.ResourceData) []map[string]interface{} { result := make([]map[string]interface{}, 0, len(vifs)) - for _, vif := range vifs { + + expectedCidrs := map[string]string{} + + networks := d.Get("network").([]interface{}) + for index, network := range networks { + netMap := network.(map[string]interface{}) + expectedCidr := netMap["expected_ip_cidr"].(string) + if expectedCidr == "" { + continue + } + expectedCidrs[strconv.Itoa(index)] = expectedCidr + } + for index, vif := range vifs { ipv6Addrs := []string{} ipv4Addrs := []string{} device, _ := strconv.Atoi(vif.Device) @@ -701,6 +719,10 @@ func vifsToMapList(vifs []client.VIF, guestNets []guestNetwork) []map[string]int "ipv4_addresses": ipv4Addrs, "ipv6_addresses": ipv6Addrs, } + + if cidr, ok := expectedCidrs[strconv.Itoa(index)]; ok { + vifMap["expected_ip_cidr"] = cidr + } result = append(result, vifMap) } @@ -1174,14 +1196,18 @@ func recordToData(resource client.Vm, vifs []client.VIF, disks []client.Disk, cd } log.Printf("[DEBUG] Found the following ip addresses: %v\n", resource.Addresses) - networkIps := extractIpsFromNetworks(resource.Addresses) - nets := vifsToMapList(vifs, networkIps) - err := d.Set("network", nets) + networkIps, err := extractIpsFromNetworks(resource.Addresses) if err != nil { return err } + nets := vifsToMapList(vifs, networkIps, d) + fmt.Printf("[INFO] Setting the vifsToMapList: %v\n", nets) + if err := d.Set("network", nets); err != nil { + return err + } + disksMapList := disksToMapList(disks) err = d.Set("disk", disksMapList) if err != nil { @@ -1418,10 +1444,10 @@ type guestNetwork map[string][]string // "ipv6": []string{"ip1", "ip2"} // }, // } -func extractIpsFromNetworks(networks map[string]string) []guestNetwork { +func extractIpsFromNetworks(networks map[string]string) ([]guestNetwork, error) { if len(networks) < 1 { - return []guestNetwork{} + return []guestNetwork{}, nil } IP_REGEX := `^(\d+)\/(ip(?:v4|v6)?)(?:\/(\d+))?$` @@ -1437,7 +1463,7 @@ func extractIpsFromNetworks(networks map[string]string) []guestNetwork { matches := reg.FindStringSubmatch(last) if matches == nil || len(matches) != 4 { - panic("this should never happen") + return nil, fmt.Errorf("received a malformed IP address field from XO api: line=%s regex matches=%v", last, matches) } cap, _ := strconv.Atoi(matches[1]) devices := make([]guestNetwork, 0, cap) @@ -1463,7 +1489,7 @@ func extractIpsFromNetworks(networks map[string]string) []guestNetwork { devices[deviceNum][proto] = append(devices[deviceNum][proto], networks[key]) } log.Printf("[DEBUG] Extracted the following network interface ips: %v\n", devices) - return devices + return devices, nil } func suppressEquivalentMAC(k, old, new string, d *schema.ResourceData) (suppress bool) { diff --git a/xoa/resource_xenorchestra_vm_test.go b/xoa/resource_xenorchestra_vm_test.go index d830e72..0af101e 100644 --- a/xoa/resource_xenorchestra_vm_test.go +++ b/xoa/resource_xenorchestra_vm_test.go @@ -10,10 +10,12 @@ import ( "testing" "time" - "github.com/vatesfr/terraform-provider-xenorchestra/client" - "github.com/vatesfr/terraform-provider-xenorchestra/xoa/internal" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/vatesfr/terraform-provider-xenorchestra/client" + "github.com/vatesfr/terraform-provider-xenorchestra/xoa/internal" + "github.com/vatesfr/terraform-provider-xenorchestra/xoa/internal/state" ) func init() { @@ -68,7 +70,7 @@ func Test_extractIpsFromNetworks(t *testing.T) { for _, test := range tests { expected := test.expected nets := test.networks - actual := extractIpsFromNetworks(nets) + actual, _ := extractIpsFromNetworks(nets) if len(expected) != len(actual) { t.Errorf("expected '%+v' to have the same length as: %+v", expected, actual) @@ -610,7 +612,6 @@ func TestAccXenorchestraVm_createWhenWaitingForIp(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccVmExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "id"), - resource.TestCheckResourceAttr(resourceName, "wait_for_ip", "true"), resource.TestMatchResourceAttr(resourceName, "ipv6_addresses.#", regex), resource.TestCheckResourceAttrSet(resourceName, "ipv6_addresses.0"), resource.TestMatchResourceAttr(resourceName, "network.0.ipv6_addresses.#", regex), @@ -621,6 +622,31 @@ func TestAccXenorchestraVm_createWhenWaitingForIp(t *testing.T) { }) } +func TestAccXenorchestraVm_waitForIpFailed(t *testing.T) { + resourceName := "xenorchestra_vm.bar" + vmName := fmt.Sprintf("%s - %s", accTestPrefix, t.Name()) + regex := regexp.MustCompile(`[1-9]*`) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckXenorchestraVmDestroy, + Steps: []resource.TestStep{ + { + Config: testAccVmConfigWithWaitForIp(vmName, "8.8.8.8/32"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccVmExists(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "id"), + resource.TestMatchResourceAttr(resourceName, "ipv6_addresses.#", regex), + resource.TestCheckResourceAttrSet(resourceName, "ipv6_addresses.0"), + resource.TestMatchResourceAttr(resourceName, "network.0.ipv6_addresses.#", regex), + resource.TestCheckResourceAttrSet(resourceName, "network.0.ipv6_addresses.0"), + ), + ExpectError: regexp.MustCompile(`network\[0\] never converged to the following cidr: 8.8.8.8\/32`), + }, + }, + }) +} + func TestAccXenorchestraVm_createAndUpdateXenstoreData(t *testing.T) { resourceName := "xenorchestra_vm.bar" vmName := fmt.Sprintf("%s - %s", accTestPrefix, t.Name()) @@ -1377,7 +1403,7 @@ func TestAccXenorchestraVm_addVifAndRemoveVif(t *testing.T) { CheckDestroy: testAccCheckXenorchestraVmDestroy, Steps: []resource.TestStep{ { - Config: testAccVmConfigWithWaitForIp(vmName, "true"), + Config: testAccVmConfigWithWaitForIp(vmName, "0.0.0.0/0"), Check: resource.ComposeAggregateTestCheckFunc( testAccVmExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "id"), @@ -1736,7 +1762,7 @@ func TestAccXenorchestraVm_createWithV0StateMigration(t *testing.T) { VersionConstraint: "0.24.2", }, }, - Config: testAccVmConfigWithWaitForIp(vmName, "false"), + Config: state.V1TestAccVmConfigWithWaitForIp(accTestPrefix, vmName, testTemplate.NameLabel, accDefaultNetwork.NameLabel, accTestPool.Id, accDefaultSr.Id), Check: resource.ComposeAggregateTestCheckFunc( testAccVmExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "id"), @@ -1750,7 +1776,7 @@ func TestAccXenorchestraVm_createWithV0StateMigration(t *testing.T) { VersionConstraint: "0.24.2", }, }, - Config: testAccVmConfigWithDeletionBlocked(vmName, "false"), + Config: state.TestAccV1VmConfigWithDeletionBlocked(accTestPrefix, vmName, testTemplate.NameLabel, accDefaultNetwork.NameLabel, accTestPool.Id, accDefaultSr.Id, "false"), Check: resource.ComposeAggregateTestCheckFunc( testAccVmExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "id"), @@ -1764,7 +1790,7 @@ func TestAccXenorchestraVm_createWithV0StateMigration(t *testing.T) { VersionConstraint: "0.25.0", }, }, - Config: testAccVmConfigWithDeletionBlocked(vmName, "true"), + Config: state.TestAccV1VmConfigWithDeletionBlocked(accTestPrefix, vmName, testTemplate.NameLabel, accDefaultNetwork.NameLabel, accTestPool.Id, accDefaultSr.Id, "true"), Check: resource.ComposeAggregateTestCheckFunc( testAccVmExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "id"), @@ -1781,7 +1807,7 @@ func TestAccXenorchestraVm_createWithV0StateMigration(t *testing.T) { VersionConstraint: "0.25.1", }, }, - Config: testAccVmConfigWithDeletionBlocked(vmName, "true"), + Config: state.TestAccV1VmConfigWithDeletionBlocked(accTestPrefix, vmName, testTemplate.NameLabel, accDefaultNetwork.NameLabel, accTestPool.Id, accDefaultSr.Id, "true"), Check: resource.ComposeAggregateTestCheckFunc( testAccVmExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "id"), @@ -1795,7 +1821,7 @@ func TestAccXenorchestraVm_createWithV0StateMigration(t *testing.T) { VersionConstraint: "0.25.1", }, }, - Config: testAccVmConfigWithDeletionBlocked(vmName, "true"), + Config: state.TestAccV1VmConfigWithDeletionBlocked(accTestPrefix, vmName, testTemplate.NameLabel, accDefaultNetwork.NameLabel, accTestPool.Id, accDefaultSr.Id, "true"), Check: resource.ComposeAggregateTestCheckFunc( testAccVmExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "id"), @@ -1811,7 +1837,7 @@ func TestAccXenorchestraVm_createWithV0StateMigration(t *testing.T) { VersionConstraint: "0.25.1", }, }, - Config: testAccVmConfigWithWaitForIp(vmName, "true"), + Config: state.V1TestAccVmConfigWithWaitForIp(accTestPrefix, vmName, testTemplate.NameLabel, accDefaultNetwork.NameLabel, accTestPool.Id, accDefaultSr.Id), Check: resource.ComposeAggregateTestCheckFunc( testAccVmExists(resourceName), resource.TestCheckResourceAttrSet(resourceName, "id"), @@ -1822,6 +1848,46 @@ func TestAccXenorchestraVm_createWithV0StateMigration(t *testing.T) { }) } +func TestAccXenorchestraVm_createWithV1StateMigration(t *testing.T) { + resourceName := "xenorchestra_vm.bar" + vmName := fmt.Sprintf("%s - %s", accTestPrefix, t.Name()) + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + CheckDestroy: testAccCheckXenorchestraVmDestroy, + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "xenorchestra": { + Source: "vatesfr/xenorchestra", + VersionConstraint: "0.28.1", + }, + }, + Config: state.V1TestAccVmConfigWithWaitForIp(accTestPrefix, vmName, testTemplate.NameLabel, accDefaultNetwork.NameLabel, accTestPool.Id, accDefaultSr.Id), + Check: resource.ComposeAggregateTestCheckFunc( + testAccVmExists(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "id"), + resource.TestCheckNoResourceAttr(resourceName, "network.0.expected_ip_cidr"), + resource.TestCheckResourceAttr(resourceName, "wait_for_ip", "true"), + ), + }, + { + ProviderFactories: map[string]func() (*schema.Provider, error){ + "xenorchestra": func() (*schema.Provider, error) { + return Provider(), nil + }, + }, + Config: testAccVmConfigWithWaitForIp(vmName, "0.0.0.0/0"), + Check: resource.ComposeAggregateTestCheckFunc( + testAccVmExists(resourceName), + resource.TestCheckResourceAttrSet(resourceName, "id"), + resource.TestCheckResourceAttr(resourceName, "network.0.expected_ip_cidr", "0.0.0.0/0"), + resource.TestCheckNoResourceAttr(resourceName, "wait_for_ip"), + ), + }, + }, + }) +} + func testAccVmExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[resourceName] @@ -2042,42 +2108,10 @@ resource "xenorchestra_vm" "bar" { } func testAccVmConfig(vmName string) string { - return testAccVmConfigWithWaitForIp(vmName, "false") -} - -// terraform configuration that can be used to block changes that should not destroy a VM. -// While this doesn't integrate nicely with the sdk's test helpers (failure is vague), there -// are some cases were options are limited (testing pinned provider versions). -func testAccVmConfigWithDeletionBlocked(vmName, waitForIp string) string { - return testAccCloudConfigConfig(fmt.Sprintf("vm-template-%s", vmName), "template") + testAccTemplateConfig() + fmt.Sprintf(` -data "xenorchestra_network" "network" { - name_label = "%s" - pool_id = "%s" -} - -resource "xenorchestra_vm" "bar" { - memory_max = 4295000000 - cpus = 1 - cloud_config = xenorchestra_cloud_config.bar.template - name_label = "%s" - name_description = "description" - template = data.xenorchestra_template.template.id - network { - network_id = data.xenorchestra_network.network.id - } - - disk { - sr_id = "%s" - name_label = "disk 1" - size = 10001317888 - } - wait_for_ip = %s - blocked_operations = ["destroy"] -} -`, accDefaultNetwork.NameLabel, accTestPool.Id, vmName, accDefaultSr.Id, waitForIp) + return testAccVmConfigWithWaitForIp(vmName, "") } -func testAccVmConfigWithWaitForIp(vmName, waitForIp string) string { +func testAccVmConfigWithWaitForIp(vmName, expectedIpCidr string) string { return testAccCloudConfigConfig(fmt.Sprintf("vm-template-%s", vmName), "template") + testAccTemplateConfig() + fmt.Sprintf(` data "xenorchestra_network" "network" { name_label = "%s" @@ -2093,6 +2127,7 @@ resource "xenorchestra_vm" "bar" { template = data.xenorchestra_template.template.id network { network_id = data.xenorchestra_network.network.id + expected_ip_cidr = "%s" } disk { @@ -2100,9 +2135,8 @@ resource "xenorchestra_vm" "bar" { name_label = "disk 1" size = 10001317888 } - wait_for_ip = %s } -`, accDefaultNetwork.NameLabel, accTestPool.Id, vmName, accDefaultSr.Id, waitForIp) +`, accDefaultNetwork.NameLabel, accTestPool.Id, vmName, expectedIpCidr, accDefaultSr.Id) } func testAccVmConfigFullClone(vmName string) string { @@ -2163,7 +2197,7 @@ resource "xenorchestra_vm" "bar" { `, accDefaultNetwork.NameLabel, accTestPool.Id, vmName, accDefaultSr.Id, powerState) } -// This sets destroy_cloud_config_vdi_after_boot and wait_for_ip. The former is required for +// This sets destroy_cloud_config_vdi_after_boot and expected_ip_cidr. The former is required for // the test expectations while the latter is to ensure the test holds its assertions until the // disk was actually deleted. The XO api uses the guest metrics to determine when it can remove // the disk, so an IP address allocation happens at the same time. @@ -2184,9 +2218,9 @@ resource "xenorchestra_vm" "bar" { destroy_cloud_config_vdi_after_boot = true network { network_id = data.xenorchestra_network.network.id + expected_ip_cidr = "0.0.0.0/0" } power_state = "%s" - wait_for_ip = true disk { sr_id = "%s" @@ -2347,7 +2381,6 @@ data "xenorchestra_network" "network" { resource "xenorchestra_vm" "bar" { memory_max = 4295000000 - wait_for_ip = true cpus = 1 cloud_config = xenorchestra_cloud_config.bar.template name_label = "%s" @@ -2355,6 +2388,7 @@ resource "xenorchestra_vm" "bar" { template = data.xenorchestra_template.template.id network { network_id = data.xenorchestra_network.network.id + expected_ip_cidr = "0.0.0.0/0" } disk { @@ -2375,7 +2409,6 @@ data "xenorchestra_network" "network" { resource "xenorchestra_vm" "bar" { memory_max = 4295000000 - wait_for_ip = true cpus = 1 cloud_config = xenorchestra_cloud_config.bar.template name_label = "%s" @@ -2383,6 +2416,7 @@ resource "xenorchestra_vm" "bar" { template = data.xenorchestra_template.template.id network { network_id = data.xenorchestra_network.network.id + expected_ip_cidr = "0.0.0.0/0" } disk { @@ -2413,7 +2447,6 @@ data "xenorchestra_network" "network" { resource "xenorchestra_vm" "bar" { memory_max = 4295000000 - wait_for_ip = true cpus = 1 cloud_config = xenorchestra_cloud_config.bar.template name_label = "%s" @@ -2421,6 +2454,7 @@ resource "xenorchestra_vm" "bar" { template = data.xenorchestra_template.template.id network { network_id = data.xenorchestra_network.network.id + expected_ip_cidr = "0.0.0.0/0" } disk {