Skip to content

Commit

Permalink
fix: mapping service to lanes when services are in random order (#528)
Browse files Browse the repository at this point in the history
If services are defined in random order in the layer config, for
example, svc1, svc2, then svc3, but svc3 depends on svc1, when mapping
these services into lanes, the maximum lane count is incorrectly
calculated.

This PR fixes this issue and closes
#525.

Manual test:

Layer config:

```yaml
summary: a simple layer
services:
  test1:
    override: replace
    command: bash -c "sleep 1000"
    startup: enabled
  test2:
    override: replace
    command: bash -c "sleep 1000"
    startup: enabled
  test3:
    override: replace
    command: bash -c "sleep 1000"
    startup: enabled
    requires:
      - test1
```

Result:

```bash
$ ./pebble run
2024-11-28T03:00:55.321Z [pebble] Started daemon.
2024-11-28T03:00:55.329Z [pebble] POST /v1/services 2.313041ms 202
2024-11-28T03:00:55.331Z [pebble] Service "test1" starting: bash -c "sleep 1000"
2024-11-28T03:00:55.332Z [pebble] Service "test2" starting: bash -c "sleep 1000"
2024-11-28T03:00:56.343Z [pebble] Service "test3" starting: bash -c "sleep 1000"
2024-11-28T03:00:57.351Z [pebble] GET /v1/changes/1/wait 2.022350876s 200
2024-11-28T03:00:57.352Z [pebble] Started default services with change 1.
^C2024-11-28T03:00:58.497Z [pebble] Exiting on interrupt signal.
2024-11-28T03:00:58.500Z [pebble] Stopping all running services.
2024-11-28T03:00:58.505Z [pebble] Service "test2" stopped
2024-11-28T03:00:58.505Z [pebble] Service "test1" stopped
2024-11-28T03:00:58.512Z [pebble] Service "test3" stopped
```

A [test
case](https://github.com/canonical/pebble/pull/528/files#diff-f3aedc0e48287c8fa95ea71f4a1bd0a350d89ab1c5559ee7db3298d44207e93bR2076)
is also added to cover this scenario.
  • Loading branch information
IronCore864 authored Nov 28, 2024
1 parent 4559a6a commit 77decbd
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 2 deletions.
8 changes: 6 additions & 2 deletions internals/plan/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -1133,7 +1133,8 @@ func createLanes(names []string, services map[string]*Service) ([][]string, erro
serviceLaneMapping := make(map[string]int)

// Map all services into lanes.
var lane = -1
lane := -1
maxLane := 0
for _, name := range names {
service, ok := services[name]
if !ok {
Expand All @@ -1143,10 +1144,13 @@ func createLanes(names []string, services map[string]*Service) ([][]string, erro
}

lane = getOrCreateLane(lane, service, serviceLaneMapping)
if lane > maxLane {
maxLane = lane
}
}

// Create lanes
lanes := make([][]string, lane+1)
lanes := make([][]string, maxLane+1)
for _, service := range names {
lane := serviceLaneMapping[service]
lanes[lane] = append(lanes[lane], service)
Expand Down
53 changes: 53 additions & 0 deletions internals/plan/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2073,6 +2073,59 @@ func (s *S) TestStartStopOrderMultipleLanes(c *C) {
c.Assert(lanes[2], DeepEquals, []string{"srv3"})
}

func (s *S) TestStartStopOrderMultipleLanesRandomOrder(c *C) {
layer := &plan.Layer{
Summary: "services with no dependencies in different lanes",
Description: "a simple layer",
Services: map[string]*plan.Service{
"srv1": {
Name: "srv1",
Override: "replace",
Command: `cmd`,
Startup: plan.StartupEnabled,
},
"srv2": {
Name: "srv2",
Override: "replace",
Command: `cmd`,
Startup: plan.StartupEnabled,
},
"srv3": {
Name: "srv3",
Override: "replace",
Command: `cmd`,
Startup: plan.StartupEnabled,
},
"srv4": {
Name: "srv4",
Override: "replace",
Command: `cmd`,
Startup: plan.StartupEnabled,
Requires: []string{"srv1"},
After: []string{"srv1"},
},
},
Checks: map[string]*plan.Check{},
LogTargets: map[string]*plan.LogTarget{},
}

p := plan.Plan{Services: layer.Services}

lanes, err := p.StartOrder([]string{"srv1", "srv2", "srv3", "srv4"})
c.Assert(err, IsNil)
c.Assert(len(lanes), Equals, 3)
c.Assert(lanes[0], DeepEquals, []string{"srv1", "srv4"})
c.Assert(lanes[1], DeepEquals, []string{"srv2"})
c.Assert(lanes[2], DeepEquals, []string{"srv3"})

lanes, err = p.StopOrder([]string{"srv1", "srv2", "srv3", "srv4"})
c.Assert(err, IsNil)
c.Assert(len(lanes), Equals, 3)
c.Assert(lanes[0], DeepEquals, []string{"srv4", "srv1"})
c.Assert(lanes[1], DeepEquals, []string{"srv2"})
c.Assert(lanes[2], DeepEquals, []string{"srv3"})
}

// TestSectionFieldStability detects changes in plan.Layer and plan.Plan
// YAML fields, and fails on any change that could break hardcoded section
// fields in the code. On failure, please carefully inspect and update
Expand Down

0 comments on commit 77decbd

Please sign in to comment.