From fa3b1971578b3e270417df0215eeacaa03e881cf Mon Sep 17 00:00:00 2001 From: Qiang Huang Date: Wed, 24 Feb 2016 14:23:48 +0800 Subject: [PATCH] Restore container configs when update failed Signed-off-by: Qiang Huang --- daemon/update.go | 13 ++++++++++ .../docker_cli_update_unix_test.go | 25 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/daemon/update.go b/daemon/update.go index f89d914278..ccd0b2cc90 100644 --- a/daemon/update.go +++ b/daemon/update.go @@ -45,6 +45,17 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro return err } + restoreConfig := false + backupHostConfig := *container.HostConfig + defer func() { + if restoreConfig { + container.Lock() + container.HostConfig = &backupHostConfig + container.ToDisk() + container.Unlock() + } + }() + if container.RemovalInProgress || container.Dead { errMsg := fmt.Errorf("Container is marked for removal and cannot be \"update\".") return derr.ErrorCodeCantUpdate.WithArgs(container.ID, errMsg) @@ -56,6 +67,7 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro } if err := container.UpdateContainer(hostConfig); err != nil { + restoreConfig = true return derr.ErrorCodeCantUpdate.WithArgs(container.ID, err.Error()) } @@ -73,6 +85,7 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro // to the real world. if container.IsRunning() && !container.IsRestarting() { if err := daemon.execDriver.Update(container.Command); err != nil { + restoreConfig = true return derr.ErrorCodeCantUpdate.WithArgs(container.ID, err.Error()) } } diff --git a/integration-cli/docker_cli_update_unix_test.go b/integration-cli/docker_cli_update_unix_test.go index 3bdb17210a..c40ad3ee12 100644 --- a/integration-cli/docker_cli_update_unix_test.go +++ b/integration-cli/docker_cli_update_unix_test.go @@ -155,6 +155,31 @@ func (s *DockerSuite) TestUpdateSwapMemoryOnly(c *check.C) { c.Assert(strings.TrimSpace(out), checker.Equals, "629145600") } +func (s *DockerSuite) TestUpdateInvalidSwapMemory(c *check.C) { + testRequires(c, DaemonIsLinux) + testRequires(c, memoryLimitSupport) + testRequires(c, swapMemorySupport) + + name := "test-update-container" + dockerCmd(c, "run", "-d", "--name", name, "--memory", "300M", "--memory-swap", "500M", "busybox", "top") + _, _, err := dockerCmdWithError("update", "--memory-swap", "200M", name) + // Update invalid swap memory should fail. + // This will pass docker config validation, but failed at kernel validation + c.Assert(err, check.NotNil) + + // Update invalid swap memory with failure should not change HostConfig + c.Assert(inspectField(c, name, "HostConfig.Memory"), checker.Equals, "314572800") + c.Assert(inspectField(c, name, "HostConfig.MemorySwap"), checker.Equals, "524288000") + + dockerCmd(c, "update", "--memory-swap", "600M", name) + + c.Assert(inspectField(c, name, "HostConfig.MemorySwap"), checker.Equals, "629145600") + + file := "/sys/fs/cgroup/memory/memory.memsw.limit_in_bytes" + out, _ := dockerCmd(c, "exec", name, "cat", file) + c.Assert(strings.TrimSpace(out), checker.Equals, "629145600") +} + func (s *DockerSuite) TestUpdateStats(c *check.C) { testRequires(c, DaemonIsLinux) testRequires(c, memoryLimitSupport)