Skip to content

Conversation

@internet-diglett
Copy link
Contributor

Related

#9501

@internet-diglett internet-diglett changed the title Enable wicket to configure the rack subnet Enable wicket / config-rss.toml to configure the rack subnet Jan 15, 2026
@internet-diglett internet-diglett added this to the 18 milestone Jan 15, 2026
@internet-diglett internet-diglett marked this pull request as ready for review January 15, 2026 00:36
Copy link
Contributor

@jgallagher jgallagher left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me - just a couple minor questions.

Have you given this a spin on a racklette? (Can we, before merging?)

if rack_subnet_address.octets()[6] == 0x00 {
return Err("rack number (seventh octet) cannot be 0".into());
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to check that the low 72 bits are all 0? That's guaranteed for our random ones, and converting the subnet address into an Ipv6Net<56> will do that, but it may indicate some confusion if an operator asks for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a reasonable check to add, as it does make things a bit clearer in terms of what we expect (and what the system will do if you set the lower bits of the address)

rack_network_config.as_ref().map_or("".into(), |c| {
match c.rack_subnet_address {
Some(v) => v.to_string(),
None => "".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want something like "(will be chosen randomly)" instead of leaving it blank?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is also a good idea

@internet-diglett
Copy link
Contributor Author

Have you given this a spin on a racklette? (Can we, before merging?)

I'll do that today!

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.

3 participants