Merge pull request #12 from stacklok/separate-interfaces

refactor: Separate interfaces from implementation for container runtime

Author: JAORMX

cmd/vt/list.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/stacklok/vibetool/pkg/client"
 	"github.com/stacklok/vibetool/pkg/container"
+	rt "github.com/stacklok/vibetool/pkg/container/runtime"
 	"github.com/stacklok/vibetool/pkg/labels"
 )
 
@@ -62,7 +63,7 @@ func listCmdFunc(_ *cobra.Command, _ []string) error {
 	}
 
 	// Filter containers to only show those managed by Vibe Tool
-	var vibeToolContainers []container.ContainerInfo
+	var vibeToolContainers []rt.ContainerInfo
 	for _, c := range containers {
 		if labels.IsVibeToolContainer(c.Labels) {
 			vibeToolContainers = append(vibeToolContainers, c)
@@ -71,7 +72,7 @@ func listCmdFunc(_ *cobra.Command, _ []string) error {
 
 	// Filter containers if not showing all
 	if !listAll {
-		var runningContainers []container.ContainerInfo
+		var runningContainers []rt.ContainerInfo
 		for _, c := range vibeToolContainers {
 			if c.State == "running" {
 				runningContainers = append(runningContainers, c)
@@ -97,7 +98,7 @@ func listCmdFunc(_ *cobra.Command, _ []string) error {
 }
 
 // printJSONOutput prints container information in JSON format
-func printJSONOutput(containers []container.ContainerInfo) error {
+func printJSONOutput(containers []rt.ContainerInfo) error {
 	var output []ContainerOutput
 
 	for _, c := range containers {
@@ -155,7 +156,7 @@ func printJSONOutput(containers []container.ContainerInfo) error {
 }
 
 // printTextOutput prints container information in text format
-func printTextOutput(containers []container.ContainerInfo) {
+func printTextOutput(containers []rt.ContainerInfo) {
 	// Create a tabwriter for pretty output
 	w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0)
 	fmt.Fprintln(w, "CONTAINER ID\tNAME\tIMAGE\tSTATE\tTRANSPORT\tPORT\tURL")

cmd/vt/stop.go

@@ -8,6 +8,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/stacklok/vibetool/pkg/container"
+	rt "github.com/stacklok/vibetool/pkg/container/runtime"
 	"github.com/stacklok/vibetool/pkg/labels"
 	"github.com/stacklok/vibetool/pkg/process"
 )
@@ -29,7 +30,7 @@ func init() {
 }
 
 // findContainerID finds the container ID by name or ID prefix
-func findContainerID(ctx context.Context, runtime container.Runtime, containerName string) (string, error) {
+func findContainerID(ctx context.Context, runtime rt.Runtime, containerName string) (string, error) {
 	// List containers to find the one with the given name
 	containers, err := runtime.ListContainers(ctx)
 	if err != nil {
@@ -59,7 +60,7 @@ func findContainerID(ctx context.Context, runtime container.Runtime, containerNa
 }
 
 // getContainerBaseName gets the base container name from the container labels
-func getContainerBaseName(ctx context.Context, runtime container.Runtime, containerID string) (string, error) {
+func getContainerBaseName(ctx context.Context, runtime rt.Runtime, containerID string) (string, error) {
 	containers, err := runtime.ListContainers(ctx)
 	if err != nil {
 		return "", fmt.Errorf("failed to list containers: %v", err)
@@ -103,7 +104,7 @@ func stopProxyProcess(containerBaseName string) {
 }
 
 // stopContainer stops the container
-func stopContainer(ctx context.Context, runtime container.Runtime, containerID, containerName string) error {
+func stopContainer(ctx context.Context, runtime rt.Runtime, containerID, containerName string) error {
 	fmt.Printf("Stopping container %s...\n", containerName)
 	if err := runtime.StopContainer(ctx, containerID); err != nil {
 		return fmt.Errorf("failed to stop container: %v", err)

pkg/container/client.go pkg/container/docker/client.gopkg/container/client.go renamed to pkg/container/docker/client.go

@@ -1,6 +1,6 @@
-// Package container provides utilities for managing containers,
+// Package docker provides Docker-specific implementation of container runtime,
 // including creating, starting, stopping, and monitoring containers.
-package container
+package docker
 
 import (
 	"context"
@@ -21,6 +21,7 @@ import (
 	"github.com/docker/docker/client"
 	"github.com/docker/go-connections/nat"
 
+	"github.com/stacklok/vibetool/pkg/container/runtime"
 	"github.com/stacklok/vibetool/pkg/permissions"
 )
 
@@ -36,7 +37,7 @@ const (
 
 // Client implements the Runtime interface for container operations
 type Client struct {
-	runtimeType RuntimeType
+	runtimeType runtime.Type
 	socketPath  string
 	client      *client.Client
 }
@@ -53,7 +54,7 @@ func NewClient(ctx context.Context) (*Client, error) {
 }
 
 // NewClientWithSocketPath creates a new container client with a specific socket path
-func NewClientWithSocketPath(ctx context.Context, socketPath string, runtimeType RuntimeType) (*Client, error) {
+func NewClientWithSocketPath(ctx context.Context, socketPath string, runtimeType runtime.Type) (*Client, error) {
 	// Create a custom HTTP client that uses the Unix socket
 	httpClient := &http.Client{
 		Transport: &http.Transport{
@@ -99,32 +100,32 @@ func (c *Client) ping(ctx context.Context) error {
 }
 
 // findContainerSocket finds a container socket path, preferring Podman over Docker
-func findContainerSocket() (string, RuntimeType, error) {
+func findContainerSocket() (string, runtime.Type, error) {
 	// Try Podman sockets first
 	// Check standard Podman location
 	if _, err := os.Stat(PodmanSocketPath); err == nil {
-		return PodmanSocketPath, RuntimeTypePodman, nil
+		return PodmanSocketPath, runtime.TypePodman, nil
 	}
 
 	// Check XDG_RUNTIME_DIR location for Podman
 	if xdgRuntimeDir := os.Getenv("XDG_RUNTIME_DIR"); xdgRuntimeDir != "" {
 		xdgSocketPath := filepath.Join(xdgRuntimeDir, PodmanXDGRuntimeSocketPath)
 		if _, err := os.Stat(xdgSocketPath); err == nil {
-			return xdgSocketPath, RuntimeTypePodman, nil
+			return xdgSocketPath, runtime.TypePodman, nil
 		}
 	}
 
 	// Check user-specific location for Podman
 	if home := os.Getenv("HOME"); home != "" {
 		userSocketPath := filepath.Join(home, ".local/share/containers/podman/machine/podman.sock")
 		if _, err := os.Stat(userSocketPath); err == nil {
-			return userSocketPath, RuntimeTypePodman, nil
+			return userSocketPath, runtime.TypePodman, nil
 		}
 	}
 
 	// Try Docker socket as fallback
 	if _, err := os.Stat(DockerSocketPath); err == nil {
-		return DockerSocketPath, RuntimeTypeDocker, nil
+		return DockerSocketPath, runtime.TypeDocker, nil
 	}
 
 	return "", "", ErrRuntimeNotFound
@@ -142,7 +143,7 @@ func convertEnvVars(envVars map[string]string) []string {
 }
 
 // convertMounts converts internal mount format to Docker mount format
-func convertMounts(mounts []Mount) []mount.Mount {
+func convertMounts(mounts []runtime.Mount) []mount.Mount {
 	result := make([]mount.Mount, 0, len(mounts))
 	for _, m := range mounts {
 		result = append(result, mount.Mount{
@@ -174,7 +175,7 @@ func setupExposedPorts(config *container.Config, exposedPorts map[string]struct{
 }
 
 // setupPortBindings configures port bindings for a container
-func setupPortBindings(hostConfig *container.HostConfig, portBindings map[string][]PortBinding) error {
+func setupPortBindings(hostConfig *container.HostConfig, portBindings map[string][]runtime.PortBinding) error {
 	if len(portBindings) == 0 {
 		return nil
 	}
@@ -209,7 +210,7 @@ func (c *Client) CreateContainer(
 	envVars, labels map[string]string,
 	permissionProfile *permissions.Profile,
 	transportType string,
-	options *CreateContainerOptions,
+	options *runtime.CreateContainerOptions,
 ) (string, error) {
 	// Get permission config from profile
 	permissionConfig, err := c.getPermissionConfigFromProfile(permissionProfile, transportType)
@@ -281,7 +282,7 @@ func (c *Client) StartContainer(ctx context.Context, containerID string) error {
 }
 
 // ListContainers lists containers
-func (c *Client) ListContainers(ctx context.Context) ([]ContainerInfo, error) {
+func (c *Client) ListContainers(ctx context.Context) ([]runtime.ContainerInfo, error) {
 	// Create filter for vibetool containers
 	filterArgs := filters.NewArgs()
 	filterArgs.Add("label", "vibetool=true")
@@ -296,7 +297,7 @@ func (c *Client) ListContainers(ctx context.Context) ([]ContainerInfo, error) {
 	}
 
 	// Convert to our ContainerInfo format
-	result := make([]ContainerInfo, 0, len(containers))
+	result := make([]runtime.ContainerInfo, 0, len(containers))
 	for _, c := range containers {
 		// Extract container name (remove leading slash)
 		name := ""
@@ -306,9 +307,9 @@ func (c *Client) ListContainers(ctx context.Context) ([]ContainerInfo, error) {
 		}
 
 		// Extract port mappings
-		ports := make([]PortMapping, 0, len(c.Ports))
+		ports := make([]runtime.PortMapping, 0, len(c.Ports))
 		for _, p := range c.Ports {
-			ports = append(ports, PortMapping{
+			ports = append(ports, runtime.PortMapping{
 				ContainerPort: int(p.PrivatePort),
 				HostPort:      int(p.PublicPort),
 				Protocol:      p.Type,
@@ -318,7 +319,7 @@ func (c *Client) ListContainers(ctx context.Context) ([]ContainerInfo, error) {
 		// Convert creation time
 		created := time.Unix(c.Created, 0)
 
-		result = append(result, ContainerInfo{
+		result = append(result, runtime.ContainerInfo{
 			ID:      c.ID,
 			Name:    name,
 			Image:   c.Image,
@@ -394,19 +395,19 @@ func (c *Client) IsContainerRunning(ctx context.Context, containerID string) (bo
 }
 
 // GetContainerInfo gets container information
-func (c *Client) GetContainerInfo(ctx context.Context, containerID string) (ContainerInfo, error) {
+func (c *Client) GetContainerInfo(ctx context.Context, containerID string) (runtime.ContainerInfo, error) {
 	// Inspect container
 	info, err := c.client.ContainerInspect(ctx, containerID)
 	if err != nil {
 		// Check if the error is because the container doesn't exist
 		if client.IsErrNotFound(err) {
-			return ContainerInfo{}, NewContainerError(ErrContainerNotFound, containerID, "container not found")
+			return runtime.ContainerInfo{}, NewContainerError(ErrContainerNotFound, containerID, "container not found")
 		}
-		return ContainerInfo{}, NewContainerError(err, containerID, fmt.Sprintf("failed to inspect container: %v", err))
+		return runtime.ContainerInfo{}, NewContainerError(err, containerID, fmt.Sprintf("failed to inspect container: %v", err))
 	}
 
 	// Extract port mappings
-	ports := make([]PortMapping, 0)
+	ports := make([]runtime.PortMapping, 0)
 	for containerPort, bindings := range info.NetworkSettings.Ports {
 		for _, binding := range bindings {
 			hostPort := 0
@@ -415,7 +416,7 @@ func (c *Client) GetContainerInfo(ctx context.Context, containerID string) (Cont
 				fmt.Printf("Warning: Failed to parse host port %s: %v\n", binding.HostPort, err)
 			}
 
-			ports = append(ports, PortMapping{
+			ports = append(ports, runtime.PortMapping{
 				ContainerPort: containerPort.Int(),
 				HostPort:      hostPort,
 				Protocol:      containerPort.Proto(),
@@ -429,7 +430,7 @@ func (c *Client) GetContainerInfo(ctx context.Context, containerID string) (Cont
 		created = time.Time{} // Use zero time if parsing fails
 	}
 
-	return ContainerInfo{
+	return runtime.ContainerInfo{
 		ID:      info.ID,
 		Name:    strings.TrimPrefix(info.Name, "/"),
 		Image:   info.Config.Image,
@@ -544,7 +545,7 @@ func (c *Client) PullImage(ctx context.Context, imageName string) error {
 // getPermissionConfigFromProfile converts a permission profile to a container permission config
 // with transport-specific settings (internal function)
 // addReadOnlyMounts adds read-only mounts to the permission config
-func (*Client) addReadOnlyMounts(config *PermissionConfig, mounts []permissions.MountDeclaration) {
+func (*Client) addReadOnlyMounts(config *runtime.PermissionConfig, mounts []permissions.MountDeclaration) {
 	for _, mountDecl := range mounts {
 		source, target, err := mountDecl.Parse()
 		if err != nil {
@@ -565,7 +566,7 @@ func (*Client) addReadOnlyMounts(config *PermissionConfig, mounts []permissions.
 			continue
 		}
 
-		config.Mounts = append(config.Mounts, Mount{
+		config.Mounts = append(config.Mounts, runtime.Mount{
 			Source:   source,
 			Target:   target,
 			ReadOnly: true,
@@ -574,7 +575,7 @@ func (*Client) addReadOnlyMounts(config *PermissionConfig, mounts []permissions.
 }
 
 // addReadWriteMounts adds read-write mounts to the permission config
-func (*Client) addReadWriteMounts(config *PermissionConfig, mounts []permissions.MountDeclaration) {
+func (*Client) addReadWriteMounts(config *runtime.PermissionConfig, mounts []permissions.MountDeclaration) {
 	for _, mountDecl := range mounts {
 		source, target, err := mountDecl.Parse()
 		if err != nil {
@@ -608,7 +609,7 @@ func (*Client) addReadWriteMounts(config *PermissionConfig, mounts []permissions
 
 		// If not already mounted, add a new mount
 		if !alreadyMounted {
-			config.Mounts = append(config.Mounts, Mount{
+			config.Mounts = append(config.Mounts, runtime.Mount{
 				Source:   source,
 				Target:   target,
 				ReadOnly: false,
@@ -640,10 +641,14 @@ func (*Client) needsNetworkAccess(profile *permissions.Profile, transportType st
 	return false
 }
 
-func (c *Client) getPermissionConfigFromProfile(profile *permissions.Profile, transportType string) (*PermissionConfig, error) {
+// getPermissionConfigFromProfile converts a permission profile to a container permission config
+func (c *Client) getPermissionConfigFromProfile(
+	profile *permissions.Profile,
+	transportType string,
+) (*runtime.PermissionConfig, error) {
 	// Start with a default permission config
-	config := &PermissionConfig{
-		Mounts:      []Mount{},
+	config := &runtime.PermissionConfig{
+		Mounts:      []runtime.Mount{},
 		NetworkMode: "none",
 		CapDrop:     []string{"ALL"},
 		CapAdd:      []string{},
@@ -666,3 +671,70 @@ func (c *Client) getPermissionConfigFromProfile(profile *permissions.Profile, tr
 
 	return config, nil
 }
+
+// Error types for container operations
+var (
+	// ErrContainerNotFound is returned when a container is not found
+	ErrContainerNotFound = fmt.Errorf("container not found")
+
+	// ErrContainerAlreadyExists is returned when a container already exists
+	ErrContainerAlreadyExists = fmt.Errorf("container already exists")
+
+	// ErrContainerNotRunning is returned when a container is not running
+	ErrContainerNotRunning = fmt.Errorf("container not running")
+
+	// ErrContainerAlreadyRunning is returned when a container is already running
+	ErrContainerAlreadyRunning = fmt.Errorf("container already running")
+
+	// ErrRuntimeNotFound is returned when a container runtime is not found
+	ErrRuntimeNotFound = fmt.Errorf("container runtime not found")
+
+	// ErrInvalidRuntimeType is returned when an invalid runtime type is specified
+	ErrInvalidRuntimeType = fmt.Errorf("invalid runtime type")
+
+	// ErrAttachFailed is returned when attaching to a container fails
+	ErrAttachFailed = fmt.Errorf("failed to attach to container")
+
+	// ErrContainerExited is returned when a container has exited unexpectedly
+	ErrContainerExited = fmt.Errorf("container exited unexpectedly")
+)
+
+// ContainerError represents an error related to container operations
+type ContainerError struct {
+	// Err is the underlying error
+	Err error
+	// ContainerID is the ID of the container
+	ContainerID string
+	// Message is an optional error message
+	Message string
+}
+
+// Error returns the error message
+func (e *ContainerError) Error() string {
+	if e.Message != "" {
+		if e.ContainerID != "" {
+			return fmt.Sprintf("%s: %s (container: %s)", e.Err, e.Message, e.ContainerID)
+		}
+		return fmt.Sprintf("%s: %s", e.Err, e.Message)
+	}
+
+	if e.ContainerID != "" {
+		return fmt.Sprintf("%s (container: %s)", e.Err, e.ContainerID)
+	}
+
+	return e.Err.Error()
+}
+
+// Unwrap returns the underlying error
+func (e *ContainerError) Unwrap() error {
+	return e.Err
+}
+
+// NewContainerError creates a new container error
+func NewContainerError(err error, containerID, message string) *ContainerError {
+	return &ContainerError{
+		Err:         err,
+		ContainerID: containerID,
+		Message:     message,
+	}
+}