Skip to content

Remove old sled agent api versions - #11205

Open
jmpesp wants to merge 35 commits into
oxidecomputer:mainfrom
jmpesp:remove_old_sled_agent_api_versions
Open

Remove old sled agent api versions#11205
jmpesp wants to merge 35 commits into
oxidecomputer:mainfrom
jmpesp:remove_old_sled_agent_api_versions

Conversation

@jmpesp

@jmpesp jmpesp commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Retire up to sled-agent version 23 - this removes several problematic API versions present in old versions and cleans up the associated code. sled-agent-type-versions types that are no longer used have also been removed, as many as a tool I wrote could be found.

#[path = "typed_sled_config_generation/mod.rs"]
pub mod v50;
#[path = "retire_up_to_version_23/mod.rs"]
pub mod v51;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We chatted about this offline - I think we don't want to add a new types module here (because we're not adding / changing any types) - anything that's in this module as written should be left wherever it was originally, which should mean we leave latest.rs untouched.

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.

+1, addressed in 1d51fb5

@jgallagher jgallagher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This all looks reasonable to me! I'd love to get a glance from @sunshowers, time permitting, particularly to make sure we made the right call on #11205 (comment). The only other option that seems reasonable to me now is "just leave all the stale types around", but that's mostly because it seems so painful to do the work to prune them all.

Comment thread sled-agent/api/src/lib.rs
path = "/network-bootstore-config",
versions = VERSION_BGP_V6..VERSION_REMOVE_READ_BOOTSTORE_CONFIG_CACHE,
}]
async fn read_network_bootstore_config_cache(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@andrewjstone will be very, very happy to see this gone

@jmpesp

jmpesp commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

This all looks reasonable to me! I'd love to get a glance from @sunshowers, time permitting, particularly to make sure we made the right call on #11205 (comment). The only other option that seems reasonable to me now is "just leave all the stale types around", but that's mostly because it seems so painful to do the work to prune them all.

One thing that clicked for me when we were chatting in our DMs - if we follow the "just leave the stale types around" idea, and imagine retiring all the sled-agent API versions up to the latest, then the only types that remain will be the ones directly referenced in sled-agent/types/versions/src/latest.rs, and all others can then be considered unused.

But I think I may be misunderstanding the quoted comment - is there stuff I missed pruning?

@jgallagher

Copy link
Copy Markdown
Contributor

One thing that clicked for me when we were chatting in our DMs - if we follow the "just leave the stale types around" idea, and imagine retiring all the sled-agent API versions up to the latest, then the only types that remain will be the ones directly referenced in sled-agent/types/versions/src/latest.rs, and all others can then be considered unused.

I think that's true but in practice it'll be rare we can return all but the latest (we'd have to go a full release cycle with no changes for that to be safe).

But I think I may be misunderstanding the quoted comment - is there stuff I missed pruning?

Not that I noticed! I was mostly commenting on the level of effort to do the pruning at all.

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