diff --git a/internals/daemon/api_services.go b/internals/daemon/api_services.go index e2b9d58a2..dfc045c6b 100644 --- a/internals/daemon/api_services.go +++ b/internals/daemon/api_services.go @@ -1,4 +1,4 @@ -// Copyright (c) 2014-2020 Canonical Ltd +// Copyright (c) 2014-2024 Canonical Ltd // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License version 3 as @@ -104,37 +104,38 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response { defer st.Unlock() var taskSet *state.TaskSet + var lanes [][]string var services []string switch payload.Action { case "start", "autostart": - services, err = servmgr.StartOrder(payload.Services) + lanes, err = servmgr.StartOrder(payload.Services) if err != nil { break } - taskSet, err = servstate.Start(st, services) + taskSet, err = servstate.Start(st, lanes) case "stop": - services, err = servmgr.StopOrder(payload.Services) + lanes, err = servmgr.StopOrder(payload.Services) if err != nil { break } - taskSet, err = servstate.Stop(st, services) + taskSet, err = servstate.Stop(st, lanes) case "restart": - services, err = servmgr.StopOrder(payload.Services) + lanes, err = servmgr.StopOrder(payload.Services) if err != nil { break } - services = intersectOrdered(payload.Services, services) + lanes = intersectOrdered(payload.Services, lanes) var stopTasks *state.TaskSet - stopTasks, err = servstate.Stop(st, services) + stopTasks, err = servstate.Stop(st, lanes) if err != nil { break } - services, err = servmgr.StartOrder(payload.Services) + lanes, err = servmgr.StartOrder(payload.Services) if err != nil { break } var startTasks *state.TaskSet - startTasks, err = servstate.Start(st, services) + startTasks, err = servstate.Start(st, lanes) if err != nil { break } @@ -143,18 +144,18 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response { taskSet.AddAll(stopTasks) taskSet.AddAll(startTasks) case "replan": - var stopNames, startNames []string - stopNames, startNames, err = servmgr.Replan() + var stopLanes, startLanes [][]string + stopLanes, startLanes, err = servmgr.Replan() if err != nil { break } var stopTasks *state.TaskSet - stopTasks, err = servstate.Stop(st, stopNames) + stopTasks, err = servstate.Stop(st, stopLanes) if err != nil { break } var startTasks *state.TaskSet - startTasks, err = servstate.Start(st, startNames) + startTasks, err = servstate.Start(st, startLanes) if err != nil { break } @@ -165,11 +166,15 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response { // Populate a list of services affected by the replan for summary. replanned := make(map[string]bool) - for _, v := range stopNames { - replanned[v] = true + for _, lane := range stopLanes { + for _, v := range lane { + replanned[v] = true + } } - for _, v := range startNames { - replanned[v] = true + for _, lane := range startLanes { + for _, v := range lane { + replanned[v] = true + } } for k := range replanned { services = append(services, k) @@ -186,6 +191,9 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response { // Use the original requested service name for the summary, not the // resolved one. But do use the resolved set for the count. var summary string + for _, row := range lanes { + services = append(services, row...) + } switch { case len(taskSet.Tasks()) == 0: // Can happen with a replan that has no services to stop/start. A @@ -222,15 +230,22 @@ func v1PostService(c *Command, r *http.Request, _ *UserState) Response { // intersectOrdered returns the intersection of left and right where // the right's ordering is persisted in the resulting set. -func intersectOrdered(left []string, orderedRight []string) []string { +func intersectOrdered(left []string, orderedRight [][]string) [][]string { m := map[string]bool{} for _, v := range left { m[v] = true } - var out []string - for _, v := range orderedRight { - if m[v] { - out = append(out, v) + + var out [][]string + for _, lane := range orderedRight { + var intersectLane []string + for _, v := range lane { + if m[v] { + intersectLane = append(intersectLane, v) + } + } + if len(intersectLane) > 0 { + out = append(out, intersectLane) } } return out diff --git a/internals/overlord/servstate/manager.go b/internals/overlord/servstate/manager.go index ae1ff9e08..43a379d27 100644 --- a/internals/overlord/servstate/manager.go +++ b/internals/overlord/servstate/manager.go @@ -204,19 +204,28 @@ func (m *ServiceManager) DefaultServiceNames() ([]string, error) { } } - return currentPlan.StartOrder(names) + lanes, err := currentPlan.StartOrder(names) + if err != nil { + return nil, err + } + + var result []string + for _, lane := range lanes { + result = append(result, lane...) + } + return result, err } // StartOrder returns the provided services, together with any required -// dependencies, in the proper order for starting them all up. -func (m *ServiceManager) StartOrder(services []string) ([]string, error) { +// dependencies, in the proper order, put in lanes, for starting them all up. +func (m *ServiceManager) StartOrder(services []string) ([][]string, error) { currentPlan := m.getPlan() return currentPlan.StartOrder(services) } // StopOrder returns the provided services, together with any dependants, -// in the proper order for stopping them all. -func (m *ServiceManager) StopOrder(services []string) ([]string, error) { +// in the proper order, put in lanes, for stopping them all. +func (m *ServiceManager) StopOrder(services []string) ([][]string, error) { currentPlan := m.getPlan() return currentPlan.StopOrder(services) } @@ -251,9 +260,9 @@ func (m *ServiceManager) ServiceLogs(services []string, last int) (map[string]se return iterators, nil } -// Replan returns a list of services to stop and services to start because -// their plans had changed between when they started and this call. -func (m *ServiceManager) Replan() ([]string, []string, error) { +// Replan returns a list of services in lanes to stop and services to start +// because their plans had changed between when they started and this call. +func (m *ServiceManager) Replan() ([][]string, [][]string, error) { currentPlan := m.getPlan() m.servicesLock.Lock() defer m.servicesLock.Unlock() @@ -278,7 +287,7 @@ func (m *ServiceManager) Replan() ([]string, []string, error) { } } - stop, err := currentPlan.StopOrder(stop) + stopLanes, err := currentPlan.StopOrder(stop) if err != nil { return nil, nil, err } @@ -288,12 +297,12 @@ func (m *ServiceManager) Replan() ([]string, []string, error) { } } - start, err = currentPlan.StartOrder(start) + startLanes, err := currentPlan.StartOrder(start) if err != nil { return nil, nil, err } - return stop, start, nil + return stopLanes, startLanes, nil } func (m *ServiceManager) SendSignal(services []string, signal string) error { @@ -342,8 +351,9 @@ func (m *ServiceManager) CheckFailed(name string) { // exit. If it starts just before, it would continue to run after the service // manager is terminated. If it starts just after (before the main process // exits), it would generate a runtime error as the reaper would already be dead. -// This function returns a slice of service names to stop, in dependency order. -func servicesToStop(m *ServiceManager) ([]string, error) { +// This function returns a slice of service names to stop, in dependency order, +// put in lanes. +func servicesToStop(m *ServiceManager) ([][]string, error) { currentPlan := m.getPlan() // Get all service names in plan. services := make([]string, 0, len(currentPlan.Services)) @@ -360,12 +370,18 @@ func servicesToStop(m *ServiceManager) ([]string, error) { // Filter down to only those that are running or in backoff m.servicesLock.Lock() defer m.servicesLock.Unlock() - var notStopped []string - for _, name := range stop { - s := m.services[name] - if s != nil && (s.state == stateRunning || s.state == stateBackoff) { - notStopped = append(notStopped, name) + var result [][]string + for _, services := range stop { + var notStopped []string + for _, name := range services { + s := m.services[name] + if s != nil && (s.state == stateRunning || s.state == stateBackoff) { + notStopped = append(notStopped, name) + } + } + if len(notStopped) > 0 { + result = append(result, notStopped) } } - return notStopped, nil + return result, nil } diff --git a/internals/overlord/servstate/manager_test.go b/internals/overlord/servstate/manager_test.go index 9798a516d..391e41628 100644 --- a/internals/overlord/servstate/manager_test.go +++ b/internals/overlord/servstate/manager_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2014-2020 Canonical Ltd +// Copyright (c) 2014-2024 Canonical Ltd // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License version 3 as @@ -290,7 +290,7 @@ services: _, _, err := s.manager.Replan() c.Assert(err, IsNil) - s.startServices(c, []string{"test9"}) + s.startServices(c, [][]string{{"test9"}}) s.waitUntilService(c, "test9", func(service *servstate.ServiceInfo) bool { return service.Current == servstate.StatusActive }) @@ -312,13 +312,13 @@ services: _, _, err := s.manager.Replan() c.Assert(err, IsNil) - s.startServices(c, []string{"test6"}) + s.startServices(c, [][]string{{"test6"}}) s.waitUntilService(c, "test6", func(service *servstate.ServiceInfo) bool { return service.Current == servstate.StatusActive }) startTime := time.Now() - chg := s.stopServices(c, []string{"test6"}) + chg := s.stopServices(c, [][]string{{"test6"}}) s.st.Lock() c.Check(chg.Status(), Equals, state.DoneStatus, Commentf("Error: %v", chg.Err())) s.st.Unlock() @@ -351,8 +351,8 @@ services: stops, starts, err := s.manager.Replan() c.Assert(err, IsNil) - c.Check(stops, DeepEquals, []string{"test2"}) - c.Check(starts, DeepEquals, []string{"test1", "test2"}) + c.Check(stops, DeepEquals, [][]string{{"test2", "test1"}}) + c.Check(starts, DeepEquals, [][]string{{"test1", "test2"}}) s.stopTestServices(c) } @@ -441,7 +441,7 @@ func (s *S) TestStartBadCommand(c *C) { s.planAddLayer(c, testPlanLayer) s.planChanged(c) - chg := s.startServices(c, []string{"test3"}) + chg := s.startServices(c, [][]string{{"test3"}}) s.st.Lock() c.Check(chg.Status(), Equals, state.ErrorStatus) @@ -479,7 +479,7 @@ services: )) s.planChanged(c) - chg := s.startServices(c, []string{"usrtest"}) + chg := s.startServices(c, [][]string{{"usrtest"}}) s.st.Lock() c.Assert(chg.Err(), IsNil) s.st.Unlock() @@ -511,7 +511,7 @@ func (s *S) TestUserGroupFails(c *C) { }) defer restore() - chg := s.startServices(c, []string{"test5"}) + chg := s.startServices(c, [][]string{{"test5"}}) s.st.Lock() c.Check(chg.Status(), Equals, state.ErrorStatus) @@ -567,7 +567,7 @@ services: os.Setenv("USER", "y") // Start service and ensure it has enough time to write to log. - chg := s.startServices(c, []string{"usrgrp"}) + chg := s.startServices(c, [][]string{{"usrgrp"}}) s.waitUntilService(c, "usrgrp", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -585,7 +585,7 @@ func (s *S) TestStartFastExitCommand(c *C) { s.planAddLayer(c, testPlanLayer) s.planChanged(c) - chg := s.startServices(c, []string{"test4"}) + chg := s.startServices(c, [][]string{{"test4"}}) s.st.Lock() c.Check(chg.Status(), Equals, state.ErrorStatus) @@ -623,7 +623,7 @@ func (s *S) TestServices(c *C) { }) // Start a service and ensure it's marked active - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) services, err = s.manager.Services(nil) c.Assert(err, IsNil) @@ -669,7 +669,7 @@ services: c.Assert(err, IsNil) // Start "envtest" service - chg := s.startServices(c, []string{"envtest"}) + chg := s.startServices(c, [][]string{{"envtest"}}) s.st.Lock() c.Check(chg.Status(), Equals, state.DoneStatus, Commentf("Error: %v", chg.Err())) s.st.Unlock() @@ -731,7 +731,7 @@ services: s.planChanged(c) // Start the "test2" service - chg := s.startServices(c, []string{"test2"}) + chg := s.startServices(c, [][]string{{"test2"}}) // Wait until "test2" service completes the echo command (so that we // know the log buffer contains stdout). s.waitForDoneCheck(c, "test2") @@ -792,7 +792,7 @@ services: s.planChanged(c) // Start service and wait till it starts up the first time. - chg := s.startServices(c, []string{"test2"}) + chg := s.startServices(c, [][]string{{"test2"}}) s.waitUntilService(c, "test2", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -807,7 +807,7 @@ services: }) // Ensure it can be stopped successfully. - chg = s.stopServices(c, []string{"test2"}) + chg = s.stopServices(c, [][]string{{"test2"}}) s.st.Lock() c.Check(chg.Status(), Equals, state.DoneStatus, Commentf("Error: %v", chg.Err())) s.st.Unlock() @@ -868,7 +868,7 @@ checks: checkMgr.PlanChanged(s.plan) // Start service and wait till it starts up - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitForDoneCheck(c, "test2") @@ -964,7 +964,7 @@ checks: checkMgr.PlanChanged(s.plan) // Start service and wait till it starts up - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitForDoneCheck(c, "test2") @@ -1054,7 +1054,7 @@ checks: checkMgr.PlanChanged(s.plan) // Start service and wait till it starts up (output file is written to) - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitForDoneCheck(c, "test2") @@ -1139,7 +1139,7 @@ checks: checkMgr.PlanChanged(s.plan) // Start service and wait till it starts up (output file is written to) - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitForDoneCheck(c, "test2") @@ -1179,7 +1179,7 @@ services: s.planChanged(c) // Start service and wait till it starts up the first time. - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitUntilService(c, "test2", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -1210,7 +1210,7 @@ services: s.planChanged(c) // Start service and wait till it starts up the first time. - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitUntilService(c, "test2", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -1241,7 +1241,7 @@ services: s.planChanged(c) // Start service and wait till it starts up the first time. - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitUntilService(c, "test2", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -1272,7 +1272,7 @@ services: s.planChanged(c) // Start service and wait till it starts up the first time. - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitUntilService(c, "test2", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -1303,7 +1303,7 @@ services: s.planChanged(c) // Start service and wait till it starts up the first time. - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitUntilService(c, "test2", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -1471,7 +1471,7 @@ services: )) s.planChanged(c) - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) // The child process creates a grandchild and will print the // PID of the grandchild for us to inspect here. We need to @@ -1547,7 +1547,7 @@ func (s *S) TestStopRunning(c *C) { s.planAddLayer(c, testPlanLayer) s.planChanged(c) - s.startServices(c, []string{"test2"}) + s.startServices(c, [][]string{{"test2"}}) s.waitUntilService(c, "test2", func(svc *servstate.ServiceInfo) bool { return svc.Current == servstate.StatusActive }) @@ -1607,7 +1607,7 @@ services: // Service command should run in current directory (package directory) // if "working-dir" config option not set. - chg := s.startServices(c, []string{"nowrkdir"}) + chg := s.startServices(c, [][]string{{"nowrkdir"}}) s.st.Lock() c.Assert(chg.Err(), IsNil) s.st.Unlock() @@ -1640,7 +1640,7 @@ services: )) s.planChanged(c) - chg := s.startServices(c, []string{"wrkdir"}) + chg := s.startServices(c, [][]string{{"wrkdir"}}) s.st.Lock() c.Assert(chg.Err(), IsNil) s.st.Unlock() @@ -1675,7 +1675,7 @@ services: s.planChanged(c) // Start service and wait for it to be started - chg := s.startServices(c, []string{"waitdelay"}) + chg := s.startServices(c, [][]string{{"waitdelay"}}) s.st.Lock() c.Assert(chg.Err(), IsNil) s.st.Unlock() @@ -1685,7 +1685,7 @@ services: // Try to stop the service; it will only stop if WaitDelay logic is working, // otherwise the goroutine waiting for the child's stdout will never finish. - chg = s.stopServices(c, []string{"waitdelay"}) + chg = s.stopServices(c, [][]string{{"waitdelay"}}) s.st.Lock() c.Assert(chg.Err(), IsNil) s.st.Unlock() @@ -1818,9 +1818,9 @@ func (s *S) testServiceLogs(c *C, outputs map[string]string) { s.stopTestServices(c) } -func (s *S) startServices(c *C, services []string) *state.Change { +func (s *S) startServices(c *C, lanes [][]string) *state.Change { s.st.Lock() - ts, err := servstate.Start(s.st, services) + ts, err := servstate.Start(s.st, lanes) c.Check(err, IsNil) chg := s.st.NewChange("test", "Start test") chg.AddAll(ts) @@ -1829,9 +1829,9 @@ func (s *S) startServices(c *C, services []string) *state.Change { return chg } -func (s *S) stopServices(c *C, services []string) *state.Change { +func (s *S) stopServices(c *C, lanes [][]string) *state.Change { s.st.Lock() - ts, err := servstate.Stop(s.st, services) + ts, err := servstate.Stop(s.st, lanes) c.Check(err, IsNil) chg := s.st.NewChange("test", "Stop test") chg.AddAll(ts) @@ -1848,7 +1848,7 @@ func (s *S) serviceByName(c *C, name string) *servstate.ServiceInfo { } func (s *S) startTestServices(c *C, logCheck bool) { - chg := s.startServices(c, []string{"test1", "test2"}) + chg := s.startServices(c, [][]string{{"test1", "test2"}}) s.st.Lock() c.Check(chg.Status(), Equals, state.DoneStatus, Commentf("Error: %v", chg.Err())) s.st.Unlock() @@ -1872,7 +1872,7 @@ func (s *S) stopTestServices(c *C) { cmds := s.manager.RunningCmds() c.Check(cmds, HasLen, 2) - chg := s.stopServices(c, []string{"test1", "test2"}) + chg := s.stopServices(c, [][]string{{"test1", "test2"}}) // Ensure processes are gone indeed. c.Assert(cmds, HasLen, 2) @@ -1894,7 +1894,7 @@ func (s *S) stopTestServicesAlreadyDead(c *C) { cmds := s.manager.RunningCmds() c.Check(cmds, HasLen, 0) - chg := s.stopServices(c, []string{"test1", "test2"}) + chg := s.stopServices(c, [][]string{{"test1", "test2"}}) c.Assert(cmds, HasLen, 0) diff --git a/internals/overlord/servstate/request.go b/internals/overlord/servstate/request.go index 49c23f4f3..7746741e7 100644 --- a/internals/overlord/servstate/request.go +++ b/internals/overlord/servstate/request.go @@ -12,37 +12,45 @@ type ServiceRequest struct { } // Start creates and returns a task set for starting the given services. -func Start(s *state.State, services []string) (*state.TaskSet, error) { +func Start(s *state.State, lanes [][]string) (*state.TaskSet, error) { var tasks []*state.Task - for _, name := range services { - task := s.NewTask("start", fmt.Sprintf("Start service %q", name)) - req := ServiceRequest{ - Name: name, + for _, services := range lanes { + lane := s.NewLane() + for i, name := range services { + task := s.NewTask("start", fmt.Sprintf("Start service %q", name)) + req := ServiceRequest{ + Name: name, + } + task.Set("service-request", &req) + task.JoinLane(lane) + // Wait for the previous task in the same lane. + if i > 0 { + task.WaitFor(tasks[len(tasks)-1]) + } + tasks = append(tasks, task) } - task.Set("service-request", &req) - if len(tasks) > 0 { - // TODO Allow non-dependent services to start in parallel. - task.WaitFor(tasks[len(tasks)-1]) - } - tasks = append(tasks, task) } return state.NewTaskSet(tasks...), nil } // Stop creates and returns a task set for stopping the given services. -func Stop(s *state.State, services []string) (*state.TaskSet, error) { +func Stop(s *state.State, lanes [][]string) (*state.TaskSet, error) { var tasks []*state.Task - for _, name := range services { - task := s.NewTask("stop", fmt.Sprintf("Stop service %q", name)) - req := ServiceRequest{ - Name: name, - } - task.Set("service-request", &req) - if len(tasks) > 1 { - // TODO Allow non-dependent services to stop in parallel. - task.WaitFor(tasks[len(tasks)-1]) + for _, services := range lanes { + lane := s.NewLane() + for i, name := range services { + task := s.NewTask("stop", fmt.Sprintf("Stop service %q", name)) + req := ServiceRequest{ + Name: name, + } + task.Set("service-request", &req) + task.JoinLane(lane) + // Wait for the previous task in the same lane. + if i > 0 { + task.WaitFor(tasks[len(tasks)-1]) + } + tasks = append(tasks, task) } - tasks = append(tasks, task) } return state.NewTaskSet(tasks...), nil } @@ -50,18 +58,18 @@ func Stop(s *state.State, services []string) (*state.TaskSet, error) { // StopRunning creates and returns a task set for stopping all running // services. It returns a nil *TaskSet if there are no services to stop. func StopRunning(s *state.State, m *ServiceManager) (*state.TaskSet, error) { - services, err := servicesToStop(m) + lanes, err := servicesToStop(m) if err != nil { return nil, err } - if len(services) == 0 { + if len(lanes) == 0 { return nil, nil } // One change to stop them all. s.Lock() defer s.Unlock() - taskSet, err := Stop(s, services) + taskSet, err := Stop(s, lanes) if err != nil { return nil, err } diff --git a/internals/overlord/servstate/request_test.go b/internals/overlord/servstate/request_test.go index b915ee471..c1bd8ecfd 100644 --- a/internals/overlord/servstate/request_test.go +++ b/internals/overlord/servstate/request_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2014-2020 Canonical Ltd +// Copyright (c) 2014-2024 Canonical Ltd // // This program is free software: you can redistribute it and/or modify // it under the terms of the GNU General Public License version 3 as @@ -21,10 +21,69 @@ import ( ) func (s *S) TestStart(c *C) { + s.newServiceManager(c) + layer := ` +services: + one: + override: replace + command: /bin/sh -c "echo one; sleep 10" + startup: enabled + + two: + override: replace + command: /bin/sh -c "echo two; sleep 10" + startup: enabled +` + s.planAddLayer(c, layer) + s.planChanged(c) + + s.st.Lock() + defer s.st.Unlock() + + tset, err := servstate.Start(s.st, [][]string{{"one"}, {"two"}}) + c.Assert(err, IsNil) + + tasks := tset.Tasks() + c.Assert(len(tasks), Equals, 2) + + c.Assert(tasks[0].Kind(), Equals, "start") + req, err := servstate.TaskServiceRequest(tasks[0]) + c.Assert(err, IsNil) + c.Assert(req.Name, Equals, "one") + + c.Assert(tasks[1].Kind(), Equals, "start") + req, err = servstate.TaskServiceRequest(tasks[1]) + c.Assert(err, IsNil) + c.Assert(req.Name, Equals, "two") + + c.Assert(tasks[0].Lanes()[0], Not(Equals), tasks[1].Lanes()[0]) +} + +func (s *S) TestStartInTheSameLaneAfter(c *C) { + s.newServiceManager(c) + layer := ` +services: + one: + override: replace + command: /bin/sh -c "echo one; sleep 10" + startup: enabled + requires: + - two + + two: + override: replace + command: /bin/sh -c "echo two; sleep 10" + startup: enabled + after: + - one +` + s.planAddLayer(c, layer) + s.planChanged(c) + s.st.Lock() defer s.st.Unlock() - tset, err := servstate.Start(s.st, []string{"one", "two"}) + tset, err := servstate.Start(s.st, [][]string{{"one", "two"}}) c.Assert(err, IsNil) tasks := tset.Tasks() @@ -39,13 +98,58 @@ func (s *S) TestStart(c *C) { req, err = servstate.TaskServiceRequest(tasks[1]) c.Assert(err, IsNil) c.Assert(req.Name, Equals, "two") + + c.Assert(tasks[0].Lanes()[0], Equals, tasks[1].Lanes()[0]) +} + +func (s *S) TestStartInTheSameLaneBefore(c *C) { + s.newServiceManager(c) + layer := ` +services: + one: + override: replace + command: /bin/sh -c "echo one; sleep 10" + startup: enabled + requires: + - two + before: + - two + + two: + override: replace + command: /bin/sh -c "echo two; sleep 10" + startup: enabled +` + s.planAddLayer(c, layer) + s.planChanged(c) + + s.st.Lock() + defer s.st.Unlock() + + tset, err := servstate.Start(s.st, [][]string{{"one", "two"}}) + c.Assert(err, IsNil) + + tasks := tset.Tasks() + c.Assert(len(tasks), Equals, 2) + + c.Assert(tasks[0].Kind(), Equals, "start") + req, err := servstate.TaskServiceRequest(tasks[0]) + c.Assert(err, IsNil) + c.Assert(req.Name, Equals, "one") + + c.Assert(tasks[1].Kind(), Equals, "start") + req, err = servstate.TaskServiceRequest(tasks[1]) + c.Assert(err, IsNil) + c.Assert(req.Name, Equals, "two") + + c.Assert(tasks[0].Lanes()[0], Equals, tasks[1].Lanes()[0]) } func (s *S) TestStop(c *C) { s.st.Lock() defer s.st.Unlock() - tset, err := servstate.Stop(s.st, []string{"one", "two"}) + tset, err := servstate.Stop(s.st, [][]string{{"one", "two"}}) c.Assert(err, IsNil) tasks := tset.Tasks() diff --git a/internals/plan/plan.go b/internals/plan/plan.go index 2d5f3bcdc..1b0a7ad87 100644 --- a/internals/plan/plan.go +++ b/internals/plan/plan.go @@ -1,4 +1,4 @@ -// Copyright (c) 2021 Canonical Ltd +// Copyright (c) 2024 Canonical Ltd // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -924,16 +924,81 @@ func (p *Plan) Validate() error { // services to be properly started, in the order that they must be started. // An error is returned when a provided service name does not exist, or there // is an order cycle involving the provided service or its dependencies. -func (p *Plan) StartOrder(names []string) ([]string, error) { - return order(p.Services, names, false) +func (p *Plan) StartOrder(names []string) ([][]string, error) { + orderedNames, err := order(p.Services, names, false) + if err != nil { + return nil, err + } + + return createLanes(orderedNames, p.Services) } // StopOrder returns the required services that must be stopped for the named // services to be properly stopped, in the order that they must be stopped. // An error is returned when a provided service name does not exist, or there // is an order cycle involving the provided service or its dependencies. -func (p *Plan) StopOrder(names []string) ([]string, error) { - return order(p.Services, names, true) +func (p *Plan) StopOrder(names []string) ([][]string, error) { + orderedNames, err := order(p.Services, names, true) + if err != nil { + return nil, err + } + + return createLanes(orderedNames, p.Services) +} + +func getOrCreateLane(currentLane int, service *Service, serviceLaneMapping map[string]int) int { + // if the service has been mapped to a lane + if lane, ok := serviceLaneMapping[service.Name]; ok { + mapServiceToLane(service, lane, serviceLaneMapping) + return lane + } + + // if any dependency has been mapped to a lane + for _, dependency := range service.Requires { + if lane, ok := serviceLaneMapping[dependency]; ok { + mapServiceToLane(service, lane, serviceLaneMapping) + return lane + } + } + + // neither the service itself nor any of its dependencies is mapped to an existing lane + lane := currentLane + 1 + mapServiceToLane(service, lane, serviceLaneMapping) + return lane +} + +func mapServiceToLane(service *Service, lane int, serviceLaneMapping map[string]int) { + serviceLaneMapping[service.Name] = lane + + // map the service's dependencies to the same lane + for _, dependency := range service.Requires { + serviceLaneMapping[dependency] = lane + } +} + +func createLanes(names []string, services map[string]*Service) ([][]string, error) { + serviceLaneMapping := make(map[string]int) + + // Map all services into lanes. + var lane = -1 + for _, name := range names { + service, ok := services[name] + if !ok { + return nil, &FormatError{ + Message: fmt.Sprintf("service %q does not exist", name), + } + } + + lane = getOrCreateLane(lane, service, serviceLaneMapping) + } + + // Create lanes + lanes := make([][]string, lane+1) + for _, service := range names { + lane := serviceLaneMapping[service] + lanes[lane] = append(lanes[lane], service) + } + return lanes, nil } func order(services map[string]*Service, names []string, stop bool) ([]string, error) { diff --git a/internals/plan/plan_test.go b/internals/plan/plan_test.go index 3a99fcdf4..d334699e6 100644 --- a/internals/plan/plan_test.go +++ b/internals/plan/plan_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2020 Canonical Ltd +// Copyright (c) 2024 Canonical Ltd // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -1427,15 +1427,23 @@ func (s *S) TestParseLayer(c *C) { if err == nil { for name, order := range test.start { p := plan.Plan{Services: result.Services} - names, err := p.StartOrder([]string{name}) + lanes, err := p.StartOrder([]string{name}) c.Assert(err, IsNil) - c.Assert(names, DeepEquals, order) + for _, names := range lanes { + if len(names) > 0 { + c.Assert(names, DeepEquals, order) + } + } } for name, order := range test.stop { p := plan.Plan{Services: result.Services} - names, err := p.StopOrder([]string{name}) + lanes, err := p.StopOrder([]string{name}) c.Assert(err, IsNil) - c.Assert(names, DeepEquals, order) + for _, names := range lanes { + if len(names) > 0 { + c.Assert(names, DeepEquals, order) + } + } } } if err == nil { @@ -1575,15 +1583,23 @@ func (s *S) TestReadDir(c *C) { if err == nil { for name, order := range test.start { p := plan.Plan{Services: result.Services} - names, err := p.StartOrder([]string{name}) + lanes, err := p.StartOrder([]string{name}) c.Assert(err, IsNil) - c.Assert(names, DeepEquals, order) + for _, names := range lanes { + if len(names) > 0 { + c.Assert(names, DeepEquals, order) + } + } } for name, order := range test.stop { p := plan.Plan{Services: result.Services} - names, err := p.StopOrder([]string{name}) + lanes, err := p.StopOrder([]string{name}) c.Assert(err, IsNil) - c.Assert(names, DeepEquals, order) + for _, names := range lanes { + if len(names) > 0 { + c.Assert(names, DeepEquals, order) + } + } } } } @@ -1939,3 +1955,93 @@ func (s *S) TestPebbleLabelPrefixReserved(c *C) { _, err := plan.ParseLayer(0, "pebble-foo", []byte("{}")) c.Check(err, ErrorMatches, `cannot use reserved label prefix "pebble-"`) } + +func (s *S) TestStartStopOrderSingleLane(c *C) { + layer := &plan.Layer{ + Summary: "services with dependencies in the same lane", + Description: "a simple layer", + Services: map[string]*plan.Service{ + "srv1": { + Name: "srv1", + Override: "replace", + Command: `cmd`, + Requires: []string{"srv2"}, + Before: []string{"srv2"}, + Startup: plan.StartupEnabled, + }, + "srv2": { + Name: "srv2", + Override: "replace", + Command: `cmd`, + Requires: []string{"srv3"}, + Before: []string{"srv3"}, + Startup: plan.StartupEnabled, + }, + "srv3": { + Name: "srv3", + Override: "replace", + Command: `cmd`, + Startup: plan.StartupEnabled, + }, + }, + Checks: map[string]*plan.Check{}, + LogTargets: map[string]*plan.LogTarget{}, + } + + p := plan.Plan{Services: layer.Services} + + lanes, err := p.StartOrder([]string{"srv1", "srv2", "srv3"}) + c.Assert(err, IsNil) + c.Assert(len(lanes), Equals, 1) + c.Assert(lanes[0], DeepEquals, []string{"srv1", "srv2", "srv3"}) + + lanes, err = p.StopOrder([]string{"srv1", "srv2", "srv3"}) + c.Assert(err, IsNil) + c.Assert(len(lanes), Equals, 1) + c.Assert(lanes[0], DeepEquals, []string{"srv3", "srv2", "srv1"}) +} + +func (s *S) TestStartStopOrderMultipleLanes(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, + }, + }, + Checks: map[string]*plan.Check{}, + LogTargets: map[string]*plan.LogTarget{}, + } + + p := plan.Plan{Services: layer.Services} + + lanes, err := p.StartOrder([]string{"srv1", "srv2", "srv3"}) + c.Assert(err, IsNil) + c.Assert(len(lanes), Equals, 3) + c.Assert(lanes[0], DeepEquals, []string{"srv1"}) + c.Assert(lanes[1], DeepEquals, []string{"srv2"}) + c.Assert(lanes[2], DeepEquals, []string{"srv3"}) + + lanes, err = p.StopOrder([]string{"srv1", "srv2", "srv3"}) + c.Assert(err, IsNil) + c.Assert(len(lanes), Equals, 3) + c.Assert(lanes[0], DeepEquals, []string{"srv1"}) + c.Assert(lanes[1], DeepEquals, []string{"srv2"}) + c.Assert(lanes[2], DeepEquals, []string{"srv3"}) +}