Merge pull request #468 from thaJeztah/18.03-do-not-panic

[18.03] ExportContainer: do not panic

components/engine/container/archive.go

@@ -6,6 +6,7 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/system"
+	"github.com/pkg/errors"
 )
 
 // ResolvePath resolves the given path in the container to a resource on the
@@ -13,6 +14,9 @@ import (
 // the absolute path to the resource relative to the container's rootfs, and
 // an error if the path points to outside the container's rootfs.
 func (container *Container) ResolvePath(path string) (resolvedPath, absPath string, err error) {
+	if container.BaseFS == nil {
+		return "", "", errors.New("ResolvePath: BaseFS of container " + container.ID + " is unexpectedly nil")
+	}
 	// Check if a drive letter supplied, it must be the system drive. No-op except on Windows
 	path, err = system.CheckSystemDriveAndRemoveDriveLetter(path, container.BaseFS)
 	if err != nil {
@@ -45,6 +49,9 @@ func (container *Container) ResolvePath(path string) (resolvedPath, absPath stri
 // resolved to a path on the host corresponding to the given absolute path
 // inside the container.
 func (container *Container) StatPath(resolvedPath, absPath string) (stat *types.ContainerPathStat, err error) {
+	if container.BaseFS == nil {
+		return nil, errors.New("StatPath: BaseFS of container " + container.ID + " is unexpectedly nil")
+	}
 	driver := container.BaseFS
 
 	lstat, err := driver.Lstat(resolvedPath)

components/engine/container/container.go

@@ -311,6 +311,9 @@ func (container *Container) SetupWorkingDirectory(rootIDs idtools.IDPair) error
 //       symlinking to a different path) between using this method and using the
 //       path. See symlink.FollowSymlinkInScope for more details.
 func (container *Container) GetResourcePath(path string) (string, error) {
+	if container.BaseFS == nil {
+		return "", errors.New("GetResourcePath: BaseFS of container " + container.ID + " is unexpectedly nil")
+	}
 	// IMPORTANT - These are paths on the OS where the daemon is running, hence
 	// any filepath operations must be done in an OS agnostic way.
 	r, e := container.BaseFS.ResolveScopedPath(path, false)

components/engine/daemon/export.go

@@ -61,12 +61,12 @@ func (daemon *Daemon) containerExport(container *container.Container) (arch io.R
 		}
 	}()
 
-	_, err = rwlayer.Mount(container.GetMountLabel())
+	basefs, err := rwlayer.Mount(container.GetMountLabel())
 	if err != nil {
 		return nil, err
 	}
 
-	archive, err := archivePath(container.BaseFS, container.BaseFS.Path(), &archive.TarOptions{
+	archive, err := archivePath(basefs, basefs.Path(), &archive.TarOptions{
 		Compression: archive.Uncompressed,
 		UIDMaps:     daemon.idMappings.UIDs(),
 		GIDMaps:     daemon.idMappings.GIDs(),

components/engine/daemon/oci_linux.go

@@ -705,6 +705,9 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c
 }
 
 func (daemon *Daemon) populateCommonSpec(s *specs.Spec, c *container.Container) error {
+	if c.BaseFS == nil {
+		return errors.New("populateCommonSpec: BaseFS of container " + c.ID + " is unexpectedly nil")
+	}
 	linkedEnv, err := daemon.setupLinkedContainers(c)
 	if err != nil {
 		return err

components/engine/daemon/oci_windows.go

@@ -254,6 +254,10 @@ func (daemon *Daemon) createSpecWindowsFields(c *container.Container, s *specs.S
 
 	s.Root.Readonly = false // Windows does not support a read-only root filesystem
 	if !isHyperV {
+		if c.BaseFS == nil {
+			return errors.New("createSpecWindowsFields: BaseFS of container " + c.ID + " is unexpectedly nil")
+		}
+
 		s.Root.Path = c.BaseFS.Path() // This is not set for Hyper-V containers
 		if !strings.HasSuffix(s.Root.Path, `\`) {
 			s.Root.Path = s.Root.Path + `\` // Ensure a correctly formatted volume GUID path \\?\Volume{GUID}\

components/engine/integration/container/export_test.go

@@ -7,7 +7,9 @@ import (
 	"time"
 
 	"github.com/docker/docker/api/types"
+	containerTypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/filters"
+	"github.com/docker/docker/integration-cli/daemon"
 	"github.com/docker/docker/integration/internal/container"
 	"github.com/docker/docker/integration/internal/request"
 	"github.com/docker/docker/pkg/jsonmessage"
@@ -51,3 +53,32 @@ func TestExportContainerAndImportImage(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, jm.Status, images[0].ID)
 }
+
+// TestExportContainerAfterDaemonRestart checks that a container
+// created before start of the currently running dockerd
+// can be exported (as reported in #36561). To satisfy this
+// condition, daemon restart is needed after container creation.
+func TestExportContainerAfterDaemonRestart(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType != "linux")
+	skip.If(t, testEnv.IsRemoteDaemon())
+
+	d := daemon.New(t, "", "dockerd", daemon.Config{})
+	client, err := d.NewClient()
+	require.NoError(t, err)
+
+	d.StartWithBusybox(t)
+	defer d.Stop(t)
+
+	ctx := context.Background()
+	cfg := containerTypes.Config{
+		Image: "busybox",
+		Cmd:   []string{"top"},
+	}
+	ctr, err := client.ContainerCreate(ctx, &cfg, nil, nil, "")
+	require.NoError(t, err)
+
+	d.Restart(t)
+
+	_, err = client.ContainerExport(ctx, ctr.ID)
+	assert.NoError(t, err)
+}