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

[23.0 Backport] Prevent containers from being included in List API before they are registered #44633

Merged
merged 3 commits into from
Dec 14, 2022

Commits on Dec 13, 2022

  1. daemon: fix GetContainer() returning (nil, nil)

    GetContainer() would return (nil, nil) when looking up a container
    if the container was inserted into the containersReplica ViewDB but not
    the containers Store at the time of the lookup. Callers which reasonably
    assume that the returned err == nil implies returned container != nil
    would dereference a nil pointer and panic. Change GetContainer() so that
    it always returns a container or an error.
    
    Signed-off-by: Cory Snider <csnider@mirantis.com>
    (cherry picked from commit 00157a4)
    Signed-off-by: Cory Snider <csnider@mirantis.com>
    corhere committed Dec 13, 2022
    Configuration menu
    Copy the full SHA
    42bffae View commit details
    Browse the repository at this point in the history
  2. daemon: don't checkpoint container until registered

    (*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>
    corhere committed Dec 13, 2022
    Configuration menu
    Copy the full SHA
    6149c33 View commit details
    Browse the repository at this point in the history
  3. daemon: drop side effect from registerLinks()

    (*Daemon).registerLinks() calling the WriteHostConfig() method of its
    container argument is a vestigial behaviour. In the distant past,
    registerLinks() would persist the container links in an SQLite database
    and drop the link config from the container's persisted HostConfig. This
    changed in Docker v1.10 (moby#16032) which migrated away from SQLite and
    began using the link config in the container's HostConfig as the
    persistent source of truth. registerLinks() no longer mutates the
    HostConfig at all so persisting the HostConfig to disk falls outside of
    its scope of responsibilities.
    
    Signed-off-by: Cory Snider <csnider@mirantis.com>
    (cherry picked from commit 388fe4a)
    Signed-off-by: Cory Snider <csnider@mirantis.com>
    corhere committed Dec 13, 2022
    Configuration menu
    Copy the full SHA
    dca58c6 View commit details
    Browse the repository at this point in the history