Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bind volumes over the remote api does not work in 11.0 #6231

Closed
SegFaultx64 opened this issue Jun 6, 2014 · 26 comments
Closed

Bind volumes over the remote api does not work in 11.0 #6231

SegFaultx64 opened this issue Jun 6, 2014 · 26 comments
Milestone

Comments

@SegFaultx64
Copy link

I am trying to use the docker remote API to bind volumes. I am following the directions here but it is still not working.

My test is running these two curl commands

    curl -X POST -H "Content-Type: application/json" http://localhost:4243/containers/create    
-d '{
  "Image" : "ubuntu",
  "Cmd" : [ "ls", "-la"],
  "Volumes": { "/mnt": {} },
  "WorkingDir" : "/mnt",
  "NetworkDisabled" : false
}'

curl -XPOST http://localhost:4243/containers/<whateverID>/start -d '{
    "Binds": [ "/tmp/:/mnt:ro" ]
}'

The documentation for this in api is essentially. If anyone can even point me to the revelent lines in the client code that would be something to work from. Has anyone else even gotten this working or is this a know issue?

@cpuguy83
Copy link
Member

cpuguy83 commented Jun 6, 2014

curl -X POST -h 'Content-Type: application/json' -d '{"Binds": ["/tmp:/mnt:ro"]}' http://localhost:4243/containers//start

@sfitts
Copy link

sfitts commented Sep 9, 2014

I'm seeing the same issue using the 1.13 API to a 1.2.0 server. Here is the request/response log:

INFO  [2014-09-09 17:08:49,494] com.sun.jersey.api.client.filter.LoggingFilter: 2 * Client out-bound request
2 > POST http://localhost:2375/v1.13/containers/2c5aa438afa273b467472bec4d619ffbfb02fa1849b8f5bcf6486c4475990368/start
2 > Accept: application/json
2 > Content-Type: application/json
{"Binds":["/tmp/modules/tmp3230622042736919551:/modules:ro"],"Links":[],"LxcConf":null,"PortBindings":null,"PublishAllPorts":false,"Privileged":false,"Dns":null,"VolumesFrom":null}

INFO  [2014-09-09 17:08:49,658] com.sun.jersey.api.client.filter.LoggingFilter: 2 * Client in-bound response
2 < 204
2 < Date: Sun, 07 Sep 2014 15:46:25 GMT
2 < Content-Length: 0
2 < Content-Type: application/json; charset=utf-8
2 < 

When this command is sent to a 1.1.1 docker server then it works as expected (the volume described by the bind is created). When it is sent to a 1.2.0 server the bind is silently ignored (the response is the same in both cases and there is no indication of any issue in the server's log, it just doesn't create the volume).

@sfitts
Copy link

sfitts commented Sep 9, 2014

Confirmed that if I back my Ubuntu server off to 1.1.1 then this works. Not sure if it only impacts the v1.13 API against the 1.2 server or if 1.14 is similarly impacted (I'm using docker-java which is built against 1.13 ATM).

@cpuguy83
Copy link
Member

@sfitts Perhaps related to #7869 (comment) ?

@sfitts
Copy link

sfitts commented Sep 10, 2014

@cpuguy83 possibly -- especially given it looks like the issue there is with a Java client as well. I'll take a look and see if the java-docker code is setting transfer encoding to chunked (that doesn't show in the request log unfortunately).

@sfitts
Copy link

sfitts commented Sep 10, 2014

Yep -- transfer encoding is chunked since the size of the entity isn't known in advance.

@crosbymichael
Copy link
Contributor

@cpuguy83 have you been able to confirm this with a reproducible case?

@cpuguy83
Copy link
Member

@crosbymichael Yep:

curl -XPOST -H 'Content-type: application/json' -H "Transfer-Encoding: chunked" 172.17.42.1:2375/containers/create -d '{"Image": "ubuntu:14.04", "Tty": true, "OpenStdin": true, "Volumes": { "/tmp": {}} }'

curl -XPOST -H 'Content-type: application/json' -H "Transfer-Encoding: chunked" 172.17.42.1:2375/containers/$cid/start -d '{"Binds":["/tmp:/tmp"], "CapAdd": ["NET_ADMIN"]}'

Any hostconfig fails unless you disable chunked encoding.
Create seems to work just fine with it, start does not.

@sfitts
Copy link

sfitts commented Sep 17, 2014

Any word on when this might be fixed. It is currently preventing us from upgrading to 1.2.x (which we'd otherwise love to do).

@cpuguy83
Copy link
Member

@sfitts can you disable chunked encoding?

@sfitts
Copy link

sfitts commented Sep 17, 2014

@cpuguy83 unfortunately no. It is the default behavior for the Apache client when sending POJOs and Apache is about 4 layers down in this scenario (what we're actually doing is using Gradle to script the building and running of images/containers, Gradle in turn uses java-docker, which uses Jersey as its client, which uses Apache as its HTTP handler).

It might be possible to work around the issue in java-docker by pre-serializing the body entity and then streaming that directly. However, I'm not sure you can get at what you need to in order to fix the content length (you can of course set the header yourself, but Apache just removes it before using its own logic to determine what the value should be).

Maybe if I get some spare time this weekend I can take a run at all that, but I'm not optimistic. What changed between 1.1.1 and 1.2.0 that might have caused this (since all of this is working perfectly in 1.1.1)?

@sfitts
Copy link

sfitts commented Sep 17, 2014

It is also somewhat odd that create works fine, while start does not. They both contain body content. In looking at the docker code it seems that create treats the content as a form and parses it that way while start just uses the request body directly. Of course I don't actually know go much at all, so I'm just kind of intuiting my way through the code (but it stands to reason that there is some difference in how the two commands process the request that causes one to work and the other to fail).

@cpuguy83
Copy link
Member

@sfitts It seems like a backwards compat issue... honestly I'm trying to fix it, but can't seem to properly detect the chunked encoding.
Not sure where to go from here.

@sfitts
Copy link

sfitts commented Sep 18, 2014

@cpuguy83 not sure what you mean by backwards compat... is the issue the changes added in commit 8266c381d62a0790c489e27cc93ed8f4618d03c7 where it assumes no body if content length is 0?

If that's the case then maybe it would be possible to look for a non-nil body with the appropriate content type and attempt the decode if that's true (even if the content len is 0). Then if the decode fails and content len is 0 fall back to skipping the body as you do now. Messy, but if you can't tell what the encoding is through the go libraries then you don't have much choice.

Of course that might not be the impacting change, in which case just ignore me ;).

@cpuguy83
Copy link
Member

@sfitts That is the change for sure, I just don't know why it was changed or alternative ways to solve it.

ping @crosbymichael

@sfitts
Copy link

sfitts commented Sep 25, 2014

@cpuguy83 , @crosbymichael any update on this? Turns out another major issue we have in Boot2Docker isn't fixed until 1.2.0, which we can't upgrade to due to this issue.

Below is a rough sketch of what I was thinking you might try (apologies for the pigeon go):

    // allow a nil body for backwards compatibility, but also support chunked encoding when
        // content len will be 0 even though there is content to process
    if r.Body != nil  {
                // If we have content, it must be of the correct type
        if r.ContentLength > 0 && !api.MatchesContentType(r.Header.Get("Content-Type"), "application/json") {
            return fmt.Errorf("Content-Type of application/json is required")
        }

        if err := job.DecodeEnv(r.Body); err != nil {
                        // If there should be content to decode, fail, otherwise assume nil body
                        if r.ContentLength > 0 {
                return err
                        }
        }
    }

The basic idea is to allow for the decode of Body even if content len is 0, but only fail when content len is greater than 0. This preserves the previous success path (where it turns out the decode works just fine even though the content len is 0) and also preserves the new success path where the decode fails because there is no body, but we want to continue anyway.

@crosbymichael crosbymichael added this to the 1.3.0 milestone Sep 26, 2014
@crosbymichael
Copy link
Contributor

Added to the 1.3 milestone so that this is taken care of

@sfitts
Copy link

sfitts commented Sep 26, 2014

@crosbymichael Thanks. Do you have a rough timeframe for 1.3 at this point?

@crosbymichael
Copy link
Contributor

ping @jfrazelle @erikh @aluzzardi @tiborvass @unclejack @vieux

I think this should be solved for 1.3, what about you?

@jessfraz
Copy link
Contributor

jessfraz commented Oct 1, 2014

I don't think we fixed the transfer encoding chunked error.

@aluzzardi
Copy link
Member

@crosbymichael Agreed.

On a slightly related note, we should probably update the documentation to recommend posting the host config on create rather than start, now that both work in 1.3 (since exec).

@cpuguy83
Copy link
Member

cpuguy83 commented Oct 1, 2014

I think I've got this one fixed, PR to follow.

@jessfraz
Copy link
Contributor

jessfraz commented Oct 1, 2014

\o/

On Tue, Sep 30, 2014 at 6:10 PM, Brian Goff notifications@github.com
wrote:

I think I've got this one fixed, PR to follow.


Reply to this email directly or view it on GitHub
#6231 (comment).

@sfitts
Copy link

sfitts commented Oct 1, 2014

@aluzzardi does this mean that a start with no host config will use what's already been defined for the container? Because right now it wipes out the host config (even if it was previously started with some values).

@aluzzardi
Copy link
Member

@sfitts Indeed, it should:

https://github.com/docker/docker/blob/master/daemon/start.go#L32

Start won't wipe the host config unless a new one was provided.

cpuguy83 added a commit to cpuguy83/docker that referenced this issue Oct 1, 2014
Docker-DCO-1.1-Signed-off-by: Brian Goff <cpuguy83@gmail.com> (github: cpuguy83)
crosbymichael added a commit that referenced this issue Oct 3, 2014
@crosbymichael
Copy link
Contributor

Closed by #8324

nathanleclaire pushed a commit to nathanleclaire/docker that referenced this issue Oct 12, 2014
Docker-DCO-1.1-Signed-off-by: Brian Goff <cpuguy83@gmail.com> (github: cpuguy83)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants