polish cdebug exec new pull if not present logic (docker only)

Author: iximiuz

cmd/exec/exec_docker.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
@@ -16,39 +17,6 @@ import (
 	"github.com/iximiuz/cdebug/pkg/uuid"
 )
 
-// debugImageExistsLocally checks if the debug image exists in the host and that it matches the target container
-// platform.
-func debugImageExistsLocally(ctx context.Context, client *docker.Client, debugImage string, debugImagePlatform string, target types.ContainerJSON) (bool, error) {
-	debugImageInspect, _, err := client.ImageInspectWithRaw(ctx, debugImage)
-	if err != nil {
-		// err means that the requested image wasn't found,
-		// therefore it needs to be pulled
-		logrus.Debugf("The image %s wasn't found locally", debugImage)
-		return false, nil
-	}
-
-	debugImageOs := debugImageInspect.Os
-	debugImageArchitecture := debugImageInspect.Architecture
-	// override through --platform flag
-	if debugImagePlatform != "" {
-		fmt.Sscanf(debugImagePlatform, "%s/%s", &debugImageOs, &debugImageArchitecture)
-	}
-
-	// target.Platform only contains the Os but doesn't have the Arch, however we can
-	// get the Os & Arch by analyzing the image using target.Image
-	targetImageInspect, _, err := client.ImageInspectWithRaw(ctx, target.Image)
-	if err != nil {
-		return false, fmt.Errorf("failed to inspect image %s: %v", target.Image, err)
-	}
-
-	// debug image exists, also check that the platform matches the target image platform
-	if debugImageArchitecture != targetImageInspect.Architecture || debugImageOs != targetImageInspect.Os {
-		return false, nil
-	}
-
-	return true, nil
-}
-
 func runDebuggerDocker(ctx context.Context, cli cliutil.CLI, opts *options) error {
 	client, err := docker.NewClient(docker.Options{
 		Out:  cli.AuxStream(),
@@ -66,20 +34,19 @@ func runDebuggerDocker(ctx context.Context, cli cliutil.CLI, opts *options) erro
 		return errTargetNotRunning
 	}
 
-	imageExists, err := debugImageExistsLocally(ctx, client, opts.image, opts.platform, target)
+	platform := opts.platform
+	if len(platform) == 0 {
+		platform = target.Platform
+	}
+
+	imageExists, err := imageExistsLocally(ctx, client, opts.image, platform)
 	if err != nil {
 		return err
 	}
-
 	if !imageExists {
 		cli.PrintAux("Pulling debugger image...\n")
 		if err := client.ImagePullEx(ctx, opts.image, types.ImagePullOptions{
-			Platform: func() string {
-				if len(opts.platform) == 0 {
-					return target.Platform
-				}
-				return opts.platform
-			}(),
+			Platform: platform,
 		}); err != nil {
 			return errCannotPull(opts.image, err)
 		}
@@ -265,3 +232,34 @@ func (s *ioStreamer) stream(ctx context.Context) error {
 func ptr[T any](v T) *T {
 	return &v
 }
+
+func imageExistsLocally(
+	ctx context.Context,
+	client *docker.Client,
+	image string,
+	platform string,
+) (bool, error) {
+	imageSummary, _, err := client.ImageInspectWithRaw(ctx, image)
+	if err != nil {
+		logrus.Debugf("The image %s (%s) wasn't found locally", image, platform)
+		return false, nil
+	}
+
+	parts := strings.Split(platform, "/")
+	if imageSummary.Os != parts[0] {
+		logrus.Debugf("The image %s (%s) found locally, but the OS doesn't match", image, platform)
+		return false, nil
+	}
+
+	if len(parts) > 1 && imageSummary.Architecture != parts[1] {
+		logrus.Debugf("The image %s (%s) found locally, but the architecture doesn't match", image, platform)
+		return false, nil
+	}
+
+	if len(parts) > 2 && imageSummary.Variant != parts[2] {
+		logrus.Debugf("The image %s (%s) found locally, but the variant doesn't match", image, platform)
+		return false, nil
+	}
+
+	return true, nil
+}

e2e/exec/docker_test.go

@@ -1,8 +1,7 @@
 package exec
 
 import (
-	"fmt"
-	"regexp"
+	"strings"
 	"testing"
 
 	"gotest.tools/assert"
@@ -23,27 +22,6 @@ func TestExecDockerSimple(t *testing.T) {
 	assert.Check(t, cmp.Contains(res.Stdout(), "debian"))
 }
 
-func TestExecDockerUseLocalImage(t *testing.T) {
-	localImage, cleanupLocalImage := fixture.DockerBuildLocalImage(t)
-	defer cleanupLocalImage()
-
-	targetImageID, targetImageCleanup := fixture.DockerRunBackground(t, fixture.ImageNginx, nil)
-	defer targetImageCleanup()
-
-	res := icmd.RunCmd(
-		icmd.Command("cdebug", "--image", localImage, "-l=debug", "exec", "--rm", "-q", targetImageID, "cat", "/etc/os-release"),
-	)
-	res.Assert(t, icmd.Success)
-	assert.Check(t, cmp.Contains(res.Stdout(), "debian"))
-	assert.Assert(t, func() cmp.Result {
-		re := regexp.MustCompile("Pulling debugger image...")
-		if re.MatchString(res.Stdout()) {
-			return cmp.ResultFailure(fmt.Sprintf("Image %s shouldn't be pulled because it only exists locally", localImage))
-		}
-		return cmp.ResultSuccess
-	})
-}
-
 func TestExecDockerHostNamespaces(t *testing.T) {
 	targetID, cleanup := fixture.DockerRunBackground(t, fixture.ImageNginx,
 		[]string{"--net", "host", "--pid", "host"},
@@ -90,3 +68,28 @@ func TestExecDockerNixery(t *testing.T) {
 	res.Assert(t, icmd.Success)
 	assert.Check(t, cmp.Contains(res.Stdout(), "VIM - Vi IMproved"))
 }
+
+func TestExecDockerUseLocalImage(t *testing.T) {
+	targetID, targetCleanup := fixture.DockerRunBackground(t, fixture.ImageNginx, nil)
+	defer targetCleanup()
+
+	remoteImage := "busybox:musl"
+	fixture.DockerImageRemove(t, remoteImage)
+
+	res := icmd.RunCmd(
+		icmd.Command("cdebug", "exec", "--rm", "-i", "--image", remoteImage, targetID, "cat", "/etc/os-release"),
+	)
+	res.Assert(t, icmd.Success)
+	assert.Check(t, cmp.Contains(res.Stdout(), "debian"))
+	assert.Check(t, cmp.Contains(res.Stderr(), "Pulling debugger image..."))
+
+	localImage, imageCleanup := fixture.DockerImageBuild(t, "thisimageonlyexistslocally:1.0")
+	defer imageCleanup()
+
+	res = icmd.RunCmd(
+		icmd.Command("cdebug", "exec", "--rm", "-i", "--image", localImage, targetID, "cat", "/etc/os-release"),
+	)
+	res.Assert(t, icmd.Success)
+	assert.Check(t, cmp.Contains(res.Stdout(), "debian"))
+	assert.Equal(t, strings.Contains(res.Stderr(), "Pulling debugger image..."), false)
+}

e2e/internal/fixture/Dockerfile

@@ -2,8 +2,6 @@ package fixture
 
 import (
 	"os/exec"
-	"path/filepath"
-	"runtime"
 	"strings"
 	"testing"
 
@@ -91,28 +89,30 @@ func DockerRunBackground(
 	return contID, cleanup
 }
 
-func DockerBuildLocalImage(
+func DockerImageBuild(
 	t *testing.T,
+	name string,
 ) (string, func()) {
-	localImage := "thisimageonlyexistslocally:1.0"
+	cmd := dockerCmd("build", "-t", name, "-")
 
-	// Get dirname of current file, assumes that the Dockerfile lives next to this file.
-	_, filename, _, ok := runtime.Caller(0)
-	if !ok {
-		t.Fatalf("Failed to get filename")
-	}
-	dirname := filepath.Dir(filename)
-
-	cmd := dockerCmd("build", "-t", localImage, dirname)
-
-	res := icmd.RunCmd(cmd)
+	res := icmd.RunCmd(cmd, icmd.WithStdin(strings.NewReader("FROM busybox:musl\n")))
 	res.Assert(t, icmd.Success)
 
 	cleanup := func() {
-		icmd.RunCmd(dockerCmd("rmi", localImage)).Assert(t, icmd.Success)
+		icmd.RunCmd(dockerCmd("rmi", name)).Assert(t, icmd.Success)
 	}
 
-	return localImage, cleanup
+	return name, cleanup
+}
+
+func DockerImageRemove(
+	t *testing.T,
+	name string,
+) {
+	res := icmd.RunCmd(dockerCmd("rmi", name))
+	if !strings.Contains(res.Stderr(), "No such image") {
+		res.Assert(t, icmd.Success)
+	}
 }
 
 func NerdctlRunBackground(