Merge lp:~wallyworld/golxc/lxc-console-log into lp:golxc

[Ian Booth]

Rework the method used to detect if lxc-start supports --console-log or --console.

Rework the method used to detect if lxc-start supports --console-log or --console.
Try running lxc-start with --console-log and if that fails, use --console.

As a drive by change lxc args to long form.

[Tim Penhey (thumper)]

We should probably check that the long options that we want to use are in the 0.7.5 version (precise). I think they are... actually, just checked.

lxc-create doesn't support any long options, which is why we don't do it.
lxc-start does seem to support long options.

You are going to need to fix the flags for lxc-create to be short options again.

review: Needs Fixing

[lp:~wallyworld/golxc/lxc-console-log]

13. By Ian Booth on 2014-07-30

Long args not supported for create and destroy for older versions

14. By Ian Booth on 2014-07-30

Remove Infof

15. By Ian Booth on 2014-07-30

Long args not supported for clone for older versions

[Tim Penhey (thumper)]

Thanks.

review: Approve

=== modified file 'export_test.go'
--- export_test.go	2014-07-28 21:56:33 +0000
+++ export_test.go	2014-07-30 01:57:33 +0000
@@ -15,5 +15,3 @@
 	confPath = cp
 	return orig
 }
-
-var GenerateConsoleFileArguments func(string) (string, error) = generateConsoleFileArgument
 
=== modified file 'golxc.go'
--- golxc.go	2014-07-28 22:23:49 +0000
+++ golxc.go	2014-07-30 01:57:33 +0000
@@ -12,7 +12,6 @@
 	"os"
 	"os/exec"
 	"path"
-	"regexp"
 	"strconv"
 	"strings"
 
@@ -227,27 +226,24 @@
 	}
 	args := []string{
 		"--daemon",
-		"-n", c.name,
+		"--name", c.name,
 	}
 	if configFile != "" {
-		args = append(args, "-f", configFile)
-	}
+		args = append(args, "--rcfile", configFile)
+	}
+	if c.logFile != "" {
+		args = append(args, "--logfile", c.logFile, "--logpriority", string(c.logLevel))
+	}
+	startArgs := args
 	if consoleFile != "" {
-		version, err := lxcStartVersion()
-		if err != nil {
-			return fmt.Errorf("cannot determine the lxc-start version: %v", err)
-		}
-
-		consoleFlag, err := generateConsoleFileArgument(version)
-		if err != nil {
-			return fmt.Errorf("cannot determine the flag for specifying a console-output log file: %v", err)
-		}
-		args = append(args, consoleFlag, consoleFile)
-	}
-	if c.logFile != "" {
-		args = append(args, "-o", c.logFile, "-l", string(c.logLevel))
-	}
-	_, err := run("lxc-start", args...)
+		startArgs = append(args, "--console-log", consoleFile)
+	}
+	_, err := run("lxc-start", startArgs...)
+	// We may need to fallback to the older --console arg name.
+	if err != nil && consoleFile != "" {
+		startArgs = append(args, "--console", consoleFile)
+		_, err = run("lxc-start", startArgs...)
+	}
 	if err != nil {
 		return err
 	}
@@ -270,10 +266,10 @@
 		return nil
 	}
 	args := []string{
-		"-n", c.name,
+		"--name", c.name,
 	}
 	if c.logFile != "" {
-		args = append(args, "-o", c.logFile, "-l", string(c.logLevel))
+		args = append(args, "--logfile", c.logFile, "--logpriority", string(c.logLevel))
 	}
 	_, err := run("lxc-stop", args...)
 	if err != nil {
@@ -326,10 +322,10 @@
 		return fmt.Errorf("container %q is not running", c.name)
 	}
 	args := []string{
-		"-n", c.name,
+		"--name", c.name,
 	}
 	if c.logFile != "" {
-		args = append(args, "-o", c.logFile, "-l", string(c.logLevel))
+		args = append(args, "--logfile", c.logFile, "--logpriority", string(c.logLevel))
 	}
 	_, err := run("lxc-freeze", args...)
 	if err != nil {
@@ -347,10 +343,10 @@
 		return fmt.Errorf("container %q is not frozen", c.name)
 	}
 	args := []string{
-		"-n", c.name,
+		"--name", c.name,
 	}
 	if c.logFile != "" {
-		args = append(args, "-o", c.logFile, "-l", string(c.logLevel))
+		args = append(args, "--logfile", c.logFile, "--logpriority", string(c.logLevel))
 	}
 	_, err := run("lxc-unfreeze", args...)
 	if err != nil {
@@ -384,7 +380,7 @@
 		stateStrs[i] = string(state)
 	}
 	waitStates := strings.Join(stateStrs, "|")
-	_, err := run("lxc-wait", "-n", c.name, "-s", waitStates)
+	_, err := run("lxc-wait", "--name", c.name, "--state", waitStates)
 	if err != nil {
 		return err
 	}
@@ -393,7 +389,7 @@
 
 // Info returns the status and the process id of the container.
 func (c *container) Info() (State, int, error) {
-	out, err := run("lxc-info", "-n", c.name)
+	out, err := run("lxc-info", "--name", c.name)
 	if err != nil {
 		return StateUnknown, -1, err
 	}
@@ -444,26 +440,6 @@
 	return path.Join(c.containerHome(), "rootfs")
 }
 
-func generateConsoleFileArgument(lxcVersion string) (flag string, err error) {
-	// Version <= 0.7 specified a console file via the -c flag. Newer
-	// versions utilize the -L flag.
-	if matched, err := regexp.MatchString(`^0\.[0-8]`, lxcVersion); err == nil {
-		if matched {
-			flag = "-c"
-		} else {
-			flag = "-L"
-		}
-	}
-
-	return flag, err
-}
-
-// Returns the lxc-start version
-func lxcStartVersion() (string, error) {
-	version, err := exec.Command("lxc-start", "--version").Output()
-	return string(version), err
-}
-
 // run executes the passed command and returns the out.
 func run(name string, args ...string) (string, error) {
 	logger := loggo.GetLogger(fmt.Sprintf("golxc.run.%s", name))
 
=== modified file 'golxc_test.go'
--- golxc_test.go	2014-07-28 22:24:12 +0000
+++ golxc_test.go	2014-07-30 01:57:33 +0000
@@ -368,21 +368,87 @@
 	s.IsolationSuite.SetUpSuite(c)
 }
 
-// Test that our function which parses versions and returns the proper
-// console flag is working as intended.
-func (s *commandArgs) TestGenerateCorrectConsoleFileFlag(c *C) {
-
-	assertGood := func(desiredFlag string) func(string, error) {
-		return func(flag string, err error) {
-			c.Assert(err, IsNil)
-			c.Assert(flag, Equals, desiredFlag)
-		}
-	}
-
-	assertGood("-c")(golxc.GenerateConsoleFileArguments("0.8"))
-	assertGood("-L")(golxc.GenerateConsoleFileArguments("0.9"))
-	assertGood("-L")(golxc.GenerateConsoleFileArguments("1.8"))
-	assertGood("-L")(golxc.GenerateConsoleFileArguments("1.0.4"))
+func (s *commandArgs) setupLxcStart(c *C) {
+	dir := c.MkDir()
+	// Make the rootfs for the "test" container so it thinks it is created.
+	rootfs := filepath.Join(dir, "test", "rootfs")
+	err := os.MkdirAll(rootfs, 0755)
+	c.Assert(err, IsNil)
+	c.Assert(rootfs, jc.IsDirectory)
+
+	s.PatchValue(&golxc.ContainerDir, dir)
+	s.PatchEnvironment("PATH", s.originalPath)
+	testing.PatchExecutableAsEchoArgs(c, s, "lxc-wait")
+	testing.PatchExecutableAsEchoArgs(c, s, "lxc-start")
+	info := "PID: 1234\nSTATE: RUNNING"
+	testing.PatchExecutable(c, s, "lxc-info", "#!/bin/sh\necho '"+info+"'\n")
+}
+
+func (s *commandArgs) TestStartArgs(c *C) {
+	s.setupLxcStart(c)
+	factory := golxc.Factory()
+	container := factory.New("test")
+	err := container.Start("config-file", "console-log")
+	c.Assert(err, IsNil)
+	testing.AssertEchoArgs(
+		c, "lxc-start",
+		"--daemon",
+		"--name", "test",
+		"--rcfile", "config-file",
+		"--console-log", "console-log")
+}
+
+func (s *commandArgs) TestStartArgsNoConsoleLog(c *C) {
+	s.setupLxcStart(c)
+	factory := golxc.Factory()
+	container := factory.New("test")
+	err := container.Start("config-file", "")
+	c.Assert(err, IsNil)
+	testing.AssertEchoArgs(
+		c, "lxc-start",
+		"--daemon",
+		"--name", "test",
+		"--rcfile", "config-file")
+}
+
+func (s *commandArgs) TestStartArgsFallback(c *C) {
+	dir := c.MkDir()
+	// Make the rootfs for the "test" container so it thinks it is created.
+	rootfs := filepath.Join(dir, "test", "rootfs")
+	err := os.MkdirAll(rootfs, 0755)
+	c.Assert(err, IsNil)
+	c.Assert(rootfs, jc.IsDirectory)
+
+	s.PatchValue(&golxc.ContainerDir, dir)
+	s.PatchEnvironment("PATH", s.originalPath)
+	testing.PatchExecutableAsEchoArgs(c, s, "lxc-wait")
+	EchoQuotedArgs := `
+name=` + "`basename $0`" + `
+argfile="$name.out"
+rm -f $argfile
+printf "%s" $name | tee -a $argfile
+for arg in "$@"; do
+  printf " \"%s\""  "$arg" | tee -a $argfile
+done
+printf "\n" | tee -a $argfile
+`
+	// Make lxc-start error if called with --console-log but succeed otherwise.
+	// On success, echo the args.
+	testing.PatchExecutable(
+		c, s, "lxc-start", "#!/bin/bash\nif [ $6 == '--console-log' ]; then\nexit 1\nelse\n"+EchoQuotedArgs+"fi")
+	info := "PID: 1234\nSTATE: RUNNING"
+	testing.PatchExecutable(c, s, "lxc-info", "#!/bin/sh\necho '"+info+"'\n")
+
+	factory := golxc.Factory()
+	container := factory.New("test")
+	err = container.Start("config-file", "console-log")
+	c.Assert(err, IsNil)
+	testing.AssertEchoArgs(
+		c, "lxc-start",
+		"--daemon",
+		"--name", "test",
+		"--rcfile", "config-file",
+		"--console", "console-log")
 }
 
 func (s *commandArgs) TestCreateArgs(c *C) {