From a54bccedf6db2bd96546af86d9b4af7d2d76462a Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Thu, 29 Aug 2024 15:23:36 -0600 Subject: [PATCH 1/2] Don't error when image pull fails and image exists: HookOS recently got the capability to embed container images. With this capability, pulling an image is not desired. This is expecially true if the image name is not resolvable or there is no network connection to the registry. An image name 127.0.0.1/embedded/myimage, for example. Currently, tink worker will always try to pull an image and will fail if the image pull fails. To allow for embedded images to function properly, when an image pull fails we check if the image already exists in the local Docker cache. If it does we don't fail the method call. Signed-off-by: Jacob Weinstock --- cmd/tink-worker/worker/registry.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/tink-worker/worker/registry.go b/cmd/tink-worker/worker/registry.go index 144d3714f..9b4b06dbb 100644 --- a/cmd/tink-worker/worker/registry.go +++ b/cmd/tink-worker/worker/registry.go @@ -31,6 +31,7 @@ type ImagePullStatus struct { } // PullImage outputs to stdout the contents of the requested image (relative to the registry). +// If a pull fails but the image already exists then we will return a nil error. func (m *containerManager) PullImage(ctx context.Context, img string) error { l := m.getLogger(ctx) authConfig := registry.AuthConfig{ @@ -46,6 +47,9 @@ func (m *containerManager) PullImage(ctx context.Context, img string) error { out, err := m.cli.ImagePull(ctx, path.Join(m.registryDetails.Registry, img), image.PullOptions{RegistryAuth: authStr}) if err != nil { + if _, _, err := m.cli.ImageInspectWithRaw(ctx, path.Join(m.registryDetails.Registry, img)); err == nil { + return nil + } return errors.Wrap(err, "DOCKER PULL") } defer func() { From c0998f95f9716a5f9b2c8a4d2353fe5954bebe72 Mon Sep 17 00:00:00 2001 From: Jacob Weinstock Date: Thu, 29 Aug 2024 15:41:22 -0600 Subject: [PATCH 2/2] Update tests for PullImage: With the change in behavior, the tests needed to be updated. Signed-off-by: Jacob Weinstock --- .../worker/container_manager_test.go | 19 +++++++++++++++++-- cmd/tink-worker/worker/registry_test.go | 16 +++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/cmd/tink-worker/worker/container_manager_test.go b/cmd/tink-worker/worker/container_manager_test.go index 9a0a7dfc7..ab85b5a89 100644 --- a/cmd/tink-worker/worker/container_manager_test.go +++ b/cmd/tink-worker/worker/container_manager_test.go @@ -26,10 +26,19 @@ type fakeDockerClient struct { statusCode int err error waitErr error + imageInspectErr error } -func newFakeDockerClient(containerID, imagePullContent string, delay time.Duration, statusCode int, err, waitErr error) *fakeDockerClient { - return &fakeDockerClient{ +type dockerClientOpt func(*fakeDockerClient) + +func withImageInspectErr(err error) dockerClientOpt { + return func(c *fakeDockerClient) { + c.imageInspectErr = err + } +} + +func newFakeDockerClient(containerID, imagePullContent string, delay time.Duration, statusCode int, err, waitErr error, opts ...dockerClientOpt) *fakeDockerClient { + f := &fakeDockerClient{ containerID: containerID, imagePullContent: imagePullContent, delay: delay, @@ -37,6 +46,12 @@ func newFakeDockerClient(containerID, imagePullContent string, delay time.Durati err: err, waitErr: waitErr, } + + for _, opt := range opts { + opt(f) + } + + return f } func (c *fakeDockerClient) ContainerCreate( diff --git a/cmd/tink-worker/worker/registry_test.go b/cmd/tink-worker/worker/registry_test.go index fcc5a40ad..f3989bc7f 100644 --- a/cmd/tink-worker/worker/registry_test.go +++ b/cmd/tink-worker/worker/registry_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/image" "github.com/go-logr/zapr" "go.uber.org/zap" @@ -19,6 +20,10 @@ func (c *fakeDockerClient) ImagePull(context.Context, string, image.PullOptions) return io.NopCloser(strings.NewReader(c.imagePullContent)), nil } +func (c *fakeDockerClient) ImageInspectWithRaw(context.Context, string) (types.ImageInspect, []byte, error) { + return types.ImageInspect{}, nil, c.imageInspectErr +} + func TestContainerManagerPullImage(t *testing.T) { cases := []struct { name string @@ -27,6 +32,7 @@ func TestContainerManagerPullImage(t *testing.T) { registry RegistryConnDetails clientErr error wantErr error + imageInspectErr error }{ { name: "Happy Path", @@ -39,19 +45,27 @@ func TestContainerManagerPullImage(t *testing.T) { responseContent: "{", clientErr: errors.New("You missed the shot"), wantErr: errors.New("DOCKER PULL: You missed the shot"), + imageInspectErr: errors.New("Image not in local cache"), }, { name: "pull error", image: "yav.in/4/deathstar:nomedalforchewie", responseContent: `{"error": "You missed the shot"}`, wantErr: errors.New("DOCKER PULL: You missed the shot"), + imageInspectErr: errors.New("Image not in local cache"), + }, + { + name: "image already exists, no error", + image: "yav.in/4/deathstar:nomedalforchewie", + clientErr: errors.New("You missed the shot"), + wantErr: nil, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) - mgr := NewContainerManager(logger, newFakeDockerClient("", tc.responseContent, 0, 0, tc.clientErr, nil), tc.registry) + mgr := NewContainerManager(logger, newFakeDockerClient("", tc.responseContent, 0, 0, tc.clientErr, nil, withImageInspectErr(tc.imageInspectErr)), tc.registry) ctx := context.Background() gotErr := mgr.PullImage(ctx, tc.image)