Skip to content

[meta] auto-replace iddqd in sled-agent-client - #11551

Merged
sunshowers merged 3 commits into
mainfrom
sunshowers/spr/meta-auto-replace-iddqd-in-sled-agent-client
Oct 22, 2026
Merged

sunshowers merged 3 commits into
mainfrom
sunshowers/spr/meta-auto-replace-iddqd-in-sled-agent-client

Conversation

@sunshowers

Copy link
Copy Markdown
Contributor

This required many other replaces since we needed to pull in the appropriate key type impls. (This is a deliberate decision to make clients and servers agree on type semantics.)

There are two places where duplicates are newly detected on the client:

  1. In list_all_probe_create_params_for_sled_batched.
  2. In group_attached_subnets_by_sled_and_vmm.

What happens if there were duplicates? Previously, with both 1 and 2:

  • The client would generate and send a Vec with the duplicates.
  • The server would attempt to deserialize as an IdOrdMap.
  • IdOrdMap rejects duplicates, so the server would return a 400.

With this change, duplicates are now detected on the client before they're sent to the server.

  • For list_all_probe_create_params_for_sled_batched, this doesn't change anything -- we previously fail closed and now continue to fail closed. I didn't investigate beyond that point.

  • For group_attached_subnets_by_sled_and_vmm, there is a change, because instead of failing with a 400, we now push duplicates to the SledSubnetDetails::errors list and otherwise successfully create a config to send to the server. I did spend some time trying to figure out whether uniqueness was guaranteed (because if it is, then this isn't a functional change in practice). My understanding is that:

    I've tried capturing my understanding in code comments -- happy to take suggestions on improvements.

@sunshowers

Copy link
Copy Markdown
Contributor Author

@bnaecker added you since you filed #9580 and probably have the most context about attached subnets.

//
// We should:
//
// * Make the primary-NIC join 1:1 with a database constraint (e.g., a

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you file an issue for this? We should have such a constraint.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#11568. thanks!

sunshowers added a commit that referenced this pull request Oct 22, 2026
Similar to #11551, convert over the clickhouse-admin clients to iddqd.

In particular, we get to share a common `Days` definition now, which is
nicer.

Also switch over automatic replacements to use the versions crate, as
[required](https://rfd.shared.oxide.computer/rfd/0619#determinations-progenitor-matching)
by RFD 619.
sunshowers added a commit that referenced this pull request Oct 22, 2026
…and tests (#11555)

Part of work identified while reviewing #11551. Prepare to use the
attached subnets provided in the local config.

This does not change production behavior, but does make it so that tests
pick up the DHCP config, host name, and instance ID.
@sunshowers
sunshowers merged commit 79a9d4b into main Oct 22, 2026
20 checks passed
@sunshowers
sunshowers deleted the sunshowers/spr/meta-auto-replace-iddqd-in-sled-agent-client branch October 22, 2026 21:04
sunshowers added a commit that referenced this pull request Oct 22, 2026
#11553)

Followup from #11551 and #11552 -- previously, errors (duplicates or
otherwise) would not be displayed in omdb's output. Now they are, and we
destructure the struct exhaustively to ensure that if new fields are
added, those are also displayed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants