Skip to content

Commit

Permalink
daemon: don't checkpoint container until registered
Browse files Browse the repository at this point in the history
(*Container).CheckpointTo() upserts a snapshot of the container to the
daemon's in-memory ViewDB and also persists the snapshot to disk. It
does not register the live container object with the daemon's container
store, however. The ViewDB and container store are used as the source of
truth for different operations, so having a container registered in one
but not the other can result in inconsistencies. In particular, the List
Containers API uses the ViewDB as its source of truth and the Container
Inspect API uses the container store.

The (*Daemon).setHostConfig() method is called fairly early in the
process of creating a container, long before the container is registered
in the daemon's container store. Due to a rogue CheckpointTo() call
inside setHostConfig(), there is a window of time where a container can
be included in a List Containers API response but "not exist" according
to the Container Inspect API and similar endpoints which operate on a
particular container. Remove the rogue call so that the caller has full
control over when the container is checkpointed and update callers to
checkpoint explicitly. No changes to (*Daemon).create() are needed as it
checkpoints the fully-created container via (*Daemon).Register().

Fixes moby#44512.

Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 0141c6d)
Signed-off-by: Cory Snider <csnider@mirantis.com>
  • Loading branch information
corhere committed Dec 13, 2022
1 parent 42bffae commit 6149c33
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
2 changes: 1 addition & 1 deletion daemon/container.go
Expand Up @@ -235,7 +235,7 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *

runconfig.SetDefaultNetModeIfBlank(hostConfig)
container.HostConfig = hostConfig
return container.CheckpointTo(daemon.containersReplica)
return nil
}

// verifyContainerSettings performs validation of the hostconfig and config
Expand Down
6 changes: 3 additions & 3 deletions daemon/start.go
Expand Up @@ -67,9 +67,9 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos
// if user has change the network mode on starting, clean up the
// old networks. It is a deprecated feature and has been removed in Docker 1.12
ctr.NetworkSettings.Networks = nil
if err := ctr.CheckpointTo(daemon.containersReplica); err != nil {
return errdefs.System(err)
}
}
if err := ctr.CheckpointTo(daemon.containersReplica); err != nil {
return errdefs.System(err)
}
ctr.InitDNSHostConfig()
}
Expand Down

0 comments on commit 6149c33

Please sign in to comment.