Merge lp:~axwalk/juju-core/lp1238948-sigabrt-uninstall into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

provider/null: implement Environ.Destroy

A new worker is introduced: worker/terminationworker.
This worker simply waits for a signal (SIGABRT), and
then exits with ErrTerminateAgent. This causes the
agent to uninstall itself. We do not use SIGTERM, as
that would cause "service stop jujud-machine-N" to
uninstall the agent.

The null/manual provider has been updated at the same
time to implement Environ.Destroy. The Destroy method
simply connects to the bootstrap machine via SSH, and
issues "sudo pkill -SIGABRT jujud". We only do this
for machine 0, as Destroy will never be reached until
all non-manager machines are destroyed.

Fixes #1238948

https://codereview.appspot.com/48950043/

provider/null: implement Environ.Destroy

A new worker is introduced: worker/terminationworker.
This worker simply waits for a signal (SIGABRT), and
then exits with ErrTerminateAgent. This causes the
agent to uninstall itself. We do not use SIGTERM, as
that would cause "service stop jujud-machine-N" to
uninstall the agent.

The null/manual provider has been updated at the same
time to implement Environ.Destroy. The Destroy method
simply connects to the bootstrap machine via SSH, and
issues "sudo pkill -SIGABRT jujud". We only do this
for machine 0, as Destroy will never be reached until
all non-manager machines are destroyed.

Fixes #1238948

https://codereview.appspot.com/48950043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+200767_code.launchpad.net,

Message:
Please take a look.

Description:
provider/null: implement Environ.Destroy

A new worker is introduced: worker/terminationworker.
This worker simply waits for a signal (SIGABRT), and
then exits with ErrTerminateAgent. This causes the
agent to uninstall itself. We do not use SIGTERM, as
that would cause "service stop jujud-machine-N" to
uninstall the agent.

The null/manual provider has been updated at the same
time to implement Environ.Destroy. The Destroy method
simply connects to the bootstrap machine via SSH, and
issues "sudo pkill -SIGABRT jujud". We only do this
for machine 0, as Destroy will never be reached until
all non-manager machines are destroyed.

Fixes #1238948

https://code.launchpad.net/~axwalk/juju-core/lp1238948-sigabrt-uninstall/+merge/200767

(do not edit description out of merge proposal)

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

Affected files (+177, -2 lines):
   A [revision details]
   M cmd/jujud/machine.go
   M provider/null/environ.go
   M provider/null/environ_test.go
   A worker/terminationworker/worker.go
   A worker/terminationworker/worker_test.go

[Tim Penhey (thumper)]

On 2014/01/08 05:57:42, axw wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/48950043/

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2013-12-13 02:44:21 +0000
+++ cmd/jujud/machine.go	2014-01-08 06:05:52 +0000
@@ -39,6 +39,7 @@
 	"launchpad.net/juju-core/worker/minunitsworker"
 	"launchpad.net/juju-core/worker/provisioner"
 	"launchpad.net/juju-core/worker/resumer"
+	"launchpad.net/juju-core/worker/terminationworker"
 	"launchpad.net/juju-core/worker/upgrader"
 )
 
@@ -136,6 +137,9 @@
 	a.runner.StartWorker("api", func() (worker.Worker, error) {
 		return a.APIWorker(ensureStateWorker)
 	})
+	a.runner.StartWorker("termination", func() (worker.Worker, error) {
+		return terminationworker.NewWorker(), nil
+	})
 	err := a.runner.Wait()
 	if err == worker.ErrTerminateAgent {
 		err = a.uninstallAgent()

=== modified file 'provider/null/environ.go'
--- provider/null/environ.go	2014-01-06 07:17:46 +0000
+++ provider/null/environ.go	2014-01-08 06:05:52 +0000
@@ -4,9 +4,12 @@
 package null
 
 import (
+	"bytes"
 	"errors"
+	"fmt"
 	"net"
 	"path"
+	"strings"
 	"sync"
 
 	"launchpad.net/loggo"
@@ -26,7 +29,9 @@
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/tools"
+	"launchpad.net/juju-core/utils/ssh"
 	"launchpad.net/juju-core/worker/localstorage"
+	"launchpad.net/juju-core/worker/terminationworker"
 )
 
 const (
@@ -228,8 +233,25 @@
 	return nil
 }
 
+var runSSHCommand = func(host string, command []string) (stderr string, err error) {
+	cmd := ssh.Command(host, command, ssh.NoPasswordAuthentication)
+	var stderrBuf bytes.Buffer
+	cmd.Stderr = &stderrBuf
+	err = cmd.Run()
+	return stderrBuf.String(), err
+}
+
 func (e *nullEnviron) Destroy() error {
-	return errors.New("null provider destruction is not implemented yet")
+	stderr, err := runSSHCommand(
+		"ubuntu@"+e.envConfig().bootstrapHost(),
+		[]string{"sudo", "pkill", fmt.Sprintf("-%d", terminationworker.TerminationSignal), "jujud"},
+	)
+	if err != nil {
+		if stderr := strings.TrimSpace(stderr); len(stderr) > 0 {
+			err = fmt.Errorf("%v (%v)", err, stderr)
+		}
+	}
+	return err
 }
 
 func (e *nullEnviron) OpenPorts(ports []instance.Port) error {

=== modified file 'provider/null/environ_test.go'
--- provider/null/environ_test.go	2014-01-03 05:14:11 +0000
+++ provider/null/environ_test.go	2014-01-08 06:05:52 +0000
@@ -80,7 +80,35 @@
 }
 
 func (s *environSuite) TestDestroy(c *gc.C) {
-	c.Assert(s.env.Destroy(), gc.ErrorMatches, "null provider destruction is not implemented yet")
+	var resultStderr string
+	var resultErr error
+	runSSHCommandTesting := func(host string, command []string) (string, error) {
+		c.Assert(host, gc.Equals, "ubuntu@hostname")
+		c.Assert(command, gc.DeepEquals, []string{"sudo", "pkill", "-6", "jujud"})
+		return resultStderr, resultErr
+	}
+	s.PatchValue(&runSSHCommand, runSSHCommandTesting)
+	type test struct {
+		stderr string
+		err    error
+		match  string
+	}
+	tests := []test{
+		{"", nil, ""},
+		{"abc", nil, ""},
+		{"", errors.New("oh noes"), "oh noes"},
+		{"123", errors.New("abc"), "abc \\(123\\)"},
+	}
+	for i, t := range tests {
+		c.Logf("test %d: %v", i, t)
+		resultStderr, resultErr = t.stderr, t.err
+		err := s.env.Destroy()
+		if t.match == "" {
+			c.Assert(err, gc.IsNil)
+		} else {
+			c.Assert(err, gc.ErrorMatches, t.match)
+		}
+	}
 }
 
 func (s *environSuite) TestLocalStorageConfig(c *gc.C) {

=== added directory 'worker/terminationworker'
=== added file 'worker/terminationworker/worker.go'
--- worker/terminationworker/worker.go	1970-01-01 00:00:00 +0000
+++ worker/terminationworker/worker.go	2014-01-08 06:05:52 +0000
@@ -0,0 +1,60 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package terminationworker
+
+import (
+	"os"
+	"os/signal"
+	"syscall"
+
+	"launchpad.net/tomb"
+
+	"launchpad.net/juju-core/worker"
+)
+
+// TerminationSignal is the signal that
+// indicates the agent should terminate
+// and uninstall itself.
+//
+// We do not use SIGTERM as SIGTERM is the
+// default signal used to initiate a graceful
+// shutdown.
+const TerminationSignal = syscall.SIGABRT
+
+// NewWorker returns a worker that wais for a
+
+type terminationWorker struct {
+	tomb tomb.Tomb
+}
+
+// TerminationSignal signal and exits with
+// ErrTerminateAgent.
+func NewWorker() worker.Worker {
+	u := &terminationWorker{}
+	go func() {
+		defer u.tomb.Done()
+		u.tomb.Kill(u.loop())
+	}()
+	return u
+}
+
+func (u *terminationWorker) Kill() {
+	u.tomb.Kill(nil)
+}
+
+func (u *terminationWorker) Wait() error {
+	return u.tomb.Wait()
+}
+
+func (u *terminationWorker) loop() (err error) {
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, TerminationSignal)
+	defer signal.Stop(c)
+	select {
+	case <-c:
+		return worker.ErrTerminateAgent
+	case <-u.tomb.Dying():
+		return nil
+	}
+}

=== added file 'worker/terminationworker/worker_test.go'
--- worker/terminationworker/worker_test.go	1970-01-01 00:00:00 +0000
+++ worker/terminationworker/worker_test.go	2014-01-08 06:05:52 +0000
@@ -0,0 +1,59 @@
+// Copyright 2014 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package terminationworker_test
+
+import (
+	"os"
+	"os/signal"
+	stdtesting "testing"
+
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/testing/testbase"
+	"launchpad.net/juju-core/worker"
+	"launchpad.net/juju-core/worker/terminationworker"
+)
+
+func TestPackage(t *stdtesting.T) {
+	gc.TestingT(t)
+}
+
+var _ = gc.Suite(&TerminationWorkerSuite{})
+
+type TerminationWorkerSuite struct {
+	testbase.LoggingSuite
+	// c is a channel that will wait for the termination
+	// signal, to prevent signals terminating the process.
+	c chan os.Signal
+}
+
+func (s *TerminationWorkerSuite) SetUpTest(c *gc.C) {
+	s.LoggingSuite.SetUpTest(c)
+	s.c = make(chan os.Signal, 1)
+	signal.Notify(s.c, terminationworker.TerminationSignal)
+}
+
+func (s *TerminationWorkerSuite) TearDownTest(c *gc.C) {
+	close(s.c)
+	signal.Stop(s.c)
+	s.LoggingSuite.TearDownTest(c)
+}
+
+func (s *TerminationWorkerSuite) TestStartStop(c *gc.C) {
+	w := terminationworker.NewWorker()
+	w.Kill()
+	err := w.Wait()
+	c.Assert(err, gc.IsNil)
+}
+
+func (s *TerminationWorkerSuite) TestSignal(c *gc.C) {
+	w := terminationworker.NewWorker()
+	proc, err := os.FindProcess(os.Getpid())
+	c.Assert(err, gc.IsNil)
+	defer proc.Release()
+	err = proc.Signal(terminationworker.TerminationSignal)
+	c.Assert(err, gc.IsNil)
+	err = w.Wait()
+	c.Assert(err, gc.Equals, worker.ErrTerminateAgent)
+}