Add reboot_vapp_on_removal flag to vcd_vapp_network and vcd_vapp_org_…

…network (#1004)

.changes/v3.9.0/1004-improvements.md

@@ -0,0 +1,4 @@
+* Resources `vcd_vapp_network` and `vcd_vapp_org_network` add convenience flag
+  `reboot_vapp_on_removal`. When enabled, it will power off parent vApp (and power back on after
+  if it was before) during vApp network removal. This improves workflows with VCD 10.4.1+ which
+  returns an error when removing vApp networks from powered on vApps [GH-1004]

scripts/skip-upgrade-tests.txt

@@ -260,4 +260,6 @@ vcd.TestAccVcdVappFirewallRules-step2.tf v3.7.0 "vApp network removal from power
 vcd.TestAccVcdVappFirewallRules.tf v3.7.0 "vApp network removal from powered on vApp is not supported in API V37.1+"
 vcd.TestAccVcdVAppVmCustomizationSettings.tf v3.7.0 "vApp network removal from powered on vApp is not supported in API V37.1+"
 vcd.TestAccVcdVAppVmMulti.tf v3.7.0 "vApp network removal from powered on vApp is not supported in API V37.1+"
-vcd.ResourceSchema-vcd_nsxt_alb_settings.tf v3.8.2 "New fields to support latest ALB features"
+vcd.ResourceSchema-vcd_nsxt_alb_settings.tf v3.8.2 "New fields to support latest ALB features"
+vcd.ResourceSchema-vcd_vapp_org_network.tf v3.8.2 "New field 'reboot_vapp_on_removal'"
+vcd.ResourceSchema-vcd_vapp_network.tf v3.8.2 "New field 'reboot_vapp_on_removal'"

vcd/config_test.go

@@ -10,7 +10,6 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
-	"github.com/kr/pretty"
 	"net/url"
 	"os"
 	"path"
@@ -22,6 +21,8 @@ import (
 	"text/template"
 	"time"
 
+	"github.com/kr/pretty"
+
 	"github.com/hashicorp/go-version"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -135,21 +136,23 @@ type TestConfig struct {
 		} `json:"peer"`
 	} `json:"networking"`
 	Nsxt struct {
-		Manager             string `json:"manager"`
-		Tier0router         string `json:"tier0router"`
-		Tier0routerVrf      string `json:"tier0routervrf"`
-		Vdc                 string `json:"vdc"`
-		ExternalNetwork     string `json:"externalNetwork"`
-		EdgeGateway         string `json:"edgeGateway"`
-		VdcGroup            string `json:"vdcGroup"`
-		VdcGroupEdgeGateway string `json:"vdcGroupEdgeGateway"`
-		NsxtImportSegment   string `json:"nsxtImportSegment"`
-		NsxtEdgeCluster     string `json:"nsxtEdgeCluster"`
-
+		Manager                   string `json:"manager"`
+		Tier0router               string `json:"tier0router"`
+		Tier0routerVrf            string `json:"tier0routervrf"`
+		Vdc                       string `json:"vdc"`
+		ExternalNetwork           string `json:"externalNetwork"`
+		EdgeGateway               string `json:"edgeGateway"`
+		VdcGroup                  string `json:"vdcGroup"`
+		VdcGroupEdgeGateway       string `json:"vdcGroupEdgeGateway"`
+		NsxtImportSegment         string `json:"nsxtImportSegment"`
+		NsxtEdgeCluster           string `json:"nsxtEdgeCluster"`
 		NsxtAlbControllerUrl      string `json:"nsxtAlbControllerUrl"`
 		NsxtAlbControllerUser     string `json:"nsxtAlbControllerUser"`
 		NsxtAlbControllerPassword string `json:"nsxtAlbControllerPassword"`
 		NsxtAlbImportableCloud    string `json:"nsxtAlbImportableCloud"`
+		RoutedNetwork             string `json:"routedNetwork"`
+		IsolatedNetwork           string `json:"isolatedNetwork"`
+		DirectNetwork             string `json:"directNetwork"`
 	} `json:"nsxt"`
 	Logging struct {
 		Enabled         bool   `json:"enabled,omitempty"`

vcd/resource_vcd_standalone_vm_with_vm_sizing_test.go

@@ -414,8 +414,8 @@ resource "vcd_vm" "{{.VMName2}}" {
 
   power_on = true
 
-  description   = "test empty VM2"
-  name          = "{{.VMName2}}"
+  description = "test empty VM2"
+  name        = "{{.VMName2}}"
   
   os_type                        = "sles11_64Guest"
   hardware_version               = "vmx-13"

vcd/resource_vcd_vapp_network.go

@@ -1,5 +1,6 @@
 package vcd
 
+//lint:file-ignore SA1019 ignore GetOkExists deprecated function error
 import (
 	"bytes"
 	"context"
@@ -20,7 +21,7 @@ func resourceVcdVappNetwork() *schema.Resource {
 		CreateContext: resourceVappNetworkCreate,
 		ReadContext:   resourceVappNetworkRead,
 		UpdateContext: resourceVappNetworkUpdate,
-		DeleteContext: resourceVappNetworkDelete,
+		DeleteContext: resourceVappAndVappOrgNetworkDelete,
 		Importer: &schema.ResourceImporter{
 			StateContext: resourceVcdVappNetworkImport,
 		},
@@ -157,6 +158,12 @@ func resourceVcdVappNetwork() *schema.Resource {
 				},
 				Set: resourceVcdNetworkStaticIpPoolHash,
 			},
+			"reboot_vapp_on_removal": {
+				Type:        schema.TypeBool,
+				Optional:    true,
+				Default:     false,
+				Description: "Specifies whether the vApp should be rebooted when the vApp network is removed. Default is false.",
+			},
 		},
 	}
 }
@@ -363,6 +370,12 @@ func genericVappNetworkRead(d *schema.ResourceData, meta interface{}, origin str
 }
 
 func resourceVappNetworkUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+	// reboot_vapp_on_removal does not have any effect on update therefore skipping update if only
+	// this field was modified
+	if !d.HasChangeExcept("reboot_vapp_on_removal") {
+		return resourceVappNetworkRead(ctx, d, meta)
+	}
+
 	vcdClient := meta.(*VCDClient)
 	vcdClient.lockParentVapp(d)
 	defer vcdClient.unLockParentVapp(d)
@@ -418,11 +431,31 @@ func resourceVappNetworkUpdate(ctx context.Context, d *schema.ResourceData, meta
 	return resourceVappNetworkRead(ctx, d, meta)
 }
 
-func resourceVappNetworkDelete(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
+// resourceVappAndVappOrgNetworkDelete deletes a vApp network.
+// Starting with VCD 10.4.1, a vApp network cannot be deleted from a powered on vApp. To avoid
+// inconvenience (especially in `terraform destroy` scenarios), there is a
+// 'reboot_vapp_on_removal=true' flag.
+// When the flag is set and vApp status is not POWERED_OFF, the vApp is powered off, the network is
+// deleted and the vApp powered on (if it was not powered off before).
+//
+// Note. This function is used for both resource `vcd_vapp_network` and `vcd_vapp_org_network`
+// because deletion of these networks is the same operation and maintaining two functions might
+// become inconsistent. They can be split again, if required.
+func resourceVappAndVappOrgNetworkDelete(_ context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	vcdClient := meta.(*VCDClient)
 	vcdClient.lockParentVapp(d)
 	defer vcdClient.unLockParentVapp(d)
 
+	// Should vApp be power cycled before deleting network? ('reboot_vapp_on_removal=true')
+	rebootVAppOnRemoval := false
+	vappStatusBeforeOperation := ""
+	vAppRebootEnabled, isSet := d.GetOkExists("reboot_vapp_on_removal")
+	if isSet && vAppRebootEnabled.(bool) {
+		util.Logger.Printf("[TRACE] reboot_vapp_on_removal=true is enabled with parent vApp '%s",
+			d.Get("vapp_name").(string))
+		rebootVAppOnRemoval = true
+	}
+
 	_, vdc, err := vcdClient.GetOrgAndVdcFromResource(d)
 	if err != nil {
 		return diag.Errorf(errorRetrievingOrgAndVdc, err)
@@ -433,11 +466,60 @@ func resourceVappNetworkDelete(_ context.Context, d *schema.ResourceData, meta i
 		return diag.Errorf("error finding vApp: %s", err)
 	}
 
+	if rebootVAppOnRemoval {
+		vappStatusBeforeOperation, err = vapp.GetStatus()
+		if err != nil {
+			return diag.Errorf("error getting vApp '%s' status before vApp network removal: %s",
+				vapp.VApp.Name, err)
+		}
+
+		util.Logger.Printf("[TRACE] reboot_vapp_on_removal=true, vApp '%s' status before network removal is '%s'",
+			vapp.VApp.Name, vappStatusBeforeOperation)
+		if vappStatusBeforeOperation != "POWERED_OFF" {
+			util.Logger.Println("[TRACE] reboot_vapp_on_removal=true, powering off vApp")
+			task, err := vapp.Undeploy() // UI Button "Power Off" calls undeploy API endpoint
+			if err != nil {
+				return diag.Errorf("error Powering Off: %s", err)
+			}
+			err = task.WaitTaskCompletion()
+			if err != nil {
+				return diag.Errorf("error completing vApp Power Off task: %s", err)
+			}
+		}
+	}
+
+	// Remove vApp network
 	_, err = vapp.RemoveNetwork(d.Id())
 	if err != nil {
+		// VCD 10.4.1+ API returns error when removing network from powered off vApp
+		// If this error occurs - we add a hint to use 'reboot_vapp_on_removal' flag
+		if strings.Contains(err.Error(), "Stop the vApp and try again") {
+			return diag.Errorf("error removing vApp network: %s \n\n"+
+				"Parent vApp '%s' must be powered off in VCD 10.4.1+ to remove a vApp network. \n"+
+				"You can use 'reboot_vapp_on_removal=true' flag to power off vApp before removing network.",
+				vapp.VApp.Name, err)
+		}
+
 		return diag.Errorf("error removing vApp network: %s", err)
 	}
 
+	// If vApp was not powered off before and 'reboot_vapp_on_removal' flag was used - power it on
+	// again. The reason we check for vappStatusBeforeOperation != "POWERED_ON" is that a vApp could
+	// have had different states than "POWERED_OFF" and "POWERED_ON" (e.g. "PARTIALLY_POWERED_OFF"),
+	// but we cannot restore exactly such state. So we restore "POWERED_ON" state.
+	if rebootVAppOnRemoval && vappStatusBeforeOperation != "POWERED_OFF" {
+		util.Logger.Printf("[TRACE] reboot_vapp_on_removal=true, restoring vApp '%s' power state, was '%s'",
+			vapp.VApp.Name, vappStatusBeforeOperation)
+		task, err := vapp.PowerOn()
+		if err != nil {
+			return diag.Errorf("error powering on vApp: %s", err)
+		}
+		err = task.WaitTaskCompletion()
+		if err != nil {
+			return diag.Errorf("error completing vApp power on task: %s", err)
+		}
+	}
+
 	d.SetId("")
 
 	return nil