Merge lp:~axwalk/juju-core/lp1027876-implement-debug-hooks into lp:~go-bot/juju-core/trunk
- lp1027876-implement-debug-hooks
- Merge into trunk
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Andrew Wilkins | ||||
| Approved revision: | no longer in the source branch. | ||||
| Merged at revision: | 1596 | ||||
| Proposed branch: | lp:~axwalk/juju-core/lp1027876-implement-debug-hooks | ||||
| Merge into: | lp:~go-bot/juju-core/trunk | ||||
| Diff against target: |
943 lines (+781/-26) 13 files modified
cmd/juju/debughooks.go (+106/-0) cmd/juju/debughooks_test.go (+89/-0) cmd/juju/main.go (+2/-1) cmd/juju/main_test.go (+1/-0) cmd/juju/scp.go (+1/-2) cmd/juju/ssh.go (+16/-6) worker/uniter/context.go (+31/-17) worker/uniter/debug/client.go (+120/-0) worker/uniter/debug/client_test.go (+47/-0) worker/uniter/debug/common.go (+35/-0) worker/uniter/debug/common_test.go (+30/-0) worker/uniter/debug/server.go (+114/-0) worker/uniter/debug/server_test.go (+189/-0) |
||||
| To merge this branch: | bzr merge lp:~axwalk/juju-core/lp1027876-implement-debug-hooks | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Juju Engineering | Pending | ||
|
Review via email:
|
|||
Commit message
Support juju debug-hooks
This implementation is a departure from pyjuju, as we do not
have Zookeeper's ephemeral nodes to rely upon for locking.
The approach taken is described here:
https:/
Rather than periodically checking tmux, I've just gone for
a simple approach of calling "tmux has-session" each time a
hook is to be executed. If this proves to be too expensive,
then reassess.
Description of the change
Support juju debug-hooks
This implementation is a departure from pyjuju, as we do not
have Zookeeper's ephemeral nodes to rely upon for locking.
The approach taken is described here:
https:/
Rather than periodically checking tmux, I've just gone for
a simple approach of calling "tmux has-session" each time a
hook is to be executed. If this proves to be too expensive,
then reassess.
| Andrew Wilkins (axwalk) wrote : | # |
| William Reade (fwereade) wrote : | # |
Thanks, this is looking really nice; I have a number of thoughts below,
of varying actionability (and perhaps sanity). Let me know what you
think.
https:/
File cmd/juju/
https:/
cmd/juju/
hook %q", unit.Name(), hook)
Would you check whether the python includes handling for, say,
"db-relation-*"? If so we should preserve it.
https:/
cmd/juju/
"StrictHostKeyC
"--"}
This looks awfully familiar. Can a useful set of common args be
extracted?
https:/
File cmd/juju/
https:/
cmd/juju/
Local convention would lean somewhat towards:
var debugHooksTests = []struct{
...
}{{
args: []string{
result: regexp.
}, {
...
}}
... ie tightening the indentation slightly and explicitly naming every
non-zero field.
https:/
cmd/juju/
hook everything.
I'd add these comments as an "info" field, and log them -- I find it
really helps when diagnosing surprising failures.
https:/
cmd/juju/
t.args)
Again by approximate mild convention:
for i, test := range debugHooksTests {
c.Logf("test %d: %s\n%s", i, test.info, test.args)
...might be neatest. The "testing juju debug-hooks" bit is somewhat
redundant in the light of the test name, but the args are clearly
interesting enough to log..
https:/
File worker/
https:/
worker/
I'd prefer to call this plain "debug".
https:/
worker/
It's big and distracting, and not really related to this CL; but, as you
hit packages using log it's nice to switch them over to loggo as you do.
A followup doing so would be much appreciated.
https:/
worker/
maybe pull this block out into a runCharmHook() method?
https:/
File worker/
https:/
worker/
ClientScript(hooks []string) ...
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| Andrew Wilkins (axwalk) wrote : | # |
https:/
File cmd/juju/
https:/
cmd/juju/
hook %q", unit.Name(), hook)
On 2013/07/31 02:02:07, fwereade wrote:
> Would you check whether the python includes handling for, say,
"db-relation-*"?
> If so we should preserve it.
It didn't before: it's just '*' by itself (i.e. the default), or a named
hook without wildcards. I think it makes sense to support it, but I'd
rather leave it till another changeset.
https:/
cmd/juju/
"StrictHostKeyC
"--"}
On 2013/07/31 02:02:07, fwereade wrote:
> This looks awfully familiar. Can a useful set of common args be
extracted?
Done. I've modified it to defer to SSHCommand, and modified
SSHCommand.Run to check if Conn is already initialised.
https:/
File cmd/juju/
https:/
cmd/juju/
On 2013/07/31 02:02:07, fwereade wrote:
> Local convention would lean somewhat towards:
> var debugHooksTests = []struct{
> ...
> }{{
> args: []string{
> result: regexp.
> }, {
> ...
> }}
> ... ie tightening the indentation slightly and explicitly naming every
non-zero
> field.
Done.
https:/
cmd/juju/
hook everything.
On 2013/07/31 02:02:07, fwereade wrote:
> I'd add these comments as an "info" field, and log them -- I find it
really
> helps when diagnosing surprising failures.
Done.
https:/
cmd/juju/
t.args)
On 2013/07/31 02:02:07, fwereade wrote:
> Again by approximate mild convention:
> for i, test := range debugHooksTests {
> c.Logf("test %d: %s\n%s", i, test.info, test.args)
> ...might be neatest. The "testing juju debug-hooks" bit is somewhat
redundant in
> the light of the test name, but the args are clearly interesting
enough to log..
Done.
https:/
File worker/
https:/
worker/
On 2013/07/31 02:02:07, fwereade wrote:
> I'd prefer to call this plain "debug".
Done.
https:/
worker/
On 2013/07/31 02:02:07, fwereade wrote:
> It's big and distracting, and not really related to this CL; but, as
you hit
> packages using log it's nice to switch them over to loggo as you do. A
followup
> doing...
| Ian Booth (wallyworld) wrote : | # |
LGTM, but I am not at all familiar with Juju's debug hooks. Not sure if
we would prefer another opinion. If it works, I say just land it and we
can iterate.
https:/
File cmd/juju/
https:/
cmd/juju/
Please group imports
https:/
File worker/
https:/
worker/
convention is to use err for errors
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| Andrew Wilkins (axwalk) wrote : | # |
https:/
File cmd/juju/
https:/
cmd/juju/
On 2013/08/02 00:09:17, wallyworld wrote:
> Please group imports
Done.
https:/
File worker/
https:/
worker/
On 2013/08/02 00:09:17, wallyworld wrote:
> convention is to use err for errors
This is existing code, I just moved it. I was going to change it to err
anyway, but there's a reason for it being named differently: see the
conditional assignment of "err = e" below.
| William Reade (fwereade) wrote : | # |
LGTM, just a couple of trivials.
https:/
File cmd/juju/
https:/
cmd/juju/
/bin/bash -c '%s'", fmt.Sprintf(
. $F`, script))}
Could we break the inner Sprintf out please?
https:/
File cmd/juju/
https:/
cmd/juju/
nonexistent
https:/
cmd/juju/
"machines" would be nicer than "m"
https:/
cmd/juju/
dummy := s.AddTestingCha
https:/
File cmd/juju/ssh.go (right):
https:/
cmd/juju/ssh.go:84: c.Conn, err = juju.NewConnFro
If we're doing this, how about adding it to EnvCommandBase instead?
Almost every command that uses an env also needs to do this.
| Andrew Wilkins (axwalk) wrote : | # |
https:/
File cmd/juju/
https:/
cmd/juju/
/bin/bash -c '%s'", fmt.Sprintf(
. $F`, script))}
On 2013/08/02 09:31:30, fwereade wrote:
> Could we break the inner Sprintf out please?
Done.
https:/
File cmd/juju/
https:/
cmd/juju/
On 2013/08/02 09:31:30, fwereade wrote:
> nonexistent
Done.
https:/
cmd/juju/
On 2013/08/02 09:31:30, fwereade wrote:
> "machines" would be nicer than "m"
Done.
https:/
cmd/juju/
On 2013/08/02 09:31:30, fwereade wrote:
> dummy := s.AddTestingCha
Done.
https:/
cmd/juju/
On 2013/08/02 09:31:30, fwereade wrote:
> dummy := s.AddTestingCha
Done.
https:/
File cmd/juju/ssh.go (right):
https:/
cmd/juju/ssh.go:84: c.Conn, err = juju.NewConnFro
On 2013/08/02 09:31:30, fwereade wrote:
> If we're doing this, how about adding it to EnvCommandBase instead?
Almost every
> command that uses an env also needs to do this.
I don't think that it's particularly valuable. SSHCommand is special,
because it's reusing the Conn field initialised by an enclosing command.
The reason for the initConn method is to document whose responsibility
it is to close the connection, and make people aware that the conn is
stored and reused.
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| William Reade (fwereade) wrote : | # |
LGTM, thanks.
https:/
File cmd/juju/ssh.go (right):
https:/
cmd/juju/ssh.go:84: c.Conn, err = juju.NewConnFro
On 2013/08/02 10:06:26, axw1 wrote:
> On 2013/08/02 09:31:30, fwereade wrote:
> > If we're doing this, how about adding it to EnvCommandBase instead?
Almost
> every
> > command that uses an env also needs to do this.
> I don't think that it's particularly valuable. SSHCommand is special,
because
> it's reusing the Conn field initialised by an enclosing command.
> The reason for the initConn method is to document whose responsibility
it is to
> close the connection, and make people aware that the conn is stored
and reused.
Fair enough, SGTM.
| Go Bot (go-bot) wrote : | # |
Attempt to merge into lp:juju-core failed due to conflicts:
text conflict in cmd/juju/main.go
Preview Diff
| 1 | === added file 'cmd/juju/debughooks.go' |
| 2 | --- cmd/juju/debughooks.go 1970-01-01 00:00:00 +0000 |
| 3 | +++ cmd/juju/debughooks.go 2013-08-05 01:28:25 +0000 |
| 4 | @@ -0,0 +1,106 @@ |
| 5 | +// Copyright 2012, 2013 Canonical Ltd. |
| 6 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 7 | + |
| 8 | +package main |
| 9 | + |
| 10 | +import ( |
| 11 | + "encoding/base64" |
| 12 | + "errors" |
| 13 | + "fmt" |
| 14 | + |
| 15 | + "launchpad.net/juju-core/charm/hooks" |
| 16 | + "launchpad.net/juju-core/cmd" |
| 17 | + "launchpad.net/juju-core/state" |
| 18 | + unitdebug "launchpad.net/juju-core/worker/uniter/debug" |
| 19 | +) |
| 20 | + |
| 21 | +// DebugHooksCommand is responsible for launching a ssh shell on a given unit or machine. |
| 22 | +type DebugHooksCommand struct { |
| 23 | + SSHCommand |
| 24 | + hooks []string |
| 25 | +} |
| 26 | + |
| 27 | +const debugHooksDoc = ` |
| 28 | +Interactively debug a hook remotely on a service unit. |
| 29 | +` |
| 30 | + |
| 31 | +func (c *DebugHooksCommand) Info() *cmd.Info { |
| 32 | + return &cmd.Info{ |
| 33 | + Name: "debug-hooks", |
| 34 | + Args: "<unit name> [hook names]", |
| 35 | + Purpose: "launch a tmux session to debug a hook", |
| 36 | + Doc: debugHooksDoc, |
| 37 | + } |
| 38 | +} |
| 39 | + |
| 40 | +func (c *DebugHooksCommand) Init(args []string) error { |
| 41 | + if len(args) < 1 { |
| 42 | + return errors.New("no unit name specified") |
| 43 | + } |
| 44 | + c.Target = args[0] |
| 45 | + |
| 46 | + // If any of the hooks is "*", then debug all hooks. |
| 47 | + c.hooks = append([]string{}, args[1:]...) |
| 48 | + for _, h := range c.hooks { |
| 49 | + if h == "*" { |
| 50 | + c.hooks = nil |
| 51 | + break |
| 52 | + } |
| 53 | + } |
| 54 | + return nil |
| 55 | +} |
| 56 | + |
| 57 | +func (c *DebugHooksCommand) validateHooks(unit *state.Unit) error { |
| 58 | + if len(c.hooks) == 0 { |
| 59 | + return nil |
| 60 | + } |
| 61 | + service, err := unit.Service() |
| 62 | + if err != nil { |
| 63 | + return err |
| 64 | + } |
| 65 | + eps, err := service.Endpoints() |
| 66 | + if err != nil { |
| 67 | + return err |
| 68 | + } |
| 69 | + validHooks := make(map[string]bool) |
| 70 | + for _, hook := range hooks.UnitHooks() { |
| 71 | + validHooks[string(hook)] = true |
| 72 | + } |
| 73 | + for _, ep := range eps { |
| 74 | + for _, hook := range hooks.RelationHooks() { |
| 75 | + hook := fmt.Sprintf("%s-%s", ep.Relation.Name, hook) |
| 76 | + validHooks[hook] = true |
| 77 | + } |
| 78 | + } |
| 79 | + for _, hook := range c.hooks { |
| 80 | + if !validHooks[hook] { |
| 81 | + return fmt.Errorf("unit %q does not contain hook %q", unit.Name(), hook) |
| 82 | + } |
| 83 | + } |
| 84 | + return nil |
| 85 | +} |
| 86 | + |
| 87 | +// Run ensures c.Target is a unit, and resolves its address, |
| 88 | +// and connects to it via SSH to execute the debug-hooks |
| 89 | +// script. |
| 90 | +func (c *DebugHooksCommand) Run(ctx *cmd.Context) error { |
| 91 | + conn, err := c.initConn() |
| 92 | + if err != nil { |
| 93 | + return err |
| 94 | + } |
| 95 | + defer conn.Close() |
| 96 | + unit, err := conn.State.Unit(c.Target) |
| 97 | + if err != nil { |
| 98 | + return err |
| 99 | + } |
| 100 | + err = c.validateHooks(unit) |
| 101 | + if err != nil { |
| 102 | + return err |
| 103 | + } |
| 104 | + debugctx := unitdebug.NewHooksContext(c.Target) |
| 105 | + script := base64.StdEncoding.EncodeToString([]byte(unitdebug.ClientScript(debugctx, c.hooks))) |
| 106 | + innercmd := fmt.Sprintf(`F=$(mktemp); echo %s | base64 -d > $F; . $F`, script) |
| 107 | + args := []string{"--", fmt.Sprintf("sudo /bin/bash -c '%s'", innercmd)} |
| 108 | + c.Args = args |
| 109 | + return c.SSHCommand.Run(ctx) |
| 110 | +} |
| 111 | |
| 112 | === added file 'cmd/juju/debughooks_test.go' |
| 113 | --- cmd/juju/debughooks_test.go 1970-01-01 00:00:00 +0000 |
| 114 | +++ cmd/juju/debughooks_test.go 2013-08-05 01:28:25 +0000 |
| 115 | @@ -0,0 +1,89 @@ |
| 116 | +// Copyright 2012, 2013 Canonical Ltd. |
| 117 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 118 | + |
| 119 | +package main |
| 120 | + |
| 121 | +import ( |
| 122 | + "bytes" |
| 123 | + "regexp" |
| 124 | + |
| 125 | + . "launchpad.net/gocheck" |
| 126 | + |
| 127 | + "launchpad.net/juju-core/cmd" |
| 128 | + coretesting "launchpad.net/juju-core/testing" |
| 129 | +) |
| 130 | + |
| 131 | +var _ = Suite(&DebugHooksSuite{}) |
| 132 | + |
| 133 | +type DebugHooksSuite struct { |
| 134 | + SSHCommonSuite |
| 135 | +} |
| 136 | + |
| 137 | +const debugHooksArgs = `-l ubuntu -t ` + commonArgs |
| 138 | + |
| 139 | +var debugHooksTests = []struct { |
| 140 | + info string |
| 141 | + args []string |
| 142 | + code int |
| 143 | + result string |
| 144 | + stderr string |
| 145 | +}{{ |
| 146 | + args: []string{"mysql/0"}, |
| 147 | + result: regexp.QuoteMeta(debugHooksArgs + "dummyenv-0.dns -- sudo /bin/bash -c 'F=$(mktemp); echo IyEvYmluL2Jhc2gKKAojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnIGxvY2tmaWxlLgpmbG9jayAtbiA4IHx8IChlY2hvICJGYWlsZWQgdG8gYWNxdWlyZSAvdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzOiB1bml0IGlzIGFscmVhZHkgYmVpbmcgZGVidWdnZWQiIDI+JjE7IGV4aXQgMSkKKAojIENsb3NlIHRoZSBpbmhlcml0ZWQgbG9jayBGRCwgb3IgdG11eCB3aWxsIGtlZXAgaXQgb3Blbi4KZXhlYyA4PiYtCgojIFdyaXRlIG91dCB0aGUgZGVidWctaG9va3MgYXJncy4KZWNobyAiZTMwSyIgfCBiYXNlNjQgLWQgPiAvdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzCgojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnLWV4aXQgbG9ja2ZpbGUuCmZsb2NrIC1uIDkgfHwgZXhpdCAxCgojIFdhaXQgZm9yIHRtdXggdG8gYmUgaW5zdGFsbGVkLgp3aGlsZSBbICEgLWYgL3Vzci9iaW4vdG11eCBdOyBkbwogICAgc2xlZXAgMQpkb25lCgppZiBbICEgLWYgfi8udG11eC5jb25mIF07IHRoZW4KICAgICAgICBpZiBbIC1mIC91c3Ivc2hhcmUvYnlvYnUvcHJvZmlsZXMvdG11eCBdOyB0aGVuCiAgICAgICAgICAgICAgICAjIFVzZSBieW9idS90bXV4IHByb2ZpbGUgZm9yIGZhbWlsaWFyIGtleWJpbmRpbmdzIGFuZCBicmFuZGluZwogICAgICAgICAgICAgICAgZWNobyAic291cmNlLWZpbGUgL3Vzci9zaGFyZS9ieW9idS9wcm9maWxlcy90bXV4IiA+IH4vLnRtdXguY29uZgogICAgICAgIGVsc2UKICAgICAgICAgICAgICAgICMgT3RoZXJ3aXNlLCB1c2UgdGhlIGxlZ2FjeSBqdWp1L3RtdXggY29uZmlndXJhdGlvbgogICAgICAgICAgICAgICAgY2F0ID4gfi8udG11eC5jb25mIDw8RU5ECiAgICAgICAgICAgICAgICAKIyBTdGF0dXMgYmFyCnNldC1vcHRpb24gLWcgc3RhdHVzLWJnIGJsYWNrCnNldC1vcHRpb24gLWcgc3RhdHVzLWZnIHdoaXRlCgpzZXQtd2luZG93LW9wdGlvbiAtZyB3aW5kb3ctc3RhdHVzLWN1cnJlbnQtYmcgcmVkCnNldC13aW5kb3ctb3B0aW9uIC1nIHdpbmRvdy1zdGF0dXMtY3VycmVudC1hdHRyIGJyaWdodAoKc2V0LW9wdGlvbiAtZyBzdGF0dXMtcmlnaHQgJycKCiMgUGFuZXMKc2V0LW9wdGlvbiAtZyBwYW5lLWJvcmRlci1mZyB3aGl0ZQpzZXQtb3B0aW9uIC1nIHBhbmUtYWN0aXZlLWJvcmRlci1mZyB3aGl0ZQoKIyBNb25pdG9yIGFjdGl2aXR5IG9uIHdpbmRvd3MKc2V0LXdpbmRvdy1vcHRpb24gLWcgbW9uaXRvci1hY3Rpdml0eSBvbgoKIyBTY3JlZW4gYmluZGluZ3MsIHNpbmNlIHBlb3BsZSBhcmUgbW9yZSBmYW1pbGlhciB3aXRoIHRoYXQuCnNldC1vcHRpb24gLWcgcHJlZml4IEMtYQpiaW5kIEMtYSBsYXN0LXdpbmRvdwpiaW5kIGEgc2VuZC1rZXkgQy1hCgpiaW5kIHwgc3BsaXQtd2luZG93IC1oCmJpbmQgLSBzcGxpdC13aW5kb3cgLXYKCiMgRml4IENUUkwtUEdVUC9QR0RPV04gZm9yIHZpbQpzZXQtd2luZG93LW9wdGlvbiAtZyB4dGVybS1rZXlzIG9uCgojIFByZXZlbnQgRVNDIGtleSBmcm9tIGFkZGluZyBkZWxheSBhbmQgYnJlYWtpbmcgVmltJ3MgRVNDID4gYXJyb3cga2V5CnNldC1vcHRpb24gLXMgZXNjYXBlLXRpbWUgMAoKRU5ECiAgICAgICAgZmkKZmkKCigKICAgICMgQ2xvc2UgdGhlIGluaGVyaXRlZCBsb2NrIEZELCBvciB0bXV4IHdpbGwga2VlcCBpdCBvcGVuLgogICAgZXhlYyA5PiYtCiAgICBleGVjIHRtdXggbmV3LXNlc3Npb24gLXMgbXlzcWwvMAopCikgOT4vdG1wL2p1anUtdW5pdC1teXNxbC0wLWRlYnVnLWhvb2tzLWV4aXQKKSA4Pi90bXAvanVqdS11bml0LW15c3FsLTAtZGVidWctaG9va3MKZXhpdCAkPwo= | base64 -d > $F; . $F'\n"), |
| 148 | +}, { |
| 149 | + args: []string{"mongodb/1"}, |
| 150 | + result: regexp.QuoteMeta(debugHooksArgs + "dummyenv-2.dns -- sudo /bin/bash -c 'F=$(mktemp); echo IyEvYmluL2Jhc2gKKAojIExvY2sgdGhlIGp1anUtPHVuaXQ+LWRlYnVnIGxvY2tmaWxlLgpmbG9jayAtbiA4IHx8IChlY2hvICJGYWlsZWQgdG8gYWNxdWlyZSAvdG1wL2p1anUtdW5pdC1tb25nb2RiLTEtZGVidWctaG9va3M6IHVuaXQgaXMgYWxyZWFkeSBiZWluZyBkZWJ1Z2dlZCIgMj4mMTsgZXhpdCAxKQooCiMgQ2xvc2UgdGhlIGluaGVyaXRlZCBsb2NrIEZELCBvciB0bXV4IHdpbGwga2VlcCBpdCBvcGVuLgpleGVjIDg+Ji0KCiMgV3JpdGUgb3V0IHRoZSBkZWJ1Zy1ob29rcyBhcmdzLgplY2hvICJlMzBLIiB8IGJhc2U2NCAtZCA+IC90bXAvanVqdS11bml0LW1vbmdvZGItMS1kZWJ1Zy1ob29rcwoKIyBMb2NrIHRoZSBqdWp1LTx1bml0Pi1kZWJ1Zy1leGl0IGxvY2tmaWxlLgpmbG9jayAtbiA5IHx8IGV4aXQgMQoKIyBXYWl0IGZvciB0bXV4IHRvIGJlIGluc3RhbGxlZC4Kd2hpbGUgWyAhIC1mIC91c3IvYmluL3RtdXggXTsgZG8KICAgIHNsZWVwIDEKZG9uZQoKaWYgWyAhIC1mIH4vLnRtdXguY29uZiBdOyB0aGVuCiAgICAgICAgaWYgWyAtZiAvdXNyL3NoYXJlL2J5b2J1L3Byb2ZpbGVzL3RtdXggXTsgdGhlbgogICAgICAgICAgICAgICAgIyBVc2UgYnlvYnUvdG11eCBwcm9maWxlIGZvciBmYW1pbGlhciBrZXliaW5kaW5ncyBhbmQgYnJhbmRpbmcKICAgICAgICAgICAgICAgIGVjaG8gInNvdXJjZS1maWxlIC91c3Ivc2hhcmUvYnlvYnUvcHJvZmlsZXMvdG11eCIgPiB+Ly50bXV4LmNvbmYKICAgICAgICBlbHNlCiAgICAgICAgICAgICAgICAjIE90aGVyd2lzZSwgdXNlIHRoZSBsZWdhY3kganVqdS90bXV4IGNvbmZpZ3VyYXRpb24KICAgICAgICAgICAgICAgIGNhdCA+IH4vLnRtdXguY29uZiA8PEVORAogICAgICAgICAgICAgICAgCiMgU3RhdHVzIGJhcgpzZXQtb3B0aW9uIC1nIHN0YXR1cy1iZyBibGFjawpzZXQtb3B0aW9uIC1nIHN0YXR1cy1mZyB3aGl0ZQoKc2V0LXdpbmRvdy1vcHRpb24gLWcgd2luZG93LXN0YXR1cy1jdXJyZW50LWJnIHJlZApzZXQtd2luZG93LW9wdGlvbiAtZyB3aW5kb3ctc3RhdHVzLWN1cnJlbnQtYXR0ciBicmlnaHQKCnNldC1vcHRpb24gLWcgc3RhdHVzLXJpZ2h0ICcnCgojIFBhbmVzCnNldC1vcHRpb24gLWcgcGFuZS1ib3JkZXItZmcgd2hpdGUKc2V0LW9wdGlvbiAtZyBwYW5lLWFjdGl2ZS1ib3JkZXItZmcgd2hpdGUKCiMgTW9uaXRvciBhY3Rpdml0eSBvbiB3aW5kb3dzCnNldC13aW5kb3ctb3B0aW9uIC1nIG1vbml0b3ItYWN0aXZpdHkgb24KCiMgU2NyZWVuIGJpbmRpbmdzLCBzaW5jZSBwZW9wbGUgYXJlIG1vcmUgZmFtaWxpYXIgd2l0aCB0aGF0LgpzZXQtb3B0aW9uIC1nIHByZWZpeCBDLWEKYmluZCBDLWEgbGFzdC13aW5kb3cKYmluZCBhIHNlbmQta2V5IEMtYQoKYmluZCB8IHNwbGl0LXdpbmRvdyAtaApiaW5kIC0gc3BsaXQtd2luZG93IC12CgojIEZpeCBDVFJMLVBHVVAvUEdET1dOIGZvciB2aW0Kc2V0LXdpbmRvdy1vcHRpb24gLWcgeHRlcm0ta2V5cyBvbgoKIyBQcmV2ZW50IEVTQyBrZXkgZnJvbSBhZGRpbmcgZGVsYXkgYW5kIGJyZWFraW5nIFZpbSdzIEVTQyA+IGFycm93IGtleQpzZXQtb3B0aW9uIC1zIGVzY2FwZS10aW1lIDAKCkVORAogICAgICAgIGZpCmZpCgooCiAgICAjIENsb3NlIHRoZSBpbmhlcml0ZWQgbG9jayBGRCwgb3IgdG11eCB3aWxsIGtlZXAgaXQgb3Blbi4KICAgIGV4ZWMgOT4mLQogICAgZXhlYyB0bXV4IG5ldy1zZXNzaW9uIC1zIG1vbmdvZGIvMQopCikgOT4vdG1wL2p1anUtdW5pdC1tb25nb2RiLTEtZGVidWctaG9va3MtZXhpdAopIDg+L3RtcC9qdWp1LXVuaXQtbW9uZ29kYi0xLWRlYnVnLWhvb2tzCmV4aXQgJD8K | base64 -d > $F; . $F'\n"), |
| 151 | +}, { |
| 152 | + info: `"*" is a valid hook name: it means hook everything`, |
| 153 | + args: []string{"mysql/0", "*"}, |
| 154 | + result: ".*\n", |
| 155 | +}, { |
| 156 | + info: `"*" mixed with named hooks is equivalent to "*"`, |
| 157 | + args: []string{"mysql/0", "*", "relation-get"}, |
| 158 | + result: ".*\n", |
| 159 | +}, { |
| 160 | + info: `multiple named hooks may be specified`, |
| 161 | + args: []string{"mysql/0", "start", "stop"}, |
| 162 | + result: ".*\n", |
| 163 | +}, { |
| 164 | + info: `relation hooks have the relation name prefixed`, |
| 165 | + args: []string{"mysql/0", "juju-info-relation-joined"}, |
| 166 | + result: ".*\n", |
| 167 | +}, { |
| 168 | + info: `invalid unit syntax`, |
| 169 | + args: []string{"mysql"}, |
| 170 | + code: 1, |
| 171 | + stderr: `error: "mysql" is not a valid unit name` + "\n", |
| 172 | +}, { |
| 173 | + info: `invalid unit`, |
| 174 | + args: []string{"nonexistent/123"}, |
| 175 | + code: 1, |
| 176 | + stderr: `error: unit "nonexistent/123" not found` + "\n", |
| 177 | +}, { |
| 178 | + info: `invalid hook`, |
| 179 | + args: []string{"mysql/0", "invalid-hook"}, |
| 180 | + code: 1, |
| 181 | + stderr: `error: unit "mysql/0" does not contain hook "invalid-hook"` + "\n", |
| 182 | +}} |
| 183 | + |
| 184 | +func (s *DebugHooksSuite) TestDebugHooksCommand(c *C) { |
| 185 | + machines := s.makeMachines(3, c) |
| 186 | + dummy := s.AddTestingCharm(c, "dummy") |
| 187 | + srv, err := s.State.AddService("mysql", dummy) |
| 188 | + c.Assert(err, IsNil) |
| 189 | + s.addUnit(srv, machines[0], c) |
| 190 | + |
| 191 | + srv, err = s.State.AddService("mongodb", dummy) |
| 192 | + c.Assert(err, IsNil) |
| 193 | + s.addUnit(srv, machines[1], c) |
| 194 | + s.addUnit(srv, machines[2], c) |
| 195 | + |
| 196 | + for i, t := range debugHooksTests { |
| 197 | + c.Logf("test %d: %s\n\t%s\n", i, t.info, t.args) |
| 198 | + ctx := coretesting.Context(c) |
| 199 | + code := cmd.Main(&DebugHooksCommand{}, ctx, t.args) |
| 200 | + c.Check(code, Equals, t.code) |
| 201 | + c.Check(ctx.Stderr.(*bytes.Buffer).String(), Matches, t.stderr) |
| 202 | + c.Check(ctx.Stdout.(*bytes.Buffer).String(), Matches, t.result) |
| 203 | + } |
| 204 | +} |
| 205 | |
| 206 | === modified file 'cmd/juju/main.go' |
| 207 | --- cmd/juju/main.go 2013-07-30 23:46:30 +0000 |
| 208 | +++ cmd/juju/main.go 2013-08-05 01:28:25 +0000 |
| 209 | @@ -66,11 +66,12 @@ |
| 210 | jujucmd.Register(&StatusCommand{}) |
| 211 | jujucmd.Register(&SwitchCommand{}) |
| 212 | |
| 213 | - // Error resolution commands. |
| 214 | + // Error resolution and debugging commands. |
| 215 | jujucmd.Register(&SCPCommand{}) |
| 216 | jujucmd.Register(&SSHCommand{}) |
| 217 | jujucmd.Register(&ResolvedCommand{}) |
| 218 | jujucmd.Register(&DebugLogCommand{sshCmd: &SSHCommand{}}) |
| 219 | + jujucmd.Register(&DebugHooksCommand{}) |
| 220 | |
| 221 | // Configuration commands. |
| 222 | jujucmd.Register(&InitCommand{}) |
| 223 | |
| 224 | === modified file 'cmd/juju/main_test.go' |
| 225 | --- cmd/juju/main_test.go 2013-08-02 02:34:12 +0000 |
| 226 | +++ cmd/juju/main_test.go 2013-08-05 01:28:25 +0000 |
| 227 | @@ -218,6 +218,7 @@ |
| 228 | "add-relation", |
| 229 | "add-unit", |
| 230 | "bootstrap", |
| 231 | + "debug-hooks", |
| 232 | "debug-log", |
| 233 | "deploy", |
| 234 | "destroy-environment", |
| 235 | |
| 236 | === modified file 'cmd/juju/scp.go' |
| 237 | --- cmd/juju/scp.go 2013-05-02 15:55:42 +0000 |
| 238 | +++ cmd/juju/scp.go 2013-08-05 01:28:25 +0000 |
| 239 | @@ -9,7 +9,6 @@ |
| 240 | "strings" |
| 241 | |
| 242 | "launchpad.net/juju-core/cmd" |
| 243 | - "launchpad.net/juju-core/juju" |
| 244 | ) |
| 245 | |
| 246 | // SCPCommand is responsible for launching a scp command to copy files to/from remote machine(s) |
| 247 | @@ -39,7 +38,7 @@ |
| 248 | // forks ssh with c.Args, if provided. |
| 249 | func (c *SCPCommand) Run(ctx *cmd.Context) error { |
| 250 | var err error |
| 251 | - c.Conn, err = juju.NewConnFromName(c.EnvName) |
| 252 | + c.Conn, err = c.initConn() |
| 253 | if err != nil { |
| 254 | return err |
| 255 | } |
| 256 | |
| 257 | === modified file 'cmd/juju/ssh.go' |
| 258 | --- cmd/juju/ssh.go 2013-07-30 23:46:30 +0000 |
| 259 | +++ cmd/juju/ssh.go 2013-08-05 01:28:25 +0000 |
| 260 | @@ -21,7 +21,7 @@ |
| 261 | SSHCommon |
| 262 | } |
| 263 | |
| 264 | -// SSHCommon provides common methods for SSHCommand and SCPCommand. |
| 265 | +// SSHCommon provides common methods for SSHCommand, SCPCommand and DebugHooksCommand. |
| 266 | type SSHCommon struct { |
| 267 | cmd.EnvCommandBase |
| 268 | Target string |
| 269 | @@ -55,12 +55,14 @@ |
| 270 | // Run resolves c.Target to a machine, to the address of a i |
| 271 | // machine or unit forks ssh passing any arguments provided. |
| 272 | func (c *SSHCommand) Run(ctx *cmd.Context) error { |
| 273 | - var err error |
| 274 | - c.Conn, err = juju.NewConnFromName(c.EnvName) |
| 275 | - if err != nil { |
| 276 | - return err |
| 277 | + if c.Conn == nil { |
| 278 | + var err error |
| 279 | + c.Conn, err = c.initConn() |
| 280 | + if err != nil { |
| 281 | + return err |
| 282 | + } |
| 283 | + defer c.Close() |
| 284 | } |
| 285 | - defer c.Close() |
| 286 | host, err := c.hostFromTarget(c.Target) |
| 287 | if err != nil { |
| 288 | return err |
| 289 | @@ -75,6 +77,14 @@ |
| 290 | return cmd.Run() |
| 291 | } |
| 292 | |
| 293 | +// initConn initialises the state connection. |
| 294 | +// It is the caller's responsibility to close the connection. |
| 295 | +func (c *SSHCommon) initConn() (*juju.Conn, error) { |
| 296 | + var err error |
| 297 | + c.Conn, err = juju.NewConnFromName(c.EnvName) |
| 298 | + return c.Conn, err |
| 299 | +} |
| 300 | + |
| 301 | func (c *SSHCommon) hostFromTarget(target string) (string, error) { |
| 302 | // is the target the id of a machine ? |
| 303 | if names.IsMachine(target) { |
| 304 | |
| 305 | === modified file 'worker/uniter/context.go' |
| 306 | --- worker/uniter/context.go 2013-06-10 21:04:37 +0000 |
| 307 | +++ worker/uniter/context.go 2013-08-05 01:28:25 +0000 |
| 308 | @@ -10,6 +10,7 @@ |
| 309 | "launchpad.net/juju-core/charm" |
| 310 | "launchpad.net/juju-core/log" |
| 311 | "launchpad.net/juju-core/state" |
| 312 | + unitdebug "launchpad.net/juju-core/worker/uniter/debug" |
| 313 | "launchpad.net/juju-core/worker/uniter/jujuc" |
| 314 | "os" |
| 315 | "os/exec" |
| 316 | @@ -148,8 +149,37 @@ |
| 317 | // RunHook executes a hook in an environment which allows it to to call back |
| 318 | // into ctx to execute jujuc tools. |
| 319 | func (ctx *HookContext) RunHook(hookName, charmDir, toolsDir, socketPath string) error { |
| 320 | + var err error |
| 321 | + env := ctx.hookVars(charmDir, toolsDir, socketPath) |
| 322 | + debugctx := unitdebug.NewHooksContext(ctx.unit.Name()) |
| 323 | + if session, _ := debugctx.FindSession(); session != nil && session.MatchHook(hookName) { |
| 324 | + log.Infof("worker/uniter: executing %s via debug-hooks", hookName) |
| 325 | + err = session.RunHook(hookName, charmDir, env) |
| 326 | + } else { |
| 327 | + err = runCharmHook(hookName, charmDir, env) |
| 328 | + } |
| 329 | + write := err == nil |
| 330 | + for id, rctx := range ctx.relations { |
| 331 | + if write { |
| 332 | + if e := rctx.WriteSettings(); e != nil { |
| 333 | + e = fmt.Errorf( |
| 334 | + "could not write settings from %q to relation %d: %v", |
| 335 | + hookName, id, e, |
| 336 | + ) |
| 337 | + log.Errorf("worker/uniter: %v", e) |
| 338 | + if err == nil { |
| 339 | + err = e |
| 340 | + } |
| 341 | + } |
| 342 | + } |
| 343 | + rctx.ClearCache() |
| 344 | + } |
| 345 | + return err |
| 346 | +} |
| 347 | + |
| 348 | +func runCharmHook(hookName, charmDir string, env []string) error { |
| 349 | ps := exec.Command(filepath.Join(charmDir, "hooks", hookName)) |
| 350 | - ps.Env = ctx.hookVars(charmDir, toolsDir, socketPath) |
| 351 | + ps.Env = env |
| 352 | ps.Dir = charmDir |
| 353 | outReader, outWriter, err := os.Pipe() |
| 354 | if err != nil { |
| 355 | @@ -175,22 +205,6 @@ |
| 356 | return nil |
| 357 | } |
| 358 | } |
| 359 | - write := err == nil |
| 360 | - for id, rctx := range ctx.relations { |
| 361 | - if write { |
| 362 | - if e := rctx.WriteSettings(); e != nil { |
| 363 | - e = fmt.Errorf( |
| 364 | - "could not write settings from %q to relation %d: %v", |
| 365 | - hookName, id, e, |
| 366 | - ) |
| 367 | - log.Errorf("worker/uniter: %v", e) |
| 368 | - if err == nil { |
| 369 | - err = e |
| 370 | - } |
| 371 | - } |
| 372 | - } |
| 373 | - rctx.ClearCache() |
| 374 | - } |
| 375 | return err |
| 376 | } |
| 377 | |
| 378 | |
| 379 | === added directory 'worker/uniter/debug' |
| 380 | === added file 'worker/uniter/debug/client.go' |
| 381 | --- worker/uniter/debug/client.go 1970-01-01 00:00:00 +0000 |
| 382 | +++ worker/uniter/debug/client.go 2013-08-05 01:28:25 +0000 |
| 383 | @@ -0,0 +1,120 @@ |
| 384 | +// Copyright 2013 Canonical Ltd. |
| 385 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 386 | + |
| 387 | +package debug |
| 388 | + |
| 389 | +import ( |
| 390 | + "encoding/base64" |
| 391 | + "strings" |
| 392 | + |
| 393 | + "launchpad.net/goyaml" |
| 394 | +) |
| 395 | + |
| 396 | +type hookArgs struct { |
| 397 | + Hooks []string `yaml:"hooks,omitempty"` |
| 398 | +} |
| 399 | + |
| 400 | +// ClientScript returns a bash script suitable for executing |
| 401 | +// on the unit system to intercept hooks via tmux shell. |
| 402 | +func ClientScript(c *HooksContext, hooks []string) string { |
| 403 | + // If any hook is "*", then the client is interested in all. |
| 404 | + for _, hook := range hooks { |
| 405 | + if hook == "*" { |
| 406 | + hooks = nil |
| 407 | + break |
| 408 | + } |
| 409 | + } |
| 410 | + |
| 411 | + s := strings.Replace(debugHooksClientScript, "{unit_name}", c.Unit, -1) |
| 412 | + s = strings.Replace(s, "{tmux_conf}", tmuxConf, 1) |
| 413 | + s = strings.Replace(s, "{entry_flock}", c.ClientFileLock(), -1) |
| 414 | + s = strings.Replace(s, "{exit_flock}", c.ClientExitFileLock(), -1) |
| 415 | + |
| 416 | + yamlArgs := encodeArgs(hooks) |
| 417 | + base64Args := base64.StdEncoding.EncodeToString(yamlArgs) |
| 418 | + s = strings.Replace(s, "{hook_args}", base64Args, 1) |
| 419 | + return s |
| 420 | +} |
| 421 | + |
| 422 | +func encodeArgs(hooks []string) []byte { |
| 423 | + // Marshal to YAML, then encode in base64 to avoid shell escapes. |
| 424 | + yamlArgs, err := goyaml.Marshal(hookArgs{Hooks: hooks}) |
| 425 | + if err != nil { |
| 426 | + // This should not happen: we're in full control. |
| 427 | + panic(err) |
| 428 | + } |
| 429 | + return yamlArgs |
| 430 | +} |
| 431 | + |
| 432 | +const debugHooksClientScript = `#!/bin/bash |
| 433 | +( |
| 434 | +# Lock the juju-<unit>-debug lockfile. |
| 435 | +flock -n 8 || (echo "Failed to acquire {entry_flock}: unit is already being debugged" 2>&1; exit 1) |
| 436 | +( |
| 437 | +# Close the inherited lock FD, or tmux will keep it open. |
| 438 | +exec 8>&- |
| 439 | + |
| 440 | +# Write out the debug-hooks args. |
| 441 | +echo "{hook_args}" | base64 -d > {entry_flock} |
| 442 | + |
| 443 | +# Lock the juju-<unit>-debug-exit lockfile. |
| 444 | +flock -n 9 || exit 1 |
| 445 | + |
| 446 | +# Wait for tmux to be installed. |
| 447 | +while [ ! -f /usr/bin/tmux ]; do |
| 448 | + sleep 1 |
| 449 | +done |
| 450 | + |
| 451 | +if [ ! -f ~/.tmux.conf ]; then |
| 452 | + if [ -f /usr/share/byobu/profiles/tmux ]; then |
| 453 | + # Use byobu/tmux profile for familiar keybindings and branding |
| 454 | + echo "source-file /usr/share/byobu/profiles/tmux" > ~/.tmux.conf |
| 455 | + else |
| 456 | + # Otherwise, use the legacy juju/tmux configuration |
| 457 | + cat > ~/.tmux.conf <<END |
| 458 | + {tmux_conf} |
| 459 | +END |
| 460 | + fi |
| 461 | +fi |
| 462 | + |
| 463 | +( |
| 464 | + # Close the inherited lock FD, or tmux will keep it open. |
| 465 | + exec 9>&- |
| 466 | + exec tmux new-session -s {unit_name} |
| 467 | +) |
| 468 | +) 9>{exit_flock} |
| 469 | +) 8>{entry_flock} |
| 470 | +exit $? |
| 471 | +` |
| 472 | + |
| 473 | +const tmuxConf = ` |
| 474 | +# Status bar |
| 475 | +set-option -g status-bg black |
| 476 | +set-option -g status-fg white |
| 477 | + |
| 478 | +set-window-option -g window-status-current-bg red |
| 479 | +set-window-option -g window-status-current-attr bright |
| 480 | + |
| 481 | +set-option -g status-right '' |
| 482 | + |
| 483 | +# Panes |
| 484 | +set-option -g pane-border-fg white |
| 485 | +set-option -g pane-active-border-fg white |
| 486 | + |
| 487 | +# Monitor activity on windows |
| 488 | +set-window-option -g monitor-activity on |
| 489 | + |
| 490 | +# Screen bindings, since people are more familiar with that. |
| 491 | +set-option -g prefix C-a |
| 492 | +bind C-a last-window |
| 493 | +bind a send-key C-a |
| 494 | + |
| 495 | +bind | split-window -h |
| 496 | +bind - split-window -v |
| 497 | + |
| 498 | +# Fix CTRL-PGUP/PGDOWN for vim |
| 499 | +set-window-option -g xterm-keys on |
| 500 | + |
| 501 | +# Prevent ESC key from adding delay and breaking Vim's ESC > arrow key |
| 502 | +set-option -s escape-time 0 |
| 503 | +` |
| 504 | |
| 505 | === added file 'worker/uniter/debug/client_test.go' |
| 506 | --- worker/uniter/debug/client_test.go 1970-01-01 00:00:00 +0000 |
| 507 | +++ worker/uniter/debug/client_test.go 2013-08-05 01:28:25 +0000 |
| 508 | @@ -0,0 +1,47 @@ |
| 509 | +// Copyright 2013 Canonical Ltd. |
| 510 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 511 | + |
| 512 | +package debug_test |
| 513 | + |
| 514 | +import ( |
| 515 | + "fmt" |
| 516 | + "regexp" |
| 517 | + |
| 518 | + . "launchpad.net/gocheck" |
| 519 | + |
| 520 | + "launchpad.net/juju-core/worker/uniter/debug" |
| 521 | +) |
| 522 | + |
| 523 | +type DebugHooksClientSuite struct{} |
| 524 | + |
| 525 | +var _ = Suite(&DebugHooksClientSuite{}) |
| 526 | + |
| 527 | +func (*DebugHooksClientSuite) TestClientScript(c *C) { |
| 528 | + ctx := debug.NewHooksContext("foo/8") |
| 529 | + |
| 530 | + // Test the variable substitutions. |
| 531 | + result := debug.ClientScript(ctx, nil) |
| 532 | + // No variables left behind. |
| 533 | + c.Assert(result, Matches, "[^{}]*") |
| 534 | + // tmux new-session -d -s {unit_name} |
| 535 | + c.Assert(result, Matches, fmt.Sprintf("(.|\n)*tmux new-session -s %s(.|\n)*", regexp.QuoteMeta(ctx.Unit))) |
| 536 | + //) 9>{exit_flock} |
| 537 | + c.Assert(result, Matches, fmt.Sprintf("(.|\n)*\\) 9>%s(.|\n)*", regexp.QuoteMeta(ctx.ClientExitFileLock()))) |
| 538 | + //) 8>{entry_flock} |
| 539 | + c.Assert(result, Matches, fmt.Sprintf("(.|\n)*\\) 8>%s(.|\n)*", regexp.QuoteMeta(ctx.ClientFileLock()))) |
| 540 | + |
| 541 | + // nil is the same as empty slice is the same as "*". |
| 542 | + // Also, if "*" is present as well as a named hook, |
| 543 | + // it is equivalent to "*". |
| 544 | + c.Assert(debug.ClientScript(ctx, nil), Equals, debug.ClientScript(ctx, []string{})) |
| 545 | + c.Assert(debug.ClientScript(ctx, []string{"*"}), Equals, debug.ClientScript(ctx, nil)) |
| 546 | + c.Assert(debug.ClientScript(ctx, []string{"*", "something"}), Equals, debug.ClientScript(ctx, []string{"*"})) |
| 547 | + |
| 548 | + // debug.ClientScript does not validate hook names, as it doesn't have |
| 549 | + // a full state API connection to determine valid relation hooks. |
| 550 | + expected := fmt.Sprintf( |
| 551 | + `(.|\n)*echo "aG9va3M6Ci0gc29tZXRoaW5nIHNvbWV0aGluZ2Vsc2UK" | base64 -d > %s(.|\n)*`, |
| 552 | + regexp.QuoteMeta(ctx.ClientFileLock()), |
| 553 | + ) |
| 554 | + c.Assert(debug.ClientScript(ctx, []string{"something somethingelse"}), Matches, expected) |
| 555 | +} |
| 556 | |
| 557 | === added file 'worker/uniter/debug/common.go' |
| 558 | --- worker/uniter/debug/common.go 1970-01-01 00:00:00 +0000 |
| 559 | +++ worker/uniter/debug/common.go 2013-08-05 01:28:25 +0000 |
| 560 | @@ -0,0 +1,35 @@ |
| 561 | +// Copyright 2013 Canonical Ltd. |
| 562 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 563 | + |
| 564 | +package debug |
| 565 | + |
| 566 | +import ( |
| 567 | + "fmt" |
| 568 | + "path/filepath" |
| 569 | + |
| 570 | + "launchpad.net/juju-core/names" |
| 571 | +) |
| 572 | + |
| 573 | +const defaultFlockDir = "/tmp" |
| 574 | + |
| 575 | +type HooksContext struct { |
| 576 | + Unit string |
| 577 | + FlockDir string |
| 578 | +} |
| 579 | + |
| 580 | +func NewHooksContext(unitName string) *HooksContext { |
| 581 | + return &HooksContext{Unit: unitName, FlockDir: defaultFlockDir} |
| 582 | +} |
| 583 | + |
| 584 | +func (c *HooksContext) ClientFileLock() string { |
| 585 | + basename := fmt.Sprintf("juju-%s-debug-hooks", names.UnitTag(c.Unit)) |
| 586 | + return filepath.Join(c.FlockDir, basename) |
| 587 | +} |
| 588 | + |
| 589 | +func (c *HooksContext) ClientExitFileLock() string { |
| 590 | + return c.ClientFileLock() + "-exit" |
| 591 | +} |
| 592 | + |
| 593 | +func (c *HooksContext) tmuxSessionName() string { |
| 594 | + return c.Unit |
| 595 | +} |
| 596 | |
| 597 | === added file 'worker/uniter/debug/common_test.go' |
| 598 | --- worker/uniter/debug/common_test.go 1970-01-01 00:00:00 +0000 |
| 599 | +++ worker/uniter/debug/common_test.go 2013-08-05 01:28:25 +0000 |
| 600 | @@ -0,0 +1,30 @@ |
| 601 | +// Copyright 2013 Canonical Ltd. |
| 602 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 603 | + |
| 604 | +package debug_test |
| 605 | + |
| 606 | +import ( |
| 607 | + "testing" |
| 608 | + |
| 609 | + . "launchpad.net/gocheck" |
| 610 | + |
| 611 | + "launchpad.net/juju-core/worker/uniter/debug" |
| 612 | +) |
| 613 | + |
| 614 | +type DebugHooksCommonSuite struct{} |
| 615 | + |
| 616 | +var _ = Suite(&DebugHooksCommonSuite{}) |
| 617 | + |
| 618 | +func TestPackage(t *testing.T) { |
| 619 | + TestingT(t) |
| 620 | +} |
| 621 | + |
| 622 | +// TestCommonScript tests the behaviour of HooksContext. |
| 623 | +func (*DebugHooksCommonSuite) TestHooksContext(c *C) { |
| 624 | + ctx := debug.NewHooksContext("foo/8") |
| 625 | + c.Assert(ctx.Unit, Equals, "foo/8") |
| 626 | + c.Assert(ctx.FlockDir, Equals, "/tmp") |
| 627 | + ctx.FlockDir = "/var/lib/juju" |
| 628 | + c.Assert(ctx.ClientFileLock(), Equals, "/var/lib/juju/juju-unit-foo-8-debug-hooks") |
| 629 | + c.Assert(ctx.ClientExitFileLock(), Equals, "/var/lib/juju/juju-unit-foo-8-debug-hooks-exit") |
| 630 | +} |
| 631 | |
| 632 | === added file 'worker/uniter/debug/server.go' |
| 633 | --- worker/uniter/debug/server.go 1970-01-01 00:00:00 +0000 |
| 634 | +++ worker/uniter/debug/server.go 2013-08-05 01:28:25 +0000 |
| 635 | @@ -0,0 +1,114 @@ |
| 636 | +// Copyright 2013 Canonical Ltd. |
| 637 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 638 | + |
| 639 | +package debug |
| 640 | + |
| 641 | +import ( |
| 642 | + "bytes" |
| 643 | + "errors" |
| 644 | + "io/ioutil" |
| 645 | + "os" |
| 646 | + "os/exec" |
| 647 | + |
| 648 | + "launchpad.net/goyaml" |
| 649 | + |
| 650 | + "launchpad.net/juju-core/utils/set" |
| 651 | +) |
| 652 | + |
| 653 | +// ServerSession represents a "juju debug-hooks" session. |
| 654 | +type ServerSession struct { |
| 655 | + *HooksContext |
| 656 | + hooks set.Strings |
| 657 | +} |
| 658 | + |
| 659 | +// MatchHook returns true if the specified hook name matches |
| 660 | +// the hook specified by the debug-hooks client. |
| 661 | +func (s *ServerSession) MatchHook(hookName string) bool { |
| 662 | + return s.hooks.IsEmpty() || s.hooks.Contains(hookName) |
| 663 | +} |
| 664 | + |
| 665 | +// RunHook "runs" the hook with the specified name via debug-hooks. |
| 666 | +func (s *ServerSession) RunHook(hookName, charmDir string, env []string) error { |
| 667 | + env = append(env, "JUJU_HOOK_NAME="+hookName) |
| 668 | + cmd := exec.Command("/bin/bash", "-s") |
| 669 | + cmd.Env = env |
| 670 | + cmd.Dir = charmDir |
| 671 | + cmd.Stdin = bytes.NewBufferString(debugHooksServerScript) |
| 672 | + if err := cmd.Start(); err != nil { |
| 673 | + return err |
| 674 | + } |
| 675 | + go func(proc *os.Process) { |
| 676 | + // Wait for the SSH client to exit (i.e. release the flock), |
| 677 | + // then kill the server hook process in case the client |
| 678 | + // exited uncleanly. |
| 679 | + path := s.ClientExitFileLock() |
| 680 | + exec.Command("flock", path, "-c", "true").Run() |
| 681 | + proc.Kill() |
| 682 | + }(cmd.Process) |
| 683 | + return cmd.Wait() |
| 684 | +} |
| 685 | + |
| 686 | +// FindSession attempts to find a debug hooks session for the unit specified |
| 687 | +// in the context, and returns a new ServerSession structure for it. |
| 688 | +func (c *HooksContext) FindSession() (*ServerSession, error) { |
| 689 | + cmd := exec.Command("tmux", "has-session", "-t", c.tmuxSessionName()) |
| 690 | + out, err := cmd.CombinedOutput() |
| 691 | + if err != nil { |
| 692 | + if len(out) != 0 { |
| 693 | + return nil, errors.New(string(out)) |
| 694 | + } else { |
| 695 | + return nil, err |
| 696 | + } |
| 697 | + } |
| 698 | + // Parse the debug-hooks file for an optional hook name. |
| 699 | + data, err := ioutil.ReadFile(c.ClientFileLock()) |
| 700 | + if err != nil { |
| 701 | + return nil, err |
| 702 | + } |
| 703 | + var args hookArgs |
| 704 | + err = goyaml.Unmarshal(data, &args) |
| 705 | + if err != nil { |
| 706 | + return nil, err |
| 707 | + } |
| 708 | + hooks := set.NewStrings(args.Hooks...) |
| 709 | + session := &ServerSession{c, hooks} |
| 710 | + return session, nil |
| 711 | +} |
| 712 | + |
| 713 | +const debugHooksServerScript = `set -e |
| 714 | +export JUJU_DEBUG=$(mktemp -d) |
| 715 | +exec > $JUJU_DEBUG/debug.log >&1 |
| 716 | + |
| 717 | +# Save environment variables and export them for sourcing. |
| 718 | +FILTER='^\(LS_COLORS\|LESSOPEN\|LESSCLOSE\|PWD\)=' |
| 719 | +env | grep -v $FILTER > $JUJU_DEBUG/env.sh |
| 720 | +sed -i 's/^/export /' $JUJU_DEBUG/env.sh |
| 721 | + |
| 722 | +# Create an internal script which will load the hook environment. |
| 723 | +cat > $JUJU_DEBUG/hook.sh <<END |
| 724 | +#!/bin/bash |
| 725 | +. $JUJU_DEBUG/env.sh |
| 726 | +echo \$\$ > $JUJU_DEBUG/hook.pid |
| 727 | +exec /bin/bash |
| 728 | +END |
| 729 | +chmod +x $JUJU_DEBUG/hook.sh |
| 730 | + |
| 731 | +tmux new-window -t $JUJU_UNIT_NAME -n $JUJU_HOOK_NAME "$JUJU_DEBUG/hook.sh" |
| 732 | + |
| 733 | +# If we exit for whatever reason, kill the hook shell. |
| 734 | +exit_handler() { |
| 735 | + if [ -f $JUJU_DEBUG/hook.pid ]; then |
| 736 | + kill -9 $(cat $JUJU_DEBUG/hook.pid) || true |
| 737 | + fi |
| 738 | +} |
| 739 | +trap exit_handler EXIT |
| 740 | + |
| 741 | +# Wait for the hook shell to start, and then wait for it to exit. |
| 742 | +while [ ! -f $JUJU_DEBUG/hook.pid ]; do |
| 743 | + sleep 1 |
| 744 | +done |
| 745 | +HOOK_PID=$(cat $JUJU_DEBUG/hook.pid) |
| 746 | +while kill -0 "$HOOK_PID" 2> /dev/null; do |
| 747 | + sleep 1 |
| 748 | +done |
| 749 | +` |
| 750 | |
| 751 | === added file 'worker/uniter/debug/server_test.go' |
| 752 | --- worker/uniter/debug/server_test.go 1970-01-01 00:00:00 +0000 |
| 753 | +++ worker/uniter/debug/server_test.go 2013-08-05 01:28:25 +0000 |
| 754 | @@ -0,0 +1,189 @@ |
| 755 | +// Copyright 2013 Canonical Ltd. |
| 756 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 757 | + |
| 758 | +package debug_test |
| 759 | + |
| 760 | +import ( |
| 761 | + "io/ioutil" |
| 762 | + "os" |
| 763 | + "os/exec" |
| 764 | + "path/filepath" |
| 765 | + "regexp" |
| 766 | + "time" |
| 767 | + |
| 768 | + . "launchpad.net/gocheck" |
| 769 | + |
| 770 | + "launchpad.net/juju-core/testing/checkers" |
| 771 | + "launchpad.net/juju-core/worker/uniter/debug" |
| 772 | +) |
| 773 | + |
| 774 | +type DebugHooksServerSuite struct { |
| 775 | + ctx *debug.HooksContext |
| 776 | + fakebin string |
| 777 | + tmpdir string |
| 778 | + oldenv []string |
| 779 | +} |
| 780 | + |
| 781 | +var _ = Suite(&DebugHooksServerSuite{}) |
| 782 | + |
| 783 | +// echocommand outputs its name and arguments to stdout for verification, |
| 784 | +// and exits with the value of $EXIT_CODE |
| 785 | +var echocommand = `#!/bin/bash |
| 786 | +echo $(basename $0) $@ |
| 787 | +exit $EXIT_CODE |
| 788 | +` |
| 789 | + |
| 790 | +var fakecommands = []string{"tmux"} |
| 791 | + |
| 792 | +func (s *DebugHooksServerSuite) setenv(key, value string) { |
| 793 | + s.oldenv = append(s.oldenv, key, os.Getenv(key)) |
| 794 | + os.Setenv(key, value) |
| 795 | +} |
| 796 | + |
| 797 | +func (s *DebugHooksServerSuite) SetUpTest(c *C) { |
| 798 | + s.fakebin = c.MkDir() |
| 799 | + s.tmpdir = c.MkDir() |
| 800 | + s.setenv("PATH", s.fakebin+":"+os.Getenv("PATH")) |
| 801 | + s.setenv("TMPDIR", s.tmpdir) |
| 802 | + s.setenv("TEST_RESULT", "") |
| 803 | + for _, name := range fakecommands { |
| 804 | + err := ioutil.WriteFile(filepath.Join(s.fakebin, name), []byte(echocommand), 0777) |
| 805 | + c.Assert(err, IsNil) |
| 806 | + } |
| 807 | + s.ctx = debug.NewHooksContext("foo/8") |
| 808 | + s.ctx.FlockDir = s.tmpdir |
| 809 | + s.setenv("JUJU_UNIT_NAME", s.ctx.Unit) |
| 810 | +} |
| 811 | + |
| 812 | +func (s *DebugHooksServerSuite) TearDownTest(c *C) { |
| 813 | + if len(s.oldenv) > 0 { |
| 814 | + for i := len(s.oldenv) - 2; i >= 0; i = i - 2 { |
| 815 | + os.Setenv(s.oldenv[i], s.oldenv[i+1]) |
| 816 | + } |
| 817 | + s.oldenv = nil |
| 818 | + } |
| 819 | +} |
| 820 | + |
| 821 | +func (s *DebugHooksServerSuite) TestFindSession(c *C) { |
| 822 | + // Test "tmux has-session" failure. The error |
| 823 | + // message is the output of tmux has-session. |
| 824 | + os.Setenv("EXIT_CODE", "1") |
| 825 | + session, err := s.ctx.FindSession() |
| 826 | + c.Assert(session, IsNil) |
| 827 | + c.Assert(err, ErrorMatches, regexp.QuoteMeta("tmux has-session -t "+s.ctx.Unit+"\n")) |
| 828 | + os.Setenv("EXIT_CODE", "") |
| 829 | + |
| 830 | + // tmux session exists, but missing debug-hooks file: error. |
| 831 | + session, err = s.ctx.FindSession() |
| 832 | + c.Assert(session, IsNil) |
| 833 | + c.Assert(err, NotNil) |
| 834 | + c.Assert(err, checkers.Satisfies, os.IsNotExist) |
| 835 | + |
| 836 | + // Hooks file is present, empty. |
| 837 | + err = ioutil.WriteFile(s.ctx.ClientFileLock(), []byte{}, 0777) |
| 838 | + c.Assert(err, IsNil) |
| 839 | + session, err = s.ctx.FindSession() |
| 840 | + c.Assert(session, NotNil) |
| 841 | + c.Assert(err, IsNil) |
| 842 | + // If session.hooks is empty, it'll match anything. |
| 843 | + c.Assert(session.MatchHook(""), Equals, true) |
| 844 | + c.Assert(session.MatchHook("something"), Equals, true) |
| 845 | + |
| 846 | + // Hooks file is present, non-empty |
| 847 | + err = ioutil.WriteFile(s.ctx.ClientFileLock(), []byte(`hooks: [foo, bar, baz]`), 0777) |
| 848 | + c.Assert(err, IsNil) |
| 849 | + session, err = s.ctx.FindSession() |
| 850 | + c.Assert(session, NotNil) |
| 851 | + c.Assert(err, IsNil) |
| 852 | + // session should only match "foo", "bar" or "baz". |
| 853 | + c.Assert(session.MatchHook(""), Equals, false) |
| 854 | + c.Assert(session.MatchHook("something"), Equals, false) |
| 855 | + c.Assert(session.MatchHook("foo"), Equals, true) |
| 856 | + c.Assert(session.MatchHook("bar"), Equals, true) |
| 857 | + c.Assert(session.MatchHook("baz"), Equals, true) |
| 858 | + c.Assert(session.MatchHook("foo bar baz"), Equals, false) |
| 859 | +} |
| 860 | + |
| 861 | +func (s *DebugHooksServerSuite) TestRunHookExceptional(c *C) { |
| 862 | + err := ioutil.WriteFile(s.ctx.ClientFileLock(), []byte{}, 0777) |
| 863 | + c.Assert(err, IsNil) |
| 864 | + session, err := s.ctx.FindSession() |
| 865 | + c.Assert(session, NotNil) |
| 866 | + c.Assert(err, IsNil) |
| 867 | + |
| 868 | + // Run the hook in debug mode with no exit flock held. |
| 869 | + // The exit flock will be acquired immediately, and the |
| 870 | + // debug-hooks server process killed. |
| 871 | + err = session.RunHook("myhook", s.tmpdir, os.Environ()) |
| 872 | + c.Assert(err, ErrorMatches, "signal: killed") |
| 873 | + |
| 874 | + // Run the hook in debug mode with the exit flock held. |
| 875 | + // This simulates the client process starting but not |
| 876 | + // cleanly exiting (normally the .pid file is updated, |
| 877 | + // and the server waits on the client process' death). |
| 878 | + cmd := exec.Command("flock", s.ctx.ClientExitFileLock(), "-c", "sleep 1s") |
| 879 | + c.Assert(cmd.Start(), IsNil) |
| 880 | + expected := time.Now().Add(time.Second) |
| 881 | + err = session.RunHook("myhook", s.tmpdir, os.Environ()) |
| 882 | + after := time.Now() |
| 883 | + c.Assert(after, checkers.TimeBetween(expected.Add(-100*time.Millisecond), expected.Add(100*time.Millisecond))) |
| 884 | + c.Assert(err, ErrorMatches, "signal: killed") |
| 885 | + c.Assert(cmd.Wait(), IsNil) |
| 886 | +} |
| 887 | + |
| 888 | +func (s *DebugHooksServerSuite) TestRunHook(c *C) { |
| 889 | + err := ioutil.WriteFile(s.ctx.ClientFileLock(), []byte{}, 0777) |
| 890 | + c.Assert(err, IsNil) |
| 891 | + session, err := s.ctx.FindSession() |
| 892 | + c.Assert(session, NotNil) |
| 893 | + c.Assert(err, IsNil) |
| 894 | + |
| 895 | + // Run the hook in debug mode with the exit flock held, |
| 896 | + // and also create the .pid file. We'll populate it with |
| 897 | + // an invalid PID; this will cause the server process to |
| 898 | + // exit cleanly (as if the PID were real and no longer running). |
| 899 | + cmd := exec.Command("flock", s.ctx.ClientExitFileLock(), "-c", "sleep 5s") |
| 900 | + c.Assert(cmd.Start(), IsNil) |
| 901 | + ch := make(chan error) |
| 902 | + go func() { |
| 903 | + ch <- session.RunHook("myhook", s.tmpdir, os.Environ()) |
| 904 | + }() |
| 905 | + var debugdir os.FileInfo |
| 906 | + for i := 0; i < 10; i++ { |
| 907 | + tmpdir, err := os.Open(s.tmpdir) |
| 908 | + if err != nil { |
| 909 | + c.Fatalf("Faiked to open $TMPDIR: %s", err) |
| 910 | + } |
| 911 | + fi, err := tmpdir.Readdir(-1) |
| 912 | + if err != nil { |
| 913 | + c.Fatalf("Failed to read $TMPDIR: %s", err) |
| 914 | + } |
| 915 | + tmpdir.Close() |
| 916 | + for _, fi := range fi { |
| 917 | + if fi.IsDir() { |
| 918 | + hooksh := filepath.Join(s.tmpdir, fi.Name(), "hook.sh") |
| 919 | + if _, err = os.Stat(hooksh); err == nil { |
| 920 | + debugdir = fi |
| 921 | + break |
| 922 | + } |
| 923 | + } |
| 924 | + } |
| 925 | + if debugdir != nil { |
| 926 | + break |
| 927 | + } |
| 928 | + time.Sleep(10 * time.Millisecond) |
| 929 | + } |
| 930 | + if debugdir == nil { |
| 931 | + c.Error("could not find hook.sh") |
| 932 | + } else { |
| 933 | + hookpid := filepath.Join(s.tmpdir, debugdir.Name(), "hook.pid") |
| 934 | + err = ioutil.WriteFile(hookpid, []byte("not a pid"), 0777) |
| 935 | + c.Assert(err, IsNil) |
| 936 | + |
| 937 | + // RunHook should complete without waiting to be |
| 938 | + // killed, and despite the exit lock being held. |
| 939 | + err = <-ch |
| 940 | + c.Assert(err, IsNil) |
| 941 | + } |
| 942 | + cmd.Process.Kill() // kill flock |
| 943 | +} |
Reviewers: mp+177353_
Message:
Please take a look.
Description:
Support juju debug-hooks
This implementation is a departure from pyjuju, as we do not
have Zookeeper's ephemeral nodes to rely upon for locking.
The approach taken is described here:
https:/
Rather than periodically checking tmux, I've just gone for
a simple approach of calling "tmux has-session" each time a
hook is to be executed. If this proves to be too expensive,
then reassess.
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files:
A [revision details]
A cmd/juju/
A cmd/juju/
M cmd/juju/main.go
M cmd/juju/
M cmd/juju/ssh.go
M worker/
A worker/
A worker/
A worker/