Merge pull request #1069 from sboeuf/fix_fc_k8s

mounts: Ignore existing mounts if they cannot be honored

virtcontainers/container.go

@@ -431,10 +431,10 @@ func (c *Container) createContainersDirs() error {
 	return nil
 }
 
-func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir string) (string, error) {
+func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir string) (string, bool, error) {
 	randBytes, err := utils.GenerateRandomBytes(8)
 	if err != nil {
-		return "", err
+		return "", false, err
 	}
 
 	filename := fmt.Sprintf("%s-%s-%s", c.id, hex.EncodeToString(randBytes), filepath.Base(m.Destination))
@@ -445,20 +445,35 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s
 	caps := c.sandbox.hypervisor.capabilities()
 	if !caps.isFsSharingSupported() {
 		c.Logger().Debug("filesystem sharing is not supported, files will be copied")
+
+		fileInfo, err := os.Stat(m.Source)
+		if err != nil {
+			return "", false, err
+		}
+
+		// Ignore the mount if this is not a regular file (excludes
+		// directory, socket, device, ...) as it cannot be handled by
+		// a simple copy. But this should not be treated as an error,
+		// only as a limitation.
+		if !fileInfo.Mode().IsRegular() {
+			c.Logger().WithField("ignored-file", m.Source).Debug("Ignoring non-regular file as FS sharing not supported")
+			return "", true, nil
+		}
+
 		if err := c.sandbox.agent.copyFile(m.Source, guestDest); err != nil {
-			return "", err
+			return "", false, err
 		}
 	} else {
 		// These mounts are created in the shared dir
 		mountDest := filepath.Join(hostSharedDir, c.sandbox.id, filename)
 		if err := bindMount(c.ctx, m.Source, mountDest, false); err != nil {
-			return "", err
+			return "", false, err
 		}
 		// Save HostPath mount value into the mount list of the container.
 		c.mounts[idx].HostPath = mountDest
 	}
 
-	return guestDest, nil
+	return guestDest, false, nil
 }
 
 // mountSharedDirMounts handles bind-mounts by bindmounting to the host shared
@@ -466,8 +481,9 @@ func (c *Container) shareFiles(m Mount, idx int, hostSharedDir, guestSharedDir s
 // It also updates the container mount list with the HostPath info, and store
 // container mounts to the storage. This way, we will have the HostPath info
 // available when we will need to unmount those mounts.
-func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ([]Mount, error) {
+func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) ([]Mount, []Mount, error) {
 	var sharedDirMounts []Mount
+	var ignoredMounts []Mount
 	for idx, m := range c.mounts {
 		if isSystemMount(m.Destination) || m.Type != "bind" {
 			continue
@@ -485,12 +501,12 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 		if len(m.BlockDeviceID) > 0 {
 			// Attach this block device, all other devices passed in the config have been attached at this point
 			if err := c.sandbox.devManager.AttachDevice(m.BlockDeviceID, c.sandbox); err != nil {
-				return nil, err
+				return nil, nil, err
 			}
 
 			if err := c.sandbox.storeSandboxDevices(); err != nil {
 				//TODO: roll back?
-				return nil, err
+				return nil, nil, err
 			}
 			continue
 		}
@@ -502,9 +518,15 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 			continue
 		}
 
-		guestDest, err := c.shareFiles(m, idx, hostSharedDir, guestSharedDir)
+		guestDest, ignore, err := c.shareFiles(m, idx, hostSharedDir, guestSharedDir)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
+		}
+
+		// Expand the list of mounts to ignore.
+		if ignore {
+			ignoredMounts = append(ignoredMounts, Mount{Source: m.Source})
+			continue
 		}
 
 		// Check if mount is readonly, let the agent handle the readonly mount
@@ -528,10 +550,10 @@ func (c *Container) mountSharedDirMounts(hostSharedDir, guestSharedDir string) (
 	}
 
 	if err := c.storeMounts(); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	return sharedDirMounts, nil
+	return sharedDirMounts, ignoredMounts, nil
 }
 
 func (c *Container) unmountHostMounts() error {

virtcontainers/hyperstart_agent.go

@@ -549,7 +549,7 @@ func (h *hyper) startOneContainer(sandbox *Sandbox, c *Container) error {
 	//TODO : Enter mount namespace
 
 	// Handle container mounts
-	newMounts, err := c.mountSharedDirMounts(defaultSharedDir, "")
+	newMounts, _, err := c.mountSharedDirMounts(defaultSharedDir, "")
 	if err != nil {
 		bindUnmountAllRootfs(c.ctx, defaultSharedDir, sandbox)
 		return err

virtcontainers/kata_agent.go

@@ -719,6 +719,30 @@ func (k *kataAgent) replaceOCIMountSource(spec *specs.Spec, guestMounts []Mount)
 	return nil
 }
 
+func (k *kataAgent) removeIgnoredOCIMount(spec *specs.Spec, ignoredMounts []Mount) error {
+	var mounts []specs.Mount
+
+	for _, m := range spec.Mounts {
+		found := false
+		for _, ignoredMount := range ignoredMounts {
+			if ignoredMount.Source == m.Source {
+				k.Logger().WithField("removed-mount", m.Source).Debug("Removing OCI mount")
+				found = true
+				break
+			}
+		}
+
+		if !found {
+			mounts = append(mounts, m)
+		}
+	}
+
+	// Replace the OCI mounts with the updated list.
+	spec.Mounts = mounts
+
+	return nil
+}
+
 func (k *kataAgent) replaceOCIMountsForStorages(spec *specs.Spec, volumeStorages []*grpc.Storage) error {
 	ociMounts := spec.Mounts
 	var index int
@@ -981,7 +1005,7 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
 	}
 
 	// Handle container mounts
-	newMounts, err := c.mountSharedDirMounts(kataHostSharedDir, kataGuestSharedDir)
+	newMounts, ignoredMounts, err := c.mountSharedDirMounts(kataHostSharedDir, kataGuestSharedDir)
 	if err != nil {
 		return nil, err
 	}
@@ -995,6 +1019,11 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
 		return nil, err
 	}
 
+	// Remove all mounts that should be ignored from the spec
+	if err = k.removeIgnoredOCIMount(ociSpec, ignoredMounts); err != nil {
+		return nil, err
+	}
+
 	// Append container devices for block devices passed with --device.
 	ctrDevices = k.appendDevices(ctrDevices, c)
 
@@ -1769,6 +1798,12 @@ func (k *kataAgent) copyFile(src, dst string) error {
 		Gid:      int32(st.Gid),
 	}
 
+	// Handle the special case where the file is empty
+	if fileSize == 0 {
+		_, err = k.sendReq(cpReq)
+		return err
+	}
+
 	// Copy file by parts if it's needed
 	remainingBytes := fileSize
 	offset := int64(0)

virtcontainers/sandbox_test.go

@@ -1706,10 +1706,12 @@ func TestPreAddDevice(t *testing.T) {
 		},
 	}
 
-	mounts, err := container.mountSharedDirMounts("", "")
+	mounts, ignoreMounts, err := container.mountSharedDirMounts("", "")
 	assert.Nil(t, err)
 	assert.Equal(t, len(mounts), 0,
 		"mounts should contain nothing because it only contains a block device")
+	assert.Equal(t, len(ignoreMounts), 0,
+		"ignoreMounts should contain nothing because it only contains a block device")
 }
 
 func TestGetNetNs(t *testing.T) {