Skip to content

Commit 1717a88

Browse files
committed
fix targeted global deployments
1 parent 9e53f36 commit 1717a88

File tree

2 files changed

+49
-3
lines changed

2 files changed

+49
-3
lines changed

pkg/client/deploy/strategy.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,11 +205,17 @@ func (s *RollingStrategy) planGlobal(svc *api.Service, spec api.ServiceSpec) (Pl
205205
return plan, err
206206
}
207207

208-
// Global mode requires all machines to satisfy the constraints.
209-
if len(availableMachines) != len(s.state.Machines) {
208+
// Global mode requires all target machines to satisfy the constraints.
209+
// If placement constraints specify machines, use that count; otherwise use all cluster machines.
210+
targetMachineCount := len(s.state.Machines)
211+
if len(spec.Placement.Machines) > 0 {
212+
targetMachineCount = len(spec.Placement.Machines)
213+
}
214+
215+
if len(availableMachines) != targetMachineCount {
210216
return plan, fmt.Errorf(
211217
"global service '%s' requires all machines to satisfy constraints, but only %d of %d machines are eligible",
212-
spec.Name, len(availableMachines), len(s.state.Machines),
218+
spec.Name, len(availableMachines), targetMachineCount,
213219
)
214220
}
215221

pkg/client/deploy/strategy_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,3 +585,43 @@ func TestRollingStrategy_Plan_RequiresClusterState(t *testing.T) {
585585
assert.Error(t, err)
586586
assert.Contains(t, err.Error(), "cluster state must be provided")
587587
}
588+
589+
func TestRollingStrategy_planGlobal_WithPlacementConstraints(t *testing.T) {
590+
t.Parallel()
591+
592+
t.Run("global service with x-machine deploys only to specified machines", func(t *testing.T) {
593+
// Cluster has 3 machines, but we only want to deploy to 2 of them using x-machine
594+
state := newTestClusterState(
595+
newTestMachine("m1", "node1", 4, 8),
596+
newTestMachine("m2", "node2", 4, 8),
597+
newTestMachine("m3", "node3", 4, 8),
598+
)
599+
600+
spec := api.ServiceSpec{
601+
Name: "test-service",
602+
Mode: api.ServiceModeGlobal,
603+
Container: api.ContainerSpec{
604+
Image: "nginx:latest",
605+
},
606+
Placement: api.Placement{
607+
Machines: []string{"node1", "node2"}, // x-machine constraint: only deploy to node1 and node2
608+
},
609+
}
610+
611+
strategy := &RollingStrategy{}
612+
plan, err := strategy.Plan(state, nil, spec)
613+
614+
// This should succeed - global with x-machine should deploy to all machines in the constraint list
615+
require.NoError(t, err, "global service with x-machine should succeed when all specified machines are eligible")
616+
617+
// Should have 2 RunContainerOperations (one per machine in the constraint)
618+
counts := countOperationsByType(plan.Operations)
619+
assert.Equal(t, 2, counts["run"], "should run containers on both specified machines")
620+
621+
// Verify containers are scheduled on the correct machines
622+
machineIDs := getMachineIDsFromRunOps(plan.Operations)
623+
assert.Contains(t, machineIDs, "m1", "should deploy to node1")
624+
assert.Contains(t, machineIDs, "m2", "should deploy to node2")
625+
assert.NotContains(t, machineIDs, "m3", "should not deploy to node3")
626+
})
627+
}

0 commit comments

Comments
 (0)