Merge lp:~axwalk/juju-core/lp1292275-localdestroy-stopremovemongod into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

provider/local: remove mongo service on Destroy

The local provider's Destroy method is modified
to stop and remove the mongodb service we create.

If bootstrap fails, it currently leaves behind
the mongo service. Destroy normally removes the
service by killing jujud with SIGABRT, which
triggers it to clean up; this only works if
bootstrap passes, and the machine agent starts
up successfully.

Since the mongo service may be stopped/removed
by jujud itself, we do not check the error of
stopping/removing the service.

Fixes lp:1292275

https://codereview.appspot.com/78770043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+212117_code.launchpad.net,

Message:
Please take a look.

Description:
provider/local: remove mongo service on Destroy

The local provider's Destroy method is modified
to stop and remove the mongodb service we create.

If bootstrap fails, it currently leaves behind
the mongo service. Destroy normally removes the
service by killing jujud with SIGABRT, which
triggers it to clean up; this only works if
bootstrap passes, and the machine agent starts
up successfully.

Since the mongo service may be stopped/removed
by jujud itself, we do not check the error of
stopping/removing the service.

Fixes lp:1292275

https://code.launchpad.net/~axwalk/juju-core/lp1292275-localdestroy-stopremovemongod/+merge/212117

(do not edit description out of merge proposal)

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

Affected files (+7, -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-20140321051700-yu7igducmicd91p5
+New revision: <email address hidden>

Index: provider/local/environ.go
=== modified file 'provider/local/environ.go'
--- provider/local/environ.go 2014-03-19 21:08:58 +0000
+++ provider/local/environ.go 2014-03-21 09:01:39 +0000
@@ -39,6 +39,7 @@
   "launchpad.net/juju-core/state"
   "launchpad.net/juju-core/state/api"
   "launchpad.net/juju-core/state/api/params"
+ "launchpad.net/juju-core/upstart"
   "launchpad.net/juju-core/utils/shell"
   "launchpad.net/juju-core/version"
   "launchpad.net/juju-core/worker/terminationworker"
@@ -437,6 +438,10 @@
     }
    }
   }
+ // Stop the mongo database. It's possible that the service
+ // doesn't exist or is not running, so don't check the error.
+ upstart.NewService(env.mongoServiceName()).StopAndRemove()
+ // Finally, remove the data-dir.
   if err := os.RemoveAll(env.config.rootDir()); err != nil
&& !os.IsNotExist(err) {
    return err
   }

[William Reade (fwereade)]

Can we think of any way to test this? Does it come down to CI tests that
carefully break a local environment in a number of ways, and check it
can still be torn down?

https://codereview.appspot.com/78770043/

[Andrew Wilkins (axwalk)]

On 2014/03/24 10:15:26, fwereade wrote:
> Can we think of any way to test this? Does it come down to CI tests
that
> carefully break a local environment in a number of ways, and check it
can still
> be torn down?

I can't think of a decent way to unit test it. It's not too hard for an
integration test:

Copy juju binary somewhere, and write a script called jujud in the same
dir.
The jujud script will be called with "version", and then later with
"bootstrap-state". It must respond to "version" with a valid version; if
it errors on bootstrap-state, it'll trigger the
destroy-on-failed-bootstrap codepath and the mongo service will be
removed (with this patch).

https://codereview.appspot.com/78770043/

=== modified file 'provider/local/environ.go'
--- provider/local/environ.go	2014-03-19 21:08:58 +0000
+++ provider/local/environ.go	2014-03-21 09:08:24 +0000
@@ -39,6 +39,7 @@
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/state/api/params"
+	"launchpad.net/juju-core/upstart"
 	"launchpad.net/juju-core/utils/shell"
 	"launchpad.net/juju-core/version"
 	"launchpad.net/juju-core/worker/terminationworker"
@@ -437,6 +438,10 @@
 			}
 		}
 	}
+	// Stop the mongo database. It's possible that the service
+	// doesn't exist or is not running, so don't check the error.
+	upstart.NewService(env.mongoServiceName()).StopAndRemove()
+	// Finally, remove the data-dir.
 	if err := os.RemoveAll(env.config.rootDir()); err != nil && !os.IsNotExist(err) {
 		return err
 	}