Merge lp:~jameinel/juju-core/api-connect-upgrade-1199915 into lp:~go-bot/juju-core/trunk

[John A Meinel]

cmd/jujud/machineagent: install LXC on start

Related to bug #1199913. On startup we will 'apt-get install lxc' if we
need to. This uses the containers/lxc code to detect if we have lxc
available, and if it fails then we request to install it.

I've tested this live, and it does solve the immediate problem.

Also, the test suite still passes because all the things that call
MachineAgent.Run are already using the lxc.TestSuite and so the
containers code "provides" lxc via the MockFactory.

One test we could do is that we try to install lxc if we run
the machine agent (and inject a MockFactory that fails the List
request). But I felt live testing was appropriate for this.

https://codereview.appspot.com/10986044/

cmd/jujud/machineagent: install LXC on start

Related to bug #1199913. On startup we will 'apt-get install lxc' if we
need to. This uses the containers/lxc code to detect if we have lxc
available, and if it fails then we request to install it.

I've tested this live, and it does solve the immediate problem.

Also, the test suite still passes because all the things that call
MachineAgent.Run are already using the lxc.TestSuite and so the
containers code "provides" lxc via the MockFactory.

One test we could do is that we try to install lxc if we run
the machine agent (and inject a MockFactory that fails the List
request). But I felt live testing was appropriate for this.

https://codereview.appspot.com/10986044/

[John A Meinel (jameinel)]

Reviewers: mp+174207_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/jujud/machineagent: install LXC on start

Related to bug #1199913. On startup we will 'apt-get install lxc' if we
need to. This uses the containers/lxc code to detect if we have lxc
available, and if it fails then we request to install it.

I've tested this live, and it does solve the immediate problem.

Also, the test suite still passes because all the things that call
MachineAgent.Run are already using the lxc.TestSuite and so the
containers code "provides" lxc via the MockFactory.

One test we could do is that we try to install lxc if we run
the machine agent (and inject a MockFactory that fails the List
request). But I felt live testing was appropriate for this.

https://code.launchpad.net/~jameinel/juju-core/api-connect-upgrade-1199915/+merge/174207

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/jujud/machine.go
   A cmd/jujud/upgradevalidation.go
   A utils/apt.go
   A utils/apt_test.go

[William Reade (fwereade)]

https://codereview.appspot.com/10986044/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/10986044/diff/1/cmd/jujud/machine.go#newcode82
cmd/jujud/machine.go:82: log.Errorf("we were unable to install the lxc
package, unable to continue: %v", err)
Quibble quibble waffle waffle, this will break on other platforms.
Unbreaks upgrade for now though, so fair enough.

https://codereview.appspot.com/10986044/diff/1/utils/apt.go
File utils/apt.go (right):

https://codereview.appspot.com/10986044/diff/1/utils/apt.go#newcode18
utils/apt.go:18: // apt concurrently, we should use that same
locking here
We already have that. uniter.go:116

https://codereview.appspot.com/10986044/

[Dimiter Naydenov (dimitern)]

LGTM

https://codereview.appspot.com/10986044/diff/1/cmd/jujud/machine.go
File cmd/jujud/machine.go (right):

https://codereview.appspot.com/10986044/diff/1/cmd/jujud/machine.go#newcode82
cmd/jujud/machine.go:82: log.Errorf("we were unable to install the lxc
package, unable to continue: %v", err)
On 2013/07/11 14:03:11, fwereade wrote:
> Quibble quibble waffle waffle, this will break on other platforms.
Unbreaks
> upgrade for now though, so fair enough.

Add TODO describing this please.

https://codereview.appspot.com/10986044/

[John A Meinel (jameinel)]

Please take a look.

https://codereview.appspot.com/10986044/

[William Reade (fwereade)]

LGTM with verbose warnings; I would *really* appreciate a followup that
somehow tested this situation live, but I'm at a little bit of a loss
how to do that sanely.

https://codereview.appspot.com/10986044/diff/5002/cmd/jujud/upgradevalidation.go
File cmd/jujud/upgradevalidation.go (right):

https://codereview.appspot.com/10986044/diff/5002/cmd/jujud/upgradevalidation.go#newcode30
cmd/jujud/upgradevalidation.go:30: // Note: Should we try to do the
install anyway, just without the lock?
Yeah, I think so. Log a great big warning and see what you can do. If we
fail, we bomb out, and upstart gives us another try; eventually we'll
mess with the unit badly enough that we succeed, I suspect, and then we
have a working machine again... and at least the unit is somewhat
user-fixable.

https://codereview.appspot.com/10986044/

[Dimiter Naydenov (dimitern)]

LGTM, just remove a TODO about locks.

https://codereview.appspot.com/10986044/diff/5002/utils/apt.go
File utils/apt.go (right):

https://codereview.appspot.com/10986044/diff/5002/utils/apt.go#newcode17
utils/apt.go:17: // TODO: When we have a unit-level lock to avoid
multiple unit agents running
delete this now?

https://codereview.appspot.com/10986044/

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2013-07-11 12:11:14 +0000
+++ cmd/jujud/machine.go	2013-07-11 15:53:26 +0000
@@ -80,6 +80,10 @@
 	if err := a.Conf.read(a.Tag()); err != nil {
 		return err
 	}
+	if err := EnsureWeHaveLXC(a.Conf.DataDir); err != nil {
+		log.Errorf("we were unable to install the lxc package, unable to continue: %v", err)
+		return err
+	}
 	charm.CacheDir = filepath.Join(a.Conf.DataDir, "charmcache")
 
 	// ensureStateWorker ensures that there is a worker that

=== added file 'cmd/jujud/upgradevalidation.go'
--- cmd/jujud/upgradevalidation.go	1970-01-01 00:00:00 +0000
+++ cmd/jujud/upgradevalidation.go	2013-07-11 15:53:26 +0000
@@ -0,0 +1,73 @@
+// Copyright 2012, 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package main
+
+import (
+	"path/filepath"
+	"time"
+
+	"launchpad.net/loggo"
+
+	"launchpad.net/juju-core/container/lxc"
+	"launchpad.net/juju-core/utils"
+	"launchpad.net/juju-core/utils/fslock"
+)
+
+// Functions in this package are here to help us with post-upgrade installing,
+// etc. It is used to validate the system status after we have upgraded.
+
+var validationLogger = loggo.GetLogger("juju.jujud.validation")
+
+var lockTimeout = 1 * time.Minute
+
+// getUniterLock grabs the "uniter-hook-execution" lock and holds it, or an error
+func getUniterLock(dataDir, message string) (*fslock.Lock, error) {
+	// Taken from worker/uniter/uniter.go setupLocks()
+	lockDir := filepath.Join(dataDir, "locks")
+	hookLock, err := fslock.NewLock(lockDir, "uniter-hook-execution")
+	if err != nil {
+		return nil, err
+	}
+	err = hookLock.LockWithTimeout(lockTimeout, message)
+	if err != nil {
+		return nil, err
+	}
+	return hookLock, nil
+}
+
+// EnsureWeHaveLXC checks if we have lxc installed, and installs it if we
+// don't. Juju 1.11 added the ability to deploy into LXC containers, and uses
+// functionality from the lxc package in order to do so. Juju 1.10 did not
+// install lxc, so we ensure it is installed.
+// See http://bugs.launchpad.net/bug/1199913
+// dataDir is the root location where data files are put. It is used to grab
+// the uniter-hook-execution lock so that we don't try run to apt-get at the
+// same time that a hook might want to run it.
+func EnsureWeHaveLXC(dataDir string) error {
+	manager := lxc.NewContainerManager("lxc-test")
+	if _, err := manager.ListContainers(); err == nil {
+		validationLogger.Debugf("found lxc, not installing")
+		// We already have it, nothing more to do
+		return nil
+	}
+	validationLogger.Debugf("got error looking for lxc, attempting to install")
+	if dataDir != "" {
+		lock, err := getUniterLock(dataDir, "apt-get install lxc for juju 1.11 upgrade")
+		if err == nil {
+			defer lock.Unlock()
+		} else {
+			validationLogger.Warningf("Failed to acquire lock: %v, will try to install lxc anyway", lock)
+		}
+		// If we got an error trying to acquire the lock, we try to install
+		// lxc anyway. Worst case the install will fail, which is where we
+		// are already
+	}
+	// TODO: This is not platform independent. If jujud is running on
+	//       something other than a debian-based Linux, and we are missing
+	//       lxc, this call will always fail. However, in juju 1.11+ we
+	//       install lxc via cloud-init or whatever bootstrap code we use.
+	//       So this is really only upgrade compatibility and juju 1.10
+	//       only supports debian-based anyway
+	return utils.AptGetInstall("lxc")
+}

=== added file 'utils/apt.go'
--- utils/apt.go	1970-01-01 00:00:00 +0000
+++ utils/apt.go	2013-07-11 15:53:26 +0000
@@ -0,0 +1,45 @@
+// Copyright 2012, 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package utils
+
+import (
+	"os"
+	"os/exec"
+
+	"launchpad.net/loggo"
+)
+
+var aptLogger = loggo.GetLogger("juju.utils.apt")
+
+// Some helpful functions for running apt in a sane way
+
+// osRunCommand calls cmd.Run, this is used as an overloading point so we can
+// test what *would* be run without actually executing another program
+func osRunCommand(cmd *exec.Cmd) error {
+	return cmd.Run()
+}
+
+var runCommand = osRunCommand
+
+// This is the default apt-get command used in cloud-init, the various settings
+// mean that apt won't actually block waiting for a prompt from the user.
+var aptGetCommand = []string{
+	"apt-get", "--option=Dpkg::Options::=--force-confold",
+	"--option=Dpkg::options::=--force-unsafe-io", "--assume-yes", "--quiet",
+}
+
+// aptEnvOptions are options we need to pass to apt-get to not have it prompt
+// the user
+var aptGetEnvOptions = []string{"DEBIAN_FRONTEND=noninteractive"}
+
+// AptGetInstall runs 'apt-get install packages' for the packages listed here
+func AptGetInstall(packages ...string) error {
+	cmdArgs := append([]string(nil), aptGetCommand...)
+	cmdArgs = append(cmdArgs, "install")
+	cmdArgs = append(cmdArgs, packages...)
+	aptLogger.Infof("Running: %s", cmdArgs)
+	cmd := exec.Command(cmdArgs[0], cmdArgs[1:]...)
+	cmd.Env = append(os.Environ(), aptGetEnvOptions...)
+	return runCommand(cmd)
+}

=== added file 'utils/apt_test.go'
--- utils/apt_test.go	1970-01-01 00:00:00 +0000
+++ utils/apt_test.go	2013-07-11 15:53:26 +0000
@@ -0,0 +1,44 @@
+// Copyright 2012, 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package utils
+
+import (
+	"os/exec"
+
+	gc "launchpad.net/gocheck"
+)
+
+type AptSuite struct{}
+
+var _ = gc.Suite(&AptSuite{})
+
+// hookRunCommand intercepts runCommand to a function that passes the actual
+// command back via a channel, and returns the error passed into this function.
+// It also returns a cleanup function so you can restore the original function
+func (s *AptSuite) hookRunCommand(err error) (<-chan *exec.Cmd, func()) {
+	cmdChan := make(chan *exec.Cmd, 1)
+	origRunCommand := runCommand
+	cleanup := func() {
+		runCommand = origRunCommand
+	}
+	runCommand = func(cmd *exec.Cmd) error {
+		cmdChan <- cmd
+		return err
+	}
+	return cmdChan, cleanup
+}
+
+func (s *AptSuite) TestOnePackage(c *gc.C) {
+	cmdChan, cleanup := s.hookRunCommand(nil)
+	defer cleanup()
+	err := AptGetInstall("test-package")
+	c.Assert(err, gc.IsNil)
+	cmd := <-cmdChan
+	c.Assert(cmd.Args, gc.DeepEquals, []string{
+		"apt-get", "--option=Dpkg::Options::=--force-confold",
+		"--option=Dpkg::options::=--force-unsafe-io", "--assume-yes", "--quiet",
+		"install", "test-package",
+	})
+	c.Assert(cmd.Env[len(cmd.Env)-1], gc.Equals, "DEBIAN_FRONTEND=noninteractive")
+}