-
Notifications
You must be signed in to change notification settings - Fork 21
Update to new networking API shape, with IPv6 #3005
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have e2e test failures in IP pools. |
| use: { ...devices['Desktop Firefox'] }, | ||
| use: { | ||
| contextOptions: { | ||
| reducedMotion: 'reduce', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming this was related to reducing flake?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes; the theory from Claude was that with some animation changes in #3007 were causing some flaking in the e2e tests, though all of my local runs were working as expected, so I'm not sure if that was it. It just seemed to be an issue with the CI runs, and they ended up getting resolved by adding a .blur step, so it's possible the reducedMotion was moot. But if it speeds things CI runs up a tiny bit, and maybe fixes one or two flakey tests along the way, I'm okay with keeping it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure, but I think only one or two of the side modals actually changed behavior in #3007 — mostly it was a slightly neater way of doing what we were already doing.
This PR updates console to work with the new API shape for IP pool selection introduced in oxidecomputer/omicron#9598 and other Omicron networking PRs, like oxidecomputer/omicron#9665. These are summarized in #3000, and I have screenshots of each of the UX changes over in #3017.
I'll go over the main changes and how they relate to Console, both for us and for aipr as we go through it. There are still a few decisions to make in terms of forms / UI, but I'll try to note those as well.
Claude's summary of the changes in this PR:
Key API Shape Changes
Before:
pool?: NameOrId | null
After:
poolSelector?: PoolSelector // discriminated union
type PoolSelector =
| { type: 'explicit'; pool: NameOrId }
| { type: 'auto'; ipVersion?: IpVersion | null } // Required when dual defaults exist
Why: Enables explicit IPv4 vs IPv6 selection when both default pools exist.
Before:
// Single IP + transit IPs
ip?: string
transitIps?: IpNet[]
After:
// Discriminated union supporting v4-only, v6-only, or dual-stack
ipConfig?: PrivateIpStackCreate
type PrivateIpStackCreate =
| { type: 'v4'; value: PrivateIpv4StackCreate }
| { type: 'v6'; value: PrivateIpv6StackCreate }
| { type: 'dual_stack'; value: { v4: ..., v6: ... } }
Each stack has ip: Ipv4Assignment | Ipv6Assignment (auto or explicit) and version-specific transitIps.
Before:
type: 'default' // Single default option
After:
type: 'default_ipv4' | 'default_ipv6' | 'default_dual_stack'
Why: Explicit control over default interface IP version(s).
Before: Max 1 default pool per silo
After: Max 4 defaults per silo:
Added poolType: 'unicast' | 'multicast' field:
UI/Form Impact Summary
New Component: IpPoolSelector – radio-button UI for "IPv4 default" / "IPv6 default" / "custom pool" selection with version filtering.
Also, a quick note on selecting Ephemeral IPs for instances:
Also …
Closes #2901
Closes #3000