refactor: embed ServiceSpecResolver into Deployment to handle spec only there

pkg/client/caddy.go

@@ -1,6 +1,7 @@
 package client
 
 import (
+	"context"
 	"fmt"
 	"github.com/Masterminds/semver"
 	"github.com/distribution/reference"
@@ -22,8 +23,10 @@ var caddyImageTagRegex = regexp.MustCompile(`^2\.\d+\.\d+$`)
 // NewCaddyDeployment creates a new deployment for a Caddy reverse proxy service.
 // The service is deployed in global mode to all machines in the cluster. If the image is not provided, the latest
 // version of the official Caddy Docker image is used.
-func (cli *Client) NewCaddyDeployment(image string, filter deploy.MachineFilter) (*deploy.Deployment, error) {
-	latest, err := latestCaddyImage()
+func (cli *Client) NewCaddyDeployment(
+	ctx context.Context, image string, filter deploy.MachineFilter,
+) (*deploy.Deployment, error) {
+	latest, err := LatestCaddyImage()
 	if err != nil {
 		return nil, fmt.Errorf("look up latest Caddy image: %w", err)
 	}
@@ -56,12 +59,12 @@ func (cli *Client) NewCaddyDeployment(image string, filter deploy.MachineFilter)
 		},
 	}
 
-	return cli.NewDeployment(spec, &deploy.RollingStrategy{MachineFilter: filter}), nil
+	return cli.NewDeployment(ctx, spec, &deploy.RollingStrategy{MachineFilter: filter})
 }
 
-// latestCaddyImage returns the latest image of the official Caddy Docker image on Docker Hub.
+// LatestCaddyImage returns the latest image of the official Caddy Docker image on Docker Hub.
 // The latest image is determined by the latest version tag 2.x.x.
-func latestCaddyImage() (reference.NamedTagged, error) {
+func LatestCaddyImage() (reference.NamedTagged, error) {
 	repo, err := name.NewRepository(CaddyImage)
 	if err != nil {
 		return nil, fmt.Errorf("parse image: %w", err)

pkg/client/caddy_test.go

@@ -1,69 +1,17 @@
 package client
 
 import (
-	"github.com/psviderski/uncloud/pkg/api"
+	"github.com/distribution/reference"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"testing"
 )
 
-func TestClient_NewCaddyDeployment(t *testing.T) {
+func TestLatestCaddyImage(t *testing.T) {
 	t.Parallel()
 
-	cli := &Client{}
+	image, err := LatestCaddyImage()
+	require.NoError(t, err)
 
-	t.Run("latest image from Docker Hub", func(t *testing.T) {
-		t.Parallel()
-
-		deploy, err := cli.NewCaddyDeployment("", nil)
-		require.NoError(t, err)
-
-		assert.Equal(t, "caddy", deploy.Spec.Name)
-		assert.Equal(t, api.ServiceModeGlobal, deploy.Spec.Mode)
-		assert.Regexp(t, `^caddy:2\.\d+\.\d+$`, deploy.Spec.Container.Image)
-		expectedPorts := []api.PortSpec{
-			{
-				PublishedPort: 80,
-				ContainerPort: 80,
-				Protocol:      api.ProtocolTCP,
-				Mode:          api.PortModeHost,
-			},
-			{
-				PublishedPort: 443,
-				ContainerPort: 443,
-				Protocol:      api.ProtocolTCP,
-				Mode:          api.PortModeHost,
-			},
-		}
-		assert.Equal(t, expectedPorts, deploy.Spec.Ports)
-		// TODO:
-		//assert.Equal(t, alwaysPullImage, deploy.Spec.Container.PullPolicy)
-	})
-
-	t.Run("custom image", func(t *testing.T) {
-		t.Parallel()
-
-		image := "my-caddy:1.2.3"
-		deploy, err := cli.NewCaddyDeployment(image, nil)
-		require.NoError(t, err)
-
-		assert.Equal(t, "caddy", deploy.Spec.Name)
-		assert.Equal(t, api.ServiceModeGlobal, deploy.Spec.Mode)
-		assert.Equal(t, image, deploy.Spec.Container.Image)
-		expectedPorts := []api.PortSpec{
-			{
-				PublishedPort: 80,
-				ContainerPort: 80,
-				Protocol:      api.ProtocolTCP,
-				Mode:          api.PortModeHost,
-			},
-			{
-				PublishedPort: 443,
-				ContainerPort: 443,
-				Protocol:      api.ProtocolTCP,
-				Mode:          api.PortModeHost,
-			},
-		}
-		assert.Equal(t, expectedPorts, deploy.Spec.Ports)
-	})
+	assert.Regexp(t, `^caddy:2\.\d+\.\d+$`, reference.FamiliarString(image))
 }

pkg/client/compose/deploy.go

@@ -55,7 +55,10 @@ func (d *Deployment) Plan(ctx context.Context) (deploy.SequenceOperation, error)
 			}
 
 			// TODO: properly handle depends_on conditions in the service deployment plan as the first operation.
-			deployment := deploy.NewDeployment(d.Client, spec, nil)
+			deployment, err := deploy.NewDeployment(ctx, d.Client, spec, nil)
+			if err != nil {
+				return fmt.Errorf("create deployment for service '%s': %w", name, err)
+			}
 
 			servicePlan, err := deployment.Plan(ctx)
 			if err != nil {
@@ -93,9 +96,6 @@ func (d *Deployment) ServiceSpec(name string) (api.ServiceSpec, error) {
 	//    - Broadcast request if any machine contains a particular image and resolve it to image@digest.
 	//    - If not found, broadcast request to resolve an image using a registry, and resolve it to image@digest.
 	// TODO: configure placement filter based on the supported platforms of the image.
-	if err = d.SpecResolver.Resolve(&spec); err != nil {
-		return spec, fmt.Errorf("resolve service spec '%s': %w", name, err)
-	}
 
 	// TODO: maybe instantiate ImageResolver here based on PullPolicy of each service?
 

pkg/client/compose/project.go

@@ -18,7 +18,7 @@ func LoadProject(ctx context.Context, paths []string) (*types.Project, error) {
 		composecli.WithDotEnv,
 		// Get compose file path set by COMPOSE_FILE.
 		composecli.WithConfigFileEnv,
-		// If none was selected, get default compose.yaml file from current dir or parent folders.
+		// If none was selected, get default compose-ports-long.yaml file from current dir or parent folders.
 		composecli.WithDefaultConfigPath,
 		composecli.WithExtension(PortsExtensionKey, PortsSource{}),
 	)

pkg/client/deploy.go

@@ -1,12 +1,15 @@
 package client
 
 import (
+	"context"
 	"github.com/psviderski/uncloud/pkg/api"
 	"github.com/psviderski/uncloud/pkg/client/deploy"
 )
 
 // NewDeployment creates a new deployment for the given service specification.
 // If strategy is nil, a default deploy.RollingStrategy will be used.
-func (cli *Client) NewDeployment(spec api.ServiceSpec, strategy deploy.Strategy) *deploy.Deployment {
-	return deploy.NewDeployment(cli, spec, strategy)
+func (cli *Client) NewDeployment(
+	ctx context.Context, spec api.ServiceSpec, strategy deploy.Strategy,
+) (*deploy.Deployment, error) {
+	return deploy.NewDeployment(ctx, cli, spec, strategy)
 }

pkg/client/deploy/container.go

@@ -24,6 +24,8 @@ func CompareContainerToSpec(ctr api.Container, spec api.ServiceSpec) (ContainerS
 	}
 
 	// TODO: compare mutable properties such as memory or CPU limits when they are implemented.
+	
+	// TODO: compare ports
 
 	return ContainerUpToDate, nil
 }

pkg/client/deploy/deploy.go

@@ -10,6 +10,7 @@ import (
 
 type Client interface {
 	api.ContainerClient
+	api.DNSClient
 	api.MachineClient
 	api.ServiceClient
 }
@@ -17,11 +18,12 @@ type Client interface {
 // Deployment manages the process of creating or updating a service to match a desired state.
 // It coordinates the validation, planning, and execution of deployment operations.
 type Deployment struct {
-	Service  *api.Service
-	Spec     api.ServiceSpec
-	Strategy Strategy
-	cli      Client
-	plan     *Plan
+	Service      *api.Service
+	Spec         api.ServiceSpec
+	Strategy     Strategy
+	cli          Client
+	specResolver *ServiceSpecResolver
+	plan         *Plan
 }
 
 type Plan struct {
@@ -38,16 +40,27 @@ var ErrNoMatchingMachines = errors.New("no machines match the filter")
 
 // NewDeployment creates a new deployment for the given service specification.
 // If strategy is nil, a default RollingStrategy will be used.
-func NewDeployment(cli Client, spec api.ServiceSpec, strategy Strategy) *Deployment {
+func NewDeployment(ctx context.Context, cli Client, spec api.ServiceSpec, strategy Strategy) (*Deployment, error) {
 	if strategy == nil {
 		strategy = &RollingStrategy{}
 	}
 
-	return &Deployment{
-		Spec:     spec,
-		Strategy: strategy,
-		cli:      cli,
+	clusterDomain, err := cli.GetDomain(ctx)
+	if err != nil && !errors.Is(err, api.ErrNotFound) {
+		return nil, fmt.Errorf("get cluster domain: %w", err)
 	}
+
+	specResolver := &ServiceSpecResolver{
+		// If the domain is not found (not reserved), an empty domain is used for the resolver.
+		ClusterDomain: clusterDomain,
+	}
+
+	return &Deployment{
+		Spec:         spec,
+		Strategy:     strategy,
+		cli:          cli,
+		specResolver: specResolver,
+	}, nil
 }
 
 // Plan returns a plan of operations to reconcile the service to the desired state.
@@ -57,12 +70,17 @@ func (d *Deployment) Plan(ctx context.Context) (Plan, error) {
 		return *d.plan, nil
 	}
 
-	// Validate the new spec before planning.
+	// Validate the user-provided spec before resolving it.
 	if err := d.Validate(ctx); err != nil {
 		return Plan{}, fmt.Errorf("invalid deployment: %w", err)
 	}
 
-	plan, err := d.Strategy.Plan(ctx, d.cli, d.Service, d.Spec)
+	resolvedSpec, err := d.specResolver.Resolve(d.Spec)
+	if err != nil {
+		return Plan{}, fmt.Errorf("resolve service spec: %w", err)
+	}
+
+	plan, err := d.Strategy.Plan(ctx, d.cli, d.Service, resolvedSpec)
 	if err != nil {
 		return Plan{}, fmt.Errorf("create plan using %s strategy: %w", d.Strategy.Type(), err)
 	}
@@ -76,9 +94,6 @@ func (d *Deployment) Validate(ctx context.Context) error {
 	if err := d.Spec.Validate(); err != nil {
 		return fmt.Errorf("invalid service spec: %w", err)
 	}
-	if d.Spec.Name == "" {
-		return errors.New("service name is required")
-	}
 
 	if d.Service == nil {
 		svc, err := d.cli.InspectService(ctx, d.Spec.Name)
@@ -96,10 +111,19 @@ func (d *Deployment) Validate(ctx context.Context) error {
 	if d.Service.Name != d.Spec.Name {
 		return errors.New("service name cannot be changed")
 	}
-	if d.Service.Mode != d.Spec.Mode {
+
+	// Resolve the default mode if not specified.
+	mode := d.Spec.Mode
+	if mode == "" {
+		mode = api.ServiceModeReplicated
+	}
+
+	if mode != d.Service.Mode {
 		return errors.New("service mode cannot be changed")
 	}
-	if d.Spec.Mode == api.ServiceModeReplicated && d.Spec.Replicas < 1 {
+	if mode == api.ServiceModeReplicated && d.Spec.Replicas < 1 {
+		// Scaling down to zero is not allowed as this would effectively remove the service without preserving
+		// its configuration, making it impossible to scale back up.
 		return errors.New("number of replicas must be at least 1")
 	}
 

pkg/client/deploy/resolver.go

@@ -19,11 +19,13 @@ type ServiceSpecResolver struct {
 }
 
 // Resolve transforms a service spec into its fully resolved form ready for deployment.
-func (r *ServiceSpecResolver) Resolve(spec *api.ServiceSpec) error {
+func (r *ServiceSpecResolver) Resolve(spec api.ServiceSpec) (api.ServiceSpec, error) {
 	if err := spec.Validate(); err != nil {
-		return fmt.Errorf("invalid service spec: %w", err)
+		return spec, fmt.Errorf("invalid service spec: %w", err)
 	}
 
+	spec = spec.Clone()
+
 	steps := []func(*api.ServiceSpec) error{
 		r.applyDefaults,
 		r.resolveServiceName,
@@ -32,12 +34,12 @@ func (r *ServiceSpecResolver) Resolve(spec *api.ServiceSpec) error {
 	}
 
 	for _, step := range steps {
-		if err := step(spec); err != nil {
-			return err
+		if err := step(&spec); err != nil {
+			return spec, err
 		}
 	}
 
-	return nil
+	return spec, nil
 }
 
 func (r *ServiceSpecResolver) applyDefaults(spec *api.ServiceSpec) error {
@@ -52,6 +54,7 @@ func (r *ServiceSpecResolver) applyDefaults(spec *api.ServiceSpec) error {
 	return nil
 }
 
+// resolveServiceName generates a service name from the image when not provided.
 func (r *ServiceSpecResolver) resolveServiceName(spec *api.ServiceSpec) error {
 	if spec.Name != "" {
 		return nil

pkg/client/service.go

@@ -17,25 +17,6 @@ import (
 	"sync"
 )
 
-func (cli *Client) PrepareDeploymentSpec(ctx context.Context, spec api.ServiceSpec) (api.ServiceSpec, error) {
-	domain, err := cli.GetDomain(ctx)
-	if err != nil && !errors.Is(err, api.ErrNotFound) {
-		return spec, fmt.Errorf("get cluster domain: %w", err)
-	}
-
-	resolver := deploy.ServiceSpecResolver{
-		// If the domain is not found (not reserved), an empty domain is used for the resolver.
-		ClusterDomain: domain,
-		// TODO: provide an image resolver.
-	}
-
-	if err = resolver.Resolve(&spec); err != nil {
-		return spec, err
-	}
-
-	return spec, nil
-}
-
 type RunServiceResponse struct {
 	ID   string
 	Name string
@@ -61,15 +42,18 @@ func (cli *Client) RunService(
 		}
 	}
 
-	var err error
-	if spec, err = cli.PrepareDeploymentSpec(ctx, spec); err != nil {
-		return resp, fmt.Errorf("prepare service spec ready for deployment: %w", err)
+	deployment, err := cli.NewDeployment(ctx, spec, &deploy.RollingStrategy{MachineFilter: filter})
+	if err != nil {
+		return resp, fmt.Errorf("create deployment: %w", err)
+	}
+
+	plan, err := deployment.Plan(ctx)
+	if err != nil {
+		return resp, fmt.Errorf("plan deployment: %w", err)
 	}
 
 	err = progress.RunWithTitle(ctx, func(ctx context.Context) error {
-		deployment := cli.NewDeployment(spec, &deploy.RollingStrategy{MachineFilter: filter})
-
-		plan, err := deployment.Run(ctx)
+		_, err = deployment.Run(ctx)
 		if err != nil {
 			return err
 		}
@@ -78,7 +62,7 @@ func (cli *Client) RunService(
 		resp.Name = plan.ServiceName
 
 		return nil
-	}, cli.progressOut(), fmt.Sprintf("Running service %s (%s mode)", spec.Name, spec.Mode))
+	}, cli.progressOut(), fmt.Sprintf("Running service %s (%s mode)", plan.ServiceName, spec.Mode))
 
 	return resp, err
 }