overlord/ifacestate: use state as source of truth in interface handlers

overlord/ifacestate/handlers.go

@@ -125,6 +125,12 @@ func (m *InterfaceManager) setupAffectedSnaps(task *state.Task, affectingSnap st
 			task.Errorf("skipping security profiles setup for snap %q when handling snap %q: %v", affectedInstanceName, affectingSnap, err)
 			continue
 		}
+		// For profiles update to be meaningful the snap has to be active, or
+		// have a PendingSecurity.SideInfo set. In all other cases, the snap
+		// has no profiles and thus nothing needing an update.
+		if !snapst.Active && (snapst.PendingSecurity == nil || snapst.PendingSecurity.SideInfo == nil) {
+			continue
+		}
 		affectedSnapInfo, err := snapst.CurrentInfo()
 		if err != nil {
 			return err
@@ -626,14 +632,39 @@ func (m *InterfaceManager) doRemoveProfiles(task *state.Task, tomb *tomb.Tomb) e
 }
 
 func (m *InterfaceManager) removeProfilesForSnap(task *state.Task, _ *tomb.Tomb, snapName string, tm timings.Measurer) error {
-	// Disconnect the snap entirely.
-	// This is required to remove the snap from the interface repository.
-	// The returned list of affected snaps will need to have its security setup
-	// to reflect the change.
-	affectedSnaps, err := m.repo.DisconnectSnap(snapName)
+	st := task.State()
+
+	// Only active connections count, matching what the repo would have
+	// reported.
+	conns, err := getConns(st)
 	if err != nil {
 		return err
 	}
+	affectedSet := make(map[string]bool)
+	for id, conn := range conns {
+		if conn.Undesired || conn.HotplugGone {
+			// Effectively disconnected/inactive.
+			continue
+		}
+		connRef, err := interfaces.ParseConnRef(id)
+		if err != nil {
+			return err
+		}
+		if connRef.PlugRef.Snap == snapName || connRef.SlotRef.Snap == snapName {
+			affectedSet[connRef.PlugRef.Snap] = true
+			affectedSet[connRef.SlotRef.Snap] = true
+		}
+	}
+	affectedSnaps := make([]string, 0, len(affectedSet))
+	for name := range affectedSet {
+		affectedSnaps = append(affectedSnaps, name)
+	}
+	sort.Strings(affectedSnaps)
+
+	// Return value unused: affectedSnaps is derived from conns above.
+	if _, err := m.repo.DisconnectSnap(snapName); err != nil {
+		return err
+	}
 	if err := m.setupAffectedSnaps(task, snapName, affectedSnaps, tm); err != nil {
 		return err
 	}
@@ -1103,13 +1134,17 @@ func (m *InterfaceManager) doDisconnect(task *state.Task, _ *tomb.Tomb) error {
 	if err != nil {
 		_, notConnected := err.(*interfaces.NotConnectedError)
 		_, noPlugOrSlot := err.(*interfaces.NoPlugOrSlotError)
-		// not connected, just forget it.
-		if forget && (notConnected || noPlugOrSlot) {
+		switch {
+		case forget && (notConnected || noPlugOrSlot):
+			// not connected, just forget it.
 			delete(conns, cref.ID())
 			setConns(st, conns)
 			return nil
+		case notConnected:
+			// busy-retry of this same task already disconnected it; proceed.
+		default:
+			return fmt.Errorf("snapd changed, please retry the operation: %v", err)
 		}
-		return fmt.Errorf("snapd changed, please retry the operation: %v", err)
 	}
 
 	for _, snapst := range snapStates {
@@ -1304,7 +1339,10 @@ func (m *InterfaceManager) undoConnect(task *state.Task, _ *tomb.Tomb) error {
 	setConns(st, conns)
 
 	if err := m.repo.Disconnect(connRef.PlugRef.Snap, connRef.PlugRef.Name, connRef.SlotRef.Snap, connRef.SlotRef.Name); err != nil {
-		return err
+		if _, ok := err.(*interfaces.NotConnectedError); !ok {
+			return err
+		}
+		// busy-retry of this same task already disconnected it; proceed.
 	}
 
 	var delayedSetupProfiles bool

overlord/ifacestate/helpers.go

@@ -1244,6 +1244,10 @@ func getPlugAndSlotRefs(task *state.Task) (interfaces.PlugRef, interfaces.SlotRe
 
 // getConns returns information about connections from the state.
 //
+// Those are both active connections and ones that are inactive as a result of
+// being previously manually disconnected or, in case of hotplug, the device
+// disappeared from the system.
+//
 // Connections are transparently re-mapped according to remapIncomingConnRef
 func getConns(st *state.State) (conns map[string]*schema.ConnState, err error) {
 	var raw *json.RawMessage

overlord/ifacestate/ifacemgr.go

@@ -52,7 +52,9 @@ type deviceData struct {
 // installed snaps to track the current set of available plugs and slots.
 type InterfaceManager struct {
 	state *state.State
-	repo  *interfaces.Repository
+	// repo is the interface repository, which holds run-time, ephemeral
+	// information about interfaces and connections
+	repo *interfaces.Repository
 
 	// Notice Manager (because interfacesRequestsManager may be a notice backend)
 	noticeManager *notices.NoticeManager