Merge lp:~rvb/gwacl/fix-destroydeploy into lp:gwacl

[Raphaël Badin]

Do not check the error we get from stopping the VM inside DestroyDeployment.

[Gavin Panella (allenap)]

Looks good.

[1]

-        if err != nil && !IsNotFoundError(err) {
-            return err
-        }
+        // Even when the operation itself works (i.e. the VM is stopped) we
+        // seem to alway get a 500 error back.
+        // We chose to ignore the error.
+        // If the operation errors for real and the VM is still running then
+        // the next operation (deleting the disks) will fail.
+        // if err != nil && !IsNotFoundError(err) {
+        //    return err
+        // }

I suggest only ignoring 500 errors. A new IsInternalServerError()
helper would be good here.

[2]

+/*
+// This test is currently disabled as we chose to ignore the errors returned
+// when shutting down the VM.  See the comment in DestroyDeployment's code
+// for more information.
 func (suite *suiteDestroyDeployment) TestFailsShuttingDownRole(c *C) {
     var responses []DispatcherResponse
     // Prepare.
@@ -377,6 +381,7 @@
     c.Check(err, ErrorMatches, "POST request failed [(]500: Internal Server Error[)]")
     c.Check(record, HasLen, 2)
 }
+*/

You can use c.ExpectFailure(reason string) here; see
http://godoc.org/launchpad.net/gocheck#C.ExpectFailure.

review: Approve

[lp:~rvb/gwacl/fix-destroydeploy]

164. By Raphaël Badin on 2013-07-04

Remove the storage account.

165. By Raphaël Badin on 2013-07-04

Remove the storage account.

[Raphaël Badin (rvb)]

Actually, I found a better way, see https://code.launchpad.net/~rvb/gwacl/fix-destroydeploy2/+merge/173095

Unmerged revisions

=== modified file 'example/management/run.go'
--- example/management/run.go	2013-07-02 13:53:21 +0000
+++ example/management/run.go	2013-07-04 17:27:26 +0000
@@ -172,17 +172,10 @@
     fmt.Println("Done adding VM deployment\n")
 
     defer func() {
-        fmt.Println("Deleting VM disk...")
-        err = api.DeleteDisk(diskName)
-        checkError(err)
-        fmt.Println("Done deleting VM disk\n")
-    }()
-
-    defer func() {
-        fmt.Println("Deleting deployment...")
-        err = api.DeleteDeployment(hostServiceName, deployment.Name)
-        checkError(err)
-        fmt.Println("Done deleting deployment\n")
+        fmt.Printf("Destroying deployment %s...\n", machineName)
+        requestDestroyDeployment := &gwacl.DestroyDeploymentRequest{ServiceName: hostServiceName, DeploymentName: machineName}
+        api.DestroyDeployment(requestDestroyDeployment)
+        fmt.Println("Done destroying deployment\n")
     }()
 
     fmt.Println("Starting VM...")
@@ -190,13 +183,6 @@
     checkError(err)
     fmt.Println("Done starting VM\n")
 
-    defer func() {
-        fmt.Println("Stopping VM...")
-        err = api.ShutdownRole(&gwacl.ShutdownRoleRequest{hostServiceName, deployment.Name, role.RoleName})
-        checkError(err)
-        fmt.Println("Done stopping VM\n")
-    }()
-
     fmt.Println("Listing VM...")
     instances, err := api.ListInstances(&gwacl.ListInstancesRequest{hostServiceName})
     checkError(err)
 
=== modified file 'management.go'
--- management.go	2013-07-04 09:53:30 +0000
+++ management.go	2013-07-04 17:27:26 +0000
@@ -121,9 +121,14 @@
             DeploymentName: request.DeploymentName,
             RoleName:       roleInstance.RoleName,
         })
-        if err != nil && !IsNotFoundError(err) {
-            return err
-        }
+        // Even when the operation itself works (i.e. the VM is stopped) we
+        // seem to alway get a 500 error back.
+        // We chose to ignore the error.  
+        // If the operation errors for real and the VM is still running then
+        // the next operation (deleting the disks) will fail.
+        // if err != nil && !IsNotFoundError(err) {
+        //    return err
+        // }
     }
     // 2. Delete VM disks.
     diskNameMap := make(map[string]bool)
 
=== modified file 'management_test.go'
--- management_test.go	2013-07-04 10:53:40 +0000
+++ management_test.go	2013-07-04 17:27:26 +0000
@@ -359,6 +359,10 @@
     c.Check(record, HasLen, 1)
 }
 
+/*
+// This test is currently disabled as we chose to ignore the errors returned
+// when shutting down the VM.  See the comment in DestroyDeployment's code
+// for more information.
 func (suite *suiteDestroyDeployment) TestFailsShuttingDownRole(c *C) {
     var responses []DispatcherResponse
     // Prepare.
@@ -377,6 +381,7 @@
     c.Check(err, ErrorMatches, "POST request failed [(]500: Internal Server Error[)]")
     c.Check(record, HasLen, 2)
 }
+*/
 
 func (suite *suiteDestroyDeployment) TestFailsDeletingDisk(c *C) {
     var responses []DispatcherResponse