docker / engine-api

DEPRECATED: Please see https://github.com/docker/docker/tree/master/client
Apache License 2.0
265 stars 163 forks source link

Hijack is incompatible with use of CloseNotifier #303

Open raphink opened 8 years ago

raphink commented 8 years ago

This is caused by https://github.com/docker/docker/issues/12845

package main

import (
  "fmt"
  "io/ioutil"

  "github.com/docker/engine-api/client"
  "github.com/docker/engine-api/types"
  "golang.org/x/net/context"
)

// Some basic nginx container running
const ContainerID string = "ce5d8b318ca0"

func main() {
  c, _ := client.NewClient("unix:///var/run/docker.sock", "", nil, nil)

  body, _ := c.ContainerLogs(context.Background(), ContainerID, types.ContainerLogsOptions{
    ShowStdout: true,
    ShowStderr: true,
  })

  defer body.Close()

  // This line causes ContainerExecStart to fail with
  // Error response from daemon: http: Hijack is incompatible with use of CloseNotifier
  _, _ = ioutil.ReadAll(body)

  exec, _ := c.ContainerExecCreate(context.Background(), ContainerID, types.ExecConfig{
    Cmd: []string{"date"},
  })

  err := c.ContainerExecStart(context.Background(), exec.ID, types.ExecStartCheck{})
  fmt.Println(err)
}
$ go run engine-api-bug.go 
Error response from daemon: http: Hijack is incompatible with use of CloseNotifier

How can I fix this with engine-api?

cpuguy83 commented 8 years ago

This is likely from connection re-use, which is automatic from the http package.

In the client transport we'll either need to set DisableKeepAlives or maybe in ContainerExecStart we can make sure to set Close in the request?

raphink commented 8 years ago

This should work. Currently, I have to create a new Docker client for the ContainerExecStart.

cpuguy83 commented 8 years ago

This could probably be considered a golang bug. We should not have to manually manage this when the http package knows we've hijacked the connection.

LK4D4 commented 8 years ago

I belive fix for this was on go1.6 https://github.com/golang/go/commit/99fb19194c03c618c0d8faa87b91ba419ae28ee3

cpuguy83 commented 8 years ago

It would seem so. @raphink any chance you can test with go 1.6?

raphink commented 8 years ago

I get this bug with all versions of go, including tip. I think it's the docker engine that needs to be rebuilt with the new version of go.

thaJeztah commented 8 years ago

@cpuguy83 @LK4D4 saw that someone reported this issue on 1.12-rc3 (which uses go 1.6); https://github.com/docker/docker/issues/12845#issuecomment-230592427