From d02b967672255c75bb06546a167a4585edb65638 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 18 Apr 2016 14:06:56 +0200 Subject: [PATCH 01/29] interfaces: add {Plug,Slot}.Ref This patch adds two helpers so that it is easy to create a PlugRef from Plug and a SlotRef from Slot. Signed-off-by: Zygmunt Krynicki --- interfaces/core.go | 10 ++++++++++ interfaces/core_test.go | 17 +++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/interfaces/core.go b/interfaces/core.go index 2d71ed08393..62c6196ead3 100644 --- a/interfaces/core.go +++ b/interfaces/core.go @@ -33,6 +33,11 @@ type Plug struct { Connections []SlotRef `json:"connections,omitempty"` } +// Ref returns reference to a plug +func (plug *Plug) Ref() PlugRef { + return PlugRef{Snap: plug.Snap.Name(), Name: plug.Name} +} + // PlugRef is a reference to a plug. type PlugRef struct { Snap string `json:"snap"` @@ -45,6 +50,11 @@ type Slot struct { Connections []PlugRef `json:"connections,omitempty"` } +// Ref returns reference to a slot +func (slot *Slot) Ref() SlotRef { + return SlotRef{Snap: slot.Snap.Name(), Name: slot.Name} +} + // SlotRef is a reference to a slot. type SlotRef struct { Snap string `json:"snap"` diff --git a/interfaces/core_test.go b/interfaces/core_test.go index 5e9712587de..d96a08994ab 100644 --- a/interfaces/core_test.go +++ b/interfaces/core_test.go @@ -25,6 +25,7 @@ import ( . "gopkg.in/check.v1" . "github.com/ubuntu-core/snappy/interfaces" + "github.com/ubuntu-core/snappy/snap" ) func Test(t *testing.T) { @@ -66,3 +67,19 @@ func (s *CoreSuite) TestValidateName(c *C) { c.Assert(err, ErrorMatches, `invalid interface name: ".*"`) } } + +// Plug.Ref works as expected +func (s *CoreSuite) TestPlugRef(c *C) { + plug := &Plug{PlugInfo: &snap.PlugInfo{Snap: &snap.Info{SuggestedName: "consumer"}, Name: "plug"}} + ref := plug.Ref() + c.Check(ref.Snap, Equals, "consumer") + c.Check(ref.Name, Equals, "plug") +} + +// Slot.Ref works as expected +func (s *CoreSuite) TestSlotRef(c *C) { + slot := &Slot{SlotInfo: &snap.SlotInfo{Snap: &snap.Info{SuggestedName: "producer"}, Name: "slot"}} + ref := slot.Ref() + c.Check(ref.Snap, Equals, "producer") + c.Check(ref.Name, Equals, "slot") +} From 47a5852139a9f028f48a163421e887aa955f36de Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 18 Apr 2016 14:07:05 +0200 Subject: [PATCH 02/29] interfaces: add ConnRef This patch adds a structure representing a single connection. Signed-off-by: Zygmunt Krynicki --- interfaces/core.go | 11 +++++++++++ interfaces/core_test.go | 9 +++++++++ 2 files changed, 20 insertions(+) diff --git a/interfaces/core.go b/interfaces/core.go index 62c6196ead3..0ed8da014c5 100644 --- a/interfaces/core.go +++ b/interfaces/core.go @@ -67,6 +67,17 @@ type Interfaces struct { Slots []*Slot `json:"slots"` } +// ConnRef holds information about plug and slot reference that form a particular connection. +type ConnRef struct { + PlugRef PlugRef + SlotRef SlotRef +} + +// ID returns a string identifying a given connection. +func (conn *ConnRef) ID() string { + return fmt.Sprintf("%s:%s %s:%s", conn.PlugRef.Snap, conn.PlugRef.Name, conn.SlotRef.Snap, conn.SlotRef.Name) +} + // Interface describes a group of interchangeable capabilities with common features. // Interfaces act as a contract between system builders, application developers // and end users. diff --git a/interfaces/core_test.go b/interfaces/core_test.go index d96a08994ab..357bd3f3206 100644 --- a/interfaces/core_test.go +++ b/interfaces/core_test.go @@ -83,3 +83,12 @@ func (s *CoreSuite) TestSlotRef(c *C) { c.Check(ref.Snap, Equals, "producer") c.Check(ref.Name, Equals, "slot") } + +// ConnRef.ID works as expected +func (s *CoreSuite) TestConnRefID(c *C) { + conn := &ConnRef{ + PlugRef: PlugRef{Snap: "consumer", Name: "plug"}, + SlotRef: SlotRef{Snap: "producer", Name: "slot"}, + } + c.Check(conn.ID(), Equals, "consumer:plug producer:slot") +} From 982d0865b7e65c3560426d764b1950b15c7c4226 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 18 Apr 2016 11:57:09 +0200 Subject: [PATCH 03/29] interfaces: return list of affected connections and snaps from Disconnect Disconnect supports abbreviated forms where one can disconnect, for example, all the connections affecting a given snap. Since interface manager needs to setup security of all the affected snaps and to forget the persistent state of all the affected connections this information is now provided directly by the Disconnect operation. Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 77 +++++++++++++++++++++++++++++++++-------- interfaces/repo_test.go | 53 ++++++++++++++++++++++------ 2 files changed, 104 insertions(+), 26 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index 8acf8d629b3..2154b6dce90 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -322,68 +322,115 @@ func (r *Repository) Connect(plugSnapName, plugName, slotSnapName, slotName stri // 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. -func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName string) error { +// +// Disconnect returns the list of severed connections, the list of affected snaps +// and an error if one occurred. +func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName string) ([]ConnRef, []*snap.Info, error) { r.m.Lock() defer r.m.Unlock() + var conns []ConnRef + var snaps map[string]*snap.Info + var err error + switch { case plugSnapName == "" && plugName == "" && slotName == "": // Disconnect everything from slotSnapName - return r.disconnectEverythingFromSnap(slotSnapName) + conns, snaps, err = r.disconnectEverythingFromSnap(slotSnapName) case plugSnapName == "" && plugName == "": // Disconnect everything from slotSnapName:slotName - return r.disconnectEverythingFromSlot(slotSnapName, slotName) + conns, snaps, err = r.disconnectEverythingFromSlot(slotSnapName, slotName) default: - return r.disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName) + conns, snaps, err = r.disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName) } + if err != nil { + return nil, nil, err + } + // Flatten map of snaps into a sorted list + names := make([]string, 0, len(snaps)) + for snapName := range snaps { + names = append(names, snapName) + } + sort.Strings(names) + result := make([]*snap.Info, 0, len(snaps)) + for _, snapName := range names { + result = append(result, snaps[snapName]) + } + return conns, result, nil } // disconnectEverythingFromSnap finds a specific snap and disconnects all the plugs connected to all the slots therein. -func (r *Repository) disconnectEverythingFromSnap(slotSnapName string) error { +func (r *Repository) disconnectEverythingFromSnap(slotSnapName string) ([]ConnRef, map[string]*snap.Info, error) { + if _, ok := r.slots[slotSnapName]; !ok { + err := fmt.Errorf("cannot disconnect plug from snap %q, no such snap", slotSnapName) + return nil, nil, err + } if _, ok := r.slots[slotSnapName]; !ok { - return fmt.Errorf("cannot disconnect plug from snap %q, no such snap", slotSnapName) + return nil, nil, nil } + var conns []ConnRef + snaps := make(map[string]*snap.Info) for _, slot := range r.slots[slotSnapName] { for plug := range r.slotPlugs[slot] { r.disconnect(plug, slot) + snaps[plug.Snap.Name()] = plug.Snap + snaps[slot.Snap.Name()] = slot.Snap + conns = append(conns, ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}) } } - return nil + return conns, snaps, nil } // disconnectEverythingFromSlot finds a specific plug slot and disconnects all the plugs connected there. -func (r *Repository) disconnectEverythingFromSlot(slotSnapName, slotName string) error { +func (r *Repository) disconnectEverythingFromSlot(slotSnapName, slotName string) ([]ConnRef, map[string]*snap.Info, 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) + err := fmt.Errorf("cannot disconnect plug from slot %q from snap %q, no such slot", slotName, slotSnapName) + return nil, nil, err + } + if _, ok := r.slots[slotSnapName]; !ok { + return nil, nil, nil } + conns := make([]ConnRef, 0, len(r.slotPlugs[slot])) + snaps := make(map[string]*snap.Info) for plug := range r.slotPlugs[slot] { r.disconnect(plug, slot) + snaps[plug.Snap.Name()] = plug.Snap + snaps[slot.Snap.Name()] = slot.Snap + conns = append(conns, ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}) } - return nil + return conns, snaps, nil } // disconnectPlugFromSlot finds a specific plug slot and plug and disconnects it. -func (r *Repository) disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName string) error { +func (r *Repository) disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName string) ([]ConnRef, map[string]*snap.Info, 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) + err := fmt.Errorf("cannot disconnect plug %q from snap %q, no such plug", plugName, plugSnapName) + return nil, nil, err } // 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) + err := fmt.Errorf("cannot disconnect plug from slot %q from snap %q, no such slot", slotName, slotSnapName) + return nil, nil, err } // 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", + err := fmt.Errorf("cannot disconnect plug %q from snap %q from slot %q from snap %q, it is not connected", plugName, plugSnapName, slotName, slotSnapName) + return nil, nil, err } r.disconnect(plug, slot) - return nil + conns := []ConnRef{{PlugRef: plug.Ref(), SlotRef: slot.Ref()}} + snaps := map[string]*snap.Info{ + plug.Snap.Name(): plug.Snap, + slot.Snap.Name(): slot.Snap, + } + return conns, snaps, nil } // disconnect disconnects a plug from a slot. diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index afb85f152c0..4af033b0042 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -591,32 +591,40 @@ 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) + snaps, conns, 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 "provider", no such plug`) + c.Check(snaps, IsNil) + c.Check(conns, IsNil) } 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) + conns, snaps, 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 "consumer", no such slot`) + c.Check(snaps, IsNil) + c.Check(conns, IsNil) } 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) + conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) c.Assert(err, ErrorMatches, `cannot disconnect plug from slot "slot" from snap "consumer", no such slot`) + c.Check(conns, IsNil) + c.Check(snaps, IsNil) } func (s *RepositorySuite) TestDisconnectFromSnapFailsWhenSlotDoesNotExist(c *C) { err := s.testRepo.AddPlug(s.plug) 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(), "") + conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") c.Assert(err, ErrorMatches, `cannot disconnect plug from snap "consumer", no such snap`) + c.Check(conns, IsNil) + c.Check(snaps, IsNil) } func (s *RepositorySuite) TestDisconnectFailsWhenNotConnected(c *C) { @@ -625,8 +633,10 @@ func (s *RepositorySuite) TestDisconnectFailsWhenNotConnected(c *C) { 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) + conns, snaps, 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 "provider" from slot "slot" from snap "consumer", it is not connected`) + c.Check(conns, IsNil) + c.Check(snaps, IsNil) } func (s *RepositorySuite) TestDisconnectFromSnapDoesNothingWhenNotConnected(c *C) { @@ -635,8 +645,10 @@ func (s *RepositorySuite) TestDisconnectFromSnapDoesNothingWhenNotConnected(c *C 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(), "") + conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") c.Assert(err, IsNil) + c.Check(conns, HasLen, 0) + c.Check(snaps, HasLen, 0) } func (s *RepositorySuite) TestDisconnectFromSlotDoesNothingWhenNotConnected(c *C) { @@ -645,8 +657,10 @@ func (s *RepositorySuite) TestDisconnectFromSlotDoesNothingWhenNotConnected(c *C 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) + conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) + c.Check(conns, HasLen, 0) + c.Check(snaps, HasLen, 0) } func (s *RepositorySuite) TestDisconnectSucceeds(c *C) { @@ -657,8 +671,12 @@ func (s *RepositorySuite) TestDisconnectSucceeds(c *C) { err = s.testRepo.Connect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.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) + conns, snaps, err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) + c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) + c.Assert(snaps, HasLen, 2) + c.Check(snaps[0], Equals, s.slot.Snap) + c.Check(snaps[1], Equals, s.plug.Snap) c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, @@ -673,8 +691,12 @@ func (s *RepositorySuite) TestDisconnectFromSnap(c *C) { err = s.testRepo.Connect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) // Disconnecting everything from a snap works OK - err = s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") + conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") c.Assert(err, IsNil) + c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) + c.Assert(snaps, HasLen, 2) + c.Check(snaps[0], Equals, s.slot.Snap) + c.Check(snaps[1], Equals, s.plug.Snap) c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, @@ -689,8 +711,13 @@ func (s *RepositorySuite) TestDisconnectFromSlot(c *C) { err = s.testRepo.Connect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) // Disconnecting everything from a plug slot works OK - err = s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) + conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) + c.Assert(conns, HasLen, 1) + c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) + c.Assert(snaps, HasLen, 2) + c.Check(snaps[0], Equals, s.slot.Snap) + c.Check(snaps[1], Equals, s.plug.Snap) c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, @@ -719,8 +746,12 @@ func (s *RepositorySuite) TestInterfacesSmokeTest(c *C) { }}, }) // After disconnecting the connections become empty - err = s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) + conns, snaps, err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) + c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) + c.Assert(snaps, HasLen, 2) + c.Check(snaps[0], Equals, s.slot.Snap) + c.Check(snaps[1], Equals, s.plug.Snap) ifaces = s.testRepo.Interfaces() c.Assert(ifaces, DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, From e7ba90c1ae02bc86d0ca02ca9f4d255e75d55c26 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 18 Apr 2016 14:06:03 +0200 Subject: [PATCH 04/29] overlord/ifacestate: add support for abbreviated disconnect operations This patch adds support for abbreviated disconnect operations that match the suggestions printed by "$ snap disconnect --help". This also fixes a bug where snapd would crash if any of those abbreviated forms were to be used. Fixes: https://bugs.launchpad.net/snappy/+bug/1571497 Signed-off-by: Zygmunt Krynicki --- overlord/ifacestate/ifacemgr.go | 17 +++++---- overlord/ifacestate/ifacemgr_test.go | 52 +++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/overlord/ifacestate/ifacemgr.go b/overlord/ifacestate/ifacemgr.go index 609b1873a24..e798e1a3cfe 100644 --- a/overlord/ifacestate/ifacemgr.go +++ b/overlord/ifacestate/ifacemgr.go @@ -540,21 +540,20 @@ func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error { return err } - err = m.repo.Disconnect(plugRef.Snap, plugRef.Name, slotRef.Snap, slotRef.Name) + affectedConns, affectedSnaps, 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) - slot := m.repo.Slot(slotRef.Snap, slotRef.Name) - if err := setupSnapSecurity(task, plug.Snap, m.repo); err != nil { - return state.Retry + for _, snapInfo := range affectedSnaps { + if err := setupSnapSecurity(task, snapInfo, m.repo); err != nil { + return state.Retry + } } - if err := setupSnapSecurity(task, slot.Snap, m.repo); err != nil { - return state.Retry + for _, connRef := range affectedConns { + delete(conns, connRef.ID()) } - - delete(conns, connID(plugRef, slotRef)) setConns(st, conns) return nil } diff --git a/overlord/ifacestate/ifacemgr_test.go b/overlord/ifacestate/ifacemgr_test.go index c756631783b..6c497cab0d0 100644 --- a/overlord/ifacestate/ifacemgr_test.go +++ b/overlord/ifacestate/ifacemgr_test.go @@ -157,42 +157,76 @@ func (s *interfaceManagerSuite) TestDisconnectTask(c *C) { c.Assert(slot.Name, Equals, "slot") } -func (s *interfaceManagerSuite) TestEnsureProcessesDisconnectTask(c *C) { +// Disconnect works when used with the fully-spelled-out form. +func (s *interfaceManagerSuite) TestDisconnectFull(c *C) { + s.testDisconnect(c, "consumer", "plug", "producer", "slot") +} + +// Disconnect works when only the slot is fully specified. +func (s *interfaceManagerSuite) TestDisconnectSlot(c *C) { + s.testDisconnect(c, "", "", "producer", "slot") +} + +// Disconnect works when only the snap from the slot is specified. +func (s *interfaceManagerSuite) TestDisconnectSlotSnap(c *C) { + s.testDisconnect(c, "", "", "producer", "") +} + +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) 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() - task := change.Tasks()[0] - c.Check(task.Kind(), Equals, "disconnect") - c.Check(task.Status(), Equals, state.DoneStatus) + // Ensure that the task succeeded. 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) { From c1805ec732ddb821e934aec5ec37e657b44a1df5 Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Wed, 28 Sep 2016 17:30:16 +0200 Subject: [PATCH 05/29] test fixes: consumer vs producer, snaps are ordering by name --- interfaces/repo_test.go | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index 3552bafdc81..af8c88423e0 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -593,7 +593,7 @@ func (s *RepositorySuite) TestDisconnectFailsWhenPlugDoesNotExist(c *C) { c.Assert(err, IsNil) // Disconnecting an unknown plug returns and appropriate error snaps, conns, 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 "provider", no such plug`) + c.Assert(err, ErrorMatches, `cannot disconnect plug "plug" from snap "consumer", no such plug`) c.Check(snaps, IsNil) c.Check(conns, IsNil) } @@ -603,7 +603,7 @@ func (s *RepositorySuite) TestDisconnectFailsWhenSlotDoesNotExist(c *C) { c.Assert(err, IsNil) // Disconnecting from an unknown slot returns an appropriate error conns, snaps, 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 "consumer", no such slot`) + c.Assert(err, ErrorMatches, `cannot disconnect plug from slot "slot" from snap "producer", no such slot`) c.Check(snaps, IsNil) c.Check(conns, IsNil) } @@ -613,7 +613,7 @@ func (s *RepositorySuite) TestDisconnectFromSlotFailsWhenSlotDoesNotExist(c *C) c.Assert(err, IsNil) // Disconnecting everything form an unknown slot returns an appropriate error conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, ErrorMatches, `cannot disconnect plug from slot "slot" from snap "consumer", no such slot`) + c.Assert(err, ErrorMatches, `cannot disconnect plug from slot "slot" from snap "producer", no such slot`) c.Check(conns, IsNil) c.Check(snaps, IsNil) } @@ -623,7 +623,7 @@ func (s *RepositorySuite) TestDisconnectFromSnapFailsWhenSlotDoesNotExist(c *C) c.Assert(err, IsNil) // Disconnecting all plugs from a snap that is not known returns an appropriate error conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") - c.Assert(err, ErrorMatches, `cannot disconnect plug from snap "consumer", no such snap`) + c.Assert(err, ErrorMatches, `cannot disconnect plug from snap "producer", no such snap`) c.Check(conns, IsNil) c.Check(snaps, IsNil) } @@ -635,7 +635,7 @@ func (s *RepositorySuite) TestDisconnectFailsWhenNotConnected(c *C) { c.Assert(err, IsNil) // Disconnecting a plug that is not connected returns an appropriate error conns, snaps, 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 "provider" from slot "slot" from snap "consumer", it is not connected`) + c.Assert(err, ErrorMatches, `cannot disconnect plug "plug" from snap "consumer" from slot "slot" from snap "producer", it is not connected`) c.Check(conns, IsNil) c.Check(snaps, IsNil) } @@ -676,8 +676,9 @@ func (s *RepositorySuite) TestDisconnectSucceeds(c *C) { c.Assert(err, IsNil) c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) c.Assert(snaps, HasLen, 2) - c.Check(snaps[0], Equals, s.slot.Snap) - c.Check(snaps[1], Equals, s.plug.Snap) + // beware, the snaps are sorted by name + c.Check(snaps[0], Equals, s.plug.Snap) + c.Check(snaps[1], Equals, s.slot.Snap) c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, @@ -696,8 +697,9 @@ func (s *RepositorySuite) TestDisconnectFromSnap(c *C) { c.Assert(err, IsNil) c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) c.Assert(snaps, HasLen, 2) - c.Check(snaps[0], Equals, s.slot.Snap) - c.Check(snaps[1], Equals, s.plug.Snap) + // beware, the snaps are sorted by name + c.Check(snaps[0], Equals, s.plug.Snap) + c.Check(snaps[1], Equals, s.slot.Snap) c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, @@ -719,8 +721,9 @@ func (s *RepositorySuite) TestDisconnectFromSlot(c *C) { c.Assert(conns, HasLen, 1) c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) c.Assert(snaps, HasLen, 2) - c.Check(snaps[0], Equals, s.slot.Snap) - c.Check(snaps[1], Equals, s.plug.Snap) + // beware, the snaps are sorted by name + c.Check(snaps[0], Equals, s.plug.Snap) + c.Check(snaps[1], Equals, s.slot.Snap) c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, @@ -753,8 +756,9 @@ func (s *RepositorySuite) TestInterfacesSmokeTest(c *C) { c.Assert(err, IsNil) c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) c.Assert(snaps, HasLen, 2) - c.Check(snaps[0], Equals, s.slot.Snap) - c.Check(snaps[1], Equals, s.plug.Snap) + // beware, the snaps are sorted by name + c.Check(snaps[0], Equals, s.plug.Snap) + c.Check(snaps[1], Equals, s.slot.Snap) ifaces = s.testRepo.Interfaces() c.Assert(ifaces, DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, From faf4b631f893ecaa28ebfb52cc45c6ab6dd7661a Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Fri, 30 Sep 2016 10:54:10 +0200 Subject: [PATCH 06/29] Added disconnectPlugFromOsSlot. --- interfaces/repo.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/interfaces/repo.go b/interfaces/repo.go index b3cb139e645..d6f72bda8a3 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -340,6 +340,8 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s case plugSnapName == "" && plugName == "": // Disconnect everything from slotSnapName:slotName conns, snaps, err = r.disconnectEverythingFromSlot(slotSnapName, slotName) + case slotSnapName == "": + conns, snaps, err = r.disconnectPlugFromOsSlot(plugSnapName, plugName, slotName) default: conns, snaps, err = r.disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName) } @@ -404,6 +406,30 @@ func (r *Repository) disconnectEverythingFromSlot(slotSnapName, slotName string) return conns, snaps, nil } +// disconnectPlugFromOsSlot finds specific slot in the OS snap and disconnects the plug from it. +func (r *Repository) disconnectPlugFromOsSlot(plugSnapName, plugName, slotName string) ([]ConnRef, map[string]*snap.Info, error) { + // Ensure that such plug exists + plug := r.plugs[plugSnapName][plugName] + if plug == nil { + err := fmt.Errorf("cannot disconnect plug %q from snap %q, no such plug", plugName, plugSnapName) + return nil, nil, err + } + + for slot := range r.plugSlots[plug] { + if slot.Snap.Type == snap.TypeOS && slot.Name == slotName { + r.disconnect(plug, slot) + conns := []ConnRef{{PlugRef: plug.Ref(), SlotRef: slot.Ref()}} + snaps := map[string]*snap.Info{ + plug.Snap.Name(): plug.Snap, + slot.Snap.Name(): slot.Snap, + } + return conns, snaps, nil + } + } + err := fmt.Errorf("cannot disconnect plug %q from snap %q from core snap, no matching slot", plugName, plugSnapName) + return nil, nil, err +} + // disconnectPlugFromSlot finds a specific plug slot and plug and disconnects it. func (r *Repository) disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName string) ([]ConnRef, map[string]*snap.Info, error) { // Ensure that such plug exists From ec4bcae5d31ee9250999ccd2cf08443d44ee8790 Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Fri, 30 Sep 2016 11:22:22 +0200 Subject: [PATCH 07/29] Unit tests for disconnect from core snap. --- interfaces/repo_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index af8c88423e0..7f3034b782f 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -730,6 +730,36 @@ func (s *RepositorySuite) TestDisconnectFromSlot(c *C) { }) } +func (s *RepositorySuite) TestDisconnectFromOsSlot(c *C) { + err := s.testRepo.AddPlug(s.plug) + c.Assert(err, IsNil) + err = s.testRepo.AddSlot(s.slot) + c.Assert(err, IsNil) + coreSlot := &Slot{ + SlotInfo: &snap.SlotInfo{ + Snap: &snap.Info{SuggestedName: "foo-core", Type: snap.TypeOS}, + Name: "foo-slot", + Interface: "interface", + }, + } + err = s.testRepo.AddSlot(coreSlot) + c.Assert(err, IsNil) + + err = s.testRepo.Connect(s.plug.Snap.Name(), s.plug.Name, coreSlot.Snap.Name(), coreSlot.Name) + c.Assert(err, IsNil) + + // slot snap name omitted + conns, snaps, err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, "", "foo-slot") + c.Assert(err, IsNil) + c.Assert(conns, HasLen, 1) + c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: coreSlot.Ref()}}) + + c.Assert(snaps, HasLen, 2) + // beware, the snaps are sorted by name + c.Check(snaps[0], Equals, s.plug.Snap) + c.Check(snaps[1], Equals, coreSlot.Snap) +} + // Tests for Repository.Interfaces() func (s *RepositorySuite) TestInterfacesSmokeTest(c *C) { From b44bb08ae1b3990d9edf985073a1429c88c581df Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Fri, 30 Sep 2016 13:17:10 +0200 Subject: [PATCH 08/29] Added a spread test fro disconnect (WIP) --- tests/main/snap-disconnect/task.yaml | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 tests/main/snap-disconnect/task.yaml diff --git a/tests/main/snap-disconnect/task.yaml b/tests/main/snap-disconnect/task.yaml new file mode 100644 index 00000000000..533c462513e --- /dev/null +++ b/tests/main/snap-disconnect/task.yaml @@ -0,0 +1,32 @@ +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 connected to given snap" + snap connect home-consumer:home ubuntu-core:home + snap disconnect ubuntu-core + snap interfaces | grep -Pzqe "$DISCONNECTED_PATTERN" + + 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 "The plug can be disconnected from the OS snap without OS snap name" + snap connect home-consumer:home ubuntu-core:home + snap disconnect home-consumer:home :home + snap interfaces | grep -Pzqe "$DISCONNECTED_PATTERN" From ea0ee46150fe86f075ac9c18e04e830a40a704db Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Fri, 30 Sep 2016 13:41:20 +0200 Subject: [PATCH 09/29] Added disconnectPlugOrSlot --- cmd/snap/cmd_disconnect.go | 11 +++---- cmd/snap/interfaces_common.go | 4 --- interfaces/repo.go | 44 +++++++++++++++------------- interfaces/repo_test.go | 21 +++++++++++-- tests/main/snap-disconnect/task.yaml | 9 ++++-- 5 files changed, 52 insertions(+), 37 deletions(-) diff --git a/cmd/snap/cmd_disconnect.go b/cmd/snap/cmd_disconnect.go index 3c40ccbe433..05744efb554 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 : - -Disconnects any previously connected plugs from the provided slot. - -$ snap disconnect - -Disconnects all plugs from the provided snap. +$ snap disconnect : +. +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/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/interfaces/repo.go b/interfaces/repo.go index d6f72bda8a3..f6f2b08b04f 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -340,8 +340,8 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s case plugSnapName == "" && plugName == "": // Disconnect everything from slotSnapName:slotName conns, snaps, err = r.disconnectEverythingFromSlot(slotSnapName, slotName) - case slotSnapName == "": - conns, snaps, err = r.disconnectPlugFromOsSlot(plugSnapName, plugName, slotName) + case slotSnapName == "" && slotName == "": + conns, snaps, err = r.disconnectPlugOrSlot(plugSnapName, plugName) default: conns, snaps, err = r.disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName) } @@ -406,28 +406,30 @@ func (r *Repository) disconnectEverythingFromSlot(slotSnapName, slotName string) return conns, snaps, nil } -// disconnectPlugFromOsSlot finds specific slot in the OS snap and disconnects the plug from it. -func (r *Repository) disconnectPlugFromOsSlot(plugSnapName, plugName, slotName string) ([]ConnRef, map[string]*snap.Info, error) { - // Ensure that such plug exists - plug := r.plugs[plugSnapName][plugName] - if plug == nil { - err := fmt.Errorf("cannot disconnect plug %q from snap %q, no such plug", plugName, plugSnapName) - return nil, nil, err +func (r *Repository) disconnectPlugOrSlot(snapName, plugOrSlotName string) ([]ConnRef, map[string]*snap.Info, error) { + if snapName == "" { + // TODO: teach the repository about the OS snap + snapName = "ubuntu-core" } - - for slot := range r.plugSlots[plug] { - if slot.Snap.Type == snap.TypeOS && slot.Name == slotName { - r.disconnect(plug, slot) - conns := []ConnRef{{PlugRef: plug.Ref(), SlotRef: slot.Ref()}} - snaps := map[string]*snap.Info{ - plug.Snap.Name(): plug.Snap, - slot.Snap.Name(): slot.Snap, - } - return conns, snaps, nil + var conns []ConnRef + snaps := make(map[string]*snap.Info) + d := func(plug *Plug, slot *Slot) { + r.disconnect(plug, slot) + snaps[plug.Snap.Name()] = plug.Snap + snaps[slot.Snap.Name()] = slot.Snap + conns = append(conns, ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}) + } + if slot := r.slots[snapName][plugOrSlotName]; slot != nil { + for plug := range r.slotPlugs[slot] { + d(plug, slot) } } - err := fmt.Errorf("cannot disconnect plug %q from snap %q from core snap, no matching slot", plugName, plugSnapName) - return nil, nil, err + if plug := r.plugs[snapName][plugOrSlotName]; plug != nil { + for slot := range r.plugSlots[plug] { + d(plug, slot) + } + } + return conns, snaps, nil } // disconnectPlugFromSlot finds a specific plug slot and plug and disconnects it. diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index 7f3034b782f..cc50d0c22d6 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -730,7 +730,7 @@ func (s *RepositorySuite) TestDisconnectFromSlot(c *C) { }) } -func (s *RepositorySuite) TestDisconnectFromOsSlot(c *C) { +func (s *RepositorySuite) TestDisconnectPlugOrSlot(c *C) { err := s.testRepo.AddPlug(s.plug) c.Assert(err, IsNil) err = s.testRepo.AddSlot(s.slot) @@ -748,8 +748,23 @@ func (s *RepositorySuite) TestDisconnectFromOsSlot(c *C) { err = s.testRepo.Connect(s.plug.Snap.Name(), s.plug.Name, coreSlot.Snap.Name(), coreSlot.Name) c.Assert(err, IsNil) - // slot snap name omitted - conns, snaps, err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, "", "foo-slot") + // slot and snap name omitted + conns, snaps, err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, "", "") + c.Assert(err, IsNil) + c.Assert(conns, HasLen, 1) + c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: coreSlot.Ref()}}) + + c.Assert(snaps, HasLen, 2) + // beware, the snaps are sorted by name + c.Check(snaps[0], Equals, s.plug.Snap) + c.Check(snaps[1], Equals, coreSlot.Snap) + + // connect again + err = s.testRepo.Connect(s.plug.Snap.Name(), s.plug.Name, coreSlot.Snap.Name(), coreSlot.Name) + c.Assert(err, IsNil) + + // plug and snap name omitted + conns, snaps, err = s.testRepo.Disconnect(coreSlot.Snap.Name(), coreSlot.Name, "", "") c.Assert(err, IsNil) c.Assert(conns, HasLen, 1) c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: coreSlot.Ref()}}) diff --git a/tests/main/snap-disconnect/task.yaml b/tests/main/snap-disconnect/task.yaml index 533c462513e..84f9396a8fb 100644 --- a/tests/main/snap-disconnect/task.yaml +++ b/tests/main/snap-disconnect/task.yaml @@ -26,7 +26,12 @@ execute: | snap disconnect ubuntu-core:home snap interfaces | grep -Pzqe "$DISCONNECTED_PATTERN" - echo "The plug can be disconnected from the OS snap without OS snap name" + echo "Disconnect everything from given plug" snap connect home-consumer:home ubuntu-core:home - snap disconnect home-consumer:home :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" From e9c229351204f47be530a50cb976e60d49a0064c Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Fri, 30 Sep 2016 14:21:19 +0200 Subject: [PATCH 10/29] Cleanups, fixes --- cmd/snap/cmd_disconnect.go | 2 +- cmd/snap/cmd_disconnect_test.go | 9 +++------ cmd/snap/interfaces_common_test.go | 1 - interfaces/repo.go | 26 +------------------------- tests/main/snap-disconnect/task.yaml | 5 +++++ 5 files changed, 10 insertions(+), 33 deletions(-) diff --git a/cmd/snap/cmd_disconnect.go b/cmd/snap/cmd_disconnect.go index 05744efb554..ed01d42c5fa 100644 --- a/cmd/snap/cmd_disconnect.go +++ b/cmd/snap/cmd_disconnect.go @@ -42,7 +42,7 @@ $ snap disconnect : : Disconnects the specific plug from the specific slot. $ snap disconnect : -. + Disconnects everything from the provided plug or slot. The snap name may be omitted for the OS snap. `) 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_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/interfaces/repo.go b/interfaces/repo.go index f6f2b08b04f..e26b7d52ccf 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -339,9 +339,7 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s conns, snaps, err = r.disconnectEverythingFromSnap(slotSnapName) case plugSnapName == "" && plugName == "": // Disconnect everything from slotSnapName:slotName - conns, snaps, err = r.disconnectEverythingFromSlot(slotSnapName, slotName) - case slotSnapName == "" && slotName == "": - conns, snaps, err = r.disconnectPlugOrSlot(plugSnapName, plugName) + conns, snaps, err = r.disconnectPlugOrSlot(slotSnapName, slotName) default: conns, snaps, err = r.disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName) } @@ -384,28 +382,6 @@ func (r *Repository) disconnectEverythingFromSnap(slotSnapName string) ([]ConnRe return conns, snaps, nil } -// disconnectEverythingFromSlot finds a specific plug slot and disconnects all the plugs connected there. -func (r *Repository) disconnectEverythingFromSlot(slotSnapName, slotName string) ([]ConnRef, map[string]*snap.Info, error) { - // Ensure that such slot exists - slot := r.slots[slotSnapName][slotName] - if slot == nil { - err := fmt.Errorf("cannot disconnect plug from slot %q from snap %q, no such slot", slotName, slotSnapName) - return nil, nil, err - } - if _, ok := r.slots[slotSnapName]; !ok { - return nil, nil, nil - } - conns := make([]ConnRef, 0, len(r.slotPlugs[slot])) - snaps := make(map[string]*snap.Info) - for plug := range r.slotPlugs[slot] { - r.disconnect(plug, slot) - snaps[plug.Snap.Name()] = plug.Snap - snaps[slot.Snap.Name()] = slot.Snap - conns = append(conns, ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}) - } - return conns, snaps, nil -} - func (r *Repository) disconnectPlugOrSlot(snapName, plugOrSlotName string) ([]ConnRef, map[string]*snap.Info, error) { if snapName == "" { // TODO: teach the repository about the OS snap diff --git a/tests/main/snap-disconnect/task.yaml b/tests/main/snap-disconnect/task.yaml index 84f9396a8fb..cf427431bb2 100644 --- a/tests/main/snap-disconnect/task.yaml +++ b/tests/main/snap-disconnect/task.yaml @@ -35,3 +35,8 @@ execute: | 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" From 3e9348a1d8fb2d8d13b73a91f45908cb28826c70 Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Fri, 30 Sep 2016 14:40:36 +0200 Subject: [PATCH 11/29] Fixes for special cases --- interfaces/repo.go | 8 +++++++- interfaces/repo_test.go | 6 +++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index e26b7d52ccf..59de232daac 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -338,7 +338,7 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s // Disconnect everything from slotSnapName conns, snaps, err = r.disconnectEverythingFromSnap(slotSnapName) case plugSnapName == "" && plugName == "": - // Disconnect everything from slotSnapName:slotName + // Disconnect everything from either slot or plug conns, snaps, err = r.disconnectPlugOrSlot(slotSnapName, slotName) default: conns, snaps, err = r.disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName) @@ -387,6 +387,12 @@ func (r *Repository) disconnectPlugOrSlot(snapName, plugOrSlotName string) ([]Co // TODO: teach the repository about the OS snap snapName = "ubuntu-core" } + + if r.slots[snapName][plugOrSlotName] == nil && r.plugs[snapName][plugOrSlotName] == nil { + err := fmt.Errorf("cannot disconnect plug or slot %q from snap %q, no such plug/slot", plugOrSlotName, snapName) + return nil, nil, err + } + var conns []ConnRef snaps := make(map[string]*snap.Info) d := func(plug *Plug, slot *Slot) { diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index cc50d0c22d6..8a34a4aac5b 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -613,7 +613,7 @@ func (s *RepositorySuite) TestDisconnectFromSlotFailsWhenSlotDoesNotExist(c *C) c.Assert(err, IsNil) // Disconnecting everything form an unknown slot returns an appropriate error conns, snaps, 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`) + c.Assert(err, ErrorMatches, `cannot disconnect plug or slot \"slot\" from snap \"producer\", no such plug/slot`) c.Check(conns, IsNil) c.Check(snaps, IsNil) } @@ -749,7 +749,7 @@ func (s *RepositorySuite) TestDisconnectPlugOrSlot(c *C) { c.Assert(err, IsNil) // slot and snap name omitted - conns, snaps, err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, "", "") + conns, snaps, err := s.testRepo.Disconnect("", "", s.plug.Snap.Name(), s.plug.Name) c.Assert(err, IsNil) c.Assert(conns, HasLen, 1) c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: coreSlot.Ref()}}) @@ -764,7 +764,7 @@ func (s *RepositorySuite) TestDisconnectPlugOrSlot(c *C) { c.Assert(err, IsNil) // plug and snap name omitted - conns, snaps, err = s.testRepo.Disconnect(coreSlot.Snap.Name(), coreSlot.Name, "", "") + conns, snaps, err = s.testRepo.Disconnect("", "", coreSlot.Snap.Name(), coreSlot.Name) c.Assert(err, IsNil) c.Assert(conns, HasLen, 1) c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: coreSlot.Ref()}}) From a6f36a82be140908c56f9b25d235ccfe7790669b Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Fri, 30 Sep 2016 14:54:02 +0200 Subject: [PATCH 12/29] Fix help --- cmd/snap/cmd_disconnect.go | 4 ++++ cmd/snap/cmd_disconnect_test.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/cmd/snap/cmd_disconnect.go b/cmd/snap/cmd_disconnect.go index ed01d42c5fa..4395d38a85f 100644 --- a/cmd/snap/cmd_disconnect.go +++ b/cmd/snap/cmd_disconnect.go @@ -45,6 +45,10 @@ $ snap disconnect : Disconnects everything from the provided plug or slot. The snap name may be omitted for the OS snap. + +$ snap disconnect + +Disconnects all plugs from the provided snap. `) func init() { diff --git a/cmd/snap/cmd_disconnect_test.go b/cmd/snap/cmd_disconnect_test.go index 073320c8c45..37687a9ef45 100644 --- a/cmd/snap/cmd_disconnect_test.go +++ b/cmd/snap/cmd_disconnect_test.go @@ -44,6 +44,10 @@ $ snap disconnect : Disconnects everything from the provided plug or slot. The snap name may be omitted for the OS snap. +$ snap disconnect + +Disconnects all plugs from the provided snap. + Application Options: --version Print the version and exit From 6eefa28e2a81078fa9e84ac9997d15e22c091a60 Mon Sep 17 00:00:00 2001 From: Pawel Stolowski Date: Fri, 30 Sep 2016 19:20:45 +0200 Subject: [PATCH 13/29] Removed support for disconnect --- cmd/snap/cmd_disconnect.go | 4 --- cmd/snap/cmd_disconnect_test.go | 4 --- interfaces/repo.go | 25 ---------------- interfaces/repo_test.go | 43 ---------------------------- overlord/ifacestate/ifacemgr_test.go | 5 ---- tests/main/snap-disconnect/task.yaml | 5 ---- 6 files changed, 86 deletions(-) diff --git a/cmd/snap/cmd_disconnect.go b/cmd/snap/cmd_disconnect.go index 4395d38a85f..ed01d42c5fa 100644 --- a/cmd/snap/cmd_disconnect.go +++ b/cmd/snap/cmd_disconnect.go @@ -45,10 +45,6 @@ $ snap disconnect : Disconnects everything from the provided plug or slot. The snap name may be omitted for the OS snap. - -$ snap disconnect - -Disconnects all plugs from the provided snap. `) func init() { diff --git a/cmd/snap/cmd_disconnect_test.go b/cmd/snap/cmd_disconnect_test.go index 37687a9ef45..073320c8c45 100644 --- a/cmd/snap/cmd_disconnect_test.go +++ b/cmd/snap/cmd_disconnect_test.go @@ -44,10 +44,6 @@ $ snap disconnect : Disconnects everything from the provided plug or slot. The snap name may be omitted for the OS snap. -$ snap disconnect - -Disconnects all plugs from the provided snap. - Application Options: --version Print the version and exit diff --git a/interfaces/repo.go b/interfaces/repo.go index 59de232daac..cea4183004a 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -334,9 +334,6 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s var err error switch { - case plugSnapName == "" && plugName == "" && slotName == "": - // Disconnect everything from slotSnapName - conns, snaps, err = r.disconnectEverythingFromSnap(slotSnapName) case plugSnapName == "" && plugName == "": // Disconnect everything from either slot or plug conns, snaps, err = r.disconnectPlugOrSlot(slotSnapName, slotName) @@ -360,28 +357,6 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s return conns, result, nil } -// disconnectEverythingFromSnap finds a specific snap and disconnects all the plugs connected to all the slots therein. -func (r *Repository) disconnectEverythingFromSnap(slotSnapName string) ([]ConnRef, map[string]*snap.Info, error) { - if _, ok := r.slots[slotSnapName]; !ok { - err := fmt.Errorf("cannot disconnect plug from snap %q, no such snap", slotSnapName) - return nil, nil, err - } - if _, ok := r.slots[slotSnapName]; !ok { - return nil, nil, nil - } - var conns []ConnRef - snaps := make(map[string]*snap.Info) - for _, slot := range r.slots[slotSnapName] { - for plug := range r.slotPlugs[slot] { - r.disconnect(plug, slot) - snaps[plug.Snap.Name()] = plug.Snap - snaps[slot.Snap.Name()] = slot.Snap - conns = append(conns, ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}) - } - } - return conns, snaps, nil -} - func (r *Repository) disconnectPlugOrSlot(snapName, plugOrSlotName string) ([]ConnRef, map[string]*snap.Info, error) { if snapName == "" { // TODO: teach the repository about the OS snap diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index 8a34a4aac5b..97613182c26 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -618,16 +618,6 @@ func (s *RepositorySuite) TestDisconnectFromSlotFailsWhenSlotDoesNotExist(c *C) c.Check(snaps, IsNil) } -func (s *RepositorySuite) TestDisconnectFromSnapFailsWhenSlotDoesNotExist(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - // Disconnecting all plugs from a snap that is not known returns an appropriate error - conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") - c.Assert(err, ErrorMatches, `cannot disconnect plug from snap "producer", no such snap`) - c.Check(conns, IsNil) - c.Check(snaps, IsNil) -} - func (s *RepositorySuite) TestDisconnectFailsWhenNotConnected(c *C) { err := s.testRepo.AddPlug(s.plug) c.Assert(err, IsNil) @@ -640,18 +630,6 @@ func (s *RepositorySuite) TestDisconnectFailsWhenNotConnected(c *C) { c.Check(snaps, IsNil) } -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. - conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") - c.Assert(err, IsNil) - c.Check(conns, HasLen, 0) - c.Check(snaps, HasLen, 0) -} - func (s *RepositorySuite) TestDisconnectFromSlotDoesNothingWhenNotConnected(c *C) { err := s.testRepo.AddPlug(s.plug) c.Assert(err, IsNil) @@ -685,27 +663,6 @@ func (s *RepositorySuite) TestDisconnectSucceeds(c *C) { }) } -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) - err = s.testRepo.Connect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, IsNil) - // Disconnecting everything from a snap works OK - conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), "") - c.Assert(err, IsNil) - c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) - c.Assert(snaps, HasLen, 2) - // beware, the snaps are sorted by name - c.Check(snaps[0], Equals, s.plug.Snap) - c.Check(snaps[1], Equals, s.slot.Snap) - c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ - Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, - Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, - }) -} - func (s *RepositorySuite) TestDisconnectFromSlot(c *C) { err := s.testRepo.AddPlug(s.plug) c.Assert(err, IsNil) diff --git a/overlord/ifacestate/ifacemgr_test.go b/overlord/ifacestate/ifacemgr_test.go index 98097fb112a..548c6e4529c 100644 --- a/overlord/ifacestate/ifacemgr_test.go +++ b/overlord/ifacestate/ifacemgr_test.go @@ -170,11 +170,6 @@ func (s *interfaceManagerSuite) TestDisconnectSlot(c *C) { s.testDisconnect(c, "", "", "producer", "slot") } -// Disconnect works when only the snap from the slot is specified. -func (s *interfaceManagerSuite) TestDisconnectSlotSnap(c *C) { - s.testDisconnect(c, "", "", "producer", "") -} - 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. diff --git a/tests/main/snap-disconnect/task.yaml b/tests/main/snap-disconnect/task.yaml index cf427431bb2..6f5ca28a9ec 100644 --- a/tests/main/snap-disconnect/task.yaml +++ b/tests/main/snap-disconnect/task.yaml @@ -16,11 +16,6 @@ restore: | execute: | DISCONNECTED_PATTERN="\-\s+home-consumer:home" - echo "Disconnect everything connected to given snap" - snap connect home-consumer:home ubuntu-core:home - snap disconnect ubuntu-core - snap interfaces | grep -Pzqe "$DISCONNECTED_PATTERN" - echo "Disconnect everything from given slot" snap connect home-consumer:home ubuntu-core:home snap disconnect ubuntu-core:home From f6101ebfdae97f503d74dbcc3f046e04122bf63b Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 3 Oct 2016 17:51:41 +0200 Subject: [PATCH 14/29] WIP --- interfaces/repo.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/interfaces/repo.go b/interfaces/repo.go index cea4183004a..1058e2993d0 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -357,6 +357,57 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s return conns, result, nil } +// DisconnectAll disconnects everything from a given snap and plug or slot. +func (r *Repository) DisconnectAll(snapName, plugOrSlotName string) (severed []ConnRef, affectedSnaps []string, _ error) { + r.m.Lock() + defer r.m.Unlock() + + var conns []ConnRef + var snaps map[string]*snap.Info + var err error + + if snapName == "" { + // TODO: teach the repository about the OS snap + snapName = "ubuntu-core" + } + + var conns []ConnRef + snaps := make(map[string]bool) + d := func(plug *Plug, slot *Slot) { + r.disconnect(plug, slot) + snaps[plug.Snap.Name()] = true + snaps[slot.Snap.Name()] = true + conns = append(conns, ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}) + } + if slot := r.slots[snapName][plugOrSlotName]; slot != nil { + for plug := range r.slotPlugs[slot] { + d(plug, slot) + } + } + if plug := r.plugs[snapName][plugOrSlotName]; plug != nil { + for slot := range r.plugSlots[plug] { + d(plug, slot) + } + } + return conns, snaps, nil + + + if err != nil { + return nil, nil, err + } + // Flatten map of snaps into a sorted list + names := make([]string, 0, len(snaps)) + for snapName := range snaps { + names = append(names, snapName) + } + sort.Strings(names) + result := make([]*snap.Info, 0, len(snaps)) + for _, snapName := range names { + result = append(result, snaps[snapName]) + } + return conns, affected, nil +} + func (r *Repository) disconnectPlugOrSlot(snapName, plugOrSlotName string) ([]ConnRef, map[string]*snap.Info, error) { if snapName == "" { // TODO: teach the repository about the OS snap From 1ba25f263d5fe568c9e6958c9f9be0f3a954f830 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 3 Oct 2016 19:39:46 +0200 Subject: [PATCH 15/29] interfaces: add ResolveDisconnectAll and DisconnectAll This patch adds a new pair of methods, ResolveDisconnectAll and DisconnectAll that respectively look up a list of connections to sever and disconect a list of connections. DisconnectAll returns a list of names of snaps that were affected by the operation. Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 93 ++++++++++++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 31 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index 1058e2993d0..3337d5df172 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -357,55 +357,86 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s return conns, result, nil } -// DisconnectAll disconnects everything from a given snap and plug or slot. -func (r *Repository) DisconnectAll(snapName, plugOrSlotName string) (severed []ConnRef, affectedSnaps []string, _ error) { +// ResolveDisconnectAll finds all of the connections that would be disconnected by DisconnectAll() +func (r *Repository) ResolveDisconnectAll(snapName, plugOrSlotName string) ([]ConnRef, error) { r.m.Lock() defer r.m.Unlock() - var conns []ConnRef - var snaps map[string]*snap.Info - var err error + return r.unlockedResolveDisconnectAll(snapName, plugOrSlotName) +} +func (r *Repository) unlockedResolveDisconnectAll(snapName, plugOrSlotName string) ([]ConnRef, error) { if snapName == "" { - // TODO: teach the repository about the OS snap - snapName = "ubuntu-core" + // 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 - snaps := make(map[string]bool) - d := func(plug *Plug, slot *Slot) { - r.disconnect(plug, slot) - snaps[plug.Snap.Name()] = true - snaps[slot.Snap.Name()] = true - conns = append(conns, ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}) - } - if slot := r.slots[snapName][plugOrSlotName]; slot != nil { - for plug := range r.slotPlugs[slot] { - d(plug, slot) + if plugOrSlotName == "" { + // "wildcard" mode, affect all the plugs or slots in this snap + for _, plug := range r.plugs[snapName] { + for _, slotRef := range plug.Connections { + connRef := ConnRef{PlugRef: plug.Ref(), SlotRef: slotRef} + conns = append(conns, connRef) + } } - } - if plug := r.plugs[snapName][plugOrSlotName]; plug != nil { - for slot := range r.plugSlots[plug] { - d(plug, slot) + for _, slot := range r.slots[snapName] { + for _, plugRef := range slot.Connections { + connRef := ConnRef{PlugRef: plugRef, SlotRef: slot.Ref()} + conns = append(conns, connRef) + } + } + } else { + // Precise mode, affect specific plug or slot in this snap + 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) + } + } + // Check if plugOrSlotName actually maps to anything + if len(conns) == 0 { + return nil, fmt.Errorf("cannot resolve disconnect, snap %q does not have plug or slot named %q", snapName, plugOrSlotName) } } - return conns, snaps, nil + return conns, nil +} +// DisconnectAll disconnects all of the given connections, returning the list of affected snap names. +func (r *Repository) DisconnectAll(conns []ConnRef) []string { + r.m.Lock() + defer r.m.Unlock() - if err != nil { - return nil, nil, err + // Sever all the connections, keeping track of affected snaps + snaps := make(map[string]bool) + 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 { + snaps[plug.Snap.Name()] = true + snaps[slot.Snap.Name()] = true + r.disconnect(plug, slot) + } } - // Flatten map of snaps into a sorted list + // Flatten map of affected snaps into a sorted list names := make([]string, 0, len(snaps)) for snapName := range snaps { names = append(names, snapName) } sort.Strings(names) - result := make([]*snap.Info, 0, len(snaps)) - for _, snapName := range names { - result = append(result, snaps[snapName]) - } - return conns, affected, nil + return names } func (r *Repository) disconnectPlugOrSlot(snapName, plugOrSlotName string) ([]ConnRef, map[string]*snap.Info, error) { From 853b2276c5914ca7fa48a8b44113b72c618ef3f6 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 3 Oct 2016 20:38:10 +0200 Subject: [PATCH 16/29] interfaces: tweak error message in ResolveDisconnectAll Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index 4c42cbcb896..2e3c15014f9 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -413,7 +413,7 @@ func (r *Repository) unlockedResolveDisconnectAll(snapName, plugOrSlotName strin } // Check if plugOrSlotName actually maps to anything if len(conns) == 0 { - return nil, fmt.Errorf("cannot resolve disconnect, snap %q does not have plug or slot named %q", snapName, plugOrSlotName) + return nil, fmt.Errorf("snap %q has no plug or slot name %q", snapName, plugOrSlotName) } } return conns, nil From fd17380a06b78a02a63d3024b4fd84a7b479f628 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Mon, 3 Oct 2016 20:42:21 +0200 Subject: [PATCH 17/29] interfaces: fix typo Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index 2e3c15014f9..a728a719192 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -413,7 +413,7 @@ func (r *Repository) unlockedResolveDisconnectAll(snapName, plugOrSlotName strin } // Check if plugOrSlotName actually maps to anything if len(conns) == 0 { - return nil, fmt.Errorf("snap %q has no plug or slot name %q", snapName, plugOrSlotName) + return nil, fmt.Errorf("snap %q has no plug or slot named %q", snapName, plugOrSlotName) } } return conns, nil From fc5233a5cabe1be7c1d7bad58787bb7bfab4edd6 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 11:47:51 +0200 Subject: [PATCH 18/29] interfaces: drop unlocked version of ResolveDisconnectAll Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index c9e74449b5a..97658d44a0b 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -434,10 +434,6 @@ func (r *Repository) ResolveDisconnectAll(snapName, plugOrSlotName string) ([]Co r.m.Lock() defer r.m.Unlock() - return r.unlockedResolveDisconnectAll(snapName, plugOrSlotName) -} - -func (r *Repository) unlockedResolveDisconnectAll(snapName, plugOrSlotName string) ([]ConnRef, error) { if snapName == "" { // Look up the core snap if no snap name was given switch { From 535459503c3aa33a388d2053176b39fb2605be59 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 11:52:18 +0200 Subject: [PATCH 19/29] interfaces: don't return anything from DisconnctAll Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index 97658d44a0b..06b06a11f93 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -483,28 +483,17 @@ func (r *Repository) ResolveDisconnectAll(snapName, plugOrSlotName string) ([]Co } // DisconnectAll disconnects all of the given connections, returning the list of affected snap names. -func (r *Repository) DisconnectAll(conns []ConnRef) []string { +func (r *Repository) DisconnectAll(conns []ConnRef) { r.m.Lock() defer r.m.Unlock() - // Sever all the connections, keeping track of affected snaps - snaps := make(map[string]bool) 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 { - snaps[plug.Snap.Name()] = true - snaps[slot.Snap.Name()] = true r.disconnect(plug, slot) } } - // Flatten map of affected snaps into a sorted list - names := make([]string, 0, len(snaps)) - for snapName := range snaps { - names = append(names, snapName) - } - sort.Strings(names) - return names } func (r *Repository) disconnectPlugOrSlot(snapName, plugOrSlotName string) ([]ConnRef, map[string]*snap.Info, error) { From d496ce02601cfa9a741efd112ef45ed8acfe1a5f Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 12:28:04 +0200 Subject: [PATCH 20/29] interfaces: tweak documentation Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index 06b06a11f93..afc2571d0b2 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -429,8 +429,9 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s return conns, result, nil } -// ResolveDisconnectAll finds all of the connections that would be disconnected by DisconnectAll() -func (r *Repository) ResolveDisconnectAll(snapName, plugOrSlotName string) ([]ConnRef, error) { +// Connected returns references for all connections that are currently +// established with the provided plug or slot. +func (r *Repository) Connecte(snapName, plugOrSlotName string) ([]ConnRef, error) { r.m.Lock() defer r.m.Unlock() @@ -482,7 +483,7 @@ func (r *Repository) ResolveDisconnectAll(snapName, plugOrSlotName string) ([]Co return conns, nil } -// DisconnectAll disconnects all of the given connections, returning the list of affected snap names. +// DisconnectAll disconnects all provided connection references. func (r *Repository) DisconnectAll(conns []ConnRef) { r.m.Lock() defer r.m.Unlock() From dc380e4dd834c4fd8e9460b92e31c37ab7e3fd01 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 12:29:33 +0200 Subject: [PATCH 21/29] interfaces: fix typo Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index afc2571d0b2..6ce02cbfa4d 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -431,7 +431,7 @@ func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName s // Connected returns references for all connections that are currently // established with the provided plug or slot. -func (r *Repository) Connecte(snapName, plugOrSlotName string) ([]ConnRef, error) { +func (r *Repository) Connected(snapName, plugOrSlotName string) ([]ConnRef, error) { r.m.Lock() defer r.m.Unlock() From 1c0ff1ff6c39138e20143d0257699c40c99a5747 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 12:30:07 +0200 Subject: [PATCH 22/29] interfaces: drop wildcard mode on repo.Connected Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index 6ce02cbfa4d..9599eb5db30 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -448,38 +448,25 @@ func (r *Repository) Connected(snapName, plugOrSlotName string) ([]ConnRef, erro } var conns []ConnRef if plugOrSlotName == "" { - // "wildcard" mode, affect all the plugs or slots in this snap - for _, plug := range r.plugs[snapName] { - for _, slotRef := range plug.Connections { - connRef := ConnRef{PlugRef: plug.Ref(), SlotRef: slotRef} - conns = append(conns, connRef) - } - } - for _, slot := range r.slots[snapName] { - for _, plugRef := range slot.Connections { - connRef := ConnRef{PlugRef: plugRef, SlotRef: slot.Ref()} - conns = append(conns, connRef) - } - } - } else { - // Precise mode, affect specific plug or slot in this snap - 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 nil, fmt.Errorf("cannot resolve disconnect, plug or slot name is empty") + } + // Precise mode, affect specific plug or slot in this snap + if plug, ok := r.plugs[snapName][plugOrSlotName]; ok { + for _, slotRef := range plug.Connections { + connRef := ConnRef{PlugRef: plug.Ref(), SlotRef: slotRef} + conns = append(conns, connRef) } - // Check if plugOrSlotName actually maps to anything - if len(conns) == 0 { - return nil, fmt.Errorf("snap %q has no plug or slot named %q", snapName, plugOrSlotName) + } + if slot, ok := r.slots[snapName][plugOrSlotName]; ok { + for _, plugRef := range slot.Connections { + connRef := ConnRef{PlugRef: plugRef, SlotRef: slot.Ref()} + conns = append(conns, connRef) } } + // Check if plugOrSlotName actually maps to anything + if len(conns) == 0 { + return nil, fmt.Errorf("snap %q has no plug or slot named %q", snapName, plugOrSlotName) + } return conns, nil } From 1a1f189192da4c5f81c97968d2a161ce25014a62 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 12:35:00 +0200 Subject: [PATCH 23/29] interfaces: tweak error test Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index 9599eb5db30..eb75096b398 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -450,7 +450,11 @@ func (r *Repository) Connected(snapName, plugOrSlotName string) ([]ConnRef, erro if plugOrSlotName == "" { return nil, fmt.Errorf("cannot resolve disconnect, plug or slot name is empty") } - // Precise mode, affect specific plug or slot in this snap + // 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} @@ -463,10 +467,6 @@ func (r *Repository) Connected(snapName, plugOrSlotName string) ([]ConnRef, erro conns = append(conns, connRef) } } - // Check if plugOrSlotName actually maps to anything - if len(conns) == 0 { - return nil, fmt.Errorf("snap %q has no plug or slot named %q", snapName, plugOrSlotName) - } return conns, nil } From 9bb858f8a56106df1c2639017aa56592db56c570 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 13:40:36 +0200 Subject: [PATCH 24/29] interfaces: simplify and tweak disconnect methods and tests Signed-off-by: Zygmunt Krynicki --- interfaces/repo.go | 131 ++++++------------------ interfaces/repo_test.go | 216 +++++++++++++++++----------------------- 2 files changed, 124 insertions(+), 223 deletions(-) diff --git a/interfaces/repo.go b/interfaces/repo.go index eb75096b398..6060dca81e6 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -379,54 +379,44 @@ 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 returns the list of severed connections, the list of affected snaps -// and an error if one occurred. -func (r *Repository) Disconnect(plugSnapName, plugName, slotSnapName, slotName string) ([]ConnRef, []*snap.Info, error) { +// 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() - var conns []ConnRef - var snaps map[string]*snap.Info - var err error - - switch { - case plugSnapName == "" && plugName == "": - // Disconnect everything from either slot or plug - conns, snaps, err = r.disconnectPlugOrSlot(slotSnapName, slotName) - default: - conns, snaps, err = r.disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName) + // Sanity check + if plugSnapName == "" { + return fmt.Errorf("cannot disconnect, plug snap name is empty") + } + if plugName == "" { + return fmt.Errorf("cannot disconnect, plug name is empty") + } + if slotSnapName == "" { + return fmt.Errorf("cannot disconnect, slot snap name is empty") + } + if slotName == "" { + return fmt.Errorf("cannot disconnect, slot name is empty") } - if err != nil { - return nil, nil, err + // Ensure that such plug exists + plug := r.plugs[plugSnapName][plugName] + if plug == nil { + return fmt.Errorf("snap %q has no plug named %q", plugSnapName, plugName) } - // Flatten map of snaps into a sorted list - names := make([]string, 0, len(snaps)) - for snapName := range snaps { - names = append(names, snapName) + // Ensure that such slot exists + slot := r.slots[slotSnapName][slotName] + if slot == nil { + return fmt.Errorf("snap %q has no slot named %q", slotSnapName, slotName) } - sort.Strings(names) - result := make([]*snap.Info, 0, len(snaps)) - for _, snapName := range names { - result = append(result, snaps[snapName]) + // Ensure that slot and plug are connected + if !r.slotPlugs[slot][plug] { + return fmt.Errorf("cannot disconnect %s:%s from %s:%s, it is not connected", + plugSnapName, plugName, slotSnapName, slotName) } - return conns, result, nil + r.disconnect(plug, slot) + return nil } // Connected returns references for all connections that are currently @@ -484,67 +474,6 @@ func (r *Repository) DisconnectAll(conns []ConnRef) { } } -func (r *Repository) disconnectPlugOrSlot(snapName, plugOrSlotName string) ([]ConnRef, map[string]*snap.Info, error) { - if snapName == "" { - // TODO: teach the repository about the OS snap - snapName = "ubuntu-core" - } - - if r.slots[snapName][plugOrSlotName] == nil && r.plugs[snapName][plugOrSlotName] == nil { - err := fmt.Errorf("cannot disconnect plug or slot %q from snap %q, no such plug/slot", plugOrSlotName, snapName) - return nil, nil, err - } - - var conns []ConnRef - snaps := make(map[string]*snap.Info) - d := func(plug *Plug, slot *Slot) { - r.disconnect(plug, slot) - snaps[plug.Snap.Name()] = plug.Snap - snaps[slot.Snap.Name()] = slot.Snap - conns = append(conns, ConnRef{PlugRef: plug.Ref(), SlotRef: slot.Ref()}) - } - if slot := r.slots[snapName][plugOrSlotName]; slot != nil { - for plug := range r.slotPlugs[slot] { - d(plug, slot) - } - } - if plug := r.plugs[snapName][plugOrSlotName]; plug != nil { - for slot := range r.plugSlots[plug] { - d(plug, slot) - } - } - return conns, snaps, nil -} - -// disconnectPlugFromSlot finds a specific plug slot and plug and disconnects it. -func (r *Repository) disconnectPlugFromSlot(plugSnapName, plugName, slotSnapName, slotName string) ([]ConnRef, map[string]*snap.Info, error) { - // Ensure that such plug exists - plug := r.plugs[plugSnapName][plugName] - if plug == nil { - err := fmt.Errorf("cannot disconnect plug %q from snap %q, no such plug", plugName, plugSnapName) - return nil, nil, err - } - // Ensure that such slot exists - slot := r.slots[slotSnapName][slotName] - if slot == nil { - err := fmt.Errorf("cannot disconnect plug from slot %q from snap %q, no such slot", slotName, slotSnapName) - return nil, nil, err - } - // Ensure that slot and plug are connected - if !r.slotPlugs[slot][plug] { - err := fmt.Errorf("cannot disconnect plug %q from snap %q from slot %q from snap %q, it is not connected", - plugName, plugSnapName, slotName, slotSnapName) - return nil, nil, err - } - r.disconnect(plug, slot) - conns := []ConnRef{{PlugRef: plug.Ref(), SlotRef: slot.Ref()}} - snaps := map[string]*snap.Info{ - plug.Snap.Name(): plug.Snap, - slot.Snap.Name(): slot.Snap, - } - return conns, snaps, nil -} - // 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 7b200917830..c1f20128bf9 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -768,152 +768,129 @@ 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 - snaps, conns, 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`) - c.Check(snaps, IsNil) - c.Check(conns, IsNil) -} - -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 - conns, snaps, 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`) - c.Check(snaps, IsNil) - c.Check(conns, IsNil) +// 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) TestDisconnectFromSlotFailsWhenSlotDoesNotExist(c *C) { - err := s.testRepo.AddPlug(s.plug) - c.Assert(err, IsNil) - // Disconnecting everything form an unknown slot returns an appropriate error - conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, ErrorMatches, `cannot disconnect plug or slot \"slot\" from snap \"producer\", no such plug/slot`) - c.Check(conns, IsNil) - c.Check(snaps, IsNil) +// 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"`) } +// Disconnect fails if there's no connection to disconnect 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 - conns, snaps, 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`) - c.Check(conns, IsNil) - c.Check(snaps, IsNil) -} - -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. - conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) - c.Assert(err, IsNil) - c.Check(conns, HasLen, 0) - c.Check(snaps, HasLen, 0) + 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`) } +// Disconnect works when plug and slot exist and are connected 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) - c.Assert(err, IsNil) - // Disconnecting a connected plug works okay - conns, snaps, err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) + 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) - c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) - c.Assert(snaps, HasLen, 2) - // beware, the snaps are sorted by name - c.Check(snaps[0], Equals, s.plug.Snap) - c.Check(snaps[1], Equals, s.slot.Snap) c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, }) } -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) +// Tests for Repository.Connected - // Disconnecting everything from a slot works OK - conns, snaps, err := s.testRepo.Disconnect("", "", s.slot.Snap.Name(), s.slot.Name) +// 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`) +} - c.Assert(err, IsNil) - c.Assert(conns, HasLen, 1) - c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) - c.Assert(snaps, HasLen, 2) - // beware, the snaps are sorted by name - c.Check(snaps[0], Equals, s.plug.Snap) - c.Check(snaps[1], Equals, s.slot.Snap) - c.Assert(s.testRepo.Interfaces(), DeepEquals, &Interfaces{ - Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, - Slots: []*Slot{{SlotInfo: s.slot.SlotInfo}}, - }) +// 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) TestDisconnectPlugOrSlot(c *C) { - err := s.testRepo.AddPlug(s.plug) +// 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"`) +} + +// 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) - err = s.testRepo.AddSlot(s.slot) + 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) - coreSlot := &Slot{ + c.Check(conns, DeepEquals, []ConnRef{ + {s.plug.Ref(), s.slot.Ref()}, + }) +} + +// 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: "foo-core", Type: snap.TypeOS}, - Name: "foo-slot", + Snap: &snap.Info{SuggestedName: "core", Type: snap.TypeOS}, + Name: "slot", Interface: "interface", }, } - err = s.testRepo.AddSlot(coreSlot) - - connRef := ConnRef{PlugRef: s.plug.Ref(), SlotRef: coreSlot.Ref()} - err = s.testRepo.Connect(connRef) - c.Assert(err, IsNil) + 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) - // slot and snap name omitted - conns, snaps, err := s.testRepo.Disconnect("", "", s.plug.Snap.Name(), s.plug.Name) + conns, err := s.testRepo.Connected("", s.slot.Name) c.Assert(err, IsNil) - c.Assert(conns, HasLen, 1) - c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: coreSlot.Ref()}}) + c.Check(conns, DeepEquals, []ConnRef{ + {s.plug.Ref(), slot.Ref()}, + }) +} - c.Assert(snaps, HasLen, 2) - // beware, the snaps are sorted by name - c.Check(snaps[0], Equals, s.plug.Snap) - c.Check(snaps[1], Equals, coreSlot.Snap) +// Tests for Repository.DisconnectAll() - // connect again - err = s.testRepo.Connect(connRef) - c.Assert(err, IsNil) - - // plug and snap name omitted - conns, snaps, err = s.testRepo.Disconnect("", "", coreSlot.Snap.Name(), coreSlot.Name) - c.Assert(err, IsNil) - c.Assert(conns, HasLen, 1) - c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: coreSlot.Ref()}}) +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) - c.Assert(snaps, HasLen, 2) - // beware, the snaps are sorted by name - c.Check(snaps[0], Equals, s.plug.Snap) - c.Check(snaps[1], Equals, coreSlot.Snap) + 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}}, + }) } // Tests for Repository.Interfaces() @@ -939,13 +916,8 @@ func (s *RepositorySuite) TestInterfacesSmokeTest(c *C) { }}, }) // After disconnecting the connections become empty - conns, snaps, err := s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) + err = s.testRepo.Disconnect(s.plug.Snap.Name(), s.plug.Name, s.slot.Snap.Name(), s.slot.Name) c.Assert(err, IsNil) - c.Check(conns, DeepEquals, []ConnRef{{PlugRef: s.plug.Ref(), SlotRef: s.slot.Ref()}}) - c.Assert(snaps, HasLen, 2) - // beware, the snaps are sorted by name - c.Check(snaps[0], Equals, s.plug.Snap) - c.Check(snaps[1], Equals, s.slot.Snap) ifaces = s.testRepo.Interfaces() c.Assert(ifaces, DeepEquals, &Interfaces{ Plugs: []*Plug{{PlugInfo: s.plug.PlugInfo}}, From e34153dc12c45e4d37d281613414b4a1e5fe4b88 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 14:05:05 +0200 Subject: [PATCH 25/29] overlord/interfaces: use new Disconnect/DisconnectAll This patch changes the interface manager to use the new APIs offered by interfaces.Repository for disconnecting. In particular we no longer obtain snap.Info of the affected snaps from the repository (which can contain stale data). Instead we use the snapstate to fetch the current info). In addition of the fully-spelled-out "disconnect snap:plug snap:slot" we now correctly support "disconnect snap:plug" or "disconnect snap:slot". In those two new forms the snap name can be empty (it then defaults to the core snap). This is thanks to the use of the new DisconnectAll method. Signed-off-by: Zygmunt Krynicki --- overlord/ifacestate/handlers.go | 51 +++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go index 295fa68fc8b..52c2c3220fa 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,23 +342,41 @@ func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error { return err } - affectedConns, affectedSnaps, err := m.repo.Disconnect( - plugRef.Snap, plugRef.Name, slotRef.Snap, slotRef.Name) - if 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 != "" { + affectedConns, err = m.repo.Connected(plugRef.Snap, plugRef.Name) + if err != nil { + return err + } + m.repo.DisconnectAll(affectedConns) + } else { + affectedConns, err = m.repo.Connected(slotRef.Snap, slotRef.Name) + if err != nil { + return err + } + m.repo.DisconnectAll(affectedConns) } - - for _, snapInfo := range affectedSnaps { + affectedSnaps := snapNamesFromConns(affectedConns) + for _, snapName := range affectedSnaps { var snapst snapstate.SnapState - if err := snapstate.Get(st, snapInfo.Name(), &snapst); err != nil { + 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{} } } - for _, connRef := range affectedConns { - delete(conns, connRef.ID()) + for _, conn := range affectedConns { + delete(conns, conn.ID()) } setConns(st, conns) From 3f884924fb61a4862523c1a14c1b49c94cdf34b1 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 14:11:00 +0200 Subject: [PATCH 26/29] overlord/ifacestate: add more disconnect tests Signed-off-by: Zygmunt Krynicki --- overlord/ifacestate/ifacemgr_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/overlord/ifacestate/ifacemgr_test.go b/overlord/ifacestate/ifacemgr_test.go index c0a349549f9..a225159d659 100644 --- a/overlord/ifacestate/ifacemgr_test.go +++ b/overlord/ifacestate/ifacemgr_test.go @@ -370,16 +370,21 @@ func (s *interfaceManagerSuite) TestDisconnectTask(c *C) { c.Assert(slot.Name, Equals, "slot") } -// Disconnect works when used with the fully-spelled-out form. +// 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 only the slot is fully specified. +// 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. From f653e81e6d16cd6a9c61f4ff7631c3f5478b5f66 Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 14:22:19 +0200 Subject: [PATCH 27/29] daemon: adjust tests after message changes Signed-off-by: Zygmunt Krynicki --- daemon/api_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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") From 3a3bc021bf93f6084ffbd186ddc8e143c3c9a03b Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 18:07:44 +0200 Subject: [PATCH 28/29] interfaces: be extra paranoid about disconnect dispatch Signed-off-by: Zygmunt Krynicki --- overlord/ifacestate/handlers.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go index 52c2c3220fa..6788a5e12d9 100644 --- a/overlord/ifacestate/handlers.go +++ b/overlord/ifacestate/handlers.go @@ -348,18 +348,22 @@ func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error { return err } affectedConns = []interfaces.ConnRef{{*plugRef, *slotRef}} - } else if plugRef.Name != "" { + } 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 { + } 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) } affectedSnaps := snapNamesFromConns(affectedConns) for _, snapName := range affectedSnaps { From 98e974cd467e0eb7150c7db35c7b27fff3877f1e Mon Sep 17 00:00:00 2001 From: Zygmunt Krynicki Date: Tue, 4 Oct 2016 18:10:57 +0200 Subject: [PATCH 29/29] overlord/ifacestate: return {Plug,Slot}Ref by value Signed-off-by: Zygmunt Krynicki --- overlord/ifacestate/handlers.go | 2 +- overlord/ifacestate/helpers.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go index 6788a5e12d9..0b86dc70769 100644 --- a/overlord/ifacestate/handlers.go +++ b/overlord/ifacestate/handlers.go @@ -347,7 +347,7 @@ func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error { if err := m.repo.Disconnect(plugRef.Snap, plugRef.Name, slotRef.Snap, slotRef.Name); err != nil { return err } - affectedConns = []interfaces.ConnRef{{*plugRef, *slotRef}} + 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) 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) {