Merge lp:~axwalk/juju-core/lp1261343-manual-ubuntu-user into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

environs/manual: add ubuntu user

Manual bootstrap and provisioning now add the
ubuntu user (if it doesn't exist), update its
authorized_keys, and enables passwordless sudo
for it. This is somewhat invasive, so we will
have to call it out clearly in the documentation.

All ssh-related things other than the user
initialisation can now do away with the special
logic for handling sudo interactions. We don't
pass stdin/stdout unless we need them; we don't
allocate a PTY unless we need it; we don't allow
password authentication unless we need it.

Fixes #1261343

https://codereview.appspot.com/47390043/

environs/manual: add ubuntu user

Manual bootstrap and provisioning now add the
ubuntu user (if it doesn't exist), update its
authorized_keys, and enables passwordless sudo
for it. This is somewhat invasive, so we will
have to call it out clearly in the documentation.

All ssh-related things other than the user
initialisation can now do away with the special
logic for handling sudo interactions. We don't
pass stdin/stdout unless we need them; we don't
allocate a PTY unless we need it; we don't allow
password authentication unless we need it.

Fixes #1261343

https://codereview.appspot.com/47390043/

[Andrew Wilkins (axwalk)]

Reviewers: mp+200365_code.launchpad.net,

Message:
Please take a look.

Description:
environs/manual: add ubuntu user

Manual bootstrap and provisioning now add the
ubuntu user (if it doesn't exist), update its
authorized_keys, and enables passwordless sudo
for it. This is somewhat invasive, so we will
have to call it out clearly in the documentation.

All ssh-related things other than the user
initialisation can now do away with the special
logic for handling sudo interactions. We don't
pass stdin/stdout unless we need them; we don't
allocate a PTY unless we need it; we don't allow
password authentication unless we need it.

https://code.launchpad.net/~axwalk/juju-core/lp1261343-manual-ubuntu-user/+merge/200365

(do not edit description out of merge proposal)

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

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/47390043/

[William Reade (fwereade)]

LGTM, definitely a move in a nice direction. Just trivials and
longer-term discussion points.

https://codereview.appspot.com/47390043/diff/20001/environs/manual/bootstrap.go
File environs/manual/bootstrap.go (right):

https://codereview.appspot.com/47390043/diff/20001/environs/manual/bootstrap.go#newcode32
environs/manual/bootstrap.go:32: PossibleTools tools.List
Not germane to this CL, but worth a discussion: we need to fix this
tools nonsense. Now we have manual bootstrap, is there any compelling
reason to worry about tools availability until we're connected to that
machine? Looking them all up is kinda tedious and expensive, and we keep
on doing it again and again, and -- worse -- we always do it from the
client (which may not see the same networks as does the bootstrapped
machine, and might be completely useless).

We *will* ofc still need *some* way to push tools up from a local
directory, but that shouldn't be too hard really, right?

https://codereview.appspot.com/47390043/diff/20001/environs/manual/provisioner_test.go
File environs/manual/provisioner_test.go (right):

https://codereview.appspot.com/47390043/diff/20001/environs/manual/provisioner_test.go#newcode95
environs/manual/provisioner_test.go:95: defer installFakeSSH(c, "", nil,
0)() // InitUbuntuUser
This stuff is both a bit repetitive and a bit opaque. Worth wrapping it
up into something a bit more literate maybe?

https://codereview.appspot.com/47390043/diff/20001/provider/null/environ.go
File provider/null/environ.go (right):

https://codereview.appspot.com/47390043/diff/20001/provider/null/environ.go#newcode53
provider/null/environ.go:53: ubuntuUserInit bool
Inited?

https://codereview.appspot.com/47390043/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/47390043/diff/20001/environs/manual/bootstrap.go
File environs/manual/bootstrap.go (right):

https://codereview.appspot.com/47390043/diff/20001/environs/manual/bootstrap.go#newcode32
environs/manual/bootstrap.go:32: PossibleTools tools.List
On 2014/01/03 13:07:55, fwereade wrote:
> Not germane to this CL, but worth a discussion: we need to fix this
tools
> nonsense. Now we have manual bootstrap, is there any compelling reason
to worry
> about tools availability until we're connected to that machine?
Looking them all
> up is kinda tedious and expensive, and we keep on doing it again and
again, and
> -- worse -- we always do it from the client (which may not see the
same networks
> as does the bootstrapped machine, and might be completely useless).

> We *will* ofc still need *some* way to push tools up from a local
directory, but
> that shouldn't be too hard really, right?

The tools lookup for the null provider happens immediately before we
check if the machine is provisioned, and then connect. I don't think
there'd be much to gain in terms of moving it in time. As for moving
from client to the server...

There is possibly one compelling reason for doing it from the client:
supporting environments with limited egress. As it is now, manual
bootstrap effectively SCPs the tools to the bootstrap node, SSHs in and
then runs the installation script. We could require those environments
to have an internally-accessible tools repository, but this seems quite
onerous.

The simplestreams/tools lookup code is all part of the Go binary, so how
would it be done from the server anyway?

https://codereview.appspot.com/47390043/diff/20001/environs/manual/provisioner_test.go
File environs/manual/provisioner_test.go (right):

https://codereview.appspot.com/47390043/diff/20001/environs/manual/provisioner_test.go#newcode95
environs/manual/provisioner_test.go:95: defer installFakeSSH(c, "", nil,
0)() // InitUbuntuUser
On 2014/01/03 13:07:55, fwereade wrote:
> This stuff is both a bit repetitive and a bit opaque. Worth wrapping
it up into
> something a bit more literate maybe?

I cleaned up these couple, and one in bootstrap_test.go. There's some
others in init_test.go that could probably be cleaned up, but I've not
come up with a good idea how to do that just yet.

https://codereview.appspot.com/47390043/diff/20001/provider/null/environ.go
File provider/null/environ.go (right):

https://codereview.appspot.com/47390043/diff/20001/provider/null/environ.go#newcode53
provider/null/environ.go:53: ubuntuUserInit bool
On 2014/01/03 13:07:55, fwereade wrote:
> Inited?

Done.

https://codereview.appspot.com/47390043/

=== modified file 'cloudinit/sshinit/configure.go'
--- cloudinit/sshinit/configure.go	2013-12-19 09:15:40 +0000
+++ cloudinit/sshinit/configure.go	2014-01-06 07:18:52 +0000
@@ -25,13 +25,6 @@
 	// Config is the cloudinit config to carry out.
 	Config *cloudinit.Config
 
-	// Stdin is required to respond to sudo prompts,
-	// and must be a terminal (except in tests)
-	Stdin io.Reader
-
-	// Stdout is required to present sudo prompts to the user.
-	Stdout io.Writer
-
 	// Stderr is required to present bootstrap progress to the user.
 	Stderr io.Writer
 }
@@ -48,12 +41,10 @@
 	script = fmt.Sprintf(`F=$(mktemp); echo %s | base64 -d > $F; . $F`, scriptBase64)
 	cmd := ssh.Command(
 		params.Host,
-		[]string{"sudo", fmt.Sprintf("bash -c '%s'", script)},
-		ssh.AllocateTTY,
+		[]string{"sudo", "-n", fmt.Sprintf("bash -c '%s'", script)},
+		ssh.NoPasswordAuthentication,
 	)
-	cmd.Stdout = params.Stdout
 	cmd.Stderr = params.Stderr
-	cmd.Stdin = params.Stdin
 	return cmd.Run()
 }
 

=== modified file 'environs/config/authkeys.go'
--- environs/config/authkeys.go	2013-08-28 20:37:53 +0000
+++ environs/config/authkeys.go	2014-01-06 07:18:52 +0000
@@ -26,14 +26,14 @@
 	return f
 }
 
-// authorizedKeys implements the standard juju behaviour for finding
+// ReadAuthorizedKeys implements the standard juju behaviour for finding
 // authorized_keys. It returns a set of keys in in authorized_keys format
 // (see sshd(8) for a description).  If path is non-empty, it names the
 // file to use; otherwise the user's .ssh directory will be searched.
 // Home directory expansion will be performed on the path if it starts with
 // a ~; if the expanded path is relative, it will be interpreted relative
 // to $HOME/.ssh.
-func readAuthorizedKeys(path string) (string, error) {
+func ReadAuthorizedKeys(path string) (string, error) {
 	var files []string
 	if path == "" {
 		files = []string{"id_dsa.pub", "id_rsa.pub", "identity.pub"}

=== modified file 'environs/config/config.go'
--- environs/config/config.go	2013-12-19 00:38:03 +0000
+++ environs/config/config.go	2014-01-06 07:18:52 +0000
@@ -147,7 +147,7 @@
 	keys := c.asString("authorized-keys")
 	if path != "" || keys == "" {
 		var err error
-		c.m["authorized-keys"], err = readAuthorizedKeys(path)
+		c.m["authorized-keys"], err = ReadAuthorizedKeys(path)
 		if err != nil {
 			return err
 		}

=== modified file 'environs/manual/bootstrap.go'
--- environs/manual/bootstrap.go	2013-12-20 03:12:26 +0000
+++ environs/manual/bootstrap.go	2014-01-06 07:18:52 +0000
@@ -27,16 +27,11 @@
 }
 
 type BootstrapArgs struct {
-	Host          string
-	DataDir       string
-	Environ       LocalStorageEnviron
-	PossibleTools tools.List
-	Context       environs.BootstrapContext
-
-	// If series and hardware characteristics
-	// are known ahead of time, they can be
-	// set here and Bootstrap will not attempt
-	// to detect them again.
+	Host                    string
+	DataDir                 string
+	Environ                 LocalStorageEnviron
+	PossibleTools           tools.List
+	Context                 environs.BootstrapContext
 	Series                  string
 	HardwareCharacteristics *instance.HardwareCharacteristics
 }
@@ -48,6 +43,9 @@
 // NewManualBootstrapEnviron wraps a LocalStorageEnviron with another which
 // overrides the Bootstrap method; when Bootstrap is invoked, the specified
 // host will be manually bootstrapped.
+//
+// InitUbuntuUser is expected to have been executed successfully against
+// the host being bootstrapped.
 func Bootstrap(args BootstrapArgs) (err error) {
 	if args.Host == "" {
 		return errors.New("host argument is empty")
@@ -58,6 +56,15 @@
 	if args.DataDir == "" {
 		return errors.New("data-dir argument is empty")
 	}
+	if args.Series == "" {
+		return errors.New("series argument is empty")
+	}
+	if args.HardwareCharacteristics == nil {
+		return errors.New("hardware characteristics argument is empty")
+	}
+	if len(args.PossibleTools) == 0 {
+		return errors.New("possible tools is empty")
+	}
 
 	provisioned, err := checkProvisioned(args.Host)
 	if err != nil {
@@ -67,23 +74,11 @@
 		return ErrProvisioned
 	}
 
-	var series string
-	var hc instance.HardwareCharacteristics
-	if args.Series != "" && args.HardwareCharacteristics != nil {
-		series = args.Series
-		hc = *args.HardwareCharacteristics
-	} else {
-		hc, series, err = DetectSeriesAndHardwareCharacteristics(args.Host)
-		if err != nil {
-			return fmt.Errorf("error detecting hardware characteristics: %v", err)
-		}
-	}
-
 	// Filter tools based on detected series/arch.
 	logger.Infof("Filtering possible tools: %v", args.PossibleTools)
 	possibleTools, err := args.PossibleTools.Match(tools.Filter{
-		Arch:   *hc.Arch,
-		Series: series,
+		Arch:   *args.HardwareCharacteristics.Arch,
+		Series: args.Series,
 	})
 	if err != nil {
 		return err
@@ -96,7 +91,7 @@
 		bootstrapStorage,
 		&bootstrap.BootstrapState{
 			StateInstances:  []instance.Id{BootstrapInstanceId},
-			Characteristics: []instance.HardwareCharacteristics{hc},
+			Characteristics: []instance.HardwareCharacteristics{*args.HardwareCharacteristics},
 		},
 	)
 	if err != nil {
@@ -141,5 +136,5 @@
 	for k, v := range agentEnv {
 		mcfg.AgentEnvironment[k] = v
 	}
-	return provisionMachineAgent(args.Host, mcfg, args.Context.Stdin(), args.Context.Stdout(), args.Context.Stderr())
+	return provisionMachineAgent(args.Host, mcfg, args.Context.Stderr())
 }

=== modified file 'environs/manual/bootstrap_test.go'
--- environs/manual/bootstrap_test.go	2013-12-20 02:38:56 +0000
+++ environs/manual/bootstrap_test.go	2014-01-06 07:18:52 +0000
@@ -71,12 +71,17 @@
 	c.Assert(err, gc.IsNil)
 	toolsList, err := tools.FindBootstrapTools(s.Conn.Environ, tools.BootstrapToolsParams{})
 	c.Assert(err, gc.IsNil)
+	arch := "amd64"
 	return BootstrapArgs{
 		Host:          hostname,
 		DataDir:       "/var/lib/juju",
 		Environ:       s.env,
 		PossibleTools: toolsList,
-		Context:       envtesting.NewBootstrapContext(coretesting.Context(c)),
+		Series:        "precise",
+		HardwareCharacteristics: &instance.HardwareCharacteristics{
+			Arch: &arch,
+		},
+		Context: envtesting.NewBootstrapContext(coretesting.Context(c)),
 	}
 }
 
@@ -84,7 +89,7 @@
 	args := s.getArgs(c)
 	args.Host = "ubuntu@" + args.Host
 
-	defer fakeSSH{Series: s.Conn.Environ.Config().DefaultSeries()}.install(c).Restore()
+	defer fakeSSH{SkipDetection: true}.install(c).Restore()
 	err := Bootstrap(args)
 	c.Assert(err, gc.IsNil)
 
@@ -99,60 +104,24 @@
 	// Do it all again; this should work, despite the fact that
 	// there's a bootstrap state file. Existence for that is
 	// checked in general bootstrap code (environs/bootstrap).
-	defer fakeSSH{Series: s.Conn.Environ.Config().DefaultSeries()}.install(c).Restore()
+	defer fakeSSH{SkipDetection: true}.install(c).Restore()
 	err = Bootstrap(args)
 	c.Assert(err, gc.IsNil)
 
 	// We *do* check that the machine has no juju* upstart jobs, though.
-	defer installFakeSSH(c, "", "/etc/init/jujud-machine-0.conf", 0).Restore()
+	defer fakeSSH{
+		Provisioned:        true,
+		SkipDetection:      true,
+		SkipProvisionAgent: true,
+	}.install(c).Restore()
 	err = Bootstrap(args)
 	c.Assert(err, gc.Equals, ErrProvisioned)
 }
 
-func (s *bootstrapSuite) TestBootstrapSkipDetection(c *gc.C) {
-	args := s.getArgs(c)
-	hc, err := instance.ParseHardware("arch=amd64")
-	c.Assert(err, gc.IsNil)
-
-	type test struct {
-		series string
-		hc     *instance.HardwareCharacteristics
-	}
-	tests := []test{{
-		series: "",
-		hc:     nil,
-	}, {
-		series: "precise",
-		hc:     nil,
-	}, {
-		series: "",
-		hc:     &hc,
-	}, {
-		series: "precise",
-		hc:     &hc,
-	}}
-
-	for i, test := range tests {
-		c.Logf("test %d: %+v", i, test)
-		args.Series = test.series
-		args.HardwareCharacteristics = test.hc
-		var ssh fakeSSH
-		if args.Series != "" && args.HardwareCharacteristics != nil {
-			// If neither series nor hardware-characteristics
-			// is missing, detection is skipped.
-			ssh.SkipDetection = true
-		}
-		defer ssh.install(c).Restore()
-		err = Bootstrap(args)
-		c.Assert(err, gc.IsNil)
-	}
-}
-
 func (s *bootstrapSuite) TestBootstrapScriptFailure(c *gc.C) {
 	args := s.getArgs(c)
 	args.Host = "ubuntu@" + args.Host
-	series := s.Conn.Environ.Config().DefaultSeries()
-	defer fakeSSH{Series: series, ProvisionAgentExitCode: 1}.install(c).Restore()
+	defer fakeSSH{SkipDetection: true, ProvisionAgentExitCode: 1}.install(c).Restore()
 	err := Bootstrap(args)
 	c.Assert(err, gc.NotNil)
 
@@ -184,12 +153,12 @@
 	// Empty tools list.
 	args := s.getArgs(c)
 	args.PossibleTools = nil
-	series := s.Conn.Environ.Config().DefaultSeries()
-	defer fakeSSH{Series: series, SkipProvisionAgent: true}.install(c).Restore()
-	c.Assert(Bootstrap(args), gc.ErrorMatches, "no matching tools available")
+	defer fakeSSH{SkipDetection: true, SkipProvisionAgent: true}.install(c).Restore()
+	c.Assert(Bootstrap(args), gc.ErrorMatches, "possible tools is empty")
 
 	// Non-empty list, but none that match the series/arch.
-	defer fakeSSH{Series: "edgy", SkipProvisionAgent: true}.install(c).Restore()
 	args = s.getArgs(c)
+	args.Series = "edgy"
+	defer fakeSSH{SkipDetection: true, SkipProvisionAgent: true}.install(c).Restore()
 	c.Assert(Bootstrap(args), gc.ErrorMatches, "no matching tools available")
 }

=== modified file 'environs/manual/fakessh.go'
--- environs/manual/fakessh.go	2013-11-18 05:41:36 +0000
+++ environs/manual/fakessh.go	2014-01-06 07:18:52 +0000
@@ -91,16 +91,28 @@
 	Series string
 	Arch   string
 
+	// Provisioned should be set to true if the fakeSSH script
+	// should respond to checkProvisioned with a non-empty result.
+	Provisioned bool
+
+	// exit code for the checkProvisioned script.
+	CheckProvisionedExitCode int
+
 	// exit code for the machine agent provisioning script.
 	ProvisionAgentExitCode int
 
+	// InitUbuntuUser should be set to true if the fakeSSH script
+	// should respond to an attempt to initialise the ubuntu user.
+	InitUbuntuUser bool
+
 	// there are conditions other than error in the above
 	// that might cause provisioning to not go ahead, such
 	// as tools being missing.
 	SkipProvisionAgent bool
 
 	// detection will be skipped if the series/hardware were
-	// detected ahead of time.
+	// detected ahead of time. This should always be set to
+	// true when testing Bootstrap.
 	SkipDetection bool
 }
 
@@ -118,6 +130,13 @@
 	if !r.SkipDetection {
 		restore.Add(installDetectionFakeSSH(c, r.Series, r.Arch))
 	}
-	add("", nil, 0) // checkProvisioned
+	var checkProvisionedOutput interface{}
+	if r.Provisioned {
+		checkProvisionedOutput = "/etc/init/jujud-machine-0.conf"
+	}
+	add("", checkProvisionedOutput, r.CheckProvisionedExitCode)
+	if r.InitUbuntuUser {
+		add("", nil, 0)
+	}
 	return restore
 }

=== renamed file 'environs/manual/detection.go' => 'environs/manual/init.go'
--- environs/manual/detection.go	2013-12-19 06:39:26 +0000
+++ environs/manual/init.go	2014-01-06 07:18:52 +0000
@@ -6,6 +6,7 @@
 import (
 	"bytes"
 	"fmt"
+	"io"
 	"regexp"
 	"strconv"
 	"strings"
@@ -24,9 +25,9 @@
 
 // checkProvisioned checks if any juju upstart jobs already
 // exist on the host machine.
-func checkProvisioned(sshHost string) (bool, error) {
-	logger.Infof("Checking if %s is already provisioned", sshHost)
-	cmd := ssh.Command(sshHost, []string{"bash", "-c", utils.ShQuote(checkProvisionedScript)})
+func checkProvisioned(host string) (bool, error) {
+	logger.Infof("Checking if %s is already provisioned", host)
+	cmd := ssh.Command("ubuntu@"+host, []string{"bash", "-c", utils.ShQuote(checkProvisionedScript)}, ssh.NoPasswordAuthentication)
 	var stdout, stderr bytes.Buffer
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
@@ -39,9 +40,9 @@
 	output := strings.TrimSpace(stdout.String())
 	provisioned := len(output) > 0
 	if provisioned {
-		logger.Infof("%s is already provisioned [%q]", sshHost, output)
+		logger.Infof("%s is already provisioned [%q]", host, output)
 	} else {
-		logger.Infof("%s is not provisioned", sshHost)
+		logger.Infof("%s is not provisioned", host)
 	}
 	return provisioned, nil
 }
@@ -49,11 +50,9 @@
 // DetectSeriesAndHardwareCharacteristics detects the OS
 // series and hardware characteristics of the remote machine
 // by connecting to the machine and executing a bash script.
-//
-// The sshHost argument must be a hostname of the form [user@]host.
-func DetectSeriesAndHardwareCharacteristics(sshHost string) (hc instance.HardwareCharacteristics, series string, err error) {
-	logger.Infof("Detecting series and characteristics on %s", sshHost)
-	cmd := ssh.Command(sshHost, []string{"bash"})
+func DetectSeriesAndHardwareCharacteristics(host string) (hc instance.HardwareCharacteristics, series string, err error) {
+	logger.Infof("Detecting series and characteristics on %s", host)
+	cmd := ssh.Command("ubuntu@"+host, []string{"bash"}, ssh.NoPasswordAuthentication)
 	var stdout, stderr bytes.Buffer
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
@@ -136,3 +135,66 @@
 uname -m
 grep MemTotal /proc/meminfo
 cat /proc/cpuinfo`
+
+// InitUbuntuUser adds the ubuntu user if it doesn't
+// already exist, updates its ~/.ssh/authorized_keys,
+// and enables passwordless sudo for it.
+//
+// InitUbuntuUser will initially attempt to login as
+// the ubuntu user, and verify that passwordless sudo
+// is enabled; only if this is false will there be an
+// attempt with the specified login.
+//
+// authorizedKeys may be empty, in which case the file
+// will be created and left empty.
+//
+// stdin and stdout will be used for remote sudo prompts,
+// if the ubuntu user must be created/updated.
+func InitUbuntuUser(host, login, authorizedKeys string, stdin io.Reader, stdout io.Writer) error {
+	logger.Infof("initialising %q, user %q", host, login)
+
+	// To avoid unnecessary prompting for the specified login,
+	// initUbuntuUser will first attempt to ssh to the machine
+	// as "ubuntu" with password authentication disabled, and
+	// ensure that it can use sudo without a password.
+	//
+	// Note that we explicitly do not allocate a PTY, so we
+	// get a failure if sudo prompts.
+	cmd := ssh.Command("ubuntu@"+host, []string{"sudo", "-n", "true"}, ssh.NoPasswordAuthentication)
+	if cmd.Run() == nil {
+		return nil
+	}
+
+	// Failed to login as ubuntu (or passwordless sudo is not enabled).
+	// Use specified login, and execute the initUbuntuScript below.
+	if login != "" {
+		host = login + "@" + host
+	}
+	script := fmt.Sprintf(initUbuntuScript, utils.ShQuote(authorizedKeys))
+	cmd = ssh.Command(host, []string{"sudo", "bash -c " + utils.ShQuote(script)}, ssh.AllocateTTY)
+	var stderr bytes.Buffer
+	cmd.Stdin = stdin
+	cmd.Stdout = stdout // for sudo prompt
+	cmd.Stderr = &stderr
+	if err := cmd.Run(); err != nil {
+		if stderr.Len() != 0 {
+			err = fmt.Errorf("%v (%v)", err, strings.TrimSpace(stderr.String()))
+		}
+		return err
+	}
+	return nil
+}
+
+const initUbuntuScript = `
+set -e
+(id ubuntu &> /dev/null) || useradd -m ubuntu
+umask 0077
+temp=$(mktemp)
+echo 'ubuntu ALL=(ALL) NOPASSWD:ALL' > $temp
+install -m 0440 $temp /etc/sudoers.d/90-juju-ubuntu
+rm $temp
+su ubuntu -c 'install -D -m 0600 /dev/null ~/.ssh/authorized_keys'
+export authorized_keys=%s
+if [ ! -z "$authorized_keys" ]; then
+    su ubuntu -c 'printf "%%s\n" "$authorized_keys" >> ~/.ssh/authorized_keys'
+fi`

=== renamed file 'environs/manual/detection_test.go' => 'environs/manual/init_test.go'
--- environs/manual/detection_test.go	2013-12-19 06:39:26 +0000
+++ environs/manual/init_test.go	2014-01-06 07:18:52 +0000
@@ -12,13 +12,13 @@
 	"launchpad.net/juju-core/testing/testbase"
 )
 
-type detectionSuite struct {
+type initialisationSuite struct {
 	testbase.LoggingSuite
 }
 
-var _ = gc.Suite(&detectionSuite{})
+var _ = gc.Suite(&initialisationSuite{})
 
-func (s *detectionSuite) TestDetectSeries(c *gc.C) {
+func (s *initialisationSuite) TestDetectSeries(c *gc.C) {
 	response := strings.Join([]string{
 		"edgy",
 		"armv4",
@@ -31,7 +31,7 @@
 	c.Assert(series, gc.Equals, "edgy")
 }
 
-func (s *detectionSuite) TestDetectionError(c *gc.C) {
+func (s *initialisationSuite) TestDetectionError(c *gc.C) {
 	scriptResponse := strings.Join([]string{
 		"edgy",
 		"armv4",
@@ -50,7 +50,7 @@
 	c.Assert(hc.String(), gc.Equals, "arch=arm cpu-cores=1 mem=4M")
 }
 
-func (s *detectionSuite) TestDetectHardwareCharacteristics(c *gc.C) {
+func (s *initialisationSuite) TestDetectHardwareCharacteristics(c *gc.C) {
 	tests := []struct {
 		summary        string
 		scriptResponse []string
@@ -112,7 +112,7 @@
 	}
 }
 
-func (s *detectionSuite) TestCheckProvisioned(c *gc.C) {
+func (s *initialisationSuite) TestCheckProvisioned(c *gc.C) {
 	defer installFakeSSH(c, "", "", 0)()
 	provisioned, err := checkProvisioned("example.com")
 	c.Assert(err, gc.IsNil)
@@ -135,3 +135,22 @@
 	_, err = checkProvisioned("example.com")
 	c.Assert(err, gc.ErrorMatches, "exit status 255 \\(non-empty-stderr\\)")
 }
+
+func (s *initialisationSuite) TestInitUbuntuUserNonExisting(c *gc.C) {
+	defer installFakeSSH(c, "", "", 0)() // successful creation of ubuntu user
+	defer installFakeSSH(c, "", "", 1)() // simulate failure of ubuntu@ login
+	err := InitUbuntuUser("testhost", "testuser", "", nil, nil)
+	c.Assert(err, gc.IsNil)
+}
+
+func (s *initialisationSuite) TestInitUbuntuUserExisting(c *gc.C) {
+	defer installFakeSSH(c, "", nil, 0)()
+	InitUbuntuUser("testhost", "testuser", "", nil, nil)
+}
+
+func (s *initialisationSuite) TestInitUbuntuUserError(c *gc.C) {
+	defer installFakeSSH(c, "", []string{"", "failed to create ubuntu user"}, 123)()
+	defer installFakeSSH(c, "", "", 1)() // simulate failure of ubuntu@ login
+	err := InitUbuntuUser("testhost", "testuser", "", nil, nil)
+	c.Assert(err, gc.ErrorMatches, "exit status 123 \\(failed to create ubuntu user\\)")
+}

=== modified file 'environs/manual/provisioner.go'
--- environs/manual/provisioner.go	2013-12-20 09:25:57 +0000
+++ environs/manual/provisioner.go	2014-01-06 07:18:52 +0000
@@ -81,16 +81,26 @@
 		client.Close()
 	}()
 
+	// Create the "ubuntu" user and initialise passwordless sudo. We populate
+	// the ubuntu user's authorized_keys file with the public keys in the current
+	// user's ~/.ssh directory. The authenticationworker will later update the
+	// ubuntu user's authorized_keys.
+	user, host := splitUserHost(args.Host)
+	authorizedKeys, err := config.ReadAuthorizedKeys("")
+	if err := InitUbuntuUser(host, user, authorizedKeys, args.Stdin, args.Stdout); err != nil {
+		return "", err
+	}
+
 	// Generate a unique nonce for the machine.
 	uuid, err := utils.NewUUID()
 	if err != nil {
 		return "", err
 	}
-	instanceId := instance.Id(manualInstancePrefix + hostWithoutUser(args.Host))
+	instanceId := instance.Id(manualInstancePrefix + host)
 	nonce := fmt.Sprintf("%s:%s", instanceId, uuid.String())
 
 	// Inform Juju that the machine exists.
-	machineId, series, arch, err := recordMachineInState(client, args.Host, nonce, instanceId)
+	machineId, series, arch, err := recordMachineInState(client, host, nonce, instanceId)
 	if err != nil {
 		return "", err
 	}
@@ -102,7 +112,7 @@
 	}
 
 	// Finally, provision the machine agent.
-	err = provisionMachineAgent(args.Host, mcfg, args.Stdin, args.Stdout, args.Stderr)
+	err = provisionMachineAgent(host, mcfg, args.Stderr)
 	if err != nil {
 		return machineId, err
 	}
@@ -111,20 +121,18 @@
 	return machineId, nil
 }
 
-func hostWithoutUser(host string) string {
-	hostWithoutUser := host
-	if at := strings.Index(hostWithoutUser, "@"); at != -1 {
-		hostWithoutUser = hostWithoutUser[at+1:]
+func splitUserHost(host string) (string, string) {
+	if at := strings.Index(host, "@"); at != -1 {
+		return host[:at], host[at+1:]
 	}
-	return hostWithoutUser
+	return "", host
 }
 
 func recordMachineInState(
 	client *api.Client, host, nonce string, instanceId instance.Id) (machineId, series, arch string, err error) {
 
 	// First, gather the parameters needed to inject the existing host into state.
-	sshHostWithoutUser := hostWithoutUser(host)
-	if ip := net.ParseIP(sshHostWithoutUser); ip != nil {
+	if ip := net.ParseIP(host); ip != nil {
 		// Do a reverse-lookup on the IP. The IP may not have
 		// a DNS entry, so just log a warning if this fails.
 		names, err := net.LookupAddr(ip.String())
@@ -132,14 +140,14 @@
 			logger.Infof("failed to resolve %v: %v", ip, err)
 		} else {
 			logger.Infof("resolved %v to %v", ip, names)
-			sshHostWithoutUser = names[0]
+			host = names[0]
 		}
 	}
-	addrs, err := instance.HostAddresses(sshHostWithoutUser)
+	addrs, err := instance.HostAddresses(host)
 	if err != nil {
 		return "", "", "", err
 	}
-	logger.Infof("addresses for %v: %v", sshHostWithoutUser, addrs)
+	logger.Infof("addresses for %v: %v", host, addrs)
 
 	provisioned, err := checkProvisioned(host)
 	if err != nil {
@@ -216,7 +224,7 @@
 	return mcfg, nil
 }
 
-func provisionMachineAgent(host string, mcfg *cloudinit.MachineConfig, stdin io.Reader, stdout, stderr io.Writer) error {
+func provisionMachineAgent(host string, mcfg *cloudinit.MachineConfig, stderr io.Writer) error {
 	cloudcfg := coreCloudinit.New()
 	if err := cloudinit.ConfigureJuju(mcfg, cloudcfg); err != nil {
 		return err
@@ -225,10 +233,8 @@
 	// the target machine's existing configuration.
 	cloudcfg.SetAptUpgrade(false)
 	return sshinit.Configure(sshinit.ConfigureParams{
-		Host:   host,
+		Host:   "ubuntu@" + host,
 		Config: cloudcfg,
-		Stdin:  stdin,
-		Stdout: stdout,
 		Stderr: stderr,
 	})
 }

=== modified file 'environs/manual/provisioner_test.go'
--- environs/manual/provisioner_test.go	2013-11-28 05:33:34 +0000
+++ environs/manual/provisioner_test.go	2014-01-06 07:18:52 +0000
@@ -43,7 +43,10 @@
 
 	envtesting.RemoveTools(c, s.Conn.Environ.Storage())
 	defer fakeSSH{
-		Series: series, Arch: arch, SkipProvisionAgent: true,
+		Series:             series,
+		Arch:               arch,
+		InitUbuntuUser:     true,
+		SkipProvisionAgent: true,
 	}.install(c).Restore()
 	// Attempt to provision a machine with no tools available, expect it to fail.
 	machineId, err := ProvisionMachine(args)
@@ -59,8 +62,9 @@
 	for i, errorCode := range []int{255, 0} {
 		c.Logf("test %d: code %d", i, errorCode)
 		defer fakeSSH{
-			Series: series,
-			Arch:   arch,
+			Series:                 series,
+			Arch:                   arch,
+			InitUbuntuUser:         true,
 			ProvisionAgentExitCode: errorCode,
 		}.install(c).Restore()
 		machineId, err = ProvisionMachine(args)
@@ -83,10 +87,21 @@
 
 	// Attempting to provision a machine twice should fail. We effect
 	// this by checking for existing juju upstart configurations.
-	defer installFakeSSH(c, "", "/etc/init/jujud-machine-0.conf", 0)()
+	defer fakeSSH{
+		Provisioned:        true,
+		InitUbuntuUser:     true,
+		SkipDetection:      true,
+		SkipProvisionAgent: true,
+	}.install(c).Restore()
 	_, err = ProvisionMachine(args)
 	c.Assert(err, gc.Equals, ErrProvisioned)
-	defer installFakeSSH(c, "", "/etc/init/jujud-machine-0.conf", 255)()
+	defer fakeSSH{
+		Provisioned:              true,
+		CheckProvisionedExitCode: 255,
+		InitUbuntuUser:           true,
+		SkipDetection:            true,
+		SkipProvisionAgent:       true,
+	}.install(c).Restore()
 	_, err = ProvisionMachine(args)
 	c.Assert(err, gc.ErrorMatches, "error checking if provisioned: exit status 255")
 }
@@ -94,7 +109,11 @@
 func (s *provisionerSuite) TestCreateMachineConfig(c *gc.C) {
 	const series = "precise"
 	const arch = "amd64"
-	defer fakeSSH{Series: series, Arch: arch}.install(c).Restore()
+	defer fakeSSH{
+		Series:         series,
+		Arch:           arch,
+		InitUbuntuUser: true,
+	}.install(c).Restore()
 	machineId, err := ProvisionMachine(s.getArgs(c))
 	c.Assert(err, gc.IsNil)
 

=== modified file 'environs/sshstorage/storage.go'
--- environs/sshstorage/storage.go	2013-12-11 09:34:29 +0000
+++ environs/sshstorage/storage.go	2014-01-06 07:18:52 +0000
@@ -43,12 +43,8 @@
 	scanner *bufio.Scanner
 }
 
-var sshCommand = func(host string, tty bool, command string) *exec.Cmd {
-	var options []ssh.Option
-	if tty {
-		options = append(options, ssh.AllocateTTY)
-	}
-	return ssh.Command(host, []string{command}, options...)
+var sshCommand = func(host string, command string) *exec.Cmd {
+	return ssh.Command(host, []string{command}, ssh.NoPasswordAuthentication)
 }
 
 type flockmode string
@@ -73,13 +69,6 @@
 	// will attempt to reassign ownership to the login user, and will return
 	// an error if it cannot do so.
 	TmpDir string
-
-	// Stdin in required to respond to sudo passwords,
-	// and must be a terminal (except in tests).
-	Stdin io.Reader
-
-	// Stdout is required to present sudo prompts to the user.
-	Stdout io.Writer
 }
 
 // NewSSHStorage creates a new SSHStorage, connected to the
@@ -98,9 +87,7 @@
 		utils.ShQuote(params.TmpDir),
 	)
 
-	cmd := sshCommand(params.Host, true, fmt.Sprintf("sudo bash -c %s", utils.ShQuote(script)))
-	cmd.Stdin = params.Stdin
-	cmd.Stdout = params.Stdout // for sudo prompts/output
+	cmd := sshCommand(params.Host, fmt.Sprintf("sudo -n bash -c %s", utils.ShQuote(script)))
 	var stderr bytes.Buffer
 	cmd.Stderr = &stderr
 	if err := cmd.Run(); err != nil {
@@ -111,7 +98,7 @@
 	// We could use sftp, but then we'd be at the mercy of
 	// sftp's output messages for checking errors. Instead,
 	// we execute an interactive bash shell.
-	cmd = sshCommand(params.Host, false, "bash")
+	cmd = sshCommand(params.Host, "bash")
 	stdin, err := cmd.StdinPipe()
 	if err != nil {
 		return nil, err

=== modified file 'environs/sshstorage/storage_test.go'
--- environs/sshstorage/storage_test.go	2013-12-06 07:02:50 +0000
+++ environs/sshstorage/storage_test.go	2014-01-06 07:18:52 +0000
@@ -30,7 +30,7 @@
 
 var _ = gc.Suite(&storageSuite{})
 
-func sshCommandTesting(host string, tty bool, command string) *exec.Cmd {
+func sshCommandTesting(host string, command string) *exec.Cmd {
 	cmd := exec.Command("bash", "-c", command)
 	uid := fmt.Sprint(os.Getuid())
 	gid := fmt.Sprint(os.Getgid())
@@ -45,8 +45,6 @@
 		Host:       host,
 		StorageDir: storageDir,
 		TmpDir:     tmpDir,
-		Stdin:      &bytes.Buffer{},
-		Stdout:     ioutil.Discard,
 	}
 	return NewSSHStorage(params)
 }
@@ -65,8 +63,8 @@
 	restoreEnv := testbase.PatchEnvironment("PATH", bin+":"+os.Getenv("PATH"))
 	s.AddSuiteCleanup(func(*gc.C) { restoreEnv() })
 
-	// Create a "sudo" command which just executes its args.
-	err = os.Symlink("/usr/bin/env", filepath.Join(bin, "sudo"))
+	// Create a "sudo" command which shifts away the "-n" and executes the remaining args.
+	err = ioutil.WriteFile(filepath.Join(bin, "sudo"), []byte("#!/bin/sh\nshift; exec \"$@\""), 0755)
 	c.Assert(err, gc.IsNil)
 	restoreSshCommand := testbase.PatchValue(&sshCommand, sshCommandTesting)
 	s.AddSuiteCleanup(func(*gc.C) { restoreSshCommand() })
@@ -169,7 +167,7 @@
 	//  3: second "install"
 	//  4: touch
 	var invocations int
-	badSshCommand := func(host string, tty bool, command string) *exec.Cmd {
+	badSshCommand := func(host string, command string) *exec.Cmd {
 		invocations++
 		switch invocations {
 		case 1, 3:

=== modified file 'provider/common/bootstrap.go'
--- provider/common/bootstrap.go	2013-12-20 03:12:26 +0000
+++ provider/common/bootstrap.go	2014-01-06 07:18:52 +0000
@@ -185,8 +185,6 @@
 	return sshinit.Configure(sshinit.ConfigureParams{
 		Host:   "ubuntu@" + addr,
 		Config: cloudcfg,
-		Stdin:  ctx.Stdin(),
-		Stdout: ctx.Stdout(),
 		Stderr: ctx.Stderr(),
 	})
 }
@@ -332,7 +330,9 @@
 // connectSSH is called to connect to the specified host and
 // execute the "checkHostScript" bash script on it.
 var connectSSH = func(host, checkHostScript string) error {
-	cmd := ssh.Command("ubuntu@"+host, []string{"/bin/bash", "-c", utils.ShQuote(checkHostScript)})
+	cmd := ssh.Command("ubuntu@"+host, []string{
+		"/bin/bash", "-c", utils.ShQuote(checkHostScript),
+	}, ssh.NoPasswordAuthentication)
 	output, err := cmd.CombinedOutput()
 	if err != nil && len(output) > 0 {
 		err = fmt.Errorf("%s", strings.TrimSpace(string(output)))

=== modified file 'provider/null/config.go'
--- provider/null/config.go	2013-10-15 05:11:29 +0000
+++ provider/null/config.go	2014-01-06 07:18:52 +0000
@@ -44,14 +44,6 @@
 	return c.attrs["bootstrap-user"].(string)
 }
 
-func (c *environConfig) sshHost() string {
-	host := c.bootstrapHost()
-	if user := c.bootstrapUser(); user != "" {
-		host = user + "@" + host
-	}
-	return host
-}
-
 func (c *environConfig) storageListenIPAddress() string {
 	return c.attrs["storage-listen-ip"].(string)
 }

=== modified file 'provider/null/config_test.go'
--- provider/null/config_test.go	2013-10-09 06:54:15 +0000
+++ provider/null/config_test.go	2014-01-06 07:18:52 +0000
@@ -103,13 +103,11 @@
 	testConfig := getEnvironConfig(c, values)
 	c.Assert(testConfig.bootstrapHost(), gc.Equals, "hostname")
 	c.Assert(testConfig.bootstrapUser(), gc.Equals, "")
-	c.Assert(testConfig.sshHost(), gc.Equals, "hostname")
 	values["bootstrap-host"] = "127.0.0.1"
 	values["bootstrap-user"] = "ubuntu"
 	testConfig = getEnvironConfig(c, values)
 	c.Assert(testConfig.bootstrapHost(), gc.Equals, "127.0.0.1")
 	c.Assert(testConfig.bootstrapUser(), gc.Equals, "ubuntu")
-	c.Assert(testConfig.sshHost(), gc.Equals, "ubuntu@127.0.0.1")
 }
 
 func (s *configSuite) TestStorageParams(c *gc.C) {

=== modified file 'provider/null/environ.go'
--- provider/null/environ.go	2013-12-19 09:15:40 +0000
+++ provider/null/environ.go	2014-01-06 07:18:52 +0000
@@ -48,8 +48,10 @@
 type nullEnviron struct {
 	cfg                   *environConfig
 	cfgmutex              sync.Mutex
-	bootstrapStorage      *sshstorage.SSHStorage
+	bootstrapStorage      storage.Storage
 	bootstrapStorageMutex sync.Mutex
+	ubuntuUserInited      bool
+	ubuntuUserInitMutex   sync.Mutex
 }
 
 var _ environs.BootstrapStorager = (*nullEnviron)(nil)
@@ -85,9 +87,32 @@
 	return e.envConfig().Name()
 }
 
+var initUbuntuUser = manual.InitUbuntuUser
+
+func (e *nullEnviron) ensureBootstrapUbuntuUser(ctx environs.BootstrapContext) error {
+	e.ubuntuUserInitMutex.Lock()
+	defer e.ubuntuUserInitMutex.Unlock()
+	if e.ubuntuUserInited {
+		return nil
+	}
+	cfg := e.envConfig()
+	err := initUbuntuUser(cfg.bootstrapHost(), cfg.bootstrapUser(), cfg.AuthorizedKeys(), ctx.Stdin(), ctx.Stdout())
+	if err != nil {
+		logger.Errorf("initializing ubuntu user: %v", err)
+		return err
+	}
+	logger.Infof("initialized ubuntu user")
+	e.ubuntuUserInited = true
+	return nil
+}
+
 func (e *nullEnviron) Bootstrap(ctx environs.BootstrapContext, cons constraints.Value) error {
+	if err := e.ensureBootstrapUbuntuUser(ctx); err != nil {
+		return err
+	}
 	envConfig := e.envConfig()
-	hc, series, err := manual.DetectSeriesAndHardwareCharacteristics(envConfig.sshHost())
+	host := envConfig.bootstrapHost()
+	hc, series, err := manual.DetectSeriesAndHardwareCharacteristics(host)
 	if err != nil {
 		return err
 	}
@@ -97,7 +122,7 @@
 	}
 	return manual.Bootstrap(manual.BootstrapArgs{
 		Context:                 ctx,
-		Host:                    e.envConfig().sshHost(),
+		Host:                    host,
 		DataDir:                 dataDir,
 		Environ:                 e,
 		PossibleTools:           selectedTools,
@@ -144,6 +169,14 @@
 	return instances, err
 }
 
+var newSSHStorage = func(sshHost, storageDir, storageTmpdir string) (storage.Storage, error) {
+	return sshstorage.NewSSHStorage(sshstorage.NewSSHStorageParams{
+		Host:       sshHost,
+		StorageDir: storageDir,
+		TmpDir:     storageTmpdir,
+	})
+}
+
 // Implements environs.BootstrapStorager.
 func (e *nullEnviron) EnableBootstrapStorage(ctx environs.BootstrapContext) error {
 	e.bootstrapStorageMutex.Lock()
@@ -151,17 +184,13 @@
 	if e.bootstrapStorage != nil {
 		return nil
 	}
+	if err := e.ensureBootstrapUbuntuUser(ctx); err != nil {
+		return err
+	}
 	cfg := e.envConfig()
 	storageDir := e.StorageDir()
 	storageTmpdir := path.Join(dataDir, storageTmpSubdir)
-	params := sshstorage.NewSSHStorageParams{
-		Host:       cfg.sshHost(),
-		StorageDir: storageDir,
-		TmpDir:     storageTmpdir,
-		Stdin:      ctx.Stdin(),
-		Stdout:     ctx.Stdout(),
-	}
-	bootstrapStorage, err := sshstorage.NewSSHStorage(params)
+	bootstrapStorage, err := newSSHStorage("ubuntu@"+cfg.bootstrapHost(), storageDir, storageTmpdir)
 	if err != nil {
 		return err
 	}

=== modified file 'provider/null/environ_test.go'
--- provider/null/environ_test.go	2013-12-20 02:38:56 +0000
+++ provider/null/environ_test.go	2014-01-06 07:18:52 +0000
@@ -1,19 +1,18 @@
-// Copyright 2013 Canonical Ltd.
+// Copyright 2012 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package null
 
 import (
-	"io/ioutil"
-	"os"
-	"path/filepath"
+	"errors"
+	"io"
 	"strings"
 
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/manual"
-	"launchpad.net/juju-core/environs/sshstorage"
+	"launchpad.net/juju-core/environs/storage"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	"launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/instance"
@@ -101,35 +100,39 @@
 	c.Assert(strings.Contains(url, "/tools"), jc.IsTrue)
 }
 
+type dummyStorage struct {
+	storage.Storage
+}
+
 func (s *environSuite) TestEnvironBootstrapStorager(c *gc.C) {
-	var sshScript = `
-#!/bin/bash --norc
-if echo "$*" | grep -q -v sudo; then
-    # We're executing bash inside ssh. Wait
-    # for input to be written before exiting.
-    head -n 1 > /dev/null
-    echo JUJU-RC: $RC
-fi
-`[1:]
-	bin := c.MkDir()
-	ssh := filepath.Join(bin, "ssh")
-	err := ioutil.WriteFile(ssh, []byte(sshScript), 0755)
-	c.Assert(err, gc.IsNil)
-	s.PatchEnvironment("PATH", bin+":"+os.Getenv("PATH"))
-
-	s.PatchEnvironment("RC", "99") // simulate ssh failure
+	var newSSHStorageResult = struct {
+		stor storage.Storage
+		err  error
+	}{dummyStorage{}, errors.New("failed to get SSH storage")}
+	s.PatchValue(&newSSHStorage, func(sshHost, storageDir, storageTmpdir string) (storage.Storage, error) {
+		return newSSHStorageResult.stor, newSSHStorageResult.err
+	})
+
+	var initUbuntuResult error
+	s.PatchValue(&initUbuntuUser, func(host, user, authorizedKeys string, stdin io.Reader, stdout io.Writer) error {
+		return initUbuntuResult
+	})
+
 	ctx := envtesting.NewBootstrapContext(coretesting.Context(c))
-	err = s.env.EnableBootstrapStorage(ctx)
-	c.Assert(err, gc.ErrorMatches, "exit code 99")
-	c.Assert(s.env.Storage(), gc.Not(gc.FitsTypeOf), new(sshstorage.SSHStorage))
-
-	s.PatchEnvironment("RC", "0")
-	err = s.env.EnableBootstrapStorage(ctx)
-	c.Assert(err, gc.IsNil)
-	c.Assert(s.env.Storage(), gc.FitsTypeOf, new(sshstorage.SSHStorage))
-
-	// Check idempotency
-	err = s.env.EnableBootstrapStorage(ctx)
-	c.Assert(err, gc.IsNil)
-	c.Assert(s.env.Storage(), gc.FitsTypeOf, new(sshstorage.SSHStorage))
+	initUbuntuResult = errors.New("failed to initialise ubuntu user")
+	c.Assert(s.env.EnableBootstrapStorage(ctx), gc.Equals, initUbuntuResult)
+	initUbuntuResult = nil
+	c.Assert(s.env.EnableBootstrapStorage(ctx), gc.Equals, newSSHStorageResult.err)
+	// after the user is initialised once successfully,
+	// another attempt will not be made.
+	initUbuntuResult = errors.New("failed to initialise ubuntu user")
+	c.Assert(s.env.EnableBootstrapStorage(ctx), gc.Equals, newSSHStorageResult.err)
+
+	// after the bootstrap storage is initialised once successfully,
+	// another attempt will not be made.
+	backup := newSSHStorageResult.err
+	newSSHStorageResult.err = nil
+	c.Assert(s.env.EnableBootstrapStorage(ctx), gc.IsNil)
+	newSSHStorageResult.err = backup
+	c.Assert(s.env.EnableBootstrapStorage(ctx), gc.IsNil)
 }