fix: new Compose deployment with volumes and --recreate flag (fixes #176)

This commit is contained in:
Pasha Sviderski
2025-11-20 15:19:44 +10:00
parent ce4a8cce72
commit 57205e1149
7 changed files with 89 additions and 23 deletions
+3 -6
View File
@@ -49,10 +49,6 @@ func NewDeploymentWithStrategy(ctx context.Context, cli Client, project *types.P
ClusterDomain: domain, ClusterDomain: domain,
} }
if strategy == nil {
strategy = &deploy.RollingStrategy{State: state}
}
return &Deployment{ return &Deployment{
Client: cli, Client: cli,
Project: project, Project: project,
@@ -88,6 +84,7 @@ func (d *Deployment) Plan(ctx context.Context) (deploy.SequenceOperation, error)
} }
// Check external volumes and plan the creation of missing volumes before deploying services. // Check external volumes and plan the creation of missing volumes before deploying services.
// Updates the cluster state (d.state) with the scheduled volumes.
volumeOps, err := d.planVolumes(serviceSpecs) volumeOps, err := d.planVolumes(serviceSpecs)
if err != nil { if err != nil {
return plan, err return plan, err
@@ -98,8 +95,8 @@ func (d *Deployment) Plan(ctx context.Context) (deploy.SequenceOperation, error)
for _, spec := range serviceSpecs { for _, spec := range serviceSpecs {
// TODO: properly handle depends_on conditions in the service deployment plan as the first operation. // TODO: properly handle depends_on conditions in the service deployment plan as the first operation.
// Pass the update cluster state with scheduled volumes to the deployment. // Pass the updated cluster state with the scheduled volumes to the deployment.
deployment := deploy.NewDeployment(d.Client, spec, d.Strategy) deployment := deploy.NewDeploymentWithClusterState(d.Client, spec, d.Strategy, d.state)
servicePlan, err := deployment.Plan(ctx) servicePlan, err := deployment.Plan(ctx)
if err != nil { if err != nil {
return plan, fmt.Errorf("create deployment plan for service '%s': %w", spec.Name, err) return plan, fmt.Errorf("create deployment plan for service '%s': %w", spec.Name, err)
+23
View File
@@ -3,12 +3,15 @@ package compose
import ( import (
"context" "context"
"fmt" "fmt"
"os"
"path/filepath"
"strings" "strings"
composecli "github.com/compose-spec/compose-go/v2/cli" composecli "github.com/compose-spec/compose-go/v2/cli"
"github.com/compose-spec/compose-go/v2/types" "github.com/compose-spec/compose-go/v2/types"
) )
// LoadProject loads a Compose project from the default locations or the given paths.
func LoadProject(ctx context.Context, paths []string, opts ...composecli.ProjectOptionsFn) (*types.Project, error) { func LoadProject(ctx context.Context, paths []string, opts ...composecli.ProjectOptionsFn) (*types.Project, error) {
defaultOpts := []composecli.ProjectOptionsFn{ defaultOpts := []composecli.ProjectOptionsFn{
// First apply os.Environment, always wins. // First apply os.Environment, always wins.
@@ -61,6 +64,26 @@ func LoadProject(ctx context.Context, paths []string, opts ...composecli.Project
return project, nil return project, nil
} }
// LoadProjectFromContent loads a Compose project from the given YAML content.
func LoadProjectFromContent(
ctx context.Context, content string, opts ...composecli.ProjectOptionsFn,
) (*types.Project, error) {
// Create a temporary directory for the compose file.
tmpDir, err := os.MkdirTemp("", "uncloud-compose-*")
if err != nil {
return nil, fmt.Errorf("create temporary directory: %w", err)
}
defer os.RemoveAll(tmpDir)
// Write the YAML content to compose.yaml in the temporary directory.
composePath := filepath.Join(tmpDir, "compose.yaml")
if err := os.WriteFile(composePath, []byte(content), 0644); err != nil {
return nil, fmt.Errorf("write compose file: %w", err)
}
return LoadProject(ctx, []string{composePath}, opts...)
}
// removeProjectPrefixFromNames removes the project name prefix from volume names. // removeProjectPrefixFromNames removes the project name prefix from volume names.
func removeProjectPrefixFromNames(project *types.Project) { func removeProjectPrefixFromNames(project *types.Project) {
prefix := project.Name + "_" prefix := project.Name + "_"
+1
View File
@@ -22,6 +22,7 @@ import (
// loadProjectFromContent loads a compose project from YAML content. // loadProjectFromContent loads a compose project from YAML content.
// Keep the implementation in sync with LoadProject. // Keep the implementation in sync with LoadProject.
// TODO(lhf): remove and replace with compose.LoadProjectFromContent
func loadProjectFromContent(t *testing.T, content string) (*types.Project, error) { func loadProjectFromContent(t *testing.T, content string) (*types.Project, error) {
t.Helper() t.Helper()
ctx := context.Background() ctx := context.Background()
+21 -1
View File
@@ -6,6 +6,7 @@ import (
"fmt" "fmt"
"github.com/psviderski/uncloud/pkg/api" "github.com/psviderski/uncloud/pkg/api"
"github.com/psviderski/uncloud/pkg/client/deploy/scheduler"
) )
type Client interface { type Client interface {
@@ -25,6 +26,8 @@ type Deployment struct {
Strategy Strategy Strategy Strategy
cli Client cli Client
plan *Plan plan *Plan
// state is an optional current and planned cluster state used for scheduling decisions.
state *scheduler.ClusterState
} }
type Plan struct { type Plan struct {
@@ -47,6 +50,16 @@ func NewDeployment(cli Client, spec api.ServiceSpec, strategy Strategy) *Deploym
} }
} }
// NewDeploymentWithClusterState creates a new deployment like NewDeployment but also with a provided current cluster
// state used for scheduling decisions.
func NewDeploymentWithClusterState(
cli Client, spec api.ServiceSpec, strategy Strategy, state *scheduler.ClusterState,
) *Deployment {
d := NewDeployment(cli, spec, strategy)
d.state = state
return d
}
// Plan returns a plan of operations to reconcile the service to the desired state. // 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. // 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) (Plan, error) {
@@ -73,7 +86,14 @@ func (d *Deployment) Plan(ctx context.Context) (Plan, error) {
return Plan{}, fmt.Errorf("resolve service spec: %w", err) return Plan{}, fmt.Errorf("resolve service spec: %w", err)
} }
plan, err := d.Strategy.Plan(ctx, d.cli, d.Service, resolvedSpec) if d.state == nil {
d.state, err = scheduler.InspectClusterState(ctx, d.cli)
if err != nil {
return Plan{}, fmt.Errorf("inspect cluster state: %w", err)
}
}
plan, err := d.Strategy.Plan(d.state, d.Service, resolvedSpec)
if err != nil { if err != nil {
return Plan{}, fmt.Errorf("create plan using %s strategy: %w", d.Strategy.Type(), err) return Plan{}, fmt.Errorf("create plan using %s strategy: %w", d.Strategy.Type(), err)
} }
+2 -1
View File
@@ -17,7 +17,7 @@ import (
// - If a volume already exists on a machine, it must be used instead of creating a new one. // - If a volume already exists on a machine, it must be used instead of creating a new one.
// - A missing volume must only be created on one machine. // - A missing volume must only be created on one machine.
type VolumeScheduler struct { type VolumeScheduler struct {
// state is the current state of machines and their resources in the cluster. // state is the current and planned state of machines and their resources in the cluster.
state *ClusterState state *ClusterState
// serviceSpecs is a list of service specifications included in the deployment. // serviceSpecs is a list of service specifications included in the deployment.
serviceSpecs []api.ServiceSpec serviceSpecs []api.ServiceSpec
@@ -106,6 +106,7 @@ func NewVolumeScheduler(state *ClusterState, specs []api.ServiceSpec) (*VolumeSc
// Schedule determines what missing volumes should be created and where for services in the multi-service deployment. // Schedule determines what missing volumes should be created and where for services in the multi-service deployment.
// It returns a map of machine IDs to a list of api.VolumeSpec that should be created on that machine, // It returns a map of machine IDs to a list of api.VolumeSpec that should be created on that machine,
// or an error if services can't be scheduled due to scheduling constraints. // or an error if services can't be scheduled due to scheduling constraints.
// It also updates the state of the machines in the cluster state to reflect the scheduled volumes.
func (s *VolumeScheduler) Schedule() (map[string][]api.VolumeSpec, error) { func (s *VolumeScheduler) Schedule() (map[string][]api.VolumeSpec, error) {
if len(s.serviceSpecs) == 0 { if len(s.serviceSpecs) == 0 {
// No services with volume mounts, nothing to schedule. // No services with volume mounts, nothing to schedule.
+14 -15
View File
@@ -1,7 +1,6 @@
package deploy package deploy
import ( import (
"context"
"fmt" "fmt"
"math/rand/v2" "math/rand/v2"
"slices" "slices"
@@ -18,31 +17,31 @@ type Strategy interface {
// Type returns the type of the deployment strategy, e.g. "rolling", "blue-green". // Type returns the type of the deployment strategy, e.g. "rolling", "blue-green".
Type() string Type() string
// Plan returns the operation to reconcile the service to the desired state. // Plan returns the operation to reconcile the service to the desired state.
// If the service does not exist (new deployment), svc will be nil. // If the service does not exist (new deployment), svc will be nil. state provides the current and planned state
Plan(ctx context.Context, cli scheduler.Client, svc *api.Service, spec api.ServiceSpec) (Plan, error) // of the cluster for scheduling decisions.
Plan(state *scheduler.ClusterState, svc *api.Service, spec api.ServiceSpec) (Plan, error)
} }
// RollingStrategy implements a rolling update deployment pattern where containers are updated one at a time // RollingStrategy implements a rolling update deployment pattern where containers are updated one at a time
// to minimize service disruption. // to minimize service disruption.
type RollingStrategy struct { type RollingStrategy struct {
State *scheduler.ClusterState // ForceRecreate indicates whether all containers should be recreated during the deployment,
// regardless of whether their specifications have changed.
ForceRecreate bool ForceRecreate bool
// state is the current and planned state of the cluster used for scheduling decisions.
state *scheduler.ClusterState
} }
func (s *RollingStrategy) Type() string { func (s *RollingStrategy) Type() string {
return "rolling" return "rolling"
} }
func (s *RollingStrategy) Plan( func (s *RollingStrategy) Plan(state *scheduler.ClusterState, svc *api.Service, spec api.ServiceSpec) (Plan, error) {
ctx context.Context, cli scheduler.Client, svc *api.Service, spec api.ServiceSpec, if state == nil {
) (Plan, error) { return Plan{}, fmt.Errorf("cluster state must be provided")
if s.State == nil {
state, err := scheduler.InspectClusterState(ctx, cli)
if err != nil {
return Plan{}, fmt.Errorf("inspect cluster state: %w", err)
}
s.State = state
} }
s.state = state
// We can assume that the spec is valid at this point because it has been validated by the deployment. // We can assume that the spec is valid at this point because it has been validated by the deployment.
switch spec.Mode { switch spec.Mode {
@@ -65,7 +64,7 @@ func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec)
return plan, err return plan, err
} }
sched := scheduler.NewServiceScheduler(s.State, spec) sched := scheduler.NewServiceScheduler(s.state, spec)
// TODO: return a detailed report on required constraints and which ones are satisfied? // TODO: return a detailed report on required constraints and which ones are satisfied?
availableMachines, err := sched.EligibleMachines() availableMachines, err := sched.EligibleMachines()
if err != nil { if err != nil {
@@ -221,7 +220,7 @@ func (s *RollingStrategy) planGlobal(svc *api.Service, spec api.ServiceSpec) (Pl
} }
} }
sched := scheduler.NewServiceScheduler(s.State, spec) sched := scheduler.NewServiceScheduler(s.state, spec)
availableMachines, err := sched.EligibleMachines() availableMachines, err := sched.EligibleMachines()
if err != nil { if err != nil {
return plan, err return plan, err
+25
View File
@@ -533,4 +533,29 @@ func TestComposeDeployment(t *testing.T) {
assert.ElementsMatch(t, serviceMachines.ToSlice(), []string{c.Machines[0].ID, c.Machines[2].ID}, assert.ElementsMatch(t, serviceMachines.ToSlice(), []string{c.Machines[0].ID, c.Machines[2].ID},
"Service containers should be on machines 1 and 3 from comma-separated list") "Service containers should be on machines 1 and 3 from comma-separated list")
}) })
// Catches regression: https://github.com/psviderski/uncloud/issues/176
t.Run("plan new deployment with volumes and recreate strategy", func(t *testing.T) {
t.Parallel()
project, err := compose.LoadProjectFromContent(ctx, `
services:
redis:
image: redis:7-alpine
volumes:
- redis-data:/data
volumes:
redis-data:
`)
require.NoError(t, err)
deployment, err := compose.NewDeploymentWithStrategy(ctx, cli, project,
&deploy.RollingStrategy{ForceRecreate: true})
require.NoError(t, err)
plan, err := deployment.Plan(ctx)
require.NoError(t, err)
assert.Len(t, plan.Operations, 2, "Expected 1 volume creation and 1 service to deploy")
})
} }