From 1df87b95066198c30312147393c18e0be0564fd0 Mon Sep 17 00:00:00 2001 From: Andrea Luzzardi Date: Thu, 25 Sep 2014 14:23:59 -0700 Subject: [PATCH] API: Provide the HostConfig during "run". Currently, the HostConfig is only passed from the CLI to Docker only when issuing a docker create, but not when doing a docker run. In the near future, in order to allocate ports at creation time rather than start time, we will need to have the HostConfig readily available at container creation. This PR makes the client always pass the HostConfig when creating a container (regardless of whether it's for a run or create). Signed-off-by: Andrea Luzzardi --- api/client/commands.go | 13 ++++--------- builder/internals.go | 4 ++-- daemon/create.go | 25 ++++++++++++++++--------- daemon/start.go | 2 ++ integration/container_test.go | 5 +++++ integration/runtime_test.go | 18 +++++++++++++----- integration/utils_test.go | 2 +- 7 files changed, 43 insertions(+), 26 deletions(-) diff --git a/api/client/commands.go b/api/client/commands.go index fa0f69f3d6..7ee6248d3b 100644 --- a/api/client/commands.go +++ b/api/client/commands.go @@ -2027,12 +2027,7 @@ func (cli *DockerCli) createContainer(config *runconfig.Config, hostConfig *runc containerValues.Set("name", name) } - var data interface{} - if hostConfig != nil { - data = runconfig.MergeConfigs(config, hostConfig) - } else { - data = config - } + mergedConfig := runconfig.MergeConfigs(config, hostConfig) var containerIDFile *cidFile if cidfile != "" { @@ -2044,7 +2039,7 @@ func (cli *DockerCli) createContainer(config *runconfig.Config, hostConfig *runc } //create the container - stream, statusCode, err := cli.call("POST", "/containers/create?"+containerValues.Encode(), data, false) + stream, statusCode, err := cli.call("POST", "/containers/create?"+containerValues.Encode(), mergedConfig, false) //if image not found try to pull it if statusCode == 404 { fmt.Fprintf(cli.err, "Unable to find image '%s' locally\n", config.Image) @@ -2053,7 +2048,7 @@ func (cli *DockerCli) createContainer(config *runconfig.Config, hostConfig *runc return nil, err } // Retry - if stream, _, err = cli.call("POST", "/containers/create?"+containerValues.Encode(), data, false); err != nil { + if stream, _, err = cli.call("POST", "/containers/create?"+containerValues.Encode(), mergedConfig, false); err != nil { return nil, err } } else if err != nil { @@ -2155,7 +2150,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { sigProxy = false } - runResult, err := cli.createContainer(config, nil, hostConfig.ContainerIDFile, *flName) + runResult, err := cli.createContainer(config, hostConfig, hostConfig.ContainerIDFile, *flName) if err != nil { return err } diff --git a/builder/internals.go b/builder/internals.go index 1ca95c5933..63907a4e6b 100644 --- a/builder/internals.go +++ b/builder/internals.go @@ -173,7 +173,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowDecomp return nil } - container, _, err := b.Daemon.Create(b.Config, "") + container, _, err := b.Daemon.Create(b.Config, nil, "") if err != nil { return err } @@ -442,7 +442,7 @@ func (b *Builder) create() (*daemon.Container, error) { config := *b.Config // Create the container - c, warnings, err := b.Daemon.Create(b.Config, "") + c, warnings, err := b.Daemon.Create(b.Config, nil, "") if err != nil { return nil, err } diff --git a/daemon/create.go b/daemon/create.go index a66831e786..3309d034c1 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -26,7 +26,16 @@ func (daemon *Daemon) ContainerCreate(job *engine.Job) engine.Status { job.Errorf("Your kernel does not support swap limit capabilities. Limitation discarded.\n") config.MemorySwap = -1 } - container, buildWarnings, err := daemon.Create(config, name) + + var hostConfig *runconfig.HostConfig + if job.EnvExists("HostConfig") { + hostConfig = runconfig.ContainerHostConfigFromJob(job) + } else { + // Older versions of the API don't provide a HostConfig. + hostConfig = nil + } + + container, buildWarnings, err := daemon.Create(config, hostConfig, name) if err != nil { if daemon.Graph().IsNotExist(err) { _, tag := parsers.ParseRepositoryTag(config.Image) @@ -51,18 +60,11 @@ func (daemon *Daemon) ContainerCreate(job *engine.Job) engine.Status { job.Errorf("%s\n", warning) } - if job.EnvExists("HostConfig") { - hostConfig := runconfig.ContainerHostConfigFromJob(job) - if err := daemon.setHostConfig(container, hostConfig); err != nil { - return job.Error(err) - } - } - return engine.StatusOK } // Create creates a new container from the given configuration with a given name. -func (daemon *Daemon) Create(config *runconfig.Config, name string) (*Container, []string, error) { +func (daemon *Daemon) Create(config *runconfig.Config, hostConfig *runconfig.HostConfig, name string) (*Container, []string, error) { var ( container *Container warnings []string @@ -84,6 +86,11 @@ func (daemon *Daemon) Create(config *runconfig.Config, name string) (*Container, if err := daemon.createRootfs(container, img); err != nil { return nil, nil, err } + if hostConfig != nil { + if err := daemon.setHostConfig(container, hostConfig); err != nil { + return nil, nil, err + } + } if err := container.ToDisk(); err != nil { return nil, nil, err } diff --git a/daemon/start.go b/daemon/start.go index 207dff85aa..f2c375ddc9 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -27,6 +27,8 @@ func (daemon *Daemon) ContainerStart(job *engine.Job) engine.Status { } // If no environment was set, then no hostconfig was passed. + // This is kept for backward compatibility - hostconfig should be passed when + // creating a container, not during start. if len(job.Environ()) > 0 { hostConfig := runconfig.ContainerHostConfigFromJob(job) if err := daemon.setHostConfig(container, hostConfig); err != nil { diff --git a/integration/container_test.go b/integration/container_test.go index 6486c5ad4b..ab94cbc679 100644 --- a/integration/container_test.go +++ b/integration/container_test.go @@ -18,6 +18,7 @@ func TestRestartStdin(t *testing.T) { OpenStdin: true, }, + &runconfig.HostConfig{}, "", ) if err != nil { @@ -94,6 +95,7 @@ func TestStdin(t *testing.T) { OpenStdin: true, }, + &runconfig.HostConfig{}, "", ) if err != nil { @@ -139,6 +141,7 @@ func TestTty(t *testing.T) { OpenStdin: true, }, + &runconfig.HostConfig{}, "", ) if err != nil { @@ -183,6 +186,7 @@ func BenchmarkRunSequential(b *testing.B) { Image: GetTestImage(daemon).ID, Cmd: []string{"echo", "-n", "foo"}, }, + &runconfig.HostConfig{}, "", ) if err != nil { @@ -216,6 +220,7 @@ func BenchmarkRunParallel(b *testing.B) { Image: GetTestImage(daemon).ID, Cmd: []string{"echo", "-n", "foo"}, }, + &runconfig.HostConfig{}, "", ) if err != nil { diff --git a/integration/runtime_test.go b/integration/runtime_test.go index e592bb67f1..6720485e28 100644 --- a/integration/runtime_test.go +++ b/integration/runtime_test.go @@ -266,6 +266,7 @@ func TestDaemonCreate(t *testing.T) { Image: GetTestImage(daemon).ID, Cmd: []string{"ls", "-al"}, }, + &runconfig.HostConfig{}, "", ) if err != nil { @@ -309,14 +310,15 @@ func TestDaemonCreate(t *testing.T) { Image: GetTestImage(daemon).ID, Cmd: []string{"ls", "-al"}, }, + &runconfig.HostConfig{}, "conflictname", ) - if _, _, err := daemon.Create(&runconfig.Config{Image: GetTestImage(daemon).ID, Cmd: []string{"ls", "-al"}}, testContainer.Name); err == nil || !strings.Contains(err.Error(), utils.TruncateID(testContainer.ID)) { + if _, _, err := daemon.Create(&runconfig.Config{Image: GetTestImage(daemon).ID, Cmd: []string{"ls", "-al"}}, &runconfig.HostConfig{}, testContainer.Name); err == nil || !strings.Contains(err.Error(), utils.TruncateID(testContainer.ID)) { t.Fatalf("Name conflict error doesn't include the correct short id. Message was: %s", err.Error()) } // Make sure create with bad parameters returns an error - if _, _, err = daemon.Create(&runconfig.Config{Image: GetTestImage(daemon).ID}, ""); err == nil { + if _, _, err = daemon.Create(&runconfig.Config{Image: GetTestImage(daemon).ID}, &runconfig.HostConfig{}, ""); err == nil { t.Fatal("Builder.Create should throw an error when Cmd is missing") } @@ -325,6 +327,7 @@ func TestDaemonCreate(t *testing.T) { Image: GetTestImage(daemon).ID, Cmd: []string{}, }, + &runconfig.HostConfig{}, "", ); err == nil { t.Fatal("Builder.Create should throw an error when Cmd is empty") @@ -335,7 +338,7 @@ func TestDaemonCreate(t *testing.T) { Cmd: []string{"/bin/ls"}, PortSpecs: []string{"80"}, } - container, _, err = daemon.Create(config, "") + container, _, err = daemon.Create(config, &runconfig.HostConfig{}, "") _, err = daemon.Commit(container, "testrepo", "testtag", "", "", true, config) if err != nil { @@ -348,6 +351,7 @@ func TestDaemonCreate(t *testing.T) { Cmd: []string{"ls", "-al"}, PortSpecs: []string{"80:8000"}, }, + &runconfig.HostConfig{}, "", ) if err != nil { @@ -365,7 +369,9 @@ func TestDestroy(t *testing.T) { container, _, err := daemon.Create(&runconfig.Config{ Image: GetTestImage(daemon).ID, Cmd: []string{"ls", "-al"}, - }, "") + }, + &runconfig.HostConfig{}, + "") if err != nil { t.Fatal(err) } @@ -857,7 +863,9 @@ func TestDestroyWithInitLayer(t *testing.T) { container, _, err := daemon.Create(&runconfig.Config{ Image: GetTestImage(daemon).ID, Cmd: []string{"ls", "-al"}, - }, "") + }, + &runconfig.HostConfig{}, + "") if err != nil { t.Fatal(err) diff --git a/integration/utils_test.go b/integration/utils_test.go index fc80f19744..e1abfa72fc 100644 --- a/integration/utils_test.go +++ b/integration/utils_test.go @@ -263,7 +263,7 @@ func mkContainer(r *daemon.Daemon, args []string, t *testing.T) (*daemon.Contain if config.Image == "_" { config.Image = GetTestImage(r).ID } - c, _, err := r.Create(config, "") + c, _, err := r.Create(config, nil, "") if err != nil { return nil, nil, err }