diff --git a/components/engine/api/swagger.yaml b/components/engine/api/swagger.yaml index 8ff7415d62..6fabc89a07 100644 --- a/components/engine/api/swagger.yaml +++ b/components/engine/api/swagger.yaml @@ -443,6 +443,10 @@ definitions: OomKillDisable: description: "Disable OOM Killer for the container." type: "boolean" + Init: + description: "Run an init inside the container that forwards signals and reaps processes. This field is omitted if empty, and the default (as configured on the daemon) is used." + type: "boolean" + x-nullable: true PidsLimit: description: "Tune a container's pids limit. Set -1 for unlimited." type: "integer" diff --git a/components/engine/client/container_copy.go b/components/engine/client/container_copy.go index b7d5252ab2..6985036df8 100644 --- a/components/engine/client/container_copy.go +++ b/components/engine/client/container_copy.go @@ -23,7 +23,7 @@ func (cli *Client) ContainerStatPath(ctx context.Context, containerID, path stri urlStr := "/containers/" + containerID + "/archive" response, err := cli.head(ctx, urlStr, query, nil) if err != nil { - return types.ContainerPathStat{}, err + return types.ContainerPathStat{}, wrapResponseError(err, response, "container:path", containerID+":"+path) } defer ensureReaderClosed(response) return getContainerPathStatFromHeader(response.header) @@ -31,9 +31,9 @@ func (cli *Client) ContainerStatPath(ctx context.Context, containerID, path stri // CopyToContainer copies content into the container filesystem. // Note that `content` must be a Reader for a TAR archive -func (cli *Client) CopyToContainer(ctx context.Context, container, path string, content io.Reader, options types.CopyToContainerOptions) error { +func (cli *Client) CopyToContainer(ctx context.Context, containerID, dstPath string, content io.Reader, options types.CopyToContainerOptions) error { query := url.Values{} - query.Set("path", filepath.ToSlash(path)) // Normalize the paths used in the API. + query.Set("path", filepath.ToSlash(dstPath)) // Normalize the paths used in the API. // Do not allow for an existing directory to be overwritten by a non-directory and vice versa. if !options.AllowOverwriteDirWithFile { query.Set("noOverwriteDirNonDir", "true") @@ -43,11 +43,11 @@ func (cli *Client) CopyToContainer(ctx context.Context, container, path string, query.Set("copyUIDGID", "true") } - apiPath := "/containers/" + container + "/archive" + apiPath := "/containers/" + containerID + "/archive" response, err := cli.putRaw(ctx, apiPath, query, content, nil) if err != nil { - return err + return wrapResponseError(err, response, "container:path", containerID+":"+dstPath) } defer ensureReaderClosed(response) @@ -60,14 +60,14 @@ func (cli *Client) CopyToContainer(ctx context.Context, container, path string, // CopyFromContainer gets the content from the container and returns it as a Reader // for a TAR archive to manipulate it in the host. It's up to the caller to close the reader. -func (cli *Client) CopyFromContainer(ctx context.Context, container, srcPath string) (io.ReadCloser, types.ContainerPathStat, error) { +func (cli *Client) CopyFromContainer(ctx context.Context, containerID, srcPath string) (io.ReadCloser, types.ContainerPathStat, error) { query := make(url.Values, 1) query.Set("path", filepath.ToSlash(srcPath)) // Normalize the paths used in the API. - apiPath := "/containers/" + container + "/archive" + apiPath := "/containers/" + containerID + "/archive" response, err := cli.get(ctx, apiPath, query, nil) if err != nil { - return nil, types.ContainerPathStat{}, err + return nil, types.ContainerPathStat{}, wrapResponseError(err, response, "container:path", containerID+":"+srcPath) } if response.statusCode != http.StatusOK { diff --git a/components/engine/client/container_copy_test.go b/components/engine/client/container_copy_test.go index c84f82e9fb..706711ece0 100644 --- a/components/engine/client/container_copy_test.go +++ b/components/engine/client/container_copy_test.go @@ -25,6 +25,16 @@ func TestContainerStatPathError(t *testing.T) { } } +func TestContainerStatPathNotFoundError(t *testing.T) { + client := &Client{ + client: newMockClient(errorMock(http.StatusNotFound, "Not found")), + } + _, err := client.ContainerStatPath(context.Background(), "container_id", "path") + if !IsErrNotFound(err) { + t.Fatalf("expected a not found error, got %v", err) + } +} + func TestContainerStatPathNoHeaderError(t *testing.T) { client := &Client{ client: newMockClient(func(req *http.Request) (*http.Response, error) { @@ -95,6 +105,16 @@ func TestCopyToContainerError(t *testing.T) { } } +func TestCopyToContainerNotFoundError(t *testing.T) { + client := &Client{ + client: newMockClient(errorMock(http.StatusNotFound, "Not found")), + } + err := client.CopyToContainer(context.Background(), "container_id", "path/to/file", bytes.NewReader([]byte("")), types.CopyToContainerOptions{}) + if !IsErrNotFound(err) { + t.Fatalf("expected a not found error, got %v", err) + } +} + func TestCopyToContainerNotStatusOKError(t *testing.T) { client := &Client{ client: newMockClient(errorMock(http.StatusNoContent, "No content")), @@ -161,6 +181,16 @@ func TestCopyFromContainerError(t *testing.T) { } } +func TestCopyFromContainerNotFoundError(t *testing.T) { + client := &Client{ + client: newMockClient(errorMock(http.StatusNotFound, "Not found")), + } + _, _, err := client.CopyFromContainer(context.Background(), "container_id", "path/to/file") + if !IsErrNotFound(err) { + t.Fatalf("expected a not found error, got %v", err) + } +} + func TestCopyFromContainerNotStatusOKError(t *testing.T) { client := &Client{ client: newMockClient(errorMock(http.StatusNoContent, "No content")), diff --git a/components/engine/daemon/graphdriver/lcow/lcow_svm.go b/components/engine/daemon/graphdriver/lcow/lcow_svm.go index 26f6df4f03..d4a42df334 100644 --- a/components/engine/daemon/graphdriver/lcow/lcow_svm.go +++ b/components/engine/daemon/graphdriver/lcow/lcow_svm.go @@ -299,7 +299,10 @@ func (svm *serviceVM) createUnionMount(mountName string, mvds ...hcsshim.MappedV } var cmd string - if mvds[0].ReadOnly { + if len(mvds) == 1 { + // `FROM SCRATCH` case and the only layer. No overlay required. + cmd = fmt.Sprintf("mount %s %s", mvds[0].ContainerPath, mountName) + } else if mvds[0].ReadOnly { // Readonly overlay cmd = fmt.Sprintf("mount -t overlay overlay -olowerdir=%s %s", strings.Join(lowerLayers, ","), diff --git a/components/engine/docs/api/version-history.md b/components/engine/docs/api/version-history.md index 4eca7fbb11..3de652d941 100644 --- a/components/engine/docs/api/version-history.md +++ b/components/engine/docs/api/version-history.md @@ -236,7 +236,9 @@ keywords: "API, Docker, rcli, REST, documentation" * `POST /secrets/{id}/update` updates the secret `id`. * `POST /services/(id or name)/update` now accepts service name or prefix of service id as a parameter. * `POST /containers/create` added 2 built-in log-opts that work on all logging drivers, -`mode` (`blocking`|`non-blocking`), and `max-buffer-size` (e.g. `2m`) which enables a non-blocking log buffer. + `mode` (`blocking`|`non-blocking`), and `max-buffer-size` (e.g. `2m`) which enables a non-blocking log buffer. +* `POST /containers/create` now takes `HostConfig.Init` field to run an init + inside the container that forwards signals and reaps processes. ## v1.24 API changes diff --git a/components/engine/layer/ro_layer.go b/components/engine/layer/ro_layer.go index 29fa85362a..baa742e6e9 100644 --- a/components/engine/layer/ro_layer.go +++ b/components/engine/layer/ro_layer.go @@ -142,11 +142,7 @@ func storeLayer(tx MetadataTransaction, layer *roLayer) error { return err } } - if err := tx.setOS(layer.layerStore.os); err != nil { - return err - } - - return nil + return tx.setOS(layer.layerStore.os) } func newVerifiedReadCloser(rc io.ReadCloser, dgst digest.Digest) (io.ReadCloser, error) { diff --git a/components/engine/plugin/backend_linux.go b/components/engine/plugin/backend_linux.go index 7d71808e34..92e8a9c5aa 100644 --- a/components/engine/plugin/backend_linux.go +++ b/components/engine/plugin/backend_linux.go @@ -639,14 +639,10 @@ func (pm *Manager) Remove(name string, config *types.PluginRmConfig) error { return errors.Wrap(err, "error unmounting plugin data") } - removeDir := pluginDir + "-removing" - if err := os.Rename(pluginDir, removeDir); err != nil { - return errors.Wrap(err, "error performing atomic remove of plugin dir") + if err := atomicRemoveAll(pluginDir); err != nil { + return err } - if err := system.EnsureRemoveAll(removeDir); err != nil { - return errors.Wrap(err, "error removing plugin dir") - } pm.config.Store.Remove(p) pm.config.LogPluginEvent(id, name, "remove") pm.publisher.Publish(EventRemove{Plugin: p.PluginObj}) @@ -835,3 +831,35 @@ func splitConfigRootFSFromTar(in io.ReadCloser, config *[]byte) io.ReadCloser { }() return pr } + +func atomicRemoveAll(dir string) error { + renamed := dir + "-removing" + + err := os.Rename(dir, renamed) + switch { + case os.IsNotExist(err), err == nil: + // even if `dir` doesn't exist, we can still try and remove `renamed` + case os.IsExist(err): + // Some previous remove failed, check if the origin dir exists + if e := system.EnsureRemoveAll(renamed); e != nil { + return errors.Wrap(err, "rename target already exists and could not be removed") + } + if _, err := os.Stat(dir); os.IsNotExist(err) { + // origin doesn't exist, nothing left to do + return nil + } + + // attempt to rename again + if err := os.Rename(dir, renamed); err != nil { + return errors.Wrap(err, "failed to rename dir for atomic removal") + } + default: + return errors.Wrap(err, "failed to rename dir for atomic removal") + } + + if err := system.EnsureRemoveAll(renamed); err != nil { + os.Rename(renamed, dir) + return err + } + return nil +} diff --git a/components/engine/plugin/backend_linux_test.go b/components/engine/plugin/backend_linux_test.go new file mode 100644 index 0000000000..6cbe4cde12 --- /dev/null +++ b/components/engine/plugin/backend_linux_test.go @@ -0,0 +1,81 @@ +package plugin + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" +) + +func TestAtomicRemoveAllNormal(t *testing.T) { + dir, err := ioutil.TempDir("", "atomic-remove-with-normal") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) // just try to make sure this gets cleaned up + + if err := atomicRemoveAll(dir); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(dir); !os.IsNotExist(err) { + t.Fatalf("dir should be gone: %v", err) + } + if _, err := os.Stat(dir + "-removing"); !os.IsNotExist(err) { + t.Fatalf("dir should be gone: %v", err) + } +} + +func TestAtomicRemoveAllAlreadyExists(t *testing.T) { + dir, err := ioutil.TempDir("", "atomic-remove-already-exists") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) // just try to make sure this gets cleaned up + + if err := os.MkdirAll(dir+"-removing", 0755); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir + "-removing") + + if err := atomicRemoveAll(dir); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(dir); !os.IsNotExist(err) { + t.Fatalf("dir should be gone: %v", err) + } + if _, err := os.Stat(dir + "-removing"); !os.IsNotExist(err) { + t.Fatalf("dir should be gone: %v", err) + } +} + +func TestAtomicRemoveAllNotExist(t *testing.T) { + if err := atomicRemoveAll("/not-exist"); err != nil { + t.Fatal(err) + } + + dir, err := ioutil.TempDir("", "atomic-remove-already-exists") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) // just try to make sure this gets cleaned up + + // create the removing dir, but not the "real" one + foo := filepath.Join(dir, "foo") + removing := dir + "-removing" + if err := os.MkdirAll(removing, 0755); err != nil { + t.Fatal(err) + } + + if err := atomicRemoveAll(dir); err != nil { + t.Fatal(err) + } + + if _, err := os.Stat(foo); !os.IsNotExist(err) { + t.Fatalf("dir should be gone: %v", err) + } + if _, err := os.Stat(removing); !os.IsNotExist(err) { + t.Fatalf("dir should be gone: %v", err) + } +} diff --git a/components/engine/vendor.conf b/components/engine/vendor.conf index 6be316124f..177e54b73d 100644 --- a/components/engine/vendor.conf +++ b/components/engine/vendor.conf @@ -7,7 +7,7 @@ github.com/docker/libtrust 9cbd2a1374f46905c68a4eb3694a130610adc62a github.com/go-check/check 4ed411733c5785b40214c70bce814c3a3a689609 https://github.com/cpuguy83/check.git github.com/gorilla/context v1.1 github.com/gorilla/mux v1.1 -github.com/Microsoft/opengcs v0.3.5 +github.com/Microsoft/opengcs v0.3.6 github.com/kr/pty 5cf931ef8f github.com/mattn/go-shellwords v1.0.3 github.com/sirupsen/logrus v1.0.3 diff --git a/components/engine/vendor/github.com/Microsoft/opengcs/client/process.go b/components/engine/vendor/github.com/Microsoft/opengcs/client/process.go index 2b98f5331e..822a90b18e 100644 --- a/components/engine/vendor/github.com/Microsoft/opengcs/client/process.go +++ b/components/engine/vendor/github.com/Microsoft/opengcs/client/process.go @@ -84,8 +84,11 @@ func (config *Config) RunProcess(commandLine string, stdin io.Reader, stdout io. } // Don't need stdin now we've sent everything. This signals GCS that we are finished sending data. - if err := process.Process.CloseStdin(); err != nil { - return nil, err + if err := process.Process.CloseStdin(); err != nil && !hcsshim.IsNotExist(err) && !hcsshim.IsAlreadyClosed(err) { + // This error will occur if the compute system is currently shutting down + if perr, ok := err.(*hcsshim.ProcessError); ok && perr.Err != hcsshim.ErrVmcomputeOperationInvalidState { + return nil, err + } } }