Implement manual bootstrapping

environs/manual/sshresponse_test.go

+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package manual
+
+import (
+        "fmt"
+        "io/ioutil"
+        "os"
+        "path/filepath"
+        "strings"
+
+        gc "launchpad.net/gocheck"
+
+        "launchpad.net/juju-core/testing"
+)
+
+// sshscript should only print the result on the first execution,
+// to handle the case where it's called multiple times. On
+// subsequent executions, it should find the next 'ssh' in $PATH
+// and exec that.
+var sshscript = `#!/bin/bash
+if [ ! -e "$0.run" ]; then
+    touch "$0.run"
+    diff "$0.expected-input" -
+    exitcode=$?
+    if [ $exitcode -ne 0 ]; then
+        echo "ERROR: did not match expected input" >&2
+        exit $exitcode
+    fi
+%s
+    exit %d
+else
+    export PATH=${PATH#*:}
+    exec ssh $*
+fi`
+
+// sshresponse creates a fake "ssh" command in a new $PATH,
+// updates $PATH, and returns a function to reset $PATH to
+// its original value when called.
+func sshresponse(c *gc.C, input, output string, rc int) func() {

[rog, 2013/09/14 07:28:03]

I found these names quite confusing; the respond method doesn't
respond to anything...

How about something like "installFakeSSH" instead instead of sshresponse?

Then the type could be fakeSSH, and the respond method could be named SetUp or
similar.

[axw1, 2013/09/16 04:56:48]

Done.

+        fakebin := c.MkDir()
+        ssh := filepath.Join(fakebin, "ssh")
+        sshexpectedinput := ssh + ".expected-input"
+        if output != "" {
+                output = fmt.Sprintf("cat<<EOF\n%s\nEOF", output)
+        }
+        script := fmt.Sprintf(sshscript, output, rc)
+        err := ioutil.WriteFile(ssh, []byte(script), 0777)
+        c.Assert(err, gc.IsNil)
+        err = ioutil.WriteFile(sshexpectedinput, []byte(input), 0644)
+        c.Assert(err, gc.IsNil)
+        return testing.PatchEnvironment("PATH", fakebin+":"+os.Getenv("PATH"))

[thumper, 2013/09/16 03:02:08]

I've been thinking of better ways to get test code to clean up after itself.

If we had an interface called:
  type TestCleanup interface {
    AddCleanup(func())

we could then have this function take an interface pointer,
and instead of returning a function that the caller needs to defer,
you could just go:

func sshresponse(c *gc.C, cleaner testing.TestCleanup, input, output string, rc
int) {
  // everything else
  cleaner.AddCleanup(testing.PatchEnvironment("PATH",
fakebin+":"+os.Getenv("PATH")))
}

[axw1, 2013/09/16 04:56:48]

That sounds fine. Since there's no way in hell our tests could run in parallel,
we could probably go a step further and make it a top-level function in
juju-core/testing. Then PatchEnvironment itself could be responsible for calling
AddCleanup.

+}
+
+// sshsesponder wraps the invocation of sshresponse based on the parameters.
+type sshresponder struct {
+        series string
+        arch   string
+
+        // exit code for the machine agent provisioning script.
+        provisionAgentExitCode int
+
+        // there are conditions other than error in the above
+        // that might cause provisioning to not go ahead, such
+        // as tools being missing.
+        skipProvisionAgent bool
+}
+
+func (r sshresponder) respond(c *gc.C) func() {

[rog, 2013/09/14 07:28:03]

I'd like to see a doc comment on this please.

[axw1, 2013/09/16 04:56:48]

Done.

+        series := r.series
+        if series == "" {
+                series = "precise"
+        }
+        arch := r.arch
+        if arch == "" {
+                arch = "amd64"
+        }
+        detectionoutput := strings.Join([]string{
+                series,
+                arch,
+                "MemTotal: 4096 kB",
+                "processor: 0",
+        }, "\n")
+        // Responses are elicited conditionally, hence this mess of dynamicity.
+        add := func(oldf func(), input, output string, rc int) func() {
+                f := sshresponse(c, input, output, rc)

[rog, 2013/09/14 07:28:03]

I think things can be simpler here. How about something like:

var restores := []func()
add := func(input, output string, rc int) {
    restore := sshresponse(c, input, output, rc)
    restores = append(restores, restore)
}
if !r.skipProvisionAgent {
    add("", "", r.provisionAgentExitCode)
}
add(detectionScript, detectionoutput, 0)
add(checkProvisionedScript, "", 0)
return func() {
    for i := len(restores)-1; i >= 0; i-- {
        restores[i]()
    }
}


alternatively, you could use jc.Restorer throughout, and
add a method to it:

// Add returns a Restorer that restores first f1
// and then f. It is valid to call this on a nil
// Restorer.
func (f Restorer) Add(f1 Restorer) Restorer {
     return func() { 
         f1.Restore()
         if f != nil {
             f.Restore()
         }
     }
}

then you could do:

var restore jc.Restorer
add := func(input, output string, rc int) {
    restore = restore.Add(sshresponse(c, input, output, rc))
}
if !r.skipProvisionAgent {
    add("", "", r.provisionAgentExitCode)
}
add(detectionScript, detectionoutput, 0)
add(checkProvisionedScript, "", 0)
return restore


On balance I prefer the latter, because the defer
calls can be:

defer sshresponder{...}.respond().Restore()

which is more obvious to my eyes (I had missed the extra set of () in the call).

[axw1, 2013/09/16 04:56:48]

Much nicer, thank you.

+                if oldf != nil {
+                        newf := f
+                        f = func() {
+                                newf()
+                                oldf()
+                        }
+                }
+                return f
+        }
+        var f func()
+        if !r.skipProvisionAgent {
+                f = add(f, "", "", r.provisionAgentExitCode)
+        }
+        f = add(f, detectionScript, detectionoutput, 0)
+        return add(f, checkProvisionedScript, "", 0)
+}