o/hookstate/ctlcmd: introduce --user, --system and --users switches for snap service operations

client/clientutil/service_scope.go

@@ -0,0 +1,76 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2024 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * 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 clientutil
+
+import (
+	"fmt"
+
+	"github.com/snapcore/snapd/client"
+	"github.com/snapcore/snapd/strutil"
+)
+
+// ServiceScopeOptions represents shared options between service operations
+// that change the scope of services affected.
+type ServiceScopeOptions struct {
+	System    bool   `long:"system"`
+	User      bool   `long:"user"`
+	Usernames string `long:"users"`
+}
+
+func (us *ServiceScopeOptions) Validate() error {
+	switch {
+	case us.System && us.User:
+		return fmt.Errorf("--system and --user cannot be used in conjunction with each other")
+	case us.Usernames != "" && us.User:
+		return fmt.Errorf("--user and --users cannot be used in conjunction with each other")
+	case us.Usernames != "" && us.Usernames != "all":
+		return fmt.Errorf("only \"all\" is supported as a value for --users")
+	}
+	return nil
+}
+
+func (us *ServiceScopeOptions) Scope() client.ScopeSelector {
+	switch {
+	case (us.User || us.Usernames != "") && !us.System:
+		return client.ScopeSelector([]string{"user"})
+	case !(us.User || us.Usernames != "") && us.System:
+		return client.ScopeSelector([]string{"system"})
+	}
+	return nil
+}
+
+func (us *ServiceScopeOptions) Users() client.UserSelector {
+	switch {
+	case us.User:
+		return client.UserSelector{
+			Selector: client.UserSelectionSelf,
+		}
+	case us.Usernames == "all":
+		return client.UserSelector{
+			Selector: client.UserSelectionAll,
+		}
+	}
+	// Currently not reachable as us.Usernames can only be 'all' for now, but when
+	// we introduce support for lists of usernames, this will be hit.
+	return client.UserSelector{
+		Selector: client.UserSelectionList,
+		Names:    strutil.CommaSeparatedList(us.Usernames),
+	}
+}

client/clientutil/service_scope_test.go

@@ -0,0 +1,82 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2024 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * 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 clientutil_test
+
+import (
+	"github.com/snapcore/snapd/client"
+	"github.com/snapcore/snapd/client/clientutil"
+	. "gopkg.in/check.v1"
+)
+
+type serviceScopeSuite struct{}
+
+var _ = Suite(&serviceScopeSuite{})
+
+func (s *serviceScopeSuite) TestScopes(c *C) {
+	tests := []struct {
+		opts     clientutil.ServiceScopeOptions
+		expected client.ScopeSelector
+	}{
+		// when expected is nil it means both scopes
+		{clientutil.ServiceScopeOptions{}, nil},
+		{clientutil.ServiceScopeOptions{User: true}, client.ScopeSelector{"user"}},
+		{clientutil.ServiceScopeOptions{Usernames: "all"}, client.ScopeSelector{"user"}},
+		{clientutil.ServiceScopeOptions{System: true}, client.ScopeSelector{"system"}},
+		{clientutil.ServiceScopeOptions{User: true, System: true}, nil},
+		{clientutil.ServiceScopeOptions{Usernames: "all", System: true}, nil},
+	}
+
+	for _, t := range tests {
+		c.Check(t.opts.Scope(), DeepEquals, t.expected)
+	}
+}
+
+func (s *serviceScopeSuite) TestUsers(c *C) {
+	tests := []struct {
+		opts     clientutil.ServiceScopeOptions
+		expected client.UserSelector
+	}{
+		{clientutil.ServiceScopeOptions{}, client.UserSelector{Names: []string{}, Selector: client.UserSelectionList}},
+		{clientutil.ServiceScopeOptions{User: true}, client.UserSelector{Selector: client.UserSelectionSelf}},
+		{clientutil.ServiceScopeOptions{Usernames: "all"}, client.UserSelector{Selector: client.UserSelectionAll}},
+		{clientutil.ServiceScopeOptions{System: true}, client.UserSelector{Names: []string{}, Selector: client.UserSelectionList}},
+		{clientutil.ServiceScopeOptions{User: true, System: true}, client.UserSelector{Selector: client.UserSelectionSelf}},
+		{clientutil.ServiceScopeOptions{Usernames: "all", System: true}, client.UserSelector{Selector: client.UserSelectionAll}},
+	}
+
+	for _, t := range tests {
+		c.Check(t.opts.Users(), DeepEquals, t.expected)
+	}
+}
+
+func (s *serviceScopeSuite) TestInvalidOptions(c *C) {
+	tests := []struct {
+		opts     clientutil.ServiceScopeOptions
+		expected string
+	}{
+		{clientutil.ServiceScopeOptions{Usernames: "foo"}, `only "all" is supported as a value for --users`},
+		{clientutil.ServiceScopeOptions{User: true, System: true}, `--system and --user cannot be used in conjunction with each other`},
+		{clientutil.ServiceScopeOptions{Usernames: "all", User: true}, `--user and --users cannot be used in conjunction with each other`},
+	}
+
+	for _, t := range tests {
+		c.Check(t.opts.Validate(), ErrorMatches, t.expected)
+	}
+}

cmd/snap/cmd_services.go

@@ -29,7 +29,6 @@ import (
 	"github.com/snapcore/snapd/client/clientutil"
 	"github.com/snapcore/snapd/i18n"
 	"github.com/snapcore/snapd/snap"
-	"github.com/snapcore/snapd/strutil"
 )
 
 type svcStatus struct {
@@ -185,12 +184,6 @@ func (s *svcLogs) Execute(args []string) error {
 	return nil
 }
 
-type userAndScopeMixin struct {
-	System bool   `long:"system"`
-	User   bool   `long:"user"`
-	Users  string `long:"users"`
-}
-
 var userAndScopeDescs = mixinDescs{
 	// TRANSLATORS: This should not start with a lowercase letter.
 	"system": i18n.G("The operation should only affect system services."),
@@ -200,50 +193,9 @@ var userAndScopeDescs = mixinDescs{
 	"users": i18n.G("If provided and set to 'all', the operation should affect services for all users."),
 }
 
-func (um *userAndScopeMixin) validateScopes() error {
-	switch {
-	case um.System && um.User:
-		return fmt.Errorf("--system and --user cannot be used in conjunction with each other")
-	case um.Users != "" && um.User:
-		return fmt.Errorf("--user and --users cannot be used in conjunction with each other")
-	case um.Users != "" && um.Users != "all":
-		return fmt.Errorf("only \"all\" is supported as a value for --users")
-	}
-	return nil
-}
-
-func (um *userAndScopeMixin) serviceScope() client.ScopeSelector {
-	switch {
-	case (um.User || um.Users != "") && !um.System:
-		return client.ScopeSelector([]string{"user"})
-	case !(um.User || um.Users != "") && um.System:
-		return client.ScopeSelector([]string{"system"})
-	}
-	return nil
-}
-
-func (um *userAndScopeMixin) serviceUsers() client.UserSelector {
-	switch {
-	case um.User:
-		return client.UserSelector{
-			Selector: client.UserSelectionSelf,
-		}
-	case um.Users == "all":
-		return client.UserSelector{
-			Selector: client.UserSelectionAll,
-		}
-	}
-	// Currently not reachable as um.Users can only be 'all' for now, but when
-	// we introduce support for lists of usernames, this will be hit.
-	return client.UserSelector{
-		Selector: client.UserSelectionList,
-		Names:    strutil.CommaSeparatedList(um.Users),
-	}
-}
-
 type svcStart struct {
 	waitMixin
-	userAndScopeMixin
+	clientutil.ServiceScopeOptions
 	Positional struct {
 		ServiceNames []serviceName `required:"1"`
 	} `positional-args:"yes" required:"yes"`
@@ -254,11 +206,11 @@ func (s *svcStart) Execute(args []string) error {
 	if len(args) > 0 {
 		return ErrExtraArgs
 	}
-	if err := s.validateScopes(); err != nil {
+	if err := s.Validate(); err != nil {
 		return err
 	}
 	names := svcNames(s.Positional.ServiceNames)
-	changeID, err := s.client.Start(names, s.serviceScope(), s.serviceUsers(), client.StartOptions{Enable: s.Enable})
+	changeID, err := s.client.Start(names, s.Scope(), s.Users(), client.StartOptions{Enable: s.Enable})
 	if err != nil {
 		return err
 	}
@@ -276,7 +228,7 @@ func (s *svcStart) Execute(args []string) error {
 
 type svcStop struct {
 	waitMixin
-	userAndScopeMixin
+	clientutil.ServiceScopeOptions
 	Positional struct {
 		ServiceNames []serviceName `required:"1"`
 	} `positional-args:"yes" required:"yes"`
@@ -287,11 +239,11 @@ func (s *svcStop) Execute(args []string) error {
 	if len(args) > 0 {
 		return ErrExtraArgs
 	}
-	if err := s.validateScopes(); err != nil {
+	if err := s.Validate(); err != nil {
 		return err
 	}
 	names := svcNames(s.Positional.ServiceNames)
-	changeID, err := s.client.Stop(names, s.serviceScope(), s.serviceUsers(), client.StopOptions{Disable: s.Disable})
+	changeID, err := s.client.Stop(names, s.Scope(), s.Users(), client.StopOptions{Disable: s.Disable})
 	if err != nil {
 		return err
 	}
@@ -309,7 +261,7 @@ func (s *svcStop) Execute(args []string) error {
 
 type svcRestart struct {
 	waitMixin
-	userAndScopeMixin
+	clientutil.ServiceScopeOptions
 	Positional struct {
 		ServiceNames []serviceName `required:"1"`
 	} `positional-args:"yes" required:"yes"`
@@ -320,11 +272,11 @@ func (s *svcRestart) Execute(args []string) error {
 	if len(args) > 0 {
 		return ErrExtraArgs
 	}
-	if err := s.validateScopes(); err != nil {
+	if err := s.Validate(); err != nil {
 		return err
 	}
 	names := svcNames(s.Positional.ServiceNames)
-	changeID, err := s.client.Restart(names, s.serviceScope(), s.serviceUsers(), client.RestartOptions{Reload: s.Reload})
+	changeID, err := s.client.Restart(names, s.Scope(), s.Users(), client.RestartOptions{Reload: s.Reload})
 	if err != nil {
 		return err
 	}

overlord/hookstate/ctlcmd/restart.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2016-2017 Canonical Ltd
+ * Copyright (C) 2016-2024 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -21,6 +21,7 @@ package ctlcmd
 
 import (
 	"github.com/snapcore/snapd/client"
+	"github.com/snapcore/snapd/client/clientutil"
 	"github.com/snapcore/snapd/i18n"
 	"github.com/snapcore/snapd/overlord/servicestate"
 )
@@ -38,19 +39,26 @@ func init() {
 
 type restartCommand struct {
 	baseCommand
+	clientutil.ServiceScopeOptions
 	Positional struct {
 		ServiceNames []string `positional-arg-name:"<service>" required:"yes"`
 	} `positional-args:"yes" required:"yes"`
 	Reload bool `long:"reload" description:"Reload the given services if they support it (see man systemctl for details)"`
 }
 
 func (c *restartCommand) Execute(args []string) error {
+	if err := c.Validate(); err != nil {
+		return err
+	}
+
 	inst := servicestate.Instruction{
 		Action: "restart",
 		Names:  c.Positional.ServiceNames,
 		RestartOptions: client.RestartOptions{
 			Reload: c.Reload,
 		},
+		Scope: c.Scope(),
+		Users: c.Users(),
 	}
 	return runServiceCommand(c.context(), &inst)
 }