Merge pull request #984 from jacobweinstock/tink-worker-image-pull

[tink worker] Don't error when image pull fails and image exists:

## Description

<!--- Please describe what this PR is going to change -->
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 named `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.

## Why is this needed

<!--- Link to issue you have raised -->

Fixes: #

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->


## How are existing users impacted? What migration steps/scripts do we need?

<!--- Fixes a bug, unblocks installation, removes a component of the stack etc -->
<!--- Requires a DB migration script, etc. -->


## Checklist:

I have:

- [ ] updated the documentation and/or roadmap (if required)
- [ ] added unit or e2e tests
- [ ] provided instructions on how to upgrade

cmd/tink-worker/worker/container_manager_test.go

@@ -26,17 +26,32 @@ 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,
 		statusCode:       statusCode,
 		err:              err,
 		waitErr:          waitErr,
 	}
+
+	for _, opt := range opts {
+		opt(f)
+	}
+
+	return f
 }
 
 func (c *fakeDockerClient) ContainerCreate(

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() {

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)