Skip to content
This repository has been archived by the owner on Feb 1, 2021. It is now read-only.

Start container command not forwarding the HostConfig to the container #1781

Closed
sboschman opened this issue Feb 9, 2016 · 22 comments · Fixed by #1972
Closed

Start container command not forwarding the HostConfig to the container #1781

sboschman opened this issue Feb 9, 2016 · 22 comments · Fixed by #1972

Comments

@sboschman
Copy link
Contributor

Client tools (Docker plugin for Jenkins in our case) that rely on passing (additional) HostConfig in the start container command do not work since 1.1.0. For example the port bindings are not passed and the container is inaccessible.

I believe the commit for 'Add support for container rescheduling on node failure.' (13f6021) broke this flow.

From the Docker Remote API docs:

Start a container:
For backwards compatibility, this endpoint accepts a HostConfig as JSON-encoded request body.
Reference: https://docs.docker.com/engine/reference/api/docker_remote_api_v1.22/#start-a-container

@nishanttotla
Copy link
Contributor

@sboschman can you provide more details about your setup, and how to reproduce this issue?

@sboschman
Copy link
Contributor Author

added a testcase to the integration tests (https://github.com/sboschman/swarm/blob/master/test/integration/api/start.bats) to reproduce this issue

it passes on the 1.0.1 swarm tag, but fails on swarm master

Start container function from github.com/samalba/dockerclient:
func (client *DockerClient) StartContainer(id string, config *HostConfig) error

Function call in container.go implemented in the earlier mentioned commit 13f6021:
return c.Engine.client.StartContainer(c.Id, nil)

@nishanttotla
Copy link
Contributor

@sboschman thanks, I'm looking into it.

@nishanttotla
Copy link
Contributor

@sboschman I'm fixing this now, do you mind if I include your commit (for the test) into my PR, so that you get credit?

@nishanttotla
Copy link
Contributor

@sboschman if you want me to include your commits, you'll have to sign them. Do you mind either creating new commits, or signing them?

I'll then port over those commits to my fork (https://github.com/nishanttotla/swarm/tree/1781-pass-hostconfig) and submit a PR to Swarm so you get credit. We'd like for this to be done by tonight, so please let me know if you can get the tests in.

Thanks,

@sboschman
Copy link
Contributor Author

timezone delay :) thanks for fixing the issue

I made a new commit and signed it off: https://github.com/sboschman/swarm/commit/395a5fa6ddad6a5daf1e1a9fc2b59ed92cf629b7; hope it is oke like that

@nishanttotla
Copy link
Contributor

@sboschman thanks!

@nishanttotla
Copy link
Contributor

@sboschman if you don't mind, can you rebase your branch on top of the latest swarm master? That way, I can update my current PR with the fewest extra commits. I'm trying to figure how to add your commit to my PR. If that isn't feasible eventually, I'll open a separate PR for your commit.

@sboschman
Copy link
Contributor Author

@nishanttotla rebased the branch

@nishanttotla
Copy link
Contributor

Reopening issue since the PR had to be reverted due to docker-compose related issues.

@nfoonf
Copy link

nfoonf commented Mar 10, 2016

Hi,

is there any idea, how to fix/workaround this issue?

@nishanttotla
Copy link
Contributor

@nfoonf if you don't really need to do it, the support for passing HostConfig at /start is deprecated, so not recommended. We're trying to figure out how to best fix this until then.

Instead, you should pass HostConfig at /create.

@nfoonf
Copy link

nfoonf commented Mar 12, 2016

I know it is deprecated. But I am nailed on using it because the docker-build-steps plug in and probably also shipyard are bitten by this bug. I therefore would highly appreciate a fix, even if it is obviously wrong to use it like this in the first place

@nishanttotla
Copy link
Contributor

@nfoonf the reason I asked you is because I think Docker Engine will stop supporting this in the next release (in less than a month), after which Swarm will not be able to.

@thaJeztah can you confirm that, or suggest what can be done?

@nfoonf
Copy link

nfoonf commented Mar 14, 2016

@nishanttotla to hear, that this won't be supported in the near future is good news in my eyes.
So Docker Build Step will have to fix their code using the Docker-API anyway.

@thaJeztah
Copy link
Member

Hm, yes, it is scheduled to be removed in docker 1.12 (see moby/moby#17799). Not sure what's best here, fix the regression, or document it as a difference between the Docker API and the Swarm API (because there's already some differences); guess it depends on how difficult it is to fix

@nishanttotla
Copy link
Contributor

@vieux WDYT?

@vieux
Copy link
Contributor

vieux commented Mar 15, 2016

@nishanttotla looking into in now

@vieux
Copy link
Contributor

vieux commented Mar 15, 2016

it is an easy fix, see #1972

@calavera
Copy link

The host configuration should be sent when the container is created, never after. This behavior has been deprecated in the docker engine for very very long time and you should not rely on it. Please, consider also dropping supported engine versions from Swarm if the only reason to do this is to keep compatibility with them.

@phagunbaya
Copy link

phagunbaya commented Aug 5, 2016

I'm still facing having this issue. HostConfig I sent while creating a container is not considered when running the container. It gets reset to the default.

Docker version - v1.12
OS - Ubuntu 14.04.4-LTS
Swarm setup - https://docs.docker.com/swarm/install-manual/

Creating the container:
[POST] http://localhost:2375/container/create?name={id}

    {
    "Env": [
            "env1=val1"
       ],
       "Cmd": [
           "/Run.sh",
           "launch
        ],
       "Image": "<image>",
       "Volumes": {
           "/tmp" : {}
       },
       "HostConfig": {
         "NetworkMode": "my-net"
       }
    }

Response of $docker inspect:

    "HostConfig": {
        "Binds": null,
        "ContainerIDFile": "",
        "LogConfig": {
            "Type": "json-file",
            "Config": {}
        },
        "NetworkMode": "my-net",
        "PortBindings": null,
        "RestartPolicy": {
            "Name": "",
            "MaximumRetryCount": 0
        },
        "AutoRemove": false,
        "VolumeDriver": "",
        "VolumesFrom": null,
    }

This creates container with networkMode as my-net. But when I run
the container using the rest API [POST]
http://localhost:2375/containers/{id}/start networkMode changes to
default.

Response of $docker inspect:

    "HostConfig": {
        "Binds": null,
        "ContainerIDFile": "",
        "LogConfig": {
            "Type": "json-file",
            "Config": {}
        },
        "NetworkMode": "default",
        "PortBindings": null,
        "RestartPolicy": {
            "Name": "",
            "MaximumRetryCount": 0
        },
        "AutoRemove": false,
        "VolumeDriver": "",
        "VolumesFrom": null,
    }

This only happens when using rest API not with command line API.

@phagunbaya
Copy link

phagunbaya commented Aug 5, 2016

So, there is definitely gap between the swarm container start and create API with respect to docker's. To configure networkMode, we need to pass hostConfig object as payload to container start API.

Example:

[POST] http://localhost:2375/containers/{id}/start

{
  "NetworkMode": "my-net",
  "PublishAllPorts": true
}

But this creates problem when I want to bind a host volume. After I put Binds property in the above payload, volume mounting does not work at all.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants