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 <burning9699@gmail.com>
This commit is contained in:
Zhang Tianyang 2024-01-13 17:44:52 +08:00
parent b87d78f456
commit 5611db5309
2 changed files with 59 additions and 56 deletions

View File

@ -1008,27 +1008,37 @@ func (c *criService) linuxContainerMounts(sandboxID string, config *runtime.Cont
} }
if !isInCRIMounts(etcHosts, config.GetMounts()) { 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. // Mount sandbox resolv.config.
// TODO: Need to figure out whether we should always mount it as read-only // TODO: Need to figure out whether we should always mount it as read-only
if !isInCRIMounts(resolvConfPath, config.GetMounts()) { 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()) { 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 { if securityContext.GetNamespaceOptions().GetIpc() == runtime.NamespaceMode_NODE {
sandboxDevShm = devShm 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 return mounts
} }

View File

@ -733,32 +733,21 @@ func TestLinuxContainerMounts(t *testing.T) {
expectedMounts: nil, 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) { 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{}, 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 test := test