From 3a7b9f64339229e0bf3ca2bf55b5e724ef851b5e Mon Sep 17 00:00:00 2001 From: Ivan Velichko Date: Sun, 18 Feb 2024 15:18:19 +0000 Subject: [PATCH] polish cdebug exec new pull if not present logic (docker only) --- cmd/exec/exec_docker.go | 80 ++++++++++++++++----------------- e2e/exec/docker_test.go | 49 ++++++++++---------- e2e/internal/fixture/Dockerfile | 1 - e2e/internal/fixture/fixture.go | 32 ++++++------- 4 files changed, 81 insertions(+), 81 deletions(-) delete mode 100644 e2e/internal/fixture/Dockerfile diff --git a/cmd/exec/exec_docker.go b/cmd/exec/exec_docker.go index 92a784e..5243f1d 100644 --- a/cmd/exec/exec_docker.go +++ b/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 +} diff --git a/e2e/exec/docker_test.go b/e2e/exec/docker_test.go index 238e922..3535005 100644 --- a/e2e/exec/docker_test.go +++ b/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) +} diff --git a/e2e/internal/fixture/Dockerfile b/e2e/internal/fixture/Dockerfile deleted file mode 100644 index 2e9ee5a..0000000 --- a/e2e/internal/fixture/Dockerfile +++ /dev/null @@ -1 +0,0 @@ -FROM busybox:musl diff --git a/e2e/internal/fixture/fixture.go b/e2e/internal/fixture/fixture.go index 9346c10..6f8f370 100644 --- a/e2e/internal/fixture/fixture.go +++ b/e2e/internal/fixture/fixture.go @@ -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(