Fix issue-1170 (vmware#1274)

* Fix Issue vmware#1170 * Add Changelog item --------- Signed-off-by: Giuseppe Maxia <[email protected]>
dataclouder · Jun 13, 2024 · 8b89760 · 8b89760
1 parent cba468a
commit 8b89760
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 36 deletions.
diff --git a/.changes/v3.13.0/1274-bug-fixes.md b/.changes/v3.13.0/1274-bug-fixes.md
@@ -0,0 +1 @@
+* Fix Issue [1170](https://github.com/vmware/terraform-provider-vcd/issues/1170) where an imported VM complains about missing or altered fields and Terraform tries to re-create the resource [GH-1274]
diff --git a/vcd/resource_vcd_nsxt_standalone_vm_test.go b/vcd/resource_vcd_nsxt_standalone_vm_test.go
@@ -126,7 +126,7 @@ func TestAccVcdNsxtStandaloneVmTemplate(t *testing.T) {
 				// "network_dhcp_wait_seconds" is a user setting and cannot be imported
 				ImportStateVerifyIgnore: []string{"template_name", "catalog_name",
 					"accept_all_eulas", "power_on", "computer_name", "prevent_update_power_off", "network.1.ip",
-					"network_dhcp_wait_seconds", "consolidate_disks_on_create"},
+					"network_dhcp_wait_seconds", "consolidate_disks_on_create", "imported", "vapp_template_id"},
 			},
 			// This step ensures that VM and disk are removed, but networks are left
 			{

diff --git a/vcd/resource_vcd_standalone_vm_test.go b/vcd/resource_vcd_standalone_vm_test.go
@@ -88,7 +88,7 @@ func TestAccVcdStandaloneVmTemplate(t *testing.T) {
 				// These fields can't be retrieved from user data
 				ImportStateVerifyIgnore: []string{"template_name", "catalog_name",
 					"accept_all_eulas", "power_on", "computer_name", "prevent_update_power_off",
-					"consolidate_disks_on_create"},
+					"consolidate_disks_on_create", "imported", "vapp_template_id"},
 			},
 		},
 	})

diff --git a/vcd/resource_vcd_vapp_vm.go b/vcd/resource_vcd_vapp_vm.go
@@ -129,19 +129,21 @@ func vmSchemaFunc(vmType typeOfVm) map[string]*schema.Schema {
 			Description: "The name of VDC to use, optional if defined at provider level",
 		},
 		"template_name": {
-			Type:          schema.TypeString,
-			Optional:      true,
-			ForceNew:      true,
-			Deprecated:    "Use `vapp_template_id` instead",
-			Description:   "The name of the vApp Template to use",
-			ConflictsWith: []string{"vapp_template_id"},
+			Type:             schema.TypeString,
+			Optional:         true,
+			ForceNew:         true,
+			Deprecated:       "Use `vapp_template_id` instead",
+			Description:      "The name of the vApp Template to use",
+			ConflictsWith:    []string{"vapp_template_id"},
+			DiffSuppressFunc: suppressTextAfterImport(),
 		},
 		"vapp_template_id": {
-			Type:          schema.TypeString,
-			Optional:      true,
-			ForceNew:      true,
-			Description:   "The URN of the vApp Template to use",
-			ConflictsWith: []string{"template_name", "catalog_name"},
+			Type:             schema.TypeString,
+			Optional:         true,
+			ForceNew:         true,
+			Description:      "The URN of the vApp Template to use",
+			ConflictsWith:    []string{"template_name", "catalog_name"},
+			DiffSuppressFunc: suppressTextAfterImport(),
 		},
 		"vm_name_in_template": {
 			Type:        schema.TypeString,
@@ -150,11 +152,12 @@ func vmSchemaFunc(vmType typeOfVm) map[string]*schema.Schema {
 			Description: "The name of the VM in vApp Template to use. In cases when vApp template has more than one VM",
 		},
 		"catalog_name": {
-			Type:          schema.TypeString,
-			Optional:      true,
-			Deprecated:    "You should use `vapp_template_id` or `boot_image_id` without the need of a catalog name",
-			Description:   "The catalog name in which to find the given vApp Template or media for boot_image",
-			ConflictsWith: []string{"vapp_template_id", "boot_image_id"},
+			Type:             schema.TypeString,
+			Optional:         true,
+			Deprecated:       "You should use `vapp_template_id` or `boot_image_id` without the need of a catalog name",
+			Description:      "The catalog name in which to find the given vApp Template or media for boot_image",
+			ConflictsWith:    []string{"vapp_template_id", "boot_image_id"},
+			DiffSuppressFunc: suppressTextAfterImport(),
 		},
 		"copy_from_vm_id": {
 			Type:          schema.TypeString,
@@ -254,16 +257,18 @@ func vmSchemaFunc(vmType typeOfVm) map[string]*schema.Schema {
 			Description: "VM Hyper Reference",
 		},
 		"accept_all_eulas": {
-			Type:        schema.TypeBool,
-			Optional:    true,
-			Default:     true,
-			Description: "Automatically accept EULA if OVA has it",
+			Type:             schema.TypeBool,
+			Optional:         true,
+			Default:          true,
+			Description:      "Automatically accept EULA if OVA has it",
+			DiffSuppressFunc: suppressFieldAfterImport("accept_all_eulas"),
 		},
 		"power_on": {
-			Type:        schema.TypeBool,
-			Optional:    true,
-			Default:     true,
-			Description: "A boolean value stating if this VM should be powered on",
+			Type:             schema.TypeBool,
+			Optional:         true,
+			Default:          true,
+			Description:      "A boolean value stating if this VM should be powered on",
+			DiffSuppressFunc: suppressFieldAfterImport("power_on"),
 		},
 		"storage_profile": {
 			Type:        schema.TypeString,
@@ -402,11 +407,12 @@ func vmSchemaFunc(vmType typeOfVm) map[string]*schema.Schema {
 			Set:      resourceVcdVmIndependentDiskHash,
 		},
 		"consolidate_disks_on_create": {
-			Type:        schema.TypeBool,
-			Optional:    true,
-			Default:     false,
-			ForceNew:    true,
-			Description: "Consolidates disks during creation and allows to change disk size using 'override_template_disk' in fast provisioned VDCs",
+			Type:             schema.TypeBool,
+			Optional:         true,
+			Default:          false,
+			ForceNew:         true,
+			Description:      "Consolidates disks during creation and allows to change disk size using 'override_template_disk' in fast provisioned VDCs",
+			DiffSuppressFunc: suppressFieldAfterImport("consolidate_disks_on_create"),
 		},
 		"override_template_disk": {
 			Type:        schema.TypeSet,
@@ -721,10 +727,11 @@ func vmSchemaFunc(vmType typeOfVm) map[string]*schema.Schema {
 			Description: "True if the virtual machine supports addition of memory while powered on.",
 		},
 		"prevent_update_power_off": {
-			Type:        schema.TypeBool,
-			Optional:    true,
-			Default:     false,
-			Description: "True if the update of resource should fail when virtual machine power off needed.",
+			Type:             schema.TypeBool,
+			Optional:         true,
+			Default:          false,
+			Description:      "True if the update of resource should fail when virtual machine power off needed.",
+			DiffSuppressFunc: suppressFieldAfterImport("prevent_update_power_off"),
 		},
 		"sizing_policy_id": {
 			Type:        schema.TypeString,
@@ -760,6 +767,11 @@ func vmSchemaFunc(vmType typeOfVm) map[string]*schema.Schema {
 			Computed:    true,
 			Description: "A map that contains metadata that is automatically added by VCD (10.5.1+) and provides details on the origin of the VM",
 		},
+		"imported": {
+			Type:        schema.TypeBool,
+			Computed:    true,
+			Description: "Tells whether this resource has been imported",
+		},
 	}
 }
 
@@ -836,6 +848,7 @@ func resourceVcdVAppVmCreate(_ context.Context, d *schema.ResourceData, meta int
 	if len(diags) != 0 {
 		return append(diags, genericVcdVmRead(d, meta, "resource")...)
 	}
+	dSet(d, "imported", false)
 	return genericVcdVmRead(d, meta, "resource")
 }
 
@@ -2616,6 +2629,15 @@ func resourceVcdVappVmImport(_ context.Context, d *schema.ResourceData, meta int
 	dSet(d, "org", orgName)
 	dSet(d, "vdc", vdcName)
 	dSet(d, "vapp_name", vappName)
+	// Set defaults value for fields that can't be recovered from VCD
+	dSet(d, "vapp_template_id", defaultImportedValue)
+	dSet(d, "catalog_name", defaultImportedValue)
+	dSet(d, "template_name", defaultImportedValue)
+	dSet(d, "accept_all_eulas", true)
+	dSet(d, "prevent_update_power_off", d.Get("prevent_update_power_off"))
+	dSet(d, "power_on", d.Get("power_on"))
+	dSet(d, "consolidate_disks_on_create", d.Get("consolidate_disks_on_create"))
+	dSet(d, "imported", true)
 	d.SetId(vm.VM.ID)
 	return []*schema.ResourceData{d}, nil
 }
diff --git a/vcd/resource_vcd_vapp_vm_test.go b/vcd/resource_vcd_vapp_vm_test.go
@@ -87,7 +87,7 @@ func TestAccVcdVAppVm_Basic(t *testing.T) {
 				// These fields can't be retrieved from user data
 				ImportStateVerifyIgnore: []string{"template_name", "catalog_name",
 					"accept_all_eulas", "power_on", "computer_name", "prevent_update_power_off",
-					"consolidate_disks_on_create"},
+					"consolidate_disks_on_create", "imported", "vapp_template_id"},
 			},
 		},
 	})

diff --git a/vcd/suppress_funcs.go b/vcd/suppress_funcs.go
@@ -7,6 +7,8 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
+const defaultImportedValue = "DEFAULT_IMPORTED_VALUE"
+
 // suppressWordToEmptyString is a DiffSuppressFunc which ignore the change from word to empty string "".
 // This is useful when API returns some default value but it is not set (and not sent via API) in config.
 func suppressWordToEmptyString(word string) schema.SchemaDiffSuppressFunc {
@@ -34,6 +36,28 @@ func suppressNetworkUpgradedInterface() schema.SchemaDiffSuppressFunc {
 	}
 }
 
+// suppressTextAfterImport will ignore the field value if it was set during import
+func suppressTextAfterImport() schema.SchemaDiffSuppressFunc {
+	return func(k string, old string, new string, d *schema.ResourceData) bool {
+		if old == defaultImportedValue || new == defaultImportedValue {
+			return true
+		}
+		return false
+	}
+}
+
+// suppressFieldAfterImport will ignore the field value if its value has changed
+// (Useful for resources that have values set to default on import)
+// Note: don't use this function unless the resource has a Boolean field named "imported" that is set during import
+func suppressFieldAfterImport(ignoreField string) schema.SchemaDiffSuppressFunc {
+	return func(k string, old string, new string, d *schema.ResourceData) bool {
+		if old != new && k == ignoreField && d.Get("imported").(bool) {
+			return true
+		}
+		return false
+	}
+}
+
 // falseBoolSuppress suppresses change if value is set to false or is empty
 func falseBoolSuppress() schema.SchemaDiffSuppressFunc {
 	return func(k string, old string, new string, d *schema.ResourceData) bool {

diff --git a/website/docs/r/vapp_vm.html.markdown b/website/docs/r/vapp_vm.html.markdown
@@ -570,6 +570,7 @@ example for usage details.
 * `inherited_metadata` - (*v3.11+*; *VCD 10.5.1+*) A map that contains read-only metadata that is automatically added by VCD (10.5.1+) and provides
   details on the origin of the VM (e.g. `vm.origin.id`, `vm.origin.name`, `vm.origin.type`).
 * `extra_config` - (*v3.13.+*) The VM extra configuration. See [Extra Configuration](#extra-configuration) for more detail.
+* `imported` - (*v3.13.+*) A true/false value telling whether the resource was imported.
 
 <a id="disk"></a>
 ## Disk
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		* Fix Issue [1170](https://github.com/vmware/terraform-provider-vcd/issues/1170) where an imported VM complains about missing or altered fields and Terraform tries to re-create the resource [GH-1274]