Skip to content

Nexus background task for distributing attached subnets#9780

Merged
bnaecker merged 4 commits intomainfrom
nexus-attached-subnet-bg-task
Feb 5, 2026
Merged

Nexus background task for distributing attached subnets#9780
bnaecker merged 4 commits intomainfrom
nexus-attached-subnet-bg-task

Conversation

@bnaecker
Copy link
Collaborator

@bnaecker bnaecker commented Feb 3, 2026

@bnaecker
Add sled-agent API for managing attached subnets
5851b43 
- Add APIs to the sled agent for attaching and detaching either a single
  subnet on an instance, or setting / clearing the entire set for an
  instance.
- Add list of attached subnets in the instance-creation request body,
  and fill that in from Nexus with the (currently-empty) set of attached
  subnets for the target instnace.
- Plumb attachment requests all the way through the sled-agent internals
  to the new APIs in OPTE.
- Add mapping of attached subnets per-instance to the simulated sled
  agent for testing.
- Fixes #9702
@bnaecker
Add Nexus sagas for external subnet attach / detach
2b032b6 
- Adds a subnet attach and detach saga in Nexus, modeled after the
  existing floating IP attachment sagas.
- Update the common code for sagas to include passing or removing the
  attached subnets to Dendrite and / or OPTE. This is to pick up those
  changes during the existing instance sagas, e.g. instance update.
- Fixes #9685
@bnaecker
Copy link
Collaborator Author

bnaecker commented Feb 3, 2026

Stacked on #9779

@bnaecker bnaecker mentioned this pull request Feb 3, 2026
Open
@bnaecker bnaecker added this to the 18 milestone Feb 3, 2026
@bnaecker
Nexus background task for distributing attached subnets
8e62a71 
- Add a background task to Nexus that periodically pushes all attached
  subnets to Dendrite and the sled-agents / OPTE.
- Add task output to `omdb`.
- Fixes #9581 and fixes #9582
@bnaecker bnaecker force-pushed the nexus-attached-subnet-bg-task branch from 2867c38 to 8e62a71 Compare February 3, 2026 06:35
@bnaecker bnaecker mentioned this pull request Feb 3, 2026
Merged
rcgoodfellow
rcgoodfellow reviewed Feb 3, 2026
View reviewed changes
Copy link
Contributor

@rcgoodfellow rcgoodfellow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meta comment for this PR in combo with #9779, should uplink movement be a trigger for this RPW? That can come as a follow later on if so.

nexus/src/app/background/tasks/attached_subnets.rs
&desired_attachments,
);

// Remove any attachments Dendrite has that we no longer want.
Copy link
Contributor

@rcgoodfellow rcgoodfellow Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If nexus instance 1 is told to add subnet X, while nexus instance 2 is operating on old DB data but new dendrite data, nexus-2 may delete X. Is the theory that subsequent rounds of the reconciler will pick this up and things will become eventually consistent?

Copy link
Collaborator Author

@bnaecker bnaecker Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so.

Copy link
Contributor

@jgallagher jgallagher Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would presumably be a much larger change, but FWIW, we solve the analogous problem for deploying sled configs from blueprints by putting a generation number on each sled config as a whole, which allows sled-agent to detect (and reject) stale data coming from an out-of-date Nexus. I don't think we ever came up with any reasonable proposals to address this problem from Nexus only; we really want the recipient to be able to correctly handle (ie: ignore) stale requests.

Copy link
Collaborator Author

@bnaecker bnaecker Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that sounds useful but a big change. The sled-agent could be made to work that way, if we have it maintain the whole-sled set of attachments instead of a per-VMM set. But Dendrite would also need to change to make full use of that.

Copy link
Contributor

@rcgoodfellow rcgoodfellow Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've logged the following for longer term follow ups.

This was referenced Feb 3, 2026
Open
Open
mergeconflict
mergeconflict approved these changes Feb 4, 2026
View reviewed changes
Base automatically changed from subnet-attach-detach-sagas to main February 5, 2026 03:09
@bnaecker
Copy link
Collaborator Author

bnaecker commented Feb 5, 2026

Alright, I've fixed up conflicts with main. If folks could take a quick pass through one more time, that would be much appreciated.

rcgoodfellow
rcgoodfellow reviewed Feb 5, 2026
View reviewed changes
nexus/db-model/src/external_subnet.rs
use nexus_types::external_api::params;
use nexus_types::external_api::views;
use nexus_types::identity::Resource;
use nexus_types::identity::Resource as _;
Copy link
Contributor

@rcgoodfellow rcgoodfellow Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello there. We meet again.

Copy link
Collaborator Author

@bnaecker bnaecker Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe I fucked up the last merge with main before I integrated #9779. This is on main, in any case.

illumos-utils/src/opte/port_manager.rs
Copy link
Contributor

@rcgoodfellow rcgoodfellow Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's going on with these asserts? Is this a crashworthy situation?

Copy link
Collaborator Author

@bnaecker bnaecker Feb 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, GitHub's UI is such trash that I actually can't tell exactly what you're referring to. Your comment appears attached to a line building some tuple, with no assertions in sight. So that's cool.

But if it's this:

omicron/illumos-utils/src/opte/port_manager.rs

Line 1054 in 08c0d9b

assert!(result.error.replace(e.into()).is_none());

then

  • that's already on main too, and
  • I think it's crash-worthy. It means we fucked up the code badly enough that a previous attempt to attach / detach a subnet failed, but we didn't notice. The sled-agent and OPTE disagree about what's attached and I'd be suspicious of any future operations.

@bnaecker bnaecker enabled auto-merge (squash) February 5, 2026 06:39
@bnaecker bnaecker merged commit 360801b into main Feb 5, 2026
16 checks passed
@bnaecker bnaecker deleted the nexus-attached-subnet-bg-task branch February 5, 2026 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rcgoodfellow rcgoodfellow rcgoodfellow approved these changes

@mergeconflict mergeconflict Awaiting requested review from mergeconflict

@jgallagher jgallagher Awaiting requested review from jgallagher

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

18

Development

Successfully merging this pull request may close these issues.

Nexus background task for managing external subnets in OPTE Nexus background task for managing external subnet routes through Dendrite

4 participants

@bnaecker @mergeconflict @rcgoodfellow @jgallagher