From d8ec97ebafe93fdcf5c52d1b15f4ef1b9df064ec Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Dec 2025 15:45:27 +0100 Subject: [PATCH 1/7] use consistent aliases and re-group imports Signed-off-by: Sebastiaan van Stijn --- container/docker/factory.go | 11 +++++------ container/docker/handler.go | 12 ++++++------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/container/docker/factory.go b/container/docker/factory.go index 7e6a5cdbcc..29d7a593c0 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -25,9 +25,12 @@ import ( "github.com/blang/semver/v4" dockersystem "github.com/docker/docker/api/types/system" - "github.com/google/cadvisor/container/containerd" + dclient "github.com/docker/docker/client" + "golang.org/x/net/context" + "k8s.io/klog/v2" "github.com/google/cadvisor/container" + "github.com/google/cadvisor/container/containerd" dockerutil "github.com/google/cadvisor/container/docker/utils" "github.com/google/cadvisor/container/libcontainer" "github.com/google/cadvisor/devicemapper" @@ -36,10 +39,6 @@ import ( "github.com/google/cadvisor/machine" "github.com/google/cadvisor/watcher" "github.com/google/cadvisor/zfs" - - docker "github.com/docker/docker/client" - "golang.org/x/net/context" - "k8s.io/klog/v2" ) var ArgDockerEndpoint = flag.String("docker", "unix:///var/run/docker.sock", "docker endpoint") @@ -110,7 +109,7 @@ type dockerFactory struct { storageDriver StorageDriver storageDir string - client *docker.Client + client *dclient.Client containerdClient containerd.ContainerdClient // Information about the mounted cgroup subsystems. diff --git a/container/docker/handler.go b/container/docker/handler.go index b9936de3c1..052c29bb45 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -24,6 +24,11 @@ import ( "strings" "time" + dclient "github.com/docker/docker/client" + "github.com/opencontainers/cgroups" + "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/net/context" + "github.com/google/cadvisor/container" "github.com/google/cadvisor/container/common" "github.com/google/cadvisor/container/containerd" @@ -34,11 +39,6 @@ import ( "github.com/google/cadvisor/fs" info "github.com/google/cadvisor/info/v1" "github.com/google/cadvisor/zfs" - "github.com/opencontainers/cgroups" - "github.com/opencontainers/runtime-spec/specs-go" - - docker "github.com/docker/docker/client" - "golang.org/x/net/context" ) const ( @@ -118,7 +118,7 @@ func getRwLayerID(containerID, storageDir string, sd StorageDriver, dockerVersio // newDockerContainerHandler returns a new container.ContainerHandler func newDockerContainerHandler( - client *docker.Client, + client *dclient.Client, containerdClient containerd.ContainerdClient, name string, machineInfoFactory info.MachineInfoFactory, From 86dae15b4765e6fa235424ebc34b724c4c1b2a25 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Dec 2025 16:40:29 +0100 Subject: [PATCH 2/7] container/docker: GetSpec(): fix unhandled errors Signed-off-by: Sebastiaan van Stijn --- container/docker/handler.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/container/docker/handler.go b/container/docker/handler.go index 052c29bb45..ecac9cff04 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -315,13 +315,16 @@ func (h *dockerContainerHandler) GetSpec() (info.ContainerSpec, error) { hasFilesystem := h.includedMetrics.Has(container.DiskUsageMetrics) hasNetwork := h.includedMetrics.Has(container.NetworkUsageMetrics) spec, err := common.GetSpec(h.cgroupPaths, h.machineInfoFactory, hasNetwork, hasFilesystem) + if err != nil { + return info.ContainerSpec{}, err + } spec.Labels = h.labels spec.Envs = h.envs spec.Image = h.image spec.CreationTime = h.creationTime - return spec, err + return spec, nil } func (h *dockerContainerHandler) GetStats() (*info.ContainerStats, error) { From e274e11261b66448c5086317cb81646cf964bd27 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Dec 2025 15:50:38 +0100 Subject: [PATCH 3/7] container/(docker|podman) use more generic names use more generic names to start aligning the implementations for easier maintenance. Signed-off-by: Sebastiaan van Stijn --- container/docker/factory.go | 2 +- container/docker/handler.go | 34 ++++++++-------- container/podman/factory.go | 2 +- container/podman/handler.go | 80 ++++++++++++++++++------------------- 4 files changed, 59 insertions(+), 59 deletions(-) diff --git a/container/docker/factory.go b/container/docker/factory.go index 29d7a593c0..37254540a0 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -147,7 +147,7 @@ func (f *dockerFactory) NewContainerHandler(name string, metadataEnvAllowList [] dockerMetadataEnvAllowList = metadataEnvAllowList } - handler, err = newDockerContainerHandler( + handler, err = newContainerHandler( client, f.containerdClient, name, diff --git a/container/docker/handler.go b/container/docker/handler.go index ecac9cff04..3f7a57c7b1 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -51,7 +51,7 @@ const ( pathToContainersDir = "containers" ) -type dockerContainerHandler struct { +type containerHandler struct { // machineInfoFactory provides info.MachineInfo machineInfoFactory info.MachineInfoFactory @@ -97,7 +97,7 @@ type dockerContainerHandler struct { client docker.APIClient } -var _ container.ContainerHandler = &dockerContainerHandler{} +var _ container.ContainerHandler = &containerHandler{} func getRwLayerID(containerID, storageDir string, sd StorageDriver, dockerVersion []int) (string, error) { const ( @@ -116,8 +116,8 @@ func getRwLayerID(containerID, storageDir string, sd StorageDriver, dockerVersio return string(bytes), err } -// newDockerContainerHandler returns a new container.ContainerHandler -func newDockerContainerHandler( +// newContainerHandler returns a new container.ContainerHandler +func newContainerHandler( client *dclient.Client, containerdClient containerd.ContainerdClient, name string, @@ -192,7 +192,7 @@ func newDockerContainerHandler( metrics := common.RemoveNetMetrics(includedMetrics, ctnr.HostConfig.NetworkMode.IsContainer()) // TODO: extract object mother method - handler := &dockerContainerHandler{ + handler := &containerHandler{ machineInfoFactory: machineInfoFactory, cgroupPaths: cgroupPaths, fsInfo: fsInfo, @@ -295,23 +295,23 @@ func DetermineDeviceStorage(storageDriver StorageDriver, storageDir string, rwLa return } -func (h *dockerContainerHandler) Start() { +func (h *containerHandler) Start() { if h.fsHandler != nil { h.fsHandler.Start() } } -func (h *dockerContainerHandler) Cleanup() { +func (h *containerHandler) Cleanup() { if h.fsHandler != nil { h.fsHandler.Stop() } } -func (h *dockerContainerHandler) ContainerReference() (info.ContainerReference, error) { +func (h *containerHandler) ContainerReference() (info.ContainerReference, error) { return h.reference, nil } -func (h *dockerContainerHandler) GetSpec() (info.ContainerSpec, error) { +func (h *containerHandler) GetSpec() (info.ContainerSpec, error) { hasFilesystem := h.includedMetrics.Has(container.DiskUsageMetrics) hasNetwork := h.includedMetrics.Has(container.NetworkUsageMetrics) spec, err := common.GetSpec(h.cgroupPaths, h.machineInfoFactory, hasNetwork, hasFilesystem) @@ -327,7 +327,7 @@ func (h *dockerContainerHandler) GetSpec() (info.ContainerSpec, error) { return spec, nil } -func (h *dockerContainerHandler) GetStats() (*info.ContainerStats, error) { +func (h *containerHandler) GetStats() (*info.ContainerStats, error) { // TODO(vmarmol): Get from libcontainer API instead of cgroup manager when we don't have to support older Dockers. stats, err := h.libcontainerHandler.GetStats() if err != nil { @@ -354,12 +354,12 @@ func (h *dockerContainerHandler) GetStats() (*info.ContainerStats, error) { return stats, nil } -func (h *dockerContainerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { +func (h *containerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { // No-op for Docker driver. return []info.ContainerReference{}, nil } -func (h *dockerContainerHandler) GetCgroupPath(resource string) (string, error) { +func (h *containerHandler) GetCgroupPath(resource string) (string, error) { var res string if !cgroups.IsCgroup2UnifiedMode() { res = resource @@ -371,22 +371,22 @@ func (h *dockerContainerHandler) GetCgroupPath(resource string) (string, error) return path, nil } -func (h *dockerContainerHandler) GetContainerLabels() map[string]string { +func (h *containerHandler) GetContainerLabels() map[string]string { return h.labels } -func (h *dockerContainerHandler) GetContainerIPAddress() string { +func (h *containerHandler) GetContainerIPAddress() string { return h.ipAddress } -func (h *dockerContainerHandler) ListProcesses(listType container.ListType) ([]int, error) { +func (h *containerHandler) ListProcesses(listType container.ListType) ([]int, error) { return h.libcontainerHandler.GetProcesses() } -func (h *dockerContainerHandler) Exists() bool { +func (h *containerHandler) Exists() bool { return common.CgroupExists(h.cgroupPaths) } -func (h *dockerContainerHandler) Type() container.ContainerType { +func (h *containerHandler) Type() container.ContainerType { return container.ContainerTypeDocker } diff --git a/container/podman/factory.go b/container/podman/factory.go index 007148676b..b02abae919 100644 --- a/container/podman/factory.go +++ b/container/podman/factory.go @@ -109,7 +109,7 @@ func (f *podmanFactory) String() string { } func (f *podmanFactory) NewContainerHandler(name string, metadataEnvAllowList []string, inHostNamespace bool) (handler container.ContainerHandler, err error) { - return newPodmanContainerHandler(name, f.machineInfoFactory, f.fsInfo, + return newContainerHandler(name, f.machineInfoFactory, f.fsInfo, f.storageDriver, f.storageDir, f.cgroupSubsystem, inHostNamespace, metadataEnvAllowList, f.metrics, f.thinPoolName, f.thinPoolWatcher, f.zfsWatcher) } diff --git a/container/podman/handler.go b/container/podman/handler.go index 41133969aa..f22f4cc383 100644 --- a/container/podman/handler.go +++ b/container/podman/handler.go @@ -35,7 +35,7 @@ import ( "github.com/google/cadvisor/zfs" ) -type podmanContainerHandler struct { +type containerHandler struct { // machineInfoFactory provides info.MachineInfo machineInfoFactory info.MachineInfoFactory @@ -72,7 +72,7 @@ type podmanContainerHandler struct { libcontainerHandler *containerlibcontainer.Handler } -func newPodmanContainerHandler( +func newContainerHandler( name string, machineInfoFactory info.MachineInfoFactory, fsInfo fs.FsInfo, @@ -125,7 +125,7 @@ func newPodmanContainerHandler( otherStorageDir := filepath.Join(storageDir, string(storageDriver)+"-containers", id) - handler := &podmanContainerHandler{ + handler := &containerHandler{ machineInfoFactory: machineInfoFactory, cgroupPaths: cgroupPaths, storageDriver: storageDriver, @@ -212,46 +212,46 @@ func determineDeviceStorage(storageDriver docker.StorageDriver, storageDir strin } } -func (p podmanContainerHandler) ContainerReference() (info.ContainerReference, error) { - return p.reference, nil +func (h containerHandler) ContainerReference() (info.ContainerReference, error) { + return h.reference, nil } -func (p podmanContainerHandler) needNet() bool { - if p.metrics.Has(container.NetworkUsageMetrics) { - p.networkMode.IsContainer() - return !p.networkMode.IsContainer() +func (h containerHandler) needNet() bool { + if h.metrics.Has(container.NetworkUsageMetrics) { + h.networkMode.IsContainer() + return !h.networkMode.IsContainer() } return false } -func (p podmanContainerHandler) GetSpec() (info.ContainerSpec, error) { - hasFilesystem := p.metrics.Has(container.DiskUsageMetrics) +func (h containerHandler) GetSpec() (info.ContainerSpec, error) { + hasFilesystem := h.metrics.Has(container.DiskUsageMetrics) - spec, err := common.GetSpec(p.cgroupPaths, p.machineInfoFactory, p.needNet(), hasFilesystem) + spec, err := common.GetSpec(h.cgroupPaths, h.machineInfoFactory, h.needNet(), hasFilesystem) if err != nil { return info.ContainerSpec{}, err } - spec.Labels = p.labels - spec.Envs = p.envs - spec.Image = p.image - spec.CreationTime = p.creationTime + spec.Labels = h.labels + spec.Envs = h.envs + spec.Image = h.image + spec.CreationTime = h.creationTime return spec, nil } -func (p podmanContainerHandler) GetStats() (*info.ContainerStats, error) { - stats, err := p.libcontainerHandler.GetStats() +func (h containerHandler) GetStats() (*info.ContainerStats, error) { + stats, err := h.libcontainerHandler.GetStats() if err != nil { return stats, err } - if !p.needNet() { + if !h.needNet() { stats.Network = info.NetworkStats{} } - err = docker.FsStats(stats, p.machineInfoFactory, p.metrics, p.storageDriver, - p.fsHandler, p.fsInfo, p.thinPoolName, p.rootfsStorageDir, p.zfsParent) + err = docker.FsStats(stats, h.machineInfoFactory, h.metrics, h.storageDriver, + h.fsHandler, h.fsInfo, h.thinPoolName, h.rootfsStorageDir, h.zfsParent) if err != nil { return stats, err } @@ -259,51 +259,51 @@ func (p podmanContainerHandler) GetStats() (*info.ContainerStats, error) { return stats, nil } -func (p podmanContainerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { +func (h containerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { return []info.ContainerReference{}, nil } -func (p podmanContainerHandler) ListProcesses(listType container.ListType) ([]int, error) { - return p.libcontainerHandler.GetProcesses() +func (h containerHandler) ListProcesses(listType container.ListType) ([]int, error) { + return h.libcontainerHandler.GetProcesses() } -func (p podmanContainerHandler) GetCgroupPath(resource string) (string, error) { +func (h containerHandler) GetCgroupPath(resource string) (string, error) { var res string if !cgroups.IsCgroup2UnifiedMode() { res = resource } - path, ok := p.cgroupPaths[res] + path, ok := h.cgroupPaths[res] if !ok { - return "", fmt.Errorf("couldn't find path for resource %q for container %q", resource, p.reference.Name) + return "", fmt.Errorf("couldn't find path for resource %q for container %q", resource, h.reference.Name) } return path, nil } -func (p podmanContainerHandler) GetContainerLabels() map[string]string { - return p.labels +func (h containerHandler) GetContainerLabels() map[string]string { + return h.labels } -func (p podmanContainerHandler) GetContainerIPAddress() string { - return p.ipAddress +func (h containerHandler) GetContainerIPAddress() string { + return h.ipAddress } -func (p podmanContainerHandler) Exists() bool { - return common.CgroupExists(p.cgroupPaths) +func (h containerHandler) Exists() bool { + return common.CgroupExists(h.cgroupPaths) } -func (p podmanContainerHandler) Cleanup() { - if p.fsHandler != nil { - p.fsHandler.Stop() +func (h containerHandler) Cleanup() { + if h.fsHandler != nil { + h.fsHandler.Stop() } } -func (p podmanContainerHandler) Start() { - if p.fsHandler != nil { - p.fsHandler.Start() +func (h containerHandler) Start() { + if h.fsHandler != nil { + h.fsHandler.Start() } } -func (p podmanContainerHandler) Type() container.ContainerType { +func (h containerHandler) Type() container.ContainerType { return container.ContainerTypePodman } From f36f455717de941e577cfcbdc8808ee9a5626ec1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Dec 2025 15:57:57 +0100 Subject: [PATCH 4/7] container/podman: containerhandler: use pointer-receiver It's more idiomatic, and aligns with the docker implementation for the same. Signed-off-by: Sebastiaan van Stijn --- container/podman/handler.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/container/podman/handler.go b/container/podman/handler.go index f22f4cc383..8e074e61d4 100644 --- a/container/podman/handler.go +++ b/container/podman/handler.go @@ -212,11 +212,11 @@ func determineDeviceStorage(storageDriver docker.StorageDriver, storageDir strin } } -func (h containerHandler) ContainerReference() (info.ContainerReference, error) { +func (h *containerHandler) ContainerReference() (info.ContainerReference, error) { return h.reference, nil } -func (h containerHandler) needNet() bool { +func (h *containerHandler) needNet() bool { if h.metrics.Has(container.NetworkUsageMetrics) { h.networkMode.IsContainer() return !h.networkMode.IsContainer() @@ -224,7 +224,7 @@ func (h containerHandler) needNet() bool { return false } -func (h containerHandler) GetSpec() (info.ContainerSpec, error) { +func (h *containerHandler) GetSpec() (info.ContainerSpec, error) { hasFilesystem := h.metrics.Has(container.DiskUsageMetrics) spec, err := common.GetSpec(h.cgroupPaths, h.machineInfoFactory, h.needNet(), hasFilesystem) @@ -240,7 +240,7 @@ func (h containerHandler) GetSpec() (info.ContainerSpec, error) { return spec, nil } -func (h containerHandler) GetStats() (*info.ContainerStats, error) { +func (h *containerHandler) GetStats() (*info.ContainerStats, error) { stats, err := h.libcontainerHandler.GetStats() if err != nil { return stats, err @@ -259,15 +259,15 @@ func (h containerHandler) GetStats() (*info.ContainerStats, error) { return stats, nil } -func (h containerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { +func (h *containerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { return []info.ContainerReference{}, nil } -func (h containerHandler) ListProcesses(listType container.ListType) ([]int, error) { +func (h *containerHandler) ListProcesses(listType container.ListType) ([]int, error) { return h.libcontainerHandler.GetProcesses() } -func (h containerHandler) GetCgroupPath(resource string) (string, error) { +func (h *containerHandler) GetCgroupPath(resource string) (string, error) { var res string if !cgroups.IsCgroup2UnifiedMode() { res = resource @@ -280,30 +280,30 @@ func (h containerHandler) GetCgroupPath(resource string) (string, error) { return path, nil } -func (h containerHandler) GetContainerLabels() map[string]string { +func (h *containerHandler) GetContainerLabels() map[string]string { return h.labels } -func (h containerHandler) GetContainerIPAddress() string { +func (h *containerHandler) GetContainerIPAddress() string { return h.ipAddress } -func (h containerHandler) Exists() bool { +func (h *containerHandler) Exists() bool { return common.CgroupExists(h.cgroupPaths) } -func (h containerHandler) Cleanup() { +func (h *containerHandler) Cleanup() { if h.fsHandler != nil { h.fsHandler.Stop() } } -func (h containerHandler) Start() { +func (h *containerHandler) Start() { if h.fsHandler != nil { h.fsHandler.Start() } } -func (h containerHandler) Type() container.ContainerType { +func (h *containerHandler) Type() container.ContainerType { return container.ContainerTypePodman } From a0cc8da47db625852b4c84ca264d0a71b788788b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Dec 2025 15:59:14 +0100 Subject: [PATCH 5/7] container/(docker|podman): rename var that shadowed import Signed-off-by: Sebastiaan van Stijn --- container/docker/handler.go | 4 ++-- container/podman/handler.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/container/docker/handler.go b/container/docker/handler.go index 3f7a57c7b1..85c6d8809a 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -364,11 +364,11 @@ func (h *containerHandler) GetCgroupPath(resource string) (string, error) { if !cgroups.IsCgroup2UnifiedMode() { res = resource } - path, ok := h.cgroupPaths[res] + cgroupPath, ok := h.cgroupPaths[res] if !ok { return "", fmt.Errorf("could not find path for resource %q for container %q", resource, h.reference.Name) } - return path, nil + return cgroupPath, nil } func (h *containerHandler) GetContainerLabels() map[string]string { diff --git a/container/podman/handler.go b/container/podman/handler.go index 8e074e61d4..b979757900 100644 --- a/container/podman/handler.go +++ b/container/podman/handler.go @@ -113,12 +113,12 @@ func newContainerHandler( return nil, err } - rwLayerID, err := rwLayerID(storageDriver, storageDir, id) + layerID, err := rwLayerID(storageDriver, storageDir, id) if err != nil { return nil, err } - rootfsStorageDir, zfsFilesystem, zfsParent, err := determineDeviceStorage(storageDriver, storageDir, rwLayerID) + rootfsStorageDir, zfsFilesystem, zfsParent, err := determineDeviceStorage(storageDriver, storageDir, layerID) if err != nil { return nil, err } @@ -163,12 +163,12 @@ func newContainerHandler( // This happens in cases such as kubernetes where the containers doesn't have an IP address itself and we need to use the pod's address networkMode := string(handler.networkMode) if handler.ipAddress == "" && strings.HasPrefix(networkMode, "container:") { - id := strings.TrimPrefix(networkMode, "container:") - ctnr, err := InspectContainer(id) + containerID := strings.TrimPrefix(networkMode, "container:") + c, err := InspectContainer(containerID) if err != nil { return nil, err } - handler.ipAddress = ctnr.NetworkSettings.IPAddress + handler.ipAddress = c.NetworkSettings.IPAddress } if metrics.Has(container.DiskUsageMetrics) { @@ -272,12 +272,12 @@ func (h *containerHandler) GetCgroupPath(resource string) (string, error) { if !cgroups.IsCgroup2UnifiedMode() { res = resource } - path, ok := h.cgroupPaths[res] + cgroupPath, ok := h.cgroupPaths[res] if !ok { return "", fmt.Errorf("couldn't find path for resource %q for container %q", resource, h.reference.Name) } - return path, nil + return cgroupPath, nil } func (h *containerHandler) GetContainerLabels() map[string]string { From 3314af1eb0be6b591dbab0ed13a74fb2738b34df Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Dec 2025 16:19:45 +0100 Subject: [PATCH 6/7] container/(docker|podman): align code Align the code between the implementations for easier comparison; mostly renaming some vars, changing order, and syncing comments. Signed-off-by: Sebastiaan van Stijn --- container/docker/handler.go | 68 +++++++++++++++++-------------------- container/podman/handler.go | 33 +++++++++++++----- 2 files changed, 56 insertions(+), 45 deletions(-) diff --git a/container/docker/handler.go b/container/docker/handler.go index 85c6d8809a..350018547e 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Handler for Docker containers. +// Package docker implements a handler for Docker containers. package docker import ( @@ -80,10 +80,10 @@ type containerHandler struct { // The IP address of the container ipAddress string - includedMetrics container.MetricSet + metrics container.MetricSet // the devicemapper poolname - poolName string + thinPoolName string // zfsParent is the parent for docker zfs zfsParent string @@ -129,7 +129,7 @@ func newContainerHandler( inHostNamespace bool, metadataEnvAllowList []string, dockerVersion []int, - includedMetrics container.MetricSet, + metrics container.MetricSet, thinPoolName string, thinPoolWatcher *devicemapper.ThinPoolWatcher, zfsWatcher *zfs.ZfsWatcher, @@ -189,26 +189,24 @@ func newContainerHandler( } // Do not report network metrics for containers that share netns with another container. - metrics := common.RemoveNetMetrics(includedMetrics, ctnr.HostConfig.NetworkMode.IsContainer()) + includedMetrics := common.RemoveNetMetrics(metrics, ctnr.HostConfig.NetworkMode.IsContainer()) - // TODO: extract object mother method handler := &containerHandler{ machineInfoFactory: machineInfoFactory, cgroupPaths: cgroupPaths, - fsInfo: fsInfo, storageDriver: storageDriver, - poolName: thinPoolName, + fsInfo: fsInfo, rootfsStorageDir: rootfsStorageDir, envs: make(map[string]string), labels: ctnr.Config.Labels, - includedMetrics: metrics, + metrics: includedMetrics, + thinPoolName: thinPoolName, zfsParent: zfsParent, client: client, } // Timestamp returned by Docker is in time.RFC3339Nano format. handler.creationTime, err = time.Parse(time.RFC3339Nano, ctnr.Created) if err != nil { - // This should not happen, report the error just in case return nil, fmt.Errorf("failed to parse the create timestamp %q for container %q: %v", ctnr.Created, id, err) } handler.libcontainerHandler = containerlibcontainer.NewHandler(cgroupManager, rootFs, ctnr.State.Pid, metrics) @@ -221,7 +219,7 @@ func newContainerHandler( Namespace: DockerNamespace, } handler.image = ctnr.Config.Image - // Only adds restartcount label if it's greater than 0 + if ctnr.RestartCount > 0 { handler.labels["restartcount"] = strconv.Itoa(ctnr.RestartCount) } @@ -235,11 +233,10 @@ func newContainerHandler( containerID := strings.TrimPrefix(networkMode, "container:") c, err := client.ContainerInspect(context.Background(), containerID) if err != nil { - return nil, fmt.Errorf("failed to inspect container %q: %v", id, err) + return nil, fmt.Errorf("failed to inspect container %q: %v", containerID, err) } ipAddress = c.NetworkSettings.IPAddress } - handler.ipAddress = ipAddress if includedMetrics.Has(container.DiskUsageMetrics) { @@ -252,7 +249,7 @@ func newContainerHandler( } } - // split env vars to get metadata map. + // Split env vars to get metadata map. for _, exposedEnv := range metadataEnvAllowList { if exposedEnv == "" { // if no dockerEnvWhitelist provided, len(metadataEnvAllowList) == 1, metadataEnvAllowList[0] == "" @@ -295,25 +292,13 @@ func DetermineDeviceStorage(storageDriver StorageDriver, storageDir string, rwLa return } -func (h *containerHandler) Start() { - if h.fsHandler != nil { - h.fsHandler.Start() - } -} - -func (h *containerHandler) Cleanup() { - if h.fsHandler != nil { - h.fsHandler.Stop() - } -} - func (h *containerHandler) ContainerReference() (info.ContainerReference, error) { return h.reference, nil } func (h *containerHandler) GetSpec() (info.ContainerSpec, error) { - hasFilesystem := h.includedMetrics.Has(container.DiskUsageMetrics) - hasNetwork := h.includedMetrics.Has(container.NetworkUsageMetrics) + hasFilesystem := h.metrics.Has(container.DiskUsageMetrics) + hasNetwork := h.metrics.Has(container.NetworkUsageMetrics) spec, err := common.GetSpec(h.cgroupPaths, h.machineInfoFactory, hasNetwork, hasFilesystem) if err != nil { return info.ContainerSpec{}, err @@ -345,8 +330,8 @@ func (h *containerHandler) GetStats() (*info.ContainerStats, error) { } // Get filesystem stats. - err = FsStats(stats, h.machineInfoFactory, h.includedMetrics, h.storageDriver, - h.fsHandler, h.fsInfo, h.poolName, h.rootfsStorageDir, h.zfsParent) + err = FsStats(stats, h.machineInfoFactory, h.metrics, h.storageDriver, + h.fsHandler, h.fsInfo, h.thinPoolName, h.rootfsStorageDir, h.zfsParent) if err != nil { return stats, err } @@ -354,11 +339,14 @@ func (h *containerHandler) GetStats() (*info.ContainerStats, error) { return stats, nil } -func (h *containerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { - // No-op for Docker driver. +func (h *containerHandler) ListContainers(container.ListType) ([]info.ContainerReference, error) { return []info.ContainerReference{}, nil } +func (h *containerHandler) ListProcesses(container.ListType) ([]int, error) { + return h.libcontainerHandler.GetProcesses() +} + func (h *containerHandler) GetCgroupPath(resource string) (string, error) { var res string if !cgroups.IsCgroup2UnifiedMode() { @@ -379,14 +367,22 @@ func (h *containerHandler) GetContainerIPAddress() string { return h.ipAddress } -func (h *containerHandler) ListProcesses(listType container.ListType) ([]int, error) { - return h.libcontainerHandler.GetProcesses() -} - func (h *containerHandler) Exists() bool { return common.CgroupExists(h.cgroupPaths) } +func (h *containerHandler) Cleanup() { + if h.fsHandler != nil { + h.fsHandler.Stop() + } +} + +func (h *containerHandler) Start() { + if h.fsHandler != nil { + h.fsHandler.Start() + } +} + func (h *containerHandler) Type() container.ContainerType { return container.ContainerTypeDocker } diff --git a/container/podman/handler.go b/container/podman/handler.go index b979757900..617ff06ee2 100644 --- a/container/podman/handler.go +++ b/container/podman/handler.go @@ -12,12 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Package podman implements a handler for Podman containers. package podman import ( "fmt" "path" "path/filepath" + "strconv" "strings" "time" @@ -43,30 +45,38 @@ type containerHandler struct { // (e.g.: "cpu" -> "/sys/fs/cgroup/cpu/test") cgroupPaths map[string]string + // the docker storage driver storageDriver docker.StorageDriver fsInfo fs.FsInfo rootfsStorageDir string + // Time at which this container was created. creationTime time.Time // Metadata associated with the container. envs map[string]string labels map[string]string + // Image name used for this container. image string networkMode dockercontainer.NetworkMode + // Filesystem handler. fsHandler common.FsHandler + // The IP address of the container ipAddress string metrics container.MetricSet + // the devicemapper poolname thinPoolName string + // zfsParent is the parent for docker zfs zfsParent string + // Reference to the container reference info.ContainerReference libcontainerHandler *containerlibcontainer.Handler @@ -89,6 +99,7 @@ func newContainerHandler( // Create the cgroup paths. cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems, name) + // Generate the equivalent cgroup manager for this container. cgroupManager, err := containerlibcontainer.NewCgroupManager(name, cgroupPaths) if err != nil { return nil, err @@ -118,6 +129,8 @@ func newContainerHandler( return nil, err } + // Determine the rootfs storage dir OR the pool name to determine the device. + // For devicemapper, we only need the thin pool name, and that is passed in to this call rootfsStorageDir, zfsFilesystem, zfsParent, err := determineDeviceStorage(storageDriver, storageDir, layerID) if err != nil { return nil, err @@ -131,7 +144,6 @@ func newContainerHandler( storageDriver: storageDriver, fsInfo: fsInfo, rootfsStorageDir: rootfsStorageDir, - ipAddress: ctnr.NetworkSettings.IPAddress, envs: make(map[string]string), labels: ctnr.Config.Labels, image: ctnr.Config.Image, @@ -155,21 +167,23 @@ func newContainerHandler( } if ctnr.RestartCount > 0 { - handler.labels["restartcount"] = fmt.Sprint(ctnr.RestartCount) + handler.labels["restartcount"] = strconv.Itoa(ctnr.RestartCount) } // Obtain the IP address for the container. // If the NetworkMode starts with 'container:' then we need to use the IP address of the container specified. // This happens in cases such as kubernetes where the containers doesn't have an IP address itself and we need to use the pod's address - networkMode := string(handler.networkMode) - if handler.ipAddress == "" && strings.HasPrefix(networkMode, "container:") { + ipAddress := ctnr.NetworkSettings.IPAddress + networkMode := string(ctnr.HostConfig.NetworkMode) + if ipAddress == "" && strings.HasPrefix(networkMode, "container:") { containerID := strings.TrimPrefix(networkMode, "container:") c, err := InspectContainer(containerID) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to inspect container %q: %v", containerID, err) } - handler.ipAddress = c.NetworkSettings.IPAddress + ipAddress = c.NetworkSettings.IPAddress } + handler.ipAddress = ipAddress if metrics.Has(container.DiskUsageMetrics) { handler.fsHandler = &docker.FsHandler{ @@ -250,6 +264,7 @@ func (h *containerHandler) GetStats() (*info.ContainerStats, error) { stats.Network = info.NetworkStats{} } + // Get filesystem stats. err = docker.FsStats(stats, h.machineInfoFactory, h.metrics, h.storageDriver, h.fsHandler, h.fsInfo, h.thinPoolName, h.rootfsStorageDir, h.zfsParent) if err != nil { @@ -259,11 +274,11 @@ func (h *containerHandler) GetStats() (*info.ContainerStats, error) { return stats, nil } -func (h *containerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { +func (h *containerHandler) ListContainers(container.ListType) ([]info.ContainerReference, error) { return []info.ContainerReference{}, nil } -func (h *containerHandler) ListProcesses(listType container.ListType) ([]int, error) { +func (h *containerHandler) ListProcesses(container.ListType) ([]int, error) { return h.libcontainerHandler.GetProcesses() } @@ -274,7 +289,7 @@ func (h *containerHandler) GetCgroupPath(resource string) (string, error) { } cgroupPath, ok := h.cgroupPaths[res] if !ok { - return "", fmt.Errorf("couldn't find path for resource %q for container %q", resource, h.reference.Name) + return "", fmt.Errorf("could not find path for resource %q for container %q", resource, h.reference.Name) } return cgroupPath, nil From afc60749b10b56a044ea7569f6dfe3d269372348 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 4 Dec 2025 16:54:46 +0100 Subject: [PATCH 7/7] container/docker: inline some vars Signed-off-by: Sebastiaan van Stijn --- container/docker/handler.go | 22 +++++++++++----------- container/podman/handler.go | 1 + 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/container/docker/handler.go b/container/docker/handler.go index 350018547e..8fa422e580 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -94,7 +94,7 @@ type containerHandler struct { libcontainerHandler *containerlibcontainer.Handler // the docker client is needed to inspect the container and get the health status - client docker.APIClient + client dclient.APIClient } var _ container.ContainerHandler = &containerHandler{} @@ -199,26 +199,26 @@ func newContainerHandler( rootfsStorageDir: rootfsStorageDir, envs: make(map[string]string), labels: ctnr.Config.Labels, + image: ctnr.Config.Image, metrics: includedMetrics, thinPoolName: thinPoolName, zfsParent: zfsParent, client: client, + reference: info.ContainerReference{ + // Add the name and bare ID as aliases of the container. + Id: id, + Name: name, + Aliases: []string{strings.TrimPrefix(ctnr.Name, "/"), id}, + Namespace: DockerNamespace, + }, + libcontainerHandler: containerlibcontainer.NewHandler(cgroupManager, rootFs, ctnr.State.Pid, metrics), } + // Timestamp returned by Docker is in time.RFC3339Nano format. handler.creationTime, err = time.Parse(time.RFC3339Nano, ctnr.Created) if err != nil { return nil, fmt.Errorf("failed to parse the create timestamp %q for container %q: %v", ctnr.Created, id, err) } - handler.libcontainerHandler = containerlibcontainer.NewHandler(cgroupManager, rootFs, ctnr.State.Pid, metrics) - - // Add the name and bare ID as aliases of the container. - handler.reference = info.ContainerReference{ - Id: id, - Name: name, - Aliases: []string{strings.TrimPrefix(ctnr.Name, "/"), id}, - Namespace: DockerNamespace, - } - handler.image = ctnr.Config.Image if ctnr.RestartCount > 0 { handler.labels["restartcount"] = strconv.Itoa(ctnr.RestartCount) diff --git a/container/podman/handler.go b/container/podman/handler.go index 617ff06ee2..a93c25d283 100644 --- a/container/podman/handler.go +++ b/container/podman/handler.go @@ -153,6 +153,7 @@ func newContainerHandler( thinPoolName: thinPoolName, zfsParent: zfsParent, reference: info.ContainerReference{ + // Add the name and bare ID as aliases of the container. Id: id, Name: name, Aliases: []string{strings.TrimPrefix(ctnr.Name, "/"), id},