diff --git a/pkg/client/compose/deploy.go b/pkg/client/compose/deploy.go index 692227d1..a5c976f8 100644 --- a/pkg/client/compose/deploy.go +++ b/pkg/client/compose/deploy.go @@ -49,10 +49,6 @@ func NewDeploymentWithStrategy(ctx context.Context, cli Client, project *types.P ClusterDomain: domain, } - if strategy == nil { - strategy = &deploy.RollingStrategy{State: state} - } - return &Deployment{ Client: cli, 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. + // Updates the cluster state (d.state) with the scheduled volumes. volumeOps, err := d.planVolumes(serviceSpecs) if err != nil { return plan, err @@ -98,8 +95,8 @@ func (d *Deployment) Plan(ctx context.Context) (deploy.SequenceOperation, error) for _, spec := range serviceSpecs { // 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. - deployment := deploy.NewDeployment(d.Client, spec, d.Strategy) + // Pass the updated cluster state with the scheduled volumes to the deployment. + deployment := deploy.NewDeploymentWithClusterState(d.Client, spec, d.Strategy, d.state) servicePlan, err := deployment.Plan(ctx) if err != nil { return plan, fmt.Errorf("create deployment plan for service '%s': %w", spec.Name, err) diff --git a/pkg/client/compose/project.go b/pkg/client/compose/project.go index d8cf5e84..ad685869 100644 --- a/pkg/client/compose/project.go +++ b/pkg/client/compose/project.go @@ -3,12 +3,15 @@ package compose import ( "context" "fmt" + "os" + "path/filepath" "strings" composecli "github.com/compose-spec/compose-go/v2/cli" "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) { defaultOpts := []composecli.ProjectOptionsFn{ // First apply os.Environment, always wins. @@ -61,6 +64,26 @@ func LoadProject(ctx context.Context, paths []string, opts ...composecli.Project 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. func removeProjectPrefixFromNames(project *types.Project) { prefix := project.Name + "_" diff --git a/pkg/client/compose/service_test.go b/pkg/client/compose/service_test.go index 89beb4ca..ad6c80da 100644 --- a/pkg/client/compose/service_test.go +++ b/pkg/client/compose/service_test.go @@ -22,6 +22,7 @@ import ( // loadProjectFromContent loads a compose project from YAML content. // 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) { t.Helper() ctx := context.Background() diff --git a/pkg/client/deploy/deploy.go b/pkg/client/deploy/deploy.go index d9f1b99e..fe5a6e01 100644 --- a/pkg/client/deploy/deploy.go +++ b/pkg/client/deploy/deploy.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/psviderski/uncloud/pkg/api" + "github.com/psviderski/uncloud/pkg/client/deploy/scheduler" ) type Client interface { @@ -25,6 +26,8 @@ type Deployment struct { Strategy Strategy cli Client plan *Plan + // state is an optional current and planned cluster state used for scheduling decisions. + state *scheduler.ClusterState } 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. // If a plan has already been created, the same plan will be returned. 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) } - 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 { return Plan{}, fmt.Errorf("create plan using %s strategy: %w", d.Strategy.Type(), err) } diff --git a/pkg/client/deploy/scheduler/volume.go b/pkg/client/deploy/scheduler/volume.go index 8677c9b3..51927150 100644 --- a/pkg/client/deploy/scheduler/volume.go +++ b/pkg/client/deploy/scheduler/volume.go @@ -17,7 +17,7 @@ import ( // - 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. 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 // serviceSpecs is a list of service specifications included in the deployment. 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. // 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. +// 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) { if len(s.serviceSpecs) == 0 { // No services with volume mounts, nothing to schedule. diff --git a/pkg/client/deploy/strategy.go b/pkg/client/deploy/strategy.go index 72b82c8c..09cd07aa 100644 --- a/pkg/client/deploy/strategy.go +++ b/pkg/client/deploy/strategy.go @@ -1,7 +1,6 @@ package deploy import ( - "context" "fmt" "math/rand/v2" "slices" @@ -18,31 +17,31 @@ type Strategy interface { // Type returns the type of the deployment strategy, e.g. "rolling", "blue-green". Type() string // Plan returns the operation to reconcile the service to the desired state. - // If the service does not exist (new deployment), svc will be nil. - Plan(ctx context.Context, cli scheduler.Client, svc *api.Service, spec api.ServiceSpec) (Plan, error) + // 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) } // RollingStrategy implements a rolling update deployment pattern where containers are updated one at a time // to minimize service disruption. 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 + + // state is the current and planned state of the cluster used for scheduling decisions. + state *scheduler.ClusterState } func (s *RollingStrategy) Type() string { return "rolling" } -func (s *RollingStrategy) Plan( - ctx context.Context, cli scheduler.Client, svc *api.Service, spec api.ServiceSpec, -) (Plan, error) { - 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 +func (s *RollingStrategy) Plan(state *scheduler.ClusterState, svc *api.Service, spec api.ServiceSpec) (Plan, error) { + if state == nil { + return Plan{}, fmt.Errorf("cluster state must be provided") } + s.state = state // We can assume that the spec is valid at this point because it has been validated by the deployment. switch spec.Mode { @@ -65,7 +64,7 @@ func (s *RollingStrategy) planReplicated(svc *api.Service, spec api.ServiceSpec) 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? availableMachines, err := sched.EligibleMachines() 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() if err != nil { return plan, err diff --git a/test/e2e/compose_deploy_test.go b/test/e2e/compose_deploy_test.go index c942c0f8..efbab9b5 100644 --- a/test/e2e/compose_deploy_test.go +++ b/test/e2e/compose_deploy_test.go @@ -533,4 +533,29 @@ func TestComposeDeployment(t *testing.T) { 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") }) + + // 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") + }) }