hooks: commands for controlling own services from snapctl

daemon/api.go

@@ -51,12 +51,12 @@ import (
 	"github.com/snapcore/snapd/osutil"
 	"github.com/snapcore/snapd/overlord/assertstate"
 	"github.com/snapcore/snapd/overlord/auth"
-	"github.com/snapcore/snapd/overlord/cmdstate"
 	"github.com/snapcore/snapd/overlord/configstate"
 	"github.com/snapcore/snapd/overlord/configstate/config"
 	"github.com/snapcore/snapd/overlord/devicestate"
 	"github.com/snapcore/snapd/overlord/hookstate/ctlcmd"
 	"github.com/snapcore/snapd/overlord/ifacestate"
+	"github.com/snapcore/snapd/overlord/servicestate"
 	"github.com/snapcore/snapd/overlord/snapstate"
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/overlord/storestate"
@@ -2604,16 +2604,8 @@ func getLogs(c *Command, r *http.Request, user *auth.UserState) Response {
 	}
 }
 
-type appInstruction struct {
-	Action string   `json:"action"`
-	Names  []string `json:"names"`
-	client.StartOptions
-	client.StopOptions
-	client.RestartOptions
-}
-
 func postApps(c *Command, r *http.Request, user *auth.UserState) Response {
-	var inst appInstruction
+	var inst servicestate.Instruction
 	decoder := json.NewDecoder(r.Body)
 	if err := decoder.Decode(&inst); err != nil {
 		return BadRequest("cannot decode request body into service operation: %v", err)
@@ -2634,56 +2626,13 @@ func postApps(c *Command, r *http.Request, user *auth.UserState) Response {
 		return InternalError("no services found")
 	}
 
-	// the argv to call systemctl will need at most one entry per appInfo,
-	// plus one for "systemctl", one for the action, and sometimes one for
-	// an option. That's a maximum of 3+len(appInfos).
-	argv := make([]string, 2, 3+len(appInfos))
-	argv[0] = "systemctl"
-
-	argv[1] = inst.Action
-	switch inst.Action {
-	case "start":
-		if inst.Enable {
-			argv[1] = "enable"
-			argv = append(argv, "--now")
-		}
-	case "stop":
-		if inst.Disable {
-			argv[1] = "disable"
-			argv = append(argv, "--now")
-		}
-	case "restart":
-		if inst.Reload {
-			argv[1] = "reload-or-restart"
-		}
-	default:
-		return BadRequest("unknown action %q", inst.Action)
-	}
-
-	snapNames := make([]string, 0, len(appInfos))
-	lastName := ""
-	names := make([]string, len(appInfos))
-	for i, svc := range appInfos {
-		argv = append(argv, svc.ServiceName())
-		snapName := svc.Snap.Name()
-		names[i] = snapName + "." + svc.Name
-		if snapName != lastName {
-			snapNames = append(snapNames, snapName)
-			lastName = snapName
+	chg, err := servicestate.Change(st, appInfos, &inst)
+	if err != nil {
+		if _, ok := err.(servicestate.ServiceActionConflictError); ok {
+			return Conflict(err.Error())
 		}
+		return BadRequest(err.Error())
 	}
-
-	desc := fmt.Sprintf("%s of %v", inst.Action, names)
-
-	st.Lock()
-	defer st.Unlock()
-	if err := snapstate.CheckChangeConflictMany(st, snapNames, nil); err != nil {
-		return InternalError(err.Error())
-	}
-
-	ts := cmdstate.Exec(st, desc, argv)
-	chg := st.NewChange("service-control", desc)
-	chg.AddAll(ts)
 	st.EnsureBefore(0)
 	return AsyncResponse(nil, &Meta{Change: chg.ID()})
 }

daemon/api_test.go

@@ -61,6 +61,7 @@ import (
 	"github.com/snapcore/snapd/overlord/auth"
 	"github.com/snapcore/snapd/overlord/configstate/config"
 	"github.com/snapcore/snapd/overlord/ifacestate"
+	"github.com/snapcore/snapd/overlord/servicestate"
 	"github.com/snapcore/snapd/overlord/snapstate"
 	"github.com/snapcore/snapd/overlord/state"
 	"github.com/snapcore/snapd/overlord/storestate"
@@ -6104,7 +6105,7 @@ func (s *appSuite) TestLogsSad(c *check.C) {
 	c.Assert(rsp.Type, check.Equals, ResponseTypeError)
 }
 
-func (s *appSuite) testPostApps(c *check.C, inst appInstruction, systemctlCall []string) *state.Change {
+func (s *appSuite) testPostApps(c *check.C, inst servicestate.Instruction, systemctlCall []string) *state.Change {
 	postBody, err := json.Marshal(inst)
 	c.Assert(err, check.IsNil)
 
@@ -6132,55 +6133,55 @@ func (s *appSuite) testPostApps(c *check.C, inst appInstruction, systemctlCall [
 }
 
 func (s *appSuite) TestPostAppsStartOne(c *check.C) {
-	inst := appInstruction{Action: "start", Names: []string{"snap-a.svc2"}}
+	inst := servicestate.Instruction{Action: "start", Names: []string{"snap-a.svc2"}}
 	expected := []string{"systemctl", "start", "snap.snap-a.svc2.service"}
 	s.testPostApps(c, inst, expected)
 }
 
 func (s *appSuite) TestPostAppsStartTwo(c *check.C) {
-	inst := appInstruction{Action: "start", Names: []string{"snap-a"}}
+	inst := servicestate.Instruction{Action: "start", Names: []string{"snap-a"}}
 	expected := []string{"systemctl", "start", "snap.snap-a.svc1.service", "snap.snap-a.svc2.service"}
 	chg := s.testPostApps(c, inst, expected)
 	// check the summary expands the snap into actual apps
 	c.Check(chg.Summary(), check.Equals, "start of [snap-a.svc1 snap-a.svc2]")
 }
 
 func (s *appSuite) TestPostAppsStartThree(c *check.C) {
-	inst := appInstruction{Action: "start", Names: []string{"snap-a", "snap-b"}}
+	inst := servicestate.Instruction{Action: "start", Names: []string{"snap-a", "snap-b"}}
 	expected := []string{"systemctl", "start", "snap.snap-a.svc1.service", "snap.snap-a.svc2.service", "snap.snap-b.svc3.service"}
 	chg := s.testPostApps(c, inst, expected)
 	// check the summary expands the snap into actual apps
 	c.Check(chg.Summary(), check.Equals, "start of [snap-a.svc1 snap-a.svc2 snap-b.svc3]")
 }
 
 func (s *appSuite) TestPosetAppsStop(c *check.C) {
-	inst := appInstruction{Action: "stop", Names: []string{"snap-a.svc2"}}
+	inst := servicestate.Instruction{Action: "stop", Names: []string{"snap-a.svc2"}}
 	expected := []string{"systemctl", "stop", "snap.snap-a.svc2.service"}
 	s.testPostApps(c, inst, expected)
 }
 
 func (s *appSuite) TestPosetAppsRestart(c *check.C) {
-	inst := appInstruction{Action: "restart", Names: []string{"snap-a.svc2"}}
+	inst := servicestate.Instruction{Action: "restart", Names: []string{"snap-a.svc2"}}
 	expected := []string{"systemctl", "restart", "snap.snap-a.svc2.service"}
 	s.testPostApps(c, inst, expected)
 }
 
 func (s *appSuite) TestPosetAppsReload(c *check.C) {
-	inst := appInstruction{Action: "restart", Names: []string{"snap-a.svc2"}}
+	inst := servicestate.Instruction{Action: "restart", Names: []string{"snap-a.svc2"}}
 	inst.Reload = true
 	expected := []string{"systemctl", "reload-or-restart", "snap.snap-a.svc2.service"}
 	s.testPostApps(c, inst, expected)
 }
 
 func (s *appSuite) TestPosetAppsEnableNow(c *check.C) {
-	inst := appInstruction{Action: "start", Names: []string{"snap-a.svc2"}}
+	inst := servicestate.Instruction{Action: "start", Names: []string{"snap-a.svc2"}}
 	inst.Enable = true
 	expected := []string{"systemctl", "enable", "--now", "snap.snap-a.svc2.service"}
 	s.testPostApps(c, inst, expected)
 }
 
 func (s *appSuite) TestPosetAppsDisableNow(c *check.C) {
-	inst := appInstruction{Action: "stop", Names: []string{"snap-a.svc2"}}
+	inst := servicestate.Instruction{Action: "stop", Names: []string{"snap-a.svc2"}}
 	inst.Disable = true
 	expected := []string{"systemctl", "disable", "--now", "snap.snap-a.svc2.service"}
 	s.testPostApps(c, inst, expected)
@@ -6251,7 +6252,7 @@ func (s *appSuite) TestPostAppsConflict(c *check.C) {
 	req, err := http.NewRequest("POST", "/v2/apps", bytes.NewBufferString(`{"action": "start", "names": ["snap-a.svc1"]}`))
 	c.Assert(err, check.IsNil)
 	rsp := postApps(appsCmd, req, nil).(*resp)
-	c.Check(rsp.Status, check.Equals, 500)
+	c.Check(rsp.Status, check.Equals, 400)
 	c.Check(rsp.Type, check.Equals, ResponseTypeError)
 	c.Check(rsp.Result.(*errorResult).Message, check.Equals, `snap "snap-a" has changes in progress`)
 }

overlord/configstate/configstate.go

@@ -36,7 +36,7 @@ func init() {
 	snapstate.Configure = Configure
 }
 
-func configureHookTimeout() time.Duration {
+func ConfigureHookTimeout() time.Duration {
 	timeout := 5 * time.Minute
 	if s := os.Getenv("SNAPD_CONFIGURE_HOOK_TIMEOUT"); s != "" {
 		if to, err := time.ParseDuration(s); err == nil {
@@ -55,7 +55,7 @@ func Configure(s *state.State, snapName string, patch map[string]interface{}, fl
 		IgnoreError: flags&snapstate.IgnoreHookError != 0,
 		TrackError:  flags&snapstate.TrackHookError != 0,
 		// all configure hooks must finish within this timeout
-		Timeout: configureHookTimeout(),
+		Timeout: ConfigureHookTimeout(),
 	}
 	var contextData map[string]interface{}
 	if flags&snapstate.UseConfigDefaults != 0 {

overlord/hookstate/ctlcmd/export_test.go

@@ -19,11 +19,22 @@
 
 package ctlcmd
 
-import "fmt"
+import (
+	"fmt"
+	"github.com/snapcore/snapd/overlord/servicestate"
+	"github.com/snapcore/snapd/overlord/state"
+	"github.com/snapcore/snapd/snap"
+)
 
 var AttributesTask = attributesTask
 var CopyAttributes = copyAttributes
 
+func MockServiceChangeFunc(f func(*state.State, []*snap.AppInfo, *servicestate.Instruction) (*state.Change, error)) (restore func()) {
+	old := servicechangeImpl
+	servicechangeImpl = f
+	return func() { servicechangeImpl = old }
+}
+
 func AddMockCommand(name string) *MockCommand {
 	mockCommand := NewMockCommand()
 	addCommand(name, "", "", func() command { return mockCommand })

overlord/hookstate/ctlcmd/helpers.go

@@ -0,0 +1,100 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package ctlcmd
+
+import (
+	"fmt"
+	"strings"
+	"time"
+
+	"github.com/snapcore/snapd/i18n"
+	"github.com/snapcore/snapd/overlord/configstate"
+	"github.com/snapcore/snapd/overlord/hookstate"
+	"github.com/snapcore/snapd/overlord/servicestate"
+	"github.com/snapcore/snapd/overlord/snapstate"
+	"github.com/snapcore/snapd/overlord/state"
+	"github.com/snapcore/snapd/snap"
+)
+
+func getServiceInfos(st *state.State, snapName string, serviceNames []string) ([]*snap.AppInfo, error) {
+	st.Lock()
+	defer st.Unlock()
+
+	var snapst snapstate.SnapState
+	if err := snapstate.Get(st, snapName, &snapst); err != nil {
+		return nil, err
+	}
+
+	info, err := snapst.CurrentInfo()
+	if err != nil {
+		return nil, err
+	}
+
+	var svcs []*snap.AppInfo
+	for _, svcName := range serviceNames {
+		if svcName == snapName {
+			// all the services
+			return info.Services(), nil
+		}
+		if !strings.HasPrefix(svcName, snapName+".") {
+			return nil, fmt.Errorf(i18n.G("unknown service: %q"), svcName)
+		}
+		// this doesn't support service aliases
+		app, ok := info.Apps[svcName[1+len(snapName):]]
+		if !(ok && app.IsService()) {
+			return nil, fmt.Errorf(i18n.G("unknown service: %q"), svcName)
+		}
+		svcs = append(svcs, app)
+	}
+
+	return svcs, nil
+}
+
+var servicechangeImpl = servicestate.Change
+
+func runServiceCommand(context *hookstate.Context, inst *servicestate.Instruction, serviceNames []string) error {
+	if context == nil {
+		return fmt.Errorf(i18n.G("cannot %s without a context"), inst.Action)
+	}
+
+	st := context.State()
+	appInfos, err := getServiceInfos(st, context.SnapName(), serviceNames)
+	if err != nil {
+		return err
+	}
+
+	chg, err := servicechangeImpl(st, appInfos, inst)
+	if err != nil {
+		return err
+	}
+
+	st.Lock()
+	st.EnsureBefore(0)
+	st.Unlock()
+
+	select {
+	case <-chg.Ready():
+		st.Lock()
+		defer st.Unlock()
+		return chg.Err()
+	case <-time.After(configstate.ConfigureHookTimeout() / 2):
+		return fmt.Errorf("%s command is taking too long", inst.Action)
+	}
+}

overlord/hookstate/ctlcmd/restart.go

@@ -0,0 +1,53 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2016-2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package ctlcmd
+
+import (
+	"github.com/snapcore/snapd/client"
+	"github.com/snapcore/snapd/i18n"
+	"github.com/snapcore/snapd/overlord/servicestate"
+)
+
+var (
+	shortRestartHelp = i18n.G("Restart services")
+)
+
+func init() {
+	addCommand("restart", shortRestartHelp, "", func() command { return &restartCommand{} })
+}
+
+type restartCommand struct {
+	baseCommand
+	Positional struct {
+		ServiceNames []string `positional-arg-name:"<service>" required:"yes"`
+	} `positional-args:"yes" required:"yes"`
+	Reload bool `long:"reload"`
+}
+
+func (c *restartCommand) Execute(args []string) error {
+	inst := servicestate.Instruction{
+		Action: "restart",
+		Names:  c.Positional.ServiceNames,
+		RestartOptions: client.RestartOptions{
+			Reload: c.Reload,
+		},
+	}
+	return runServiceCommand(c.context(), &inst, c.Positional.ServiceNames)
+}