Fix catalog delete (#392)

* Fix Issue #390: catalog.Delete() ignores task

Signed-off-by: Giuseppe Maxia <gmaxia@vmware.com>

Author: dataclouder

.changes/v2.13.0/392-bug-fixes.md

@@ -0,0 +1,2 @@
+* Fix Issue #390: `catalog.Delete()` ignores returned task and responds immediately which could have caused failures [GH-392]
+

govcd/admincatalog.go

@@ -29,7 +29,7 @@ func NewAdminCatalog(client *Client) *AdminCatalog {
 	}
 }
 
-// Deletes the Catalog, returning an error if the vCD call fails.
+// Delete deletes the Catalog, returning an error if the vCD call fails.
 // Link to API call: https://code.vmware.com/apis/220/vcloud#/doc/doc/operations/DELETE-Catalog.html
 func (adminCatalog *AdminCatalog) Delete(force, recursive bool) error {
 	catalog := NewCatalog(adminCatalog.client)

govcd/catalog.go

@@ -42,7 +42,7 @@ func NewCatalog(client *Client) *Catalog {
 	}
 }
 
-// Deletes the Catalog, returning an error if the vCD call fails.
+// Delete deletes the Catalog, returning an error if the vCD call fails.
 // Link to API call: https://code.vmware.com/apis/220/vcloud#/doc/doc/operations/DELETE-Catalog.html
 func (catalog *Catalog) Delete(force, recursive bool) error {
 
@@ -52,7 +52,7 @@ func (catalog *Catalog) Delete(force, recursive bool) error {
 		return err
 	}
 	if catalogID == "" {
-		return fmt.Errorf("empty ID returned for catalog ID %s", catalog.Catalog.ID)
+		return fmt.Errorf("empty ID returned for catalog %s", catalog.Catalog.Name)
 	}
 	adminCatalogHREF.Path += "/admin/catalog/" + catalogID
 
@@ -61,13 +61,18 @@ func (catalog *Catalog) Delete(force, recursive bool) error {
 		"recursive": strconv.FormatBool(recursive),
 	}, http.MethodDelete, adminCatalogHREF, nil)
 
-	_, err = checkResp(catalog.client.Http.Do(req))
-
+	resp, err := checkResp(catalog.client.Http.Do(req))
 	if err != nil {
-		return fmt.Errorf("error deleting Catalog %s: %s", catalog.Catalog.ID, err)
+		return fmt.Errorf("error deleting Catalog %s: %s", catalog.Catalog.Name, err)
 	}
-
-	return nil
+	task := NewTask(catalog.client)
+	if err = decodeBody(types.BodyTypeXML, resp, task.Task); err != nil {
+		return fmt.Errorf("error decoding task response: %s", err)
+	}
+	if task.Task.Status == "error" {
+		return fmt.Errorf(combinedTaskErrorMessage(task.Task, fmt.Errorf("catalog %s not properly destroyed", catalog.Catalog.Name)))
+	}
+	return task.WaitTaskCompletion()
 }
 
 // Envelope is a ovf description root element. File contains information for vmdk files.

govcd/catalog_test.go

@@ -131,6 +131,13 @@ func (vcd *TestVCD) Test_DeleteCatalog(check *C) {
 	// If something fails after this point, the entity will be removed
 	AddToCleanupList(TestDeleteCatalog, "catalog", vcd.config.VCD.Org, "Test_DeleteCatalog")
 	check.Assert(adminCatalog.AdminCatalog.Name, Equals, TestDeleteCatalog)
+
+	checkUploadOvf(vcd, check, vcd.config.OVA.OvaPath, TestDeleteCatalog, TestUploadOvf)
+	err = adminCatalog.Delete(false, false)
+	check.Assert(err, NotNil)
+	// Catalog is not empty. An attempt to delete without recursion will fail
+	check.Assert(strings.Contains(err.Error(), "You must remove"), Equals, true)
+
 	err = adminCatalog.Delete(true, true)
 	check.Assert(err, IsNil)
 	doesCatalogExist(check, org)
@@ -317,14 +324,14 @@ func countFolders() int {
 }
 
 func checkUploadOvf(vcd *TestVCD, check *C, ovaFileName, catalogName, itemName string) {
-	catalog, org := findCatalog(vcd, check, vcd.config.VCD.Catalog.Name)
+	catalog, org := findCatalog(vcd, check, catalogName)
 
 	uploadTask, err := catalog.UploadOvf(ovaFileName, itemName, "upload from test", 1024)
 	check.Assert(err, IsNil)
 	err = uploadTask.WaitTaskCompletion()
 	check.Assert(err, IsNil)
 
-	AddToCleanupList(itemName, "catalogItem", vcd.org.Org.Name+"|"+vcd.config.VCD.Catalog.Name, "Test_UploadOvf")
+	AddToCleanupList(itemName, "catalogItem", vcd.org.Org.Name+"|"+catalogName, "checkUploadOvf")
 
 	catalog, err = org.GetCatalogByName(catalogName, false)
 	check.Assert(err, IsNil)
@@ -772,9 +779,6 @@ func cleanupCatalogOrgVdc(check *C, sharedCatalog Catalog, vdc *Vdc, vcd *TestVC
 	err := sharedCatalog.Delete(true, true)
 	check.Assert(err, IsNil)
 
-	// There are cases where it just takes a a few seconds after catalog deletion when one can delete VDC
-	time.Sleep(2 * time.Second)
-
 	err = vdc.DeleteWait(true, true)
 	check.Assert(err, IsNil)
 

govcd/catalogitem.go

@@ -37,7 +37,7 @@ func (catalogItem *CatalogItem) GetVAppTemplate() (VAppTemplate, error) {
 
 }
 
-// Deletes the Catalog Item, returning an error if the vCD call fails.
+// Delete deletes the Catalog Item, returning an error if the vCD call fails.
 // Link to API call: https://code.vmware.com/apis/220/vcloud#/doc/doc/operations/DELETE-CatalogItem.html
 func (catalogItem *CatalogItem) Delete() error {
 	util.Logger.Printf("[TRACE] Deleting catalog item: %#v", catalogItem.CatalogItem)