diff --git a/container/docker/factory.go b/container/docker/factory.go index fca1811a66..ba7623cdcb 100644 --- a/container/docker/factory.go +++ b/container/docker/factory.go @@ -355,10 +355,11 @@ func Register(factory info.MachineInfoFactory, fsInfo fs.FsInfo, includedMetrics } } - if StorageDriver(dockerInfo.Driver) == ContainerdSnapshotterStorageDriver { + if StorageDriver(dockerInfo.Driver) == ContainerdSnapshotterStorageDriver && includedMetrics.Has(container.DiskUsageMetrics) { containerdClient, err = containerd.Client(*containerd.ArgContainerdEndpoint, "moby") if err != nil { - return fmt.Errorf("unable to create containerd client: %v", err) + klog.Warningf("Docker filesystem stats will not be reported: unable to create containerd client: %v", err) + includedMetrics = includedMetrics.Difference(container.MetricSet{container.DiskUsageMetrics: struct{}{}}) } } diff --git a/container/docker/handler.go b/container/docker/handler.go index 330ea46723..a7f4e2def5 100644 --- a/container/docker/handler.go +++ b/container/docker/handler.go @@ -125,7 +125,7 @@ func getRwLayerID(containerID, storageDir string, sd StorageDriver, dockerVersio // newContainerHandler returns a new container.ContainerHandler func newContainerHandler( - client *dclient.Client, + client dclient.APIClient, containerdClient containerd.ContainerdClient, name string, machineInfoFactory info.MachineInfoFactory, @@ -163,29 +163,34 @@ func newContainerHandler( otherStorageDir := path.Join(storageDir, pathToContainersDir, id) var rootfsStorageDir, zfsFilesystem, zfsParent string - if storageDriver == ContainerdSnapshotterStorageDriver { - ctx := namespaces.WithNamespace(context.Background(), "moby") - cntr, err := containerdClient.LoadContainer(ctx, id) - if err != nil { - return nil, err - } + if metrics.Has(container.DiskUsageMetrics) { + if storageDriver == ContainerdSnapshotterStorageDriver { + if containerdClient == nil { + return nil, fmt.Errorf("containerd client is required for Docker filesystem stats with %q storage driver", storageDriver) + } + ctx := namespaces.WithNamespace(context.Background(), "moby") + cntr, err := containerdClient.LoadContainer(ctx, id) + if err != nil { + return nil, err + } - var spec specs.Spec - if err := json.Unmarshal(cntr.Spec.Value, &spec); err != nil { - return nil, err - } - rootfsStorageDir = spec.Root.Path - } else { - rwLayerID, err := getRwLayerID(id, storageDir, storageDriver, dockerVersion) - if err != nil { - return nil, err - } + var spec specs.Spec + if err := json.Unmarshal(cntr.Spec.Value, &spec); err != nil { + return nil, err + } + rootfsStorageDir = spec.Root.Path + } else { + rwLayerID, err := getRwLayerID(id, storageDir, storageDriver, dockerVersion) + if err != nil { + 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, rwLayerID) - if err != nil { - return nil, fmt.Errorf("unable to determine device storage: %v", 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, rwLayerID) + if err != nil { + return nil, fmt.Errorf("unable to determine device storage: %v", err) + } } } diff --git a/container/docker/handler_test.go b/container/docker/handler_test.go index 8364d7d5fd..0936f1672c 100644 --- a/container/docker/handler_test.go +++ b/container/docker/handler_test.go @@ -27,8 +27,10 @@ import ( "github.com/moby/moby/api/types/container" dclient "github.com/moby/moby/client" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" info "github.com/google/cadvisor/info/v1" + cadvisorcontainer "github.com/google/cadvisor/lib/container" "github.com/google/cadvisor/lib/fs" ) @@ -160,6 +162,56 @@ func TestDockerEnvWhitelist(t *testing.T) { } +func TestNewContainerHandlerAllowsContainerdSnapshotterWithoutDiskMetrics(t *testing.T) { + as := assert.New(t) + + containerID := "1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef" + created := "2026-07-09T15:47:24.000000000Z" + client := &mockDockerClientForExitCode{ + inspectResp: dclient.ContainerInspectResult{ + Container: container.InspectResponse{ + ID: containerID, + Name: "/test-container", + Config: &container.Config{ + Image: "redis:7-alpine", + Labels: map[string]string{"com.docker.compose.service": "redis"}, + }, + HostConfig: &container.HostConfig{}, + NetworkSettings: &container.NetworkSettings{}, + State: &container.State{ + Pid: 1, + }, + Created: created, + }, + }, + } + + handler, err := newContainerHandler( + client, + nil, + "/docker/"+containerID, + nil, + nil, + ContainerdSnapshotterStorageDriver, + "/var/lib/docker", + map[string]string{}, + true, + nil, + []int{28, 5, 1}, + cadvisorcontainer.MetricSet{cadvisorcontainer.CpuUsageMetrics: struct{}{}}, + "", + nil, + nil, + ) + require.NoError(t, err) + + ref, err := handler.ContainerReference() + as.NoError(err) + as.Equal("test-container", ref.Aliases[0]) + + as.Equal("redis", handler.GetContainerLabels()["com.docker.compose.service"]) +} + func TestAddDiskStatsCheck(t *testing.T) { var readsCompleted, readsMerged, sectorsRead, readTime, writesCompleted, writesMerged, sectorsWritten, writeTime, ioInProgress, ioTime, weightedIoTime uint64 = 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11