[2.7-rc6] Invalid exec format for a hook file does not result in a unit error

[Dmitrii Shcherbakov]

A simple charm that has `exit 2` in hooks/install without a shebang (!) does not make Juju to error out on a hook execution and block further hook executions. Instead, Juju sets a unit status to "unknown" and other hooks are then executed.

Expectation: ENOEXEC ("exec format error") would be caught by the unit agent and a hook execution would be marked as failed.

➜ dummy-leaver tree
.
├── config.yaml
├── hooks
│   └── install
└── metadata.yaml

1 directory, 3 files

# NOTE: invalid without a shebang
➜ dummy-leaver cat hooks/install
exit 2

➜ dummy-leaver stat hooks/install
  File: hooks/install
  Size: 7 Blocks: 8 IO Block: 4096 regular file
Device: 1dh/29d Inode: 13251814 Links: 1
Access: (0755/-rwxr-xr-x) Uid: ( 1000/ ubuntu) Gid: ( 1000/ ubuntu)
Access: 2019-11-28 13:00:15.267308455 +0300
Modify: 2019-11-28 12:59:56.195241078 +0300
Change: 2019-11-28 12:59:56.195241078 +0300
 Birth: -

➜ dummy-leaver juju show-status-log leaver/1
Time Type Status Message
28 Nov 2019 13:02:43+03:00 workload waiting waiting for machine
28 Nov 2019 13:02:43+03:00 juju-unit allocating
28 Nov 2019 13:04:03+03:00 workload waiting installing agent
28 Nov 2019 13:04:06+03:00 workload waiting agent initializing
28 Nov 2019 13:04:07+03:00 workload maintenance installing charm software
28 Nov 2019 13:04:07+03:00 juju-unit executing running install hook
28 Nov 2019 13:04:07+03:00 juju-unit executing running leader-elected hook
28 Nov 2019 13:04:07+03:00 juju-unit executing running config-changed hook
28 Nov 2019 13:04:07+03:00 juju-unit executing running start hook
28 Nov 2019 13:04:07+03:00 workload unknown
28 Nov 2019 13:04:07+03:00 juju-unit idle

➜ dummy-leaver juju status leaver/1
Model Controller Cloud/Region Version SLA Timestamp
default localhost-localhost localhost/localhost 2.7-rc6 unsupported 13:07:34+03:00

App Version Status Scale Charm Store Rev OS Notes
leaver unknown 1 dummy-leaver local 2 ubuntu

Unit Workload Agent Machine Public address Ports Message
leaver/1* unknown idle 3 10.190.92.218

Machine State DNS Inst id Series AZ Message
3 started 10.190.92.218 juju-2a1341-3 bionic Running

A simple go program like this manages to catch it:

package main

import (
 "log"
 "os/exec"
)

func main() {
 cmd := exec.Command("/tmp/dummy-leaver/hooks/install")
 err := cmd.Run()
 if err != nil {
  log.Fatal(err)
 }
}

./main
2019/11/28 13:15:05 fork/exec /tmp/dummy-leaver/hooks/install: exec format error

[Dmitrii Shcherbakov (dmitriis)]

Attached a debug-log with

juju model-config logging-config='<root>=WARNING;unit=TRACE;juju.state=TRACE;juju.worker.uniter.resolver=TRACE;juju.worker.uniter.runner=TRACE':

[John A Meinel (jameinel)]

Digging through the code, the issue is that if ps.Start() returns an error, then we trigger a code path that doesn't wait for the process to exit (which is correct), but then we don't handle the error correctly, because the rest of the code is *only* looking at the exit error.
Something like:

--- a/worker/uniter/runner/runner.go
+++ b/worker/uniter/runner/runner.go
@@ -515,6 +515,8 @@ func (runner *runner) runCharmHookOnLocal(hookName string, env []string, charmLo
                runner.context.SetProcess(hookProcess{ps.Process})
                // Block until execution finishes
                exitErr = ps.Wait()
+ } else {
+ exitErr = err
        }
        // Ensure hook loggers are stopped before reading stdout/stderr
        // so all the output is captured.

should fix this case.

[John A Meinel (jameinel)]

https://github.com/juju/juju/pull/10966