Merge lp:~thumper/juju-core/fix-local-destroy into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Fix intermittent destroy error.

In order to manage the intermittent error that is returned
by os.RemoveAll if the directory is removed by another process
while it is trying, we double check before returning an error.

https://codereview.appspot.com/87500044/

Fix intermittent destroy error.

In order to manage the intermittent error that is returned
by os.RemoveAll if the directory is removed by another process
while it is trying, we double check before returning an error.

https://codereview.appspot.com/87500044/

[Tim Penhey (thumper)]

Reviewers: mp+215601_code.launchpad.net,

Message:
Please take a look.

Description:
Fix intermittent destroy error.

In order to manage the intermittent error that is returned
by os.RemoveAll if the directory is removed by another process
while it is trying, we double check before returning an error.

https://code.launchpad.net/~thumper/juju-core/fix-local-destroy/+merge/215601

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/87500044/

Affected files (+10, -0 lines):
   A [revision details]
   M provider/local/environ.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140413170514-ii8w6q2fht3eiizi
+New revision: <email address hidden>

Index: provider/local/environ.go
=== modified file 'provider/local/environ.go'
--- provider/local/environ.go 2014-04-09 23:13:50 +0000
+++ provider/local/environ.go 2014-04-14 06:47:17 +0000
@@ -453,6 +453,14 @@

   // Finally, remove the data-dir.
   if err := os.RemoveAll(env.config.rootDir()); err != nil
&& !os.IsNotExist(err) {
+ // Before we return the error, just check to see if the directory is
+ // there. There is a race condition with the agent with the removing
+ // of the directory, and due to a bug
+ // (https://code.google.com/p/go/issues/detail?id=7776) the
+ // os.IsNotExist error isn't always returned.
+ if _, statErr := os.Stat(env.config.rootDir()); os.IsNotExist(statErr) {
+ return nil
+ }
    return err
   }
   return nil

[Andrew Wilkins (axwalk)]

On 2014/04/14 06:52:04, thumper wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/87500044/

=== modified file 'provider/local/environ.go'
--- provider/local/environ.go	2014-04-09 23:13:50 +0000
+++ provider/local/environ.go	2014-04-14 06:50:03 +0000
@@ -453,6 +453,14 @@
 
 	// Finally, remove the data-dir.
 	if err := os.RemoveAll(env.config.rootDir()); err != nil && !os.IsNotExist(err) {
+		// Before we return the error, just check to see if the directory is
+		// there. There is a race condition with the agent with the removing
+		// of the directory, and due to a bug
+		// (https://code.google.com/p/go/issues/detail?id=7776) the
+		// os.IsNotExist error isn't always returned.
+		if _, statErr := os.Stat(env.config.rootDir()); os.IsNotExist(statErr) {
+			return nil
+		}
 		return err
 	}
 	return nil