Interface hooks - disconnect hooks (WIP)

snapd
1

I’ve recently started working on a disconnect hooks for the upcoming interface hooks - the idea is to support “disconnect-plug-…” and “disconnect-slot-…” hooks on respective ends of a connection, and fire them up on snap disconnect. When it comes to implementation, there will be two extra hook tasks apart from the existing “disconnect” task, similiar to how “connect” hooks are implemented.

I hit an obstacle though with our internal logic for resolving disconnect parameters. In order to create hook tasks for disconnect in ifacestate I need to know snap and plug/slot names on both ends upfront (otherwise hooks fail immediately); the current implementation of “disconnect” task is tolerant and allows for empty snap/slot/plug names, they are resolved late in doDisconnect handler.

I could move the logic of resolveDisonnect to an earlier stage when disconnect task and hooks are created and either:

  • create multiple tasks for disconnects and it shooks, for concrete plug and slot pairs.
  • have a single disconnect task (as it is now) and two hook tasks, have all the resolved plugs and slots resolved upfront and stuffed into disconnect task data.

The latter looks like a better and natural solution to me and this is what I’m proposing here. Does anyone has any other suggestions to solve this problem? Does that sound sensible to you @niemeyer ?

Note, I had similiar issue when implementing connect hooks, but it was simpler there, the resolveConnect logic could only yield a single connection candidate, it was moved to api level before a call to Connect(). Disconnect is more complicated as it can return multiple connections to disconnect.

1 Like
2

I didn’t realize “connect” hooks were implemented. They don’t seem to be documented with the other hooks. Are they documented elsewhere?

3

@pstolowski Sorry for not providing feedback so far. Still need to look more closely into the code to provide proper feedback.

@kyrofa This is still work in progress and not supposed to be used yet.

5

Upon further consideration I realized that the 2nd option is not really solving the issue of hooks, because we need separate hook task for every interface to disconnect, so single disconnect task with all names resolved doesn’t help much.

To summarize, I think we need to:

  • perform ResolveDisconnect early in the process, similiar to what we do with connect. I think the best place to do it is in api.go, before we call Disconnect() as we have access to interface repo there.
  • Create separate tasks for every disconnected plug-slot pair, and corresponding tasks for their hooks.

While investigating this I realized we have a bug in the disconnect code (in ifacestate.go - Disconnect()): early in Disconnect() we check for potential conflict with other changes (snapstate.CheckChangeConflict() calls), but because we do that before resolving disconnects, we may have empty snap names and will never find a potentially conflicting change in progress affecting same snap. This bug will naturally be fixed with the proposed change to do ResolveDisconnect early.

6

First related PR - https://github.com/snapcore/snapd/pull/3212 - this just moves disconnect resolving logic and prepares ground for disconnect hooks.

7

@pstolowski Sounds like a good plan. The direction we went over with connect seems appropriate for disconnect too, and the fact connect is a single connection today seems perhaps like a special case. We might end up doing many of them at once, and there the independent tasks idea also seems like the proper way to go.

1 Like