Fix storage profile update and catalog deletion

* Fix Issue vmware#648 adding a storage profile requires the vdc to be replaced * Fix Issue vmware#696 Catalog deletion failure returns as success Signed-off-by: Giuseppe Maxia <[email protected]>
dataclouder · Jul 28, 2021 · 5f77e78 · 5f77e78
1 parent 5579bf5
commit 5f77e78
Show file tree

Hide file tree

Showing 3 changed files with 156 additions and 50 deletions.
diff --git a/go.mod b/go.mod
@@ -9,3 +9,5 @@ require (
 	github.com/kr/pretty v0.2.1
 	github.com/vmware/go-vcloud-director/v2 v2.12.1
 )
+
+replace github.com/vmware/go-vcloud-director/v2 => github.com/dataclouder/go-vcloud-director/v2 v2.12.2-0.20210727073529-c168b34fc56f
diff --git a/go.sum b/go.sum
@@ -70,6 +70,8 @@ github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMn
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
+github.com/dataclouder/go-vcloud-director/v2 v2.12.2-0.20210727073529-c168b34fc56f h1:BLuzaW4iJXMEA4aw1m+g88KCAqM3EAxHXYtDInYP27c=
+github.com/dataclouder/go-vcloud-director/v2 v2.12.2-0.20210727073529-c168b34fc56f/go.mod h1:poaOwg7CoXO4m9Pv4TVhMpNF1wQQwKzxpdGYTfjzajs=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -288,8 +290,6 @@ github.com/ulikunitz/xz v0.5.8/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oW
 github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
 github.com/vmihailenco/msgpack v4.0.4+incompatible h1:dSLoQfGFAo3F6OoNhwUmLwVgaUXK79GlxNBwueZn0xI=
 github.com/vmihailenco/msgpack v4.0.4+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk=
-github.com/vmware/go-vcloud-director/v2 v2.12.1 h1:PO+dVQyZlECX87oVWgLb4/S2v5hiqKREVGsAwTCJHbQ=
-github.com/vmware/go-vcloud-director/v2 v2.12.1/go.mod h1:poaOwg7CoXO4m9Pv4TVhMpNF1wQQwKzxpdGYTfjzajs=
 github.com/xanzy/ssh-agent v0.2.1 h1:TCbipTQL2JiiCprBWx9frJ2eJlCYT00NmctrHxVAr70=
 github.com/xanzy/ssh-agent v0.2.1/go.mod h1:mLlQY/MoOhWBj+gOGMQkOeiEvkx+8pJSI+0Bx9h2kr4=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

diff --git a/vcd/resource_vcd_org_vdc.go b/vcd/resource_vcd_org_vdc.go
@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 	"github.com/vmware/go-vcloud-director/v2/govcd"
 	"github.com/vmware/go-vcloud-director/v2/types/v56"
+	"github.com/vmware/go-vcloud-director/v2/util"
 )
 
 func resourceVcdOrgVdc() *schema.Resource {
@@ -113,7 +114,7 @@ func resourceVcdOrgVdc() *schema.Resource {
 			"storage_profile": &schema.Schema{
 				Type:        schema.TypeSet,
 				Required:    true,
-				ForceNew:    true,
+				ForceNew:    false,
 				MinItems:    1,
 				Description: "Storage profiles supported by this VDC.",
 				Elem: &schema.Resource{
@@ -456,7 +457,7 @@ func getComputeStorageProfiles(vcdClient *VCDClient, profile *types.VdcStoragePr
 	root := make([]map[string]interface{}, 0)
 
 	for _, vdcStorageProfile := range profile.VdcStorageProfile {
-		vdcStorageProfileDetails, err := govcd.GetStorageProfileByHref(vcdClient.VCDClient, vdcStorageProfile.HREF)
+		vdcStorageProfileDetails, err := vcdClient.GetStorageProfileByHref(vdcStorageProfile.HREF)
 		if err != nil {
 			return nil, err
 		}
@@ -571,41 +572,162 @@ func resourceVcdVdcUpdate(d *schema.ResourceData, meta interface{}) error {
 
 	if d.HasChange("storage_profile") {
 		vdcStorageProfilesConfigurations := d.Get("storage_profile").(*schema.Set)
-		for _, storageConfigurationValues := range vdcStorageProfilesConfigurations.List() {
-			storageConfiguration := storageConfigurationValues.(map[string]interface{})
-			var matchedStorageProfile types.Reference
-			for _, vdcStorageProfile := range adminVdc.AdminVdc.VdcStorageProfiles.VdcStorageProfile {
-				if storageConfiguration["name"].(string) == vdcStorageProfile.Name {
-					matchedStorageProfile = *vdcStorageProfile
+		err = updateStorageProfiles(vdcStorageProfilesConfigurations, vcdClient, adminVdc, changedAdminVdc)
+		if err != nil {
+			return err
+		}
+	}
+	log.Printf("[TRACE] VDC update completed: %s", adminVdc.AdminVdc.Name)
+	return resourceVcdVdcRead(d, meta)
+}
+
+func updateStorageProfileDetails(vcdClient *VCDClient, adminVdc *govcd.AdminVdc, storageProfile *types.Reference, storageConfiguration map[string]interface{}) error {
+	util.Logger.Printf("updating storage profile %#v", storageProfile)
+	uuid, err := govcd.GetUuidFromHref(storageProfile.HREF, true)
+	if err != nil {
+		return fmt.Errorf("error parsing VDC storage profile ID : %s", err)
+	}
+	vdcStorageProfileDetails, err := vcdClient.GetStorageProfileByHref(storageProfile.HREF)
+	if err != nil {
+		return fmt.Errorf("error getting VDC storage profile: %s", err)
+	}
+	_, err = adminVdc.UpdateStorageProfile(uuid, &types.AdminVdcStorageProfile{
+		Name:         storageConfiguration["name"].(string),
+		IopsSettings: nil,
+		Units:        "MB", // only this value is supported
+		Limit:        int64(storageConfiguration["limit"].(int)),
+		Default:      storageConfiguration["default"].(bool),
+		Enabled:      takeBoolPointer(storageConfiguration["enabled"].(bool)),
+		ProviderVdcStorageProfile: &types.Reference{
+			HREF: vdcStorageProfileDetails.ProviderVdcStorageProfile.HREF,
+		},
+	})
+	if err != nil {
+		return fmt.Errorf("error updating VDC storage profile: %s", err)
+	}
+	return nil
+}
+
+func updateStorageProfiles(set *schema.Set, client *VCDClient, adminVdc, changedAdminVdc *govcd.AdminVdc) error {
+
+	type storageProfileCombo struct {
+		configuration map[string]interface{}
+		reference     *types.Reference
+	}
+	var (
+		existingStorageProfiles []storageProfileCombo
+		newStorageProfiles      []storageProfileCombo
+		removeStorageProfiles   []storageProfileCombo
+		defaultSp               = make(map[string]bool)
+	)
+
+	// 1. find existing storage profiles: SP are both in the definition and in the VDC
+	for _, storageConfigurationValues := range set.List() {
+		storageConfiguration := storageConfigurationValues.(map[string]interface{})
+		for _, vdcStorageProfile := range adminVdc.AdminVdc.VdcStorageProfiles.VdcStorageProfile {
+			if storageConfiguration["name"].(string) == vdcStorageProfile.Name {
+				if storageConfiguration["default"].(bool) {
+					defaultSp[storageConfiguration["name"].(string)] = true
 				}
+				existingStorageProfiles = append(existingStorageProfiles, storageProfileCombo{
+					configuration: storageConfiguration,
+					reference:     vdcStorageProfile,
+				})
 			}
-			uuid, err := govcd.GetUuidFromHref(matchedStorageProfile.HREF, true)
-			if err != nil {
-				return fmt.Errorf("error parsing VDC storage profile ID : %s", err)
+		}
+	}
+
+	// 2. find new storage profiles: SP are in the definition, but not in the VDC
+	for _, storageConfigurationValues := range set.List() {
+		storageConfiguration := storageConfigurationValues.(map[string]interface{})
+		found := false
+		for _, vdcStorageProfile := range adminVdc.AdminVdc.VdcStorageProfiles.VdcStorageProfile {
+			if storageConfiguration["name"].(string) == vdcStorageProfile.Name {
+				found = true
 			}
-			vdcStorageProfileDetails, err := govcd.GetStorageProfileByHref(vcdClient.VCDClient, matchedStorageProfile.HREF)
-			if err != nil {
-				return fmt.Errorf("error getting VDC storage profile: %s", err)
+		}
+		if !found {
+			if storageConfiguration["default"].(bool) {
+				defaultSp[storageConfiguration["name"].(string)] = true
 			}
-			_, err = changedAdminVdc.UpdateStorageProfile(uuid, &types.AdminVdcStorageProfile{
-				Name:         storageConfiguration["name"].(string),
-				IopsSettings: nil,
-				Units:        "MB", // only this value is supported
-				Limit:        int64(storageConfiguration["limit"].(int)),
-				Default:      storageConfiguration["default"].(bool),
-				Enabled:      takeBoolPointer(storageConfiguration["enabled"].(bool)),
-				ProviderVdcStorageProfile: &types.Reference{
-					HREF: vdcStorageProfileDetails.ProviderVdcStorageProfile.HREF,
-				},
+			newStorageProfiles = append(newStorageProfiles, storageProfileCombo{
+				configuration: storageConfiguration,
+				reference:     nil,
 			})
-			if err != nil {
-				return fmt.Errorf("error updating VDC storage profile: %s", err)
+		}
+	}
+
+	// 3 find removed storage profiles: SP are in the VDC, but not in the definition
+	for _, vdcStorageProfile := range adminVdc.AdminVdc.VdcStorageProfiles.VdcStorageProfile {
+		found := false
+		for _, storageConfigurationValues := range set.List() {
+			storageConfiguration := storageConfigurationValues.(map[string]interface{})
+			if storageConfiguration["name"].(string) == vdcStorageProfile.Name {
+				found = true
 			}
 		}
+		if !found {
+			_, isDefault := defaultSp[vdcStorageProfile.Name]
+			if isDefault {
+				delete(defaultSp, vdcStorageProfile.Name)
+			}
+			removeStorageProfiles = append(removeStorageProfiles, storageProfileCombo{
+				configuration: nil,
+				reference:     vdcStorageProfile,
+			})
+		}
 	}
 
-	log.Printf("[TRACE] VDC update completed: %s", adminVdc.AdminVdc.Name)
-	return resourceVcdVdcRead(d, meta)
+	// 4. Check that there is one and only one default element
+	if len(defaultSp) == 0 {
+		return fmt.Errorf("no default storage profile left after update")
+	}
+	if len(defaultSp) > 1 {
+		defaultItems := ""
+		for d := range defaultSp {
+			defaultItems += " " + d
+		}
+		return fmt.Errorf("more than one default storage profile defined [%s]", defaultItems)
+	}
+
+	// 5. Update existing storage profiles
+	for _, spCombo := range existingStorageProfiles {
+		err := updateStorageProfileDetails(client, adminVdc, spCombo.reference, spCombo.configuration)
+		if err != nil {
+			return err
+		}
+	}
+
+	// 6. Add new storage profiles
+	for _, spCombo := range newStorageProfiles {
+		storageProfile, err := client.QueryProviderVdcStorageProfileByName(spCombo.configuration["name"].(string), adminVdc.AdminVdc.ProviderVdcReference.HREF)
+		if err != nil {
+			return err
+		}
+		err = adminVdc.AddStorageProfileWait(&types.VdcStorageProfileConfiguration{
+			Enabled: spCombo.configuration["enabled"].(bool),
+			Units:   "MB",
+			Limit:   int64(spCombo.configuration["limit"].(int)),
+			Default: spCombo.configuration["default"].(bool),
+			ProviderVdcStorageProfile: &types.Reference{
+				HREF: storageProfile.HREF,
+				Name: storageProfile.Name,
+			},
+		}, "")
+		if err != nil {
+			return fmt.Errorf("error adding new storage profile: %s", err)
+		}
+	}
+
+	// 7. Delete unwanted storage profiles
+	for _, spCombo := range removeStorageProfiles {
+		err := adminVdc.RemoveStorageProfileWait(spCombo.reference.Name)
+		if err != nil {
+			return fmt.Errorf("error removing storage profile %s: %s", spCombo.reference.Name, err)
+		}
+	}
+
+	return nil
 }
 
 // Deletes a VDC, optionally removing all objects in it as well
@@ -1051,7 +1173,7 @@ func getVcdVdcInput(d *schema.ResourceData, vcdClient *VCDClient) (*types.VdcCon
 	for _, storageConfigurationValues := range vdcStorageProfilesConfigurations.List() {
 		storageConfiguration := storageConfigurationValues.(map[string]interface{})
 
-		href, err := getStorageProfileHREF(vcdClient, storageConfiguration["name"].(string))
+		sp, err := vcdClient.QueryProviderVdcStorageProfileByName(storageConfiguration["name"].(string), providerVdcResults[0].HREF)
 		if err != nil {
 			return &types.VdcConfiguration{}, err
 		}
@@ -1062,7 +1184,7 @@ func getVcdVdcInput(d *schema.ResourceData, vcdClient *VCDClient) (*types.VdcCon
 			Default: storageConfiguration["default"].(bool),
 			Enabled: storageConfiguration["enabled"].(bool),
 			ProviderVdcStorageProfile: &types.Reference{
-				HREF: href,
+				HREF: sp.HREF,
 			},
 		}
 		vdcStorageProfiles = append(vdcStorageProfiles, vdcStorageProfile)
@@ -1148,24 +1270,6 @@ func getVcdVdcInput(d *schema.ResourceData, vcdClient *VCDClient) (*types.VdcCon
 	return params, nil
 }
 
-func getStorageProfileHREF(vcdClient *VCDClient, name string) (string, error) {
-	storageProfileRecords, err := govcd.QueryProviderVdcStorageProfileByName(vcdClient.VCDClient, name)
-	if err != nil {
-		return "", err
-	}
-	if len(storageProfileRecords) == 0 {
-		return "", fmt.Errorf("no provider VDC storage profile found with name %s", name)
-	}
-
-	// additional filtering done cause name like `*` returns more value and have to be manually selected
-	for _, profileRecord := range storageProfileRecords {
-		if profileRecord.Name == name {
-			return profileRecord.HREF, nil
-		}
-	}
-	return "", fmt.Errorf("no provider VDC storage profile found with name %s", name)
-}
-
 // resourceVcdOrgVdcImport is responsible for importing the resource.
 // The following steps happen as part of import
 // 1. The user supplies `terraform import _resource_name_ _the_id_string_` command