skybet / cali

Cali Automation Layout Initialiser
MIT License
32 stars 7 forks source link

SEGV on DockerClient without InitDocker #36

Closed wheresalice closed 6 years ago

wheresalice commented 6 years ago

In a task I can create a new DockerClient to pull newer versions of the image I'm working with. This seems to work (although I'm about to raise a separate issue on this):

docker := cali.NewDockerClient()
        docker.InitDocker()
        if docker.ImageExists("staticli/rake:latest") {

But if I skip the docker.InitDocker() step then I get a segfault. Which is not user-friendly.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x58 pc=0x1391dca]

goroutine 1 [running]:
github.com/staticli/staticli/vendor/github.com/docker/docker/client.(*Client).getAPIPath(0x0, 0xc4202fc150, 0x21, 0x0, 0x1042a26, 0xc4202fc16c)
        /go/src/github.com/staticli/staticli/vendor/github.com/docker/docker/client/client.go:196 +0x3a
github.com/staticli/staticli/vendor/github.com/docker/docker/client.(*Client).sendRequest(0x0, 0x1539140, 0xc4200200f0, 0x14d7a9d, 0x3, 0xc4202fc150, 0x21, 0x0, 0x0, 0x0, ...)
        /go/src/github.com/staticli/staticli/vendor/github.com/docker/docker/client/request.go:117 +0x56
github.com/staticli/staticli/vendor/github.com/docker/docker/client.(*Client).get(0x0, 0x1539140, 0xc4200200f0, 0xc4202fc150, 0x21, 0x0, 0x0, 0xc4202fc150, 0x21, 0x0, ...)
        /go/src/github.com/staticli/staticli/vendor/github.com/docker/docker/client/request.go:36 +0xab
github.com/staticli/staticli/vendor/github.com/docker/docker/client.(*Client).ImageInspectWithRaw(0x0, 0x1539140, 0xc4200200f0, 0x14e2419, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /go/src/github.com/staticli/staticli/vendor/github.com/docker/docker/client/image_inspect.go:15 +0x101
github.com/staticli/staticli/vendor/github.com/skybet/cali.(*DockerClient).ImageExists(0xc4201dbc88, 0x14e2419, 0x14, 0x9)
        /go/src/github.com/staticli/staticli/vendor/github.com/skybet/cali/docker.go:346 +0x1ef
github.com/staticli/staticli/cmd.init.17.func1(0xc4202239e0, 0x17a7478, 0x0, 0x0)
        /go/src/github.com/staticli/staticli/cmd/update.go:17 +0x15b
github.com/staticli/staticli/vendor/github.com/skybet/cali.(*Cli).NewCommand.func2(0xc4202ebb00, 0x17a7478, 0x0, 0x0)
        /go/src/github.com/staticli/staticli/vendor/github.com/skybet/cali/cli.go:234 +0x4f
github.com/staticli/staticli/vendor/github.com/spf13/cobra.(*Command).execute(0xc4202ebb00, 0x17a7478, 0x0, 0x0, 0xc4202ebb00, 0x17a7478)
        /go/src/github.com/staticli/staticli/vendor/github.com/spf13/cobra/command.go:702 +0x2c6
github.com/staticli/staticli/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0xc42009f440, 0x1, 0x1, 0xc42000c4a8)
        /go/src/github.com/staticli/staticli/vendor/github.com/spf13/cobra/command.go:783 +0x2e4
github.com/staticli/staticli/vendor/github.com/spf13/cobra.(*Command).Execute(0xc42009f440, 0x1, 0x1)
        /go/src/github.com/staticli/staticli/vendor/github.com/spf13/cobra/command.go:736 +0x2b
github.com/staticli/staticli/vendor/github.com/skybet/cali.(*Cli).Start(0xc420153f20)
        /go/src/github.com/staticli/staticli/vendor/github.com/skybet/cali/cli.go:309 +0xad
github.com/staticli/staticli/cmd.Execute()
        /go/src/github.com/staticli/staticli/cmd/root.go:20 +0x2d
main.main()
        /go/src/github.com/staticli/staticli/main.go:8 +0x20

We should somehow catch this (by checking InitDocker has run(?)

lucymhdavies commented 6 years ago

I think the problem here is that cali.NewDockerClient() does not run docker.InitDocker()

Instead, Cali runs it as part of defaultTaskFunc.

I can sorta see why; no point initialising docker if you don't need to. And considering the fact that InitDocker can return an error if it can't talk to the docker daemon, we don't want staticli update to fail just because we can't talk to Docker.

lucymhdavies commented 6 years ago

I think the solution is something like:

// Init initialises the client
func (c *DockerClient) InitDocker() error {
+    // Do nothing if already initialised Docker
+    if c.Cli != nil {
+        return nil
+    }

    var cli *client.Client

    defaultHeaders := map[string]string{"User-Agent": "engine-api-cli-1.0"}
    cli, err := client.NewClient(dockerHost, "v1.22", nil, defaultHeaders)

    if err != nil {
        return fmt.Errorf("Could not connect to Docker daemon on %s: %s", dockerHost, err)
    }
    c.Cli = cli
    return nil
}

Then, in ImageExists (and all otherDockerClient functions that need it), add something like:

    if err := cli.InitDocker(); err != nil {
        return err
    }

This also has the added benefit of us being able to pull InitDocker out of defaultTaskFunc, where it doesn't really belong.