diff --git a/cmd/snap/cmd_disconnect.go b/cmd/snap/cmd_disconnect.go index 3c40ccbe433..ed01d42c5fa 100644 --- a/cmd/snap/cmd_disconnect.go +++ b/cmd/snap/cmd_disconnect.go @@ -41,13 +41,10 @@ $ snap disconnect : : Disconnects the specific plug from the specific slot. -$ snap disconnect : +$ snap disconnect : -Disconnects any previously connected plugs from the provided slot. - -$ snap disconnect - -Disconnects all plugs from the provided snap. +Disconnects everything from the provided plug or slot. +The snap name may be omitted for the OS snap. `) func init() { diff --git a/cmd/snap/cmd_disconnect_test.go b/cmd/snap/cmd_disconnect_test.go index f067a6e3c4f..073320c8c45 100644 --- a/cmd/snap/cmd_disconnect_test.go +++ b/cmd/snap/cmd_disconnect_test.go @@ -39,13 +39,10 @@ $ snap disconnect : : Disconnects the specific plug from the specific slot. -$ snap disconnect : +$ snap disconnect : -Disconnects any previously connected plugs from the provided slot. - -$ snap disconnect - -Disconnects all plugs from the provided snap. +Disconnects everything from the provided plug or slot. +The snap name may be omitted for the OS snap. Application Options: --version Print the version and exit diff --git a/cmd/snap/interfaces_common.go b/cmd/snap/interfaces_common.go index fa819d44746..9f292d7416e 100644 --- a/cmd/snap/interfaces_common.go +++ b/cmd/snap/interfaces_common.go @@ -73,10 +73,6 @@ func (sn *SnapAndName) UnmarshalFlag(value string) error { case 2: sn.Snap = parts[0] sn.Name = parts[1] - // Reject ":name" (that is invalid) - if sn.Snap == "" { - sn.Name = "" - } // Reject "snap:" (that should be spelled as "snap") if sn.Name == "" { sn.Snap = "" diff --git a/cmd/snap/interfaces_common_test.go b/cmd/snap/interfaces_common_test.go index af0e8fec2ac..3b46ab9b7ee 100644 --- a/cmd/snap/interfaces_common_test.go +++ b/cmd/snap/interfaces_common_test.go @@ -93,7 +93,6 @@ func (s *SnapAndNameSuite) TestUnmarshalFlag(c *C) { c.Check(sn.Name, Equals, "") // Invalid for _, input := range []string{ - ":name", // Empty snap, makes no sense "snap:", // Empty name, should be spelled as "snap" ":", // Both snap and name empty, makes no sense "snap:name:more", // Name containing :, probably a typo diff --git a/daemon/api_test.go b/daemon/api_test.go index ed89a5fa61f..41374c72625 100644 --- a/daemon/api_test.go +++ b/daemon/api_test.go @@ -2988,7 +2988,7 @@ func (s *apiSuite) TestDisconnectPlugFailureNoSuchPlug(c *check.C) { st.Unlock() c.Assert(err, check.NotNil) c.Check(err.Error(), check.Equals, `cannot perform the following tasks: -- Disconnect consumer:plug from producer:slot (cannot disconnect plug "plug" from snap "consumer", no such plug)`) +- Disconnect consumer:plug from producer:slot (snap "consumer" has no plug named "plug")`) repo := d.overlord.InterfaceManager().Repository() slot := repo.Slot("producer", "slot") @@ -3036,7 +3036,7 @@ func (s *apiSuite) TestDisconnectPlugFailureNoSuchSlot(c *check.C) { st.Unlock() c.Assert(err, check.NotNil) c.Check(err.Error(), check.Equals, `cannot perform the following tasks: -- Disconnect consumer:plug from producer:slot (cannot disconnect plug from slot "slot" from snap "producer", no such slot)`) +- Disconnect consumer:plug from producer:slot (snap "producer" has no slot named "slot")`) repo := d.overlord.InterfaceManager().Repository() plug := repo.Plug("consumer", "plug") @@ -3084,7 +3084,7 @@ func (s *apiSuite) TestDisconnectPlugFailureNotConnected(c *check.C) { st.Unlock() c.Assert(err, check.NotNil) c.Check(err.Error(), check.Equals, `cannot perform the following tasks: -- Disconnect consumer:plug from producer:slot (cannot disconnect plug "plug" from snap "consumer" from slot "slot" from snap "producer", it is not connected)`) +- Disconnect consumer:plug from producer:slot (cannot disconnect consumer:plug from producer:slot, it is not connected)`) repo := d.overlord.InterfaceManager().Repository() plug := repo.Plug("consumer", "plug") diff --git a/interfaces/repo.go b/interfaces/repo.go index 3c6f348cf35..6060dca81e6 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -379,85 +379,101 @@ func (r *Repository) Connect(ref ConnRef) error { // Disconnect disconnects the named plug from the slot of the given snap. // -// Disconnect has three modes of operation that depend on the passed arguments: -// -// - If all the arguments are specified then Disconnect() finds a specific slot -// and a specific plug and disconnects that plug from that slot. It is -// an error if plug or slot cannot be found or if the connect does not -// exist. -// - If plugSnapName and plugName are empty then Disconnect() finds the specified -// slot and disconnects all the plugs connected there. It is not an error if -// there are no such plugs but it is still an error if the slot does -// not exist. -// - If plugSnapName, plugName and slotName are all empty then Disconnect finds -// the specified snap (designated by slotSnapName) and disconnects all the plugs -// from all the slots found therein. It is not an error if there are no -// such plugs but it is still an error if the snap does not exist or has no -// slots at all. +// Disconnect() finds a specific slot and a specific plug and disconnects that +// plug from that slot. It is an error if plug or slot cannot be found or if +// the connect does not exist. func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName string) error { r.m.Lock() defer r.m.Unlock() - switch { - case plugSnapName == "" && plugName == "" && slotName == "": - // Disconnect everything from slotSnapName - return r.disconnectEverythingFromSnap(slotSnapName) - case plugSnapName == "" && plugName == "": - // Disconnect everything from slotSnapName:slotName - return r.disconnectEverythingFromSlot(slotSnapName, slotName) - default: - return r.disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName) - } - -} - -// disconnectEverythingFromSnap finds a specific snap and disconnects all the plugs connected to all the slots therein. -func (r *Repository) disconnectEverythingFromSnap(slotSnapName string) error { - if _, ok := r.slots[slotSnapName]; !ok { - return fmt.Errorf("cannot disconnect plug from snap %q, no such snap", slotSnapName) + // Sanity check + if plugSnapName == "" { + return fmt.Errorf("cannot disconnect, plug snap name is empty") } - for _, slot := range r.slots[slotSnapName] { - for plug := range r.slotPlugs[slot] { - r.disconnect(plug, slot) - } + if plugName == "" { + return fmt.Errorf("cannot disconnect, plug name is empty") } - return nil -} - -// disconnectEverythingFromSlot finds a specific slot and disconnects all the plugs connected there. -func (r *Repository) disconnectEverythingFromSlot(slotSnapName, slotName string) error { - // Ensure that such slot exists - slot := r.slots[slotSnapName][slotName] - if slot == nil { - return fmt.Errorf("cannot disconnect plug from slot %q from snap %q, no such slot", slotName, slotSnapName) + if slotSnapName == "" { + return fmt.Errorf("cannot disconnect, slot snap name is empty") } - for plug := range r.slotPlugs[slot] { - r.disconnect(plug, slot) + if slotName == "" { + return fmt.Errorf("cannot disconnect, slot name is empty") } - return nil -} -// disconnectPlugFromSlot finds a specific slot and plug and disconnects it. -func (r *Repository) disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName string) error { // Ensure that such plug exists plug := r.plugs[plugSnapName][plugName] if plug == nil { - return fmt.Errorf("cannot disconnect plug %q from snap %q, no such plug", plugName, plugSnapName) + return fmt.Errorf("snap %q has no plug named %q", plugSnapName, plugName) } // Ensure that such slot exists slot := r.slots[slotSnapName][slotName] if slot == nil { - return fmt.Errorf("cannot disconnect plug from slot %q from snap %q, no such slot", slotName, slotSnapName) + return fmt.Errorf("snap %q has no slot named %q", slotSnapName, slotName) } // Ensure that slot and plug are connected if !r.slotPlugs[slot][plug] { - return fmt.Errorf("cannot disconnect plug %q from snap %q from slot %q from snap %q, it is not connected", - plugName, plugSnapName, slotName, slotSnapName) + return fmt.Errorf("cannot disconnect %s:%s from %s:%s, it is not connected", + plugSnapName, plugName, slotSnapName, slotName) } r.disconnect(plug, slot) return nil } +// Connected returns references for all connections that are currently +// established with the provided plug or slot. +func (r *Repository) Connected(snapName, plugOrSlotName string) ([]ConnRef, error) { + r.m.Lock() + defer r.m.Unlock() + + if snapName == "" { + // Look up the core snap if no snap name was given + switch { + case r.slots["core"] != nil: + snapName = "core" + case r.slots["ubuntu-core"] != nil: + snapName = "ubuntu-core" + default: + return nil, fmt.Errorf("cannot resolve disconnect, snap name is empty") + } + } + var conns []ConnRef + if plugOrSlotName == "" { + return nil, fmt.Errorf("cannot resolve disconnect, plug or slot name is empty") + } + // Check if plugOrSlotName actually maps to anything + if r.plugs[snapName][plugOrSlotName] == nil && r.slots[snapName][plugOrSlotName] == nil { + return nil, fmt.Errorf("snap %q has no plug or slot named %q", snapName, plugOrSlotName) + } + // Collect all the relevant connections + if plug, ok := r.plugs[snapName][plugOrSlotName]; ok { + for _, slotRef := range plug.Connections { + connRef := ConnRef{PlugRef: plug.Ref(), SlotRef: slotRef} + conns = append(conns, connRef) + } + } + if slot, ok := r.slots[snapName][plugOrSlotName]; ok { + for _, plugRef := range slot.Connections { + connRef := ConnRef{PlugRef: plugRef, SlotRef: slot.Ref()} + conns = append(conns, connRef) + } + } + return conns, nil +} + +// DisconnectAll disconnects all provided connection references. +func (r *Repository) DisconnectAll(conns []ConnRef) { + r.m.Lock() + defer r.m.Unlock() + + for _, conn := range conns { + plug := r.plugs[conn.PlugRef.Snap][conn.PlugRef.Name] + slot := r.slots[conn.SlotRef.Snap][conn.SlotRef.Name] + if plug != nil && slot != nil { + r.disconnect(plug, slot) + } + } +} + // disconnect disconnects a plug from a slot. func (r *Repository) disconnect(plug *Plug, slot *Slot) { delete(r.slotPlugs[slot], plug) diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index bf474a0c8c7..c1f20128bf9 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -768,115 +768,125 @@ func (s *RepositorySuite) TestConnectSucceeds(c *C) { c.Assert(err, IsNil) } -// Tests for Repository.Disconnect() - -func (s *RepositorySuite) TestDisconnectFailsWhenPlugDoesNotExist(c *C) { - err := s.testRepo.AddSlot(s.slot) - c.Assert(err, IsNil) - // Disconnecting an unknown plug returns and appropriate error - err = s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, ErrorMatches, `cannot disconnect plug "plug" from snap "consumer", no such plug`) +// Tests for Repository.Disconnect() and DisconnectAll() + +// Disconnect fails if any argument is empty +func (s *RepositorySuite) TestDisconnectFailsOnEmptyArgs(c *C) { + err1 := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), "") + err2 := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, "", s.slot.Name) + err3 := s.testRepo.Disconnect(s.plug.Snap.Name(), "", s.slot.Snap.Name(), s.slot.Name) + err4 := s.testRepo.Disconnect("", s.plug.Name, s.slot.Snap.Name(), s.slot.Name) + c.Assert(err1, ErrorMatches, `cannot disconnect, slot name is empty`) + c.Assert(err2, ErrorMatches, `cannot disconnect, slot snap name is empty`) + c.Assert(err3, ErrorMatches, `cannot disconnect, plug name is empty`) + c.Assert(err4, ErrorMatches, `cannot disconnect, plug snap name is empty`) +} + +// Disconnect fails if plug doesn't exist +func (s *RepositorySuite) TestDisconnectFailsWithoutPlug(c *C) { + c.Assert(s.testRepo.AddSlot(s.slot), IsNil) + err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) + c.Assert(err, ErrorMatches, `snap "consumer" has no plug named "plug"`) } -func (s *RepositorySuite) TestDisconnectFailsWhenSlotDoesNotExist(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - // Disconnecting from an unknown slot returns an appropriate error - err = s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, ErrorMatches, `cannot disconnect plug from slot "slot" from snap "producer", no such slot`) +// Disconnect fails if slot doesn't exist +func (s *RepositorySuite) TestDisconnectFailsWithutSlot(c *C) { + c.Assert(s.testRepo.AddPlug(s.plug), IsNil) + err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) + c.Assert(err, ErrorMatches, `snap "producer" has no slot named "slot"`) } -func (s *RepositorySuite) TestDisconnectFromSlotFailsWhenSlotDoesNotExist(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - // Disconnecting everything form an unknown slot returns an appropriate error - err = s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, ErrorMatches, `cannot disconnect plug from slot "slot" from snap "producer", no such slot`) +// Disconnect fails if there's no connection to disconnect +func (s *RepositorySuite) TestDisconnectFailsWhenNotConnected(c *C) { + c.Assert(s.testRepo.AddPlug(s.plug), IsNil) + c.Assert(s.testRepo.AddSlot(s.slot), IsNil) + err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) + c.Assert(err, ErrorMatches, `cannot disconnect consumer:plug from producer:slot, it is not connected`) } -func (s *RepositorySuite) TestDisconnectFromSnapFailsWhenSlotDoesNotExist(c *C) { - err := s.testRepo.AddPlug(s.plug) +// Disconnect works when plug and slot exist and are connected +func (s *RepositorySuite) TestDisconnectSucceeds(c *C) { + c.Assert(s.testRepo.AddPlug(s.plug), IsNil) + c.Assert(s.testRepo.AddSlot(s.slot), IsNil) + c.Assert(s.testRepo.Connect(ConnRef{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}), IsNil) + err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) - // Disconnecting all plugs from a snap that is not known returns an appropriate error - err = s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") - c.Assert(err, ErrorMatches, `cannot disconnect plug from snap "producer", no such snap`) + c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ + Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, + Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, + }) } -func (s *RepositorySuite) TestDisconnectFailsWhenNotConnected(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - err = s.testRepo.AddSlot(s.slot) - c.Assert(err, IsNil) - // Disconnecting a plug that is not connected returns an appropriate error - err = s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, ErrorMatches, `cannot disconnect plug "plug" from snap "consumer" from slot "slot" from snap "producer", it is not connected`) +// Tests for Repository.Connected + +// Connected fails if snap name is empty and there's no core snap around +func (s *RepositorySuite) TestConnectedFailsWithEmptySnapName(c *C) { + _, err := s.testRepo.Connected("", s.plug.Name) + c.Check(err, ErrorMatches, `cannot resolve disconnect, snap name is empty`) } -func (s *RepositorySuite) TestDisconnectFromSnapDoesNothingWhenNotConnected(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - err = s.testRepo.AddSlot(s.slot) - c.Assert(err, IsNil) - // Disconnecting a all plugs from a snap that uses nothing is not an error. - err = s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") - c.Assert(err, IsNil) +// Connected fails if plug or slot name is empty +func (s *RepositorySuite) TestConnectedFailsWithEmptyPlugSlotName(c *C) { + _, err := s.testRepo.Connected(s.plug.Snap.Name(), "") + c.Check(err, ErrorMatches, `cannot resolve disconnect, plug or slot name is empty`) } -func (s *RepositorySuite) TestDisconnectFromSlotDoesNothingWhenNotConnected(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - err = s.testRepo.AddSlot(s.slot) - c.Assert(err, IsNil) - // Disconnecting a all plugs from a slot that uses nothing is not an error. - err = s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, IsNil) +// Connected fails if plug or slot doesn't exist +func (s *RepositorySuite) TestConnectedFailsWithoutPlugOrSlot(c *C) { + _, err1 := s.testRepo.Connected(s.plug.Snap.Name(), s.plug.Name) + _, err2 := s.testRepo.Connected(s.slot.Snap.Name(), s.slot.Name) + c.Check(err1, ErrorMatches, `snap "consumer" has no plug or slot named "plug"`) + c.Check(err2, ErrorMatches, `snap "producer" has no plug or slot named "slot"`) } -func (s *RepositorySuite) TestDisconnectSucceeds(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - err = s.testRepo.AddSlot(s.slot) - c.Assert(err, IsNil) - connRef := ConnRef{PlugRef: PlugRef{Snap: s.plug.Snap.Name(), Name: s.plug.Name}, SlotRef: SlotRef{Snap: s.slot.Snap.Name(), Name: s.slot.Name}} - err = s.testRepo.Connect(connRef) +// Connected finds connections when asked from plug or from slot side +func (s *RepositorySuite) TestConnectedFindsConnections(c *C) { + c.Assert(s.testRepo.AddPlug(s.plug), IsNil) + c.Assert(s.testRepo.AddSlot(s.slot), IsNil) + c.Assert(s.testRepo.Connect(ConnRef{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}), IsNil) + + conns, err := s.testRepo.Connected(s.plug.Snap.Name(), s.plug.Name) c.Assert(err, IsNil) - // Disconnecting a connected plug works okay - err = s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) + c.Check(conns, DeepEquals, []ConnRef{ + {s.plug.Ref(), s.slot.Ref()}, + }) + + conns, err = s.testRepo.Connected(s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) - c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ - Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, - Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, + c.Check(conns, DeepEquals, []ConnRef{ + {s.plug.Ref(), s.slot.Ref()}, }) } -func (s *RepositorySuite) TestDisconnectFromSnap(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - err = s.testRepo.AddSlot(s.slot) - c.Assert(err, IsNil) - connRef := ConnRef{PlugRef: PlugRef{Snap: s.plug.Snap.Name(), Name: s.plug.Name}, SlotRef: SlotRef{Snap: s.slot.Snap.Name(), Name: s.slot.Name}} - err = s.testRepo.Connect(connRef) - c.Assert(err, IsNil) - // Disconnecting everything from a snap works OK - err = s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") +// Connected uses the core snap if snap name is empty +func (s *RepositorySuite) TestConnectedFindsCoreSnap(c *C) { + slot := &Slot{ + SlotInfo: &snap.SlotInfo{ + Snap: &snap.Info{SuggestedName: "core", Type: snap.TypeOS}, + Name: "slot", + Interface: "interface", + }, + } + c.Assert(s.testRepo.AddPlug(s.plug), IsNil) + c.Assert(s.testRepo.AddSlot(slot), IsNil) + c.Assert(s.testRepo.Connect(ConnRef{PlugRef: s.plug.Ref(), SlotRef: slot.Ref()}), IsNil) + + conns, err := s.testRepo.Connected("", s.slot.Name) c.Assert(err, IsNil) - c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ - Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, - Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, + c.Check(conns, DeepEquals, []ConnRef{ + {s.plug.Ref(), slot.Ref()}, }) } -func (s *RepositorySuite) TestDisconnectFromSlot(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - err = s.testRepo.AddSlot(s.slot) - c.Assert(err, IsNil) - connRef := ConnRef{PlugRef: PlugRef{Snap: s.plug.Snap.Name(), Name: s.plug.Name}, SlotRef: SlotRef{Snap: s.slot.Snap.Name(), Name: s.slot.Name}} - err = s.testRepo.Connect(connRef) - c.Assert(err, IsNil) - // Disconnecting everything from a slot works OK - err = s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, IsNil) +// Tests for Repository.DisconnectAll() + +func (s *RepositorySuite) TestDisconnectAll(c *C) { + c.Assert(s.testRepo.AddPlug(s.plug), IsNil) + c.Assert(s.testRepo.AddSlot(s.slot), IsNil) + c.Assert(s.testRepo.Connect(ConnRef{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}), IsNil) + + conns := []ConnRef{{s.plug.Ref(), s.slot.Ref()}} + s.testRepo.DisconnectAll(conns) c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go index 1e300de863e..0b86dc70769 100644 --- a/overlord/ifacestate/handlers.go +++ b/overlord/ifacestate/handlers.go @@ -21,6 +21,7 @@ package ifacestate import ( "fmt" + "sort" "gopkg.in/tomb.v2" @@ -312,6 +313,20 @@ func (m *InterfaceManager) doConnect(task *state.Task, _ *tomb.Tomb) error { return nil } +func snapNamesFromConns(conns []interfaces.ConnRef) []string { + m := make(map[string]bool) + for _, conn := range conns { + m[conn.PlugRef.Snap] = true + m[conn.SlotRef.Snap] = true + } + l := make([]string, 0, len(m)) + for name := range m { + l = append(l, name) + } + sort.Strings(l) + return l +} + func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error { st := task.State() st.Lock() @@ -327,30 +342,47 @@ func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error { return err } - err = m.repo.Disconnect(plugRef.Snap, plugRef.Name, slotRef.Snap, slotRef.Name) - if err != nil { - return err - } - - plug := m.repo.Plug(plugRef.Snap, plugRef.Name) - var plugSnapst snapstate.SnapState - if err := snapstate.Get(st, plugRef.Snap, &plugSnapst); err != nil { - return err - } - slot := m.repo.Slot(slotRef.Snap, slotRef.Name) - var slotSnapst snapstate.SnapState - if err := snapstate.Get(st, slotRef.Snap, &slotSnapst); err != nil { - return err + var affectedConns []interfaces.ConnRef + if plugRef.Snap != "" && plugRef.Name != "" && slotRef.Snap != "" && slotRef.Name != "" { + if err := m.repo.Disconnect(plugRef.Snap, plugRef.Name, slotRef.Snap, slotRef.Name); err != nil { + return err + } + affectedConns = []interfaces.ConnRef{{plugRef, slotRef}} + } else if plugRef.Name != "" && slotRef.Snap == "" && slotRef.Name == "" { + // NOTE: plugRef.Snap can be either empty or not, Connected handles both + affectedConns, err = m.repo.Connected(plugRef.Snap, plugRef.Name) + if err != nil { + return err + } + m.repo.DisconnectAll(affectedConns) + } else if plugRef.Snap == "" && plugRef.Name == "" && slotRef.Name != "" { + // Symmetrically, slotRef.Snap can be either empty or not + affectedConns, err = m.repo.Connected(slotRef.Snap, slotRef.Name) + if err != nil { + return err + } + m.repo.DisconnectAll(affectedConns) + } else { + return fmt.Errorf("internal error, unhandled disconnect case plug: %q, slot: %q", plugRef, slotRef) } - - if err := setupSnapSecurity(task, plug.Snap, plugSnapst.DevModeAllowed(), m.repo); err != nil { - return err + affectedSnaps := snapNamesFromConns(affectedConns) + for _, snapName := range affectedSnaps { + var snapst snapstate.SnapState + if err := snapstate.Get(st, snapName, &snapst); err != nil { + return err + } + snapInfo, err := snapst.CurrentInfo() + if err != nil { + return err + } + if err := setupSnapSecurity(task, snapInfo, snapst.DevModeAllowed(), m.repo); err != nil { + return &state.Retry{} + } } - if err := setupSnapSecurity(task, slot.Snap, slotSnapst.DevModeAllowed(), m.repo); err != nil { - return err + for _, conn := range affectedConns { + delete(conns, conn.ID()) } - delete(conns, connID(plugRef, slotRef)) setConns(st, conns) return nil } diff --git a/overlord/ifacestate/helpers.go b/overlord/ifacestate/helpers.go index c6bd2c76c43..01ca1668292 100644 --- a/overlord/ifacestate/helpers.go +++ b/overlord/ifacestate/helpers.go @@ -190,16 +190,16 @@ func (m *InterfaceManager) autoConnect(task *state.Task, snapName string, blackl return nil } -func getPlugAndSlotRefs(task *state.Task) (*interfaces.PlugRef, *interfaces.SlotRef, error) { +func getPlugAndSlotRefs(task *state.Task) (interfaces.PlugRef, interfaces.SlotRef, error) { var plugRef interfaces.PlugRef var slotRef interfaces.SlotRef if err := task.Get("plug", &plugRef); err != nil { - return nil, nil, err + return plugRef, slotRef, err } if err := task.Get("slot", &slotRef); err != nil { - return nil, nil, err + return plugRef, slotRef, err } - return &plugRef, &slotRef, nil + return plugRef, slotRef, nil } func getConns(st *state.State) (map[string]connState, error) { diff --git a/overlord/ifacestate/ifacemgr_test.go b/overlord/ifacestate/ifacemgr_test.go index bd762ceff53..a225159d659 100644 --- a/overlord/ifacestate/ifacemgr_test.go +++ b/overlord/ifacestate/ifacemgr_test.go @@ -370,48 +370,87 @@ func (s *interfaceManagerSuite) TestDisconnectTask(c *C) { c.Assert(slot.Name, Equals, "slot") } -func (s *interfaceManagerSuite) TestEnsureProcessesDisconnectTask(c *C) { +// Disconnect works when both plug and slot are specified +func (s *interfaceManagerSuite) TestDisconnectFull(c *C) { + s.testDisconnect(c, "consumer", "plug", "producer", "slot") +} + +// Disconnect works when just the slot is fully specified. +func (s *interfaceManagerSuite) TestDisconnectSlot(c *C) { + s.testDisconnect(c, "", "", "producer", "slot") +} + +// Disconnect works when just the plug is fully specified. +func (s *interfaceManagerSuite) TestDisconnectPlug(c *C) { + s.testDisconnect(c, "consumer", "plug", "", "") +} + +func (s *interfaceManagerSuite) testDisconnect(c *C, plugSnap, plugName, slotSnap, slotName string) { + // Put two snaps in place They consumer has an plug that can be connected + // to slot on the producer. s.mockIface(c, &interfaces.TestInterface{InterfaceName: "test"}) s.mockSnap(c, consumerYaml) s.mockSnap(c, producerYaml) + // Put a connection in the state so that it automatically gets set up when + // we create the manager. s.state.Lock() s.state.Set("conns", map[string]interface{}{ "consumer:plug producer:slot": map[string]interface{}{"interface": "test"}, }) s.state.Unlock() + // Initialize the manager. This registers both snaps and reloads the connection. + mgr := s.manager(c) + + // Run the disconnect task and let it finish. s.state.Lock() - change := s.state.NewChange("kind", "summary") - ts, err := ifacestate.Disconnect(s.state, "consumer", "plug", "producer", "slot") + change := s.state.NewChange("disconnect", "...") + ts, err := ifacestate.Disconnect(s.state, plugSnap, plugName, slotSnap, slotName) ts.Tasks()[0].Set("snap-setup", &snapstate.SnapSetup{ SideInfo: &snap.SideInfo{ RealName: "consumer", }, }) + c.Assert(err, IsNil) change.AddAll(ts) s.state.Unlock() - - mgr := s.manager(c) mgr.Ensure() mgr.Wait() s.state.Lock() defer s.state.Unlock() + // Ensure that the task succeeded. c.Assert(change.Err(), IsNil) task := change.Tasks()[0] c.Check(task.Kind(), Equals, "disconnect") c.Check(task.Status(), Equals, state.DoneStatus) + c.Check(change.Status(), Equals, state.DoneStatus) - // The connection is gone + // Ensure that the connection has been removed from the state + var conns map[string]interface{} + err = s.state.Get("conns", &conns) + c.Assert(err, IsNil) + c.Check(conns, HasLen, 0) + + // Ensure that the connection has been removed from the repository repo := mgr.Repository() plug := repo.Plug("consumer", "plug") slot := repo.Slot("producer", "slot") c.Assert(plug.Connections, HasLen, 0) c.Assert(slot.Connections, HasLen, 0) + + // Ensure that the backend was used to setup security of both snaps + c.Assert(s.secBackend.SetupCalls, HasLen, 2) + c.Assert(s.secBackend.RemoveCalls, HasLen, 0) + c.Check(s.secBackend.SetupCalls[0].SnapInfo.Name(), Equals, "consumer") + c.Check(s.secBackend.SetupCalls[1].SnapInfo.Name(), Equals, "producer") + + c.Check(s.secBackend.SetupCalls[0].DevMode, Equals, false) + c.Check(s.secBackend.SetupCalls[1].DevMode, Equals, false) } func (s *interfaceManagerSuite) mockIface(c *C, iface interfaces.Interface) { diff --git a/tests/main/snap-disconnect/task.yaml b/tests/main/snap-disconnect/task.yaml new file mode 100644 index 00000000000..6f5ca28a9ec --- /dev/null +++ b/tests/main/snap-disconnect/task.yaml @@ -0,0 +1,37 @@ +summary: Check that snap disconnect works + +systems: [-ubuntu-core-16-64] + +environment: + SNAP_FILE: "home-consumer_1.0_all.snap" + +prepare: | + echo "Install a test snap" + snapbuild $TESTSLIB/snaps/home-consumer . + snap install --dangerous $SNAP_FILE + +restore: | + rm -f *.snap + +execute: | + DISCONNECTED_PATTERN="\-\s+home-consumer:home" + + echo "Disconnect everything from given slot" + snap connect home-consumer:home ubuntu-core:home + snap disconnect ubuntu-core:home + snap interfaces | grep -Pzqe "$DISCONNECTED_PATTERN" + + echo "Disconnect everything from given plug" + snap connect home-consumer:home ubuntu-core:home + snap disconnect home-consumer:home + snap interfaces | grep -Pzqe "$DISCONNECTED_PATTERN" + + echo "Disconnect everything from given slot of OS snap" + snap connect home-consumer:home ubuntu-core:home + snap disconnect :home + snap interfaces | grep -Pzqe "$DISCONNECTED_PATTERN" + + echo "Disconnect specific plug and slot" + snap connect home-consumer:home ubuntu-core:home + snap disconnect home-consumer:home ubuntu-core:home + snap interfaces | grep -Pzqe "$DISCONNECTED_PATTERN"