Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions pkg/devcontainer/image_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"

"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/filters"
"github.com/docker/docker/api/types/image"
"github.com/docker/docker/client"
"github.com/docker/docker/pkg/archive"
Expand All @@ -16,6 +17,7 @@ import (
type ImageManager interface {
PullImage(ctx context.Context, name string) error
BuildImage(ctx context.Context, workingDir string, config Config) (string, error)
LocalImageExists(ctx context.Context, name string) (bool, error)
}

type DockerImageManager struct {
Expand Down Expand Up @@ -128,6 +130,20 @@ func (im *DockerImageManager) BuildImage(ctx context.Context, workingDir string,
return tag, nil
}

// LocalImageExists checks if an image with the given name exists locally.
// It returns true if the image exists, false if it doesn't, and an error if the check fails.
func (im *DockerImageManager) LocalImageExists(ctx context.Context, name string) (bool, error) {
imgs, err := im.ImageList(ctx, image.ListOptions{Filters: filters.NewArgs(filters.Arg("reference", name))})
if err != nil {
return false, fmt.Errorf("failed to list local images: %w", err)
}
exists := len(imgs) != 0
if exists {
log.Debug().Str("image", name).Msg("Local image exists")
}
return exists, nil
}

func sanitizeContainerName(containerName string) string {
containerName = strings.ReplaceAll(containerName, " ", "-")
return containerName
Expand Down
66 changes: 66 additions & 0 deletions pkg/devcontainer/image_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/artmoskvin/hide/pkg/devcontainer/mocks"
"github.com/artmoskvin/hide/pkg/random"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/image"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)
Expand Down Expand Up @@ -259,6 +260,71 @@ func TestDockerImageManager_BuildImage(t *testing.T) {
}
}

func TestDockerImageManager_LocalImageExists(t *testing.T) {
tests := []struct {
name string
image string
mockSetup func(m *mocks.MockDockerImageClient)
expectedResult bool
expectedError string
}{
{
name: "image exists",
image: "test-image",
mockSetup: func(m *mocks.MockDockerImageClient) {
m.On("ImageList", mock.Anything, mock.Anything).
Return([]image.Summary{{ID: "test-image"}}, nil)
},
expectedResult: true,
},
{
name: "image doesn't exist",
image: "test-image",
mockSetup: func(m *mocks.MockDockerImageClient) {
m.On("ImageList", mock.Anything, mock.Anything).
Return([]image.Summary{}, nil)
},
expectedResult: false,
},
{
name: "error",
image: "test-image",
mockSetup: func(m *mocks.MockDockerImageClient) {
m.On("ImageList", mock.Anything, mock.Anything).
Return([]image.Summary{}, errors.New("test error"))
},
expectedResult: false,
expectedError: "test error",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockClient := &mocks.MockDockerImageClient{}
tt.mockSetup(mockClient)

imageManager := devcontainer.NewImageManager(mockClient, random.String, nil)

result, err := imageManager.LocalImageExists(context.Background(), tt.image)

if tt.expectedResult {
assert.True(t, result)
} else {
assert.False(t, result)
}

if tt.expectedError != "" {
assert.Error(t, err)
assert.Contains(t, err.Error(), tt.expectedError)
} else {
assert.NoError(t, err)
}

mockClient.AssertExpectations(t)
})
}
}

func strPtr(s string) *string {
return &s
}
5 changes: 5 additions & 0 deletions pkg/devcontainer/mocks/mock_image_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,8 @@ func (m *MockImageManager) BuildImage(ctx context.Context, workingDir string, co
args := m.Called(ctx, workingDir, config)
return args.String(0), args.Error(1)
}

func (m *MockImageManager) LocalImageExists(ctx context.Context, name string) (bool, error) {
args := m.Called(ctx, name)
return args.Bool(0), args.Error(1)
}
68 changes: 42 additions & 26 deletions pkg/devcontainer/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,36 +43,14 @@ func (r *DockerRunner) Run(ctx context.Context, projectPath string, config Confi
}
}

// Pull or build image
var imageId string
var err error

switch {
case config.IsImageDevContainer():
imageId = config.DockerImageProps.Image
err = r.imageManager.PullImage(ctx, config.DockerImageProps.Image)
if err != nil {
err = fmt.Errorf("Failed to pull image: %w", err)
}
case config.IsDockerfileDevContainer():
imageId, err = r.imageManager.BuildImage(ctx, projectPath, config)
if err != nil {
err = fmt.Errorf("Failed to build image: %w", err)
}
case config.IsComposeDevContainer():
// TODO: build docker-compose file
err = fmt.Errorf("Docker Compose is not supported yet")
default:
err = fmt.Errorf("Invalid devcontainer configuration")
}

// Get image
imageId, err := r.getImage(ctx, config, projectPath)
if err != nil {
return "", fmt.Errorf("Failed to pull or build image: %w", err)
return "", fmt.Errorf("Failed to get image: %w", err)
}

// Create container
containerId, err := r.containerManager.CreateContainer(ctx, imageId, projectPath, config)

if err != nil {
return "", fmt.Errorf("Failed to create container: %w", err)
}
Expand Down Expand Up @@ -145,7 +123,6 @@ func (r *DockerRunner) executeLifecycleCommandInContainer(ctx context.Context, l
log.Debug().Str("name", name).Str("command", fmt.Sprintf("%s", command)).Msg("Running command")

result, err := r.Exec(ctx, containerId, command)

if err != nil {
return fmt.Errorf("Failed to run command %s %s in container %s: %w", name, command, containerId, err)
}
Expand All @@ -158,3 +135,42 @@ func (r *DockerRunner) executeLifecycleCommandInContainer(ctx context.Context, l

return nil
}

func (r *DockerRunner) getImage(ctx context.Context, config Config, projectPath string) (string, error) {
switch {
case config.IsImageDevContainer():
return r.getOrPullImage(ctx, config.DockerImageProps.Image)
case config.IsDockerfileDevContainer():
imageId, err := r.imageManager.BuildImage(ctx, projectPath, config)
if err != nil {
return "", fmt.Errorf("Failed to build image: %w", err)
}
return imageId, nil
case config.IsComposeDevContainer():
// TODO: build docker-compose file
return "", fmt.Errorf("Docker Compose is not supported yet")
default:
return "", fmt.Errorf("Invalid devcontainer configuration")
}
}

func (r *DockerRunner) getOrPullImage(ctx context.Context, imageId string) (string, error) {
if imageId == "" {
return "", fmt.Errorf("image id is empty")
}

exists, err := r.imageManager.LocalImageExists(ctx, imageId)
if err != nil {
return "", fmt.Errorf("Failed to check if image %s exists: %w", imageId, err)
}

if exists {
return imageId, nil
}

if err := r.imageManager.PullImage(ctx, imageId); err != nil {
return "", fmt.Errorf("Failed to pull image %s: %w", imageId, err)
}

return imageId, nil
}
42 changes: 35 additions & 7 deletions pkg/devcontainer/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,28 @@ func TestDockerRunner_Run(t *testing.T) {
wantError string
}{
{
name: "Successful run with image",
name: "Successful run with local image",
config: devcontainer.Config{
DockerImageProps: devcontainer.DockerImageProps{
Image: "test-image",
},
},
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("LocalImageExists", mock.Anything, "test-image").Return(true, nil)
mcm.On("CreateContainer", mock.Anything, "test-image", mock.Anything, mock.Anything).Return("container-id", nil)
mcm.On("StartContainer", mock.Anything, "container-id").Return(nil)
},
wantResult: "container-id",
},
{
name: "Successful run with pulled image",
config: devcontainer.Config{
DockerImageProps: devcontainer.DockerImageProps{
Image: "test-image",
},
},
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("LocalImageExists", mock.Anything, "test-image").Return(false, nil)
mim.On("PullImage", mock.Anything, "test-image").Return(nil)
mcm.On("CreateContainer", mock.Anything, "test-image", mock.Anything, mock.Anything).Return("container-id", nil)
mcm.On("StartContainer", mock.Anything, "container-id").Return(nil)
Expand Down Expand Up @@ -64,6 +79,18 @@ func TestDockerRunner_Run(t *testing.T) {
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {},
wantError: "Invalid devcontainer configuration",
},
{
name: "Failed local image check",
config: devcontainer.Config{
DockerImageProps: devcontainer.DockerImageProps{
Image: "test-image",
},
},
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("LocalImageExists", mock.Anything, "test-image").Return(false, errors.New("check error"))
},
wantError: "Failed to check if image test-image exists: check error",
},
{
name: "Failed image pull",
config: devcontainer.Config{
Expand All @@ -72,9 +99,10 @@ func TestDockerRunner_Run(t *testing.T) {
},
},
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("LocalImageExists", mock.Anything, "test-image").Return(false, nil)
mim.On("PullImage", mock.Anything, "test-image").Return(errors.New("pull error"))
},
wantError: "Failed to pull or build image: Failed to pull image: pull error",
wantError: "Failed to pull image test-image: pull error",
},
{
name: "Failed image build",
Expand All @@ -86,7 +114,7 @@ func TestDockerRunner_Run(t *testing.T) {
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("BuildImage", mock.Anything, mock.Anything, mock.Anything).Return("", errors.New("build error"))
},
wantError: "Failed to pull or build image: Failed to build image: build error",
wantError: "Failed to build image: build error",
},
{
name: "Failed container creation",
Expand All @@ -96,7 +124,7 @@ func TestDockerRunner_Run(t *testing.T) {
},
},
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("PullImage", mock.Anything, "test-image").Return(nil)
mim.On("LocalImageExists", mock.Anything, "test-image").Return(true, nil)
mcm.On("CreateContainer", mock.Anything, "test-image", mock.Anything, mock.Anything).Return("", errors.New("create error"))
},
wantError: "Failed to create container: create error",
Expand All @@ -109,7 +137,7 @@ func TestDockerRunner_Run(t *testing.T) {
},
},
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("PullImage", mock.Anything, "test-image").Return(nil)
mim.On("LocalImageExists", mock.Anything, "test-image").Return(true, nil)
mcm.On("CreateContainer", mock.Anything, "test-image", mock.Anything, mock.Anything).Return("container-id", nil)
mcm.On("StartContainer", mock.Anything, "container-id").Return(errors.New("start error"))
},
Expand All @@ -131,7 +159,7 @@ func TestDockerRunner_Run(t *testing.T) {
},
},
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("PullImage", mock.Anything, "test-image").Return(nil)
mim.On("LocalImageExists", mock.Anything, "test-image").Return(true, nil)
mcm.On("CreateContainer", mock.Anything, "test-image", mock.Anything, mock.Anything).Return("container-id", nil)
mcm.On("StartContainer", mock.Anything, "container-id").Return(nil)
me.On("Run", []string{"initialize"}, mock.Anything, mock.Anything, mock.Anything).Return(nil)
Expand All @@ -154,7 +182,7 @@ func TestDockerRunner_Run(t *testing.T) {
},
},
setupMocks: func(me *mocks.MockExecutor, mim *mocks.MockImageManager, mcm *mocks.MockContainerManager) {
mim.On("PullImage", mock.Anything, "test-image").Return(nil)
mim.On("LocalImageExists", mock.Anything, "test-image").Return(true, nil)
mcm.On("CreateContainer", mock.Anything, "test-image", mock.Anything, mock.Anything).Return("container-id", nil)
mcm.On("StartContainer", mock.Anything, "container-id").Return(nil)
mcm.On("Exec", mock.Anything, "container-id", []string{"test", "command"}).Return(devcontainer.ExecResult{ExitCode: 1, StdErr: "command failed"}, nil)
Expand Down
10 changes: 10 additions & 0 deletions pkg/jsonc/jsonc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,13 @@ func TestToJSON(t *testing.T) {
t.Fatalf("expected '%s', got '%s'", expect, out)
}
}

func TestToJSON2(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aleh-null do we still need this test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea to have test on the actual dev container. WDYT of using https://github.com/marcozac/go-jsonc in the future?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it better?

json := "{\n \"name\": \"Sample Dev Container\",\n \"image\": \"pttest:local\",\n \"settings\": {},\n \"extensions\": [],\n \"postCreateCommand\": \"echo Welcome to the dev container\",\n \"forwardPorts\": [3000],\n \"remoteUser\": \"vscode\"\n}"
expect := json

out := string(ToJSON([]byte(json)))
if out != expect {
t.Fatalf("expected '%s', got '%s'", expect, out)
}
}