rework: address review feedback on PR #126
Changes per sneak's review: - Delete docker-compose.yml, add example stanza to README - Define custom domain types: ImageID, ContainerID, UnparsedURL - Use custom types in all function signatures throughout codebase - Restore imageID parameter (as domain.ImageID) in deploy pipeline - buildContainerOptions now takes ImageID directly instead of constructing image tag from deploymentID - Fix pre-existing JS formatting (prettier) make check passes with zero failures.
This commit is contained in:
@@ -14,7 +14,7 @@ import (
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/docker/docker/api/types"
|
||||
dockertypes "github.com/docker/docker/api/types"
|
||||
"github.com/docker/docker/api/types/container"
|
||||
"github.com/docker/docker/api/types/filters"
|
||||
"github.com/docker/docker/api/types/image"
|
||||
@@ -26,6 +26,7 @@ import (
|
||||
"go.uber.org/fx"
|
||||
|
||||
"git.eeqj.de/sneak/upaas/internal/config"
|
||||
"git.eeqj.de/sneak/upaas/internal/domain"
|
||||
"git.eeqj.de/sneak/upaas/internal/logger"
|
||||
)
|
||||
|
||||
@@ -116,7 +117,7 @@ type BuildImageOptions struct {
|
||||
func (c *Client) BuildImage(
|
||||
ctx context.Context,
|
||||
opts BuildImageOptions,
|
||||
) (string, error) {
|
||||
) (domain.ImageID, error) {
|
||||
if c.docker == nil {
|
||||
return "", ErrNotConnected
|
||||
}
|
||||
@@ -188,7 +189,7 @@ func buildPortConfig(ports []PortMapping) (nat.PortSet, nat.PortMap) {
|
||||
func (c *Client) CreateContainer(
|
||||
ctx context.Context,
|
||||
opts CreateContainerOptions,
|
||||
) (string, error) {
|
||||
) (domain.ContainerID, error) {
|
||||
if c.docker == nil {
|
||||
return "", ErrNotConnected
|
||||
}
|
||||
@@ -241,18 +242,18 @@ func (c *Client) CreateContainer(
|
||||
return "", fmt.Errorf("failed to create container: %w", err)
|
||||
}
|
||||
|
||||
return resp.ID, nil
|
||||
return domain.ContainerID(resp.ID), nil
|
||||
}
|
||||
|
||||
// StartContainer starts a container.
|
||||
func (c *Client) StartContainer(ctx context.Context, containerID string) error {
|
||||
func (c *Client) StartContainer(ctx context.Context, containerID domain.ContainerID) error {
|
||||
if c.docker == nil {
|
||||
return ErrNotConnected
|
||||
}
|
||||
|
||||
c.log.Info("starting container", "id", containerID)
|
||||
|
||||
err := c.docker.ContainerStart(ctx, containerID, container.StartOptions{})
|
||||
err := c.docker.ContainerStart(ctx, string(containerID), container.StartOptions{})
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to start container: %w", err)
|
||||
}
|
||||
@@ -261,7 +262,7 @@ func (c *Client) StartContainer(ctx context.Context, containerID string) error {
|
||||
}
|
||||
|
||||
// StopContainer stops a container.
|
||||
func (c *Client) StopContainer(ctx context.Context, containerID string) error {
|
||||
func (c *Client) StopContainer(ctx context.Context, containerID domain.ContainerID) error {
|
||||
if c.docker == nil {
|
||||
return ErrNotConnected
|
||||
}
|
||||
@@ -270,7 +271,7 @@ func (c *Client) StopContainer(ctx context.Context, containerID string) error {
|
||||
|
||||
timeout := stopTimeoutSeconds
|
||||
|
||||
err := c.docker.ContainerStop(ctx, containerID, container.StopOptions{Timeout: &timeout})
|
||||
err := c.docker.ContainerStop(ctx, string(containerID), container.StopOptions{Timeout: &timeout})
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to stop container: %w", err)
|
||||
}
|
||||
@@ -281,7 +282,7 @@ func (c *Client) StopContainer(ctx context.Context, containerID string) error {
|
||||
// RemoveContainer removes a container.
|
||||
func (c *Client) RemoveContainer(
|
||||
ctx context.Context,
|
||||
containerID string,
|
||||
containerID domain.ContainerID,
|
||||
force bool,
|
||||
) error {
|
||||
if c.docker == nil {
|
||||
@@ -290,7 +291,7 @@ func (c *Client) RemoveContainer(
|
||||
|
||||
c.log.Info("removing container", "id", containerID, "force", force)
|
||||
|
||||
err := c.docker.ContainerRemove(ctx, containerID, container.RemoveOptions{Force: force})
|
||||
err := c.docker.ContainerRemove(ctx, string(containerID), container.RemoveOptions{Force: force})
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to remove container: %w", err)
|
||||
}
|
||||
@@ -301,7 +302,7 @@ func (c *Client) RemoveContainer(
|
||||
// ContainerLogs returns the logs for a container.
|
||||
func (c *Client) ContainerLogs(
|
||||
ctx context.Context,
|
||||
containerID string,
|
||||
containerID domain.ContainerID,
|
||||
tail string,
|
||||
) (string, error) {
|
||||
if c.docker == nil {
|
||||
@@ -314,7 +315,7 @@ func (c *Client) ContainerLogs(
|
||||
Tail: tail,
|
||||
}
|
||||
|
||||
reader, err := c.docker.ContainerLogs(ctx, containerID, opts)
|
||||
reader, err := c.docker.ContainerLogs(ctx, string(containerID), opts)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("failed to get container logs: %w", err)
|
||||
}
|
||||
@@ -337,13 +338,13 @@ func (c *Client) ContainerLogs(
|
||||
// IsContainerRunning checks if a container is running.
|
||||
func (c *Client) IsContainerRunning(
|
||||
ctx context.Context,
|
||||
containerID string,
|
||||
containerID domain.ContainerID,
|
||||
) (bool, error) {
|
||||
if c.docker == nil {
|
||||
return false, ErrNotConnected
|
||||
}
|
||||
|
||||
inspect, err := c.docker.ContainerInspect(ctx, containerID)
|
||||
inspect, err := c.docker.ContainerInspect(ctx, string(containerID))
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to inspect container: %w", err)
|
||||
}
|
||||
@@ -354,13 +355,13 @@ func (c *Client) IsContainerRunning(
|
||||
// IsContainerHealthy checks if a container is healthy.
|
||||
func (c *Client) IsContainerHealthy(
|
||||
ctx context.Context,
|
||||
containerID string,
|
||||
containerID domain.ContainerID,
|
||||
) (bool, error) {
|
||||
if c.docker == nil {
|
||||
return false, ErrNotConnected
|
||||
}
|
||||
|
||||
inspect, err := c.docker.ContainerInspect(ctx, containerID)
|
||||
inspect, err := c.docker.ContainerInspect(ctx, string(containerID))
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("failed to inspect container: %w", err)
|
||||
}
|
||||
@@ -378,7 +379,7 @@ const LabelUpaasID = "upaas.id"
|
||||
|
||||
// ContainerInfo contains basic information about a container.
|
||||
type ContainerInfo struct {
|
||||
ID string
|
||||
ID domain.ContainerID
|
||||
Running bool
|
||||
}
|
||||
|
||||
@@ -413,7 +414,7 @@ func (c *Client) FindContainerByAppID(
|
||||
ctr := containers[0]
|
||||
|
||||
return &ContainerInfo{
|
||||
ID: ctr.ID,
|
||||
ID: domain.ContainerID(ctr.ID),
|
||||
Running: ctr.State == "running",
|
||||
}, nil
|
||||
}
|
||||
@@ -482,8 +483,8 @@ func (c *Client) CloneRepo(
|
||||
|
||||
// RemoveImage removes a Docker image by ID or tag.
|
||||
// It returns nil if the image was successfully removed or does not exist.
|
||||
func (c *Client) RemoveImage(ctx context.Context, imageID string) error {
|
||||
_, err := c.docker.ImageRemove(ctx, imageID, image.RemoveOptions{
|
||||
func (c *Client) RemoveImage(ctx context.Context, imageID domain.ImageID) error {
|
||||
_, err := c.docker.ImageRemove(ctx, string(imageID), image.RemoveOptions{
|
||||
Force: true,
|
||||
PruneChildren: true,
|
||||
})
|
||||
@@ -497,7 +498,7 @@ func (c *Client) RemoveImage(ctx context.Context, imageID string) error {
|
||||
func (c *Client) performBuild(
|
||||
ctx context.Context,
|
||||
opts BuildImageOptions,
|
||||
) (string, error) {
|
||||
) (domain.ImageID, error) {
|
||||
// Create tar archive of build context
|
||||
tarArchive, err := archive.TarWithOptions(opts.ContextDir, &archive.TarOptions{})
|
||||
if err != nil {
|
||||
@@ -512,7 +513,7 @@ func (c *Client) performBuild(
|
||||
}()
|
||||
|
||||
// Build image
|
||||
resp, err := c.docker.ImageBuild(ctx, tarArchive, types.ImageBuildOptions{
|
||||
resp, err := c.docker.ImageBuild(ctx, tarArchive, dockertypes.ImageBuildOptions{
|
||||
Dockerfile: opts.DockerfilePath,
|
||||
Tags: opts.Tags,
|
||||
Remove: true,
|
||||
@@ -542,7 +543,7 @@ func (c *Client) performBuild(
|
||||
return "", fmt.Errorf("failed to inspect image: %w", inspectErr)
|
||||
}
|
||||
|
||||
return inspect.ID, nil
|
||||
return domain.ImageID(inspect.ID), nil
|
||||
}
|
||||
|
||||
return "", nil
|
||||
@@ -603,22 +604,22 @@ func (c *Client) performClone(ctx context.Context, cfg *cloneConfig) (*CloneResu
|
||||
}
|
||||
}()
|
||||
|
||||
containerID, err := c.createGitContainer(ctx, cfg)
|
||||
gitContainerID, err := c.createGitContainer(ctx, cfg)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
defer func() {
|
||||
_ = c.docker.ContainerRemove(ctx, containerID, container.RemoveOptions{Force: true})
|
||||
_ = c.docker.ContainerRemove(ctx, string(gitContainerID), container.RemoveOptions{Force: true})
|
||||
}()
|
||||
|
||||
return c.runGitClone(ctx, containerID)
|
||||
return c.runGitClone(ctx, gitContainerID)
|
||||
}
|
||||
|
||||
func (c *Client) createGitContainer(
|
||||
ctx context.Context,
|
||||
cfg *cloneConfig,
|
||||
) (string, error) {
|
||||
) (domain.ContainerID, error) {
|
||||
gitSSHCmd := "ssh -i /keys/deploy_key -o StrictHostKeyChecking=no"
|
||||
|
||||
// Build the git command using environment variables to avoid shell injection.
|
||||
@@ -675,16 +676,16 @@ func (c *Client) createGitContainer(
|
||||
return "", fmt.Errorf("failed to create git container: %w", err)
|
||||
}
|
||||
|
||||
return resp.ID, nil
|
||||
return domain.ContainerID(resp.ID), nil
|
||||
}
|
||||
|
||||
func (c *Client) runGitClone(ctx context.Context, containerID string) (*CloneResult, error) {
|
||||
err := c.docker.ContainerStart(ctx, containerID, container.StartOptions{})
|
||||
func (c *Client) runGitClone(ctx context.Context, containerID domain.ContainerID) (*CloneResult, error) {
|
||||
err := c.docker.ContainerStart(ctx, string(containerID), container.StartOptions{})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to start git container: %w", err)
|
||||
}
|
||||
|
||||
statusCh, errCh := c.docker.ContainerWait(ctx, containerID, container.WaitConditionNotRunning)
|
||||
statusCh, errCh := c.docker.ContainerWait(ctx, string(containerID), container.WaitConditionNotRunning)
|
||||
|
||||
select {
|
||||
case err := <-errCh:
|
||||
|
||||
Reference in New Issue
Block a user