From 448d767495d268ddd7f1185e4e5eec3833fa9b21 Mon Sep 17 00:00:00 2001 From: Dmitrii Tikhomirov Date: Tue, 29 Oct 2024 12:07:13 -0700 Subject: [PATCH] kie-issues#1549: kn-workflow-plugin check for the presence of an image in the local Docker image does not cover all cases (#2685) --- packages/kn-plugin-workflow/go.mod | 3 +- packages/kn-plugin-workflow/go.sum | 1 + .../pkg/common/containers.go | 95 ++++++++++++++----- .../pkg/common/containers_test.go | 75 +++++++++++++++ 4 files changed, 147 insertions(+), 27 deletions(-) create mode 100644 packages/kn-plugin-workflow/pkg/common/containers_test.go diff --git a/packages/kn-plugin-workflow/go.mod b/packages/kn-plugin-workflow/go.mod index 35e3663862a..b8dfab167a6 100644 --- a/packages/kn-plugin-workflow/go.mod +++ b/packages/kn-plugin-workflow/go.mod @@ -12,6 +12,7 @@ require ( github.com/apache/incubator-kie-tools/packages/sonataflow-operator/api v0.0.0 github.com/apache/incubator-kie-tools/packages/sonataflow-operator/workflowproj v0.0.0 github.com/beevik/etree v1.2.0 + github.com/docker/distribution v2.8.2+incompatible github.com/docker/docker v24.0.9+incompatible github.com/docker/go-connections v0.4.0 github.com/jstemmer/go-junit-report/v2 v2.0.0 @@ -31,7 +32,6 @@ require ( github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/dgraph-io/ristretto v0.1.1 // indirect - github.com/docker/distribution v2.8.2+incompatible // indirect github.com/docker/go-units v0.5.0 // indirect github.com/dprotaso/go-yit v0.0.0-20220510233725-9ba8df137936 // indirect github.com/dustin/go-humanize v1.0.1 // indirect @@ -88,6 +88,7 @@ require ( github.com/spf13/cast v1.5.1 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect + github.com/stretchr/objx v0.5.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/vmware-labs/yaml-jsonpath v0.3.2 // indirect golang.org/x/crypto v0.21.0 // indirect diff --git a/packages/kn-plugin-workflow/go.sum b/packages/kn-plugin-workflow/go.sum index f4a9214d352..7c2d83ca084 100644 --- a/packages/kn-plugin-workflow/go.sum +++ b/packages/kn-plugin-workflow/go.sum @@ -347,6 +347,7 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= diff --git a/packages/kn-plugin-workflow/pkg/common/containers.go b/packages/kn-plugin-workflow/pkg/common/containers.go index af06f6a68a9..afb2482b69e 100644 --- a/packages/kn-plugin-workflow/pkg/common/containers.go +++ b/packages/kn-plugin-workflow/pkg/common/containers.go @@ -20,10 +20,13 @@ package common import ( + "bufio" + "bytes" "context" "encoding/json" "errors" "fmt" + "github.com/docker/distribution/reference" "io" "os" "os/exec" @@ -31,11 +34,11 @@ import ( "runtime" "strings" "syscall" + "time" "github.com/apache/incubator-kie-tools/packages/kn-plugin-workflow/pkg/metadata" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" - "github.com/docker/docker/api/types/filters" "github.com/docker/docker/client" "github.com/docker/docker/pkg/stdcopy" "github.com/docker/go-connections/nat" @@ -51,6 +54,10 @@ type DockerLogMessage struct { ID string `json:"id,omitempty"` } +type DockerClient interface { + ImageList(ctx context.Context, options types.ImageListOptions) ([]types.ImageSummary, error) +} + func getDockerClient() (*client.Client, error) { cli, err := client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) if err != nil { @@ -198,29 +205,71 @@ func GracefullyStopTheContainerWhenInterrupted(containerTool string) { }() } -func pullDockerImage(cli *client.Client, ctx context.Context) (io.ReadCloser, error) { +func pullDockerImage(cli *client.Client, ctx context.Context) error { // Check if the image exists locally. - // For that we should check only the image name and tag, removing the registry, - // as `docker image ls --filter reference=` will return empty if the image_full_url is not the first tag - // of an image. - imageNameWithoutRegistry := strings.Split(metadata.DevModeImage, "/") - imageFilters := filters.NewArgs() - imageFilters.Add("reference", fmt.Sprintf("*/%s", imageNameWithoutRegistry[len(imageNameWithoutRegistry)-1])) - images, err := cli.ImageList(ctx, types.ImageListOptions{Filters: imageFilters}) + exists, err := CheckImageExists(cli, ctx, metadata.DevModeImage) if err != nil { - return nil, fmt.Errorf("error listing images: %s", err) + return fmt.Errorf("error listing images: %s", err) } // If the image is not found locally, pull it from the remote registry - if len(images) == 0 { - reader, err := cli.ImagePull(ctx, metadata.DevModeImage, types.ImagePullOptions{}) - if err != nil { - return nil, fmt.Errorf("\nError pulling image: %s. Error is: %s", metadata.DevModeImage, err) + if !exists { + fmt.Printf("\n⏳ Retrieving (%s), this could take some time...\n", metadata.DevModeImage) + + ctx, cancel := context.WithTimeout(ctx, 1*time.Minute) + defer cancel() + + reader, writer := io.Pipe() + defer writer.Close() + + var stderr bytes.Buffer + + go func() { + scanner := bufio.NewScanner(reader) + for scanner.Scan() { + fmt.Print(".") + } + }() + + // we use local docker client to pull the image + cmd := exec.CommandContext(ctx, "docker", "pull", metadata.DevModeImage) + cmd.Stdout = writer + cmd.Stderr = &stderr + + if err := cmd.Start(); err != nil { + return fmt.Errorf("\nError pulling image: %s. Error is: %s", metadata.DevModeImage, err) + } + + if err := cmd.Wait(); err != nil { + return fmt.Errorf("\nError pulling image: %s. Error is: %s", metadata.DevModeImage, stderr.String()) } - return reader, nil + fmt.Println("\n🎉 Successfully pulled the image") + } + + return nil +} + +func CheckImageExists(cli DockerClient, ctx context.Context, imageName string) (bool, error) { + named, err := reference.ParseNormalizedNamed(imageName) + + if tagged, ok := named.(reference.Tagged); ok { + imageName = fmt.Sprintf("%s:%s", reference.Path(named), tagged.Tag()) + } else { + imageName = fmt.Sprintf("%s:%s", reference.Path(named), "latest") + } + images, err := cli.ImageList(ctx, types.ImageListOptions{All: true}) + if err != nil { + return false, fmt.Errorf("error listing images: %s", err) } - return nil, nil + for _, image := range images { + for _, tag := range image.RepoTags { + if strings.HasSuffix(tag, imageName) { + return true, nil + } + } + } + return false, nil } func processDockerImagePullLogs(reader io.ReadCloser) error { @@ -286,24 +335,18 @@ func startDockerContainer(cli *client.Client, ctx context.Context, resp containe } func runDockerContainer(portMapping string, path string) error { - ctx := context.Background() + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute) + defer cancel() + cli, err := getDockerClient() if err != nil { return err } - - reader, err := pullDockerImage(cli, ctx) + err = pullDockerImage(cli, ctx) if err != nil { return err } - if reader != nil { - fmt.Printf("\n⏳ Retrieving (%s), this could take some time...\n", metadata.DevModeImage) - if err := processDockerImagePullLogs(reader); err != nil { - return err - } - } - resp, err := createDockerContainer(cli, ctx, portMapping, path) if err != nil { return err diff --git a/packages/kn-plugin-workflow/pkg/common/containers_test.go b/packages/kn-plugin-workflow/pkg/common/containers_test.go new file mode 100644 index 00000000000..c54cfc7f6c6 --- /dev/null +++ b/packages/kn-plugin-workflow/pkg/common/containers_test.go @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package common + +import ( + "context" + "github.com/docker/docker/api/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "testing" +) + +type MockDockerClient struct { + mock.Mock +} + +func (m *MockDockerClient) ImageList(ctx context.Context, options types.ImageListOptions) ([]types.ImageSummary, error) { + args := m.Called(ctx, options) + return args.Get(0).([]types.ImageSummary), args.Error(1) +} + +func TestCheckImageExists(t *testing.T) { + + tests := []struct { + lookup string + images []string + expected bool + }{ + {"docker.io/example/app-image:latest", []string{"docker.io/example/app-image:latest"}, true}, + {"docker.io/demo/service-image:1.0", []string{"demo/service-image:1.0"}, true}, + + {"docker.io/testuser/sample-app", []string{"docker.io/testuser/sample-app:latest"}, true}, + {"docker.io/testuser/sample-app", []string{"testuser/sample-app:latest"}, true}, + + {"testuser/sample-app:dev", []string{"docker.io/testuser/sample-app:dev"}, true}, + {"testuser/sample-app:dev", []string{"testuser/sample-app:dev"}, true}, + + {"docker.io/example/app-image:latest", []string{"app-image:latest"}, false}, + {"docker.io/testuser/sample-app", []string{"sample-app:latest"}, false}, + {"testuser/sample-app:dev", []string{"sample-app:dev"}, false}, + } + + for _, test := range tests { + ctx := context.Background() + mockClient := new(MockDockerClient) + + mockClient.On("ImageList", ctx, mock.Anything).Return([]types.ImageSummary{ + { + RepoTags: test.images, + }, + }, nil) + + exists, err := CheckImageExists(mockClient, ctx, test.lookup) + assert.NoError(t, err, "Error should be nil") + assert.True(t, exists == test.expected, "Expected %t, got %t", test.expected, exists) + } +}