Nexus must hang onto qorb resolvers used for MGS updates

[davepacheco]

The failure mode here is that when using Nexus to update SPs, all the updates fail with messages like this:

23:40:39.213Z INFO bea4cc1e-0758-4643-a4b4-669f67689c6c (ServerContext): update attempt done
    artifact_hash = 100352eb10b8e657bfa168dddd6a3e1cff3f1a3f5600feacfa5bf6b8983ec145
    artifact_version = 1.0.39
    elapsed_millis = 0
    error = found no MGS backends in DNS
    expected_active_version = 1.0.38
    expected_inactive_version = Version(ArtifactVersion("1.0.38"))
    file = nexus/mgs-updates/src/driver.rs:422
    part_number = 913-0000006
    serial_number = BRM23230002
    sp_slot = 0
    sp_type = Switch
    update_id = e17b53c0-f1d6-4f1c-a77d-d63fc08587e5

It says "found no MGS backends in DNS" but there are MGS backends in DNS. The problem is that we dropped the resolvers and so they're not doing any DNS resolution.

[smklein]

Normally these resolvers are handed off to the pool, so their lifetime is coupled with the pool itself.

Do you think qorb could have done anything more clear to identify that the resolver object needs to be kept alive for resolution to keep happening?

[davepacheco]

Good question. Maybe update the Resolver docs to say something like:

Resolvers generally do the bulk of their work (e.g., DNS resolution) in a separate tokio task. Dropping the resolver aborts that task. Any Receivers previously returned by monitor() will contain the last-updated set of backends indefinitely.

This last thing isn't qorb's fault but I found it to be a surprising footgun (with watch channels, I guess) and so worth calling out. I thought I'd have seen a RecvError because the other end of the channel got dropped. But my watch consumer only ever uses borrow() so it didn't notice the channel was closed.

I'd also update the docs (and maybe name?) for monitor(). The name sounds like it's going to take some action and the docs say "Start running a resolver". But that's not right. The resolver is already running before you call it. Maybe call it subscribe() and just drop the "Start running a resolver" sentence? I understand though if it's not worth making a breaking change for this.

I also think it's worth mentioning the thing above under monitor(), something like:

Note that if the Resolver gets dropped, then Receivers previously returned by this method will stop getting updated, but they will contain to report the last known set of backends.

[smklein]

Updated docs in oxidecomputer/qorb#112 for Resolver::monitor

[davepacheco]

See #8291 for testing notes.