Merge pull request #9635 from Burning1020/fix-cri-mounts

cri: Stat host sandbox files before adding them

internal/cri/server/container_create.go

@@ -1008,27 +1008,37 @@ func (c *criService) linuxContainerMounts(sandboxID string, config *runtime.Cont
 	}
 
 	if !isInCRIMounts(etcHosts, config.GetMounts()) {
-		mounts = append(mounts, &runtime.Mount{
+		hostpath := c.getSandboxHosts(sandboxID)
-			ContainerPath:  etcHosts,
+		// /etc/hosts could be delegated to remote sandbox controller. That file isn't required to be existed
-			HostPath:       c.getSandboxHosts(sandboxID),
+		// in host side for some sandbox runtimes. Skip it if we don't need it.
-			Readonly:       securityContext.GetReadonlyRootfs(),
+		if _, err := c.os.Stat(hostpath); err == nil {
-			SelinuxRelabel: true,
+			mounts = append(mounts, &runtime.Mount{
-			UidMappings:    uidMappings,
+				ContainerPath:  etcHosts,
-			GidMappings:    gidMappings,
+				HostPath:       hostpath,
-		})
+				Readonly:       securityContext.GetReadonlyRootfs(),
+				SelinuxRelabel: true,
+				UidMappings:    uidMappings,
+				GidMappings:    gidMappings,
+			})
+		}
 	}
 
 	// Mount sandbox resolv.config.
 	// TODO: Need to figure out whether we should always mount it as read-only
 	if !isInCRIMounts(resolvConfPath, config.GetMounts()) {
-		mounts = append(mounts, &runtime.Mount{
+		hostpath := c.getResolvPath(sandboxID)
-			ContainerPath:  resolvConfPath,
+		// The ownership of /etc/resolv.conf could be delegated to remote sandbox controller. That file isn't
-			HostPath:       c.getResolvPath(sandboxID),
+		// required to be existed in host side for some sandbox runtimes. Skip it if we don't need it.
-			Readonly:       securityContext.GetReadonlyRootfs(),
+		if _, err := c.os.Stat(hostpath); err == nil {
-			SelinuxRelabel: true,
+			mounts = append(mounts, &runtime.Mount{
-			UidMappings:    uidMappings,
+				ContainerPath:  resolvConfPath,
-			GidMappings:    gidMappings,
+				HostPath:       hostpath,
-		})
+				Readonly:       securityContext.GetReadonlyRootfs(),
+				SelinuxRelabel: true,
+				UidMappings:    uidMappings,
+				GidMappings:    gidMappings,
+			})
+		}
 	}
 
 	if !isInCRIMounts(devShm, config.GetMounts()) {
@@ -1036,23 +1046,27 @@ func (c *criService) linuxContainerMounts(sandboxID string, config *runtime.Cont
 		if securityContext.GetNamespaceOptions().GetIpc() == runtime.NamespaceMode_NODE {
 			sandboxDevShm = devShm
 		}
-		mounts = append(mounts, &runtime.Mount{
+		// The ownership of /dev/shm could be delegated to remote sandbox controller. That file isn't required
-			ContainerPath:  devShm,
+		// to be existed in host side for some sandbox runtimes. Skip it if we don't need it.
-			HostPath:       sandboxDevShm,
+		if _, err := c.os.Stat(sandboxDevShm); err == nil {
-			Readonly:       false,
+			mounts = append(mounts, &runtime.Mount{
-			SelinuxRelabel: sandboxDevShm != devShm,
+				ContainerPath:  devShm,
-			// XXX: tmpfs support for idmap mounts got merged in
+				HostPath:       sandboxDevShm,
-			// Linux 6.3.
+				Readonly:       false,
-			// Our Ubuntu 22.04 CI runs with 5.15 kernels, so
+				SelinuxRelabel: sandboxDevShm != devShm,
-			// disabling idmap mounts for this case makes the CI
+				// XXX: tmpfs support for idmap mounts got merged in
-			// happy (the other fs used support idmap mounts in 5.15
+				// Linux 6.3.
-			// kernels).
+				// Our Ubuntu 22.04 CI runs with 5.15 kernels, so
-			// We can enable this at a later stage, but as this
+				// disabling idmap mounts for this case makes the CI
-			// tmpfs mount is exposed empty to the container (no
+				// happy (the other fs used support idmap mounts in 5.15
-			// prepopulated files) and using the hostIPC with userns
+				// kernels).
-			// is blocked by k8s, we can just avoid using the
+				// We can enable this at a later stage, but as this
-			// mappings and it should work fine.
+				// tmpfs mount is exposed empty to the container (no
-		})
+				// prepopulated files) and using the hostIPC with userns
+				// is blocked by k8s, we can just avoid using the
+				// mappings and it should work fine.
+			})
+		}
 	}
 	return mounts
 }

internal/cri/server/container_create_test.go

@@ -733,32 +733,21 @@ func TestLinuxContainerMounts(t *testing.T) {
 			expectedMounts:  nil,
 		},
 		{
-			desc: "should skip hostname mount if the old sandbox doesn't have hostname file",
+			desc: "should skip sandbox mounts if the old sandbox doesn't have sandbox file",
 			statFn: func(path string) (os.FileInfo, error) {
-				assert.Equal(t, filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hostname"), path)
+				sandboxRootDir := filepath.Join(testRootDir, sandboxesDir, testSandboxID)
-				return nil, errors.New("random error")
+				sandboxStateDir := filepath.Join(testStateDir, sandboxesDir, testSandboxID)
+				switch path {
+				case filepath.Join(sandboxRootDir, "hostname"), filepath.Join(sandboxRootDir, "hosts"),
+					filepath.Join(sandboxRootDir, "resolv.conf"), filepath.Join(sandboxStateDir, "shm"):
+					return nil, errors.New("random error")
+				default:
+					t.Fatalf("expected sandbox files, got: %s", path)
+				}
+				return nil, nil
 			},
 			securityContext: &runtime.LinuxContainerSecurityContext{},
-			expectedMounts: []*runtime.Mount{
+			expectedMounts:  nil,
-				{
-					ContainerPath:  "/etc/hosts",
-					HostPath:       filepath.Join(testRootDir, sandboxesDir, testSandboxID, "hosts"),
-					Readonly:       false,
-					SelinuxRelabel: true,
-				},
-				{
-					ContainerPath:  resolvConfPath,
-					HostPath:       filepath.Join(testRootDir, sandboxesDir, testSandboxID, "resolv.conf"),
-					Readonly:       false,
-					SelinuxRelabel: true,
-				},
-				{
-					ContainerPath:  "/dev/shm",
-					HostPath:       filepath.Join(testStateDir, sandboxesDir, testSandboxID, "shm"),
-					Readonly:       false,
-					SelinuxRelabel: true,
-				},
-			},
 		},
 	} {
 		test := test