Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CONTINT-3905] Make sure we retrieve the cgroup inode when the cid is not found #302

Merged
merged 2 commits into from Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 19 additions & 15 deletions statsd/container_linux.go
Expand Up @@ -184,23 +184,27 @@ func inodeForPath(path string) string {

// internalInitContainerID initializes the container ID.
// It can either be provided by the user or read from cgroups.
func internalInitContainerID(userProvidedID string, cgroupFallback bool) {
func internalInitContainerID(userProvidedID string, cgroupFallback, isHostCgroupNs bool) {
initOnce.Do(func() {
if userProvidedID != "" {
containerID = userProvidedID
readCIDOrInode(userProvidedID, cgroupPath, selfMountInfoPath, defaultCgroupMountPath, cgroupFallback, isHostCgroupNs)
})
}

// readCIDOrInode reads the container ID from the user provided ID, cgroups or mountinfo.
func readCIDOrInode(userProvidedID, cgroupPath, selfMountInfoPath, defaultCgroupMountPath string, cgroupFallback, isHostCgroupNs bool) {
if userProvidedID != "" {
containerID = userProvidedID
return
}

if cgroupFallback {
if isHostCgroupNs {
containerID = readContainerID(cgroupPath)
return
}

if cgroupFallback {
isHostCgroupNs := isHostCgroupNamespace()
if isHostCgroupNs {
containerID = readContainerID(cgroupPath)
return
}
containerID = readMountinfo(selfMountInfoPath)
if containerID != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug is just this line. All the rest is about moving stuff around to add unit tests

containerID = getCgroupInode(defaultCgroupMountPath, cgroupPath)
}
containerID = readMountinfo(selfMountInfoPath)
if containerID == "" {
containerID = getCgroupInode(defaultCgroupMountPath, cgroupPath)
}
})
}
}
6 changes: 5 additions & 1 deletion statsd/container_stub.go
Expand Up @@ -3,7 +3,11 @@

package statsd

var initContainerID = func(userProvidedID string, cgroupFallback bool) {
func isHostCgroupNamespace() bool {
return false
}

var initContainerID = func(userProvidedID string, _, _ bool) {
initOnce.Do(func() {
if userProvidedID != "" {
containerID = userProvidedID
Expand Down
71 changes: 71 additions & 0 deletions statsd/container_test.go
Expand Up @@ -400,3 +400,74 @@ func TestGetCgroupInode(t *testing.T) {
})
}
}

func TestReadCIDOrInode(t *testing.T) {
tests := []struct {
description string
procSelfCgroupContent string
mountInfoContent string
isHostCgroupNs bool
expectedResult string
cgroupNodeDir string
}{
{
description: "extract container-id from /proc/self/cgroup",
procSelfCgroupContent: "4:blkio:/kubepods/burstable/podfd52ef25-a87d-11e9-9423-0800271a638e/8c046cb0b72cd4c99f51b5591cd5b095967f58ee003710a45280c28ee1a9c7fa\n",
isHostCgroupNs: true,
expectedResult: "8c046cb0b72cd4c99f51b5591cd5b095967f58ee003710a45280c28ee1a9c7fa", // Will be formatted with inode number
},

{
description: "extract container-id from mountinfo",
mountInfoContent: "2282 2269 8:1 /var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/c0a82a3506b0366c9666f6dbe71c783abeb26ba65e312e918a49e10a277196d0/hostname /host/var/run/containerd/io.containerd.runtime.v2.task/k8s.io/fc7038bc73a8d3850c66ddbfb0b2901afa378bfcbb942cc384b051767e4ac6b0/rootfs/etc/hostname rw,nosuid,nodev,relatime - ext4 /dev/sda1 rw,commit=30\n",
expectedResult: "fc7038bc73a8d3850c66ddbfb0b2901afa378bfcbb942cc384b051767e4ac6b0",
},
{
description: "extract inode",
cgroupNodeDir: "system.slice/docker-abcdef0123456789abcdef0123456789.scope",
procSelfCgroupContent: "0::/system.slice/docker-abcdef0123456789abcdef0123456789.scope\n",
expectedResult: "in-%d",
},
}

for _, tc := range tests {
t.Run(tc.description, func(t *testing.T) {
prevContainerID := containerID
defer func() {
containerID = prevContainerID
}()

sysFsCgroupPath := path.Join(os.TempDir(), "sysfscgroup")
groupControllerPath := path.Join(sysFsCgroupPath, tc.cgroupNodeDir)
err := os.MkdirAll(groupControllerPath, 0755)
require.NoError(t, err)
defer os.RemoveAll(groupControllerPath)

stat, err := os.Stat(groupControllerPath)
require.NoError(t, err)
expectedResult := tc.expectedResult
if strings.HasPrefix(tc.expectedResult, "in-") {
expectedResult = fmt.Sprintf(tc.expectedResult, stat.Sys().(*syscall.Stat_t).Ino)
}

procSelfCgroup, err := ioutil.TempFile("", "procselfcgroup")
require.NoError(t, err)
defer os.Remove(procSelfCgroup.Name())
_, err = procSelfCgroup.WriteString(tc.procSelfCgroupContent)
require.NoError(t, err)
err = procSelfCgroup.Close()
require.NoError(t, err)

mountInfo, err := ioutil.TempFile("", "mountInfo")
require.NoError(t, err)
defer os.Remove(mountInfo.Name())
_, err = mountInfo.WriteString(tc.mountInfoContent)
require.NoError(t, err)
err = mountInfo.Close()
require.NoError(t, err)

readCIDOrInode("", procSelfCgroup.Name(), mountInfo.Name(), sysFsCgroupPath, true, tc.isHostCgroupNs)
require.Equal(t, expectedResult, containerID)
})
}
}
2 changes: 1 addition & 1 deletion statsd/statsd.go
Expand Up @@ -466,7 +466,7 @@ func newWithWriter(w Transport, o *Options, writerName string) (*Client, error)
}

if !hasEntityID {
initContainerID(o.containerID, isOriginDetectionEnabled(o, hasEntityID))
initContainerID(o.containerID, isOriginDetectionEnabled(o, hasEntityID), isHostCgroupNamespace())
}

isUDS := writerName == writerNameUDS
Expand Down