o/hookstate: add snapctl install and remove commands (canonical#14091)

* snap/info: fix comment for SnapInstancesAndComponentsFromNames and add tests

* cmd,snap: move functions to split snap plus components to snap package

* o/hookstate: add snapctl install and remove commands

* o/snapstate: check conflicts providing the change the tasks will belong to

as we should not have a conflict in that case.

* tests: add test to run snapctl install/remove for components

* cmd/snap: make sure that snaps are specified when using components

* snap: remove unused function

cmd/snap/cmd_snap_op.go

@@ -36,6 +36,7 @@ import (
 	"github.com/snapcore/snapd/i18n"
 	"github.com/snapcore/snapd/osutil"
 	"github.com/snapcore/snapd/release"
+	"github.com/snapcore/snapd/snap"
 	"github.com/snapcore/snapd/snap/channel"
 	"github.com/snapcore/snapd/strutil"
 )
@@ -198,7 +199,10 @@ func showRemovedComponents(expectedBySnap, removedBySnap map[string][]string) {
 func (x *cmdRemove) removeOne(opts *client.SnapOptions) error {
 	arg := string(x.Positional.Snaps[0])
 
-	name, comps := splitSnapAndComponents(arg)
+	name, comps := snap.SplitSnapInstanceAndComponents(arg)
+	if name == "" {
+		return errors.New(i18n.G("no snap for the component(s) was specified"))
+	}
 	// If there are components, only the components will be removed,
 	// otherwise the full snap with its components will be removed.
 	changeID, err := x.client.Remove(name, comps, opts)
@@ -237,11 +241,39 @@ func (x *cmdRemove) removeOne(opts *client.SnapOptions) error {
 	return nil
 }
 
+// snapInstancesAndComponentsFromNames splits a slice of names of the form
+// <snap_instance>+<comp1>...+<compN> into a slice of snap instances and a map
+// from these instances to components.
+func snapInstancesAndComponentsFromNames(names []string, forInstall bool) ([]string, map[string][]string, error) {
+	snaps := make([]string, 0, len(names))
+	allComps := make(map[string][]string, len(names))
+	for _, name := range names {
+		snap, comps := snap.SplitSnapInstanceAndComponents(name)
+		if snap == "" {
+			return nil, nil, errors.New(i18n.G("no snap for the component(s) was specified"))
+		}
+		// When installing we implicitly want to install the snap when
+		// we have specified also components, but when removing we
+		// actually want to remove only components if any of them have
+		// been specified.
+		if forInstall || len(comps) == 0 {
+			snaps = append(snaps, snap)
+		}
+		if len(comps) > 0 {
+			allComps[snap] = comps
+		}
+	}
+	return snaps, allComps, nil
+}
+
 func (x *cmdRemove) removeMany(opts *client.SnapOptions) error {
 	names := installedSnapNames(x.Positional.Snaps)
 
 	const forInstall = false
-	names, comps := snapsAndComponentsFromNames(names, forInstall)
+	names, comps, err := snapInstancesAndComponentsFromNames(names, forInstall)
+	if err != nil {
+		return err
+	}
 	changeID, err := x.client.RemoveMany(names, comps, opts)
 	if err != nil {
 		var name string
@@ -678,30 +710,6 @@ type cmdInstall struct {
 	} `positional-args:"yes" required:"yes"`
 }
 
-func splitSnapAndComponents(name string) (string, []string) {
-	parts := strings.Split(name, "+")
-	return parts[0], parts[1:]
-}
-
-func snapsAndComponentsFromNames(names []string, forInstall bool) ([]string, map[string][]string) {
-	snaps := make([]string, 0, len(names))
-	allComps := make(map[string][]string, len(names))
-	for _, name := range names {
-		snap, comps := splitSnapAndComponents(name)
-		// When installing we implicitly want to install the snap when
-		// we have specified also components, but when removing we
-		// actually want to remove only components if any of them have
-		// been specified.
-		if forInstall || len(comps) == 0 {
-			snaps = append(snaps, snap)
-		}
-		if len(comps) > 0 {
-			allComps[snap] = comps
-		}
-	}
-	return snaps, allComps
-}
-
 func (x *cmdInstall) installOne(nameOrPath, desiredName string, opts *client.SnapOptions) error {
 	var err error
 	var changeID string
@@ -719,7 +727,10 @@ func (x *cmdInstall) installOne(nameOrPath, desiredName string, opts *client.Sna
 			return errors.New(i18n.G("cannot use explicit name when installing from store"))
 		}
 
-		name, comps := splitSnapAndComponents(snapName)
+		name, comps := snap.SplitSnapInstanceAndComponents(snapName)
+		if name == "" {
+			return errors.New(i18n.G("no snap for the component(s) was specified"))
+		}
 
 		changeID, err = x.client.Install(name, comps, opts)
 	}
@@ -780,8 +791,11 @@ func (x *cmdInstall) installMany(names []string, opts *client.SnapOptions) error
 		}
 
 		const forInstall = true
-		names, comps := snapsAndComponentsFromNames(names, forInstall)
-		changeID, err = x.client.InstallMany(names, comps, opts)
+		names, compsBySnap, e := snapInstancesAndComponentsFromNames(names, forInstall)
+		if e != nil {
+			return e
+		}
+		changeID, err = x.client.InstallMany(names, compsBySnap, opts)
 	}
 
 	if err != nil {

cmd/snap/cmd_snap_op_test.go

@@ -3328,6 +3328,22 @@ func (s *SnapOpSuite) TestWaitServerError(c *check.C) {
 	}
 }
 
+func (s *SnapOpSuite) TestInstallCompsNoSnap(c *check.C) {
+
+	cmds := [][]string{
+		{"remove", "+comp1"},
+		{"remove", "+comp1", "+comp2"},
+		{"install", "+comp1"},
+		{"install", "+comp1", "+comp2"},
+	}
+
+	for _, cmd := range cmds {
+		_, err := snap.Parser(snap.Client()).ParseArgs(cmd)
+		c.Assert(err, check.ErrorMatches, `no snap for the component\(s\) was specified`,
+			check.Commentf("%v", cmd))
+	}
+}
+
 func (s *SnapOpSuite) TestSwitchHappy(c *check.C) {
 	s.srv.total = 4
 	s.srv.checker = func(r *http.Request) {
@@ -3508,3 +3524,44 @@ func (s *SnapOpSuite) TestWaitReportsInfoStatus(c *check.C) {
 	c.Check(meter.Notices, testutil.Contains, "INFO: Task set to wait until a manual system restart allows to continue")
 	c.Check(n, check.Equals, 2)
 }
+
+func (s *infoSuite) TestSnapInstancesAndComponentsFromNames(c *check.C) {
+	type testcase struct {
+		input       []string
+		instances   []string
+		instToComps map[string][]string
+		err         string
+	}
+
+	tests := []testcase{
+		{[]string{"snap"}, []string{"snap"}, map[string][]string{}, ""},
+		{[]string{"snap_instance"}, []string{"snap_instance"}, map[string][]string{}, ""},
+		{[]string{"snap+comp1+comp2"}, []string{"snap"},
+			map[string][]string{"snap": {"comp1", "comp2"}}, ""},
+		{[]string{"snap1+comp1+comp2", "snap2+comp3+comp4"}, []string{"snap1", "snap2"},
+			map[string][]string{"snap1": {"comp1", "comp2"}, "snap2": {"comp3", "comp4"}},
+			""},
+		{[]string{""}, []string{""}, map[string][]string{},
+			"no snap for the component(s) was specified"},
+		{[]string{"+comp1"}, []string{""}, map[string][]string{"": {"comp1"}},
+			"no snap for the component(s) was specified"},
+		{[]string{"+comp1+comp2"}, []string{""}, map[string][]string{"": {"comp1", "comp2"}},
+			"no snap for the component(s) was specified"},
+	}
+
+	for _, t := range tests {
+		const forInstall = true
+		c.Log("input", t.input)
+		instances, component, err :=
+			snap.SnapInstancesAndComponentsFromNames(t.input, forInstall)
+		if t.err == "" {
+			c.Check(err, check.IsNil)
+			c.Check(instances, check.DeepEquals, t.instances)
+			c.Check(component, check.DeepEquals, t.instToComps)
+		} else {
+			c.Check(err.Error(), check.Equals, t.err)
+			c.Check(instances, check.IsNil)
+			c.Check(component, check.IsNil)
+		}
+	}
+}

cmd/snap/export_test.go

@@ -97,7 +97,8 @@ var (
 
 	IsStopping = isStopping
 
-	GetSnapDirOptions = getSnapDirOptions
+	GetSnapDirOptions                   = getSnapDirOptions
+	SnapInstancesAndComponentsFromNames = snapInstancesAndComponentsFromNames
 )
 
 func HiddenCmd(descr string, completeHidden bool) *cmdInfo {

overlord/hookstate/ctlcmd/export_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/snapcore/snapd/overlord/devicestate"
 	"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"
 	"github.com/snapcore/snapd/testutil"
@@ -72,6 +73,18 @@ func MockServicestateControlFunc(f func(*state.State, []*snap.AppInfo, *services
 	return func() { servicestateControl = old }
 }
 
+func MockSnapstateInstallComponentsFunc(f func(ctx context.Context, st *state.State, names []string, info *snap.Info, opts snapstate.Options) ([]*state.TaskSet, error)) (restore func()) {
+	old := snapstateInstallComponents
+	snapstateInstallComponents = f
+	return func() { snapstateInstallComponents = old }
+}
+
+func MockSnapstateRemoveComponentsFunc(f func(st *state.State, snapName string, compName []string, opts snapstate.RemoveComponentsOpts) ([]*state.TaskSet, error)) (restore func()) {
+	old := snapstateRemoveComponents
+	snapstateRemoveComponents = f
+	return func() { snapstateRemoveComponents = old }
+}
+
 func MockDevicestateSystemModeInfoFromState(f func(*state.State) (*devicestate.SystemModeInfo, error)) (restore func()) {
 	old := devicestateSystemModeInfoFromState
 	devicestateSystemModeInfoFromState = f