Skip to content

upcoming: [M3-9706] - Fix Subnets Table for Linodes using new interfaces#11953

Merged
coliu-akamai merged 24 commits intolinode:developfrom
coliu-akamai:m3-9537
Apr 8, 2025
Merged

upcoming: [M3-9706] - Fix Subnets Table for Linodes using new interfaces#11953
coliu-akamai merged 24 commits intolinode:developfrom
coliu-akamai:m3-9537

Conversation

@coliu-akamai
Copy link
Copy Markdown
Contributor

@coliu-akamai coliu-akamai commented Apr 1, 2025

Description 📝

  • Fixes SubnetLinodeRow logic so that Linodes using new interfaces show up correctly in the table

Changes 🔄

  • send requests to Linode Interface endpoints if Linode is using new interfaces
  • since interface Data now gives us the configID, we can directly query for the relevant config profile interface. However, we still need to get a config to check for unrecommended configurations, so I've updated to query for a single config rather than all configs
  • Add test to confirm Linode Interface info shows up correctly

Target release date 🗓️

4/22

Preview 📷

Include a screenshot or screen recording of the change.

🔒 Use the Mask Sensitive Data setting for security.

💡 Use <video src="" /> tag when including recordings in table.

Before After
image image

How to test 🧪

  • using Prod API and have the customer tag (reach out for tag) on your prod admin account
  • Enable the Linode Interfaces feature flag using our local dev tools
  • have a Linode using new interfaces assigned to a VPC

Verification steps

  • Confirm VPC IP, ranges, and firewall show up as expected
  • confirm link to Linode navigates to interface details page

Logic for config profile interfaces

  • confirm things still work as expected
    • reboot chip shows up as expected * (I believe these changes should make the reboot chip friendlier for linodes with multiple configs - ie it won't show up when it's not supposed to)
    • unrecomended config notice shows up as expected *
    • everything else still works as expected
Author 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


  • I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

  • All unit tests are passing
  • TypeScript compilation succeeded without errors
  • Code passes all linting rules

const subnetInterfaceData =
subnetInterfaces.find((interfaceData) => interfaceData.active) ??
subnetInterfaces[0];
const { config_id: configId, id: interfaceId } = subnetInterfaceData;
Copy link
Copy Markdown
Contributor Author

@coliu-akamai coliu-akamai Apr 2, 2025

Choose a reason for hiding this comment

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

we are now given the config id in SubnetLinodeInterfaceData, so we'll no longer have to fetch all configs to find the interface attached to this subnet. I'm still fetching the single config the interface belongs to, due to hasUnrecommendedConfiguration (for implicit primary interfaces)

linodeInterface.purpose === 'vpc' && !linodeInterface.active
)
);
const isRebootNeeded = isRunning && !configInterface?.active;
Copy link
Copy Markdown
Contributor Author

@coliu-akamai coliu-akamai Apr 2, 2025

Choose a reason for hiding this comment

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

not sure how many Linodes have multiple configs, but I remember that the reboot chip is especially finnicky for that case. This clears up the logic a bit - now the only interface we care about is the one we're displaying.

Previously, the reboot chip would display even after we'd already rebooted our Linode with config A if some other config B had an inactive VPC interface.

This reboot chip is not needed for Linode Interfaces - we can only apply networking interface changes when the Linode is shutdown. Upon turning back on and booting, those changes get applied, so no reboot needed (if my understanding of the API spec's overview is correct)

@coliu-akamai coliu-akamai marked this pull request as ready for review April 2, 2025 17:44
@coliu-akamai coliu-akamai requested a review from a team as a code owner April 2, 2025 17:44
@coliu-akamai coliu-akamai requested review from bnussman-akamai, cpathipa, dwiley-akamai, hana-akamai and harsh-akamai and removed request for a team April 2, 2025 17:44
@coliu-akamai
Copy link
Copy Markdown
Contributor Author

@dwiley-akamai @hana-akamai also requesting you since this touches a lot of old VPC logic and would appreciate the extra eyes!

@coliu-akamai coliu-akamai requested a review from a team as a code owner April 2, 2025 21:31
@coliu-akamai coliu-akamai requested review from jdamore-linode and removed request for a team April 2, 2025 21:31
Copy link
Copy Markdown
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

VPC IP, ranges, and firewall show up as expected ✅

Linode assigned to VPC

  • w/ legacy config/interfaces: links to linode details page ✅
  • w/ linode interfaces: links to linode details > Network tab > Interface Details drawer ✅

Config interfaces

  • Reboot indicator behavior ✅
  • Unrecommended Configuration notice ✅

Copy link
Copy Markdown
Contributor

@hana-akamai hana-akamai left a comment

Choose a reason for hiding this comment

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

Functionality is looking good!

coliu-akamai and others added 5 commits April 4, 2025 09:15
Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>
Co-authored-by: Dajahi Wiley <114682940+dwiley-akamai@users.noreply.github.com>
@coliu-akamai coliu-akamai added the Add'tl Approval Needed Waiting on another approval! label Apr 4, 2025
Copy link
Copy Markdown
Contributor

@cpathipa cpathipa left a comment

Choose a reason for hiding this comment

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

Nice work @coliu-akamai confirming on the verification steps.

@github-project-automation github-project-automation bot moved this from Review to Approved in Cloud Manager Apr 7, 2025
@cpathipa cpathipa added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Apr 7, 2025
Copy link
Copy Markdown
Member

@bnussman-akamai bnussman-akamai left a comment

Choose a reason for hiding this comment

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

Nice, thanks for fixing up this flow 🙏

Table looks good for both legacy and new interfaces

Screenshot 2025-04-07 at 5 48 08 PM

@linode-gh-bot
Copy link
Copy Markdown
Collaborator

Cloud Manager UI test results

🎉 540 passing tests on test run #27 ↗︎

❌ Failing✅ Passing↪️ Skipped🕐 Duration
0 Failing540 Passing4 Skipped99m 18s

@coliu-akamai coliu-akamai merged commit 6fa94d1 into linode:develop Apr 8, 2025
28 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Merged in Cloud Manager Apr 8, 2025
@coliu-akamai coliu-akamai deleted the m3-9537 branch April 8, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved Multiple approvals and ready to merge! Linode Interfaces

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants