From 5611db53094cad0f15222368702c2dc98c7ee780 Mon Sep 17 00:00:00 2001 From: Zhang Tianyang Date: Sat, 13 Jan 2024 17:44:52 +0800 Subject: [PATCH] cri: Make sure host sandbox files exist before adding them to mounts As `setupSandboxFiles` was done in sandbox controller, it is difficult here to know if the sandbox controller has done and where the host path in. Make sure the host path exists before adding them to linux container mounts, otherwise, the container would generate some unnecessary mounts. Signed-off-by: Zhang Tianyang --- internal/cri/server/container_create.go | 80 ++++++++++++-------- internal/cri/server/container_create_test.go | 35 +++------ 2 files changed, 59 insertions(+), 56 deletions(-) diff --git a/internal/cri/server/container_create.go b/internal/cri/server/container_create.go index e1d8c6dd7..6f3893ae3 100644 --- a/internal/cri/server/container_create.go +++ b/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{ - ContainerPath: etcHosts, - HostPath: c.getSandboxHosts(sandboxID), - Readonly: securityContext.GetReadonlyRootfs(), - SelinuxRelabel: true, - UidMappings: uidMappings, - GidMappings: gidMappings, - }) + hostpath := c.getSandboxHosts(sandboxID) + // /etc/hosts could be delegated to remote sandbox controller. That file isn't required to be existed + // in host side for some sandbox runtimes. Skip it if we don't need it. + if _, err := c.os.Stat(hostpath); err == nil { + mounts = append(mounts, &runtime.Mount{ + ContainerPath: etcHosts, + 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{ - ContainerPath: resolvConfPath, - HostPath: c.getResolvPath(sandboxID), - Readonly: securityContext.GetReadonlyRootfs(), - SelinuxRelabel: true, - UidMappings: uidMappings, - GidMappings: gidMappings, - }) + hostpath := c.getResolvPath(sandboxID) + // The ownership of /etc/resolv.conf could be delegated to remote sandbox controller. That file isn't + // required to be existed in host side for some sandbox runtimes. Skip it if we don't need it. + if _, err := c.os.Stat(hostpath); err == nil { + mounts = append(mounts, &runtime.Mount{ + ContainerPath: resolvConfPath, + 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{ - ContainerPath: devShm, - HostPath: sandboxDevShm, - Readonly: false, - SelinuxRelabel: sandboxDevShm != devShm, - // XXX: tmpfs support for idmap mounts got merged in - // Linux 6.3. - // Our Ubuntu 22.04 CI runs with 5.15 kernels, so - // disabling idmap mounts for this case makes the CI - // happy (the other fs used support idmap mounts in 5.15 - // kernels). - // We can enable this at a later stage, but as this - // 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. - }) + // The ownership of /dev/shm could be delegated to remote sandbox controller. That file isn't required + // to be existed in host side for some sandbox runtimes. Skip it if we don't need it. + if _, err := c.os.Stat(sandboxDevShm); err == nil { + mounts = append(mounts, &runtime.Mount{ + ContainerPath: devShm, + HostPath: sandboxDevShm, + Readonly: false, + SelinuxRelabel: sandboxDevShm != devShm, + // XXX: tmpfs support for idmap mounts got merged in + // Linux 6.3. + // Our Ubuntu 22.04 CI runs with 5.15 kernels, so + // disabling idmap mounts for this case makes the CI + // happy (the other fs used support idmap mounts in 5.15 + // kernels). + // We can enable this at a later stage, but as this + // 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 } diff --git a/internal/cri/server/container_create_test.go b/internal/cri/server/container_create_test.go index b2b2c9678..8ae357905 100644 --- a/internal/cri/server/container_create_test.go +++ b/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) - return nil, errors.New("random error") + sandboxRootDir := filepath.Join(testRootDir, sandboxesDir, testSandboxID) + 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{ - { - 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, - }, - }, + expectedMounts: nil, }, } { test := test