From 177790aa861c50ff9abf9675fb794e63fa85496a Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Wed, 1 Feb 2023 14:27:46 +0600 Subject: [PATCH 01/22] Add optional arguments in Pebble layer schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The goal is to introduce the concept of optional and overrideable arguments for a Pebble service, but we must ensure that the layering properties of Pebble are preserved and respected. The proposed syntax re-uses therefore the existing service “command” attribute: services: myservice: command: myservice --db=/var/db [ --quiet ] The markers [ and ] are suggested because these are typical annotations for optional parameters and are rarely seen naked because they are interpreted by shells. --- internal/overlord/servstate/handlers.go | 8 ++-- internal/plan/plan.go | 52 ++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/internal/overlord/servstate/handlers.go b/internal/overlord/servstate/handlers.go index a3c562233..3bd85a6bf 100644 --- a/internal/overlord/servstate/handlers.go +++ b/internal/overlord/servstate/handlers.go @@ -325,11 +325,9 @@ func logError(err error) { // command. It assumes the caller has ensures the service is in a valid state, // and it sets s.cmd and other relevant fields. func (s *serviceData) startInternal() error { - args, err := shlex.Split(s.config.Command) + args, err := s.config.GetCommand() if err != nil { - // Shouldn't happen as it should have failed on parsing, but - // it does not hurt to double check and report. - return fmt.Errorf("cannot parse service command: %s", err) + return err } s.cmd = exec.Command(args[0], args[1:]...) s.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} @@ -386,7 +384,7 @@ func (s *serviceData) startInternal() error { s.cmd.Stderr = logWriter // Start the process! - logger.Noticef("Service %q starting: %s", serviceName, s.config.Command) + logger.Noticef("Service %q starting: %q", serviceName, args) err = reaper.StartCommand(s.cmd) if err != nil { if outputIterator != nil { diff --git a/internal/plan/plan.go b/internal/plan/plan.go index e2fbbf285..a62b5a3e6 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -65,6 +65,7 @@ type Service struct { Startup ServiceStartup `yaml:"startup,omitempty"` Override Override `yaml:"override,omitempty"` Command string `yaml:"command,omitempty"` + cmdArgs []string // Service dependencies After []string `yaml:"after,omitempty"` @@ -90,6 +91,7 @@ type Service struct { // Copy returns a deep copy of the service. func (s *Service) Copy() *Service { copied := *s + copied.cmdArgs = append([]string(nil), s.cmdArgs...) copied.After = append([]string(nil), s.After...) copied.Before = append([]string(nil), s.Before...) copied.Requires = append([]string(nil), s.Requires...) @@ -129,6 +131,7 @@ func (s *Service) Merge(other *Service) { } if other.Command != "" { s.Command = other.Command + s.cmdArgs = append([]string(nil), other.cmdArgs...) } if other.UserID != nil { userID := *other.UserID @@ -184,6 +187,53 @@ func (s *Service) Equal(other *Service) bool { return reflect.DeepEqual(s, other) } +// Returns the command as a stream of strings. +// Filters "[", "]" (if present, denoting optional arguments). +func (s *Service) GetCommand() ([]string, error) { + args, err := shlex.Split(s.Command) + if err != nil { + return nil, fmt.Errorf("cannot parse service %q command: %s", s.Name, err) + } + fargs := make([]string, 0) + for _, arg := range args { + if arg == "[" || arg == "]" { + if len(s.cmdArgs) > 0 { + break + } else { + continue + } + } + fargs = append(fargs, arg) + } + for _, arg := range s.cmdArgs { + fargs = append(fargs, arg) + } + return fargs, nil +} + +func (s *Service) checkCommand() error { + args, err := shlex.Split(s.Command) + if err != nil { + return err + } + leftCnt := 0 + rightCnt := 0 + for _, arg := range args { + if arg == "[" { + leftCnt++ + } + if arg == "]" { + rightCnt++ + } + } + if leftCnt > 0 || rightCnt > 0 { + if leftCnt != 1 || rightCnt != 1 || args[len(args)-1] != "]" { + return fmt.Errorf("bad syntax regarding optional/overridable arguments") + } + } + return nil +} + type ServiceStartup string const ( @@ -483,7 +533,7 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { Message: fmt.Sprintf(`plan must define "command" for service %q`, name), } } - _, err := shlex.Split(service.Command) + err := service.checkCommand() if err != nil { return nil, &FormatError{ Message: fmt.Sprintf("plan service %q command invalid: %v", name, err), From 4f0ea607c127ece3f32b1cf98d64a75aea7b69c1 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Tue, 7 Feb 2023 13:54:30 +0600 Subject: [PATCH 02/22] feat: Support --args functionality in CLI Arguments to a service can now be passed by using the following syntax: pebble run [opts..] --args arguments.. ; [opts..] Tests are failing right now, will be fixed soon. --- client/services.go | 33 ++++++++++++++------ cmd/pebble/cmd_run.go | 42 +++++++++++++++++++++++--- internal/daemon/api_services.go | 16 ++++++++-- internal/overlord/servstate/manager.go | 10 ++++++ internal/plan/plan.go | 27 +++++++++++++++++ 5 files changed, 111 insertions(+), 17 deletions(-) diff --git a/client/services.go b/client/services.go index 0be19d23e..48f8de15c 100644 --- a/client/services.go +++ b/client/services.go @@ -25,50 +25,63 @@ import ( type ServiceOptions struct { Names []string + Args map[string][]string } // AutoStart starts the services makes as "startup: enabled". opts.Names must // be empty for this call. func (client *Client) AutoStart(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("autostart", opts.Names) + _, changeID, err = client.doMultiServiceAction("autostart", &multiActionOptions{services: opts.Names}) return changeID, err } // Start starts the services named in opts.Names in dependency order. func (client *Client) Start(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("start", opts.Names) + _, changeID, err = client.doMultiServiceAction("start", &multiActionOptions{services: opts.Names}) return changeID, err } // Stop stops the services named in opts.Names in dependency order. func (client *Client) Stop(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("stop", opts.Names) + _, changeID, err = client.doMultiServiceAction("stop", &multiActionOptions{services: opts.Names}) return changeID, err } // Restart stops and then starts the services named in opts.Names in // dependency order. func (client *Client) Restart(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("restart", opts.Names) + _, changeID, err = client.doMultiServiceAction("restart", &multiActionOptions{services: opts.Names}) return changeID, err } // Replan stops and (re)starts the services whose configuration has changed // since they were started. opts.Names must be empty for this call. func (client *Client) Replan(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("replan", opts.Names) + _, changeID, err = client.doMultiServiceAction("replan", &multiActionOptions{services: opts.Names}) return changeID, err } +func (client *Client) PassServiceArgs(opts *ServiceOptions) (changeID string, err error) { + _, changeID, err = client.doMultiServiceAction("pass-args", &multiActionOptions{serviceArgs: opts.Args}) + return changeID, err +} + +type multiActionOptions struct { + services []string + serviceArgs map[string][]string +} + type multiActionData struct { - Action string `json:"action"` - Services []string `json:"services"` + Action string `json:"action"` + Services []string `json:"services"` + ServiceArgs map[string][]string `json:"service-args"` } -func (client *Client) doMultiServiceAction(actionName string, services []string) (result json.RawMessage, changeID string, err error) { +func (client *Client) doMultiServiceAction(actionName string, opts *multiActionOptions) (result json.RawMessage, changeID string, err error) { action := multiActionData{ - Action: actionName, - Services: services, + Action: actionName, + Services: opts.services, + ServiceArgs: opts.serviceArgs, } data, err := json.Marshal(&action) if err != nil { diff --git a/cmd/pebble/cmd_run.go b/cmd/pebble/cmd_run.go index 78d63f6b6..c6a649b08 100644 --- a/cmd/pebble/cmd_run.go +++ b/cmd/pebble/cmd_run.go @@ -39,10 +39,11 @@ The run command starts pebble and runs the configured environment. type cmdRun struct { clientMixin - CreateDirs bool `long:"create-dirs"` - Hold bool `long:"hold"` - HTTP string `long:"http"` - Verbose bool `short:"v" long:"verbose"` + CreateDirs bool `long:"create-dirs"` + Hold bool `long:"hold"` + HTTP string `long:"http"` + Verbose bool `short:"v" long:"verbose"` + Args [][]string `long:"args" terminator:";"` } func init() { @@ -52,6 +53,7 @@ func init() { "hold": "Do not start default services automatically", "http": `Start HTTP API listening on this address (e.g., ":4000")`, "verbose": "Log all output from services to stdout", + "args": "Pass terminated arguments", }, nil) } @@ -166,6 +168,20 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal) error { logger.Debugf("activation done in %v", time.Now().Truncate(time.Millisecond).Sub(t0)) + if rcmd.Args != nil { + mappedArgs, err := convertArgs(rcmd.Args) + if err != nil { + logger.Noticef("cannot parse service arguments: %v", err) + } + servopts := client.ServiceOptions{Args: mappedArgs} + changeID, err := rcmd.client.PassServiceArgs(&servopts) + if err != nil { + return fmt.Errorf("cannot pass arguments to services: %v", err) + } else { + logger.Noticef("Passed arguments to services with change %s.", changeID) + } + } + if !rcmd.Hold { servopts := client.ServiceOptions{} changeID, err := rcmd.client.AutoStart(&servopts) @@ -199,3 +215,21 @@ out: return d.Stop(ch) } + +func convertArgs(args [][]string) (map[string][]string, error) { + mappedArgs := make(map[string][]string) + + for _, arg := range args { + if len(arg) < 2 { + continue + } + + name := arg[0] + if _, ok := mappedArgs[name]; ok { + return nil, fmt.Errorf("Passing args twice to a service is not supported, in --args %s", name) + } + mappedArgs[name] = append([]string(nil), arg[1:]...) + } + + return mappedArgs, nil +} diff --git a/internal/daemon/api_services.go b/internal/daemon/api_services.go index 711142fc8..e2cf67685 100644 --- a/internal/daemon/api_services.go +++ b/internal/daemon/api_services.go @@ -61,8 +61,9 @@ func v1GetServices(c *Command, r *http.Request, _ *userState) Response { func v1PostServices(c *Command, r *http.Request, _ *userState) Response { var payload struct { - Action string `json:"action"` - Services []string `json:"services"` + Action string `json:"action"` + Services []string `json:"services"` + ServiceArgs map[string][]string `json:"service-args"` } decoder := json.NewDecoder(r.Body) @@ -93,6 +94,10 @@ func v1PostServices(c *Command, r *http.Request, _ *userState) Response { }) } payload.Services = services + case "pass-args": + if len(payload.Services) != 0 { + return statusBadRequest("%s accepts no service names", payload.Action) + } default: if len(payload.Services) == 0 { return statusBadRequest("no services to %s provided", payload.Action) @@ -176,6 +181,11 @@ func v1PostServices(c *Command, r *http.Request, _ *userState) Response { } sort.Strings(services) payload.Services = services + case "pass-args": + err = servmgr.SetServiceArgs(payload.ServiceArgs) + if err != nil { + break + } default: return statusBadRequest("action %q is unsupported", payload.Action) } @@ -187,7 +197,7 @@ func v1PostServices(c *Command, r *http.Request, _ *userState) Response { // resolved one. But do use the resolved set for the count. var summary string switch { - case len(taskSet.Tasks()) == 0: + case taskSet == nil || len(taskSet.Tasks()) == 0: // Can happen with a replan that has no services to stop/start. A // change with no tasks needs to be marked Done manually (normally a // change is marked Done when its last task is finished). diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index ea0394795..f5a50764e 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -356,6 +356,16 @@ func (m *ServiceManager) StopOrder(services []string) ([]string, error) { return m.plan.StopOrder(services) } +func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { + releasePlan, err := m.acquirePlan() + if err != nil { + return err + } + defer releasePlan() + + return m.plan.SetServiceArgs(serviceArgs) +} + // ServiceLogs returns iterators to the provided services. If last is negative, // return tail iterators; if last is zero or positive, return head iterators // going back last elements. Each iterator must be closed via the Close method. diff --git a/internal/plan/plan.go b/internal/plan/plan.go index a62b5a3e6..6e2943db0 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -211,6 +211,11 @@ func (s *Service) GetCommand() ([]string, error) { return fargs, nil } +func (s *Service) setArgs(args []string) error { + s.cmdArgs = append([]string(nil), args...) + return nil +} + func (s *Service) checkCommand() error { args, err := shlex.Split(s.Command) if err != nil { @@ -672,6 +677,28 @@ func (p *Plan) StopOrder(names []string) ([]string, error) { return order(p.Services, names, true) } +func (p *Plan) SetServiceArgs(serviceArgs map[string][]string) error { + for svcName, svcArgs := range serviceArgs { + service, ok := p.Services[svcName] + if !ok { + return fmt.Errorf("Service %s does not exist in the plan (arguments passed via --args)", svcName) + } + if err := service.setArgs(svcArgs); err != nil { + return err + } + for i := len(p.Layers) - 1; i >= 0; i-- { + layerService, ok := p.Layers[i].Services[svcName] + if ok { + if err := layerService.setArgs(svcArgs); err != nil { + return err + } + break + } + } + } + return nil +} + func order(services map[string]*Service, names []string, stop bool) ([]string, error) { // For stop, create a list of reversed dependencies. predecessors := map[string][]string(nil) From fde872981bb6900ce083c147e7f2460575a37c4c Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Tue, 7 Feb 2023 13:58:11 +0600 Subject: [PATCH 03/22] Fix failing tests --- client/services_test.go | 8 ++++---- cmd/pebble/cmd_autostart_test.go | 20 ++++++++++++-------- cmd/pebble/cmd_replan_test.go | 20 ++++++++++++-------- cmd/pebble/cmd_restart_test.go | 20 ++++++++++++-------- cmd/pebble/cmd_start_test.go | 20 ++++++++++++-------- cmd/pebble/cmd_stop_test.go | 20 ++++++++++++-------- 6 files changed, 64 insertions(+), 44 deletions(-) diff --git a/client/services_test.go b/client/services_test.go index 51eec13ad..8fd08b326 100644 --- a/client/services_test.go +++ b/client/services_test.go @@ -55,7 +55,7 @@ func (cs *clientSuite) TestStartStop(c *check.C) { var body map[string]interface{} c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 2) + c.Check(body, check.HasLen, 3) c.Check(body["action"], check.Equals, action) c.Check(body["services"], check.DeepEquals, []interface{}{"one", "two"}) } @@ -80,7 +80,7 @@ func (cs *clientSuite) TestAutostart(c *check.C) { var body map[string]interface{} c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 2) + c.Check(body, check.HasLen, 3) c.Check(body["action"], check.Equals, "autostart") } @@ -132,7 +132,7 @@ func (cs *clientSuite) TestRestart(c *check.C) { var body map[string]interface{} c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 2) + c.Check(body, check.HasLen, 3) c.Check(body["action"], check.Equals, "restart") c.Check(body["services"], check.DeepEquals, []interface{}{"one", "two"}) } @@ -156,6 +156,6 @@ func (cs *clientSuite) TestReplan(c *check.C) { var body map[string]interface{} c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 2) + c.Check(body, check.HasLen, 3) c.Check(body["action"], check.Equals, "replan") } diff --git a/cmd/pebble/cmd_autostart_test.go b/cmd/pebble/cmd_autostart_test.go index 4872f938f..6bda8dc7c 100644 --- a/cmd/pebble/cmd_autostart_test.go +++ b/cmd/pebble/cmd_autostart_test.go @@ -56,8 +56,9 @@ func (s *PebbleSuite) TestAutostart(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "autostart", - "services": nil, + "action": "autostart", + "services": nil, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -80,8 +81,9 @@ func (s *PebbleSuite) TestAutostartFailsNoDefaultServices(c *check.C) { c.Check(r.URL.Path, check.Equals, "/v1/services") body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "autostart", - "services": nil, + "action": "autostart", + "services": nil, + "service-args": nil, }) fmt.Fprint(w, `{ @@ -106,8 +108,9 @@ func (s *PebbleSuite) TestAutostartNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "autostart", - "services": nil, + "action": "autostart", + "services": nil, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -137,8 +140,9 @@ func (s *PebbleSuite) TestAutostartFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "autostart", - "services": nil, + "action": "autostart", + "services": nil, + "service-args": nil, }) fmt.Fprintf(w, `{ diff --git a/cmd/pebble/cmd_replan_test.go b/cmd/pebble/cmd_replan_test.go index da13fdb0d..498e805b6 100644 --- a/cmd/pebble/cmd_replan_test.go +++ b/cmd/pebble/cmd_replan_test.go @@ -56,8 +56,9 @@ func (s *PebbleSuite) TestReplan(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "replan", - "services": nil, + "action": "replan", + "services": nil, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -80,8 +81,9 @@ func (s *PebbleSuite) TestReplanFailsNoDefaultServices(c *check.C) { c.Check(r.URL.Path, check.Equals, "/v1/services") body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "replan", - "services": nil, + "action": "replan", + "services": nil, + "service-args": nil, }) fmt.Fprint(w, `{ @@ -106,8 +108,9 @@ func (s *PebbleSuite) TestReplanNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "replan", - "services": nil, + "action": "replan", + "services": nil, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -137,8 +140,9 @@ func (s *PebbleSuite) TestReplanFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "replan", - "services": nil, + "action": "replan", + "services": nil, + "service-args": nil, }) fmt.Fprintf(w, `{ diff --git a/cmd/pebble/cmd_restart_test.go b/cmd/pebble/cmd_restart_test.go index 624fdc2d8..4f2412e78 100644 --- a/cmd/pebble/cmd_restart_test.go +++ b/cmd/pebble/cmd_restart_test.go @@ -48,8 +48,9 @@ func (s *PebbleSuite) TestRestart(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "restart", - "services": []interface{}{"srv1", "srv2"}, + "action": "restart", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -73,8 +74,9 @@ func (s *PebbleSuite) TestRestartFails(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "restart", - "services": []interface{}{"srv1", "srv3"}, + "action": "restart", + "services": []interface{}{"srv1", "srv3"}, + "service-args": nil, }) fmt.Fprintf(w, `{"type": "error", "result": {"message": "could not foo"}}`) @@ -95,8 +97,9 @@ func (s *PebbleSuite) TestRestartNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "restart", - "services": []interface{}{"srv1", "srv2"}, + "action": "restart", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -126,8 +129,9 @@ func (s *PebbleSuite) TestRestartFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "restart", - "services": []interface{}{"srv1", "srv2"}, + "action": "restart", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ diff --git a/cmd/pebble/cmd_start_test.go b/cmd/pebble/cmd_start_test.go index 4cd0f2e79..2ffb6ed7f 100644 --- a/cmd/pebble/cmd_start_test.go +++ b/cmd/pebble/cmd_start_test.go @@ -48,8 +48,9 @@ func (s *PebbleSuite) TestStart(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "start", - "services": []interface{}{"srv1", "srv2"}, + "action": "start", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -73,8 +74,9 @@ func (s *PebbleSuite) TestStartFails(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "start", - "services": []interface{}{"srv1", "srv3"}, + "action": "start", + "services": []interface{}{"srv1", "srv3"}, + "service-args": nil, }) fmt.Fprintf(w, `{"type": "error", "result": {"message": "could not foo"}}`) @@ -95,8 +97,9 @@ func (s *PebbleSuite) TestStartNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "start", - "services": []interface{}{"srv1", "srv2"}, + "action": "start", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -126,8 +129,9 @@ func (s *PebbleSuite) TestStartFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "start", - "services": []interface{}{"srv1", "srv2"}, + "action": "start", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ diff --git a/cmd/pebble/cmd_stop_test.go b/cmd/pebble/cmd_stop_test.go index 8e4c1c83d..2ba4d3665 100644 --- a/cmd/pebble/cmd_stop_test.go +++ b/cmd/pebble/cmd_stop_test.go @@ -48,8 +48,9 @@ func (s *PebbleSuite) TestStop(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "stop", - "services": []interface{}{"srv1", "srv2"}, + "action": "stop", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -73,8 +74,9 @@ func (s *PebbleSuite) TestStopFails(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "stop", - "services": []interface{}{"srv1", "srv3"}, + "action": "stop", + "services": []interface{}{"srv1", "srv3"}, + "service-args": nil, }) fmt.Fprintf(w, `{"type": "error", "result": {"message": "could not foo"}}`) @@ -95,8 +97,9 @@ func (s *PebbleSuite) TestStopNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "stop", - "services": []interface{}{"srv1", "srv2"}, + "action": "stop", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ @@ -126,8 +129,9 @@ func (s *PebbleSuite) TestStopFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "stop", - "services": []interface{}{"srv1", "srv2"}, + "action": "stop", + "services": []interface{}{"srv1", "srv2"}, + "service-args": nil, }) fmt.Fprintf(w, `{ From 994d5fe303b3643ad9cfae94e5dd818d35ae41b9 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Tue, 7 Feb 2023 17:51:45 +0600 Subject: [PATCH 04/22] Point to go-flags fork The upstream go-flags project do not support terminated options like ``find -exec``. The repo was forked and the feature was introduced in [0]. Until the PR [1] is merged in the upstream repo, we need to maintain and point to a fork with the features there. Refs: - [0] https://github.com/rebornplusplus/go-flags/commit/1dbaf4443e95284d13e257753890c91331a8ac60 - [1] https://github.com/jessevdk/go-flags/pull/395 --- go.mod | 4 +++- go.sum | 14 ++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index b24a6b458..a1a922941 100644 --- a/go.mod +++ b/go.mod @@ -9,8 +9,10 @@ require ( github.com/jessevdk/go-flags v1.4.0 github.com/pkg/term v1.1.0 golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad - golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 + golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637 gopkg.in/yaml.v3 v3.0.1 ) + +replace github.com/jessevdk/go-flags v1.4.0 => github.com/rebornplusplus/go-flags v0.0.0-20230201113816-1dbaf4443e95 diff --git a/go.sum b/go.sum index 03507b7e5..f8f59bc9e 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,6 @@ github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/jessevdk/go-flags v1.4.0 h1:4IU2WS7AumrZ/40jfhf4QVDMsQwqA7VEHozFRrGARJA= -github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= @@ -13,6 +11,8 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= +github.com/rebornplusplus/go-flags v0.0.0-20230201113816-1dbaf4443e95 h1:3Ty6zGRkREQRPkPCG8EW2pf4HwtICpdQNdw15LMo+30= +github.com/rebornplusplus/go-flags v0.0.0-20230201113816-1dbaf4443e95/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= @@ -21,14 +21,8 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4 h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k= -golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= -golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A= -golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ= -golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4 h1:EZ2mChiOa8udjfp6rRmswTbtZN/QzUQp4ptM4rnjHvc= +golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221 h1:/ZHdbVpdR/jk3g30/d4yUL0JU9kksj8+F/bnQUVLGDM= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= From 14b98c8dd50ae5499b01468a5c43db554b26f601 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Wed, 8 Feb 2023 17:08:58 +0600 Subject: [PATCH 05/22] Add comments for reference, fix a few minor things --- client/services.go | 1 + cmd/pebble/cmd_run.go | 15 ++++++--- internal/daemon/api_services.go | 4 +-- internal/overlord/servstate/handlers.go | 3 +- internal/overlord/servstate/manager.go | 2 ++ internal/plan/plan.go | 41 +++++++++++++++++++------ 6 files changed, 48 insertions(+), 18 deletions(-) diff --git a/client/services.go b/client/services.go index 48f8de15c..71467c95a 100644 --- a/client/services.go +++ b/client/services.go @@ -61,6 +61,7 @@ func (client *Client) Replan(opts *ServiceOptions) (changeID string, err error) return changeID, err } +// PassServiceArgs sets the service arguments provided by "pebble run --args" func (client *Client) PassServiceArgs(opts *ServiceOptions) (changeID string, err error) { _, changeID, err = client.doMultiServiceAction("pass-args", &multiActionOptions{serviceArgs: opts.Args}) return changeID, err diff --git a/cmd/pebble/cmd_run.go b/cmd/pebble/cmd_run.go index c6a649b08..11f8136e6 100644 --- a/cmd/pebble/cmd_run.go +++ b/cmd/pebble/cmd_run.go @@ -53,7 +53,7 @@ func init() { "hold": "Do not start default services automatically", "http": `Start HTTP API listening on this address (e.g., ":4000")`, "verbose": "Log all output from services to stdout", - "args": "Pass terminated arguments", + "args": `Provide arguments to service (e.g., "SERVICE ARGS.. ; ")`, }, nil) } @@ -171,7 +171,7 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal) error { if rcmd.Args != nil { mappedArgs, err := convertArgs(rcmd.Args) if err != nil { - logger.Noticef("cannot parse service arguments: %v", err) + return err } servopts := client.ServiceOptions{Args: mappedArgs} changeID, err := rcmd.client.PassServiceArgs(&servopts) @@ -216,17 +216,22 @@ out: return d.Stop(ch) } +// convert args from [][]string type to map[string][]string +// and check for empty or duplicated --args usage func convertArgs(args [][]string) (map[string][]string, error) { mappedArgs := make(map[string][]string) for _, arg := range args { - if len(arg) < 2 { - continue + switch len(arg) { + case 0: + return nil, fmt.Errorf("--args cannot have empty arguments") + case 1: + return nil, fmt.Errorf("cannot pass empty arguments to service %q", arg[0]) } name := arg[0] if _, ok := mappedArgs[name]; ok { - return nil, fmt.Errorf("Passing args twice to a service is not supported, in --args %s", name) + return nil, fmt.Errorf("cannot pass args twice to service %q", name) } mappedArgs[name] = append([]string(nil), arg[1:]...) } diff --git a/internal/daemon/api_services.go b/internal/daemon/api_services.go index e2cf67685..e1fc0d60d 100644 --- a/internal/daemon/api_services.go +++ b/internal/daemon/api_services.go @@ -198,8 +198,8 @@ func v1PostServices(c *Command, r *http.Request, _ *userState) Response { var summary string switch { case taskSet == nil || len(taskSet.Tasks()) == 0: - // Can happen with a replan that has no services to stop/start. A - // change with no tasks needs to be marked Done manually (normally a + // Can happen with a pass-args or replan that has no services to stop/start. + // A change with no tasks needs to be marked Done manually (normally a // change is marked Done when its last task is finished). summary = fmt.Sprintf("%s - no services", strings.Title(payload.Action)) change := st.NewChange(payload.Action, summary) diff --git a/internal/overlord/servstate/handlers.go b/internal/overlord/servstate/handlers.go index 3bd85a6bf..ecadb9a58 100644 --- a/internal/overlord/servstate/handlers.go +++ b/internal/overlord/servstate/handlers.go @@ -10,7 +10,6 @@ import ( "syscall" "time" - "github.com/canonical/x-go/strutil/shlex" "golang.org/x/sys/unix" "gopkg.in/tomb.v2" @@ -384,7 +383,7 @@ func (s *serviceData) startInternal() error { s.cmd.Stderr = logWriter // Start the process! - logger.Noticef("Service %q starting: %q", serviceName, args) + logger.Noticef("Service %q starting: %v", serviceName, s.config.GetCommandStr(args)) err = reaper.StartCommand(s.cmd) if err != nil { if outputIterator != nil { diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index f5a50764e..7725187fd 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -356,6 +356,8 @@ func (m *ServiceManager) StopOrder(services []string) ([]string, error) { return m.plan.StopOrder(services) } +// SetServiceArgs sets the service arguments provided via "pebble run --args" +// in the plan. See plan.SetServiceArgs. func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { releasePlan, err := m.acquirePlan() if err != nil { diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 6e2943db0..60ee68375 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -65,7 +65,7 @@ type Service struct { Startup ServiceStartup `yaml:"startup,omitempty"` Override Override `yaml:"override,omitempty"` Command string `yaml:"command,omitempty"` - cmdArgs []string + CmdArgs []string `yaml:"-"` // Service dependencies After []string `yaml:"after,omitempty"` @@ -91,7 +91,7 @@ type Service struct { // Copy returns a deep copy of the service. func (s *Service) Copy() *Service { copied := *s - copied.cmdArgs = append([]string(nil), s.cmdArgs...) + copied.CmdArgs = append([]string(nil), s.CmdArgs...) copied.After = append([]string(nil), s.After...) copied.Before = append([]string(nil), s.Before...) copied.Requires = append([]string(nil), s.Requires...) @@ -131,7 +131,7 @@ func (s *Service) Merge(other *Service) { } if other.Command != "" { s.Command = other.Command - s.cmdArgs = append([]string(nil), other.cmdArgs...) + s.CmdArgs = append([]string(nil), other.CmdArgs...) } if other.UserID != nil { userID := *other.UserID @@ -187,8 +187,8 @@ func (s *Service) Equal(other *Service) bool { return reflect.DeepEqual(s, other) } -// Returns the command as a stream of strings. -// Filters "[", "]" (if present, denoting optional arguments). +// GetCommand overrides the default arguments (inside [ ]) if +// CmdArgs is present and returns the command as a stream of strings. func (s *Service) GetCommand() ([]string, error) { args, err := shlex.Split(s.Command) if err != nil { @@ -197,7 +197,7 @@ func (s *Service) GetCommand() ([]string, error) { fargs := make([]string, 0) for _, arg := range args { if arg == "[" || arg == "]" { - if len(s.cmdArgs) > 0 { + if len(s.CmdArgs) > 0 { break } else { continue @@ -205,14 +205,32 @@ func (s *Service) GetCommand() ([]string, error) { } fargs = append(fargs, arg) } - for _, arg := range s.cmdArgs { + for _, arg := range s.CmdArgs { fargs = append(fargs, arg) } return fargs, nil } +// GetCommandStr takes in the output of GetCommand and +// returns the command as a string in good print format +func (s *Service) GetCommandStr(cmd []string) string { + f := func(str string) string { + for _, c := range str { + if c < '!' { + return strconv.Quote(str) + } + } + return str + } + args := make([]string, 0) + for _, arg := range cmd { + args = append(args, f(arg)) + } + return strings.Join(args, " ") +} + func (s *Service) setArgs(args []string) error { - s.cmdArgs = append([]string(nil), args...) + s.CmdArgs = append([]string(nil), args...) return nil } @@ -677,11 +695,16 @@ func (p *Plan) StopOrder(names []string) ([]string, error) { return order(p.Services, names, true) } +// SetServiceArgs sets the service arguments provided by "pebble run --args" +// to their respective services. Additionally, for a service the arguments +// are also updated in the layer of the highest order that defines the service +// so that the arguments to a service can persist if it is not affected +// by a "pebble add" layer or similar. func (p *Plan) SetServiceArgs(serviceArgs map[string][]string) error { for svcName, svcArgs := range serviceArgs { service, ok := p.Services[svcName] if !ok { - return fmt.Errorf("Service %s does not exist in the plan (arguments passed via --args)", svcName) + return fmt.Errorf("Service %q does not exist in the plan (arguments passed via --args)", svcName) } if err := service.setArgs(svcArgs); err != nil { return err From 906b6dfa9358a33d32be50b8f784615449aebf26 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Wed, 8 Feb 2023 17:10:38 +0600 Subject: [PATCH 06/22] Add tests for ``pebble run --args`` --- client/services_test.go | 32 ++++++++ internal/plan/plan_test.go | 161 +++++++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+) diff --git a/client/services_test.go b/client/services_test.go index 8fd08b326..32628e854 100644 --- a/client/services_test.go +++ b/client/services_test.go @@ -159,3 +159,35 @@ func (cs *clientSuite) TestReplan(c *check.C) { c.Check(body, check.HasLen, 3) c.Check(body["action"], check.Equals, "replan") } + +func (cs *clientSuite) TestPassArgs(c *check.C) { + cs.rsp = `{ + "result": {}, + "status": "OK", + "status-code": 202, + "type": "async", + "change": "42" + }` + + opts := client.ServiceOptions{ + Args: map[string][]string{ + "one": {"--foo", "bar"}, + "two": {"-v", "--bar=foo"}, + }, + } + + changeId, err := cs.cli.PassServiceArgs(&opts) + c.Check(err, check.IsNil) + c.Check(changeId, check.Equals, "42") + c.Check(cs.req.Method, check.Equals, "POST") + c.Check(cs.req.URL.Path, check.Equals, "/v1/services") + + var body map[string]interface{} + c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) + c.Check(body, check.HasLen, 3) + c.Check(body["action"], check.Equals, "pass-args") + c.Check(body["service-args"], check.DeepEquals, map[string]interface{}{ + "one": []interface{}{"--foo", "bar"}, + "two": []interface{}{"-v", "--bar=foo"}, + }) +} diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index a29d6d309..0ffdcd3fb 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -476,6 +476,35 @@ var planTests = []planTest{{ override: replace command: foo ' `}, +}, { + summary: `Optional/overridable arguments in service command`, + input: []string{` + services: + "svc1": + override: replace + command: cmd -v [ --foo bar -e "x [ y ] z" ] + `}, + layers: []*plan.Layer{{ + Order: 0, + Label: "layer-0", + Services: map[string]*plan.Service{ + "svc1": { + Name: "svc1", + Override: "replace", + Command: `cmd -v [ --foo bar -e "x [ y ] z" ]`, + }, + }, + Checks: map[string]*plan.Check{}, + }}, +}, { + summary: `Invalid syntax in Optional/overridable arguments to service command`, + error: `plan service "svc1" command invalid: bad syntax regarding optional/overridable arguments`, + input: []string{` + services: + "svc1": + override: replace + command: cmd -v [ --foo ] bar + `}, }, { summary: "Checks fields parse correctly and defaults are correct", input: []string{` @@ -1017,3 +1046,135 @@ func (s *S) TestMarshalLayer(c *C) { c.Assert(err, IsNil) c.Assert(string(out), Equals, string(layerBytes)) } + +var svcArgsTests = []struct { + summary string + input []string + moreInput string + serviceArgs map[string][]string + err string + result plan.Plan +}{ + { + summary: `plan.Services and plan.Layers should be updated accordingly`, + input: []string{` + services: + svc1: + override: replace + command: svc1cmd -v [ --foo bar ] + svc2: + override: replace + command: svc2cmd [ --wait 15 ] + `, ` + services: + svc2: + override: replace + command: newsvc2cmd [ -b -c foo ] + `}, + serviceArgs: map[string][]string{ + "svc1": {"--bar", "foo"}, + "svc2": {"-xyz"}, + }, + result: plan.Plan{ + Layers: []*plan.Layer{ + { + Label: "layer-0", + Services: map[string]*plan.Service{ + "svc1": { + Name: "svc1", + Override: plan.ReplaceOverride, + Command: "svc1cmd -v [ --foo bar ]", + CmdArgs: []string{"--bar", "foo"}, + }, + "svc2": { + Name: "svc2", + Override: plan.ReplaceOverride, + Command: "svc2cmd [ --wait 15 ]", + }, + }, + }, + { + Label: "layer-1", + Services: map[string]*plan.Service{ + "svc2": { + Name: "svc2", + Override: plan.ReplaceOverride, + Command: "newsvc2cmd [ -b -c foo ]", + CmdArgs: []string{"-xyz"}, + }, + }, + }, + }, + Services: map[string]*plan.Service{ + "svc1": { + Name: "svc1", + Override: plan.ReplaceOverride, + Command: "svc1cmd -v [ --foo bar ]", + CmdArgs: []string{"--bar", "foo"}, + BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, + BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, + BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, + }, + "svc2": { + Name: "svc2", + Override: plan.ReplaceOverride, + Command: "newsvc2cmd [ -b -c foo ]", + CmdArgs: []string{"-xyz"}, + BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, + BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, + BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, + }, + }, + }, + }, { + summary: `invalid service name should raise error`, + input: []string{` + services: + svc1: + override: replace + command: svc1cmd -v [ --foo bar ] + `}, + serviceArgs: map[string][]string{ + "foo": {"bar"}, + }, + err: `Service "foo" does not exist in the plan \(arguments passed via --args\)`, + }, +} + +func (s *S) TestSetServiceArgs(c *C) { + for _, test := range svcArgsTests { + var err error + + layers := make([]*plan.Layer, 0) + for i, yml := range test.input { + layer, err := plan.ParseLayer(i, fmt.Sprintf("layer-%d", i), reindent(yml)) + c.Assert(err, IsNil) + layers = append(layers, layer) + } + + combined, err := plan.CombineLayers(layers...) + c.Assert(err, IsNil) + + p := plan.Plan{ + Layers: layers, + Services: combined.Services, + } + + err = p.SetServiceArgs(test.serviceArgs) + if err != nil || test.err != "" { + if test.err != "" { + c.Assert(err, ErrorMatches, test.err) + } else { + c.Assert(err, IsNil) + } + continue + } + + c.Assert(len(p.Layers), Equals, len(test.result.Layers)) + for i := range p.Layers { + c.Assert(fmt.Sprintf("layer-%d", i), Equals, test.result.Layers[i].Label) + c.Assert(p.Layers[i].Services, DeepEquals, test.result.Layers[i].Services) + } + c.Assert(p.Services, DeepEquals, test.result.Services) + } +} From de12f37e97a3dd74f56f83243fae9c54c34c9124 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 16 Feb 2023 16:07:57 +0600 Subject: [PATCH 07/22] WIP: Address Ben's comments, Update README --- README.md | 33 +++- client/services.go | 34 ++-- client/services_test.go | 40 +---- cmd/pebble/cmd_autostart_test.go | 20 +-- cmd/pebble/cmd_replan_test.go | 20 +-- cmd/pebble/cmd_restart_test.go | 20 +-- cmd/pebble/cmd_run.go | 45 +++--- cmd/pebble/cmd_start_test.go | 20 +-- cmd/pebble/cmd_stop_test.go | 20 +-- internal/daemon/api_services.go | 20 +-- internal/daemon/daemon.go | 6 + internal/overlord/overlord.go | 9 ++ internal/overlord/servstate/handlers.go | 9 +- internal/overlord/servstate/manager.go | 35 +++-- internal/plan/plan.go | 129 +++++----------- internal/plan/plan_test.go | 196 +++++++++--------------- 16 files changed, 269 insertions(+), 387 deletions(-) diff --git a/README.md b/README.md index 2c07f14e5..7bb8afa81 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,32 @@ are marked as `startup: enabled` (if you don't want that, use `--hold`). Then other Pebble commands may be used to interact with the running daemon, for example, in another terminal window. +To provide arguments to a service, you can use the `--args` option. The arguments will +override the default arguments (if any) specified in the plan. To indicate the end +of arguments, a terminator `;` is used which must be backslash-escaped if used in +the shell. The terminator maybe omitted for an `--args` if there are no other `run` +options that follow. For example, + +* start the Pebble server and only pass some arguments to “svc”: + + ``` + $ pebble run --args svc --verbose --foo "multi str arg" + ``` + +* start the Pebble server with all services on hold, and give some + args to “svc”: + + ``` + $ pebble run --args svc --verbose --foo "multi str arg" \; --hold + ``` + +* start the Pebble server and pass arguments to multiple services: + + ``` + $ pebble run --args svc1 --verbose --foo "multi str arg" \; \ + --args svc2 --somearg 1 --payload '{"foo": "bar"}' + ``` + To override the default configuration directory, set the `PEBBLE` environment variable when running: ``` @@ -426,8 +452,13 @@ services: # (Required in combined layer) The command to run the service. The # command is executed directly, not interpreted by a shell. # + # (Optional) The command can have default arguments inside a [ ... ] + # block, which can also be overridden by providing args via + # pebble run --args ; + # The default (or, overridden) arguments are appended to the command. + # # Example: /usr/bin/somecommand -b -t 30 - command: + command: [ ] # (Optional) A short summary of the service. summary: diff --git a/client/services.go b/client/services.go index 71467c95a..0be19d23e 100644 --- a/client/services.go +++ b/client/services.go @@ -25,64 +25,50 @@ import ( type ServiceOptions struct { Names []string - Args map[string][]string } // AutoStart starts the services makes as "startup: enabled". opts.Names must // be empty for this call. func (client *Client) AutoStart(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("autostart", &multiActionOptions{services: opts.Names}) + _, changeID, err = client.doMultiServiceAction("autostart", opts.Names) return changeID, err } // Start starts the services named in opts.Names in dependency order. func (client *Client) Start(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("start", &multiActionOptions{services: opts.Names}) + _, changeID, err = client.doMultiServiceAction("start", opts.Names) return changeID, err } // Stop stops the services named in opts.Names in dependency order. func (client *Client) Stop(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("stop", &multiActionOptions{services: opts.Names}) + _, changeID, err = client.doMultiServiceAction("stop", opts.Names) return changeID, err } // Restart stops and then starts the services named in opts.Names in // dependency order. func (client *Client) Restart(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("restart", &multiActionOptions{services: opts.Names}) + _, changeID, err = client.doMultiServiceAction("restart", opts.Names) return changeID, err } // Replan stops and (re)starts the services whose configuration has changed // since they were started. opts.Names must be empty for this call. func (client *Client) Replan(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("replan", &multiActionOptions{services: opts.Names}) + _, changeID, err = client.doMultiServiceAction("replan", opts.Names) return changeID, err } -// PassServiceArgs sets the service arguments provided by "pebble run --args" -func (client *Client) PassServiceArgs(opts *ServiceOptions) (changeID string, err error) { - _, changeID, err = client.doMultiServiceAction("pass-args", &multiActionOptions{serviceArgs: opts.Args}) - return changeID, err -} - -type multiActionOptions struct { - services []string - serviceArgs map[string][]string -} - type multiActionData struct { - Action string `json:"action"` - Services []string `json:"services"` - ServiceArgs map[string][]string `json:"service-args"` + Action string `json:"action"` + Services []string `json:"services"` } -func (client *Client) doMultiServiceAction(actionName string, opts *multiActionOptions) (result json.RawMessage, changeID string, err error) { +func (client *Client) doMultiServiceAction(actionName string, services []string) (result json.RawMessage, changeID string, err error) { action := multiActionData{ - Action: actionName, - Services: opts.services, - ServiceArgs: opts.serviceArgs, + Action: actionName, + Services: services, } data, err := json.Marshal(&action) if err != nil { diff --git a/client/services_test.go b/client/services_test.go index 32628e854..51eec13ad 100644 --- a/client/services_test.go +++ b/client/services_test.go @@ -55,7 +55,7 @@ func (cs *clientSuite) TestStartStop(c *check.C) { var body map[string]interface{} c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 3) + c.Check(body, check.HasLen, 2) c.Check(body["action"], check.Equals, action) c.Check(body["services"], check.DeepEquals, []interface{}{"one", "two"}) } @@ -80,7 +80,7 @@ func (cs *clientSuite) TestAutostart(c *check.C) { var body map[string]interface{} c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 3) + c.Check(body, check.HasLen, 2) c.Check(body["action"], check.Equals, "autostart") } @@ -132,7 +132,7 @@ func (cs *clientSuite) TestRestart(c *check.C) { var body map[string]interface{} c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 3) + c.Check(body, check.HasLen, 2) c.Check(body["action"], check.Equals, "restart") c.Check(body["services"], check.DeepEquals, []interface{}{"one", "two"}) } @@ -156,38 +156,6 @@ func (cs *clientSuite) TestReplan(c *check.C) { var body map[string]interface{} c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 3) + c.Check(body, check.HasLen, 2) c.Check(body["action"], check.Equals, "replan") } - -func (cs *clientSuite) TestPassArgs(c *check.C) { - cs.rsp = `{ - "result": {}, - "status": "OK", - "status-code": 202, - "type": "async", - "change": "42" - }` - - opts := client.ServiceOptions{ - Args: map[string][]string{ - "one": {"--foo", "bar"}, - "two": {"-v", "--bar=foo"}, - }, - } - - changeId, err := cs.cli.PassServiceArgs(&opts) - c.Check(err, check.IsNil) - c.Check(changeId, check.Equals, "42") - c.Check(cs.req.Method, check.Equals, "POST") - c.Check(cs.req.URL.Path, check.Equals, "/v1/services") - - var body map[string]interface{} - c.Assert(json.NewDecoder(cs.req.Body).Decode(&body), check.IsNil) - c.Check(body, check.HasLen, 3) - c.Check(body["action"], check.Equals, "pass-args") - c.Check(body["service-args"], check.DeepEquals, map[string]interface{}{ - "one": []interface{}{"--foo", "bar"}, - "two": []interface{}{"-v", "--bar=foo"}, - }) -} diff --git a/cmd/pebble/cmd_autostart_test.go b/cmd/pebble/cmd_autostart_test.go index 6bda8dc7c..4872f938f 100644 --- a/cmd/pebble/cmd_autostart_test.go +++ b/cmd/pebble/cmd_autostart_test.go @@ -56,9 +56,8 @@ func (s *PebbleSuite) TestAutostart(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "autostart", - "services": nil, - "service-args": nil, + "action": "autostart", + "services": nil, }) fmt.Fprintf(w, `{ @@ -81,9 +80,8 @@ func (s *PebbleSuite) TestAutostartFailsNoDefaultServices(c *check.C) { c.Check(r.URL.Path, check.Equals, "/v1/services") body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "autostart", - "services": nil, - "service-args": nil, + "action": "autostart", + "services": nil, }) fmt.Fprint(w, `{ @@ -108,9 +106,8 @@ func (s *PebbleSuite) TestAutostartNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "autostart", - "services": nil, - "service-args": nil, + "action": "autostart", + "services": nil, }) fmt.Fprintf(w, `{ @@ -140,9 +137,8 @@ func (s *PebbleSuite) TestAutostartFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "autostart", - "services": nil, - "service-args": nil, + "action": "autostart", + "services": nil, }) fmt.Fprintf(w, `{ diff --git a/cmd/pebble/cmd_replan_test.go b/cmd/pebble/cmd_replan_test.go index 498e805b6..da13fdb0d 100644 --- a/cmd/pebble/cmd_replan_test.go +++ b/cmd/pebble/cmd_replan_test.go @@ -56,9 +56,8 @@ func (s *PebbleSuite) TestReplan(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "replan", - "services": nil, - "service-args": nil, + "action": "replan", + "services": nil, }) fmt.Fprintf(w, `{ @@ -81,9 +80,8 @@ func (s *PebbleSuite) TestReplanFailsNoDefaultServices(c *check.C) { c.Check(r.URL.Path, check.Equals, "/v1/services") body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "replan", - "services": nil, - "service-args": nil, + "action": "replan", + "services": nil, }) fmt.Fprint(w, `{ @@ -108,9 +106,8 @@ func (s *PebbleSuite) TestReplanNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "replan", - "services": nil, - "service-args": nil, + "action": "replan", + "services": nil, }) fmt.Fprintf(w, `{ @@ -140,9 +137,8 @@ func (s *PebbleSuite) TestReplanFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "replan", - "services": nil, - "service-args": nil, + "action": "replan", + "services": nil, }) fmt.Fprintf(w, `{ diff --git a/cmd/pebble/cmd_restart_test.go b/cmd/pebble/cmd_restart_test.go index 4f2412e78..624fdc2d8 100644 --- a/cmd/pebble/cmd_restart_test.go +++ b/cmd/pebble/cmd_restart_test.go @@ -48,9 +48,8 @@ func (s *PebbleSuite) TestRestart(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "restart", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "restart", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ @@ -74,9 +73,8 @@ func (s *PebbleSuite) TestRestartFails(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "restart", - "services": []interface{}{"srv1", "srv3"}, - "service-args": nil, + "action": "restart", + "services": []interface{}{"srv1", "srv3"}, }) fmt.Fprintf(w, `{"type": "error", "result": {"message": "could not foo"}}`) @@ -97,9 +95,8 @@ func (s *PebbleSuite) TestRestartNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "restart", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "restart", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ @@ -129,9 +126,8 @@ func (s *PebbleSuite) TestRestartFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "restart", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "restart", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ diff --git a/cmd/pebble/cmd_run.go b/cmd/pebble/cmd_run.go index 11f8136e6..f3ceccb6e 100644 --- a/cmd/pebble/cmd_run.go +++ b/cmd/pebble/cmd_run.go @@ -34,6 +34,15 @@ import ( var shortRunHelp = "Run the pebble environment" var longRunHelp = ` The run command starts pebble and runs the configured environment. + +Additional arguments to a service command can be provided by the +--args option in the format: + --args ; +The arguments will override the default arguments if specified in +the plan within [ ... ] group. The semi-colon at the end acts as a +terminator (end-marker) for the --args option, and needs to be +backslash-escaped if used at the shell. If no other options follow +after --args, the terminator ";" may be omitted. ` type cmdRun struct { @@ -53,7 +62,7 @@ func init() { "hold": "Do not start default services automatically", "http": `Start HTTP API listening on this address (e.g., ":4000")`, "verbose": "Log all output from services to stdout", - "args": `Provide arguments to service (e.g., "SERVICE ARGS.. ; ")`, + "args": `Provide arguments to a service`, }, nil) } @@ -132,6 +141,13 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal) error { if rcmd.Verbose { dopts.ServiceOutput = os.Stdout } + if rcmd.Args != nil { + mappedArgs, err := convertArgs(rcmd.Args) + if err != nil { + return err + } + dopts.ServiceArgs = mappedArgs + } dopts.HTTPAddress = rcmd.HTTP d, err := daemon.New(&dopts) @@ -168,20 +184,6 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal) error { logger.Debugf("activation done in %v", time.Now().Truncate(time.Millisecond).Sub(t0)) - if rcmd.Args != nil { - mappedArgs, err := convertArgs(rcmd.Args) - if err != nil { - return err - } - servopts := client.ServiceOptions{Args: mappedArgs} - changeID, err := rcmd.client.PassServiceArgs(&servopts) - if err != nil { - return fmt.Errorf("cannot pass arguments to services: %v", err) - } else { - logger.Noticef("Passed arguments to services with change %s.", changeID) - } - } - if !rcmd.Hold { servopts := client.ServiceOptions{} changeID, err := rcmd.client.AutoStart(&servopts) @@ -222,18 +224,11 @@ func convertArgs(args [][]string) (map[string][]string, error) { mappedArgs := make(map[string][]string) for _, arg := range args { - switch len(arg) { - case 0: - return nil, fmt.Errorf("--args cannot have empty arguments") - case 1: - return nil, fmt.Errorf("cannot pass empty arguments to service %q", arg[0]) + if len(arg) < 2 { + return nil, fmt.Errorf("--args requires a service name and one or more additional arguments") } - name := arg[0] - if _, ok := mappedArgs[name]; ok { - return nil, fmt.Errorf("cannot pass args twice to service %q", name) - } - mappedArgs[name] = append([]string(nil), arg[1:]...) + mappedArgs[name] = append(mappedArgs[name], arg[1:]...) } return mappedArgs, nil diff --git a/cmd/pebble/cmd_start_test.go b/cmd/pebble/cmd_start_test.go index 2ffb6ed7f..4cd0f2e79 100644 --- a/cmd/pebble/cmd_start_test.go +++ b/cmd/pebble/cmd_start_test.go @@ -48,9 +48,8 @@ func (s *PebbleSuite) TestStart(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "start", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "start", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ @@ -74,9 +73,8 @@ func (s *PebbleSuite) TestStartFails(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "start", - "services": []interface{}{"srv1", "srv3"}, - "service-args": nil, + "action": "start", + "services": []interface{}{"srv1", "srv3"}, }) fmt.Fprintf(w, `{"type": "error", "result": {"message": "could not foo"}}`) @@ -97,9 +95,8 @@ func (s *PebbleSuite) TestStartNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "start", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "start", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ @@ -129,9 +126,8 @@ func (s *PebbleSuite) TestStartFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "start", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "start", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ diff --git a/cmd/pebble/cmd_stop_test.go b/cmd/pebble/cmd_stop_test.go index 2ba4d3665..8e4c1c83d 100644 --- a/cmd/pebble/cmd_stop_test.go +++ b/cmd/pebble/cmd_stop_test.go @@ -48,9 +48,8 @@ func (s *PebbleSuite) TestStop(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "stop", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "stop", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ @@ -74,9 +73,8 @@ func (s *PebbleSuite) TestStopFails(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "stop", - "services": []interface{}{"srv1", "srv3"}, - "service-args": nil, + "action": "stop", + "services": []interface{}{"srv1", "srv3"}, }) fmt.Fprintf(w, `{"type": "error", "result": {"message": "could not foo"}}`) @@ -97,9 +95,8 @@ func (s *PebbleSuite) TestStopNoWait(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "stop", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "stop", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ @@ -129,9 +126,8 @@ func (s *PebbleSuite) TestStopFailsGetChange(c *check.C) { body := DecodedRequestBody(c, r) c.Check(body, check.DeepEquals, map[string]interface{}{ - "action": "stop", - "services": []interface{}{"srv1", "srv2"}, - "service-args": nil, + "action": "stop", + "services": []interface{}{"srv1", "srv2"}, }) fmt.Fprintf(w, `{ diff --git a/internal/daemon/api_services.go b/internal/daemon/api_services.go index e1fc0d60d..711142fc8 100644 --- a/internal/daemon/api_services.go +++ b/internal/daemon/api_services.go @@ -61,9 +61,8 @@ func v1GetServices(c *Command, r *http.Request, _ *userState) Response { func v1PostServices(c *Command, r *http.Request, _ *userState) Response { var payload struct { - Action string `json:"action"` - Services []string `json:"services"` - ServiceArgs map[string][]string `json:"service-args"` + Action string `json:"action"` + Services []string `json:"services"` } decoder := json.NewDecoder(r.Body) @@ -94,10 +93,6 @@ func v1PostServices(c *Command, r *http.Request, _ *userState) Response { }) } payload.Services = services - case "pass-args": - if len(payload.Services) != 0 { - return statusBadRequest("%s accepts no service names", payload.Action) - } default: if len(payload.Services) == 0 { return statusBadRequest("no services to %s provided", payload.Action) @@ -181,11 +176,6 @@ func v1PostServices(c *Command, r *http.Request, _ *userState) Response { } sort.Strings(services) payload.Services = services - case "pass-args": - err = servmgr.SetServiceArgs(payload.ServiceArgs) - if err != nil { - break - } default: return statusBadRequest("action %q is unsupported", payload.Action) } @@ -197,9 +187,9 @@ func v1PostServices(c *Command, r *http.Request, _ *userState) Response { // resolved one. But do use the resolved set for the count. var summary string switch { - case taskSet == nil || len(taskSet.Tasks()) == 0: - // Can happen with a pass-args or replan that has no services to stop/start. - // A change with no tasks needs to be marked Done manually (normally a + case len(taskSet.Tasks()) == 0: + // Can happen with a replan that has no services to stop/start. A + // change with no tasks needs to be marked Done manually (normally a // change is marked Done when its last task is finished). summary = fmt.Sprintf("%s - no services", strings.Title(payload.Action)) change := st.NewChange(payload.Action, summary) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index a1bc33ea7..c72a5af68 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -72,6 +72,9 @@ type Options struct { // ServiceOuput is an optional io.Writer for the service log output, if set, all services // log output will be written to the writer. ServiceOutput io.Writer + + // ServiceArgs holds the service arguments provided via ``pebble run --args``. + ServiceArgs map[string][]string } // A Daemon listens for requests and routes them to the right command @@ -800,6 +803,9 @@ func New(opts *Options) (*Daemon, error) { if err != nil { return nil, err } + if err = ovld.PassServiceArgs(opts.ServiceArgs); err != nil { + return nil, err + } d.overlord = ovld d.state = ovld.State() return d, nil diff --git a/internal/overlord/overlord.go b/internal/overlord/overlord.go index c9bf1cd79..ef274f0be 100644 --- a/internal/overlord/overlord.go +++ b/internal/overlord/overlord.go @@ -407,6 +407,15 @@ func (o *Overlord) CheckManager() *checkstate.CheckManager { return o.checkMgr } +// PassServiceArgs passes the provided service arguments to the +// service manager responsible for services under the overlord. +func (o *Overlord) PassServiceArgs(serviceArgs map[string][]string) error { + if o.serviceMgr == nil { + return fmt.Errorf("cannot pass service arguments to nil service manager") + } + return o.serviceMgr.SetServiceArgs(serviceArgs) +} + // Fake creates an Overlord without any managers and with a backend // not using disk. Managers can be added with AddManager. For testing. func Fake() *Overlord { diff --git a/internal/overlord/servstate/handlers.go b/internal/overlord/servstate/handlers.go index ecadb9a58..b679d815b 100644 --- a/internal/overlord/servstate/handlers.go +++ b/internal/overlord/servstate/handlers.go @@ -324,7 +324,7 @@ func logError(err error) { // command. It assumes the caller has ensures the service is in a valid state, // and it sets s.cmd and other relevant fields. func (s *serviceData) startInternal() error { - args, err := s.config.GetCommand() + args, err := s.buildCmdArgs() if err != nil { return err } @@ -383,7 +383,7 @@ func (s *serviceData) startInternal() error { s.cmd.Stderr = logWriter // Start the process! - logger.Noticef("Service %q starting: %v", serviceName, s.config.GetCommandStr(args)) + logger.Noticef("Service %q starting: %q", serviceName, args) err = reaper.StartCommand(s.cmd) if err != nil { if outputIterator != nil { @@ -428,6 +428,11 @@ func (s *serviceData) startInternal() error { return nil } +// buildCmdArgs returns the service command as a stream of arguments +func (s *serviceData) buildCmdArgs() ([]string, error) { + return s.config.GetCommand(s.manager.serviceArgs[s.config.Name]) +} + // okayWaitElapsed is called when the okay-wait timer has elapsed (and the // service is considered running successfully). func (s *serviceData) okayWaitElapsed() error { diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index 7725187fd..450219415 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -28,6 +28,7 @@ type ServiceManager struct { servicesLock sync.Mutex services map[string]*serviceData + serviceArgs map[string][]string serviceOutput io.Writer restarter Restarter @@ -115,9 +116,10 @@ func (m *ServiceManager) Plan() (*plan.Plan, error) { return m.plan, nil } -// AppendLayer appends the given layer to the plan's layers and updates the -// layer.Order field to the new order. If a layer with layer.Label already -// exists, return an error of type *LabelExists. +// AppendLayer appends the given layer to the plan's layers, updates the +// layer.Order field to the new order and removes the existing arguments +// of this layer's services. If a layer with layer.Label already exists, +// return an error of type *LabelExists. func (m *ServiceManager) AppendLayer(layer *plan.Layer) error { releasePlan, err := m.acquirePlan() if err != nil { @@ -146,6 +148,12 @@ func (m *ServiceManager) appendLayer(layer *plan.Layer) error { return err } layer.Order = newOrder + + // Remove the existing arguments of this layer's services + for _, service := range layer.Services { + delete(m.serviceArgs, service.Name) + } + return nil } @@ -176,7 +184,8 @@ func findLayer(layers []*plan.Layer, label string) (int, *plan.Layer) { // CombineLayer combines the given layer with an existing layer that has the // same label. If no existing layer has the label, append a new one. In either -// case, update the layer.Order field to the new order. +// case, update the layer.Order field to the new order and remove existing +// arguments of this layer's services. func (m *ServiceManager) CombineLayer(layer *plan.Layer) error { releasePlan, err := m.acquirePlan() if err != nil { @@ -207,6 +216,12 @@ func (m *ServiceManager) CombineLayer(layer *plan.Layer) error { return err } layer.Order = found.Order + + // Remove the existing arguments of this layer's services + for _, service := range layer.Services { + delete(m.serviceArgs, service.Name) + } + return nil } @@ -356,16 +371,10 @@ func (m *ServiceManager) StopOrder(services []string) ([]string, error) { return m.plan.StopOrder(services) } -// SetServiceArgs sets the service arguments provided via "pebble run --args" -// in the plan. See plan.SetServiceArgs. +// SetServiceArgs sets the service arguments provided via "pebble run --args". func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { - releasePlan, err := m.acquirePlan() - if err != nil { - return err - } - defer releasePlan() - - return m.plan.SetServiceArgs(serviceArgs) + m.serviceArgs = serviceArgs + return nil } // ServiceLogs returns iterators to the provided services. If last is negative, diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 60ee68375..9dae55350 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -65,7 +65,6 @@ type Service struct { Startup ServiceStartup `yaml:"startup,omitempty"` Override Override `yaml:"override,omitempty"` Command string `yaml:"command,omitempty"` - CmdArgs []string `yaml:"-"` // Service dependencies After []string `yaml:"after,omitempty"` @@ -91,7 +90,6 @@ type Service struct { // Copy returns a deep copy of the service. func (s *Service) Copy() *Service { copied := *s - copied.CmdArgs = append([]string(nil), s.CmdArgs...) copied.After = append([]string(nil), s.After...) copied.Before = append([]string(nil), s.Before...) copied.Requires = append([]string(nil), s.Requires...) @@ -131,7 +129,6 @@ func (s *Service) Merge(other *Service) { } if other.Command != "" { s.Command = other.Command - s.CmdArgs = append([]string(nil), other.CmdArgs...) } if other.UserID != nil { userID := *other.UserID @@ -187,74 +184,59 @@ func (s *Service) Equal(other *Service) bool { return reflect.DeepEqual(s, other) } -// GetCommand overrides the default arguments (inside [ ]) if -// CmdArgs is present and returns the command as a stream of strings. -func (s *Service) GetCommand() ([]string, error) { +// GetCommand returns the service command as a stream of strings. +// It adds the arguments in cmdArgs to the command if no default +// argument is specified in [ ... ] group in the plan. +// If default arguments are specified in the plan, the default +// arguments are added to the command if cmdArgs is empty. +// If not empty, the default arguments are overrided by arguments in cmdArgs. +func (s *Service) GetCommand(cmdArgs []string) ([]string, error) { args, err := shlex.Split(s.Command) if err != nil { - return nil, fmt.Errorf("cannot parse service %q command: %s", s.Name, err) - } - fargs := make([]string, 0) - for _, arg := range args { - if arg == "[" || arg == "]" { - if len(s.CmdArgs) > 0 { - break - } else { - continue - } - } - fargs = append(fargs, arg) - } - for _, arg := range s.CmdArgs { - fargs = append(fargs, arg) + return nil, err } - return fargs, nil -} -// GetCommandStr takes in the output of GetCommand and -// returns the command as a string in good print format -func (s *Service) GetCommandStr(cmd []string) string { - f := func(str string) string { - for _, c := range str { - if c < '!' { - return strconv.Quote(str) + var inBrackets, gotBrackets bool + var fargs []string + + for idx, arg := range args { + if inBrackets { + if arg == "[" { + return nil, fmt.Errorf("cannot nest [ ... ] groups") } + if arg == "]" { + inBrackets = false + continue + } + if len(cmdArgs) == 0 { + fargs = append(fargs, arg) + } + continue + } + if gotBrackets { + return nil, fmt.Errorf("cannot have any args after [ ... ] group") } - return str - } - args := make([]string, 0) - for _, arg := range cmd { - args = append(args, f(arg)) - } - return strings.Join(args, " ") -} - -func (s *Service) setArgs(args []string) error { - s.CmdArgs = append([]string(nil), args...) - return nil -} - -func (s *Service) checkCommand() error { - args, err := shlex.Split(s.Command) - if err != nil { - return err - } - leftCnt := 0 - rightCnt := 0 - for _, arg := range args { if arg == "[" { - leftCnt++ + if idx == 0 { + return nil, fmt.Errorf("cannot have [ ... ] group as prefix") + } + if len(cmdArgs) > 0 { + fargs = append(fargs, cmdArgs...) + } + inBrackets = true + gotBrackets = true + continue } if arg == "]" { - rightCnt++ + return nil, fmt.Errorf("cannot have ] outside of [ ... ] group") } + fargs = append(fargs, arg) } - if leftCnt > 0 || rightCnt > 0 { - if leftCnt != 1 || rightCnt != 1 || args[len(args)-1] != "]" { - return fmt.Errorf("bad syntax regarding optional/overridable arguments") - } + if !gotBrackets && len(cmdArgs) > 0 { + fargs = append(fargs, cmdArgs...) } - return nil + + return fargs, nil } type ServiceStartup string @@ -556,7 +538,7 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { Message: fmt.Sprintf(`plan must define "command" for service %q`, name), } } - err := service.checkCommand() + _, err := service.GetCommand(nil) if err != nil { return nil, &FormatError{ Message: fmt.Sprintf("plan service %q command invalid: %v", name, err), @@ -695,33 +677,6 @@ func (p *Plan) StopOrder(names []string) ([]string, error) { return order(p.Services, names, true) } -// SetServiceArgs sets the service arguments provided by "pebble run --args" -// to their respective services. Additionally, for a service the arguments -// are also updated in the layer of the highest order that defines the service -// so that the arguments to a service can persist if it is not affected -// by a "pebble add" layer or similar. -func (p *Plan) SetServiceArgs(serviceArgs map[string][]string) error { - for svcName, svcArgs := range serviceArgs { - service, ok := p.Services[svcName] - if !ok { - return fmt.Errorf("Service %q does not exist in the plan (arguments passed via --args)", svcName) - } - if err := service.setArgs(svcArgs); err != nil { - return err - } - for i := len(p.Layers) - 1; i >= 0; i-- { - layerService, ok := p.Layers[i].Services[svcName] - if ok { - if err := layerService.setArgs(svcArgs); err != nil { - return err - } - break - } - } - } - return nil -} - func order(services map[string]*Service, names []string, stop bool) ([]string, error) { // For stop, create a list of reversed dependencies. predecessors := map[string][]string(nil) diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index 0ffdcd3fb..ba4c7080f 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -497,14 +497,32 @@ var planTests = []planTest{{ Checks: map[string]*plan.Check{}, }}, }, { - summary: `Invalid syntax in Optional/overridable arguments to service command`, - error: `plan service "svc1" command invalid: bad syntax regarding optional/overridable arguments`, + summary: `Invalid service command: cannot have any args after [ ... ] group`, + error: `plan service "svc1" command invalid: cannot have any args after \[ ... \] group`, input: []string{` services: "svc1": override: replace command: cmd -v [ --foo ] bar `}, +}, { + summary: `Invalid service command: cannot have ] outside of [ ... ] group`, + error: `plan service "svc1" command invalid: cannot have \] outside of \[ ... \] group`, + input: []string{` + services: + "svc1": + override: replace + command: cmd -v ] foo + `}, +}, { + summary: `Invalid service command: cannot nest [ ... ] groups`, + error: `plan service "svc1" command invalid: cannot nest \[ ... \] groups`, + input: []string{` + services: + "svc1": + override: replace + command: cmd -v [ foo [ --bar ] ] + `}, }, { summary: "Checks fields parse correctly and defaults are correct", input: []string{` @@ -1047,134 +1065,64 @@ func (s *S) TestMarshalLayer(c *C) { c.Assert(string(out), Equals, string(layerBytes)) } -var svcArgsTests = []struct { - summary string - input []string - moreInput string - serviceArgs map[string][]string - err string - result plan.Plan -}{ - { - summary: `plan.Services and plan.Layers should be updated accordingly`, - input: []string{` - services: - svc1: - override: replace - command: svc1cmd -v [ --foo bar ] - svc2: - override: replace - command: svc2cmd [ --wait 15 ] - `, ` - services: - svc2: - override: replace - command: newsvc2cmd [ -b -c foo ] - `}, - serviceArgs: map[string][]string{ - "svc1": {"--bar", "foo"}, - "svc2": {"-xyz"}, - }, - result: plan.Plan{ - Layers: []*plan.Layer{ - { - Label: "layer-0", - Services: map[string]*plan.Service{ - "svc1": { - Name: "svc1", - Override: plan.ReplaceOverride, - Command: "svc1cmd -v [ --foo bar ]", - CmdArgs: []string{"--bar", "foo"}, - }, - "svc2": { - Name: "svc2", - Override: plan.ReplaceOverride, - Command: "svc2cmd [ --wait 15 ]", - }, - }, - }, - { - Label: "layer-1", - Services: map[string]*plan.Service{ - "svc2": { - Name: "svc2", - Override: plan.ReplaceOverride, - Command: "newsvc2cmd [ -b -c foo ]", - CmdArgs: []string{"-xyz"}, - }, - }, - }, - }, - Services: map[string]*plan.Service{ - "svc1": { - Name: "svc1", - Override: plan.ReplaceOverride, - Command: "svc1cmd -v [ --foo bar ]", - CmdArgs: []string{"--bar", "foo"}, - BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, - BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, - BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - }, - "svc2": { - Name: "svc2", - Override: plan.ReplaceOverride, - Command: "newsvc2cmd [ -b -c foo ]", - CmdArgs: []string{"-xyz"}, - BackoffDelay: plan.OptionalDuration{Value: defaultBackoffDelay}, - BackoffFactor: plan.OptionalFloat{Value: defaultBackoffFactor}, - BackoffLimit: plan.OptionalDuration{Value: defaultBackoffLimit}, - }, - }, +func (s *S) TestGetCommand(c *C) { + cmdTests := []struct { + summary string + command string + cmdArgs []string + expectedCommand []string + error string + }{ + { + summary: "No default arguments, no additional cmdArgs", + command: "cmd --foo bar", + expectedCommand: []string{"cmd", "--foo", "bar"}, + }, { + summary: "No default arguments, add cmdArgs only", + command: "cmd --foo bar", + cmdArgs: []string{"-v", "--opt"}, + expectedCommand: []string{"cmd", "--foo", "bar", "-v", "--opt"}, + }, { + summary: "Default arguments only", + command: "cmd [ --foo bar ]", + expectedCommand: []string{"cmd", "--foo", "bar"}, + }, { + summary: "Override default arguments with cmdArgs", + command: "cmd [ --foo bar ]", + cmdArgs: []string{"--bar", "foo"}, + expectedCommand: []string{"cmd", "--bar", "foo"}, + }, { + summary: "Empty [ ... ], no cmdArgs", + command: "cmd --foo bar [ ]", + expectedCommand: []string{"cmd", "--foo", "bar"}, + }, { + summary: "Empty [ ... ], override with cmdArgs", + command: "cmd --foo bar [ ]", + cmdArgs: []string{"-v", "--opt"}, + expectedCommand: []string{"cmd", "--foo", "bar", "-v", "--opt"}, + }, { + summary: "[ ... ] should be a suffix", + command: "cmd [ --foo ] --bar", + error: `cannot have any args after \[ ... \] group`, + }, { + summary: "[ ... ] should not be prefix", + command: "[ cmd --foo ]", + error: `cannot have \[ ... \] group as prefix`, }, - }, { - summary: `invalid service name should raise error`, - input: []string{` - services: - svc1: - override: replace - command: svc1cmd -v [ --foo bar ] - `}, - serviceArgs: map[string][]string{ - "foo": {"bar"}, - }, - err: `Service "foo" does not exist in the plan \(arguments passed via --args\)`, - }, -} - -func (s *S) TestSetServiceArgs(c *C) { - for _, test := range svcArgsTests { - var err error - - layers := make([]*plan.Layer, 0) - for i, yml := range test.input { - layer, err := plan.ParseLayer(i, fmt.Sprintf("layer-%d", i), reindent(yml)) - c.Assert(err, IsNil) - layers = append(layers, layer) - } - - combined, err := plan.CombineLayers(layers...) - c.Assert(err, IsNil) + } - p := plan.Plan{ - Layers: layers, - Services: combined.Services, + for _, test := range cmdTests { + svc := plan.Service{Command: test.command} + cmd, err := svc.GetCommand(test.cmdArgs) + if err == nil { + c.Assert(cmd, DeepEquals, test.expectedCommand) } - - err = p.SetServiceArgs(test.serviceArgs) - if err != nil || test.err != "" { - if test.err != "" { - c.Assert(err, ErrorMatches, test.err) + if err != nil || test.error != "" { + if test.error != "" { + c.Assert(err, ErrorMatches, test.error) } else { c.Assert(err, IsNil) } - continue - } - - c.Assert(len(p.Layers), Equals, len(test.result.Layers)) - for i := range p.Layers { - c.Assert(fmt.Sprintf("layer-%d", i), Equals, test.result.Layers[i].Label) - c.Assert(p.Layers[i].Services, DeepEquals, test.result.Layers[i].Services) } - c.Assert(p.Services, DeepEquals, test.result.Services) } } From c467e33e61737f6e9bfba714b10420a9211e3e67 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Fri, 17 Feb 2023 13:10:44 +0600 Subject: [PATCH 08/22] WIP: Accept Ben's suggestions --- README.md | 49 +++++++++++---------- cmd/pebble/cmd_run.go | 25 +++++++---- internal/daemon/daemon.go | 5 +-- internal/overlord/managers_test.go | 2 +- internal/overlord/overlord.go | 14 ++---- internal/overlord/overlord_test.go | 28 ++++++------ internal/overlord/servstate/handlers.go | 7 +-- internal/overlord/servstate/manager.go | 9 +--- internal/overlord/servstate/manager_test.go | 6 +-- internal/plan/plan.go | 20 ++++----- internal/plan/plan_test.go | 2 +- 11 files changed, 77 insertions(+), 90 deletions(-) diff --git a/README.md b/README.md index 7bb8afa81..ddae9fd26 100644 --- a/README.md +++ b/README.md @@ -146,31 +146,29 @@ are marked as `startup: enabled` (if you don't want that, use `--hold`). Then other Pebble commands may be used to interact with the running daemon, for example, in another terminal window. -To provide arguments to a service, you can use the `--args` option. The arguments will -override the default arguments (if any) specified in the plan. To indicate the end -of arguments, a terminator `;` is used which must be backslash-escaped if used in -the shell. The terminator maybe omitted for an `--args` if there are no other `run` -options that follow. For example, +To provide additional arguments to a service, use `--args ...`. +If the `command` field in the service's plan has a `[ ]` +list, the `--args` arguments will replace the defaults. If not, they will be +appended to the command. -* start the Pebble server and only pass some arguments to “svc”: +To indicate the end of an `--args` list, use a `;` (semicolon) terminator, +which must be backslash-escaped if used in the shell. The terminator +may be omitted if there are no other Pebble options that follow. - ``` - $ pebble run --args svc --verbose --foo "multi str arg" - ``` +For example: -* start the Pebble server with all services on hold, and give some - args to “svc”: - - ``` - $ pebble run --args svc --verbose --foo "multi str arg" \; --hold - ``` +``` +# Start the daemon and pass additional arguments to "svc". +$ pebble run --args svc --verbose --foo "multi str arg" -* start the Pebble server and pass arguments to multiple services: +# Start the daemon with all services on hold, but pass additional +# arguments to "svc" (demonstrates using a terminator). +$ pebble run --args svc --verbose --foo "multi str arg" \; --hold - ``` - $ pebble run --args svc1 --verbose --foo "multi str arg" \; \ - --args svc2 --somearg 1 --payload '{"foo": "bar"}' - ``` +# Start the daemon and pass arguments to multiple services. +$ pebble run --args svc1 --verbose --foo "multi str arg" \; \ + --args svc2 --somearg 1 --payload '{"foo": "bar"}' +``` To override the default configuration directory, set the `PEBBLE` environment variable when running: @@ -452,10 +450,13 @@ services: # (Required in combined layer) The command to run the service. The # command is executed directly, not interpreted by a shell. # - # (Optional) The command can have default arguments inside a [ ... ] - # block, which can also be overridden by providing args via - # pebble run --args ; - # The default (or, overridden) arguments are appended to the command. + # The command may end with a list of arguments inside a [ ... ] + # block (the brackets must be surrounded by spaces). For example: + # + # command: server --verbose [ --port 8080 ] + # + # These default arguments can be overridden by calling "pebble run" + # with the --args option. # # Example: /usr/bin/somecommand -b -t 30 command: [ ] diff --git a/cmd/pebble/cmd_run.go b/cmd/pebble/cmd_run.go index f3ceccb6e..22f044da7 100644 --- a/cmd/pebble/cmd_run.go +++ b/cmd/pebble/cmd_run.go @@ -35,14 +35,18 @@ var shortRunHelp = "Run the pebble environment" var longRunHelp = ` The run command starts pebble and runs the configured environment. -Additional arguments to a service command can be provided by the ---args option in the format: - --args ; -The arguments will override the default arguments if specified in -the plan within [ ... ] group. The semi-colon at the end acts as a -terminator (end-marker) for the --args option, and needs to be -backslash-escaped if used at the shell. If no other options follow -after --args, the terminator ";" may be omitted. +Additional arguments can be provided to a service command with the +--args option, in the following format: + + --args ; + +If the command field in the service's plan has a [ ] +list, the --args arguments will replace those default arguments. If not, the +--args arguments will be appended to the command. + +The semicolon terminator at the end of the --args list is only required +if other "run" arguments follow. If used at the shell, the ';' must be +backslash-escaped. ` type cmdRun struct { @@ -62,7 +66,7 @@ func init() { "hold": "Do not start default services automatically", "http": `Start HTTP API listening on this address (e.g., ":4000")`, "verbose": "Log all output from services to stdout", - "args": `Provide arguments to a service`, + "args": `Provide additional arguments to a service`, }, nil) } @@ -228,6 +232,9 @@ func convertArgs(args [][]string) (map[string][]string, error) { return nil, fmt.Errorf("--args requires a service name and one or more additional arguments") } name := arg[0] + if _, ok := mappedArgs[name]; ok { + return nil, fmt.Errorf("cannot use --args twice on a service") + } mappedArgs[name] = append(mappedArgs[name], arg[1:]...) } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index c72a5af68..edf8f394f 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -791,7 +791,7 @@ func New(opts *Options) (*Daemon, error) { httpAddress: opts.HTTPAddress, } - ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput) + ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput, opts.ServiceArgs) if err == errExpectedReboot { // we proceed without overlord until we reach Stop // where we will schedule and wait again for a system restart. @@ -803,9 +803,6 @@ func New(opts *Options) (*Daemon, error) { if err != nil { return nil, err } - if err = ovld.PassServiceArgs(opts.ServiceArgs); err != nil { - return nil, err - } d.overlord = ovld d.state = ovld.State() return d, nil diff --git a/internal/overlord/managers_test.go b/internal/overlord/managers_test.go index c177b31c8..9253dc1d5 100644 --- a/internal/overlord/managers_test.go +++ b/internal/overlord/managers_test.go @@ -42,7 +42,7 @@ func (s *mgrsSuite) SetUpTest(c *C) { s.dir = c.MkDir() - o, err := overlord.New(s.dir, nil, nil) + o, err := overlord.New(s.dir, nil, nil, nil) c.Assert(err, IsNil) s.o = o } diff --git a/internal/overlord/overlord.go b/internal/overlord/overlord.go index ef274f0be..25ba19f8d 100644 --- a/internal/overlord/overlord.go +++ b/internal/overlord/overlord.go @@ -72,7 +72,8 @@ type Overlord struct { // New creates a new Overlord with all its state managers. // It can be provided with an optional restart.Handler. -func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writer) (*Overlord, error) { +// And optionally additional arguments to services. +func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writer, serviceArgs map[string][]string) (*Overlord, error) { o := &Overlord{ pebbleDir: pebbleDir, loopTomb: new(tomb.Tomb), @@ -105,7 +106,7 @@ func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writ } o.runner.AddOptionalHandler(matchAnyUnknownTask, handleUnknownTask, nil) - o.serviceMgr, err = servstate.NewManager(s, o.runner, o.pebbleDir, serviceOutput, restartHandler) + o.serviceMgr, err = servstate.NewManager(s, o.runner, o.pebbleDir, serviceOutput, restartHandler, serviceArgs) if err != nil { return nil, err } @@ -407,15 +408,6 @@ func (o *Overlord) CheckManager() *checkstate.CheckManager { return o.checkMgr } -// PassServiceArgs passes the provided service arguments to the -// service manager responsible for services under the overlord. -func (o *Overlord) PassServiceArgs(serviceArgs map[string][]string) error { - if o.serviceMgr == nil { - return fmt.Errorf("cannot pass service arguments to nil service manager") - } - return o.serviceMgr.SetServiceArgs(serviceArgs) -} - // Fake creates an Overlord without any managers and with a backend // not using disk. Managers can be added with AddManager. For testing. func Fake() *Overlord { diff --git a/internal/overlord/overlord_test.go b/internal/overlord/overlord_test.go index 3b182d43b..47e600a9e 100644 --- a/internal/overlord/overlord_test.go +++ b/internal/overlord/overlord_test.go @@ -58,7 +58,7 @@ func (ovs *overlordSuite) TestNew(c *C) { restore := patch.Fake(42, 2, nil) defer restore() - o, err := overlord.New(ovs.dir, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil, nil) c.Assert(err, IsNil) c.Check(o, NotNil) @@ -83,7 +83,7 @@ func (ovs *overlordSuite) TestNewWithGoodState(c *C) { err := ioutil.WriteFile(ovs.statePath, fakeState, 0600) c.Assert(err, IsNil) - o, err := overlord.New(ovs.dir, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil, nil) c.Assert(err, IsNil) state := o.State() @@ -111,7 +111,7 @@ func (ovs *overlordSuite) TestNewWithInvalidState(c *C) { err := ioutil.WriteFile(ovs.statePath, fakeState, 0600) c.Assert(err, IsNil) - _, err = overlord.New(ovs.dir, nil, nil) + _, err = overlord.New(ovs.dir, nil, nil, nil) c.Assert(err, ErrorMatches, "cannot read state: EOF") } @@ -130,7 +130,7 @@ func (ovs *overlordSuite) TestNewWithPatches(c *C) { err := ioutil.WriteFile(ovs.statePath, fakeState, 0600) c.Assert(err, IsNil) - o, err := overlord.New(ovs.dir, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil, nil) c.Assert(err, IsNil) state := o.State() @@ -175,7 +175,7 @@ func (wm *witnessManager) Ensure() error { } func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) { - o, err := overlord.New(ovs.dir, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil, nil) c.Assert(err, IsNil) o.Loop() @@ -185,7 +185,7 @@ func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) { } func (ovs *overlordSuite) TestUnknownTasks(c *C) { - o, err := overlord.New(ovs.dir, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil, nil) c.Assert(err, IsNil) // unknown tasks are ignored and succeed @@ -486,7 +486,7 @@ func (ovs *overlordSuite) TestCheckpoint(c *C) { oldUmask := syscall.Umask(0) defer syscall.Umask(oldUmask) - o, err := overlord.New(ovs.dir, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil, nil) c.Assert(err, IsNil) s := o.State() @@ -727,7 +727,7 @@ func (ovs *overlordSuite) TestSettleExplicitEnsureBefore(c *C) { } func (ovs *overlordSuite) TestRequestRestartNoHandler(c *C) { - o, err := overlord.New(ovs.dir, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil, nil) c.Assert(err, IsNil) st := o.State() @@ -760,7 +760,7 @@ func (rb *testRestartHandler) RebootIsMissing(_ *state.State) error { func (ovs *overlordSuite) TestRequestRestartHandler(c *C) { rb := &testRestartHandler{} - o, err := overlord.New(ovs.dir, rb, nil) + o, err := overlord.New(ovs.dir, rb, nil, nil) c.Assert(err, IsNil) st := o.State() @@ -779,7 +779,7 @@ func (ovs *overlordSuite) TestVerifyRebootNoPendingReboot(c *C) { rb := &testRestartHandler{} - _, err = overlord.New(ovs.dir, rb, nil) + _, err = overlord.New(ovs.dir, rb, nil, nil) c.Assert(err, IsNil) c.Check(rb.rebootState, Equals, "as-expected") @@ -792,7 +792,7 @@ func (ovs *overlordSuite) TestVerifyRebootOK(c *C) { rb := &testRestartHandler{} - _, err = overlord.New(ovs.dir, rb, nil) + _, err = overlord.New(ovs.dir, rb, nil, nil) c.Assert(err, IsNil) c.Check(rb.rebootState, Equals, "as-expected") @@ -806,7 +806,7 @@ func (ovs *overlordSuite) TestVerifyRebootOKButError(c *C) { e := errors.New("boom") rb := &testRestartHandler{rebootVerifiedErr: e} - _, err = overlord.New(ovs.dir, rb, nil) + _, err = overlord.New(ovs.dir, rb, nil, nil) c.Assert(err, Equals, e) c.Check(rb.rebootState, Equals, "as-expected") @@ -822,7 +822,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissing(c *C) { rb := &testRestartHandler{} - _, err = overlord.New(ovs.dir, rb, nil) + _, err = overlord.New(ovs.dir, rb, nil, nil) c.Assert(err, IsNil) c.Check(rb.rebootState, Equals, "did-not-happen") @@ -839,7 +839,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissingError(c *C) { e := errors.New("boom") rb := &testRestartHandler{rebootVerifiedErr: e} - _, err = overlord.New(ovs.dir, rb, nil) + _, err = overlord.New(ovs.dir, rb, nil, nil) c.Assert(err, Equals, e) c.Check(rb.rebootState, Equals, "did-not-happen") diff --git a/internal/overlord/servstate/handlers.go b/internal/overlord/servstate/handlers.go index b679d815b..3fcffaf84 100644 --- a/internal/overlord/servstate/handlers.go +++ b/internal/overlord/servstate/handlers.go @@ -324,7 +324,7 @@ func logError(err error) { // command. It assumes the caller has ensures the service is in a valid state, // and it sets s.cmd and other relevant fields. func (s *serviceData) startInternal() error { - args, err := s.buildCmdArgs() + args, err := plan.CommandArgs(s.config.Command, s.manager.serviceArgs[s.config.Name]) if err != nil { return err } @@ -428,11 +428,6 @@ func (s *serviceData) startInternal() error { return nil } -// buildCmdArgs returns the service command as a stream of arguments -func (s *serviceData) buildCmdArgs() ([]string, error) { - return s.config.GetCommand(s.manager.serviceArgs[s.config.Name]) -} - // okayWaitElapsed is called when the okay-wait timer has elapsed (and the // service is considered running successfully). func (s *serviceData) okayWaitElapsed() error { diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index 450219415..76d76b756 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -54,12 +54,13 @@ func (e *LabelExists) Error() string { return fmt.Sprintf("layer %q already exists", e.Label) } -func NewManager(s *state.State, runner *state.TaskRunner, pebbleDir string, serviceOutput io.Writer, restarter Restarter) (*ServiceManager, error) { +func NewManager(s *state.State, runner *state.TaskRunner, pebbleDir string, serviceOutput io.Writer, restarter Restarter, serviceArgs map[string][]string) (*ServiceManager, error) { manager := &ServiceManager{ state: s, runner: runner, pebbleDir: pebbleDir, services: make(map[string]*serviceData), + serviceArgs: serviceArgs, serviceOutput: serviceOutput, restarter: restarter, rand: rand.New(rand.NewSource(time.Now().UnixNano())), @@ -371,12 +372,6 @@ func (m *ServiceManager) StopOrder(services []string) ([]string, error) { return m.plan.StopOrder(services) } -// SetServiceArgs sets the service arguments provided via "pebble run --args". -func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { - m.serviceArgs = serviceArgs - return nil -} - // ServiceLogs returns iterators to the provided services. If last is negative, // return tail iterators; if last is zero or positive, return head iterators // going back last elements. Each iterator must be closed via the Close method. diff --git a/internal/overlord/servstate/manager_test.go b/internal/overlord/servstate/manager_test.go index b0d873abb..97bfda999 100644 --- a/internal/overlord/servstate/manager_test.go +++ b/internal/overlord/servstate/manager_test.go @@ -160,7 +160,7 @@ func (s *S) SetUpTest(c *C) { s.runner = state.NewTaskRunner(s.st) s.stopDaemon = make(chan struct{}) - manager, err := servstate.NewManager(s.st, s.runner, s.dir, logOutput, testRestarter{s.stopDaemon}) + manager, err := servstate.NewManager(s.st, s.runner, s.dir, logOutput, testRestarter{s.stopDaemon}, nil) c.Assert(err, IsNil) s.AddCleanup(manager.Stop) s.manager = manager @@ -600,7 +600,7 @@ func (s *S) TestAppendLayer(c *C) { dir := c.MkDir() os.Mkdir(filepath.Join(dir, "layers"), 0755) runner := state.NewTaskRunner(s.st) - manager, err := servstate.NewManager(s.st, runner, dir, nil, nil) + manager, err := servstate.NewManager(s.st, runner, dir, nil, nil, nil) c.Assert(err, IsNil) defer manager.Stop() @@ -683,7 +683,7 @@ func (s *S) TestCombineLayer(c *C) { dir := c.MkDir() os.Mkdir(filepath.Join(dir, "layers"), 0755) runner := state.NewTaskRunner(s.st) - manager, err := servstate.NewManager(s.st, runner, dir, nil, nil) + manager, err := servstate.NewManager(s.st, runner, dir, nil, nil, nil) c.Assert(err, IsNil) defer manager.Stop() diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 9dae55350..2f2b7c8a1 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -184,20 +184,20 @@ func (s *Service) Equal(other *Service) bool { return reflect.DeepEqual(s, other) } -// GetCommand returns the service command as a stream of strings. +// CommandArgs returns a service command as a stream of strings. // It adds the arguments in cmdArgs to the command if no default // argument is specified in [ ... ] group in the plan. // If default arguments are specified in the plan, the default // arguments are added to the command if cmdArgs is empty. // If not empty, the default arguments are overrided by arguments in cmdArgs. -func (s *Service) GetCommand(cmdArgs []string) ([]string, error) { - args, err := shlex.Split(s.Command) +func CommandArgs(command string, cmdArgs []string) ([]string, error) { + args, err := shlex.Split(command) if err != nil { return nil, err } var inBrackets, gotBrackets bool - var fargs []string + var result []string for idx, arg := range args { if inBrackets { @@ -209,7 +209,7 @@ func (s *Service) GetCommand(cmdArgs []string) ([]string, error) { continue } if len(cmdArgs) == 0 { - fargs = append(fargs, arg) + result = append(result, arg) } continue } @@ -221,7 +221,7 @@ func (s *Service) GetCommand(cmdArgs []string) ([]string, error) { return nil, fmt.Errorf("cannot have [ ... ] group as prefix") } if len(cmdArgs) > 0 { - fargs = append(fargs, cmdArgs...) + result = append(result, cmdArgs...) } inBrackets = true gotBrackets = true @@ -230,13 +230,13 @@ func (s *Service) GetCommand(cmdArgs []string) ([]string, error) { if arg == "]" { return nil, fmt.Errorf("cannot have ] outside of [ ... ] group") } - fargs = append(fargs, arg) + result = append(result, arg) } if !gotBrackets && len(cmdArgs) > 0 { - fargs = append(fargs, cmdArgs...) + result = append(result, cmdArgs...) } - return fargs, nil + return result, nil } type ServiceStartup string @@ -538,7 +538,7 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { Message: fmt.Sprintf(`plan must define "command" for service %q`, name), } } - _, err := service.GetCommand(nil) + _, err := CommandArgs(service.Command, nil) if err != nil { return nil, &FormatError{ Message: fmt.Sprintf("plan service %q command invalid: %v", name, err), diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index ba4c7080f..489788e94 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -1113,7 +1113,7 @@ func (s *S) TestGetCommand(c *C) { for _, test := range cmdTests { svc := plan.Service{Command: test.command} - cmd, err := svc.GetCommand(test.cmdArgs) + cmd, err := plan.CommandArgs(svc.Command, test.cmdArgs) if err == nil { c.Assert(cmd, DeepEquals, test.expectedCommand) } From c4c0e57f1f71e78e96287eb305b7aa7c72097754 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Mon, 27 Feb 2023 13:09:07 +0600 Subject: [PATCH 09/22] Point to go-flags fork force-pushed new commit: https://github.com/rebornplusplus/go-flags/commit/18b729e1c185fbfac86f4afdabb0e67b996c4889 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index a1a922941..826326e84 100644 --- a/go.mod +++ b/go.mod @@ -15,4 +15,4 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -replace github.com/jessevdk/go-flags v1.4.0 => github.com/rebornplusplus/go-flags v0.0.0-20230201113816-1dbaf4443e95 +replace github.com/jessevdk/go-flags v1.4.0 => github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185 diff --git a/go.sum b/go.sum index f8f59bc9e..c6d690dae 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,8 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= -github.com/rebornplusplus/go-flags v0.0.0-20230201113816-1dbaf4443e95 h1:3Ty6zGRkREQRPkPCG8EW2pf4HwtICpdQNdw15LMo+30= -github.com/rebornplusplus/go-flags v0.0.0-20230201113816-1dbaf4443e95/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= +github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185 h1:08TEzxf9kSvLku2H82Q68c1FYsmdZYmKLDWDbUYIooI= +github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= From 3cdd9febb4dadf7afb5798272e55823440aaad17 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Tue, 14 Mar 2023 13:13:12 +0600 Subject: [PATCH 10/22] WIP: Address Gustavo's comments Needs: https://github.com/canonical/x-go/pull/19 --- README.md | 32 ++--- cmd/pebble/cmd_run.go | 32 ++--- go.mod | 2 + go.sum | 4 +- internal/daemon/daemon.go | 18 ++- internal/overlord/managers_test.go | 2 +- internal/overlord/overlord.go | 5 +- internal/overlord/overlord_test.go | 28 ++--- internal/overlord/servstate/handlers.go | 5 +- internal/overlord/servstate/manager.go | 52 ++++++--- internal/overlord/servstate/manager_test.go | 6 +- internal/plan/plan.go | 50 ++++---- internal/plan/plan_test.go | 122 ++++++++++++-------- 13 files changed, 205 insertions(+), 153 deletions(-) diff --git a/README.md b/README.md index ddae9fd26..25a526611 100644 --- a/README.md +++ b/README.md @@ -146,7 +146,7 @@ are marked as `startup: enabled` (if you don't want that, use `--hold`). Then other Pebble commands may be used to interact with the running daemon, for example, in another terminal window. -To provide additional arguments to a service, use `--args ...`. +To provide additional arguments to a service, use `--args ...`. If the `command` field in the service's plan has a `[ ]` list, the `--args` arguments will replace the defaults. If not, they will be appended to the command. @@ -158,16 +158,14 @@ may be omitted if there are no other Pebble options that follow. For example: ``` -# Start the daemon and pass additional arguments to "svc". -$ pebble run --args svc --verbose --foo "multi str arg" +# Start the daemon and pass additional arguments to "myservice". +$ pebble run --args myservice --verbose --foo "multi str arg" -# Start the daemon with all services on hold, but pass additional -# arguments to "svc" (demonstrates using a terminator). -$ pebble run --args svc --verbose --foo "multi str arg" \; --hold +# Use args terminator to pass --hold to Pebble at the end of the line. +$ pebble run --args myservice --verbose \; --hold # Start the daemon and pass arguments to multiple services. -$ pebble run --args svc1 --verbose --foo "multi str arg" \; \ - --args svc2 --somearg 1 --payload '{"foo": "bar"}' +$ pebble run --args myservice1 --arg1 \; --args myservice2 --arg2 ``` To override the default configuration directory, set the `PEBBLE` environment variable when running: @@ -447,19 +445,11 @@ services: # override the existing service spec in the plan with the same name. override: merge | replace - # (Required in combined layer) The command to run the service. The - # command is executed directly, not interpreted by a shell. - # - # The command may end with a list of arguments inside a [ ... ] - # block (the brackets must be surrounded by spaces). For example: - # - # command: server --verbose [ --port 8080 ] - # - # These default arguments can be overridden by calling "pebble run" - # with the --args option. - # - # Example: /usr/bin/somecommand -b -t 30 - command: [ ] + # (Required in combined layer) The command to run the service. It is executed + # directly, not interpreted by a shell, and may be optionally suffixed by default + # arguments within "[" and "]" which may be overriden via --args. + # Example: /usr/bin/somedaemon --db=/db/path [ --port 8080 ] + command: # (Optional) A short summary of the service. summary: diff --git a/cmd/pebble/cmd_run.go b/cmd/pebble/cmd_run.go index 22f044da7..f43a79862 100644 --- a/cmd/pebble/cmd_run.go +++ b/cmd/pebble/cmd_run.go @@ -44,9 +44,10 @@ If the command field in the service's plan has a [ ] list, the --args arguments will replace those default arguments. If not, the --args arguments will be appended to the command. -The semicolon terminator at the end of the --args list is only required -if other "run" arguments follow. If used at the shell, the ';' must be -backslash-escaped. +The optional --args flags is followed by a service name and its arguments, and must be +terminated by ";" unless there are no further Pebble options. For example: + + pebble run --args myservice --port=8080 \; --hold ` type cmdRun struct { @@ -145,13 +146,6 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal) error { if rcmd.Verbose { dopts.ServiceOutput = os.Stdout } - if rcmd.Args != nil { - mappedArgs, err := convertArgs(rcmd.Args) - if err != nil { - return err - } - dopts.ServiceArgs = mappedArgs - } dopts.HTTPAddress = rcmd.HTTP d, err := daemon.New(&dopts) @@ -162,6 +156,16 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal) error { return err } + if rcmd.Args != nil { + mappedArgs, err := convertArgs(rcmd.Args) + if err != nil { + return err + } + if err := d.PassServiceArgs(mappedArgs); err != nil { + return err + } + } + // Run sanity check now, if anything goes wrong with the // check we go into "degraded" mode where we always report // the given error to any client. @@ -228,14 +232,14 @@ func convertArgs(args [][]string) (map[string][]string, error) { mappedArgs := make(map[string][]string) for _, arg := range args { - if len(arg) < 2 { - return nil, fmt.Errorf("--args requires a service name and one or more additional arguments") + if len(arg) < 1 { + return nil, fmt.Errorf("--args requires a service name") } name := arg[0] if _, ok := mappedArgs[name]; ok { - return nil, fmt.Errorf("cannot use --args twice on a service") + return nil, fmt.Errorf("--args provided more than once for %q service", name) } - mappedArgs[name] = append(mappedArgs[name], arg[1:]...) + mappedArgs[name] = arg[1:] } return mappedArgs, nil diff --git a/go.mod b/go.mod index 826326e84..c53045b24 100644 --- a/go.mod +++ b/go.mod @@ -16,3 +16,5 @@ require ( ) replace github.com/jessevdk/go-flags v1.4.0 => github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185 + +replace github.com/canonical/x-go => github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e diff --git a/go.sum b/go.sum index c6d690dae..bba7317fe 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,3 @@ -github.com/canonical/x-go v0.0.0-20230113154138-0ccdb0b57a43 h1:bey1JgA3D2EBabr2a7kWKj+JlEPxX1akv8rcRromotA= -github.com/canonical/x-go v0.0.0-20230113154138-0ccdb0b57a43/go.mod h1:A0/Jvt7qKuCDss37TYRNXSscVyS+tLWM5kBYipQOpWQ= github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= @@ -11,6 +9,8 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= +github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e h1:KTgUhUsyol7xrNN4OOyCsqyeKApjsWqr+CPtXqJ/xyc= +github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e/go.mod h1:A0/Jvt7qKuCDss37TYRNXSscVyS+tLWM5kBYipQOpWQ= github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185 h1:08TEzxf9kSvLku2H82Q68c1FYsmdZYmKLDWDbUYIooI= github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index edf8f394f..5c7211082 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -72,9 +72,6 @@ type Options struct { // ServiceOuput is an optional io.Writer for the service log output, if set, all services // log output will be written to the writer. ServiceOutput io.Writer - - // ServiceArgs holds the service arguments provided via ``pebble run --args``. - ServiceArgs map[string][]string } // A Daemon listens for requests and routes them to the right command @@ -783,6 +780,19 @@ func (d *Daemon) RebootIsMissing(st *state.State) error { return errExpectedReboot } +// PassServiceArgs passes the provided service arguments to +// the service manager responsible for services under daemon overlord. +func (d *Daemon) PassServiceArgs(serviceArgs map[string][]string) error { + if d.overlord == nil { + return fmt.Errorf("internal error: no Overlord") + } + serviceMgr := d.overlord.ServiceManager() + if serviceMgr == nil { + return fmt.Errorf("internal error: no Service Manager") + } + return serviceMgr.SetServiceArgs(serviceArgs) +} + func New(opts *Options) (*Daemon, error) { d := &Daemon{ pebbleDir: opts.Dir, @@ -791,7 +801,7 @@ func New(opts *Options) (*Daemon, error) { httpAddress: opts.HTTPAddress, } - ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput, opts.ServiceArgs) + ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput) if err == errExpectedReboot { // we proceed without overlord until we reach Stop // where we will schedule and wait again for a system restart. diff --git a/internal/overlord/managers_test.go b/internal/overlord/managers_test.go index 9253dc1d5..c177b31c8 100644 --- a/internal/overlord/managers_test.go +++ b/internal/overlord/managers_test.go @@ -42,7 +42,7 @@ func (s *mgrsSuite) SetUpTest(c *C) { s.dir = c.MkDir() - o, err := overlord.New(s.dir, nil, nil, nil) + o, err := overlord.New(s.dir, nil, nil) c.Assert(err, IsNil) s.o = o } diff --git a/internal/overlord/overlord.go b/internal/overlord/overlord.go index 25ba19f8d..c9bf1cd79 100644 --- a/internal/overlord/overlord.go +++ b/internal/overlord/overlord.go @@ -72,8 +72,7 @@ type Overlord struct { // New creates a new Overlord with all its state managers. // It can be provided with an optional restart.Handler. -// And optionally additional arguments to services. -func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writer, serviceArgs map[string][]string) (*Overlord, error) { +func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writer) (*Overlord, error) { o := &Overlord{ pebbleDir: pebbleDir, loopTomb: new(tomb.Tomb), @@ -106,7 +105,7 @@ func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writ } o.runner.AddOptionalHandler(matchAnyUnknownTask, handleUnknownTask, nil) - o.serviceMgr, err = servstate.NewManager(s, o.runner, o.pebbleDir, serviceOutput, restartHandler, serviceArgs) + o.serviceMgr, err = servstate.NewManager(s, o.runner, o.pebbleDir, serviceOutput, restartHandler) if err != nil { return nil, err } diff --git a/internal/overlord/overlord_test.go b/internal/overlord/overlord_test.go index 47e600a9e..3b182d43b 100644 --- a/internal/overlord/overlord_test.go +++ b/internal/overlord/overlord_test.go @@ -58,7 +58,7 @@ func (ovs *overlordSuite) TestNew(c *C) { restore := patch.Fake(42, 2, nil) defer restore() - o, err := overlord.New(ovs.dir, nil, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil) c.Assert(err, IsNil) c.Check(o, NotNil) @@ -83,7 +83,7 @@ func (ovs *overlordSuite) TestNewWithGoodState(c *C) { err := ioutil.WriteFile(ovs.statePath, fakeState, 0600) c.Assert(err, IsNil) - o, err := overlord.New(ovs.dir, nil, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil) c.Assert(err, IsNil) state := o.State() @@ -111,7 +111,7 @@ func (ovs *overlordSuite) TestNewWithInvalidState(c *C) { err := ioutil.WriteFile(ovs.statePath, fakeState, 0600) c.Assert(err, IsNil) - _, err = overlord.New(ovs.dir, nil, nil, nil) + _, err = overlord.New(ovs.dir, nil, nil) c.Assert(err, ErrorMatches, "cannot read state: EOF") } @@ -130,7 +130,7 @@ func (ovs *overlordSuite) TestNewWithPatches(c *C) { err := ioutil.WriteFile(ovs.statePath, fakeState, 0600) c.Assert(err, IsNil) - o, err := overlord.New(ovs.dir, nil, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil) c.Assert(err, IsNil) state := o.State() @@ -175,7 +175,7 @@ func (wm *witnessManager) Ensure() error { } func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) { - o, err := overlord.New(ovs.dir, nil, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil) c.Assert(err, IsNil) o.Loop() @@ -185,7 +185,7 @@ func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) { } func (ovs *overlordSuite) TestUnknownTasks(c *C) { - o, err := overlord.New(ovs.dir, nil, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil) c.Assert(err, IsNil) // unknown tasks are ignored and succeed @@ -486,7 +486,7 @@ func (ovs *overlordSuite) TestCheckpoint(c *C) { oldUmask := syscall.Umask(0) defer syscall.Umask(oldUmask) - o, err := overlord.New(ovs.dir, nil, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil) c.Assert(err, IsNil) s := o.State() @@ -727,7 +727,7 @@ func (ovs *overlordSuite) TestSettleExplicitEnsureBefore(c *C) { } func (ovs *overlordSuite) TestRequestRestartNoHandler(c *C) { - o, err := overlord.New(ovs.dir, nil, nil, nil) + o, err := overlord.New(ovs.dir, nil, nil) c.Assert(err, IsNil) st := o.State() @@ -760,7 +760,7 @@ func (rb *testRestartHandler) RebootIsMissing(_ *state.State) error { func (ovs *overlordSuite) TestRequestRestartHandler(c *C) { rb := &testRestartHandler{} - o, err := overlord.New(ovs.dir, rb, nil, nil) + o, err := overlord.New(ovs.dir, rb, nil) c.Assert(err, IsNil) st := o.State() @@ -779,7 +779,7 @@ func (ovs *overlordSuite) TestVerifyRebootNoPendingReboot(c *C) { rb := &testRestartHandler{} - _, err = overlord.New(ovs.dir, rb, nil, nil) + _, err = overlord.New(ovs.dir, rb, nil) c.Assert(err, IsNil) c.Check(rb.rebootState, Equals, "as-expected") @@ -792,7 +792,7 @@ func (ovs *overlordSuite) TestVerifyRebootOK(c *C) { rb := &testRestartHandler{} - _, err = overlord.New(ovs.dir, rb, nil, nil) + _, err = overlord.New(ovs.dir, rb, nil) c.Assert(err, IsNil) c.Check(rb.rebootState, Equals, "as-expected") @@ -806,7 +806,7 @@ func (ovs *overlordSuite) TestVerifyRebootOKButError(c *C) { e := errors.New("boom") rb := &testRestartHandler{rebootVerifiedErr: e} - _, err = overlord.New(ovs.dir, rb, nil, nil) + _, err = overlord.New(ovs.dir, rb, nil) c.Assert(err, Equals, e) c.Check(rb.rebootState, Equals, "as-expected") @@ -822,7 +822,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissing(c *C) { rb := &testRestartHandler{} - _, err = overlord.New(ovs.dir, rb, nil, nil) + _, err = overlord.New(ovs.dir, rb, nil) c.Assert(err, IsNil) c.Check(rb.rebootState, Equals, "did-not-happen") @@ -839,7 +839,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissingError(c *C) { e := errors.New("boom") rb := &testRestartHandler{rebootVerifiedErr: e} - _, err = overlord.New(ovs.dir, rb, nil, nil) + _, err = overlord.New(ovs.dir, rb, nil) c.Assert(err, Equals, e) c.Check(rb.rebootState, Equals, "did-not-happen") diff --git a/internal/overlord/servstate/handlers.go b/internal/overlord/servstate/handlers.go index 3fcffaf84..437a8eede 100644 --- a/internal/overlord/servstate/handlers.go +++ b/internal/overlord/servstate/handlers.go @@ -324,10 +324,11 @@ func logError(err error) { // command. It assumes the caller has ensures the service is in a valid state, // and it sets s.cmd and other relevant fields. func (s *serviceData) startInternal() error { - args, err := plan.CommandArgs(s.config.Command, s.manager.serviceArgs[s.config.Name]) + base, extra, err := s.config.ParseCommand() if err != nil { return err } + args := append(base, extra...) s.cmd = exec.Command(args[0], args[1:]...) s.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} @@ -383,7 +384,7 @@ func (s *serviceData) startInternal() error { s.cmd.Stderr = logWriter // Start the process! - logger.Noticef("Service %q starting: %q", serviceName, args) + logger.Noticef("Service %q starting: %s", serviceName, s.config.Command) err = reaper.StartCommand(s.cmd) if err != nil { if outputIterator != nil { diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index 76d76b756..99596f20b 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -28,7 +28,6 @@ type ServiceManager struct { servicesLock sync.Mutex services map[string]*serviceData - serviceArgs map[string][]string serviceOutput io.Writer restarter Restarter @@ -54,13 +53,12 @@ func (e *LabelExists) Error() string { return fmt.Sprintf("layer %q already exists", e.Label) } -func NewManager(s *state.State, runner *state.TaskRunner, pebbleDir string, serviceOutput io.Writer, restarter Restarter, serviceArgs map[string][]string) (*ServiceManager, error) { +func NewManager(s *state.State, runner *state.TaskRunner, pebbleDir string, serviceOutput io.Writer, restarter Restarter) (*ServiceManager, error) { manager := &ServiceManager{ state: s, runner: runner, pebbleDir: pebbleDir, services: make(map[string]*serviceData), - serviceArgs: serviceArgs, serviceOutput: serviceOutput, restarter: restarter, rand: rand.New(rand.NewSource(time.Now().UnixNano())), @@ -150,11 +148,6 @@ func (m *ServiceManager) appendLayer(layer *plan.Layer) error { } layer.Order = newOrder - // Remove the existing arguments of this layer's services - for _, service := range layer.Services { - delete(m.serviceArgs, service.Name) - } - return nil } @@ -218,11 +211,6 @@ func (m *ServiceManager) CombineLayer(layer *plan.Layer) error { } layer.Order = found.Order - // Remove the existing arguments of this layer's services - for _, service := range layer.Services { - delete(m.serviceArgs, service.Name) - } - return nil } @@ -497,6 +485,44 @@ func (m *ServiceManager) CheckFailed(name string) { } } +// SetServiceArgs sets the service arguments provided by "pebble run --args" +// to their respective services. It updates the service command in the top most +// layer which contains the service in the plan. +func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { + releasePlan, err := m.acquirePlan() + if err != nil { + return err + } + defer releasePlan() + + layers := m.plan.Layers + + for serviceName, args := range serviceArgs { + var layer *plan.Layer + + // search for the topmost layer which contains the service + // assuming the layers are sorted by order ASC + for i := len(layers) - 1; i >= 0; i-- { + if _, ok := layers[i].Services[serviceName]; ok { + layer = layers[i] + break + } + } + if layer == nil { + return fmt.Errorf("service %q not found in plan", serviceName) + } + + service := layer.Services[serviceName] + base, _, err := service.ParseCommand() + if err != nil { + return err + } + service.Command = plan.CommandString(base, args) + } + + return m.updatePlanLayers(layers) +} + // servicesToStop returns a slice of service names to stop, in dependency order. func servicesToStop(m *ServiceManager) ([]string, error) { releasePlan, err := m.acquirePlan() diff --git a/internal/overlord/servstate/manager_test.go b/internal/overlord/servstate/manager_test.go index 97bfda999..b0d873abb 100644 --- a/internal/overlord/servstate/manager_test.go +++ b/internal/overlord/servstate/manager_test.go @@ -160,7 +160,7 @@ func (s *S) SetUpTest(c *C) { s.runner = state.NewTaskRunner(s.st) s.stopDaemon = make(chan struct{}) - manager, err := servstate.NewManager(s.st, s.runner, s.dir, logOutput, testRestarter{s.stopDaemon}, nil) + manager, err := servstate.NewManager(s.st, s.runner, s.dir, logOutput, testRestarter{s.stopDaemon}) c.Assert(err, IsNil) s.AddCleanup(manager.Stop) s.manager = manager @@ -600,7 +600,7 @@ func (s *S) TestAppendLayer(c *C) { dir := c.MkDir() os.Mkdir(filepath.Join(dir, "layers"), 0755) runner := state.NewTaskRunner(s.st) - manager, err := servstate.NewManager(s.st, runner, dir, nil, nil, nil) + manager, err := servstate.NewManager(s.st, runner, dir, nil, nil) c.Assert(err, IsNil) defer manager.Stop() @@ -683,7 +683,7 @@ func (s *S) TestCombineLayer(c *C) { dir := c.MkDir() os.Mkdir(filepath.Join(dir, "layers"), 0755) runner := state.NewTaskRunner(s.st) - manager, err := servstate.NewManager(s.st, runner, dir, nil, nil, nil) + manager, err := servstate.NewManager(s.st, runner, dir, nil, nil) c.Assert(err, IsNil) defer manager.Stop() diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 2f2b7c8a1..9c9a625fd 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -184,59 +184,57 @@ func (s *Service) Equal(other *Service) bool { return reflect.DeepEqual(s, other) } -// CommandArgs returns a service command as a stream of strings. -// It adds the arguments in cmdArgs to the command if no default -// argument is specified in [ ... ] group in the plan. -// If default arguments are specified in the plan, the default -// arguments are added to the command if cmdArgs is empty. -// If not empty, the default arguments are overrided by arguments in cmdArgs. -func CommandArgs(command string, cmdArgs []string) ([]string, error) { - args, err := shlex.Split(command) +// ParseCommand returns a service command as two stream of strings. +// The base command is returned as a stream and the default arguments +// in [ ... ] group is returned as another stream. +func (s *Service) ParseCommand() (base, extra []string, err error) { + args, err := shlex.Split(s.Command) if err != nil { - return nil, err + return nil, nil, err } var inBrackets, gotBrackets bool - var result []string for idx, arg := range args { if inBrackets { if arg == "[" { - return nil, fmt.Errorf("cannot nest [ ... ] groups") + return nil, nil, fmt.Errorf("cannot nest [ ... ] groups") } if arg == "]" { inBrackets = false continue } - if len(cmdArgs) == 0 { - result = append(result, arg) - } + extra = append(extra, arg) continue } if gotBrackets { - return nil, fmt.Errorf("cannot have any args after [ ... ] group") + return nil, nil, fmt.Errorf("cannot have any args after [ ... ] group") } if arg == "[" { if idx == 0 { - return nil, fmt.Errorf("cannot have [ ... ] group as prefix") - } - if len(cmdArgs) > 0 { - result = append(result, cmdArgs...) + return nil, nil, fmt.Errorf("cannot have [ ... ] group as prefix") } inBrackets = true gotBrackets = true continue } if arg == "]" { - return nil, fmt.Errorf("cannot have ] outside of [ ... ] group") + return nil, nil, fmt.Errorf("cannot have ] outside of [ ... ] group") } - result = append(result, arg) - } - if !gotBrackets && len(cmdArgs) > 0 { - result = append(result, cmdArgs...) + base = append(base, arg) } - return result, nil + return base, extra, nil +} + +// CommandString returns a service command as a string after +// appending the arguments in "extra" to the command in "base" +func CommandString(base, extra []string) string { + output := shlex.Join(base) + if len(extra) > 0 { + output = output + " [ " + shlex.Join(extra) + " ]" + } + return output } type ServiceStartup string @@ -538,7 +536,7 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { Message: fmt.Sprintf(`plan must define "command" for service %q`, name), } } - _, err := CommandArgs(service.Command, nil) + _, _, err := service.ParseCommand() if err != nil { return nil, &FormatError{ Message: fmt.Sprintf("plan service %q command invalid: %v", name, err), diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index 489788e94..8e76ad46a 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -1065,64 +1065,86 @@ func (s *S) TestMarshalLayer(c *C) { c.Assert(string(out), Equals, string(layerBytes)) } -func (s *S) TestGetCommand(c *C) { - cmdTests := []struct { - summary string - command string - cmdArgs []string - expectedCommand []string - error string - }{ - { - summary: "No default arguments, no additional cmdArgs", - command: "cmd --foo bar", - expectedCommand: []string{"cmd", "--foo", "bar"}, - }, { - summary: "No default arguments, add cmdArgs only", - command: "cmd --foo bar", - cmdArgs: []string{"-v", "--opt"}, - expectedCommand: []string{"cmd", "--foo", "bar", "-v", "--opt"}, - }, { - summary: "Default arguments only", - command: "cmd [ --foo bar ]", - expectedCommand: []string{"cmd", "--foo", "bar"}, - }, { - summary: "Override default arguments with cmdArgs", - command: "cmd [ --foo bar ]", - cmdArgs: []string{"--bar", "foo"}, - expectedCommand: []string{"cmd", "--bar", "foo"}, - }, { - summary: "Empty [ ... ], no cmdArgs", - command: "cmd --foo bar [ ]", - expectedCommand: []string{"cmd", "--foo", "bar"}, - }, { - summary: "Empty [ ... ], override with cmdArgs", - command: "cmd --foo bar [ ]", - cmdArgs: []string{"-v", "--opt"}, - expectedCommand: []string{"cmd", "--foo", "bar", "-v", "--opt"}, - }, { - summary: "[ ... ] should be a suffix", - command: "cmd [ --foo ] --bar", - error: `cannot have any args after \[ ... \] group`, - }, { - summary: "[ ... ] should not be prefix", - command: "[ cmd --foo ]", - error: `cannot have \[ ... \] group as prefix`, - }, - } +var cmdTests = []struct { + summary string + command string + cmdArgs []string + expectedBase []string + expectedExtra []string + expectedNewCommand string + error string +}{{ + summary: "No default arguments, no additional cmdArgs", + command: "cmd --foo bar", + expectedBase: []string{"cmd", "--foo", "bar"}, + expectedNewCommand: "cmd --foo bar", +}, { + summary: "No default arguments, add cmdArgs only", + command: "cmd --foo bar", + cmdArgs: []string{"-v", "--opt"}, + expectedBase: []string{"cmd", "--foo", "bar"}, + expectedNewCommand: "cmd --foo bar [ -v --opt ]", +}, { + summary: "Override default arguments with empty cmdArgs", + command: "cmd [ --foo bar ]", + expectedBase: []string{"cmd"}, + expectedExtra: []string{"--foo", "bar"}, + expectedNewCommand: "cmd", +}, { + summary: "Override default arguments with cmdArgs", + command: "cmd [ --foo bar ]", + cmdArgs: []string{"--bar", "foo"}, + expectedBase: []string{"cmd"}, + expectedExtra: []string{"--foo", "bar"}, + expectedNewCommand: "cmd [ --bar foo ]", +}, { + summary: "Empty [ ... ], no cmdArgs", + command: "cmd --foo bar [ ]", + expectedBase: []string{"cmd", "--foo", "bar"}, + expectedNewCommand: "cmd --foo bar", +}, { + summary: "Empty [ ... ], override with cmdArgs", + command: "cmd --foo bar [ ]", + cmdArgs: []string{"-v", "--opt"}, + expectedBase: []string{"cmd", "--foo", "bar"}, + expectedNewCommand: "cmd --foo bar [ -v --opt ]", +}, { + summary: "[ ... ] should be a suffix", + command: "cmd [ --foo ] --bar", + error: `cannot have any args after \[ ... \] group`, +}, { + summary: "[ ... ] should not be prefix", + command: "[ cmd --foo ]", + error: `cannot have \[ ... \] group as prefix`, +}} +func (s *S) TestParseCommand(c *C) { for _, test := range cmdTests { - svc := plan.Service{Command: test.command} - cmd, err := plan.CommandArgs(svc.Command, test.cmdArgs) - if err == nil { - c.Assert(cmd, DeepEquals, test.expectedCommand) - } + service := plan.Service{Command: test.command} + + // parse base and the default arguments in [ ... ] + base, extra, err := service.ParseCommand() if err != nil || test.error != "" { if test.error != "" { c.Assert(err, ErrorMatches, test.error) } else { c.Assert(err, IsNil) } + continue } + c.Assert(base, DeepEquals, test.expectedBase) + c.Assert(extra, DeepEquals, test.expectedExtra) + + // add cmdArgs to base and produce a new command string + newCommand := plan.CommandString(base, test.cmdArgs) + c.Assert(newCommand, DeepEquals, test.expectedNewCommand) + + // parse the new command string again and check if base is + // the same and cmdArgs is the new default arguments in [ ... ] + service.Command = newCommand + base, extra, err = service.ParseCommand() + c.Assert(err, IsNil) + c.Assert(base, DeepEquals, test.expectedBase) + c.Assert(extra, DeepEquals, test.cmdArgs) } } From ba40a3a4a105f6d6fb60da37ab6ae3ecc9c2c343 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 16 Mar 2023 13:56:37 +0600 Subject: [PATCH 11/22] WIP: Address Tomas' comments --- internal/overlord/servstate/manager.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index 99596f20b..3b438375a 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -498,21 +498,19 @@ func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { layers := m.plan.Layers for serviceName, args := range serviceArgs { - var layer *plan.Layer + var service *plan.Service // search for the topmost layer which contains the service // assuming the layers are sorted by order ASC for i := len(layers) - 1; i >= 0; i-- { - if _, ok := layers[i].Services[serviceName]; ok { - layer = layers[i] + if service = layers[i].Services[serviceName]; service != nil { break } } - if layer == nil { + if service == nil { return fmt.Errorf("service %q not found in plan", serviceName) } - service := layer.Services[serviceName] base, _, err := service.ParseCommand() if err != nil { return err From 47995aebd8b5d3262474443438c9d3550ee26c1b Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 30 Mar 2023 13:07:27 +0600 Subject: [PATCH 12/22] WIP: Address Cris' comments --- internal/plan/plan.go | 2 +- internal/plan/plan_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 9c9a625fd..be9052a8f 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -212,7 +212,7 @@ func (s *Service) ParseCommand() (base, extra []string, err error) { } if arg == "[" { if idx == 0 { - return nil, nil, fmt.Errorf("cannot have [ ... ] group as prefix") + return nil, nil, fmt.Errorf("command cannot be started with default arguments") } inBrackets = true gotBrackets = true diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index 8e76ad46a..2bad073dc 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -1115,7 +1115,7 @@ var cmdTests = []struct { }, { summary: "[ ... ] should not be prefix", command: "[ cmd --foo ]", - error: `cannot have \[ ... \] group as prefix`, + error: `command cannot be started with default arguments`, }} func (s *S) TestParseCommand(c *C) { From d0427e93abb020c49198e89fd70206bda9cbcd48 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 30 Mar 2023 13:07:42 +0600 Subject: [PATCH 13/22] Point to canonical/go-flags https://github.com/canonical/go-flags/pull/2 has been merged. Point to the recent commit https://github.com/canonical/go-flags/commit/01157a7cbb96bf1cf9d425ce8f6c899171dec6a0. --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index c53045b24..dd54a674e 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,6 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -replace github.com/jessevdk/go-flags v1.4.0 => github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185 +replace github.com/jessevdk/go-flags v1.4.0 => github.com/canonical/go-flags v0.0.0-20230327193835-01157a7cbb96 replace github.com/canonical/x-go => github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e diff --git a/go.sum b/go.sum index bba7317fe..68a97a9f4 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/canonical/go-flags v0.0.0-20230327193835-01157a7cbb96 h1:vu+hci8voxBgl6vmdL8/WkCfPItYNJpHY1OBi1knWV8= +github.com/canonical/go-flags v0.0.0-20230327193835-01157a7cbb96/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= @@ -11,8 +13,6 @@ github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e h1:KTgUhUsyol7xrNN4OOyCsqyeKApjsWqr+CPtXqJ/xyc= github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e/go.mod h1:A0/Jvt7qKuCDss37TYRNXSscVyS+tLWM5kBYipQOpWQ= -github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185 h1:08TEzxf9kSvLku2H82Q68c1FYsmdZYmKLDWDbUYIooI= -github.com/rebornplusplus/go-flags v0.0.0-20230227064049-18b729e1c185/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= From 8e6ef62bef2c5afd5f3bce225a5d0995bc0e0d71 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 6 Apr 2023 11:42:06 +0600 Subject: [PATCH 14/22] Add new layer of service args on top of plan .. and a few rewording and cleanup. --- cmd/pebble/cmd_run.go | 19 ++++-------- internal/daemon/daemon.go | 15 +++------ internal/overlord/servstate/manager.go | 42 +++++++++++--------------- 3 files changed, 28 insertions(+), 48 deletions(-) diff --git a/cmd/pebble/cmd_run.go b/cmd/pebble/cmd_run.go index f43a79862..3304d8a39 100644 --- a/cmd/pebble/cmd_run.go +++ b/cmd/pebble/cmd_run.go @@ -35,19 +35,12 @@ var shortRunHelp = "Run the pebble environment" var longRunHelp = ` The run command starts pebble and runs the configured environment. -Additional arguments can be provided to a service command with the ---args option, in the following format: +Additional arguments may be provided to the service command with the --args option, which +must be terminated with ";" unless there are no further Pebble options. These arguments +are appended to the end of the service command, and replace any default arguments defined +in the service plan. For example: - --args ; - -If the command field in the service's plan has a [ ] -list, the --args arguments will replace those default arguments. If not, the ---args arguments will be appended to the command. - -The optional --args flags is followed by a service name and its arguments, and must be -terminated by ";" unless there are no further Pebble options. For example: - - pebble run --args myservice --port=8080 \; --hold + $ pebble run --args myservice --port 8080 \; --hold ` type cmdRun struct { @@ -161,7 +154,7 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal) error { if err != nil { return err } - if err := d.PassServiceArgs(mappedArgs); err != nil { + if err := d.SetServiceArgs(mappedArgs); err != nil { return err } } diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 5c7211082..567f70666 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -780,17 +780,10 @@ func (d *Daemon) RebootIsMissing(st *state.State) error { return errExpectedReboot } -// PassServiceArgs passes the provided service arguments to -// the service manager responsible for services under daemon overlord. -func (d *Daemon) PassServiceArgs(serviceArgs map[string][]string) error { - if d.overlord == nil { - return fmt.Errorf("internal error: no Overlord") - } - serviceMgr := d.overlord.ServiceManager() - if serviceMgr == nil { - return fmt.Errorf("internal error: no Service Manager") - } - return serviceMgr.SetServiceArgs(serviceArgs) +// SetServiceArgs sets the provided service arguments to their respective services, +// by passing the arguments to the service manager responsible under daemon overlord. +func (d *Daemon) SetServiceArgs(serviceArgs map[string][]string) error { + return d.overlord.ServiceManager().SetServiceArgs(serviceArgs) } func New(opts *Options) (*Daemon, error) { diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index 3b438375a..1cffb01b4 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -115,10 +115,9 @@ func (m *ServiceManager) Plan() (*plan.Plan, error) { return m.plan, nil } -// AppendLayer appends the given layer to the plan's layers, updates the -// layer.Order field to the new order and removes the existing arguments -// of this layer's services. If a layer with layer.Label already exists, -// return an error of type *LabelExists. +// AppendLayer appends the given layer to the plan's layers and updates the +// layer.Order field to the new order. If a layer with layer.Label already +// exists, return an error of type *LabelExists. func (m *ServiceManager) AppendLayer(layer *plan.Layer) error { releasePlan, err := m.acquirePlan() if err != nil { @@ -178,8 +177,7 @@ func findLayer(layers []*plan.Layer, label string) (int, *plan.Layer) { // CombineLayer combines the given layer with an existing layer that has the // same label. If no existing layer has the label, append a new one. In either -// case, update the layer.Order field to the new order and remove existing -// arguments of this layer's services. +// case, update the layer.Order field to the new order. func (m *ServiceManager) CombineLayer(layer *plan.Layer) error { releasePlan, err := m.acquirePlan() if err != nil { @@ -486,8 +484,8 @@ func (m *ServiceManager) CheckFailed(name string) { } // SetServiceArgs sets the service arguments provided by "pebble run --args" -// to their respective services. It updates the service command in the top most -// layer which contains the service in the plan. +// to their respective services. It adds a new layer in the plan, the layer +// consisting of services with commands having their arguments changed. func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { releasePlan, err := m.acquirePlan() if err != nil { @@ -495,30 +493,26 @@ func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { } defer releasePlan() - layers := m.plan.Layers - - for serviceName, args := range serviceArgs { - var service *plan.Service + newLayer := &plan.Layer{ + Services: make(map[string]*plan.Service), + } - // search for the topmost layer which contains the service - // assuming the layers are sorted by order ASC - for i := len(layers) - 1; i >= 0; i-- { - if service = layers[i].Services[serviceName]; service != nil { - break - } + for name, args := range serviceArgs { + service, ok := m.plan.Services[name] + if !ok { + return fmt.Errorf("service %q not found in plan", name) } - if service == nil { - return fmt.Errorf("service %q not found in plan", serviceName) - } - base, _, err := service.ParseCommand() if err != nil { return err } - service.Command = plan.CommandString(base, args) + newLayer.Services[name] = &plan.Service{ + Override: plan.MergeOverride, + Command: plan.CommandString(base, args), + } } - return m.updatePlanLayers(layers) + return m.appendLayer(newLayer) } // servicesToStop returns a slice of service names to stop, in dependency order. From 2d8ac71cabbc86bb99b7951e56b18d30b0596865 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 6 Apr 2023 12:29:36 +0600 Subject: [PATCH 15/22] Remove leftover newlines --- internal/overlord/servstate/manager.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index 1cffb01b4..7eba3e6e4 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -146,7 +146,6 @@ func (m *ServiceManager) appendLayer(layer *plan.Layer) error { return err } layer.Order = newOrder - return nil } @@ -208,7 +207,6 @@ func (m *ServiceManager) CombineLayer(layer *plan.Layer) error { return err } layer.Order = found.Order - return nil } From 6369d7831d14d535f9bf7e80985beb842b2043ba Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Wed, 3 May 2023 12:20:25 +0200 Subject: [PATCH 16/22] Add label for the new --args layer on top --- internal/overlord/servstate/manager.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index a9d36b2dc..eccfd8350 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -524,6 +524,8 @@ func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { defer releasePlan() newLayer := &plan.Layer{ + // TODO: Consider making "pebble-service-args" a reserved label. + Label: "pebble-service-args", Services: make(map[string]*plan.Service), } From 0b7d3cc2b950ec065dd680146040c700b104a906 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Wed, 3 May 2023 12:20:53 +0200 Subject: [PATCH 17/22] Add tests for servstate.ServiceManager.SetServiceArgs --- internal/overlord/servstate/manager_test.go | 48 +++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/internal/overlord/servstate/manager_test.go b/internal/overlord/servstate/manager_test.go index 570bdb54c..6a07c1560 100644 --- a/internal/overlord/servstate/manager_test.go +++ b/internal/overlord/servstate/manager_test.go @@ -851,6 +851,54 @@ services: s.planLayersHasLen(c, manager, 3) } +func (s *S) TestSetServiceArgs(c *C) { + dir := c.MkDir() + os.Mkdir(filepath.Join(dir, "layers"), 0755) + runner := state.NewTaskRunner(s.st) + manager, err := servstate.NewManager(s.st, runner, dir, nil, nil, fakeLogManager{}) + c.Assert(err, IsNil) + defer manager.Stop() + + // Append a layer with a few services having default args. + layer := parseLayer(c, 0, "base-layer", ` +services: + svc1: + override: replace + command: foo [ --bar ] + svc2: + override: replace + command: foo + svc3: + override: replace + command: foo +`) + err = manager.AppendLayer(layer) + c.Assert(err, IsNil) + c.Assert(layer.Order, Equals, 1) + s.planLayersHasLen(c, manager, 1) + + // Set arguments to services. + serviceArgs := map[string][]string{ + "svc1": {"-abc", "--xyz"}, + "svc2": {"--bar"}, + } + err = manager.SetServiceArgs(serviceArgs) + c.Assert(err, IsNil) + c.Assert(planYAML(c, manager), Equals, ` +services: + svc1: + override: replace + command: foo [ -abc --xyz ] + svc2: + override: replace + command: foo [ --bar ] + svc3: + override: replace + command: foo +`[1:]) + s.planLayersHasLen(c, manager, 2) +} + func (s *S) TestServices(c *C) { started := time.Now() services, err := s.manager.Services(nil) From 241bdf7aef2958f84d8d0b70f1d38966c023e35f Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 4 May 2023 13:45:28 +0200 Subject: [PATCH 18/22] Update TODO comment on reserved labels --- internal/overlord/servstate/manager.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index eccfd8350..70e088ade 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -525,6 +525,8 @@ func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { newLayer := &plan.Layer{ // TODO: Consider making "pebble-service-args" a reserved label. + // Or more generally, consider making any labels starting with + // the "pebble-" prefix reserved. Label: "pebble-service-args", Services: make(map[string]*plan.Service), } From 62af8fe43d4d59d11e88ae9c9fb11b5ac6ffdaf7 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Thu, 11 May 2023 14:13:11 +0600 Subject: [PATCH 19/22] Include updated strutil/shlex commit in go.mod, go.sum --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 4a85773fc..8460857a4 100644 --- a/go.mod +++ b/go.mod @@ -15,4 +15,4 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -replace github.com/canonical/x-go => github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e +replace github.com/canonical/x-go => github.com/rebornplusplus/canonical-x-go v0.0.0-20230425082201-61c9f268b5f0 diff --git a/go.sum b/go.sum index ca2f6d812..028f53c91 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,8 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= -github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e h1:KTgUhUsyol7xrNN4OOyCsqyeKApjsWqr+CPtXqJ/xyc= -github.com/rebornplusplus/canonical-x-go v0.0.0-20230309051359-1ec2b2ee698e/go.mod h1:A0/Jvt7qKuCDss37TYRNXSscVyS+tLWM5kBYipQOpWQ= +github.com/rebornplusplus/canonical-x-go v0.0.0-20230425082201-61c9f268b5f0 h1:LuYZQAYCYeQSmws6pZGIfAbr8fg+zSATmmyLNrjPAas= +github.com/rebornplusplus/canonical-x-go v0.0.0-20230425082201-61c9f268b5f0/go.mod h1:A0/Jvt7qKuCDss37TYRNXSscVyS+tLWM5kBYipQOpWQ= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= From d686b6f3a76b21b1cde43b4565dc1fcb565aafcc Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Tue, 16 May 2023 17:35:29 +0600 Subject: [PATCH 20/22] update error messages and TODO comments Update error messages in ``plan.Service.ParseCommand`` for more context. Additionally, update the TODO comment in ``servstate.ServiceManager.SetServiceArgs``. --- internal/overlord/servstate/manager.go | 3 +-- internal/plan/plan.go | 10 ++++++++-- internal/plan/plan_test.go | 16 ++++++++-------- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/internal/overlord/servstate/manager.go b/internal/overlord/servstate/manager.go index 70e088ade..691f0a546 100644 --- a/internal/overlord/servstate/manager.go +++ b/internal/overlord/servstate/manager.go @@ -524,8 +524,7 @@ func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error { defer releasePlan() newLayer := &plan.Layer{ - // TODO: Consider making "pebble-service-args" a reserved label. - // Or more generally, consider making any labels starting with + // TODO: Consider making any labels starting with // the "pebble-" prefix reserved. Label: "pebble-service-args", Services: make(map[string]*plan.Service), diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 63da646c5..e12389c81 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -215,6 +215,12 @@ func (s *Service) Equal(other *Service) bool { // The base command is returned as a stream and the default arguments // in [ ... ] group is returned as another stream. func (s *Service) ParseCommand() (base, extra []string, err error) { + defer func() { + if err != nil { + err = fmt.Errorf("cannot parse service %q command: %w", s.Name, err) + } + }() + args, err := shlex.Split(s.Command) if err != nil { return nil, nil, err @@ -235,11 +241,11 @@ func (s *Service) ParseCommand() (base, extra []string, err error) { continue } if gotBrackets { - return nil, nil, fmt.Errorf("cannot have any args after [ ... ] group") + return nil, nil, fmt.Errorf("cannot have any arguments after [ ... ] group") } if arg == "[" { if idx == 0 { - return nil, nil, fmt.Errorf("command cannot be started with default arguments") + return nil, nil, fmt.Errorf("cannot start command with [ ... ] group") } inBrackets = true gotBrackets = true diff --git a/internal/plan/plan_test.go b/internal/plan/plan_test.go index 82e590a7f..f44dc0035 100644 --- a/internal/plan/plan_test.go +++ b/internal/plan/plan_test.go @@ -475,7 +475,7 @@ var planTests = []planTest{{ `}, }, { summary: `Invalid service command`, - error: `plan service "svc1" command invalid: EOF found when expecting closing quote`, + error: `plan service "svc1" command invalid: cannot parse service "svc1" command: EOF found when expecting closing quote`, input: []string{` services: svc1: @@ -504,8 +504,8 @@ var planTests = []planTest{{ LogTargets: map[string]*plan.LogTarget{}, }}, }, { - summary: `Invalid service command: cannot have any args after [ ... ] group`, - error: `plan service "svc1" command invalid: cannot have any args after \[ ... \] group`, + summary: `Invalid service command: cannot have any arguments after [ ... ] group`, + error: `plan service "svc1" command invalid: cannot parse service "svc1" command: cannot have any arguments after \[ ... \] group`, input: []string{` services: "svc1": @@ -514,7 +514,7 @@ var planTests = []planTest{{ `}, }, { summary: `Invalid service command: cannot have ] outside of [ ... ] group`, - error: `plan service "svc1" command invalid: cannot have \] outside of \[ ... \] group`, + error: `plan service "svc1" command invalid: cannot parse service "svc1" command: cannot have \] outside of \[ ... \] group`, input: []string{` services: "svc1": @@ -523,7 +523,7 @@ var planTests = []planTest{{ `}, }, { summary: `Invalid service command: cannot nest [ ... ] groups`, - error: `plan service "svc1" command invalid: cannot nest \[ ... \] groups`, + error: `plan service "svc1" command invalid: cannot parse service "svc1" command: cannot nest \[ ... \] groups`, input: []string{` services: "svc1": @@ -1417,16 +1417,16 @@ var cmdTests = []struct { }, { summary: "[ ... ] should be a suffix", command: "cmd [ --foo ] --bar", - error: `cannot have any args after \[ ... \] group`, + error: `cannot parse service "svc" command: cannot have any arguments after \[ ... \] group`, }, { summary: "[ ... ] should not be prefix", command: "[ cmd --foo ]", - error: `command cannot be started with default arguments`, + error: `cannot parse service "svc" command: cannot start command with \[ ... \] group`, }} func (s *S) TestParseCommand(c *C) { for _, test := range cmdTests { - service := plan.Service{Command: test.command} + service := plan.Service{Name: "svc", Command: test.command} // parse base and the default arguments in [ ... ] base, extra, err := service.ParseCommand() From 53939c3c18ca270e8bd5f554ef46ea197f35a781 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Tue, 16 May 2023 17:39:06 +0600 Subject: [PATCH 21/22] revert go.mod and go.sum to upstream/master Note that tests will fail, since the new commit in canonical/x-go is not included **yet**. --- go.mod | 2 -- go.sum | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 8460857a4..7ea999110 100644 --- a/go.mod +++ b/go.mod @@ -14,5 +14,3 @@ require ( gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637 gopkg.in/yaml.v3 v3.0.1 ) - -replace github.com/canonical/x-go => github.com/rebornplusplus/canonical-x-go v0.0.0-20230425082201-61c9f268b5f0 diff --git a/go.sum b/go.sum index 028f53c91..5ab5b5583 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8 h1:zGaJEJI9qPVyM+QKFJagiyrM91Ke5S9htoL1D470g6E= github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8/go.mod h1:ZZFeR9K9iGgpwOaLYF9PdT44/+lfSJ9sQz3B+SsGsYU= +github.com/canonical/x-go v0.0.0-20230113154138-0ccdb0b57a43 h1:bey1JgA3D2EBabr2a7kWKj+JlEPxX1akv8rcRromotA= +github.com/canonical/x-go v0.0.0-20230113154138-0ccdb0b57a43/go.mod h1:A0/Jvt7qKuCDss37TYRNXSscVyS+tLWM5kBYipQOpWQ= github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= @@ -11,8 +13,6 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk= github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw= -github.com/rebornplusplus/canonical-x-go v0.0.0-20230425082201-61c9f268b5f0 h1:LuYZQAYCYeQSmws6pZGIfAbr8fg+zSATmmyLNrjPAas= -github.com/rebornplusplus/canonical-x-go v0.0.0-20230425082201-61c9f268b5f0/go.mod h1:A0/Jvt7qKuCDss37TYRNXSscVyS+tLWM5kBYipQOpWQ= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= From 43ef4ef1b60f77561f8da9b016fdf81f412f34b3 Mon Sep 17 00:00:00 2001 From: Rafid Bin Mostofa Date: Mon, 22 May 2023 16:01:57 +0600 Subject: [PATCH 22/22] Update canonical/x-go module to newer version As https://github.com/canonical/x-go/pull/19 is merged, update the canonical/x-go module version to the latest commit. Also run ``go mod tidy``. --- go.mod | 2 +- go.sum | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 7ea999110..9782aed6c 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.14 require ( github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8 - github.com/canonical/x-go v0.0.0-20230113154138-0ccdb0b57a43 + github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b github.com/gorilla/mux v1.8.0 github.com/gorilla/websocket v1.4.2 github.com/pkg/term v1.1.0 diff --git a/go.sum b/go.sum index 5ab5b5583..f335a74dc 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8 h1:zGaJEJI9qPVyM+QKFJagiyrM91Ke5S9htoL1D470g6E= github.com/canonical/go-flags v0.0.0-20230403090104-105d09a091b8/go.mod h1:ZZFeR9K9iGgpwOaLYF9PdT44/+lfSJ9sQz3B+SsGsYU= -github.com/canonical/x-go v0.0.0-20230113154138-0ccdb0b57a43 h1:bey1JgA3D2EBabr2a7kWKj+JlEPxX1akv8rcRromotA= -github.com/canonical/x-go v0.0.0-20230113154138-0ccdb0b57a43/go.mod h1:A0/Jvt7qKuCDss37TYRNXSscVyS+tLWM5kBYipQOpWQ= +github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b h1:Da2fardddn+JDlVEYtrzBLTtyzoyU3nIS0Cf0GvjmwU= +github.com/canonical/x-go v0.0.0-20230522092633-7947a7587f5b/go.mod h1:upTK9n6rlqITN9rCN69hdreI37dRDFUk2thlGGD5Cg8= github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI= github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So= github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= @@ -31,7 +31,7 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637 h1:yiW+nvdHb9LVqSHQBXfZCieqV4fzYhNBql77zY0ykqs= gopkg.in/tomb.v2 v2.0.0-20161208151619-d5d1b5820637/go.mod h1:BHsqpu/nsuzkT5BpiH1EMZPLyqSMM8JbIavyFACoFNk= -gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU= -gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= +gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=