Merge pull request moby#51294 from thaJeztah/daemon_cleanups

daemon: some minor cleanups

Author: vvoland

daemon/containerd/image_commit.go

@@ -20,6 +20,7 @@ import (
 	"github.com/moby/go-archive"
 	"github.com/moby/moby/v2/daemon/internal/image"
 	"github.com/moby/moby/v2/daemon/server/backend"
+	"github.com/moby/moby/v2/errdefs"
 	"github.com/opencontainers/go-digest"
 	"github.com/opencontainers/image-spec/identity"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -320,8 +321,7 @@ func cleanup(ctx context.Context, do func(context.Context)) {
 func (i *ImageService) CommitBuildStep(ctx context.Context, c backend.CommitConfig) (image.ID, error) {
 	ctr := i.containers.Get(c.ContainerID)
 	if ctr == nil {
-		// TODO: use typed error
-		return "", fmt.Errorf("container not found: %s", c.ContainerID)
+		return "", errdefs.NotFound(fmt.Errorf("container not found: %s", c.ContainerID))
 	}
 	c.ContainerMountLabel = ctr.MountLabel
 	c.ContainerOS = ctr.ImagePlatform.OS

daemon/daemon_unix_test.go

@@ -12,6 +12,7 @@ import (
 	containertypes "github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/v2/daemon/config"
 	"github.com/moby/moby/v2/daemon/container"
+	"github.com/moby/moby/v2/errdefs"
 	"github.com/moby/moby/v2/pkg/sysinfo"
 	"github.com/opencontainers/selinux/go-selinux"
 	"golang.org/x/sys/unix"
@@ -26,7 +27,7 @@ type fakeContainerGetter struct {
 func (f *fakeContainerGetter) GetContainer(cid string) (*container.Container, error) {
 	ctr, ok := f.containers[cid]
 	if !ok {
-		return nil, errors.New("container not found")
+		return nil, errdefs.NotFound(errors.New("container not found"))
 	}
 	return ctr, nil
 }

daemon/errors.go

@@ -54,11 +54,6 @@ func errExecPaused(id string) error {
 	return errdefs.Conflict(cause)
 }
 
-func errNotPaused(id string) error {
-	cause := errors.Errorf("Container %s is already paused", id)
-	return errdefs.Conflict(cause)
-}
-
 type nameConflictError struct {
 	id   string
 	name string

daemon/images/image_commit.go

@@ -3,14 +3,15 @@ package images
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"io"
 
 	"github.com/moby/moby/api/types/events"
 	"github.com/moby/moby/v2/daemon/internal/image"
 	"github.com/moby/moby/v2/daemon/internal/layer"
 	"github.com/moby/moby/v2/daemon/server/backend"
+	"github.com/moby/moby/v2/errdefs"
 	"github.com/moby/moby/v2/pkg/ioutils"
-	"github.com/pkg/errors"
 )
 
 // CommitImage creates a new image from a commit config
@@ -122,8 +123,7 @@ func exportContainerRw(layerStore layer.Store, id, mountLabel string) (arch io.R
 func (i *ImageService) CommitBuildStep(ctx context.Context, c backend.CommitConfig) (image.ID, error) {
 	ctr := i.containers.Get(c.ContainerID)
 	if ctr == nil {
-		// TODO: use typed error
-		return "", errors.Errorf("container not found: %s", c.ContainerID)
+		return "", errdefs.NotFound(fmt.Errorf("container not found: %s", c.ContainerID))
 	}
 	c.ContainerMountLabel = ctr.MountLabel
 	c.ContainerOS = ctr.ImagePlatform.OS

daemon/pause.go

@@ -7,6 +7,7 @@ import (
 	"github.com/containerd/log"
 	"github.com/moby/moby/api/types/events"
 	"github.com/moby/moby/v2/daemon/container"
+	"github.com/moby/moby/v2/errdefs"
 )
 
 // ContainerPause pauses a container
@@ -32,7 +33,7 @@ func (daemon *Daemon) containerPause(container *container.Container) error {
 
 	// We cannot Pause the container which is already paused
 	if container.State.Paused {
-		return errNotPaused(container.ID)
+		return errdefs.Conflict(fmt.Errorf("container %s is already paused", container.ID))
 	}
 
 	// We cannot Pause the container which is restarting

daemon/stats.go

@@ -57,8 +57,8 @@ func (daemon *Daemon) ContainerStats(ctx context.Context, prefixOrName string, c
 		return &ss
 	}
 
-	updates := daemon.subscribeToContainerStats(ctr)
-	defer daemon.unsubscribeToContainerStats(ctr, updates)
+	updates, cancel := daemon.subscribeToContainerStats(ctr)
+	defer cancel()
 
 	noStreamFirstFrame := !config.OneShot
 
@@ -90,12 +90,15 @@ func (daemon *Daemon) ContainerStats(ctx context.Context, prefixOrName string, c
 	}
 }
 
-func (daemon *Daemon) subscribeToContainerStats(c *container.Container) chan any {
-	return daemon.statsCollector.Collect(c)
-}
-
-func (daemon *Daemon) unsubscribeToContainerStats(c *container.Container, ch chan any) {
-	daemon.statsCollector.Unsubscribe(c, ch)
+// subscribeToContainerStats starts collecting stats for the given container.
+// It returns a channel containing [containertypes.StatsResponse] records,
+// and a cancel function to unsubscribe and stop collecting stats.
+func (daemon *Daemon) subscribeToContainerStats(c *container.Container) (updates chan any, cancel func()) {
+	ch := daemon.statsCollector.Collect(c)
+	cancel = func() {
+		daemon.statsCollector.Unsubscribe(c, ch)
+	}
+	return ch, cancel
 }
 
 // GetContainerStats collects all the stats published by a container

daemon/stats/collector.go

@@ -95,9 +95,12 @@ func (s *Collector) Run() {
 		// but saves allocations in further iterations
 		pairs = pairs[:0]
 
-		for container, publisher := range s.publishers {
+		for ctr, publisher := range s.publishers {
 			// copy pointers here to release the lock ASAP
-			pairs = append(pairs, publishersPair{container, publisher})
+			pairs = append(pairs, publishersPair{
+				container: ctr,
+				publisher: publisher,
+			})
 		}
 
 		s.cond.L.Unlock()

daemon/stats_unix.go

@@ -13,6 +13,7 @@ import (
 
 	statsV1 "github.com/containerd/cgroups/v3/cgroup1/stats"
 	statsV2 "github.com/containerd/cgroups/v3/cgroup2/stats"
+	cerrdefs "github.com/containerd/errdefs"
 	containertypes "github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/v2/daemon/container"
 	"github.com/pkg/errors"
@@ -40,7 +41,7 @@ func (daemon *Daemon) stats(c *container.Container) (*containertypes.StatsRespon
 	}
 	cs, err := task.Stats(context.Background())
 	if err != nil {
-		if strings.Contains(err.Error(), "container not found") {
+		if cerrdefs.IsNotFound(err) || strings.Contains(err.Error(), "container not found") {
 			return nil, containerNotFound(c.ID)
 		}
 		return nil, err