-
Notifications
You must be signed in to change notification settings - Fork 390
upcoming: [M3-9864] – Add new columns to VPC Subnet & Subnet linodes tables #12305
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
upcoming: [M3-9864] – Add new columns to VPC Subnet & Subnet linodes tables #12305
Conversation
…s and update instances where they are used
|
This PR is stale because it has been open 15 days with no activity. Please attend to this PR or it will be closed in 5 days |
hana-akamai
left a comment
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.
Can we add a changeset for the utilities package as well?
packages/manager/.changeset/pr-12305-upcoming-features-1753137015399.md
Outdated
Show resolved
Hide resolved
| })} | ||
| </TableCell> | ||
| </Hidden> | ||
| {flags.vpcIpv6 && ( |
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.
We should check if the VPC is dualstack as well so we don't show IPv6 for non-dualstack VPCs
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.
Checking with Daniel if we want to display the IPv6 columns for all VPCs or not (and also if we should use dashes instead of "None")
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.
re: using dashes vs hiding the ipv6 columns, I assigned several Linodes to a non-dualstack VPC. If the VPC has a lot of Linodes assigned to it, there will be a lot of rows with only dashes. Maybe displaying a chip or something like LKE clusters' HA cluster symbol in the VPC's description and hiding the columns could be another option?
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.
Left a more detailed comment on the ticket, but the decision (at this time) was to display the columns for all VPCs
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.
@DevDW what was UX's reasoning for displaying the IPv6 columns for all VPCs? It seems like unnecessary clutter imo
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.
@hana-akamai Essentially a consistent view across all VPCs + an effort to convey that IPv6 can't be used for non-Dual Stack VPCs (at this time, there's no way to add or remove IPv6 prefixes from a VPC and it isn't planned). I will resurface this feedback to Daniel tomorrow once he's back in though
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.
@hana-akamai @coliu-akamai Daniel is going to bring the topic up with the wider team this week -- if we end up changing how the columns are handled, I'll create a separate ticket to take care of that 👍🏾 Would be great to get this merged while that is being explored in the background
…eInterfaceFactoryVPC with IPv6 data
coliu-akamai
left a comment
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.
- ✅ Observe: the inner Linode table for that subnet should show the newly-assigned linode, with the "VPC IPv6" and "Linode IPv6 Ranges" cells populated as expected
- ✅ IPv6 columns displayed as long as the feature flag is on -- for VPCs/subnets that don't support IPv6, you should see an em dash (—) in the cell for that column value
- ✅ no IPv6 columns displayed if feature flag is disabled
thanks @dwiley-akamai!
packages/manager/src/features/VPCs/VPCDetail/SubnetLinodeRow.tsx
Outdated
Show resolved
Hide resolved
| })} | ||
| </TableCell> | ||
| </Hidden> | ||
| {flags.vpcIpv6 && ( |
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.
re: using dashes vs hiding the ipv6 columns, I assigned several Linodes to a non-dualstack VPC. If the VPC has a lot of Linodes assigned to it, there will be a lot of rows with only dashes. Maybe displaying a chip or something like LKE clusters' HA cluster symbol in the VPC's description and hiding the columns could be another option?
|
Hello @DevDW - can we not change the labels on the ranges to Linode IPv4 Ranges and Linode IPv6 Ranges? Let's keep them as they are for now VPC IPvX Ranges. |
hana-akamai
left a comment
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.
Approving pending the VPCSubnetsTable unit test failure is addressed
packages/manager/src/features/VPCs/VPCDetail/VPCSubnetsTable.test.tsx
Outdated
Show resolved
Hide resolved
Cloud Manager UI test results🔺 4 failing tests on test run #22 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/kubernetes/lke-create.spec.ts,cypress/e2e/core/linodes/linode-storage.spec.ts" |
||||||||||||||||||||||||||
Description 📝
Update the outer and inner subnet tables to include new IPv6 columns and conditionally change the names of some existing columns depending on the feature flag.
Changes 🔄
Target release date 🗓️
8/12/25
Preview 📷
How to test 🧪
Prerequisites
vpc-ipv6tag on your account(Reach out for more instructions if needed)
Verification steps
/XXpart) that matches what you selected in the Create Subnet drawerAuthor Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅