From 481a92cb4111ebc1b7d4de5eeff84f570e4ba5dd Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Tue, 21 Feb 2017 15:55:59 -0800 Subject: [PATCH 01/18] Grab a lock to read container.RemovalInProgress Signed-off-by: Fabio Kung --- daemon/daemon.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 56474a49f0..aee0a59679 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -271,6 +271,7 @@ func (daemon *Daemon) restore() error { } } + c.Lock() if c.RemovalInProgress { // We probably crashed in the middle of a removal, reset // the flag. @@ -281,10 +282,11 @@ func (daemon *Daemon) restore() error { // be removed. So we put the container in the "dead" // state and leave further processing up to them. logrus.Debugf("Resetting RemovalInProgress flag from %v", c.ID) - c.ResetRemovalInProgress() - c.SetDead() + c.RemovalInProgress = false + c.Dead = true c.ToDisk() } + c.Unlock() }(c) } wg.Wait() From cfc404a375817125e4b32a9cd6a4ec7e3c55dc4e Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Wed, 22 Feb 2017 09:11:10 -0800 Subject: [PATCH 02/18] Move platform specific mount data to Container The Solaris version (previously daemon/inspect_solaris.go) was apparently missing some fields that should be available on that platform. Signed-off-by: Fabio Kung --- container/container_unix.go | 19 +++++++++++++++++++ container/container_windows.go | 17 +++++++++++++++++ daemon/inspect.go | 4 ++-- daemon/inspect_solaris.go | 14 -------------- daemon/inspect_unix.go | 17 ----------------- daemon/inspect_windows.go | 15 --------------- 6 files changed, 38 insertions(+), 48 deletions(-) diff --git a/container/container_unix.go b/container/container_unix.go index d53d057b52..c1233144b5 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/Sirupsen/logrus" + "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/pkg/chrootarchive" @@ -462,3 +463,21 @@ func cleanResourcePath(path string) string { func (container *Container) EnableServiceDiscoveryOnDefaultNetwork() bool { return false } + +// GetMountPoints gives a platform specific transformation to types.MountPoint. Callers must hold a Container lock. +func (container *Container) GetMountPoints() []types.MountPoint { + mountPoints := make([]types.MountPoint, 0, len(container.MountPoints)) + for _, m := range container.MountPoints { + mountPoints = append(mountPoints, types.MountPoint{ + Type: m.Type, + Name: m.Name, + Source: m.Path(), + Destination: m.Destination, + Driver: m.Driver, + Mode: m.Mode, + RW: m.RW, + Propagation: m.Propagation, + }) + } + return mountPoints +} diff --git a/container/container_windows.go b/container/container_windows.go index fe11140a29..5f19351090 100644 --- a/container/container_windows.go +++ b/container/container_windows.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" + "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/pkg/system" ) @@ -194,3 +195,19 @@ func (container *Container) BuildHostnameFile() error { func (container *Container) EnableServiceDiscoveryOnDefaultNetwork() bool { return true } + +// GetMountPoints gives a platform specific transformation to types.MountPoint. Callers must hold a Container lock. +func (container *Container) GetMountPoints() []types.MountPoint { + mountPoints := make([]types.MountPoint, 0, len(container.MountPoints)) + for _, m := range container.MountPoints { + mountPoints = append(mountPoints, types.MountPoint{ + Type: m.Type, + Name: m.Name, + Source: m.Path(), + Destination: m.Destination, + Driver: m.Driver, + RW: m.RW, + }) + } + return mountPoints +} diff --git a/daemon/inspect.go b/daemon/inspect.go index c981e7701d..47c1ba4184 100644 --- a/daemon/inspect.go +++ b/daemon/inspect.go @@ -51,7 +51,7 @@ func (daemon *Daemon) ContainerInspectCurrent(name string, size bool) (*types.Co } } - mountPoints := addMountPoints(container) + mountPoints := container.GetMountPoints() networkSettings := &types.NetworkSettings{ NetworkSettingsBase: types.NetworkSettingsBase{ Bridge: container.NetworkSettings.Bridge, @@ -104,7 +104,7 @@ func (daemon *Daemon) containerInspect120(name string) (*v1p20.ContainerJSON, er return nil, err } - mountPoints := addMountPoints(container) + mountPoints := container.GetMountPoints() config := &v1p20.ContainerConfig{ Config: container.Config, MacAddress: container.Config.MacAddress, diff --git a/daemon/inspect_solaris.go b/daemon/inspect_solaris.go index 0e3dcc1119..0b275c1418 100644 --- a/daemon/inspect_solaris.go +++ b/daemon/inspect_solaris.go @@ -18,20 +18,6 @@ func (daemon *Daemon) containerInspectPre120(name string) (*v1p19.ContainerJSON, return &v1p19.ContainerJSON{}, nil } -func addMountPoints(container *container.Container) []types.MountPoint { - mountPoints := make([]types.MountPoint, 0, len(container.MountPoints)) - for _, m := range container.MountPoints { - mountPoints = append(mountPoints, types.MountPoint{ - Name: m.Name, - Source: m.Path(), - Destination: m.Destination, - Driver: m.Driver, - RW: m.RW, - }) - } - return mountPoints -} - func inspectExecProcessConfig(e *exec.Config) *backend.ExecProcessConfig { return &backend.ExecProcessConfig{ Tty: e.Tty, diff --git a/daemon/inspect_unix.go b/daemon/inspect_unix.go index 8342f7cf98..bd28481e6a 100644 --- a/daemon/inspect_unix.go +++ b/daemon/inspect_unix.go @@ -64,23 +64,6 @@ func (daemon *Daemon) containerInspectPre120(name string) (*v1p19.ContainerJSON, }, nil } -func addMountPoints(container *container.Container) []types.MountPoint { - mountPoints := make([]types.MountPoint, 0, len(container.MountPoints)) - for _, m := range container.MountPoints { - mountPoints = append(mountPoints, types.MountPoint{ - Type: m.Type, - Name: m.Name, - Source: m.Path(), - Destination: m.Destination, - Driver: m.Driver, - Mode: m.Mode, - RW: m.RW, - Propagation: m.Propagation, - }) - } - return mountPoints -} - func inspectExecProcessConfig(e *exec.Config) *backend.ExecProcessConfig { return &backend.ExecProcessConfig{ Tty: e.Tty, diff --git a/daemon/inspect_windows.go b/daemon/inspect_windows.go index b331c83ca3..5b12902dbc 100644 --- a/daemon/inspect_windows.go +++ b/daemon/inspect_windows.go @@ -12,21 +12,6 @@ func setPlatformSpecificContainerFields(container *container.Container, contJSON return contJSONBase } -func addMountPoints(container *container.Container) []types.MountPoint { - mountPoints := make([]types.MountPoint, 0, len(container.MountPoints)) - for _, m := range container.MountPoints { - mountPoints = append(mountPoints, types.MountPoint{ - Type: m.Type, - Name: m.Name, - Source: m.Path(), - Destination: m.Destination, - Driver: m.Driver, - RW: m.RW, - }) - } - return mountPoints -} - // containerInspectPre120 get containers for pre 1.20 APIs. func (daemon *Daemon) containerInspectPre120(name string) (*types.ContainerJSON, error) { return daemon.ContainerInspectCurrent(name, false) From 054728b1f555892c6c0bfd7abfbaeb2fedbc8f10 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Wed, 22 Feb 2017 10:00:50 -0800 Subject: [PATCH 03/18] in-memory ACID store for containers This can be used by readers/queries so they don't need locks. Signed-off-by: Fabio Kung --- container/snapshot.go | 152 +++++++++++++++++++++++++++++++++++++++++ container/view.go | 90 ++++++++++++++++++++++++ container/view_test.go | 58 ++++++++++++++++ 3 files changed, 300 insertions(+) create mode 100644 container/snapshot.go create mode 100644 container/view.go create mode 100644 container/view_test.go diff --git a/container/snapshot.go b/container/snapshot.go new file mode 100644 index 0000000000..a56cabcb96 --- /dev/null +++ b/container/snapshot.go @@ -0,0 +1,152 @@ +package container + +import ( + "fmt" + "strings" + "time" + + "github.com/Sirupsen/logrus" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/network" + "github.com/docker/go-connections/nat" +) + +// Snapshot is a read only view for Containers +type Snapshot struct { + ID string `json:"Id"` + Name string + Pid int + Managed bool + Image string + ImageID string + Command string + Ports []types.Port + ExposedPorts nat.PortSet + PublishPorts nat.PortSet + Labels map[string]string + State string + Status string + Health string + HostConfig struct { + NetworkMode string + Isolation string + } + NetworkSettings types.SummaryNetworkSettings + Mounts []types.MountPoint + Created time.Time + StartedAt time.Time + Running bool + Paused bool + ExitCode int +} + +// Snapshot provides a read only view of a Container. Callers must hold a Lock on the container object. +func (container *Container) Snapshot() *Snapshot { + snapshot := &Snapshot{ + ID: container.ID, + Name: container.Name, + Pid: container.Pid, + Managed: container.Managed, + ImageID: container.ImageID.String(), + Ports: []types.Port{}, + ExposedPorts: make(nat.PortSet), + PublishPorts: make(nat.PortSet), + State: container.State.StateString(), + Status: container.State.String(), + Health: container.State.HealthString(), + Mounts: container.GetMountPoints(), + Created: container.Created, + StartedAt: container.StartedAt, + Running: container.Running, + Paused: container.Paused, + ExitCode: container.ExitCode(), + } + + if container.HostConfig != nil { + snapshot.HostConfig.Isolation = string(container.HostConfig.Isolation) + snapshot.HostConfig.NetworkMode = string(container.HostConfig.NetworkMode) + for publish := range container.HostConfig.PortBindings { + snapshot.PublishPorts[publish] = struct{}{} + } + } + + if container.Config != nil { + snapshot.Image = container.Config.Image + snapshot.Labels = container.Config.Labels + for exposed := range container.Config.ExposedPorts { + snapshot.ExposedPorts[exposed] = struct{}{} + } + } + + if len(container.Args) > 0 { + args := []string{} + for _, arg := range container.Args { + if strings.Contains(arg, " ") { + args = append(args, fmt.Sprintf("'%s'", arg)) + } else { + args = append(args, arg) + } + } + argsAsString := strings.Join(args, " ") + snapshot.Command = fmt.Sprintf("%s %s", container.Path, argsAsString) + } else { + snapshot.Command = container.Path + } + + if container.NetworkSettings != nil { + networks := make(map[string]*network.EndpointSettings) + for name, netw := range container.NetworkSettings.Networks { + if netw == nil || netw.EndpointSettings == nil { + continue + } + networks[name] = &network.EndpointSettings{ + EndpointID: netw.EndpointID, + Gateway: netw.Gateway, + IPAddress: netw.IPAddress, + IPPrefixLen: netw.IPPrefixLen, + IPv6Gateway: netw.IPv6Gateway, + GlobalIPv6Address: netw.GlobalIPv6Address, + GlobalIPv6PrefixLen: netw.GlobalIPv6PrefixLen, + MacAddress: netw.MacAddress, + NetworkID: netw.NetworkID, + } + if netw.IPAMConfig != nil { + networks[name].IPAMConfig = &network.EndpointIPAMConfig{ + IPv4Address: netw.IPAMConfig.IPv4Address, + IPv6Address: netw.IPAMConfig.IPv6Address, + } + } + } + snapshot.NetworkSettings = types.SummaryNetworkSettings{Networks: networks} + for port, bindings := range container.NetworkSettings.Ports { + p, err := nat.ParsePort(port.Port()) + if err != nil { + logrus.Warnf("invalid port map %+v", err) + continue + } + if len(bindings) == 0 { + snapshot.Ports = append(snapshot.Ports, types.Port{ + PrivatePort: uint16(p), + Type: port.Proto(), + }) + continue + } + for _, binding := range bindings { + h, err := nat.ParsePort(binding.HostPort) + if err != nil { + logrus.Warnf("invalid host port map %+v", err) + continue + } + snapshot.Ports = append(snapshot.Ports, types.Port{ + PrivatePort: uint16(p), + PublicPort: uint16(h), + Type: port.Proto(), + IP: binding.HostIP, + }) + } + } + + } + + return snapshot +} diff --git a/container/view.go b/container/view.go new file mode 100644 index 0000000000..aa122fe2c7 --- /dev/null +++ b/container/view.go @@ -0,0 +1,90 @@ +package container + +import "github.com/hashicorp/go-memdb" + +const ( + memdbTable = "containers" + memdbIDField = "ID" + memdbIDIndex = "id" +) + +var schema = &memdb.DBSchema{ + Tables: map[string]*memdb.TableSchema{ + memdbTable: { + Name: memdbTable, + Indexes: map[string]*memdb.IndexSchema{ + memdbIDIndex: { + Name: memdbIDIndex, + Unique: true, + Indexer: &memdb.StringFieldIndex{Field: memdbIDField}, + }, + }, + }, + }, +} + +// MemDB provides an in-memory transactional (ACID) container Store +type MemDB struct { + store *memdb.MemDB +} + +// NewMemDB provides the default implementation, with the default schema +func NewMemDB() (*MemDB, error) { + store, err := memdb.NewMemDB(schema) + if err != nil { + return nil, err + } + return &MemDB{store: store}, nil +} + +// Snapshot provides a consistent read-only View of the database +func (db *MemDB) Snapshot() *View { + return &View{db.store.Txn(false)} +} + +// Save atomically updates the in-memory store +func (db *MemDB) Save(snapshot *Snapshot) error { + txn := db.store.Txn(true) + defer txn.Commit() + return txn.Insert(memdbTable, snapshot) +} + +// Delete removes an item by ID +func (db *MemDB) Delete(id string) error { + txn := db.store.Txn(true) + defer txn.Commit() + return txn.Delete(memdbTable, &Snapshot{ID: id}) +} + +// View can be used by readers to avoid locking +type View struct { + txn *memdb.Txn +} + +// All returns a all items in this snapshot +func (v *View) All() ([]Snapshot, error) { + var all []Snapshot + iter, err := v.txn.Get(memdbTable, memdbIDIndex) + if err != nil { + return nil, err + } + for { + item := iter.Next() + if item == nil { + break + } + snapshot := *(item.(*Snapshot)) // force a copy + all = append(all, snapshot) + } + return all, nil +} + +//Get returns an item by id +func (v *View) Get(id string) (*Snapshot, error) { + s, err := v.txn.First(memdbTable, memdbIDIndex, id) + if err != nil { + return nil, err + } + snapshot := *(s.(*Snapshot)) // force a copy + return &snapshot, nil +} diff --git a/container/view_test.go b/container/view_test.go new file mode 100644 index 0000000000..32bdf4957c --- /dev/null +++ b/container/view_test.go @@ -0,0 +1,58 @@ +package container + +import "testing" + +func TestViewSave(t *testing.T) { + db, err := NewMemDB() + if err != nil { + t.Fatal(err) + } + snapshot := NewBaseContainer("id", "root").Snapshot() + if err := db.Save(snapshot); err != nil { + t.Fatal(err) + + } +} + +func TestViewAll(t *testing.T) { + var ( + db, _ = NewMemDB() + one = NewBaseContainer("id1", "root1").Snapshot() + two = NewBaseContainer("id2", "root2").Snapshot() + ) + one.Pid = 10 + two.Pid = 20 + db.Save(one) + db.Save(two) + all, err := db.Snapshot().All() + if err != nil { + t.Fatal(err) + } + if l := len(all); l != 2 { + t.Fatalf("expected 2 items, got %d", l) + } + byID := make(map[string]Snapshot) + for i := range all { + byID[all[i].ID] = all[i] + } + if s, ok := byID["id1"]; !ok || s.Pid != 10 { + t.Fatalf("expected something different with for id1: %v", s) + } + if s, ok := byID["id2"]; !ok || s.Pid != 20 { + t.Fatalf("expected something different with for id1: %v", s) + } +} + +func TestViewGet(t *testing.T) { + db, _ := NewMemDB() + one := NewBaseContainer("id", "root") + one.ImageID = "some-image-123" + db.Save(one.Snapshot()) + s, err := db.Snapshot().Get("id") + if err != nil { + t.Fatal(err) + } + if s == nil || s.ImageID != "some-image-123" { + t.Fatalf("expected something different. Got: %v", s) + } +} From eed4c7b73f0cf98cf48943da1c082f3210b28c82 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Wed, 22 Feb 2017 14:02:20 -0800 Subject: [PATCH 04/18] keep a consistent view of containers rendered Replicate relevant mutations to the in-memory ACID store. Readers will then be able to query container state without locking. Signed-off-by: Fabio Kung --- container/container_unix.go | 5 +---- container/container_windows.go | 5 +---- daemon/container.go | 11 ++++++++++- daemon/container_operations.go | 23 ++++++++++++++++++----- daemon/create.go | 4 +++- daemon/daemon.go | 10 +++++++++- daemon/delete.go | 11 +++++++++-- daemon/health.go | 7 +++++++ daemon/health_test.go | 17 +++++++++++++++-- daemon/monitor.go | 13 +++++++++++++ daemon/rename.go | 6 ++++++ daemon/start.go | 8 ++++++-- daemon/update.go | 9 +++++++++ 13 files changed, 107 insertions(+), 22 deletions(-) diff --git a/container/container_unix.go b/container/container_unix.go index c1233144b5..deab000b86 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -262,11 +262,8 @@ func (container *Container) ConfigMounts() []Mount { return mounts } -// UpdateContainer updates configuration of a container. +// UpdateContainer updates configuration of a container. Callers must hold a Lock on the Container. func (container *Container) UpdateContainer(hostConfig *containertypes.HostConfig) error { - container.Lock() - defer container.Unlock() - // update resources of container resources := hostConfig.Resources cResources := &container.HostConfig.Resources diff --git a/container/container_windows.go b/container/container_windows.go index 5f19351090..0f2a45df99 100644 --- a/container/container_windows.go +++ b/container/container_windows.go @@ -126,11 +126,8 @@ func (container *Container) TmpfsMounts() ([]Mount, error) { return mounts, nil } -// UpdateContainer updates configuration of a container +// UpdateContainer updates configuration of a container. Callers must hold a Lock on the Container. func (container *Container) UpdateContainer(hostConfig *containertypes.HostConfig) error { - container.Lock() - defer container.Unlock() - resources := hostConfig.Resources if resources.CPUShares != 0 || resources.Memory != 0 || diff --git a/daemon/container.go b/daemon/container.go index aaa6d1ce75..5d0d0438e4 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -99,7 +99,7 @@ func (daemon *Daemon) load(id string) (*container.Container, error) { } // Register makes a container object usable by the daemon as -func (daemon *Daemon) Register(c *container.Container) { +func (daemon *Daemon) Register(c *container.Container) error { // Attach to stdout and stderr if c.Config.OpenStdin { c.StreamConfig.NewInputPipes() @@ -107,8 +107,14 @@ func (daemon *Daemon) Register(c *container.Container) { c.StreamConfig.NewNopInputPipe() } + // once in the memory store it is visible to other goroutines + // grab a Lock until it has been replicated to avoid races + c.Lock() + defer c.Unlock() + daemon.containers.Add(c.ID, c) daemon.idIndex.Add(c.ID) + return daemon.containersReplica.Save(c.Snapshot()) } func (daemon *Daemon) newContainer(name string, platform string, config *containertypes.Config, hostConfig *containertypes.HostConfig, imgID image.ID, managed bool) (*container.Container, error) { @@ -212,6 +218,9 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig * runconfig.SetDefaultNetModeIfBlank(hostConfig) container.HostConfig = hostConfig + if err := daemon.containersReplica.Save(container.Snapshot()); err != nil { + return err + } return container.ToDisk() } diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 1b6f02d0a1..fe83928472 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -44,6 +44,19 @@ func (daemon *Daemon) getDNSSearchSettings(container *container.Container) []str return nil } + +func (daemon *Daemon) saveAndReplicate(container *container.Container) error { + container.Lock() + defer container.Unlock() + if err := daemon.containersReplica.Save(container.Snapshot()); err != nil { + return fmt.Errorf("Error replicating container state: %v", err) + } + if err := container.ToDisk(); err != nil { + return fmt.Errorf("Error saving container to disk: %v", err) + } + return nil +} + func (daemon *Daemon) buildSandboxOptions(container *container.Container) ([]libnetwork.SandboxOption, error) { var ( sboxOptions []libnetwork.SandboxOption @@ -1005,7 +1018,7 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName return err } } - if err := container.ToDisk(); err != nil { + if err := daemon.saveAndReplicate(container); err != nil { return fmt.Errorf("Error saving container to disk: %v", err) } return nil @@ -1044,16 +1057,16 @@ func (daemon *Daemon) DisconnectFromNetwork(container *container.Container, netw return err } - if err := container.ToDisk(); err != nil { + if err := daemon.saveAndReplicate(container); err != nil { return fmt.Errorf("Error saving container to disk: %v", err) } if n != nil { - attributes := map[string]string{ + daemon.LogNetworkEventWithAttributes(n, "disconnect", map[string]string{ "container": container.ID, - } - daemon.LogNetworkEventWithAttributes(n, "disconnect", attributes) + }) } + return nil } diff --git a/daemon/create.go b/daemon/create.go index addc9b718a..a39dde5114 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -172,7 +172,9 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) ( logrus.Errorf("Error saving new container to disk: %v", err) return nil, err } - daemon.Register(container) + if err := daemon.Register(container); err != nil { + return nil, err + } stateCtr.set(container.ID, "stopped") daemon.LogContainerEvent(container, "create") return container, nil diff --git a/daemon/daemon.go b/daemon/daemon.go index aee0a59679..f2a83cc4d0 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -83,6 +83,7 @@ type Daemon struct { ID string repository string containers container.Store + containersReplica *container.MemDB execCommands *exec.Store downloadManager *xfer.LayerDownloadManager uploadManager *xfer.LayerUploadManager @@ -182,11 +183,15 @@ func (daemon *Daemon) restore() error { activeSandboxes := make(map[string]interface{}) for id, c := range containers { if err := daemon.registerName(c); err != nil { + logrus.Errorf("Failed to register container name %s: %s", c.ID, err) + delete(containers, id) + continue + } + if err := daemon.Register(c); err != nil { logrus.Errorf("Failed to register container %s: %s", c.ID, err) delete(containers, id) continue } - daemon.Register(c) // verify that all volumes valid and have been migrated from the pre-1.7 layout if err := daemon.verifyVolumesInfo(c); err != nil { @@ -757,6 +762,9 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe d.ID = trustKey.PublicKey().KeyID() d.repository = daemonRepo d.containers = container.NewMemoryStore() + if d.containersReplica, err = container.NewMemDB(); err != nil { + return nil, err + } d.execCommands = exec.NewStore() d.trustKey = trustKey d.idIndex = truncindex.NewTruncIndex([]string{}) diff --git a/daemon/delete.go b/daemon/delete.go index 3dcd289101..ad5eacc61c 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -103,14 +103,20 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo } // Mark container dead. We don't want anybody to be restarting it. - container.SetDead() + container.Lock() + container.Dead = true + if err = daemon.containersReplica.Save(container.Snapshot()); err != nil { + container.Unlock() + return err + } // Save container state to disk. So that if error happens before // container meta file got removed from disk, then a restart of // docker should not make a dead container alive. - if err := container.ToDiskLocking(); err != nil && !os.IsNotExist(err) { + if err := container.ToDisk(); err != nil && !os.IsNotExist(err) { logrus.Errorf("Error saving dying container to disk: %v", err) } + container.Unlock() // When container creation fails and `RWLayer` has not been created yet, we // do not call `ReleaseRWLayer` @@ -131,6 +137,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo selinuxFreeLxcContexts(container.ProcessLabel) daemon.idIndex.Delete(container.ID) daemon.containers.Delete(container.ID) + daemon.containersReplica.Delete(container.ID) if e := daemon.removeMountPoints(container, removeVolume); e != nil { logrus.Error(e) } diff --git a/daemon/health.go b/daemon/health.go index 48cf4c4255..3419511d36 100644 --- a/daemon/health.go +++ b/daemon/health.go @@ -167,6 +167,13 @@ func handleProbeResult(d *Daemon, c *container.Container, result *types.Healthch // Else we're starting or healthy. Stay in that state. } + // replicate Health status changes + if err := d.containersReplica.Save(c.Snapshot()); err != nil { + // queries will be inconsistent until the next probe runs or other state mutations + // trigger a replication + logrus.Errorf("Error replicating health state for container %s: %v", c.ID, err) + } + if oldStatus != h.Status { d.LogContainerEvent(c, "health_status: "+h.Status) } diff --git a/daemon/health_test.go b/daemon/health_test.go index 7347e7d791..0944e3fa33 100644 --- a/daemon/health_test.go +++ b/daemon/health_test.go @@ -29,7 +29,13 @@ func TestNoneHealthcheck(t *testing.T) { }, State: &container.State{}, } - daemon := &Daemon{} + store, err := container.NewMemDB() + if err != nil { + t.Fatal(err) + } + daemon := &Daemon{ + containersReplica: store, + } daemon.initHealthMonitor(c) if c.State.Health != nil { @@ -62,8 +68,15 @@ func TestHealthStates(t *testing.T) { Image: "image_name", }, } + + store, err := container.NewMemDB() + if err != nil { + t.Fatal(err) + } + daemon := &Daemon{ - EventsService: e, + EventsService: e, + containersReplica: store, } c.Config.Healthcheck = &containertypes.HealthConfig{ diff --git a/daemon/monitor.go b/daemon/monitor.go index 006db3db21..a581230ce5 100644 --- a/daemon/monitor.go +++ b/daemon/monitor.go @@ -90,6 +90,9 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { daemon.setStateCounter(c) defer c.Unlock() + if err := daemon.containersReplica.Save(c.Snapshot()); err != nil { + return err + } if err := c.ToDisk(); err != nil { return err } @@ -119,6 +122,10 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { c.HasBeenStartedBefore = true daemon.setStateCounter(c) + if err := daemon.containersReplica.Save(c.Snapshot()); err != nil { + c.Reset(false) + return err + } if err := c.ToDisk(); err != nil { c.Reset(false) return err @@ -130,6 +137,9 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { // Container is already locked in this case c.Paused = true daemon.setStateCounter(c) + if err := daemon.containersReplica.Save(c.Snapshot()); err != nil { + return err + } if err := c.ToDisk(); err != nil { return err } @@ -139,6 +149,9 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { // Container is already locked in this case c.Paused = false daemon.setStateCounter(c) + if err := daemon.containersReplica.Save(c.Snapshot()); err != nil { + return err + } if err := c.ToDisk(); err != nil { return err } diff --git a/daemon/rename.go b/daemon/rename.go index 2770ead80c..0a3730ff16 100644 --- a/daemon/rename.go +++ b/daemon/rename.go @@ -82,6 +82,9 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { daemon.nameIndex.Release(oldName + k) } daemon.releaseName(oldName) + if err = daemon.containersReplica.Save(container.Snapshot()); err != nil { + return err + } if err = container.ToDisk(); err != nil { return err } @@ -99,6 +102,9 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { if err != nil { container.Name = oldName container.NetworkSettings.IsAnonymousEndpoint = oldIsAnonymousEndpoint + if e := daemon.containersReplica.Save(container.Snapshot()); err != nil { + logrus.Errorf("%s: Failed in replicating state on rename failure: %v", container.ID, e) + } if e := container.ToDisk(); e != nil { logrus.Errorf("%s: Failed in writing to Disk on rename failure: %v", container.ID, e) } diff --git a/daemon/start.go b/daemon/start.go index 61bc32f586..5d8a1286aa 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -117,8 +117,12 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint if container.ExitCode() == 0 { container.SetExitCode(128) } - container.ToDisk() - + if err := daemon.containersReplica.Save(container.Snapshot()); err != nil { + logrus.Errorf("%s: failed replicating state on start failure: %v", container.ID, err) + } + if err := container.ToDisk(); err != nil { + logrus.Errorf("%s: failed writing to disk on start failure: %v", container.ID, err) + } container.Reset(false) daemon.Cleanup(container) diff --git a/daemon/update.go b/daemon/update.go index 76e4a3f93f..bdcfa2d067 100644 --- a/daemon/update.go +++ b/daemon/update.go @@ -38,6 +38,7 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro if restoreConfig { container.Lock() container.HostConfig = &backupHostConfig + daemon.containersReplica.Save(container.Snapshot()) container.ToDisk() container.Unlock() } @@ -47,10 +48,18 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro return errCannotUpdate(container.ID, fmt.Errorf("Container is marked for removal and cannot be \"update\".")) } + container.Lock() if err := container.UpdateContainer(hostConfig); err != nil { restoreConfig = true + container.Unlock() return errCannotUpdate(container.ID, err) } + if err := daemon.containersReplica.Save(container.Snapshot()); err != nil { + restoreConfig = true + container.Unlock() + return errCannotUpdate(container.ID, err) + } + container.Unlock() // if Restart Policy changed, we need to update container monitor if hostConfig.RestartPolicy.Name != "" { From 8e425ebc422876ddf2ffb3beaa5a0443a6097e46 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Wed, 22 Feb 2017 15:01:46 -0800 Subject: [PATCH 05/18] stop grabbing container locks during ps Container queries are now served from the consistent in-memory db, and don't need to grab a lock on every container being listed. Signed-off-by: Fabio Kung --- daemon/list.go | 189 ++++++++++++++--------------------------- daemon/list_unix.go | 2 +- daemon/list_windows.go | 2 +- 3 files changed, 68 insertions(+), 125 deletions(-) diff --git a/daemon/list.go b/daemon/list.go index e6909173e9..b810164f28 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -10,7 +10,6 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/filters" - networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/container" "github.com/docker/docker/image" "github.com/docker/docker/volume" @@ -47,7 +46,7 @@ type iterationAction int // containerReducer represents a reducer for a container. // Returns the object to serialize by the api. -type containerReducer func(*container.Container, *listContext) (*types.Container, error) +type containerReducer func(*container.Snapshot, *listContext) (*types.Container, error) const ( // includeContainer is the action to include a container in the reducer. @@ -83,9 +82,9 @@ type listContext struct { exitAllowed []int // beforeFilter is a filter to ignore containers that appear before the one given - beforeFilter *container.Container + beforeFilter *container.Snapshot // sinceFilter is a filter to stop the filtering when the iterator arrive to the given container - sinceFilter *container.Container + sinceFilter *container.Snapshot // taskFilter tells if we should filter based on wether a container is part of a task taskFilter bool @@ -102,7 +101,7 @@ type listContext struct { } // byContainerCreated is a temporary type used to sort a list of containers by creation time. -type byContainerCreated []*container.Container +type byContainerCreated []container.Snapshot func (r byContainerCreated) Len() int { return len(r) } func (r byContainerCreated) Swap(i, j int) { r[i], r[j] = r[j], r[i] } @@ -115,7 +114,7 @@ func (daemon *Daemon) Containers(config *types.ContainerListOptions) ([]*types.C return daemon.reduceContainers(config, daemon.transformContainer) } -func (daemon *Daemon) filterByNameIDMatches(ctx *listContext) []*container.Container { +func (daemon *Daemon) filterByNameIDMatches(view *container.View, ctx *listContext) ([]container.Snapshot, error) { idSearch := false names := ctx.filters.Get("name") ids := ctx.filters.Get("id") @@ -123,7 +122,9 @@ func (daemon *Daemon) filterByNameIDMatches(ctx *listContext) []*container.Conta // if name or ID filters are not in use, return to // standard behavior of walking the entire container // list from the daemon's in-memory store - return daemon.List() + all, err := view.All() + sort.Sort(sort.Reverse(byContainerCreated(all))) + return all, err } // idSearch will determine if we limit name matching to the IDs @@ -158,10 +159,14 @@ func (daemon *Daemon) filterByNameIDMatches(ctx *listContext) []*container.Conta } } - cntrs := make([]*container.Container, 0, len(matches)) + cntrs := make([]container.Snapshot, 0, len(matches)) for id := range matches { - if c := daemon.containers.Get(id); c != nil { - cntrs = append(cntrs, c) + c, err := view.Get(id) + if err != nil { + return nil, err + } + if c != nil { + cntrs = append(cntrs, *c) } } @@ -169,27 +174,31 @@ func (daemon *Daemon) filterByNameIDMatches(ctx *listContext) []*container.Conta // Created gives us nanosec resolution for sorting sort.Sort(sort.Reverse(byContainerCreated(cntrs))) - return cntrs + return cntrs, nil } // reduceContainers parses the user's filtering options and generates the list of containers to return based on a reducer. func (daemon *Daemon) reduceContainers(config *types.ContainerListOptions, reducer containerReducer) ([]*types.Container, error) { var ( + view = daemon.containersReplica.Snapshot() containers = []*types.Container{} ) - ctx, err := daemon.foldFilter(config) + ctx, err := daemon.foldFilter(view, config) if err != nil { return nil, err } // fastpath to only look at a subset of containers if specific name // or ID matches were provided by the user--otherwise we potentially - // end up locking and querying many more containers than intended - containerList := daemon.filterByNameIDMatches(ctx) + // end up querying many more containers than intended + containerList, err := daemon.filterByNameIDMatches(view, ctx) + if err != nil { + return nil, err + } - for _, container := range containerList { - t, err := daemon.reducePsContainer(container, ctx, reducer) + for i := range containerList { + t, err := daemon.reducePsContainer(&containerList[i], ctx, reducer) if err != nil { if err != errStopIteration { return nil, err @@ -206,23 +215,17 @@ func (daemon *Daemon) reduceContainers(config *types.ContainerListOptions, reduc } // reducePsContainer is the basic representation for a container as expected by the ps command. -func (daemon *Daemon) reducePsContainer(container *container.Container, ctx *listContext, reducer containerReducer) (*types.Container, error) { - container.Lock() - +func (daemon *Daemon) reducePsContainer(container *container.Snapshot, ctx *listContext, reducer containerReducer) (*types.Container, error) { // filter containers to return - action := includeContainerInList(container, ctx) - switch action { + switch includeContainerInList(container, ctx) { case excludeContainer: - container.Unlock() return nil, nil case stopIteration: - container.Unlock() return nil, errStopIteration } // transform internal container struct into api structs newC, err := reducer(container, ctx) - container.Unlock() if err != nil { return nil, err } @@ -237,7 +240,7 @@ func (daemon *Daemon) reducePsContainer(container *container.Container, ctx *lis } // foldFilter generates the container filter based on the user's filtering options. -func (daemon *Daemon) foldFilter(config *types.ContainerListOptions) (*listContext, error) { +func (daemon *Daemon) foldFilter(view *container.View, config *types.ContainerListOptions) (*listContext, error) { psFilters := config.Filters if err := psFilters.Validate(acceptedPsFilterTags); err != nil { @@ -294,10 +297,10 @@ func (daemon *Daemon) foldFilter(config *types.ContainerListOptions) (*listConte return nil, err } - var beforeContFilter, sinceContFilter *container.Container + var beforeContFilter, sinceContFilter *container.Snapshot err = psFilters.WalkValues("before", func(value string) error { - beforeContFilter, err = daemon.GetContainer(value) + beforeContFilter, err = view.Get(value) return err }) if err != nil { @@ -305,7 +308,7 @@ func (daemon *Daemon) foldFilter(config *types.ContainerListOptions) (*listConte } err = psFilters.WalkValues("since", func(value string) error { - sinceContFilter, err = daemon.GetContainer(value) + sinceContFilter, err = view.Get(value) return err }) if err != nil { @@ -383,7 +386,7 @@ func portOp(key string, filter map[nat.Port]bool) func(value string) error { // includeContainerInList decides whether a container should be included in the output or not based in the filter. // It also decides if the iteration should be stopped or not. -func includeContainerInList(container *container.Container, ctx *listContext) iterationAction { +func includeContainerInList(container *container.Snapshot, ctx *listContext) iterationAction { // Do not include container if it's in the list before the filter container. // Set the filter container to nil to include the rest of containers after this one. if ctx.beforeFilter != nil { @@ -422,7 +425,7 @@ func includeContainerInList(container *container.Container, ctx *listContext) it } // Do not include container if any of the labels don't match - if !ctx.filters.MatchKVList("label", container.Config.Labels) { + if !ctx.filters.MatchKVList("label", container.Labels) { return excludeContainer } @@ -440,7 +443,7 @@ func includeContainerInList(container *container.Container, ctx *listContext) it if len(ctx.exitAllowed) > 0 { shouldSkip := true for _, code := range ctx.exitAllowed { - if code == container.ExitCode() && !container.Running && !container.StartedAt.IsZero() { + if code == container.ExitCode && !container.Running && !container.StartedAt.IsZero() { shouldSkip = false break } @@ -451,28 +454,34 @@ func includeContainerInList(container *container.Container, ctx *listContext) it } // Do not include container if its status doesn't match the filter - if !ctx.filters.Match("status", container.State.StateString()) { + if !ctx.filters.Match("status", container.State) { return excludeContainer } // Do not include container if its health doesn't match the filter - if !ctx.filters.ExactMatch("health", container.State.HealthString()) { + if !ctx.filters.ExactMatch("health", container.Health) { return excludeContainer } if ctx.filters.Include("volume") { - volumesByName := make(map[string]*volume.MountPoint) - for _, m := range container.MountPoints { + volumesByName := make(map[string]types.MountPoint) + for _, m := range container.Mounts { if m.Name != "" { volumesByName[m.Name] = m } else { volumesByName[m.Source] = m } } + volumesByDestination := make(map[string]types.MountPoint) + for _, m := range container.Mounts { + if m.Destination != "" { + volumesByDestination[m.Destination] = m + } + } volumeExist := fmt.Errorf("volume mounted in container") err := ctx.filters.WalkValues("volume", func(value string) error { - if _, exist := container.MountPoints[value]; exist { + if _, exist := volumesByDestination[value]; exist { return volumeExist } if _, exist := volumesByName[value]; exist { @@ -489,7 +498,7 @@ func includeContainerInList(container *container.Container, ctx *listContext) it if len(ctx.images) == 0 { return excludeContainer } - if !ctx.images[container.ImageID] { + if !ctx.images[image.ID(container.ImageID)] { return excludeContainer } } @@ -501,7 +510,7 @@ func includeContainerInList(container *container.Container, ctx *listContext) it return networkExist } for _, nw := range container.NetworkSettings.Networks { - if nw.EndpointSettings == nil { + if nw == nil { continue } if strings.HasPrefix(nw.NetworkID, value) { @@ -518,7 +527,7 @@ func includeContainerInList(container *container.Container, ctx *listContext) it if len(ctx.publish) > 0 { shouldSkip := true for port := range ctx.publish { - if _, ok := container.HostConfig.PortBindings[port]; ok { + if _, ok := container.PublishPorts[port]; ok { shouldSkip = false break } @@ -531,7 +540,7 @@ func includeContainerInList(container *container.Container, ctx *listContext) it if len(ctx.expose) > 0 { shouldSkip := true for port := range ctx.expose { - if _, ok := container.Config.ExposedPorts[port]; ok { + if _, ok := container.ExposedPorts[port]; ok { shouldSkip = false break } @@ -545,104 +554,38 @@ func includeContainerInList(container *container.Container, ctx *listContext) it } // transformContainer generates the container type expected by the docker ps command. -func (daemon *Daemon) transformContainer(container *container.Container, ctx *listContext) (*types.Container, error) { +func (daemon *Daemon) transformContainer(container *container.Snapshot, ctx *listContext) (*types.Container, error) { newC := &types.Container{ - ID: container.ID, - Names: ctx.names[container.ID], - ImageID: container.ImageID.String(), + ID: container.ID, + Names: ctx.names[container.ID], + ImageID: container.ImageID, + Command: container.Command, + Created: container.Created.Unix(), + State: container.State, + Status: container.Status, + NetworkSettings: &container.NetworkSettings, + Ports: container.Ports, + Labels: container.Labels, + Mounts: container.Mounts, } if newC.Names == nil { // Dead containers will often have no name, so make sure the response isn't null newC.Names = []string{} } + newC.HostConfig.NetworkMode = container.HostConfig.NetworkMode - image := container.Config.Image // if possible keep the original ref - if image != container.ImageID.String() { + image := container.Image // if possible keep the original ref + if image != container.ImageID { id, _, err := daemon.GetImageIDAndPlatform(image) if _, isDNE := err.(ErrImageDoesNotExist); err != nil && !isDNE { return nil, err } - if err != nil || id != container.ImageID { - image = container.ImageID.String() + if err != nil || id.String() != container.ImageID { + image = container.ImageID } } newC.Image = image - if len(container.Args) > 0 { - args := []string{} - for _, arg := range container.Args { - if strings.Contains(arg, " ") { - args = append(args, fmt.Sprintf("'%s'", arg)) - } else { - args = append(args, arg) - } - } - argsAsString := strings.Join(args, " ") - - newC.Command = fmt.Sprintf("%s %s", container.Path, argsAsString) - } else { - newC.Command = container.Path - } - newC.Created = container.Created.Unix() - newC.State = container.State.StateString() - newC.Status = container.State.String() - newC.HostConfig.NetworkMode = string(container.HostConfig.NetworkMode) - // copy networks to avoid races - networks := make(map[string]*networktypes.EndpointSettings) - for name, network := range container.NetworkSettings.Networks { - if network == nil || network.EndpointSettings == nil { - continue - } - networks[name] = &networktypes.EndpointSettings{ - EndpointID: network.EndpointID, - Gateway: network.Gateway, - IPAddress: network.IPAddress, - IPPrefixLen: network.IPPrefixLen, - IPv6Gateway: network.IPv6Gateway, - GlobalIPv6Address: network.GlobalIPv6Address, - GlobalIPv6PrefixLen: network.GlobalIPv6PrefixLen, - MacAddress: network.MacAddress, - NetworkID: network.NetworkID, - } - if network.IPAMConfig != nil { - networks[name].IPAMConfig = &networktypes.EndpointIPAMConfig{ - IPv4Address: network.IPAMConfig.IPv4Address, - IPv6Address: network.IPAMConfig.IPv6Address, - } - } - } - newC.NetworkSettings = &types.SummaryNetworkSettings{Networks: networks} - - newC.Ports = []types.Port{} - for port, bindings := range container.NetworkSettings.Ports { - p, err := nat.ParsePort(port.Port()) - if err != nil { - return nil, err - } - if len(bindings) == 0 { - newC.Ports = append(newC.Ports, types.Port{ - PrivatePort: uint16(p), - Type: port.Proto(), - }) - continue - } - for _, binding := range bindings { - h, err := nat.ParsePort(binding.HostPort) - if err != nil { - return nil, err - } - newC.Ports = append(newC.Ports, types.Port{ - PrivatePort: uint16(p), - PublicPort: uint16(h), - Type: port.Proto(), - IP: binding.HostIP, - }) - } - } - - newC.Labels = container.Config.Labels - newC.Mounts = addMountPoints(container) - return newC, nil } diff --git a/daemon/list_unix.go b/daemon/list_unix.go index 91c9caccf4..ebaae4560c 100644 --- a/daemon/list_unix.go +++ b/daemon/list_unix.go @@ -6,6 +6,6 @@ import "github.com/docker/docker/container" // excludeByIsolation is a platform specific helper function to support PS // filtering by Isolation. This is a Windows-only concept, so is a no-op on Unix. -func excludeByIsolation(container *container.Container, ctx *listContext) iterationAction { +func excludeByIsolation(container *container.Snapshot, ctx *listContext) iterationAction { return includeContainer } diff --git a/daemon/list_windows.go b/daemon/list_windows.go index 7fbcd3af26..ab563c535f 100644 --- a/daemon/list_windows.go +++ b/daemon/list_windows.go @@ -8,7 +8,7 @@ import ( // excludeByIsolation is a platform specific helper function to support PS // filtering by Isolation. This is a Windows-only concept, so is a no-op on Unix. -func excludeByIsolation(container *container.Container, ctx *listContext) iterationAction { +func excludeByIsolation(container *container.Snapshot, ctx *listContext) iterationAction { i := strings.ToLower(string(container.HostConfig.Isolation)) if i == "" { i = "default" From aacddda89df05b88a6d15fb33c42864760385ab2 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Thu, 23 Feb 2017 15:12:18 -0800 Subject: [PATCH 06/18] Move checkpointing to the Container object Also hide ViewDB behind an inteface. Signed-off-by: Fabio Kung --- container/container.go | 15 ++++++++++++++ container/snapshot.go | 2 +- container/view.go | 37 ++++++++++++++++++++++------------ container/view_test.go | 21 +++++++++---------- daemon/container.go | 9 +++------ daemon/container_operations.go | 19 +++++++---------- daemon/daemon.go | 4 ++-- daemon/delete.go | 2 +- daemon/health.go | 4 ++-- daemon/health_test.go | 4 ++-- daemon/list.go | 4 ++-- daemon/monitor.go | 21 ++++--------------- daemon/rename.go | 10 ++------- daemon/start.go | 7 ++----- daemon/update.go | 5 ++--- 15 files changed, 79 insertions(+), 85 deletions(-) diff --git a/container/container.go b/container/container.go index f1c8a10cd7..4b42924301 100644 --- a/container/container.go +++ b/container/container.go @@ -189,6 +189,21 @@ func (container *Container) ToDiskLocking() error { return err } +// CheckpointTo makes the Container's current state visible to queries. +// Callers must hold a Container lock. +func (container *Container) CheckpointTo(store ViewDB) error { + return store.Save(container.snapshot()) +} + +// CheckpointAndSaveToDisk is equivalent to calling CheckpointTo and ToDisk. +// Callers must hold a Container lock. +func (container *Container) CheckpointAndSaveToDisk(store ViewDB) error { + if err := container.CheckpointTo(store); err != nil { + return err + } + return container.ToDisk() +} + // readHostConfig reads the host configuration from disk for the container. func (container *Container) readHostConfig() error { container.HostConfig = &containertypes.HostConfig{} diff --git a/container/snapshot.go b/container/snapshot.go index a56cabcb96..1e81039bbc 100644 --- a/container/snapshot.go +++ b/container/snapshot.go @@ -41,7 +41,7 @@ type Snapshot struct { } // Snapshot provides a read only view of a Container. Callers must hold a Lock on the container object. -func (container *Container) Snapshot() *Snapshot { +func (container *Container) snapshot() *Snapshot { snapshot := &Snapshot{ ID: container.ID, Name: container.Name, diff --git a/container/view.go b/container/view.go index aa122fe2c7..dd4708c313 100644 --- a/container/view.go +++ b/container/view.go @@ -8,6 +8,19 @@ const ( memdbIDIndex = "id" ) +// ViewDB provides an in-memory transactional (ACID) container Store +type ViewDB interface { + Snapshot() View + Save(snapshot *Snapshot) error + Delete(id string) error +} + +// View can be used by readers to avoid locking +type View interface { + All() ([]Snapshot, error) + Get(id string) (*Snapshot, error) +} + var schema = &memdb.DBSchema{ Tables: map[string]*memdb.TableSchema{ memdbTable: { @@ -23,46 +36,44 @@ var schema = &memdb.DBSchema{ }, } -// MemDB provides an in-memory transactional (ACID) container Store -type MemDB struct { +type memDB struct { store *memdb.MemDB } -// NewMemDB provides the default implementation, with the default schema -func NewMemDB() (*MemDB, error) { +// NewViewDB provides the default implementation, with the default schema +func NewViewDB() (ViewDB, error) { store, err := memdb.NewMemDB(schema) if err != nil { return nil, err } - return &MemDB{store: store}, nil + return &memDB{store: store}, nil } // Snapshot provides a consistent read-only View of the database -func (db *MemDB) Snapshot() *View { - return &View{db.store.Txn(false)} +func (db *memDB) Snapshot() View { + return &memdbView{db.store.Txn(false)} } // Save atomically updates the in-memory store -func (db *MemDB) Save(snapshot *Snapshot) error { +func (db *memDB) Save(snapshot *Snapshot) error { txn := db.store.Txn(true) defer txn.Commit() return txn.Insert(memdbTable, snapshot) } // Delete removes an item by ID -func (db *MemDB) Delete(id string) error { +func (db *memDB) Delete(id string) error { txn := db.store.Txn(true) defer txn.Commit() return txn.Delete(memdbTable, &Snapshot{ID: id}) } -// View can be used by readers to avoid locking -type View struct { +type memdbView struct { txn *memdb.Txn } // All returns a all items in this snapshot -func (v *View) All() ([]Snapshot, error) { +func (v *memdbView) All() ([]Snapshot, error) { var all []Snapshot iter, err := v.txn.Get(memdbTable, memdbIDIndex) if err != nil { @@ -80,7 +91,7 @@ func (v *View) All() ([]Snapshot, error) { } //Get returns an item by id -func (v *View) Get(id string) (*Snapshot, error) { +func (v *memdbView) Get(id string) (*Snapshot, error) { s, err := v.txn.First(memdbTable, memdbIDIndex, id) if err != nil { return nil, err diff --git a/container/view_test.go b/container/view_test.go index 32bdf4957c..20424ffd98 100644 --- a/container/view_test.go +++ b/container/view_test.go @@ -3,27 +3,26 @@ package container import "testing" func TestViewSave(t *testing.T) { - db, err := NewMemDB() + db, err := NewViewDB() if err != nil { t.Fatal(err) } - snapshot := NewBaseContainer("id", "root").Snapshot() - if err := db.Save(snapshot); err != nil { + c := NewBaseContainer("id", "root") + if err := c.CheckpointTo(db); err != nil { t.Fatal(err) - } } func TestViewAll(t *testing.T) { var ( - db, _ = NewMemDB() - one = NewBaseContainer("id1", "root1").Snapshot() - two = NewBaseContainer("id2", "root2").Snapshot() + db, _ = NewViewDB() + one = NewBaseContainer("id1", "root1") + two = NewBaseContainer("id2", "root2") ) one.Pid = 10 two.Pid = 20 - db.Save(one) - db.Save(two) + one.CheckpointTo(db) + two.CheckpointTo(db) all, err := db.Snapshot().All() if err != nil { t.Fatal(err) @@ -44,10 +43,10 @@ func TestViewAll(t *testing.T) { } func TestViewGet(t *testing.T) { - db, _ := NewMemDB() + db, _ := NewViewDB() one := NewBaseContainer("id", "root") one.ImageID = "some-image-123" - db.Save(one.Snapshot()) + one.CheckpointTo(db) s, err := db.Snapshot().Get("id") if err != nil { t.Fatal(err) diff --git a/daemon/container.go b/daemon/container.go index 5d0d0438e4..72689fbbaa 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -108,13 +108,13 @@ func (daemon *Daemon) Register(c *container.Container) error { } // once in the memory store it is visible to other goroutines - // grab a Lock until it has been replicated to avoid races + // grab a Lock until it has been checkpointed to avoid races c.Lock() defer c.Unlock() daemon.containers.Add(c.ID, c) daemon.idIndex.Add(c.ID) - return daemon.containersReplica.Save(c.Snapshot()) + return c.CheckpointTo(daemon.containersReplica) } func (daemon *Daemon) newContainer(name string, platform string, config *containertypes.Config, hostConfig *containertypes.HostConfig, imgID image.ID, managed bool) (*container.Container, error) { @@ -218,10 +218,7 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig * runconfig.SetDefaultNetModeIfBlank(hostConfig) container.HostConfig = hostConfig - if err := daemon.containersReplica.Save(container.Snapshot()); err != nil { - return err - } - return container.ToDisk() + return container.CheckpointAndSaveToDisk(daemon.containersReplica) } // verifyContainerSettings performs validation of the hostconfig and config diff --git a/daemon/container_operations.go b/daemon/container_operations.go index fe83928472..9e5a02c3f8 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -45,14 +45,11 @@ func (daemon *Daemon) getDNSSearchSettings(container *container.Container) []str return nil } -func (daemon *Daemon) saveAndReplicate(container *container.Container) error { +func (daemon *Daemon) checkpointAndSave(container *container.Container) error { container.Lock() defer container.Unlock() - if err := daemon.containersReplica.Save(container.Snapshot()); err != nil { - return fmt.Errorf("Error replicating container state: %v", err) - } - if err := container.ToDisk(); err != nil { - return fmt.Errorf("Error saving container to disk: %v", err) + if err := container.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + return fmt.Errorf("Error saving container state: %v", err) } return nil } @@ -1018,10 +1015,8 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName return err } } - if err := daemon.saveAndReplicate(container); err != nil { - return fmt.Errorf("Error saving container to disk: %v", err) - } - return nil + + return daemon.checkpointAndSave(container) } // DisconnectFromNetwork disconnects container from network n. @@ -1057,8 +1052,8 @@ func (daemon *Daemon) DisconnectFromNetwork(container *container.Container, netw return err } - if err := daemon.saveAndReplicate(container); err != nil { - return fmt.Errorf("Error saving container to disk: %v", err) + if err := daemon.checkpointAndSave(container); err != nil { + return err } if n != nil { diff --git a/daemon/daemon.go b/daemon/daemon.go index f2a83cc4d0..ab46ad3341 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -83,7 +83,7 @@ type Daemon struct { ID string repository string containers container.Store - containersReplica *container.MemDB + containersReplica container.ViewDB execCommands *exec.Store downloadManager *xfer.LayerDownloadManager uploadManager *xfer.LayerUploadManager @@ -762,7 +762,7 @@ func NewDaemon(config *config.Config, registryService registry.Service, containe d.ID = trustKey.PublicKey().KeyID() d.repository = daemonRepo d.containers = container.NewMemoryStore() - if d.containersReplica, err = container.NewMemDB(); err != nil { + if d.containersReplica, err = container.NewViewDB(); err != nil { return nil, err } d.execCommands = exec.NewStore() diff --git a/daemon/delete.go b/daemon/delete.go index ad5eacc61c..3609a88e9f 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -105,7 +105,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo // Mark container dead. We don't want anybody to be restarting it. container.Lock() container.Dead = true - if err = daemon.containersReplica.Save(container.Snapshot()); err != nil { + if err = container.CheckpointTo(daemon.containersReplica); err != nil { container.Unlock() return err } diff --git a/daemon/health.go b/daemon/health.go index 3419511d36..00528db2ae 100644 --- a/daemon/health.go +++ b/daemon/health.go @@ -168,9 +168,9 @@ func handleProbeResult(d *Daemon, c *container.Container, result *types.Healthch } // replicate Health status changes - if err := d.containersReplica.Save(c.Snapshot()); err != nil { + if err := c.CheckpointTo(d.containersReplica); err != nil { // queries will be inconsistent until the next probe runs or other state mutations - // trigger a replication + // checkpoint the container logrus.Errorf("Error replicating health state for container %s: %v", c.ID, err) } diff --git a/daemon/health_test.go b/daemon/health_test.go index 0944e3fa33..4fd89140d3 100644 --- a/daemon/health_test.go +++ b/daemon/health_test.go @@ -29,7 +29,7 @@ func TestNoneHealthcheck(t *testing.T) { }, State: &container.State{}, } - store, err := container.NewMemDB() + store, err := container.NewViewDB() if err != nil { t.Fatal(err) } @@ -69,7 +69,7 @@ func TestHealthStates(t *testing.T) { }, } - store, err := container.NewMemDB() + store, err := container.NewViewDB() if err != nil { t.Fatal(err) } diff --git a/daemon/list.go b/daemon/list.go index b810164f28..2d10e3d6de 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -114,7 +114,7 @@ func (daemon *Daemon) Containers(config *types.ContainerListOptions) ([]*types.C return daemon.reduceContainers(config, daemon.transformContainer) } -func (daemon *Daemon) filterByNameIDMatches(view *container.View, ctx *listContext) ([]container.Snapshot, error) { +func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContext) ([]container.Snapshot, error) { idSearch := false names := ctx.filters.Get("name") ids := ctx.filters.Get("id") @@ -240,7 +240,7 @@ func (daemon *Daemon) reducePsContainer(container *container.Snapshot, ctx *list } // foldFilter generates the container filter based on the user's filtering options. -func (daemon *Daemon) foldFilter(view *container.View, config *types.ContainerListOptions) (*listContext, error) { +func (daemon *Daemon) foldFilter(view container.View, config *types.ContainerListOptions) (*listContext, error) { psFilters := config.Filters if err := psFilters.Validate(acceptedPsFilterTags); err != nil { diff --git a/daemon/monitor.go b/daemon/monitor.go index a581230ce5..be8a500020 100644 --- a/daemon/monitor.go +++ b/daemon/monitor.go @@ -90,10 +90,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { daemon.setStateCounter(c) defer c.Unlock() - if err := daemon.containersReplica.Save(c.Snapshot()); err != nil { - return err - } - if err := c.ToDisk(); err != nil { + if err := c.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { return err } return daemon.postRunProcessing(c, e) @@ -122,11 +119,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { c.HasBeenStartedBefore = true daemon.setStateCounter(c) - if err := daemon.containersReplica.Save(c.Snapshot()); err != nil { - c.Reset(false) - return err - } - if err := c.ToDisk(); err != nil { + if err := c.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { c.Reset(false) return err } @@ -137,10 +130,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { // Container is already locked in this case c.Paused = true daemon.setStateCounter(c) - if err := daemon.containersReplica.Save(c.Snapshot()); err != nil { - return err - } - if err := c.ToDisk(); err != nil { + if err := c.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { return err } daemon.updateHealthMonitor(c) @@ -149,10 +139,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { // Container is already locked in this case c.Paused = false daemon.setStateCounter(c) - if err := daemon.containersReplica.Save(c.Snapshot()); err != nil { - return err - } - if err := c.ToDisk(); err != nil { + if err := c.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { return err } daemon.updateHealthMonitor(c) diff --git a/daemon/rename.go b/daemon/rename.go index 0a3730ff16..1a5e985cfc 100644 --- a/daemon/rename.go +++ b/daemon/rename.go @@ -82,10 +82,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { daemon.nameIndex.Release(oldName + k) } daemon.releaseName(oldName) - if err = daemon.containersReplica.Save(container.Snapshot()); err != nil { - return err - } - if err = container.ToDisk(); err != nil { + if err = container.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { return err } @@ -102,10 +99,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { if err != nil { container.Name = oldName container.NetworkSettings.IsAnonymousEndpoint = oldIsAnonymousEndpoint - if e := daemon.containersReplica.Save(container.Snapshot()); err != nil { - logrus.Errorf("%s: Failed in replicating state on rename failure: %v", container.ID, e) - } - if e := container.ToDisk(); e != nil { + if e := container.CheckpointAndSaveToDisk(daemon.containersReplica); e != nil { logrus.Errorf("%s: Failed in writing to Disk on rename failure: %v", container.ID, e) } } diff --git a/daemon/start.go b/daemon/start.go index 5d8a1286aa..c2e9b7069f 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -117,11 +117,8 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint if container.ExitCode() == 0 { container.SetExitCode(128) } - if err := daemon.containersReplica.Save(container.Snapshot()); err != nil { - logrus.Errorf("%s: failed replicating state on start failure: %v", container.ID, err) - } - if err := container.ToDisk(); err != nil { - logrus.Errorf("%s: failed writing to disk on start failure: %v", container.ID, err) + if err := container.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + logrus.Errorf("%s: failed saving state on start failure: %v", container.ID, err) } container.Reset(false) diff --git a/daemon/update.go b/daemon/update.go index bdcfa2d067..af9bf7b991 100644 --- a/daemon/update.go +++ b/daemon/update.go @@ -38,8 +38,7 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro if restoreConfig { container.Lock() container.HostConfig = &backupHostConfig - daemon.containersReplica.Save(container.Snapshot()) - container.ToDisk() + container.CheckpointAndSaveToDisk(daemon.containersReplica) container.Unlock() } }() @@ -54,7 +53,7 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro container.Unlock() return errCannotUpdate(container.ID, err) } - if err := daemon.containersReplica.Save(container.Snapshot()); err != nil { + if err := container.CheckpointTo(daemon.containersReplica); err != nil { restoreConfig = true container.Unlock() return errCannotUpdate(container.ID, err) From fcb6d37a8dde8a91896116742116e9d9808b18ac Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Fri, 10 Mar 2017 21:06:05 -0800 Subject: [PATCH 07/18] Make tests a bit less flaky Prevent tests failing when teardown tries to delete a container that is already being removed. Signed-off-by: Fabio Kung --- integration-cli/environment/clean.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/integration-cli/environment/clean.go b/integration-cli/environment/clean.go index b27838337e..809baa7b52 100644 --- a/integration-cli/environment/clean.go +++ b/integration-cli/environment/clean.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/http" + "regexp" "strings" "github.com/docker/docker/api/types" @@ -50,14 +51,17 @@ func getPausedContainers(t testingT, dockerBinary string) []string { return strings.Fields(result.Combined()) } +var alreadyExists = regexp.MustCompile(`Error response from daemon: removal of container (\w+) is already in progress`) + func deleteAllContainers(t testingT, dockerBinary string) { containers := getAllContainers(t, dockerBinary) if len(containers) > 0 { result := icmd.RunCommand(dockerBinary, append([]string{"rm", "-fv"}, containers...)...) if result.Error != nil { // If the error is "No such container: ..." this means the container doesn't exists anymore, - // we can safely ignore that one. - if strings.Contains(result.Stderr(), "No such container") { + // or if it is "... removal of container ... is already in progress" it will be removed eventually. + // We can safely ignore those. + if strings.Contains(result.Stderr(), "No such container") || alreadyExists.MatchString(result.Stderr()) { return } t.Fatalf("error removing containers %v : %v (%s)", containers, result.Error, result.Combined()) From 2ed6f9257a0ccb81583e65bf8af0e00f8e2dedcb Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Mon, 20 Mar 2017 08:20:53 -0700 Subject: [PATCH 08/18] how to maintain the container snapshot struct Signed-off-by: Fabio Kung --- container/snapshot.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/container/snapshot.go b/container/snapshot.go index 1e81039bbc..861b7ae65e 100644 --- a/container/snapshot.go +++ b/container/snapshot.go @@ -11,7 +11,8 @@ import ( "github.com/docker/go-connections/nat" ) -// Snapshot is a read only view for Containers +// Snapshot is a read only view for Containers. It holds all information necessary to serve container queries in a +// versioned ACID in-memory store. Pointers are avoided here to make sure all values are copied into the store. type Snapshot struct { ID string `json:"Id"` Name string From 03aa24721c88e508f208c1ff72d8cd1af7e6a0f8 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Mon, 27 Mar 2017 08:51:36 -0700 Subject: [PATCH 09/18] no need to save state to disk here State will be saved on the following operation once the container is properly registered on the daemon. Signed-off-by: Fabio Kung --- daemon/names.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/daemon/names.go b/daemon/names.go index 5ce16624ad..ec6ac2924f 100644 --- a/daemon/names.go +++ b/daemon/names.go @@ -30,10 +30,6 @@ func (daemon *Daemon) registerName(container *container.Container) error { return err } container.Name = name - - if err := container.ToDiskLocking(); err != nil { - logrus.Errorf("Error saving container name to disk: %v", err) - } } return daemon.nameIndex.Reserve(container.Name, container.ID) } From 201a37f7a110402a76ac1ee21b5d09ccffed2acb Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Mon, 27 Mar 2017 09:04:52 -0700 Subject: [PATCH 10/18] verifyVolumesInfo needs a container lock It operates on containers that have already been registered on the daemon, and are visible to other goroutines. Signed-off-by: Fabio Kung --- daemon/volumes_unix.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index d91100f1c9..c01c573e93 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -137,6 +137,9 @@ func migrateVolume(id, vfs string) error { // verifyVolumesInfo ports volumes configured for the containers pre docker 1.7. // It reads the container configuration and creates valid mount points for the old volumes. func (daemon *Daemon) verifyVolumesInfo(container *container.Container) error { + container.Lock() + defer container.Unlock() + // Inspect old structures only when we're upgrading from old versions // to versions >= 1.7 and the MountPoints has not been populated with volumes data. type volumes struct { From f668af4475980e32c99503c4a513668c24ea2da6 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Mon, 27 Mar 2017 10:14:28 -0700 Subject: [PATCH 11/18] no need to save container state here it is already being saved (with a lock held) on the subsequent operations. Signed-off-by: Fabio Kung --- container/container_unix.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/container/container_unix.go b/container/container_unix.go index deab000b86..0cd9312be3 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -332,11 +332,6 @@ func (container *Container) UpdateContainer(hostConfig *containertypes.HostConfi container.HostConfig.RestartPolicy = hostConfig.RestartPolicy } - if err := container.ToDisk(); err != nil { - logrus.Errorf("Error saving updated container: %v", err) - return err - } - return nil } From edad52707c536116363031002e6633e3fec16af5 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Mon, 27 Mar 2017 10:18:53 -0700 Subject: [PATCH 12/18] save deep copies of Container in the replica store Reuse existing structures and rely on json serialization to deep copy Container objects. Also consolidate all "save" operations on container.CheckpointTo, which now both saves a serialized json to disk, and replicates state to the ACID in-memory store. Signed-off-by: Fabio Kung --- container/container.go | 22 +--- container/snapshot.go | 153 ---------------------- container/view.go | 231 ++++++++++++++++++++++++++++++--- container/view_test.go | 83 +++++++++--- daemon/container.go | 2 +- daemon/container_operations.go | 3 +- daemon/create.go | 5 - daemon/daemon.go | 6 +- daemon/daemon_unix_test.go | 11 +- daemon/delete.go | 8 +- daemon/list.go | 68 ++++------ daemon/monitor.go | 8 +- daemon/rename.go | 4 +- daemon/restart.go | 2 +- daemon/start.go | 4 +- daemon/start_unix.go | 3 +- daemon/update.go | 2 +- daemon/volumes_unix.go | 2 +- 18 files changed, 341 insertions(+), 276 deletions(-) delete mode 100644 container/snapshot.go diff --git a/container/container.go b/container/container.go index 4b42924301..2fd007e0a5 100644 --- a/container/container.go +++ b/container/container.go @@ -159,7 +159,7 @@ func (container *Container) FromDisk() error { } // ToDisk saves the container configuration on disk. -func (container *Container) ToDisk() error { +func (container *Container) toDisk() error { pth, err := container.ConfigPath() if err != nil { return err @@ -181,27 +181,13 @@ func (container *Container) ToDisk() error { return container.WriteHostConfig() } -// ToDiskLocking saves the container configuration on disk in a thread safe way. -func (container *Container) ToDiskLocking() error { - container.Lock() - err := container.ToDisk() - container.Unlock() - return err -} - -// CheckpointTo makes the Container's current state visible to queries. +// CheckpointTo makes the Container's current state visible to queries, and persists state. // Callers must hold a Container lock. func (container *Container) CheckpointTo(store ViewDB) error { - return store.Save(container.snapshot()) -} - -// CheckpointAndSaveToDisk is equivalent to calling CheckpointTo and ToDisk. -// Callers must hold a Container lock. -func (container *Container) CheckpointAndSaveToDisk(store ViewDB) error { - if err := container.CheckpointTo(store); err != nil { + if err := container.toDisk(); err != nil { return err } - return container.ToDisk() + return store.Save(container) } // readHostConfig reads the host configuration from disk for the container. diff --git a/container/snapshot.go b/container/snapshot.go deleted file mode 100644 index 861b7ae65e..0000000000 --- a/container/snapshot.go +++ /dev/null @@ -1,153 +0,0 @@ -package container - -import ( - "fmt" - "strings" - "time" - - "github.com/Sirupsen/logrus" - "github.com/docker/docker/api/types" - "github.com/docker/docker/api/types/network" - "github.com/docker/go-connections/nat" -) - -// Snapshot is a read only view for Containers. It holds all information necessary to serve container queries in a -// versioned ACID in-memory store. Pointers are avoided here to make sure all values are copied into the store. -type Snapshot struct { - ID string `json:"Id"` - Name string - Pid int - Managed bool - Image string - ImageID string - Command string - Ports []types.Port - ExposedPorts nat.PortSet - PublishPorts nat.PortSet - Labels map[string]string - State string - Status string - Health string - HostConfig struct { - NetworkMode string - Isolation string - } - NetworkSettings types.SummaryNetworkSettings - Mounts []types.MountPoint - Created time.Time - StartedAt time.Time - Running bool - Paused bool - ExitCode int -} - -// Snapshot provides a read only view of a Container. Callers must hold a Lock on the container object. -func (container *Container) snapshot() *Snapshot { - snapshot := &Snapshot{ - ID: container.ID, - Name: container.Name, - Pid: container.Pid, - Managed: container.Managed, - ImageID: container.ImageID.String(), - Ports: []types.Port{}, - ExposedPorts: make(nat.PortSet), - PublishPorts: make(nat.PortSet), - State: container.State.StateString(), - Status: container.State.String(), - Health: container.State.HealthString(), - Mounts: container.GetMountPoints(), - Created: container.Created, - StartedAt: container.StartedAt, - Running: container.Running, - Paused: container.Paused, - ExitCode: container.ExitCode(), - } - - if container.HostConfig != nil { - snapshot.HostConfig.Isolation = string(container.HostConfig.Isolation) - snapshot.HostConfig.NetworkMode = string(container.HostConfig.NetworkMode) - for publish := range container.HostConfig.PortBindings { - snapshot.PublishPorts[publish] = struct{}{} - } - } - - if container.Config != nil { - snapshot.Image = container.Config.Image - snapshot.Labels = container.Config.Labels - for exposed := range container.Config.ExposedPorts { - snapshot.ExposedPorts[exposed] = struct{}{} - } - } - - if len(container.Args) > 0 { - args := []string{} - for _, arg := range container.Args { - if strings.Contains(arg, " ") { - args = append(args, fmt.Sprintf("'%s'", arg)) - } else { - args = append(args, arg) - } - } - argsAsString := strings.Join(args, " ") - snapshot.Command = fmt.Sprintf("%s %s", container.Path, argsAsString) - } else { - snapshot.Command = container.Path - } - - if container.NetworkSettings != nil { - networks := make(map[string]*network.EndpointSettings) - for name, netw := range container.NetworkSettings.Networks { - if netw == nil || netw.EndpointSettings == nil { - continue - } - networks[name] = &network.EndpointSettings{ - EndpointID: netw.EndpointID, - Gateway: netw.Gateway, - IPAddress: netw.IPAddress, - IPPrefixLen: netw.IPPrefixLen, - IPv6Gateway: netw.IPv6Gateway, - GlobalIPv6Address: netw.GlobalIPv6Address, - GlobalIPv6PrefixLen: netw.GlobalIPv6PrefixLen, - MacAddress: netw.MacAddress, - NetworkID: netw.NetworkID, - } - if netw.IPAMConfig != nil { - networks[name].IPAMConfig = &network.EndpointIPAMConfig{ - IPv4Address: netw.IPAMConfig.IPv4Address, - IPv6Address: netw.IPAMConfig.IPv6Address, - } - } - } - snapshot.NetworkSettings = types.SummaryNetworkSettings{Networks: networks} - for port, bindings := range container.NetworkSettings.Ports { - p, err := nat.ParsePort(port.Port()) - if err != nil { - logrus.Warnf("invalid port map %+v", err) - continue - } - if len(bindings) == 0 { - snapshot.Ports = append(snapshot.Ports, types.Port{ - PrivatePort: uint16(p), - Type: port.Proto(), - }) - continue - } - for _, binding := range bindings { - h, err := nat.ParsePort(binding.HostPort) - if err != nil { - logrus.Warnf("invalid host port map %+v", err) - continue - } - snapshot.Ports = append(snapshot.Ports, types.Port{ - PrivatePort: uint16(p), - PublicPort: uint16(h), - Type: port.Proto(), - IP: binding.HostIP, - }) - } - } - - } - - return snapshot -} diff --git a/container/view.go b/container/view.go index dd4708c313..e5e1542f77 100644 --- a/container/view.go +++ b/container/view.go @@ -1,18 +1,51 @@ package container -import "github.com/hashicorp/go-memdb" +import ( + "fmt" + "strings" + "time" + + "github.com/Sirupsen/logrus" + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/network" + "github.com/docker/docker/pkg/registrar" + "github.com/docker/go-connections/nat" + "github.com/hashicorp/go-memdb" +) const ( memdbTable = "containers" - memdbIDField = "ID" memdbIDIndex = "id" ) +// Snapshot is a read only view for Containers. It holds all information necessary to serve container queries in a +// versioned ACID in-memory store. +type Snapshot struct { + types.Container + + // additional info queries need to filter on + // preserve nanosec resolution for queries + CreatedAt time.Time + StartedAt time.Time + Name string + Pid int + ExitCode int + Running bool + Paused bool + Managed bool + ExposedPorts nat.PortSet + PortBindings nat.PortSet + Health string + HostConfig struct { + Isolation string + } +} + // ViewDB provides an in-memory transactional (ACID) container Store type ViewDB interface { - Snapshot() View - Save(snapshot *Snapshot) error - Delete(id string) error + Snapshot(nameIndex *registrar.Registrar) View + Save(*Container) error + Delete(*Container) error } // View can be used by readers to avoid locking @@ -29,7 +62,7 @@ var schema = &memdb.DBSchema{ memdbIDIndex: { Name: memdbIDIndex, Unique: true, - Indexer: &memdb.StringFieldIndex{Field: memdbIDField}, + Indexer: &containerByIDIndexer{}, }, }, }, @@ -50,29 +83,38 @@ func NewViewDB() (ViewDB, error) { } // Snapshot provides a consistent read-only View of the database -func (db *memDB) Snapshot() View { - return &memdbView{db.store.Txn(false)} +func (db *memDB) Snapshot(index *registrar.Registrar) View { + return &memdbView{ + txn: db.store.Txn(false), + nameIndex: index.GetAll(), + } } -// Save atomically updates the in-memory store -func (db *memDB) Save(snapshot *Snapshot) error { +// Save atomically updates the in-memory store from the current on-disk state of a Container. +func (db *memDB) Save(c *Container) error { txn := db.store.Txn(true) defer txn.Commit() - return txn.Insert(memdbTable, snapshot) + deepCopy := NewBaseContainer(c.ID, c.Root) + err := deepCopy.FromDisk() // TODO: deal with reserveLabel + if err != nil { + return err + } + return txn.Insert(memdbTable, deepCopy) } // Delete removes an item by ID -func (db *memDB) Delete(id string) error { +func (db *memDB) Delete(c *Container) error { txn := db.store.Txn(true) defer txn.Commit() - return txn.Delete(memdbTable, &Snapshot{ID: id}) + return txn.Delete(memdbTable, NewBaseContainer(c.ID, c.Root)) } type memdbView struct { - txn *memdb.Txn + txn *memdb.Txn + nameIndex map[string][]string } -// All returns a all items in this snapshot +// All returns a all items in this snapshot. Returned objects must never be modified. func (v *memdbView) All() ([]Snapshot, error) { var all []Snapshot iter, err := v.txn.Get(memdbTable, memdbIDIndex) @@ -84,18 +126,167 @@ func (v *memdbView) All() ([]Snapshot, error) { if item == nil { break } - snapshot := *(item.(*Snapshot)) // force a copy - all = append(all, snapshot) + snapshot := v.transform(item.(*Container)) + all = append(all, *snapshot) } return all, nil } -//Get returns an item by id +// Get returns an item by id. Returned objects must never be modified. func (v *memdbView) Get(id string) (*Snapshot, error) { s, err := v.txn.First(memdbTable, memdbIDIndex, id) if err != nil { return nil, err } - snapshot := *(s.(*Snapshot)) // force a copy - return &snapshot, nil + return v.transform(s.(*Container)), nil +} + +// transform maps a (deep) copied Container object to what queries need. +// A lock on the Container is not held because these are immutable deep copies. +func (v *memdbView) transform(container *Container) *Snapshot { + snapshot := &Snapshot{ + Container: types.Container{ + ID: container.ID, + Names: v.nameIndex[container.ID], + ImageID: container.ImageID.String(), + Ports: []types.Port{}, + Mounts: container.GetMountPoints(), + State: container.State.StateString(), + Status: container.State.String(), + Created: container.Created.Unix(), + }, + CreatedAt: container.Created, + StartedAt: container.StartedAt, + Name: container.Name, + Pid: container.Pid, + Managed: container.Managed, + ExposedPorts: make(nat.PortSet), + PortBindings: make(nat.PortSet), + Health: container.HealthString(), + Running: container.Running, + Paused: container.Paused, + ExitCode: container.ExitCode(), + } + + if snapshot.Names == nil { + // Dead containers will often have no name, so make sure the response isn't null + snapshot.Names = []string{} + } + + if container.HostConfig != nil { + snapshot.Container.HostConfig.NetworkMode = string(container.HostConfig.NetworkMode) + snapshot.HostConfig.Isolation = string(container.HostConfig.Isolation) + for binding := range container.HostConfig.PortBindings { + snapshot.PortBindings[binding] = struct{}{} + } + } + + if container.Config != nil { + snapshot.Image = container.Config.Image + snapshot.Labels = container.Config.Labels + for exposed := range container.Config.ExposedPorts { + snapshot.ExposedPorts[exposed] = struct{}{} + } + } + + if len(container.Args) > 0 { + args := []string{} + for _, arg := range container.Args { + if strings.Contains(arg, " ") { + args = append(args, fmt.Sprintf("'%s'", arg)) + } else { + args = append(args, arg) + } + } + argsAsString := strings.Join(args, " ") + snapshot.Command = fmt.Sprintf("%s %s", container.Path, argsAsString) + } else { + snapshot.Command = container.Path + } + + snapshot.Ports = []types.Port{} + networks := make(map[string]*network.EndpointSettings) + if container.NetworkSettings != nil { + for name, netw := range container.NetworkSettings.Networks { + if netw == nil || netw.EndpointSettings == nil { + continue + } + networks[name] = &network.EndpointSettings{ + EndpointID: netw.EndpointID, + Gateway: netw.Gateway, + IPAddress: netw.IPAddress, + IPPrefixLen: netw.IPPrefixLen, + IPv6Gateway: netw.IPv6Gateway, + GlobalIPv6Address: netw.GlobalIPv6Address, + GlobalIPv6PrefixLen: netw.GlobalIPv6PrefixLen, + MacAddress: netw.MacAddress, + NetworkID: netw.NetworkID, + } + if netw.IPAMConfig != nil { + networks[name].IPAMConfig = &network.EndpointIPAMConfig{ + IPv4Address: netw.IPAMConfig.IPv4Address, + IPv6Address: netw.IPAMConfig.IPv6Address, + } + } + } + for port, bindings := range container.NetworkSettings.Ports { + p, err := nat.ParsePort(port.Port()) + if err != nil { + logrus.Warnf("invalid port map %+v", err) + continue + } + if len(bindings) == 0 { + snapshot.Ports = append(snapshot.Ports, types.Port{ + PrivatePort: uint16(p), + Type: port.Proto(), + }) + continue + } + for _, binding := range bindings { + h, err := nat.ParsePort(binding.HostPort) + if err != nil { + logrus.Warnf("invalid host port map %+v", err) + continue + } + snapshot.Ports = append(snapshot.Ports, types.Port{ + PrivatePort: uint16(p), + PublicPort: uint16(h), + Type: port.Proto(), + IP: binding.HostIP, + }) + } + } + } + snapshot.NetworkSettings = &types.SummaryNetworkSettings{Networks: networks} + + return snapshot +} + +// containerByIDIndexer is used to extract the ID field from Container types. +// memdb.StringFieldIndex can not be used since ID is a field from an embedded struct. +type containerByIDIndexer struct{} + +// FromObject implements the memdb.SingleIndexer interface for Container objects +func (e *containerByIDIndexer) FromObject(obj interface{}) (bool, []byte, error) { + c, ok := obj.(*Container) + if !ok { + return false, nil, fmt.Errorf("%T is not a Container", obj) + } + // Add the null character as a terminator + v := c.ID + "\x00" + return true, []byte(v), nil +} + +// FromArgs implements the memdb.Indexer interface +func (e *containerByIDIndexer) FromArgs(args ...interface{}) ([]byte, error) { + if len(args) != 1 { + return nil, fmt.Errorf("must provide only a single argument") + } + arg, ok := args[0].(string) + if !ok { + return nil, fmt.Errorf("argument must be a string: %#v", args[0]) + } + // Add the null character as a terminator + arg += "\x00" + return []byte(arg), nil } diff --git a/container/view_test.go b/container/view_test.go index 20424ffd98..9b872998bd 100644 --- a/container/view_test.go +++ b/container/view_test.go @@ -1,29 +1,73 @@ package container -import "testing" +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" -func TestViewSave(t *testing.T) { + containertypes "github.com/docker/docker/api/types/container" + "github.com/docker/docker/pkg/registrar" + "github.com/pborman/uuid" +) + +var root string + +func TestMain(m *testing.M) { + var err error + root, err = ioutil.TempDir("", "docker-container-test-") + if err != nil { + panic(err) + } + defer os.RemoveAll(root) + + os.Exit(m.Run()) +} + +func newContainer(t *testing.T) *Container { + var ( + id = uuid.New() + cRoot = filepath.Join(root, id) + ) + if err := os.MkdirAll(cRoot, 0755); err != nil { + t.Fatal(err) + } + c := NewBaseContainer(id, cRoot) + c.HostConfig = &containertypes.HostConfig{} + return c +} + +func TestViewSaveDelete(t *testing.T) { db, err := NewViewDB() if err != nil { t.Fatal(err) } - c := NewBaseContainer("id", "root") + c := newContainer(t) if err := c.CheckpointTo(db); err != nil { t.Fatal(err) } + if err := db.Delete(c); err != nil { + t.Fatal(err) + } } func TestViewAll(t *testing.T) { var ( db, _ = NewViewDB() - one = NewBaseContainer("id1", "root1") - two = NewBaseContainer("id2", "root2") + names = registrar.NewRegistrar() + one = newContainer(t) + two = newContainer(t) ) one.Pid = 10 + if err := one.CheckpointTo(db); err != nil { + t.Fatal(err) + } two.Pid = 20 - one.CheckpointTo(db) - two.CheckpointTo(db) - all, err := db.Snapshot().All() + if err := two.CheckpointTo(db); err != nil { + t.Fatal(err) + } + + all, err := db.Snapshot(names).All() if err != nil { t.Fatal(err) } @@ -34,24 +78,29 @@ func TestViewAll(t *testing.T) { for i := range all { byID[all[i].ID] = all[i] } - if s, ok := byID["id1"]; !ok || s.Pid != 10 { - t.Fatalf("expected something different with for id1: %v", s) + if s, ok := byID[one.ID]; !ok || s.Pid != 10 { + t.Fatalf("expected something different with for id=%s: %v", one.ID, s) } - if s, ok := byID["id2"]; !ok || s.Pid != 20 { - t.Fatalf("expected something different with for id1: %v", s) + if s, ok := byID[two.ID]; !ok || s.Pid != 20 { + t.Fatalf("expected something different with for id=%s: %v", two.ID, s) } } func TestViewGet(t *testing.T) { - db, _ := NewViewDB() - one := NewBaseContainer("id", "root") + var ( + db, _ = NewViewDB() + names = registrar.NewRegistrar() + one = newContainer(t) + ) one.ImageID = "some-image-123" - one.CheckpointTo(db) - s, err := db.Snapshot().Get("id") + if err := one.CheckpointTo(db); err != nil { + t.Fatal(err) + } + s, err := db.Snapshot(names).Get(one.ID) if err != nil { t.Fatal(err) } if s == nil || s.ImageID != "some-image-123" { - t.Fatalf("expected something different. Got: %v", s) + t.Fatalf("expected ImageID=some-image-123. Got: %v", s) } } diff --git a/daemon/container.go b/daemon/container.go index 72689fbbaa..6582da82c8 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -218,7 +218,7 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig * runconfig.SetDefaultNetModeIfBlank(hostConfig) container.HostConfig = hostConfig - return container.CheckpointAndSaveToDisk(daemon.containersReplica) + return container.CheckpointTo(daemon.containersReplica) } // verifyContainerSettings performs validation of the hostconfig and config diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 9e5a02c3f8..8a066bc899 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -45,10 +45,11 @@ func (daemon *Daemon) getDNSSearchSettings(container *container.Container) []str return nil } +// checkpointAndSave grabs a container lock to safely call container.CheckpointTo func (daemon *Daemon) checkpointAndSave(container *container.Container) error { container.Lock() defer container.Unlock() - if err := container.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + if err := container.CheckpointTo(daemon.containersReplica); err != nil { return fmt.Errorf("Error saving container state: %v", err) } return nil diff --git a/daemon/create.go b/daemon/create.go index a39dde5114..d47de31fa8 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -167,11 +167,6 @@ func (daemon *Daemon) create(params types.ContainerCreateConfig, managed bool) ( runconfig.SetDefaultNetModeIfBlank(container.HostConfig) daemon.updateContainerNetworkSettings(container, endpointsConfigs) - - if err := container.ToDisk(); err != nil { - logrus.Errorf("Error saving new container to disk: %v", err) - return nil, err - } if err := daemon.Register(container); err != nil { return nil, err } diff --git a/daemon/daemon.go b/daemon/daemon.go index ab46ad3341..6f4ac26ac2 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -217,7 +217,7 @@ func (daemon *Daemon) restore() error { go func(c *container.Container) { defer wg.Done() daemon.backportMountSpec(c) - if err := c.ToDiskLocking(); err != nil { + if err := daemon.checkpointAndSave(c); err != nil { logrus.WithError(err).WithField("container", c.ID).Error("error saving backported mountspec to disk") } @@ -289,7 +289,9 @@ func (daemon *Daemon) restore() error { logrus.Debugf("Resetting RemovalInProgress flag from %v", c.ID) c.RemovalInProgress = false c.Dead = true - c.ToDisk() + if err := c.CheckpointTo(daemon.containersReplica); err != nil { + logrus.Errorf("Failed to update container %s state: %v", c.ID, err) + } } c.Unlock() }(c) diff --git a/daemon/daemon_unix_test.go b/daemon/daemon_unix_test.go index a2847bfbee..eb19376845 100644 --- a/daemon/daemon_unix_test.go +++ b/daemon/daemon_unix_test.go @@ -274,6 +274,10 @@ func TestMigratePre17Volumes(t *testing.T) { } `) + viewDB, err := container.NewViewDB() + if err != nil { + t.Fatal(err) + } volStore, err := store.New(volumeRoot) if err != nil { t.Fatal(err) @@ -284,7 +288,12 @@ func TestMigratePre17Volumes(t *testing.T) { } volumedrivers.Register(drv, volume.DefaultDriverName) - daemon := &Daemon{root: rootDir, repository: containerRoot, volumes: volStore} + daemon := &Daemon{ + root: rootDir, + repository: containerRoot, + containersReplica: viewDB, + volumes: volStore, + } err = ioutil.WriteFile(filepath.Join(containerRoot, cid, "config.v2.json"), config, 600) if err != nil { t.Fatal(err) diff --git a/daemon/delete.go b/daemon/delete.go index 3609a88e9f..2d3cd0f90f 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -105,15 +105,11 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo // Mark container dead. We don't want anybody to be restarting it. container.Lock() container.Dead = true - if err = container.CheckpointTo(daemon.containersReplica); err != nil { - container.Unlock() - return err - } // Save container state to disk. So that if error happens before // container meta file got removed from disk, then a restart of // docker should not make a dead container alive. - if err := container.ToDisk(); err != nil && !os.IsNotExist(err) { + if err := container.CheckpointTo(daemon.containersReplica); err != nil && !os.IsNotExist(err) { logrus.Errorf("Error saving dying container to disk: %v", err) } container.Unlock() @@ -137,7 +133,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo selinuxFreeLxcContexts(container.ProcessLabel) daemon.idIndex.Delete(container.ID) daemon.containers.Delete(container.ID) - daemon.containersReplica.Delete(container.ID) + daemon.containersReplica.Delete(container) if e := daemon.removeMountPoints(container, removeVolume); e != nil { logrus.Error(e) } diff --git a/daemon/list.go b/daemon/list.go index 2d10e3d6de..870a7bda2d 100644 --- a/daemon/list.go +++ b/daemon/list.go @@ -100,18 +100,18 @@ type listContext struct { *types.ContainerListOptions } -// byContainerCreated is a temporary type used to sort a list of containers by creation time. -type byContainerCreated []container.Snapshot +// byCreatedDescending is a temporary type used to sort a list of containers by creation time. +type byCreatedDescending []container.Snapshot -func (r byContainerCreated) Len() int { return len(r) } -func (r byContainerCreated) Swap(i, j int) { r[i], r[j] = r[j], r[i] } -func (r byContainerCreated) Less(i, j int) bool { - return r[i].Created.UnixNano() < r[j].Created.UnixNano() +func (r byCreatedDescending) Len() int { return len(r) } +func (r byCreatedDescending) Swap(i, j int) { r[i], r[j] = r[j], r[i] } +func (r byCreatedDescending) Less(i, j int) bool { + return r[j].CreatedAt.UnixNano() < r[i].CreatedAt.UnixNano() } // Containers returns the list of containers to show given the user's filtering. func (daemon *Daemon) Containers(config *types.ContainerListOptions) ([]*types.Container, error) { - return daemon.reduceContainers(config, daemon.transformContainer) + return daemon.reduceContainers(config, daemon.refreshImage) } func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContext) ([]container.Snapshot, error) { @@ -123,7 +123,7 @@ func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContex // standard behavior of walking the entire container // list from the daemon's in-memory store all, err := view.All() - sort.Sort(sort.Reverse(byContainerCreated(all))) + sort.Sort(byCreatedDescending(all)) return all, err } @@ -172,7 +172,7 @@ func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContex // Restore sort-order after filtering // Created gives us nanosec resolution for sorting - sort.Sort(sort.Reverse(byContainerCreated(cntrs))) + sort.Sort(byCreatedDescending(cntrs)) return cntrs, nil } @@ -180,7 +180,7 @@ func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContex // reduceContainers parses the user's filtering options and generates the list of containers to return based on a reducer. func (daemon *Daemon) reduceContainers(config *types.ContainerListOptions, reducer containerReducer) ([]*types.Container, error) { var ( - view = daemon.containersReplica.Snapshot() + view = daemon.containersReplica.Snapshot(daemon.nameIndex) containers = []*types.Container{} ) @@ -503,9 +503,15 @@ func includeContainerInList(container *container.Snapshot, ctx *listContext) ite } } - networkExist := fmt.Errorf("container part of network") + var ( + networkExist = errors.New("container part of network") + noNetworks = errors.New("container is not part of any networks") + ) if ctx.filters.Include("network") { err := ctx.filters.WalkValues("network", func(value string) error { + if container.NetworkSettings == nil { + return noNetworks + } if _, ok := container.NetworkSettings.Networks[value]; ok { return networkExist } @@ -527,7 +533,7 @@ func includeContainerInList(container *container.Snapshot, ctx *listContext) ite if len(ctx.publish) > 0 { shouldSkip := true for port := range ctx.publish { - if _, ok := container.PublishPorts[port]; ok { + if _, ok := container.PortBindings[port]; ok { shouldSkip = false break } @@ -553,40 +559,22 @@ func includeContainerInList(container *container.Snapshot, ctx *listContext) ite return includeContainer } -// transformContainer generates the container type expected by the docker ps command. -func (daemon *Daemon) transformContainer(container *container.Snapshot, ctx *listContext) (*types.Container, error) { - newC := &types.Container{ - ID: container.ID, - Names: ctx.names[container.ID], - ImageID: container.ImageID, - Command: container.Command, - Created: container.Created.Unix(), - State: container.State, - Status: container.Status, - NetworkSettings: &container.NetworkSettings, - Ports: container.Ports, - Labels: container.Labels, - Mounts: container.Mounts, - } - if newC.Names == nil { - // Dead containers will often have no name, so make sure the response isn't null - newC.Names = []string{} - } - newC.HostConfig.NetworkMode = container.HostConfig.NetworkMode - - image := container.Image // if possible keep the original ref - if image != container.ImageID { +// refreshImage checks if the Image ref still points to the correct ID, and updates the ref to the actual ID when it doesn't +func (daemon *Daemon) refreshImage(s *container.Snapshot, ctx *listContext) (*types.Container, error) { + c := s.Container + image := s.Image // keep the original ref if still valid (hasn't changed) + if image != s.ImageID { id, _, err := daemon.GetImageIDAndPlatform(image) if _, isDNE := err.(ErrImageDoesNotExist); err != nil && !isDNE { return nil, err } - if err != nil || id.String() != container.ImageID { - image = container.ImageID + if err != nil || id.String() != s.ImageID { + // ref changed, we need to use original ID + image = s.ImageID } } - newC.Image = image - - return newC, nil + c.Image = image + return &c, nil } // Volumes lists known volumes, using the filter to restrict the range diff --git a/daemon/monitor.go b/daemon/monitor.go index be8a500020..4f9cc37a04 100644 --- a/daemon/monitor.go +++ b/daemon/monitor.go @@ -90,7 +90,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { daemon.setStateCounter(c) defer c.Unlock() - if err := c.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + if err := c.CheckpointTo(daemon.containersReplica); err != nil { return err } return daemon.postRunProcessing(c, e) @@ -119,7 +119,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { c.HasBeenStartedBefore = true daemon.setStateCounter(c) - if err := c.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + if err := c.CheckpointTo(daemon.containersReplica); err != nil { c.Reset(false) return err } @@ -130,7 +130,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { // Container is already locked in this case c.Paused = true daemon.setStateCounter(c) - if err := c.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + if err := c.CheckpointTo(daemon.containersReplica); err != nil { return err } daemon.updateHealthMonitor(c) @@ -139,7 +139,7 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { // Container is already locked in this case c.Paused = false daemon.setStateCounter(c) - if err := c.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + if err := c.CheckpointTo(daemon.containersReplica); err != nil { return err } daemon.updateHealthMonitor(c) diff --git a/daemon/rename.go b/daemon/rename.go index 1a5e985cfc..ad21593c19 100644 --- a/daemon/rename.go +++ b/daemon/rename.go @@ -82,7 +82,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { daemon.nameIndex.Release(oldName + k) } daemon.releaseName(oldName) - if err = container.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + if err = container.CheckpointTo(daemon.containersReplica); err != nil { return err } @@ -99,7 +99,7 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) error { if err != nil { container.Name = oldName container.NetworkSettings.IsAnonymousEndpoint = oldIsAnonymousEndpoint - if e := container.CheckpointAndSaveToDisk(daemon.containersReplica); e != nil { + if e := container.CheckpointTo(daemon.containersReplica); e != nil { logrus.Errorf("%s: Failed in writing to Disk on rename failure: %v", container.ID, e) } } diff --git a/daemon/restart.go b/daemon/restart.go index 79292f3752..9f2ef569af 100644 --- a/daemon/restart.go +++ b/daemon/restart.go @@ -52,7 +52,7 @@ func (daemon *Daemon) containerRestart(container *container.Container, seconds i container.HostConfig.AutoRemove = autoRemove // containerStop will write HostConfig to disk, we shall restore AutoRemove // in disk too - if toDiskErr := container.ToDiskLocking(); toDiskErr != nil { + if toDiskErr := daemon.checkpointAndSave(container); toDiskErr != nil { logrus.Errorf("Write container to disk error: %v", toDiskErr) } diff --git a/daemon/start.go b/daemon/start.go index c2e9b7069f..a00cb901b5 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -58,7 +58,7 @@ 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 container.NetworkSettings.Networks = nil - if err := container.ToDisk(); err != nil { + if err := container.CheckpointTo(daemon.containersReplica); err != nil { return err } } @@ -117,7 +117,7 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint if container.ExitCode() == 0 { container.SetExitCode(128) } - if err := container.CheckpointAndSaveToDisk(daemon.containersReplica); err != nil { + if err := container.CheckpointTo(daemon.containersReplica); err != nil { logrus.Errorf("%s: failed saving state on start failure: %v", container.ID, err) } container.Reset(false) diff --git a/daemon/start_unix.go b/daemon/start_unix.go index 103cc73b86..12ecdab2db 100644 --- a/daemon/start_unix.go +++ b/daemon/start_unix.go @@ -9,13 +9,14 @@ import ( "github.com/docker/docker/libcontainerd" ) +// getLibcontainerdCreateOptions callers must hold a lock on the container func (daemon *Daemon) getLibcontainerdCreateOptions(container *container.Container) ([]libcontainerd.CreateOption, error) { createOptions := []libcontainerd.CreateOption{} // Ensure a runtime has been assigned to this container if container.HostConfig.Runtime == "" { container.HostConfig.Runtime = daemon.configStore.GetDefaultRuntimeName() - container.ToDisk() + container.CheckpointTo(daemon.containersReplica) } rt := daemon.configStore.GetRuntime(container.HostConfig.Runtime) diff --git a/daemon/update.go b/daemon/update.go index af9bf7b991..a65cbd51b1 100644 --- a/daemon/update.go +++ b/daemon/update.go @@ -38,7 +38,7 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro if restoreConfig { container.Lock() container.HostConfig = &backupHostConfig - container.CheckpointAndSaveToDisk(daemon.containersReplica) + container.CheckpointTo(daemon.containersReplica) container.Unlock() } }() diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index c01c573e93..d6b48d381b 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -180,7 +180,7 @@ func (daemon *Daemon) verifyVolumesInfo(container *container.Container) error { container.MountPoints[destination] = &m } } - return container.ToDisk() + return container.CheckpointTo(daemon.containersReplica) } return nil } From 9134e87afc6f3215a58d23c7261242b764357501 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Wed, 5 Apr 2017 17:37:04 -0700 Subject: [PATCH 13/18] only Daemon.load needs to call label.ReserveLabel Signed-off-by: Fabio Kung --- container/container.go | 4 ---- container/view.go | 2 +- daemon/container.go | 6 +++++- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/container/container.go b/container/container.go index 2fd007e0a5..b0ff5b7c9b 100644 --- a/container/container.go +++ b/container/container.go @@ -45,7 +45,6 @@ import ( "github.com/docker/libnetwork/options" "github.com/docker/libnetwork/types" agentexec "github.com/docker/swarmkit/agent/exec" - "github.com/opencontainers/selinux/go-selinux/label" ) const configFileName = "config.v2.json" @@ -152,9 +151,6 @@ func (container *Container) FromDisk() error { container.Platform = runtime.GOOS } - if err := label.ReserveLabel(container.ProcessLabel); err != nil { - return err - } return container.readHostConfig() } diff --git a/container/view.go b/container/view.go index e5e1542f77..5c501fd9b4 100644 --- a/container/view.go +++ b/container/view.go @@ -95,7 +95,7 @@ func (db *memDB) Save(c *Container) error { txn := db.store.Txn(true) defer txn.Commit() deepCopy := NewBaseContainer(c.ID, c.Root) - err := deepCopy.FromDisk() // TODO: deal with reserveLabel + err := deepCopy.FromDisk() if err != nil { return err } diff --git a/daemon/container.go b/daemon/container.go index 6582da82c8..149df0dec6 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -18,6 +18,7 @@ import ( "github.com/docker/docker/pkg/truncindex" "github.com/docker/docker/runconfig" "github.com/docker/go-connections/nat" + "github.com/opencontainers/selinux/go-selinux/label" ) // GetContainer looks for a container using the provided information, which could be @@ -90,6 +91,9 @@ func (daemon *Daemon) load(id string) (*container.Container, error) { if err := container.FromDisk(); err != nil { return nil, err } + if err := label.ReserveLabel(container.ProcessLabel); err != nil { + return nil, err + } if container.ID != id { return container, fmt.Errorf("Container %s is stored at %s", container.ID, id) @@ -307,7 +311,7 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon return nil, fmt.Errorf("maximum retry count cannot be negative") } case "": - // do nothing + // do nothing default: return nil, fmt.Errorf("invalid restart policy '%s'", p.Name) } From a43be3431e51b914ab170569b9e58239d64b199c Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Thu, 6 Apr 2017 10:43:10 -0700 Subject: [PATCH 14/18] avoid re-reading json files when copying containers Signed-off-by: Fabio Kung --- container/container.go | 72 +++++++++++++++++++++++----------- container/view.go | 10 ++--- daemon/container_operations.go | 2 +- daemon/daemon_unix.go | 3 +- 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/container/container.go b/container/container.go index b0ff5b7c9b..fe050bcd17 100644 --- a/container/container.go +++ b/container/container.go @@ -1,6 +1,7 @@ package container import ( + "bytes" "encoding/json" "fmt" "io" @@ -14,8 +15,6 @@ import ( "syscall" "time" - "golang.org/x/net/context" - "github.com/Sirupsen/logrus" containertypes "github.com/docker/docker/api/types/container" mounttypes "github.com/docker/docker/api/types/mount" @@ -45,6 +44,7 @@ import ( "github.com/docker/libnetwork/options" "github.com/docker/libnetwork/types" agentexec "github.com/docker/swarmkit/agent/exec" + "golang.org/x/net/context" ) const configFileName = "config.v2.json" @@ -154,36 +154,48 @@ func (container *Container) FromDisk() error { return container.readHostConfig() } -// ToDisk saves the container configuration on disk. -func (container *Container) toDisk() error { +// toDisk saves the container configuration on disk and returns a deep copy. +func (container *Container) toDisk() (*Container, error) { + var ( + buf bytes.Buffer + deepCopy Container + ) pth, err := container.ConfigPath() if err != nil { - return err + return nil, err } - jsonSource, err := ioutils.NewAtomicFileWriter(pth, 0644) - if err != nil { - return err - } - defer jsonSource.Close() - - enc := json.NewEncoder(jsonSource) - // Save container settings - if err := enc.Encode(container); err != nil { - return err + f, err := ioutils.NewAtomicFileWriter(pth, 0644) + if err != nil { + return nil, err + } + defer f.Close() + + w := io.MultiWriter(&buf, f) + if err := json.NewEncoder(w).Encode(container); err != nil { + return nil, err } - return container.WriteHostConfig() + if err := json.NewDecoder(&buf).Decode(&deepCopy); err != nil { + return nil, err + } + deepCopy.HostConfig, err = container.WriteHostConfig() + if err != nil { + return nil, err + } + + return &deepCopy, nil } // CheckpointTo makes the Container's current state visible to queries, and persists state. // Callers must hold a Container lock. func (container *Container) CheckpointTo(store ViewDB) error { - if err := container.toDisk(); err != nil { + deepCopy, err := container.toDisk() + if err != nil { return err } - return store.Save(container) + return store.Save(deepCopy) } // readHostConfig reads the host configuration from disk for the container. @@ -215,20 +227,34 @@ func (container *Container) readHostConfig() error { return nil } -// WriteHostConfig saves the host configuration on disk for the container. -func (container *Container) WriteHostConfig() error { +// WriteHostConfig saves the host configuration on disk for the container, +// and returns a deep copy of the saved object. Callers must hold a Container lock. +func (container *Container) WriteHostConfig() (*containertypes.HostConfig, error) { + var ( + buf bytes.Buffer + deepCopy containertypes.HostConfig + ) + pth, err := container.HostConfigPath() if err != nil { - return err + return nil, err } f, err := ioutils.NewAtomicFileWriter(pth, 0644) if err != nil { - return err + return nil, err } defer f.Close() - return json.NewEncoder(f).Encode(&container.HostConfig) + w := io.MultiWriter(&buf, f) + if err := json.NewEncoder(w).Encode(&container.HostConfig); err != nil { + return nil, err + } + + if err := json.NewDecoder(&buf).Decode(&deepCopy); err != nil { + return nil, err + } + return &deepCopy, nil } // SetupWorkingDirectory sets up the container's working directory as set in container.Config.WorkingDir diff --git a/container/view.go b/container/view.go index 5c501fd9b4..13c7161852 100644 --- a/container/view.go +++ b/container/view.go @@ -90,16 +90,12 @@ func (db *memDB) Snapshot(index *registrar.Registrar) View { } } -// Save atomically updates the in-memory store from the current on-disk state of a Container. +// Save atomically updates the in-memory store state for a Container. +// Only read only (deep) copies of containers may be passed in. func (db *memDB) Save(c *Container) error { txn := db.store.Txn(true) defer txn.Commit() - deepCopy := NewBaseContainer(c.ID, c.Root) - err := deepCopy.FromDisk() - if err != nil { - return err - } - return txn.Insert(memdbTable, deepCopy) + return txn.Insert(memdbTable, c) } // Delete removes an item by ID diff --git a/daemon/container_operations.go b/daemon/container_operations.go index 8a066bc899..e4b67893d0 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -579,7 +579,7 @@ func (daemon *Daemon) allocateNetwork(container *container.Container) error { } - if err := container.WriteHostConfig(); err != nil { + if _, err := container.WriteHostConfig(); err != nil { return err } networkActions.WithValues("allocate").UpdateSince(start) diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index 662098ed19..0778dde4f7 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -1146,7 +1146,8 @@ func (daemon *Daemon) registerLinks(container *container.Container, hostConfig * // After we load all the links into the daemon // set them to nil on the hostconfig - return container.WriteHostConfig() + _, err := container.WriteHostConfig() + return err } // conditionalMountOnStart is a platform specific helper function during the From 04bd768a889f94a4dc6ad25e2a014dffd0a4e04e Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Thu, 6 Apr 2017 14:42:10 -0700 Subject: [PATCH 15/18] ensure heath monitor status updates are propagated initHealthMonitor and updateHealthMonitor can cause container state to be changed (State.Health). Signed-off-by: Fabio Kung --- container/health.go | 7 ++++--- daemon/monitor.go | 9 ++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/container/health.go b/container/health.go index 6e3cd12f3b..31c5600d25 100644 --- a/container/health.go +++ b/container/health.go @@ -13,9 +13,8 @@ type Health struct { // String returns a human-readable description of the health-check state func (s *Health) String() string { - // This happens when the container is being shutdown and the monitor has stopped - // or the monitor has yet to be setup. - if s.stop == nil { + // This happens when the monitor has yet to be setup. + if s.Status == "" { return types.Unhealthy } @@ -44,6 +43,8 @@ func (s *Health) CloseMonitorChannel() { logrus.Debug("CloseMonitorChannel: waiting for probe to stop") close(s.stop) s.stop = nil + // unhealthy when the monitor has stopped for compatibility reasons + s.Status = types.Unhealthy logrus.Debug("CloseMonitorChannel done") } } diff --git a/daemon/monitor.go b/daemon/monitor.go index 4f9cc37a04..5156d9a8e1 100644 --- a/daemon/monitor.go +++ b/daemon/monitor.go @@ -39,6 +39,9 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { return errors.New("Received StateOOM from libcontainerd on Windows. This should never happen.") } daemon.updateHealthMonitor(c) + if err := c.CheckpointTo(daemon.containersReplica); err != nil { + return err + } daemon.LogContainerEvent(c, "oom") case libcontainerd.StateExit: @@ -119,30 +122,30 @@ func (daemon *Daemon) StateChanged(id string, e libcontainerd.StateInfo) error { c.HasBeenStartedBefore = true daemon.setStateCounter(c) + daemon.initHealthMonitor(c) if err := c.CheckpointTo(daemon.containersReplica); err != nil { c.Reset(false) return err } - daemon.initHealthMonitor(c) daemon.LogContainerEvent(c, "start") case libcontainerd.StatePause: // Container is already locked in this case c.Paused = true daemon.setStateCounter(c) + daemon.updateHealthMonitor(c) if err := c.CheckpointTo(daemon.containersReplica); err != nil { return err } - daemon.updateHealthMonitor(c) daemon.LogContainerEvent(c, "pause") case libcontainerd.StateResume: // Container is already locked in this case c.Paused = false daemon.setStateCounter(c) + daemon.updateHealthMonitor(c) if err := c.CheckpointTo(daemon.containersReplica); err != nil { return err } - daemon.updateHealthMonitor(c) daemon.LogContainerEvent(c, "unpause") } return nil From 66b231d598174986ad08236f0a38beb825e5a3c3 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Mon, 10 Apr 2017 09:53:27 -0700 Subject: [PATCH 16/18] delete unused code (daemon.Start) Signed-off-by: Fabio Kung --- daemon/start.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/daemon/start.go b/daemon/start.go index a00cb901b5..8d938519c4 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -86,11 +86,6 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos return daemon.containerStart(container, checkpoint, checkpointDir, true) } -// Start starts a container -func (daemon *Daemon) Start(container *container.Container) error { - return daemon.containerStart(container, "", "", true) -} - // containerStart prepares the container to run by setting up everything the // container needs, such as storage and networking, as well as links // between containers. The container is left waiting for a signal to From 76d96418b13080514f3fb861072b06cb91d71cff Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Mon, 10 Apr 2017 09:54:29 -0700 Subject: [PATCH 17/18] avoid saving container state to disk before daemon.Register Migrate legacy volumes (Daemon.verifyVolumesInfo) before containers are registered on the Daemon, so state on disk is not overwritten and legacy fields lost during registration. Signed-off-by: Fabio Kung --- daemon/daemon.go | 11 +++++------ daemon/daemon_unix_test.go | 11 +++-------- daemon/volumes_unix.go | 1 - integration-cli/docker_cli_daemon_test.go | 2 +- 4 files changed, 9 insertions(+), 16 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 6f4ac26ac2..ff40bdbe90 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -187,17 +187,16 @@ func (daemon *Daemon) restore() error { delete(containers, id) continue } - if err := daemon.Register(c); err != nil { - logrus.Errorf("Failed to register container %s: %s", c.ID, err) - delete(containers, id) - continue - } - // verify that all volumes valid and have been migrated from the pre-1.7 layout if err := daemon.verifyVolumesInfo(c); err != nil { // don't skip the container due to error logrus.Errorf("Failed to verify volumes for container '%s': %v", c.ID, err) } + if err := daemon.Register(c); err != nil { + logrus.Errorf("Failed to register container %s: %s", c.ID, err) + delete(containers, id) + continue + } // The LogConfig.Type is empty if the container was created before docker 1.12 with default log driver. // We should rewrite it to use the daemon defaults. diff --git a/daemon/daemon_unix_test.go b/daemon/daemon_unix_test.go index eb19376845..c3aa443e45 100644 --- a/daemon/daemon_unix_test.go +++ b/daemon/daemon_unix_test.go @@ -274,10 +274,6 @@ func TestMigratePre17Volumes(t *testing.T) { } `) - viewDB, err := container.NewViewDB() - if err != nil { - t.Fatal(err) - } volStore, err := store.New(volumeRoot) if err != nil { t.Fatal(err) @@ -289,10 +285,9 @@ func TestMigratePre17Volumes(t *testing.T) { volumedrivers.Register(drv, volume.DefaultDriverName) daemon := &Daemon{ - root: rootDir, - repository: containerRoot, - containersReplica: viewDB, - volumes: volStore, + root: rootDir, + repository: containerRoot, + volumes: volStore, } err = ioutil.WriteFile(filepath.Join(containerRoot, cid, "config.v2.json"), config, 600) if err != nil { diff --git a/daemon/volumes_unix.go b/daemon/volumes_unix.go index d6b48d381b..0a4cbf8493 100644 --- a/daemon/volumes_unix.go +++ b/daemon/volumes_unix.go @@ -180,7 +180,6 @@ func (daemon *Daemon) verifyVolumesInfo(container *container.Container) error { container.MountPoints[destination] = &m } } - return container.CheckpointTo(daemon.containersReplica) } return nil } diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index 4ec5ac230d..849820aa14 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -2684,7 +2684,7 @@ func (s *DockerDaemonSuite) TestDaemonBackcompatPre17Volumes(c *check.C) { `) configPath := filepath.Join(d.Root, "containers", id, "config.v2.json") - err = ioutil.WriteFile(configPath, config, 600) + c.Assert(ioutil.WriteFile(configPath, config, 600), checker.IsNil) d.Start(c) out, err = d.Cmd("inspect", "--type=container", "--format={{ json .Mounts }}", id) From 37addf0a50ccba51630368c6ed09eb08166d6f48 Mon Sep 17 00:00:00 2001 From: Fabio Kung Date: Thu, 22 Jun 2017 07:46:26 -0700 Subject: [PATCH 18/18] Net operations already hold locks to containers Fix a deadlock caused by re-entrant locks on container objects. Signed-off-by: Fabio Kung --- daemon/container_operations.go | 14 ++------------ daemon/daemon.go | 10 ++++++++++ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/daemon/container_operations.go b/daemon/container_operations.go index e4b67893d0..00ed2f4787 100644 --- a/daemon/container_operations.go +++ b/daemon/container_operations.go @@ -45,16 +45,6 @@ func (daemon *Daemon) getDNSSearchSettings(container *container.Container) []str return nil } -// checkpointAndSave grabs a container lock to safely call container.CheckpointTo -func (daemon *Daemon) checkpointAndSave(container *container.Container) error { - container.Lock() - defer container.Unlock() - if err := container.CheckpointTo(daemon.containersReplica); err != nil { - return fmt.Errorf("Error saving container state: %v", err) - } - return nil -} - func (daemon *Daemon) buildSandboxOptions(container *container.Container) ([]libnetwork.SandboxOption, error) { var ( sboxOptions []libnetwork.SandboxOption @@ -1017,7 +1007,7 @@ func (daemon *Daemon) ConnectToNetwork(container *container.Container, idOrName } } - return daemon.checkpointAndSave(container) + return container.CheckpointTo(daemon.containersReplica) } // DisconnectFromNetwork disconnects container from network n. @@ -1053,7 +1043,7 @@ func (daemon *Daemon) DisconnectFromNetwork(container *container.Container, netw return err } - if err := daemon.checkpointAndSave(container); err != nil { + if err := container.CheckpointTo(daemon.containersReplica); err != nil { return err } diff --git a/daemon/daemon.go b/daemon/daemon.go index ff40bdbe90..ac03b75c2c 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -1233,3 +1233,13 @@ func CreateDaemonRoot(config *config.Config) error { } return setupDaemonRoot(config, realRoot, idMappings.RootPair()) } + +// checkpointAndSave grabs a container lock to safely call container.CheckpointTo +func (daemon *Daemon) checkpointAndSave(container *container.Container) error { + container.Lock() + defer container.Unlock() + if err := container.CheckpointTo(daemon.containersReplica); err != nil { + return fmt.Errorf("Error saving container state: %v", err) + } + return nil +}