Apply suggestion

Signed-off-by: abarreiro <abarreiro@vmware.com>

govcd/org.go

@@ -291,14 +291,25 @@ func (org *Org) GetCatalogByNameOrId(identifier string, refresh bool) (*Catalog,
 // On success, returns a pointer to the VDC structure and a nil error
 // On failure, returns a nil pointer and an error
 func (org *Org) GetVDCByHref(vdcHref string) (*Vdc, error) {
-	vdc := NewVdc(org.client)
-	_, err := org.client.ExecuteRequest(vdcHref, http.MethodGet,
-		"", "error retrieving VDC: %s", nil, vdc.Vdc)
+	vdc, err := getVDCByHref(org.client, vdcHref)
 	if err != nil {
 		return nil, err
 	}
 	// The request was successful
-	vdc.parent = org
+	result := NewVdc(org.client)
+	result.Vdc = vdc
+	result.parent = org
+	return result, nil
+}
+
+// getVDCByHref gets a plain VDC object from its HREF.
+func getVDCByHref(client *Client, vdcHref string) (*types.Vdc, error) {
+	vdc := &types.Vdc{}
+	_, err := client.ExecuteRequest(vdcHref, http.MethodGet,
+		"", "error retrieving VDC: %s", nil, vdc)
+	if err != nil {
+		return nil, err
+	}
 	return vdc, nil
 }
 

govcd/vdc_template.go

@@ -25,7 +25,8 @@ func (vcdClient *VCDClient) CreateVdcTemplate(input types.VMWVdcTemplate) (*VdcT
 	return genericVdcTemplateRequest(&vcdClient.Client, input, &href, http.MethodPost)
 }
 
-// Update updates an existing VDC Template with the given settings
+// Update updates an existing VDC Template with the given settings.
+// Returns the updated VDC Template.
 func (vdcTemplate *VdcTemplate) Update(input types.VMWVdcTemplate) (*VdcTemplate, error) {
 	href := vdcTemplate.client.VCDHREF
 	href.Path += fmt.Sprintf("/admin/extension/vdcTemplate/%s", extractUuid(vdcTemplate.VdcTemplate.ID))
@@ -122,7 +123,6 @@ func (vcdClient *VCDClient) GetVdcTemplateByName(name string) (*VdcTemplate, err
 		}
 		return vcdClient.GetVdcTemplateById(results.Results.OrgVdcTemplateRecord[0].HREF)
 	}
-
 }
 
 // Delete deletes the receiver VDC Template
@@ -181,13 +181,15 @@ func (vdcTemplate *VdcTemplate) GetAccess() (*types.ControlAccessParams, error)
 	return result, nil
 }
 
-// Instantiate creates a new VDC from the template and returns its ID if the operation finishes successfully.
-func (vdcTemplate *VdcTemplate) Instantiate(vdcName, description, organizationId string) (string, error) {
+// InstantiateVdcAsync creates a new VDC by instantiating the receiver VDC Template. This method finishes immediately after
+// requesting the VDC instance, by returning the Task associated to the VDC instantiation process. If there's any error
+// during the process, returns a nil Task and an error.
+func (vdcTemplate *VdcTemplate) InstantiateVdcAsync(vdcName, description, organizationId string) (*Task, error) {
 	if vdcName == "" {
-		return "", fmt.Errorf("the VDC name is required to instantiate VDC Template '%s'", vdcTemplate.VdcTemplate.Name)
+		return nil, fmt.Errorf("the VDC name is required to instantiate VDC Template '%s'", vdcTemplate.VdcTemplate.Name)
 	}
 	if organizationId == "" {
-		return "", fmt.Errorf("the Organization ID is required to instantiate VDC Template '%s'", vdcTemplate.VdcTemplate.Name)
+		return nil, fmt.Errorf("the Organization ID is required to instantiate VDC Template '%s'", vdcTemplate.VdcTemplate.Name)
 	}
 
 	payload := &types.InstantiateVdcTemplateParams{
@@ -206,14 +208,31 @@ func (vdcTemplate *VdcTemplate) Instantiate(vdcName, description, organizationId
 	href.Path += fmt.Sprintf("/org/%s/action/instantiate", extractUuid(organizationId))
 	task, err := vdcTemplate.client.ExecuteTaskRequest(href.String(), http.MethodPost, types.MimeVdcTemplateInstantiate, "error instantiating the VDC Template: %s", payload)
 	if err != nil {
-		return "", err
+		return nil, err
+	}
+	return &task, nil
+}
+
+// InstantiateVdc creates a new VDC by instantiating the receiver VDC Template. This method waits for the associated Task
+// to complete and returns the instantiated VDC. If there's any error during the process or in the Task, returns a nil VDC and an error.
+func (vdcTemplate *VdcTemplate) InstantiateVdc(vdcName, description, organizationId string) (*Vdc, error) {
+	task, err := vdcTemplate.InstantiateVdcAsync(vdcName, description, organizationId)
+	if err != nil {
+		return nil, err
 	}
 	err = task.WaitTaskCompletion()
 	if err != nil {
-		return "", err
+		return nil, fmt.Errorf("failed instantiating the VDC Template: %s", err)
 	}
-	if task.Task.Owner == nil {
-		return "", fmt.Errorf("the VDC was instantiated but could not retrieve its ID from the finished task")
+	if task.Task.Owner == nil || task.Task.Owner.HREF == "" {
+		return nil, fmt.Errorf("the VDC was instantiated but could not retrieve its ID from the finished task")
+	}
+	vdc, err := getVDCByHref(vdcTemplate.client, task.Task.Owner.HREF)
+	if err != nil {
+		return nil, fmt.Errorf("could not retrieve the VDC from Task's HREF '%s': %s", task.Task.Owner.HREF, err)
 	}
-	return task.Task.Owner.ID, nil
+	return &Vdc{
+		Vdc:    vdc,
+		client: vdcTemplate.client,
+	}, nil
 }

govcd/vdc_template_test.go

@@ -230,9 +230,9 @@ func (vcd *TestVCD) Test_VdcTemplateInstantiate(check *C) {
 	}
 
 	// Pre-requisites: We need information such as Provider VDC and External networks (Provider Gateways)
-	org, err := vcd.client.GetAdminOrgByName(vcd.config.VCD.Org)
+	adminOrg, err := vcd.client.GetAdminOrgByName(vcd.config.VCD.Org)
 	check.Assert(err, IsNil)
-	check.Assert(org, NotNil)
+	check.Assert(adminOrg, NotNil)
 
 	providerVdc, err := vcd.client.GetProviderVdcByName(vcd.config.VCD.NsxtProviderVdc.Name)
 	check.Assert(err, IsNil)
@@ -316,23 +316,18 @@ func (vcd *TestVCD) Test_VdcTemplateInstantiate(check *C) {
 		check.Assert(err, IsNil)
 	}()
 
-	err = template.SetAccess([]string{org.AdminOrg.ID})
+	err = template.SetAccess([]string{adminOrg.AdminOrg.ID})
 	check.Assert(err, IsNil)
 
 	// Instantiate the VDC Template as System administrator
-	vdcId, err := template.Instantiate(check.TestName(), check.TestName(), org.AdminOrg.ID)
-	check.Assert(err, IsNil)
-	check.Assert(vdcId, Not(Equals), "")
-
-	vdc, err := org.GetVDCById(vdcId, true)
+	vdc, err := template.InstantiateVdc(check.TestName(), check.TestName(), adminOrg.AdminOrg.ID)
 	check.Assert(err, IsNil)
 	check.Assert(vdc, NotNil)
 	defer func() {
+		// Delete the instantiated VDC even on test errors
 		err = vdc.DeleteWait(true, true)
 		check.Assert(err, IsNil)
 	}()
-
-	check.Assert(vdc.Vdc.ID, Equals, vdcId)
 	check.Assert(vdc.Vdc.Name, Equals, check.TestName())
 	check.Assert(vdc.Vdc.Description, Equals, check.TestName())
 
@@ -350,22 +345,17 @@ func (vcd *TestVCD) Test_VdcTemplateInstantiate(check *C) {
 		check.Assert(err, IsNil)
 		check.Assert(templateAsTenant, NotNil)
 
-		vdcId, err := templateAsTenant.Instantiate(check.TestName()+"2", check.TestName()+"2", org.AdminOrg.ID)
-		check.Assert(err, IsNil)
-		check.Assert(vdcId, Not(Equals), "")
-
-		// Check that it really exists and delete it afterward. We do this as the System admin, as we don't need
-		// the tenant user anymore.
-		vdc, err := org.GetVDCById(vdcId, true)
+		vdc2, err := templateAsTenant.InstantiateVdc(check.TestName()+"2", check.TestName()+"2", adminOrg.AdminOrg.ID)
 		check.Assert(err, IsNil)
-		check.Assert(vdc, NotNil)
+		check.Assert(vdc2, NotNil)
 		defer func() {
-			err = vdc.DeleteWait(true, true)
+			// Also delete the second instantiated VDC even on test errors. We need to retrieve it as Admin for that.
+			adminVdc2, err := adminOrg.GetVDCById(vdc2.Vdc.ID, true)
+			check.Assert(err, IsNil)
+			err = adminVdc2.DeleteWait(true, true)
 			check.Assert(err, IsNil)
 		}()
-
-		check.Assert(vdc.Vdc.ID, Equals, vdcId)
-		check.Assert(vdc.Vdc.Name, Equals, check.TestName()+"2")
-		check.Assert(vdc.Vdc.Description, Equals, check.TestName()+"2")
+		check.Assert(vdc2.Vdc.Name, Equals, check.TestName()+"2")
+		check.Assert(vdc2.Vdc.Description, Equals, check.TestName()+"2")
 	}
 }