Vdc bug fixes for disabling storage profile and deserializing type.Task on vdc.update

.changes/v2.15.0/433-bug-fixes.md

@@ -0,0 +1,2 @@
+* Fixes Issue #431 "Wrong order in Task structure" [GH-433]
+* Fixes Issue where VDC creation with storage profile `enabled=false` wasn't working. `VdcStorageProfile.enabled` and `VdcStorageProfileConfiguration.enabled` changed to pointers [GH-433]

govcd/adminvdc.go

@@ -483,7 +483,7 @@ func (vdc *AdminVdc) RemoveStorageProfile(storageProfileName string) (Task, erro
 	if err != nil {
 		return Task{}, fmt.Errorf("cannot retrieve VDC storage profile '%s' details: %s", storageProfileName, err)
 	}
-	if vdcStorageProfileDetails.Enabled {
+	if vdcStorageProfileDetails.Enabled != nil && *vdcStorageProfileDetails.Enabled {
 		_, err = vdc.UpdateStorageProfile(extractUuid(storageProfile.HREF), &types.AdminVdcStorageProfile{
 			Name:    vdcStorageProfileDetails.Name,
 			Units:   vdcStorageProfileDetails.Units,

govcd/adminvdc_nsxt_test.go

@@ -69,7 +69,7 @@ func (vcd *TestVCD) Test_CreateNsxtOrgVdc(check *C) {
 				},
 			},
 			VdcStorageProfile: []*types.VdcStorageProfileConfiguration{&types.VdcStorageProfileConfiguration{
-				Enabled: true,
+				Enabled: takeBoolPointer(true),
 				Units:   "MB",
 				Limit:   1024,
 				Default: true,

govcd/adminvdc_test.go

@@ -41,11 +41,19 @@ func (vcd *TestVCD) Test_CreateOrgVdcWithFlex(check *C) {
 
 	providerVdcHref := getVdcProviderVdcHref(vcd, check)
 
-	storageProfile, err := vcd.client.QueryProviderVdcStorageProfileByName(vcd.config.VCD.ProviderVdc.StorageProfile, providerVdcHref)
+	storageProfile, err := vcd.client.QueryProviderVdcStorageProfileByName(vcd.config.VCD.StorageProfile.SP1, providerVdcHref)
 	check.Assert(err, IsNil)
-	providerVdcStorageProfileHref := storageProfile.HREF
+	firstStorageProfileHref := storageProfile.HREF
 	networkPoolHref := getVdcNetworkPoolHref(vcd, check)
 
+	secondStorageProfileHref := ""
+	// Make test more robust and tests additionally disabled storage profile
+	if vcd.config.VCD.StorageProfile.SP2 != "" {
+		storageProfile, err := vcd.client.QueryProviderVdcStorageProfileByName(vcd.config.VCD.StorageProfile.SP2, providerVdcHref)
+		check.Assert(err, IsNil)
+		secondStorageProfileHref = storageProfile.HREF
+	}
+
 	allocationModels := []string{"AllocationVApp", "AllocationPool", "ReservationPool", "Flex"}
 	trueValue := true
 	for i, allocationModel := range allocationModels {
@@ -67,12 +75,12 @@ func (vcd *TestVCD) Test_CreateOrgVdcWithFlex(check *C) {
 				},
 			},
 			VdcStorageProfile: []*types.VdcStorageProfileConfiguration{&types.VdcStorageProfileConfiguration{
-				Enabled: true,
+				Enabled: takeBoolPointer(true),
 				Units:   "MB",
 				Limit:   1024,
 				Default: true,
 				ProviderVdcStorageProfile: &types.Reference{
-					HREF: providerVdcStorageProfileHref,
+					HREF: firstStorageProfileHref,
 				},
 			},
 			},
@@ -92,6 +100,18 @@ func (vcd *TestVCD) Test_CreateOrgVdcWithFlex(check *C) {
 			vdcConfiguration.IncludeMemoryOverhead = &trueValue
 		}
 
+		if secondStorageProfileHref != "" {
+			vdcConfiguration.VdcStorageProfile = append(vdcConfiguration.VdcStorageProfile, &types.VdcStorageProfileConfiguration{
+				Enabled: takeBoolPointer(false),
+				Units:   "MB",
+				Limit:   1024,
+				Default: false,
+				ProviderVdcStorageProfile: &types.Reference{
+					HREF: secondStorageProfileHref,
+				},
+			})
+		}
+
 		vdc, _ := adminOrg.GetVDCByName(vdcConfiguration.Name, false)
 		if vdc != nil {
 			err = vdc.DeleteWait(true, true)
@@ -108,8 +128,8 @@ func (vcd *TestVCD) Test_CreateOrgVdcWithFlex(check *C) {
 		vdcConfiguration.ComputeCapacity[0].Memory.Units = "MB"
 
 		vdc, err = adminOrg.CreateOrgVdc(vdcConfiguration)
-		check.Assert(vdc, NotNil)
 		check.Assert(err, IsNil)
+		check.Assert(vdc, NotNil)
 
 		AddToCleanupList(vdcConfiguration.Name, "vdc", vcd.org.Org.Name, "Test_CreateVdcWithFlex")
 
@@ -119,6 +139,15 @@ func (vcd *TestVCD) Test_CreateOrgVdcWithFlex(check *C) {
 		check.Assert(vdc.Vdc.Name, Equals, vdcConfiguration.Name)
 		check.Assert(vdc.Vdc.IsEnabled, Equals, vdcConfiguration.IsEnabled)
 		check.Assert(vdc.Vdc.AllocationModel, Equals, vdcConfiguration.AllocationModel)
+		if secondStorageProfileHref != "" {
+			check.Assert(vdc.Vdc.VdcStorageProfiles, NotNil)
+			check.Assert(vdc.Vdc.VdcStorageProfiles.VdcStorageProfile, NotNil)
+			check.Assert(vdc.Vdc.VdcStorageProfiles.VdcStorageProfile[1], NotNil)
+		}
+
+		vdcStorageProfileDetails, err := adminOrg.client.GetStorageProfileByHref(vdc.Vdc.VdcStorageProfiles.VdcStorageProfile[1].HREF)
+		check.Assert(err, IsNil)
+		check.Assert(*vdcStorageProfileDetails.Enabled, Equals, false)
 
 		err = vdc.DeleteWait(true, true)
 		check.Assert(err, IsNil)
@@ -147,6 +176,41 @@ func (vcd *TestVCD) Test_UpdateVdcFlex(check *C) {
 	check.Assert(adminVdc.AdminVdc.IsEnabled, Equals, vdcConfiguration.IsEnabled)
 	check.Assert(adminVdc.AdminVdc.AllocationModel, Equals, vdcConfiguration.AllocationModel)
 
+	// test part to reproduce https://github.com/vmware/go-vcloud-director/issues/431
+	// this part manages to create task error which later on VDC update fails if type properties order is bad
+	providerVdcHref := getVdcProviderVdcHref(vcd, check)
+	pvdcStorageProfile, err := vcd.client.QueryProviderVdcStorageProfileByName(vcd.config.VCD.StorageProfile.SP2, providerVdcHref)
+	check.Assert(err, IsNil)
+
+	err = adminVdc.AddStorageProfileWait(&types.VdcStorageProfileConfiguration{
+		Enabled:                   takeBoolPointer(true),
+		Default:                   false,
+		Units:                     "MB",
+		ProviderVdcStorageProfile: &types.Reference{HREF: pvdcStorageProfile.HREF},
+	},
+		"")
+	check.Assert(err, IsNil)
+
+	vdc, err := adminOrg.GetVDCByName(vdcConfiguration.Name, true)
+	check.Assert(err, IsNil)
+
+	vappName := check.TestName()
+	vmName := check.TestName()
+	vapp, err := makeEmptyVapp(vdc, vappName, "")
+	check.Assert(err, IsNil)
+	_, err = makeEmptyVm(vapp, vmName)
+	check.Assert(err, IsNil)
+	AddToCleanupList(vappName, "vapp", "", vappName)
+
+	err = adminVdc.SetDefaultStorageProfile(vcd.config.VCD.StorageProfile.SP2)
+	check.Assert(err, IsNil)
+	err = adminVdc.RemoveStorageProfileWait(vcd.config.VCD.StorageProfile.SP1)
+	// fails with error in task which stays referenced in VDC as `history` element
+	check.Assert(err, NotNil)
+	err = adminVdc.Refresh()
+	check.Assert(err, IsNil)
+	// end
+
 	updateDescription := "updateDescription"
 	computeCapacity := []*types.ComputeCapacity{
 		&types.ComputeCapacity{

govcd/common_test.go

@@ -764,7 +764,7 @@ func spawnTestVdc(vcd *TestVCD, check *C, adminOrgName string) *Vdc {
 			},
 		},
 		VdcStorageProfile: []*types.VdcStorageProfileConfiguration{&types.VdcStorageProfileConfiguration{
-			Enabled: true,
+			Enabled: takeBoolPointer(true),
 			Units:   "MB",
 			Limit:   1024,
 			Default: true,

govcd/org_test.go

@@ -282,7 +282,7 @@ func (vcd *TestVCD) Test_CreateVdc(check *C) {
 				},
 			},
 			VdcStorageProfile: []*types.VdcStorageProfileConfiguration{&types.VdcStorageProfileConfiguration{
-				Enabled: true,
+				Enabled: takeBoolPointer(true),
 				Units:   "MB",
 				Limit:   1024,
 				Default: true,
@@ -651,7 +651,7 @@ func setupVdc(vcd *TestVCD, check *C, allocationModel string) (AdminOrg, *types.
 			},
 		},
 		VdcStorageProfile: []*types.VdcStorageProfileConfiguration{&types.VdcStorageProfileConfiguration{
-			Enabled: true,
+			Enabled: takeBoolPointer(true),
 			Units:   "MB",
 			Limit:   1024,
 			Default: true,
@@ -822,7 +822,7 @@ func (vcd *TestVCD) Test_AddRemoveVdcStorageProfiles(check *C) {
 
 	// Add another storage profile
 	err = adminVdc.AddStorageProfileWait(&types.VdcStorageProfileConfiguration{
-		Enabled: true,
+		Enabled: takeBoolPointer(true),
 		Units:   "MB",
 		Limit:   1024,
 		Default: false,
@@ -846,7 +846,7 @@ func (vcd *TestVCD) Test_AddRemoveVdcStorageProfiles(check *C) {
 
 	// Add the second storage profile again
 	err = adminVdc.AddStorageProfileWait(&types.VdcStorageProfileConfiguration{
-		Enabled: true,
+		Enabled: takeBoolPointer(true),
 		Units:   "MB",
 		Limit:   1024,
 		Default: false,

govcd/vdc_group_test.go

@@ -128,7 +128,7 @@ func createNewVdc(vcd *TestVCD, check *C, vdcName string) *Vdc {
 			},
 		},
 		VdcStorageProfile: []*types.VdcStorageProfileConfiguration{&types.VdcStorageProfileConfiguration{
-			Enabled: true,
+			Enabled: takeBoolPointer(true),
 			Units:   "MB",
 			Limit:   1024,
 			Default: true,

types/v56/types.go

@@ -470,7 +470,7 @@ type AdminVdc struct {
 // Since: 5.1
 // https://code.vmware.com/apis/220/vcloud#/doc/doc/types/VdcStorageProfileParamsType.html
 type VdcStorageProfileConfiguration struct {
-	Enabled                   bool       `xml:"Enabled,omitempty"`
+	Enabled                   *bool      `xml:"Enabled,omitempty"`
 	Units                     string     `xml:"Units"`
 	Limit                     int64      `xml:"Limit"`
 	Default                   bool       `xml:"Default"`
@@ -485,7 +485,7 @@ type VdcStorageProfileConfiguration struct {
 type VdcStorageProfile struct {
 	Xmlns                     string                         `xml:"xmlns,attr"`
 	Name                      string                         `xml:"name,attr"`
-	Enabled                   bool                           `xml:"Enabled,omitempty"`
+	Enabled                   *bool                          `xml:"Enabled,omitempty"`
 	Units                     string                         `xml:"Units"`
 	Limit                     int64                          `xml:"Limit"`
 	Default                   bool                           `xml:"Default"`
@@ -576,15 +576,16 @@ type Task struct {
 	EndTime          string           `xml:"endTime,attr,omitempty"`          // The date and time that processing of the task was completed. May not be present if the task is still being executed.
 	ExpiryTime       string           `xml:"expiryTime,attr,omitempty"`       // The date and time at which the task resource will be destroyed and no longer available for retrieval. May not be present if the task has not been executed or is still being executed.
 	CancelRequested  bool             `xml:"cancelRequested,attr,omitempty"`  // Whether user has requested this processing to be canceled.
+	Link             *Link            `xml:"Link,omitempty"`                  // A reference to an entity or operation associated with this object.
 	Description      string           `xml:"Description,omitempty"`           // Optional description.
-	Details          string           `xml:"Details,omitempty"`               // Detailed message about the task. Also contained by the Owner entity when task status is preRunning.
+	Tasks            *TasksInProgress `xml:"Tasks,omitempty"`                 // A list of queued, running, or recently completed tasks associated with this entity.
+	Owner            *Reference       `xml:"Owner,omitempty"`                 // Reference to the owner of the task. This is typically the object that the task is creating or updating.
 	Error            *Error           `xml:"Error,omitempty"`                 // Represents error information from a failed task.
-	Link             *Link            `xml:"Link,omitempty"`                  // A reference to an entity or operation associated with this object.
+	User             *Reference       `xml:"User,omitempty"`                  // The user who started the task.
 	Organization     *Reference       `xml:"Organization,omitempty"`          // The organization to which the User belongs.
-	Owner            *Reference       `xml:"Owner,omitempty"`                 // Reference to the owner of the task. This is typically the object that the task is creating or updating.
 	Progress         int              `xml:"Progress,omitempty"`              // Read-only indicator of task progress as an approximate percentage between 0 and 100. Not available for all tasks.
-	Tasks            *TasksInProgress `xml:"Tasks,omitempty"`                 // A list of queued, running, or recently completed tasks associated with this entity.
-	User             *Reference       `xml:"User,omitempty"`                  // The user who started the task.
+	Details          string           `xml:"Details,omitempty"`               // Detailed message about the task. Also contained by the Owner entity when task status is preRunning.
+
 	//
 	// TODO: add the following fields
 	// Params      anyType        The parameters with which this task was started.