From 7206c0772b0a2fec66553970109671e5b2f01020 Mon Sep 17 00:00:00 2001
From: Anna Khmelnitsky <akhmelnitsky@vmware.com>
Date: Tue, 10 Mar 2020 15:15:10 -0700
Subject: [PATCH] Optimize vm tag resources

* Make tags required in policy resource, since its the only
attribute that can be set
* Avoid update backend call if there is no change in corresponding
configuration
* Avoid update backend call in delete function if there were no
tags to provider config
---
 nsxt/resource_nsxt_policy_vm_tags.go      |  2 +-
 nsxt/resource_nsxt_policy_vm_tags_test.go | 17 +++-------
 nsxt/resource_nsxt_vm_tags.go             | 39 +++++++++++++++--------
 nsxt/utils.go                             | 13 +++++---
 4 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/nsxt/resource_nsxt_policy_vm_tags.go b/nsxt/resource_nsxt_policy_vm_tags.go
index 6dab0e24f..7d0c8d584 100644
--- a/nsxt/resource_nsxt_policy_vm_tags.go
+++ b/nsxt/resource_nsxt_policy_vm_tags.go
@@ -35,7 +35,7 @@ func resourceNsxtPolicyVMTags() *schema.Resource {
 				Description: "Instance id",
 				Required:    true,
 			},
-			"tag": getTagsSchema(),
+			"tag": getRequiredTagsSchema(),
 		},
 	}
 }
diff --git a/nsxt/resource_nsxt_policy_vm_tags_test.go b/nsxt/resource_nsxt_policy_vm_tags_test.go
index afad94a3e..01f3b2c2c 100644
--- a/nsxt/resource_nsxt_policy_vm_tags_test.go
+++ b/nsxt/resource_nsxt_policy_vm_tags_test.go
@@ -38,12 +38,7 @@ func TestAccResourceNsxtPolicyVMTags_basic(t *testing.T) {
 				),
 			},
 			{
-				Config: testAccNSXPolicyVMTagsRemoveTemplate(vmID),
-				Check: resource.ComposeTestCheckFunc(
-					testAccNSXPolicyVMTagsCheckExists(testResourceName),
-					resource.TestCheckResourceAttr(testResourceName, "tag.#", "0"),
-					resource.TestCheckResourceAttr(testResourceName, "instance_id", vmID),
-				),
+				Config: testAccNsxtPolicyEmptyTemplate(),
 			},
 		},
 	})
@@ -69,6 +64,9 @@ func TestAccResourceNsxtPolicyVMTags_import_basic(t *testing.T) {
 				ImportStateVerify:       true,
 				ImportStateVerifyIgnore: []string{"instance_id"},
 			},
+			{
+				Config: testAccNsxtPolicyEmptyTemplate(),
+			},
 		},
 	})
 }
@@ -145,10 +143,3 @@ resource "nsxt_policy_vm_tags" "test" {
   }
 }`, instanceID)
 }
-
-func testAccNSXPolicyVMTagsRemoveTemplate(instanceID string) string {
-	return fmt.Sprintf(`
-resource "nsxt_policy_vm_tags" "test" {
-  instance_id = "%s"
-}`, instanceID)
-}
diff --git a/nsxt/resource_nsxt_vm_tags.go b/nsxt/resource_nsxt_vm_tags.go
index e6a5e0e59..0072f8147 100644
--- a/nsxt/resource_nsxt_vm_tags.go
+++ b/nsxt/resource_nsxt_vm_tags.go
@@ -220,15 +220,19 @@ func resourceNsxtVMTagsCreate(d *schema.ResourceData, m interface{}) error {
 	}
 
 	tags := getTagsFromSchema(d)
-	err = updateTags(nsxClient, vm.ExternalId, tags)
-	if err != nil {
-		return err
+	if len(tags) > 0 || d.HasChange("tag") {
+		err = updateTags(nsxClient, vm.ExternalId, tags)
+		if err != nil {
+			return err
+		}
 	}
 
 	portTags := getCustomizedTagsFromSchema(d, "logical_port_tag")
-	err = updatePortTags(nsxClient, vm.ExternalId, portTags)
-	if err != nil {
-		return err
+	if len(portTags) > 0 || d.HasChange("logical_port_tag") {
+		err = updatePortTags(nsxClient, vm.ExternalId, portTags)
+		if err != nil {
+			return err
+		}
 	}
 
 	d.SetId(vm.ExternalId)
@@ -288,16 +292,23 @@ func resourceNsxtVMTagsDelete(d *schema.ResourceData, m interface{}) error {
 		return fmt.Errorf("Error during VM retrieval: %v", err)
 	}
 
-	tags := make([]common.Tag, 0)
-	err = updateTags(nsxClient, vm.ExternalId, tags)
-	err2 := updatePortTags(nsxClient, vm.ExternalId, tags)
-
-	if err != nil {
-		return err
+	noTags := make([]common.Tag, 0)
+	vmTags := getTagsFromSchema(d)
+	if len(vmTags) > 0 {
+		// Update tags only if they were configured by the provider
+		err = updateTags(nsxClient, vm.ExternalId, noTags)
+		if err != nil {
+			return err
+		}
 	}
 
-	if err2 != nil {
-		return err2
+	portTags := getCustomizedTagsFromSchema(d, "logical_port_tag")
+	if len(portTags) > 0 {
+		// Update port tags only if they were configured by the provider
+		err := updatePortTags(nsxClient, vm.ExternalId, noTags)
+		if err != nil {
+			return err
+		}
 	}
 
 	return nil
diff --git a/nsxt/utils.go b/nsxt/utils.go
index 637dbceb2..de9ad4838 100644
--- a/nsxt/utils.go
+++ b/nsxt/utils.go
@@ -87,11 +87,12 @@ func getRevisionSchema() *schema.Schema {
 }
 
 // utilities to define & handle tags
-func getTagsSchemaInternal(forceNew bool) *schema.Schema {
+func getTagsSchemaInternal(required bool, forceNew bool) *schema.Schema {
 	return &schema.Schema{
 		Type:        schema.TypeSet,
 		Description: "Set of opaque identifiers meaningful to the user",
-		Optional:    true,
+		Optional:    !required,
+		Required:    required,
 		ForceNew:    forceNew,
 		Elem: &schema.Resource{
 			Schema: map[string]*schema.Schema{
@@ -112,11 +113,15 @@ func getTagsSchemaInternal(forceNew bool) *schema.Schema {
 
 // utilities to define & handle tags
 func getTagsSchema() *schema.Schema {
-	return getTagsSchemaInternal(false)
+	return getTagsSchemaInternal(false, false)
+}
+
+func getRequiredTagsSchema() *schema.Schema {
+	return getTagsSchemaInternal(true, false)
 }
 
 func getTagsSchemaForceNew() *schema.Schema {
-	return getTagsSchemaInternal(true)
+	return getTagsSchemaInternal(false, true)
 }
 
 func getCustomizedTagsFromSchema(d *schema.ResourceData, schemaName string) []common.Tag {