scrimlet-reconcilers: flesh out dpd service NAT reconciler#10442
Open
jgallagher wants to merge 3 commits into
Open
scrimlet-reconcilers: flesh out dpd service NAT reconciler#10442jgallagher wants to merge 3 commits into
jgallagher wants to merge 3 commits into
Conversation
internet-diglett
approved these changes
May 13, 2026
Comment on lines
+203
to
+212
| /// * We'd like to eventually replace this with fewer calls, if we add different | ||
| /// APIs to `dpd` for applying NAT settings in bulk. | ||
| /// <https://github.com/oxidecomputer/dendrite/issues/255> | ||
| /// * In practice we expect the number of calls here to be small. On startup we | ||
| /// expect ~10 `to_create` calls (one for each service, which is typically 2 | ||
| /// boundary NTP, 3 Nexus, and 1-5 external DNS), and for every reconciliation | ||
| /// attempt after that we expect 0-1 (either no changes, or a single new | ||
| /// service has been added or removed; it's possible we'll see multiple, but | ||
| /// unlikely given we re-reconcile on every networking config change). | ||
| /// * We always want to report the status of every step described by `plan`, and |
Contributor
There was a problem hiding this comment.
This is probably good for the time being. Once we figure out how we want to resolve #8748, we will likely have the bulk NAT API endpoints implemented in dpd
Contributor
|
Although this is different to how we sync service zone nat entries today, the overall pattern of reconciling a diff is similar to how we synchronize other |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is almost all new code (and about 60/40 "real code" / "tests"). Today NAT entry reconciliation is inverted from what this PR implements: dpd fetches NAT entries from Nexus and does reconciliation internally. In this PR, sled-agent does the reconciliation. This requires fetching all NAT entries from dpd (to decide what to remove, if anything). My biggest concerns here are:
This is targeted at the feature / integration branch that's building up the complete
sled-agent-scrimlet-reconcilerscrate.