Merge lp:~thumper/juju-core/debug-log-api into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Hook up the debug-log command line to the API.

Have 'juju debug-log' try to use the API first. If that
fails, it falls back to tailing the log over ssh.

https://codereview.appspot.com/85570044/

Hook up the debug-log command line to the API.

Have 'juju debug-log' try to use the API first. If that
fails, it falls back to tailing the log over ssh.

https://codereview.appspot.com/85570044/

[Tim Penhey (thumper)]

Reviewers: mp+215323_code.launchpad.net,

Message:
Please take a look.

Description:
Hook up the debug-log command line to the API.

Have 'juju debug-log' try to use the API first. If that
fails, it falls back to tailing the log over ssh.

https://code.launchpad.net/~thumper/juju-core/debug-log-api/+merge/215323

(do not edit description out of merge proposal)

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

Affected files (+325, -131 lines):
   A [revision details]
   M cmd/args.go
   M cmd/args_test.go
   M cmd/juju/debuglog.go
   M cmd/juju/debuglog_test.go
   M cmd/juju/main.go
   M errors/errors.go
   M errors/errors_test.go
   M state/api/client.go

[Ian Booth (wallyworld)]

LGTM, not sure about the need for a different "api not supported"
mechanism though.

https://codereview.appspot.com/85570044/diff/1/cmd/args.go
File cmd/args.go (right):

https://codereview.appspot.com/85570044/diff/1/cmd/args.go#newcode46
cmd/args.go:46: // f.Var(cmd.NewAppendStringsValue(defaultValue,
&someMember), "name", "help")
The above comment is wrong

https://codereview.appspot.com/85570044/diff/1/cmd/args.go#newcode59
cmd/args.go:59: return fmt.Sprint(*v)
Could we do:

return strings.Join(*v, ",")

to be consistent with StringsValue

https://codereview.appspot.com/85570044/diff/1/cmd/args_test.go
File cmd/args_test.go (right):

https://codereview.appspot.com/85570044/diff/1/cmd/args_test.go#newcode161
cmd/args_test.go:161: message: "no value set by args",
I don't understand the message. Shouldn't it be "value set by args"?

https://codereview.appspot.com/85570044/diff/1/cmd/juju/debuglog.go
File cmd/juju/debuglog.go (right):

https://codereview.appspot.com/85570044/diff/1/cmd/juju/debuglog.go#newcode74
cmd/juju/debuglog.go:74: c.level, loggo.TRACE, loggo.DEBUG, loggo.INFO,
loggo.WARNING, loggo.ERROR)
Would be nice if there were a loggo helper ValidLevels()

https://codereview.appspot.com/85570044/diff/1/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/85570044/diff/1/state/api/client.go#newcode773
state/api/client.go:773: return nil,
errors.NewNotSupportedError("WatchDebugLog")
Till now, CodeNotImplemented has been used to flag an incompatible API
call. I'm not sure of the value of introducing something new here,
instead of just going with what's already been done.

https://codereview.appspot.com/85570044/

[Tim Penhey (thumper)]

https://codereview.appspot.com/85570044/diff/1/cmd/args.go
File cmd/args.go (right):

https://codereview.appspot.com/85570044/diff/1/cmd/args.go#newcode46
cmd/args.go:46: // f.Var(cmd.NewAppendStringsValue(defaultValue,
&someMember), "name", "help")
On 2014/04/11 00:54:43, wallyworld wrote:
> The above comment is wrong

Done.

https://codereview.appspot.com/85570044/diff/1/cmd/args.go#newcode59
cmd/args.go:59: return fmt.Sprint(*v)
On 2014/04/11 00:54:43, wallyworld wrote:
> Could we do:

> return strings.Join(*v, ",")

> to be consistent with StringsValue

Done.

https://codereview.appspot.com/85570044/diff/1/cmd/args_test.go
File cmd/args_test.go (right):

https://codereview.appspot.com/85570044/diff/1/cmd/args_test.go#newcode161
cmd/args_test.go:161: message: "no value set by args",
On 2014/04/11 00:54:43, wallyworld wrote:
> I don't understand the message. Shouldn't it be "value set by args"?

yes

https://codereview.appspot.com/85570044/diff/1/state/api/client.go
File state/api/client.go (right):

https://codereview.appspot.com/85570044/diff/1/state/api/client.go#newcode773
state/api/client.go:773: return nil,
errors.NewNotSupportedError("WatchDebugLog")
On 2014/04/11 00:54:43, wallyworld wrote:
> Till now, CodeNotImplemented has been used to flag an incompatible API
call. I'm
> not sure of the value of introducing something new here, instead of
just going
> with what's already been done.

Talked with Roger about this last night and we agreed to change it to
this.

https://codereview.appspot.com/85570044/

[Go Bot (go-bot)]

The attempt to merge lp:~thumper/juju-core/debug-log-api into lp:juju-core failed. Below is the output from the failed tests.

ok launchpad.net/juju-core 0.014s
ok launchpad.net/juju-core/agent 1.181s
ok launchpad.net/juju-core/agent/mongo 1.129s
ok launchpad.net/juju-core/agent/tools 0.186s
ok launchpad.net/juju-core/bzr 5.118s
ok launchpad.net/juju-core/cert 2.939s
ok launchpad.net/juju-core/charm 0.411s
? launchpad.net/juju-core/charm/hooks [no test files]
? launchpad.net/juju-core/charm/testing [no test files]
ok launchpad.net/juju-core/cloudinit 0.027s
ok launchpad.net/juju-core/cloudinit/sshinit 0.745s
ok launchpad.net/juju-core/cmd 0.171s
ok launchpad.net/juju-core/cmd/charm-admin 0.765s
? launchpad.net/juju-core/cmd/charmd [no test files]
? launchpad.net/juju-core/cmd/charmload [no test files]
ok launchpad.net/juju-core/cmd/envcmd 0.182s
ok launchpad.net/juju-core/cmd/juju 221.263s
ok launchpad.net/juju-core/cmd/jujud 79.785s
ok launchpad.net/juju-core/cmd/plugins/juju-metadata 9.623s
? launchpad.net/juju-core/cmd/plugins/juju-restore [no test files]
ok launchpad.net/juju-core/cmd/plugins/local 0.209s
? launchpad.net/juju-core/cmd/plugins/local/juju-local [no test files]
ok launchpad.net/juju-core/constraints 0.023s
ok launchpad.net/juju-core/container 0.038s
ok launchpad.net/juju-core/container/factory 0.041s
ok launchpad.net/juju-core/container/kvm 0.219s
ok launchpad.net/juju-core/container/kvm/mock 0.032s
? launchpad.net/juju-core/container/kvm/testing [no test files]
ok launchpad.net/juju-core/container/lxc 3.328s
? launchpad.net/juju-core/container/lxc/mock [no test files]
? launchpad.net/juju-core/container/lxc/testing [no test files]
? launchpad.net/juju-core/container/testing [no test files]
ok launchpad.net/juju-core/downloader 5.247s
ok launchpad.net/juju-core/environs 2.270s
ok launchpad.net/juju-core/environs/bootstrap 11.102s
ok launchpad.net/juju-core/environs/cloudinit 0.448s
ok launchpad.net/juju-core/environs/config 1.969s
ok launchpad.net/juju-core/environs/configstore 0.033s
ok launchpad.net/juju-core/environs/filestorage 0.027s
ok launchpad.net/juju-core/environs/httpstorage 0.768s
ok launchpad.net/juju-core/environs/imagemetadata 0.523s
? launchpad.net/juju-core/environs/imagemetadata/testing [no test files]
ok launchpad.net/juju-core/environs/instances 0.041s
ok launchpad.net/juju-core/environs/jujutest 0.242s
ok launchpad.net/juju-core/environs/manual 10.860s
ok launchpad.net/juju-core/environs/simplestreams 0.282s
? launchpad.net/juju-core/environs/simplestreams/testing [no test files]
ok launchpad.net/juju-core/environs/sshstorage 0.918s
ok launchpad.net/juju-core/environs/storage 0.916s
ok launchpad.net/juju-core/environs/sync 48.850s
ok launchpad.net/juju-core/environs/testing 0.121s
ok launchpad.net/juju-core/environs/tools 4.561s
? launchpad.net/juju-core/environs/tools/testing [no test files]
ok launchpad.net/juju-core/errors 0.011s
ok launchpad.net/juju-core/instance 0.018s
? launchpad.net/juju-core/instance/testing [no test files]
ok launchpad.net/juju-core/juju 20.543s
ok l...

=== modified file 'cmd/args.go'
--- cmd/args.go	2014-01-05 22:53:25 +0000
+++ cmd/args.go	2014-04-11 01:07:04 +0000
@@ -34,3 +34,26 @@
 func (v *StringsValue) String() string {
 	return strings.Join(*v, ",")
 }
+
+// AppendStringsValue implements gnuflag.Value for a value that can be set
+// multiple times, and it appends each value to the slice.
+type AppendStringsValue []string
+
+var _ gnuflag.Value = (*AppendStringsValue)(nil)
+
+// NewAppendStringsValue is used to create the type passed into the gnuflag.FlagSet Var function.
+// f.Var(cmd.NewAppendStringsValue(&someMember), "name", "help")
+func NewAppendStringsValue(target *[]string) *AppendStringsValue {
+	return (*AppendStringsValue)(target)
+}
+
+// Implements gnuflag.Value Set.
+func (v *AppendStringsValue) Set(s string) error {
+	*v = append(*v, s)
+	return nil
+}
+
+// Implements gnuflag.Value String.
+func (v *AppendStringsValue) String() string {
+	return strings.Join(*v, ",")
+}

=== modified file 'cmd/args_test.go'
--- cmd/args_test.go	2014-01-05 22:53:25 +0000
+++ cmd/args_test.go	2014-04-11 01:07:04 +0000
@@ -149,3 +149,26 @@
 		c.Assert(value.String(), gc.Equals, test.expected)
 	}
 }
+
+func (*ArgsSuite) TestAppendStringsUsage(c *gc.C) {
+	for i, test := range []struct {
+		message       string
+		args          []string
+		expectedValue []string
+	}{{
+		message: "no args",
+	}, {
+		message:       "value set by args",
+		args:          []string{"--value", "foo", "--value=bar"},
+		expectedValue: []string{"foo", "bar"},
+	}} {
+		c.Log(fmt.Sprintf("%v: %s", i, test.message))
+		f := gnuflag.NewFlagSet("test", gnuflag.ContinueOnError)
+		f.SetOutput(ioutil.Discard)
+		var value []string
+		f.Var(cmd.NewAppendStringsValue(&value), "value", "help")
+		err := f.Parse(false, test.args)
+		c.Check(err, gc.IsNil)
+		c.Check(value, gc.DeepEquals, test.expectedValue)
+	}
+}

=== modified file 'cmd/juju/debuglog.go'
--- cmd/juju/debuglog.go	2013-12-13 02:18:39 +0000
+++ cmd/juju/debuglog.go	2014-04-11 01:07:04 +0000
@@ -1,64 +1,38 @@
-// Copyright 2013 Canonical Ltd.
+// Copyright 2013, 2014 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package main
 
 import (
 	"fmt"
-	"strconv"
+	"io"
 
+	"github.com/juju/loggo"
 	"launchpad.net/gnuflag"
 
 	"launchpad.net/juju-core/cmd"
+	"launchpad.net/juju-core/cmd/envcmd"
+	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/juju"
+	"launchpad.net/juju-core/state/api"
 )
 
 type DebugLogCommand struct {
-	cmd.CommandBase
-	// The debug log command simply invokes juju ssh with the required arguments.
-	sshCmd cmd.Command
-	lines  linesValue
+	envcmd.EnvCommandBase
+
+	level  string
+	params api.DebugLogParams
 }
 
+var DefaultLogLocation = "/var/log/juju/all-machines.log"
+
 // defaultLineCount is the default number of lines to
 // display, from the end of the consolidated log.
 const defaultLineCount = 10
 
-// linesValue implements gnuflag.Value, and represents
-// a -n/--lines flag value compatible with "tail".
-//
-// A negative value (-K) corresponds to --lines=K,
-// i.e. the last K lines; a positive value (+K)
-// corresponds to --lines=+K, i.e. from line K onwards.
-type linesValue int
-
-func (v *linesValue) String() string {
-	if *v > 0 {
-		return fmt.Sprintf("+%d", *v)
-	}
-	return fmt.Sprint(-*v)
-}
-
-func (v *linesValue) Set(value string) error {
-	if len(value) > 0 {
-		sign := -1
-		if value[0] == '+' {
-			value = value[1:]
-			sign = 1
-		}
-		n, err := strconv.ParseInt(value, 10, 0)
-		if err == nil && n > 0 {
-			*v = linesValue(sign * int(n))
-			return nil
-		}
-		// err is quite verbose, and doesn't convey
-		// any additional useful information.
-	}
-	return fmt.Errorf("invalid number of lines")
-}
-
 const debuglogDoc = `
-Launch an ssh shell on the state server machine and tail the consolidated log file.
-The consolidated log file contains log messages from all nodes in the environment.
+Stream the consolidated debug log file. This file contains the log messages
+from all nodes in the environment.
 `
 
 func (c *DebugLogCommand) Info() *cmd.Info {
@@ -70,27 +44,86 @@
 }
 
 func (c *DebugLogCommand) SetFlags(f *gnuflag.FlagSet) {
-	c.sshCmd.SetFlags(f)
-
-	c.lines = -defaultLineCount
-	f.Var(&c.lines, "n", "output the last K lines; or use -n +K to output lines starting with the Kth")
-	f.Var(&c.lines, "lines", "")
-}
-
-func (c *DebugLogCommand) AllowInterspersedFlags() bool {
-	return true
+	c.EnvCommandBase.SetFlags(f)
+
+	f.Var(cmd.NewAppendStringsValue(&c.params.IncludeEntity), "i", "only show log messages for these entities")
+	f.Var(cmd.NewAppendStringsValue(&c.params.IncludeEntity), "include", "only show log messages for these entities")
+	f.Var(cmd.NewAppendStringsValue(&c.params.ExcludeEntity), "x", "only show log messages for these entities")
+	f.Var(cmd.NewAppendStringsValue(&c.params.ExcludeEntity), "exclude", "only show log messages for these entities")
+	f.Var(cmd.NewAppendStringsValue(&c.params.IncludeModule), "include-module", "only show log messages for these logging modules")
+	f.Var(cmd.NewAppendStringsValue(&c.params.ExcludeModule), "exclude-module", "do not show log messages for these logging modules")
+
+	f.StringVar(&c.level, "l", "", "log level to show, one of [TRACE, DEBUG, INFO, WARNING, ERROR]")
+	f.StringVar(&c.level, "level", "", "")
+
+	f.UintVar(&c.params.Backlog, "n", defaultLineCount, "go back this many lines from the end before starting to filter")
+	f.UintVar(&c.params.Backlog, "lines", defaultLineCount, "")
+	f.UintVar(&c.params.Limit, "limit", 0, "show at most this many lines")
+	f.BoolVar(&c.params.Replay, "replay", false, "start filtering from the start")
 }
 
 func (c *DebugLogCommand) Init(args []string) error {
-	tailcmd := fmt.Sprintf("tail -n %s -f /var/log/juju/all-machines.log", &c.lines)
-	args = append([]string{"0"}, args...)
-	args = append(args, tailcmd)
-	return c.sshCmd.Init(args)
-}
-
-// Run uses "juju ssh" to log into the state server node
-// and tails the consolidated log file which captures log
-// messages from all nodes.
-func (c *DebugLogCommand) Run(ctx *cmd.Context) error {
-	return c.sshCmd.Run(ctx)
+	err := c.EnvCommandBase.Init()
+	if err != nil {
+		return err
+	}
+	if c.level != "" {
+		level, ok := loggo.ParseLevel(c.level)
+		if !ok || level < loggo.TRACE || level > loggo.ERROR {
+			return fmt.Errorf("level value %q is not one of %q, %q, %q, %q, %q",
+				c.level, loggo.TRACE, loggo.DEBUG, loggo.INFO, loggo.WARNING, loggo.ERROR)
+		}
+		c.params.Level = level
+	}
+	return cmd.CheckEmpty(args)
+}
+
+type DebugLogAPI interface {
+	WatchDebugLog(params api.DebugLogParams) (io.ReadCloser, error)
+	Close() error
+}
+
+var getDebugLogAPI = func(envName string) (DebugLogAPI, error) {
+	return juju.NewAPIClientFromName(envName)
+}
+
+// Run retrieves the debug log via the API.
+func (c *DebugLogCommand) Run(ctx *cmd.Context) (err error) {
+	client, err := getDebugLogAPI(c.EnvName)
+	if err != nil {
+		return err
+	}
+	defer client.Close()
+	debugLog, err := client.WatchDebugLog(c.params)
+	if err != nil {
+		if errors.IsNotSupportedError(err) {
+			return c.watchDebugLog1dot18(ctx)
+		}
+		return err
+	}
+	defer debugLog.Close()
+	_, err = io.Copy(ctx.Stdout, debugLog)
+	return err
+}
+
+var runSSHCommand = func(sshCmd *SSHCommand, ctx *cmd.Context) error {
+	return sshCmd.Run(ctx)
+}
+
+// watchDebugLog1dot18 runs in case of an older API server and uses ssh
+// but with server-side grep.
+func (c *DebugLogCommand) watchDebugLog1dot18(ctx *cmd.Context) error {
+	ctx.Infof("Server does not support new stream log, falling back to tail")
+	ctx.Verbosef("filters are not supported with tail")
+	sshCmd := &SSHCommand{}
+	tailCmd := fmt.Sprintf("tail -n -%d -f %s", c.params.Backlog, DefaultLogLocation)
+	// If the api doesn't support WatchDebugLog, then it won't be running in
+	// HA either, so machine 0 is where it is all at.
+	args := []string{"0", tailCmd}
+	err := sshCmd.Init(args)
+	if err != nil {
+		return err
+	}
+	sshCmd.EnvName = c.EnvName
+	return runSSHCommand(sshCmd, ctx)
 }

=== modified file 'cmd/juju/debuglog_test.go'
--- cmd/juju/debuglog_test.go	2014-03-11 09:29:33 +0000
+++ cmd/juju/debuglog_test.go	2014-04-11 01:07:04 +0000
@@ -4,72 +4,165 @@
 package main
 
 import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"strings"
+
+	"github.com/juju/loggo"
+	jc "github.com/juju/testing/checkers"
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/cmd"
+	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/testing"
 )
 
 type DebugLogSuite struct {
+	testing.FakeHomeSuite
 }
 
 var _ = gc.Suite(&DebugLogSuite{})
 
-func runDebugLog(c *gc.C, args ...string) (*DebugLogCommand, error) {
-	cmd := &DebugLogCommand{
-		sshCmd: &dummySSHCommand{},
-	}
-	_, err := testing.RunCommand(c, cmd, args)
-	return cmd, err
-}
-
-type dummySSHCommand struct {
-	SSHCommand
-	runCalled bool
-}
-
-func (c *dummySSHCommand) Run(ctx *cmd.Context) error {
-	c.runCalled = true
+func (s *DebugLogSuite) TestArgParsing(c *gc.C) {
+	for i, test := range []struct {
+		args     []string
+		expected api.DebugLogParams
+		errMatch string
+	}{
+		{
+			expected: api.DebugLogParams{
+				Backlog: 10,
+			},
+		}, {
+			args: []string{"-n0"},
+		}, {
+			args: []string{"--lines=50"},
+			expected: api.DebugLogParams{
+				Backlog: 50,
+			},
+		}, {
+			args:     []string{"-l", "foo"},
+			errMatch: `level value "foo" is not one of "TRACE", "DEBUG", "INFO", "WARNING", "ERROR"`,
+		}, {
+			args: []string{"--level=INFO"},
+			expected: api.DebugLogParams{
+				Backlog: 10,
+				Level:   loggo.INFO,
+			},
+		}, {
+			args: []string{"--include", "machine-1", "-i", "machine-2"},
+			expected: api.DebugLogParams{
+				IncludeEntity: []string{"machine-1", "machine-2"},
+				Backlog:       10,
+			},
+		}, {
+			args: []string{"--exclude", "machine-1", "-x", "machine-2"},
+			expected: api.DebugLogParams{
+				ExcludeEntity: []string{"machine-1", "machine-2"},
+				Backlog:       10,
+			},
+		}, {
+			args: []string{"--include-module", "juju.foo", "--include-module", "unit"},
+			expected: api.DebugLogParams{
+				IncludeModule: []string{"juju.foo", "unit"},
+				Backlog:       10,
+			},
+		}, {
+			args: []string{"--exclude-module", "juju.foo", "--exclude-module", "unit"},
+			expected: api.DebugLogParams{
+				ExcludeModule: []string{"juju.foo", "unit"},
+				Backlog:       10,
+			},
+		}, {
+			args: []string{"--replay"},
+			expected: api.DebugLogParams{
+				Backlog: 10,
+				Replay:  true,
+			},
+		}, {
+			args: []string{"--limit", "100"},
+			expected: api.DebugLogParams{
+				Backlog: 10,
+				Limit:   100,
+			},
+		},
+	} {
+		c.Logf("test %v", i)
+		command := &DebugLogCommand{}
+		err := testing.InitCommand(command, test.args)
+		if test.errMatch == "" {
+			c.Check(err, gc.IsNil)
+			c.Check(command.params, jc.DeepEquals, test.expected)
+		} else {
+			c.Check(err, gc.ErrorMatches, test.errMatch)
+		}
+	}
+}
+
+func (s *DebugLogSuite) TestParamsPassed(c *gc.C) {
+	fake := &fakeDebugLogAPI{}
+	s.PatchValue(&getDebugLogAPI, func(envName string) (DebugLogAPI, error) {
+		return fake, nil
+	})
+	_, err := testing.RunCommand(c, &DebugLogCommand{}, []string{
+		"-i", "machine-1*", "-x", "machine-1-lxc-1",
+		"--include-module=juju.provisioner",
+		"--lines=500",
+		"--level=WARNING",
+	})
+	c.Assert(err, gc.IsNil)
+	c.Assert(fake.params, gc.DeepEquals, api.DebugLogParams{
+		IncludeEntity: []string{"machine-1*"},
+		IncludeModule: []string{"juju.provisioner"},
+		ExcludeEntity: []string{"machine-1-lxc-1"},
+		Backlog:       500,
+		Level:         loggo.WARNING,
+	})
+}
+
+func (s *DebugLogSuite) TestLogOutput(c *gc.C) {
+	s.PatchValue(&getDebugLogAPI, func(envName string) (DebugLogAPI, error) {
+		return &fakeDebugLogAPI{log: "this is the log output"}, nil
+	})
+	ctx, err := testing.RunCommand(c, &DebugLogCommand{}, nil)
+	c.Assert(err, gc.IsNil)
+	c.Assert(testing.Stdout(ctx), gc.Equals, "this is the log output")
+}
+
+func (s *DebugLogSuite) TestTailFallback(c *gc.C) {
+	s.PatchValue(&runSSHCommand, func(sshCmd *SSHCommand, ctx *cmd.Context) error {
+		fmt.Fprintf(ctx.Stdout, "%s", sshCmd.Args)
+		return nil
+	})
+	s.PatchValue(&getDebugLogAPI, func(envName string) (DebugLogAPI, error) {
+		return &fakeDebugLogAPI{err: errors.NewNotSupportedError("testing")}, nil
+	})
+	ctx, err := testing.RunCommand(c, &DebugLogCommand{}, []string{"-n", "100"})
+	c.Assert(err, gc.IsNil)
+	c.Check(testing.Stderr(ctx), gc.Equals, "Server does not support new stream log, falling back to tail\n")
+	c.Check(testing.Stdout(ctx), gc.Equals, "[tail -n -100 -f /var/log/juju/all-machines.log]")
+}
+
+func newFakeDebugLogAPI(log string) DebugLogAPI {
+	return &fakeDebugLogAPI{log: log}
+}
+
+type fakeDebugLogAPI struct {
+	log    string
+	params api.DebugLogParams
+	err    error
+}
+
+func (fake *fakeDebugLogAPI) WatchDebugLog(params api.DebugLogParams) (io.ReadCloser, error) {
+	if fake.err != nil {
+		return nil, fake.err
+	}
+	fake.params = params
+	return ioutil.NopCloser(strings.NewReader(fake.log)), nil
+}
+
+func (fake *fakeDebugLogAPI) Close() error {
 	return nil
 }
-
-// debug-log is implemented by invoking juju ssh with the correct arguments.
-// This test helper checks for the expected invocation.
-func (s *DebugLogSuite) assertDebugLogInvokesSSHCommand(c *gc.C, expected string, args ...string) {
-	defer testing.MakeSampleHome(c).Restore()
-	debugLogCmd, err := runDebugLog(c, args...)
-	c.Assert(err, gc.IsNil)
-	debugCmd := debugLogCmd.sshCmd.(*dummySSHCommand)
-	c.Assert(debugCmd.runCalled, gc.Equals, true)
-	c.Assert(debugCmd.Target, gc.Equals, "0")
-	c.Assert([]string{expected}, gc.DeepEquals, debugCmd.Args)
-}
-
-const logLocation = "/var/log/juju/all-machines.log"
-
-func (s *DebugLogSuite) TestDebugLog(c *gc.C) {
-	const expected = "tail -n 10 -f " + logLocation
-	s.assertDebugLogInvokesSSHCommand(c, expected)
-}
-
-func (s *DebugLogSuite) TestDebugLogFrom(c *gc.C) {
-	const expected = "tail -n +1 -f " + logLocation
-	s.assertDebugLogInvokesSSHCommand(c, expected, "-n", "+1")
-	s.assertDebugLogInvokesSSHCommand(c, expected, "--lines=+1")
-}
-
-func (s *DebugLogSuite) TestDebugLogLast(c *gc.C) {
-	const expected = "tail -n 100 -f " + logLocation
-	s.assertDebugLogInvokesSSHCommand(c, expected, "-n", "100")
-	s.assertDebugLogInvokesSSHCommand(c, expected, "--lines=100")
-}
-
-func (s *DebugLogSuite) TestDebugLogValidation(c *gc.C) {
-	defer testing.MakeSampleHome(c).Restore()
-	_, err := runDebugLog(c, "-n", "0")
-	c.Assert(err, gc.ErrorMatches, "invalid value \"0\" for flag -n: invalid number of lines")
-	_, err = runDebugLog(c, "-n", "-1")
-	c.Assert(err, gc.ErrorMatches, "invalid value \"-1\" for flag -n: invalid number of lines")
-	_, err = runDebugLog(c, "-n", "fnord")
-	c.Assert(err, gc.ErrorMatches, "invalid value \"fnord\" for flag -n: invalid number of lines")
-}

=== modified file 'cmd/juju/main.go'
--- cmd/juju/main.go	2014-03-31 00:12:13 +0000
+++ cmd/juju/main.go	2014-04-11 01:07:04 +0000
@@ -96,7 +96,7 @@
 	jujucmd.Register(wrap(&SCPCommand{}))
 	jujucmd.Register(wrap(&SSHCommand{}))
 	jujucmd.Register(wrap(&ResolvedCommand{}))
-	jujucmd.Register(wrap(&DebugLogCommand{sshCmd: &SSHCommand{}}))
+	jujucmd.Register(wrap(&DebugLogCommand{}))
 	jujucmd.Register(wrap(&DebugHooksCommand{}))
 	jujucmd.Register(wrap(&RetryProvisioningCommand{}))
 

=== modified file 'errors/errors.go'
--- errors/errors.go	2014-04-08 10:20:15 +0000
+++ errors/errors.go	2014-04-11 01:07:04 +0000
@@ -127,3 +127,25 @@
 	_, ok := err.(*alreadyExistsError)
 	return ok
 }
+
+type notSupportedError struct {
+	what string
+}
+
+// NewNotSupportedError returns an error signifying that something is not
+// supported.  For example a client API call to a server that does not support
+// the action.
+func NewNotSupportedError(what string) error {
+	return &notSupportedError{what: what}
+}
+
+func (e *notSupportedError) Error() string {
+	return e.what + " not supported"
+}
+
+// IsNotSupportedError reports whether the error
+// was created with NewNotSupportedError.
+func IsNotSupportedError(err error) bool {
+	_, ok := err.(*notSupportedError)
+	return ok
+}

=== modified file 'errors/errors_test.go'
--- errors/errors_test.go	2014-04-08 10:20:15 +0000
+++ errors/errors_test.go	2014-04-11 01:07:04 +0000
@@ -38,11 +38,13 @@
 	isUnauthorizedError := &errorSatisfier{errors.IsUnauthorizedError}
 	isNotImplementedError := &errorSatisfier{errors.IsNotImplementedError}
 	isAlreadyExistsError := &errorSatisfier{errors.IsAlreadyExistsError}
+	isNotSupportedError := &errorSatisfier{errors.IsNotSupportedError}
 	satisfiers := []*errorSatisfier{
 		isNotFoundError,
 		isUnauthorizedError,
 		isNotImplementedError,
 		isAlreadyExistsError,
+		isNotSupportedError,
 	}
 
 	// make some errors, and record the errorSatsifier
@@ -88,6 +90,10 @@
 		errors.NewAlreadyExistsError("something"),
 		"something already exists",
 		isAlreadyExistsError,
+	}, {
+		errors.NewNotSupportedError("something"),
+		"something not supported",
+		isNotSupportedError,
 	}}
 
 	for i, t := range errorTests {

=== modified file 'state/api/client.go'
--- state/api/client.go	2014-04-10 10:00:50 +0000
+++ state/api/client.go	2014-04-11 01:07:04 +0000
@@ -20,6 +20,7 @@
 
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/errors"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state/api/params"
 	"launchpad.net/juju-core/tools"
@@ -725,16 +726,6 @@
 	return websocket.DialConfig(config)
 }
 
-type connectionError struct {
-	error
-}
-
-// IsConnectionError reports whether the error is a connection error.
-func IsConnectionError(err error) bool {
-	_, ok := err.(*connectionError)
-	return ok
-}
-
 // DebugLogParams holds parameters for WatchDebugLog that control the
 // filtering of the log messages. If the structure is zero initialized, the
 // entire log file is sent back starting from the end, and until the user
@@ -772,14 +763,14 @@
 // WatchDebugLog returns a ReadCloser that the caller can read the log lines
 // from. Only log lines that match the filtering specified in the
 // DebugLogParams are returned. It returns an error that satisfies
-// IsConnectionError when the connection cannot be made.
+// IsNotSupportedError when the API server does not support the end-point.
 func (c *Client) WatchDebugLog(args DebugLogParams) (io.ReadCloser, error) {
 	// The websocket connection just hangs if the server doesn't have the log
 	// end point. So do a version check, as version was added at the same time
 	// as the remote end point.
 	_, err := c.AgentVersion()
 	if err != nil {
-		return nil, &connectionError{fmt.Errorf("server doesn't support debug log websocket")}
+		return nil, errors.NewNotSupportedError("WatchDebugLog")
 	}
 	// Prepare URL.
 	attrs := url.Values{}
@@ -811,7 +802,7 @@
 	cfg.TlsConfig = &tls.Config{RootCAs: c.st.certPool, ServerName: "anything"}
 	connection, err := websocketDialConfig(cfg)
 	if err != nil {
-		return nil, &connectionError{err}
+		return nil, err
 	}
 	// Read the initial error and translate to a real error.
 	// Read up to the first new line character. We can't use bufio here as it