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

Proposed by Raphaël Badin
Status: Rejected
Rejected by: Raphaël Badin
Proposed branch: lp://staging/~rvb/gwacl/fix-destroydeploy
Merge into: lp://staging/gwacl
Diff against target: 84 lines (+17/-21)
3 files modified
example/management/run.go (+4/-18)
management.go (+8/-3)
management_test.go (+5/-0)
To merge this branch: bzr merge lp://staging/~rvb/gwacl/fix-destroydeploy
Reviewer Review Type Date Requested Status
Gavin Panella Approve
Review via email: mp+173034@code.staging.launchpad.net

Commit message

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

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

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
164. By Raphaël Badin

Remove the storage account.

165. By Raphaël Badin

Remove the storage account.

Revision history for this message
Raphaël Badin (rvb) wrote :

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: