Merge pull request moby#31273 from fabiokung/consistent-ro-view

No container locks on `docker ps`

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,7 +44,7 @@ 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"
+	"golang.org/x/net/context"
 )
 
 const configFileName = "config.v2.json"
@@ -152,41 +151,51 @@ func (container *Container) FromDisk() error {
 		container.Platform = runtime.GOOS
 	}
 
-	if err := label.ReserveLabel(container.ProcessLabel); err != nil {
-		return err
-	}
 	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)
+	// Save container settings
+	f, err := ioutils.NewAtomicFileWriter(pth, 0644)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	defer jsonSource.Close()
+	defer f.Close()
 
-	enc := json.NewEncoder(jsonSource)
+	w := io.MultiWriter(&buf, f)
+	if err := json.NewEncoder(w).Encode(container); err != nil {
+		return nil, err
+	}
 
-	// Save container settings
-	if err := enc.Encode(container); err != nil {
-		return err
+	if err := json.NewDecoder(&buf).Decode(&deepCopy); err != nil {
+		return nil, err
+	}
+	deepCopy.HostConfig, err = container.WriteHostConfig()
+	if err != nil {
+		return nil, err
 	}
 
-	return container.WriteHostConfig()
+	return &deepCopy, nil
 }
 
-// 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, and persists state.
+// Callers must hold a Container lock.
+func (container *Container) CheckpointTo(store ViewDB) error {
+	deepCopy, err := container.toDisk()
+	if err != nil {
+		return err
+	}
+	return store.Save(deepCopy)
 }
 
 // readHostConfig reads the host configuration from disk for the container.
@@ -218,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

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"
@@ -261,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
@@ -334,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
 }
 
@@ -462,3 +455,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
+}

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"
 )
@@ -125,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 ||
@@ -194,3 +192,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
+}

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")
 	}
 }