daemon: make getInspectData() reusable

Move the bulk of the code to inspect a container into
(*Daemon).getInspectData() so other daemon code can inspect a
*container.Container (e.g. while holding its lock) and get almost the
same result as (*Daemon).ContainerInspect() of a container in the store.
The RWLayer metadata and size calculation are omitted so a container can
be inspected successfully when ctr.RWLayer == nil && !ctr.Dead.

Signed-off-by: Cory Snider <csnider@mirantis.com>

daemon/inspect.go

@@ -25,38 +25,12 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options
 	if err != nil {
 		return nil, nil, err
 	}
-
-	ctr.Lock()
-
-	base, desiredMACAddress, err := daemon.getInspectData(&daemon.config().Config, ctr)
+	base, desiredMACAddress, err := daemon.containerInspect(&daemon.config().Config, ctr)
 	if err != nil {
-		ctr.Unlock()
-		return nil, nil, err
+		return base, desiredMACAddress, err
 	}
 
-	// TODO(thaJeztah): do we need a deep copy here? Otherwise we could use maps.Clone (see https://github.com/moby/moby/commit/7917a36cc787ada58987320e67cc6d96858f3b55)
-	ports := make(networktypes.PortMap, len(ctr.NetworkSettings.Ports))
-	maps.Copy(ports, ctr.NetworkSettings.Ports)
-
-	apiNetworks := make(map[string]*networktypes.EndpointSettings)
-	for nwName, epConf := range ctr.NetworkSettings.Networks {
-		if epConf.EndpointSettings != nil {
-			// We must make a copy of this pointer object otherwise it can race with other operations
-			apiNetworks[nwName] = epConf.EndpointSettings.Copy()
-		}
-	}
-
-	networkSettings := &containertypes.NetworkSettings{
-		SandboxID:  ctr.NetworkSettings.SandboxID,
-		SandboxKey: ctr.NetworkSettings.SandboxKey,
-		Ports:      ports,
-		Networks:   apiNetworks,
-	}
-
-	mountPoints := ctr.GetMountPoints()
-
 	// Don’t hold container lock for size calculation (see https://github.com/moby/moby/issues/31158)
-	ctr.Unlock()
 	if options.Size {
 		sizeRw, sizeRootFs, err := daemon.imageService.GetContainerLayerSize(ctx, base.ID)
 		if err != nil {
@@ -65,19 +39,38 @@ func (daemon *Daemon) ContainerInspect(ctx context.Context, name string, options
 		base.SizeRw = &sizeRw
 		base.SizeRootFs = &sizeRootFs
 	}
+	return base, desiredMACAddress, nil
+}
 
-	imageManifest := ctr.ImageManifest
-	if imageManifest != nil && imageManifest.Platform == nil {
-		// Copy the image manifest to avoid mutating the original
-		c := *imageManifest
-		imageManifest = &c
+func (daemon *Daemon) containerInspect(daemonCfg *config.Config, ctr *container.Container) (_ *containertypes.InspectResponse, desiredMACAddress networktypes.HardwareAddr, _ error) {
+	ctr.Lock()
+	defer ctr.Unlock()
 
-		imageManifest.Platform = &ctr.ImagePlatform
+	base, desiredMACAddress, err := daemon.getInspectData(daemonCfg, ctr)
+	if err != nil {
+		return nil, nil, err
 	}
 
-	base.Mounts = mountPoints
-	base.NetworkSettings = networkSettings
-	base.ImageManifestDescriptor = imageManifest
+	if !daemon.UsesSnapshotter() {
+		if ctr.RWLayer == nil {
+			if ctr.State.Dead {
+				return base, desiredMACAddress, nil
+			}
+			return nil, nil, errdefs.System(errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil"))
+		}
+
+		graphDriverData, err := ctr.RWLayer.Metadata()
+		if err != nil {
+			if ctr.State.Dead {
+				// container is marked as Dead, and its graphDriver metadata may
+				// have been removed; we can ignore errors.
+				return base, desiredMACAddress, nil
+			}
+			return nil, nil, errdefs.System(err)
+		}
+
+		base.GraphDriver.Data = graphDriverData
+	}
 
 	return base, desiredMACAddress, nil
 }
@@ -154,6 +147,37 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
 		Config:          ctr.Config,
 	}
 
+	// TODO(thaJeztah): do we need a deep copy here? Otherwise we could use maps.Clone (see https://github.com/moby/moby/commit/7917a36cc787ada58987320e67cc6d96858f3b55)
+	ports := make(networktypes.PortMap, len(ctr.NetworkSettings.Ports))
+	maps.Copy(ports, ctr.NetworkSettings.Ports)
+
+	apiNetworks := make(map[string]*networktypes.EndpointSettings)
+	for nwName, epConf := range ctr.NetworkSettings.Networks {
+		if epConf.EndpointSettings != nil {
+			// We must make a copy of this pointer object otherwise it can race with other operations
+			apiNetworks[nwName] = epConf.EndpointSettings.Copy()
+		}
+	}
+
+	inspectResponse.NetworkSettings = &containertypes.NetworkSettings{
+		SandboxID:  ctr.NetworkSettings.SandboxID,
+		SandboxKey: ctr.NetworkSettings.SandboxKey,
+		Ports:      ports,
+		Networks:   apiNetworks,
+	}
+	inspectResponse.Mounts = ctr.GetMountPoints()
+
+	imageManifest := ctr.ImageManifest
+	if imageManifest != nil && imageManifest.Platform == nil {
+		// Copy the image manifest to avoid mutating the original
+		c := *imageManifest
+		imageManifest = &c
+
+		imageManifest.Platform = &ctr.ImagePlatform
+	}
+
+	inspectResponse.ImageManifestDescriptor = imageManifest
+
 	if daemon.UsesSnapshotter() {
 		inspectResponse.Storage = &storage.Storage{
 			RootFS: &storage.RootFSStorage{
@@ -162,32 +186,11 @@ func (daemon *Daemon) getInspectData(daemonCfg *config.Config, ctr *container.Co
 				},
 			},
 		}
-
-		// Additional information only applies to graphDrivers, so we're done.
-		return inspectResponse, macAddress, nil
-	}
-
-	inspectResponse.GraphDriver = &storage.DriverData{
-		Name: ctr.Driver,
-	}
-	if ctr.RWLayer == nil {
-		if ctr.State.Dead {
-			return inspectResponse, macAddress, nil
+	} else {
+		inspectResponse.GraphDriver = &storage.DriverData{
+			Name: ctr.Driver,
 		}
-		return nil, nil, errdefs.System(errors.New("RWLayer of container " + ctr.ID + " is unexpectedly nil"))
 	}
-
-	graphDriverData, err := ctr.RWLayer.Metadata()
-	if err != nil {
-		if ctr.State.Dead {
-			// container is marked as Dead, and its graphDriver metadata may
-			// have been removed; we can ignore errors.
-			return inspectResponse, macAddress, nil
-		}
-		return nil, nil, errdefs.System(err)
-	}
-
-	inspectResponse.GraphDriver.Data = graphDriverData
 	return inspectResponse, macAddress, nil
 }
 

daemon/inspect_test.go

@@ -5,16 +5,37 @@ import (
 
 	containertypes "github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/v2/daemon/container"
+	"github.com/moby/moby/v2/daemon/network"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
 
 func TestGetInspectData(t *testing.T) {
 	c := &container.Container{
-		ID:           "inspect-me",
-		HostConfig:   &containertypes.HostConfig{},
-		State:        &container.State{},
-		ExecCommands: container.NewExecStore(),
+		ID:              "inspect-me",
+		NetworkSettings: &network.Settings{},
+		HostConfig:      &containertypes.HostConfig{},
+		State:           &container.State{},
+		ExecCommands:    container.NewExecStore(),
+	}
+
+	d := &Daemon{
+		linkIndex: newLinkIndex(),
+	}
+	cfg := &configStore{}
+	d.configStore.Store(cfg)
+
+	_, _, err := d.getInspectData(&cfg.Config, c)
+	assert.Check(t, err)
+}
+
+func TestContainerInspect(t *testing.T) {
+	c := &container.Container{
+		ID:              "inspect-me",
+		NetworkSettings: &network.Settings{},
+		HostConfig:      &containertypes.HostConfig{},
+		State:           &container.State{},
+		ExecCommands:    container.NewExecStore(),
 	}
 
 	d := &Daemon{
@@ -26,10 +47,10 @@ func TestGetInspectData(t *testing.T) {
 	cfg := &configStore{}
 	d.configStore.Store(cfg)
 
-	_, _, err := d.getInspectData(&cfg.Config, c)
+	_, _, err := d.containerInspect(&cfg.Config, c)
 	assert.Check(t, is.ErrorContains(err, "RWLayer of container inspect-me is unexpectedly nil"))
 
 	c.State.Dead = true
-	_, _, err = d.getInspectData(&cfg.Config, c)
+	_, _, err = d.containerInspect(&cfg.Config, c)
 	assert.Check(t, err)
 }