refactor: update deployment plan structure to use typed volume and service operations

cmd/uncloud/deploy.go

@@ -14,7 +14,6 @@ import (
 	"github.com/psviderski/uncloud/pkg/client"
 	"github.com/psviderski/uncloud/pkg/client/compose"
 	"github.com/psviderski/uncloud/pkg/client/deploy"
-	"github.com/psviderski/uncloud/pkg/client/deploy/operation"
 	"github.com/spf13/cobra"
 )
 
@@ -170,7 +169,7 @@ func runDeploy(ctx context.Context, uncli *cli.CLI, opts deployOptions) error {
 		return fmt.Errorf("plan deployment: %w", err)
 	}
 
-	if len(plan.Operations) == 0 {
+	if plan.IsEmpty() {
 		fmt.Println("Services are up to date.")
 		return nil
 	}
@@ -206,14 +205,12 @@ func runDeploy(ctx context.Context, uncli *cli.CLI, opts deployOptions) error {
 	}, uncli.ProgressOut(), "Deploying services")
 }
 
-func printPlan(ctx context.Context, cli *client.Client, plan operation.SequenceOperation) error {
+func printPlan(ctx context.Context, cli *client.Client, plan compose.Plan) error {
-	for _, op := range plan.Operations {
+	for _, op := range plan.Volumes {
-		svcPlan, ok := op.(*deploy.Plan)
-		if !ok {
 		fmt.Println("- " + op.Format(nil))
-			continue
 	}
 
+	for _, svcPlan := range plan.Services {
 		svc, err := cli.InspectService(ctx, svcPlan.ServiceID)
 		if err != nil && !errors.Is(err, api.ErrNotFound) {
 			return fmt.Errorf("inspect service: %w", err)

pkg/client/compose/deploy.go

@@ -28,7 +28,33 @@ type Deployment struct {
 	SpecResolver *deploy.ServiceSpecResolver
 	Strategy     deploy.Strategy
 	state        *scheduler.ClusterState
-	plan         *operation.SequenceOperation
+	plan         *Plan
+}
+
+// Plan holds the compose-level deployment plan with typed volume and service operations.
+type Plan struct {
+	Volumes  []*operation.CreateVolumeOperation
+	Services []*deploy.ServicePlan
+}
+
+// IsEmpty returns true if the plan has no volume or service operations.
+func (p *Plan) IsEmpty() bool {
+	return len(p.Volumes) == 0 && len(p.Services) == 0
+}
+
+// Execute runs all volume operations followed by all service operations.
+func (p *Plan) Execute(ctx context.Context, cli operation.Client) error {
+	for _, op := range p.Volumes {
+		if err := op.Execute(ctx, cli); err != nil {
+			return err
+		}
+	}
+	for _, sp := range p.Services {
+		if err := sp.Execute(ctx, cli); err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 func NewDeployment(ctx context.Context, cli Client, project *types.Project) (*Deployment, error) {
@@ -59,11 +85,11 @@ func NewDeploymentWithStrategy(ctx context.Context, cli Client, project *types.P
 	}, nil
 }
 
-func (d *Deployment) Plan(ctx context.Context) (operation.SequenceOperation, error) {
+func (d *Deployment) Plan(ctx context.Context) (Plan, error) {
 	if d.plan != nil {
 		return *d.plan, nil
 	}
-	plan := operation.SequenceOperation{}
+	var plan Plan
 
 	// Generate service specs for all services in the project.
 	var serviceSpecs []api.ServiceSpec
@@ -90,9 +116,7 @@ func (d *Deployment) Plan(ctx context.Context) (operation.SequenceOperation, err
 	if err != nil {
 		return plan, err
 	}
-	for _, op := range volumeOps {
+	plan.Volumes = volumeOps
-		plan.Operations = append(plan.Operations, op)
-	}
 
 	for _, spec := range serviceSpecs {
 		// TODO: properly handle depends_on conditions in the service deployment plan as the first operation.
@@ -105,7 +129,7 @@ func (d *Deployment) Plan(ctx context.Context) (operation.SequenceOperation, err
 
 		// Skip no-op (up-to-date) service plans.
 		if len(servicePlan.Operations) > 0 {
-			plan.Operations = append(plan.Operations, &servicePlan)
+			plan.Services = append(plan.Services, &servicePlan)
 		}
 	}
 

pkg/client/deploy/deploy.go

@@ -26,12 +26,12 @@ type Deployment struct {
 	Spec     api.ServiceSpec
 	Strategy Strategy
 	cli      Client
-	plan     *Plan
+	plan     *ServicePlan
 	// state is an optional current and planned cluster state used for scheduling decisions.
 	state *scheduler.ClusterState
 }
 
-type Plan struct {
+type ServicePlan struct {
 	ServiceID   string
 	ServiceName string
 	operation.SequenceOperation
@@ -63,19 +63,19 @@ func NewDeploymentWithClusterState(
 
 // Plan returns a plan of operations to reconcile the service to the desired state.
 // If a plan has already been created, the same plan will be returned.
-func (d *Deployment) Plan(ctx context.Context) (Plan, error) {
+func (d *Deployment) Plan(ctx context.Context) (ServicePlan, error) {
 	if d.plan != nil {
 		return *d.plan, nil
 	}
 
 	// Validate the user-provided spec before resolving it.
 	if err := d.Validate(ctx); err != nil {
-		return Plan{}, fmt.Errorf("invalid deployment: %w", err)
+		return ServicePlan{}, fmt.Errorf("invalid deployment: %w", err)
 	}
 
 	clusterDomain, err := d.cli.GetDomain(ctx)
 	if err != nil && !errors.Is(err, api.ErrNotFound) {
-		return Plan{}, fmt.Errorf("get cluster domain: %w", err)
+		return ServicePlan{}, 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.
@@ -84,19 +84,19 @@ func (d *Deployment) Plan(ctx context.Context) (Plan, error) {
 
 	resolvedSpec, err := specResolver.Resolve(d.Spec)
 	if err != nil {
-		return Plan{}, fmt.Errorf("resolve service spec: %w", err)
+		return ServicePlan{}, fmt.Errorf("resolve service spec: %w", err)
 	}
 
 	if d.state == nil {
 		d.state, err = scheduler.InspectClusterState(ctx, d.cli)
 		if err != nil {
-			return Plan{}, fmt.Errorf("inspect cluster state: %w", err)
+			return ServicePlan{}, fmt.Errorf("inspect cluster state: %w", err)
 		}
 	}
 
 	plan, err := d.Strategy.Plan(d.state, d.Service, resolvedSpec)
 	if err != nil {
-		return Plan{}, fmt.Errorf("create plan using %s strategy: %w", d.Strategy.Type(), err)
+		return ServicePlan{}, fmt.Errorf("create plan using %s strategy: %w", d.Strategy.Type(), err)
 	}
 	d.plan = &plan
 
@@ -143,7 +143,7 @@ func (d *Deployment) Validate(ctx context.Context) error {
 // It will create a new plan if one hasn't been created yet. The deployment will either create a new service or update
 // the existing one to match the desired specification.
 // TODO: forbid to run the same deployment more than once.
-func (d *Deployment) Run(ctx context.Context) (Plan, error) {
+func (d *Deployment) Run(ctx context.Context) (ServicePlan, error) {
 	plan, err := d.Plan(ctx)
 	if err != nil {
 		return plan, fmt.Errorf("create plan: %w", err)

pkg/client/deploy/strategy.go

@@ -20,7 +20,7 @@ type Strategy interface {
 	// Plan returns the operation to reconcile the service to the desired state.
 	// If the service does not exist (new deployment), svc will be nil. state provides the current and planned state
 	// of the cluster for scheduling decisions.
-	Plan(state *scheduler.ClusterState, svc *api.Service, spec api.ServiceSpec) (Plan, error)
+	Plan(state *scheduler.ClusterState, svc *api.Service, spec api.ServiceSpec) (ServicePlan, error)
 }
 
 // RollingStrategy implements a rolling update deployment pattern where containers are updated one at a time
@@ -40,9 +40,9 @@ func (s *RollingStrategy) Type() string {
 	return "rolling"
 }
 
-func (s *RollingStrategy) Plan(state *scheduler.ClusterState, svc *api.Service, spec api.ServiceSpec) (Plan, error) {
+func (s *RollingStrategy) Plan(state *scheduler.ClusterState, svc *api.Service, spec api.ServiceSpec) (ServicePlan, error) {
 	if state == nil {
-		return Plan{}, fmt.Errorf("cluster state must be provided")
+		return ServicePlan{}, fmt.Errorf("cluster state must be provided")
 	}
 	s.state = state
 
@@ -53,7 +53,7 @@ func (s *RollingStrategy) Plan(state *scheduler.ClusterState, svc *api.Service,
 	case api.ServiceModeGlobal:
 		return s.planGlobal(svc, spec)
 	default:
-		return Plan{}, fmt.Errorf("unsupported service mode: '%s'", spec.Mode)
+		return ServicePlan{}, fmt.Errorf("unsupported service mode: '%s'", spec.Mode)
 	}
 }
 
@@ -61,8 +61,8 @@ func (s *RollingStrategy) Plan(state *scheduler.ClusterState, svc *api.Service,
 // For replicated services, we want to maintain a specific number of containers (replicas) across the available machines
 // in the cluster.
 // TODO: schedule containers only on machines that contain the image if pull policy is set to 'never'.
-func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec) (Plan, error) {
+func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec) (ServicePlan, error) {
-	plan, err := newEmptyPlan(svc, spec)
+	plan, err := newEmptyServicePlan(svc, spec)
 	if err != nil {
 		return plan, err
 	}
@@ -198,8 +198,8 @@ func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec)
 // possible. If the new container would have port conflicts with the existing one, the old container is removed first.
 // It handles multiple containers per machine (though this should not occur in normal operation) and skips machines
 // that are down.
-func (s *RollingStrategy) planGlobal(svc *api.Service, spec api.ServiceSpec) (Plan, error) {
+func (s *RollingStrategy) planGlobal(svc *api.Service, spec api.ServiceSpec) (ServicePlan, error) {
-	plan, err := newEmptyPlan(svc, spec)
+	plan, err := newEmptyServicePlan(svc, spec)
 	if err != nil {
 		return plan, err
 	}
@@ -403,9 +403,9 @@ func determineUpdateOrder(oldContainer api.ServiceContainer, spec api.ServiceSpe
 	return api.UpdateOrderStartFirst
 }
 
-// newEmptyPlan creates a new empty plan for a service deployment with initialised service ID and name.
+// newEmptyServicePlan creates a new empty plan for a service deployment with initialised service ID and name.
-func newEmptyPlan(svc *api.Service, spec api.ServiceSpec) (Plan, error) {
+func newEmptyServicePlan(svc *api.Service, spec api.ServiceSpec) (ServicePlan, error) {
-	var plan Plan
+	var plan ServicePlan
 
 	// Generate a new service ID for the initial service deployment if it doesn't exist yet.
 	if svc != nil {

test/e2e/compose_configs_test.go

@@ -39,7 +39,8 @@ func TestComposeConfigs(t *testing.T) {
 
 		plan, err := deploy.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 1, "Expected 1 service deployment")
+		assert.Len(t, plan.Services, 1, "Expected 1 service deployment")
+		assert.Empty(t, plan.Volumes, "Expected no volume operations")
 
 		err = deploy.Run(ctx)
 		require.NoError(t, err)
@@ -93,7 +94,7 @@ func TestComposeConfigs(t *testing.T) {
 
 		plan, err = deploy.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 0, "Expected no new operations after configs deployment")
+		assert.True(t, plan.IsEmpty(), "Expected no new operations after configs deployment")
 
 		// Verify the config files are actually created in the container and contain expected content
 		containerName := svc.Containers[0].Container.Name
@@ -116,7 +117,8 @@ func TestComposeConfigs(t *testing.T) {
 			groupId:     1000,
 		}, configContentSecond)
 
-		configContentThird, err := readFileInfoInContainer(t, cli, name, containerName, "/etc/new-dir/config-from-file.conf")
+		configContentThird, err := readFileInfoInContainer(t, cli, name, containerName,
+			"/etc/new-dir/config-from-file.conf")
 		require.NoError(t, err)
 		assert.Equal(t, fileInfo{
 			permissions: 0o644,

test/e2e/compose_deploy_test.go

@@ -40,7 +40,7 @@ func TestComposeDeployment(t *testing.T) {
 
 		plan, err := deployment.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 1, "Expected 1 service to deploy")
+		assert.Len(t, plan.Services, 1, "Expected 1 service to deploy")
 
 		err = deployment.Run(ctx)
 		require.NoError(t, err)
@@ -93,7 +93,7 @@ func TestComposeDeployment(t *testing.T) {
 
 		plan, err := deployment.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 3, "Expected 3 services to deploy")
+		assert.Len(t, plan.Services, 3, "Expected 3 services to deploy")
 
 		err = deployment.Run(ctx)
 		require.NoError(t, err)
@@ -168,7 +168,7 @@ func TestComposeDeployment(t *testing.T) {
 
 		redeployPlan, err := redeploy.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, redeployPlan.Operations, 0, "Expected no operations - deployment should be up to date")
+		assert.True(t, redeployPlan.IsEmpty(), "Expected no operations - deployment should be up to date")
 
 		// Deploy with ForceRecreate - should recreate all service containers.
 		strategy := &deploy.RollingStrategy{ForceRecreate: true}
@@ -177,7 +177,7 @@ func TestComposeDeployment(t *testing.T) {
 
 		recreatePlan, err := recreateDeploy.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, recreatePlan.Operations, 3, "Expected 3 services to be recreated")
+		assert.Len(t, recreatePlan.Services, 3, "Expected 3 services to be recreated")
 
 		err = recreateDeploy.Run(ctx)
 		require.NoError(t, err)
@@ -242,7 +242,8 @@ func TestComposeDeployment(t *testing.T) {
 
 		plan, err := deployment.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 5, "Expected 2 volumes creation and 3 services to deploy")
+		assert.Len(t, plan.Volumes, 2, "Expected 2 volume creation operations")
+		assert.Len(t, plan.Services, 3, "Expected 3 services to deploy")
 
 		err = deployment.Run(ctx)
 		require.NoError(t, err)
@@ -384,7 +385,7 @@ func TestComposeDeployment(t *testing.T) {
 
 		plan, err = deployment.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 0, "Expected no new operations after deployment")
+		assert.True(t, plan.IsEmpty(), "Expected no new operations after deployment")
 	})
 
 	t.Run("x-machines placement constraint", func(t *testing.T) {
@@ -403,7 +404,7 @@ func TestComposeDeployment(t *testing.T) {
 
 		plan, err := deployment.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 1, "Expected 1 service to deploy")
+		assert.Len(t, plan.Services, 1, "Expected 1 service to deploy")
 
 		err = deployment.Run(ctx)
 		require.NoError(t, err)
@@ -456,7 +457,7 @@ func TestComposeDeployment(t *testing.T) {
 
 		plan, err := deployment.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 1, "Expected 1 service to deploy")
+		assert.Len(t, plan.Services, 1, "Expected 1 service to deploy")
 
 		err = deployment.Run(ctx)
 		require.NoError(t, err)
@@ -504,7 +505,7 @@ func TestComposeDeployment(t *testing.T) {
 
 		plan, err := deployment.Plan(ctx)
 		require.NoError(t, err)
-		assert.Len(t, plan.Operations, 1, "Expected 1 service to deploy")
+		assert.Len(t, plan.Services, 1, "Expected 1 service to deploy")
 
 		err = deployment.Run(ctx)
 		require.NoError(t, err)
@@ -556,7 +557,8 @@ volumes:
 		plan, err := deployment.Plan(ctx)
 		require.NoError(t, err)
 
-		assert.Len(t, plan.Operations, 2, "Expected 1 volume creation and 1 service to deploy")
+		assert.Len(t, plan.Volumes, 1, "Expected 1 volume creation operation")
+		assert.Len(t, plan.Services, 1, "Expected 1 service to deploy")
 	})
 
 	t.Run("global service auto-creates volumes on all machines", func(t *testing.T) {