feat: [M3-7121 & M3-7122] - Support VPC in Linode Add/Edit Config dialog #9709

hana-akamai · 2023-09-21T19:06:57Z

Description 📝

Add support for VPCs in the Linode Add Config and Linode Edit Config dialogs, as well as within the config row in the Linode Configurations table.

Major Changes 🔄

VPCPanel component used in the Add/Edit Linode Configuration dialogs
- Tweaks in LinodeConfigDialog.tsx, InterfaceSelect.tsx, and VPCPanel.tsx
Network Interfaces column in Linode Config table now supports VPCs
- new InterfaceListItem component

How to test 🧪

With a properly-tagged account or with the VPC feature flag on:

For both the "Add Configuration" and "Edit Configuration" dialogs:
- The section title should be "Networking" and there should be updated helper text that reflects the VPC option
- There should be a "Primary Interface (Default Route)" dropdown
- You should be able to successfully create a config with the designated interfaces
- Field errors should be surfaced (example: uncheck the "Auto-assign a VPC IPv4 [...]" checkbox, input an invalid VPC IPv4, and observe the field error)
- The dialog should reset when closed and re-opened
For the "Edit Configuration" dialog:
- Ensure the "Primary Interface" field displays the correct primary interface (the interface designated as "primary" based on the API payload should be in the eth position indicated by the "Primary Interface" field)
- Changing the primary interface gets reflected in the payload submitted to the API, and after a successful submission, opening the dialog again for that config should reflect the changes you just made

None of the above should be viewable/possible without the proper account tags or the VPC feature flag being on.

Because the <VPCPanel /> component was changed to accommodate the Add/Edit Config use case, ensure there were no regressions to its behavior in the Linode Create flow.

…rface dropdown in Add/Edit Config dialog; casing adjustments for some variable names

… and fully fixed formatting

…eSelect.tsx

packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.tsx

…g open; surface errors

dwiley-akamai · 2023-10-02T22:14:32Z

Observed some instances of errors not surfacing, so the error mapping still needs tweaking

dwiley-akamai · 2023-10-05T19:15:07Z

If I uncheck the "Auto-assign a VPC IPv4 [...]" checkbox but do not input any IPv4, what's the expected behavior? Currently it creates (or edits) the config:

Oh is this one of the errors that don't get surfaced correctly?

Screen.Recording.2023-10-05.at.11.21.28.AM.mov
followup: when creating a Linode and leaving that blank, an error surfaces for a blank IPv4:

The latter case is because we're able to easily see if the checkbox is unchecked and if there's a blank VPC IPv4 and set the error accordingly (L758-L773 in LinodeCreateContainter.tsx). There's only one pair of those fields in the Linode Create flow, whereas here we can have multiple pairs of those fields in the form (one pair per interface)

packages/api-v4/src/linodes/types.ts

dwiley-akamai · 2023-10-06T18:37:49Z

For endpoints that take a list of interfaces, if one of the interfaces has an API error, that should be indicated in the field property -- e.g., interfaces[0].ipv4.vpc instead of ipv4.vpc. API has a bug ticket to address this and it should be done in the near future. In the meantime, I pushed up some code to surface those errors in a non-ideal capacity, just so they don't stay submerged until that fix is in. I'll make a follow-up ticket now for cleaning that up when the API fix is in.

With regards to the error when you uncheck the VPC IPv4 autoassign checkbox but don't input a VPC IPv4: I am going to make a separate ticket to handle that situation and will work on that today.

…d weird spacing in Add/Edit Linode Config dialog; make noWrap on VPC IPv4 checkbox text conditional so tooltip isn't obscured

…hub.com/hana-linode/manager into M3-7121-vpc-add-config-dialog-and-table

coliu-akamai · 2023-10-10T18:59:51Z

Functionality from above that I checked off still looks good + the error on the Subnet Assign Linodes Drawer is fixed and I saw the tickets for the other error surfacing issues 🚀

One other thing I'm noticing is that when we edit a Linode's configuration details to include or delete a VPC related interface, the configuration will update, but other information won't until we refresh - something to do with query invalidation/the query cache, most likely?

https://github.com/linode/manager/assets/139280159/9150f54e-7507-4afc-a172-7eddb6d055c7
This video here is showing how although the config has a VPC interface, this VPC isn't appearing in the linode's details header. Also, the VPC landing page isn't correctly updated (but an individual VPC's detail page is.

(Actually this reminds me about the issue for freshly deleted linodes still appearing in VPCs -- let me make a ticket for that so it doesn't get lost 😅)

coliu-akamai

awesome work Hana and Dajahi!

approving pending query caching issue is resolved (moved to a new ticket or handled in this PR)!

… purpose === 'vpc'

abailly-akamai

Nice work! things are working

I tested both flows (edit and create config) as we will as the linode creation flow and the form behaviors work well. Left a couple comments to clarify and clean up a couple things.

Also, there is no test coverage added here although we are modifying core flows and adding/modifying many components. I know there must be upcoming e2e tests covering some of those cases but we should also have unit tests testing the rendering and some of the utilities added.

packages/manager/src/features/Linodes/LinodesCreate/VPCPanel.tsx

abailly-akamai · 2023-10-11T14:08:59Z

packages/manager/src/features/Linodes/LinodesCreate/VPCPanel.tsx

@@ -268,6 +306,15 @@ export const VPCPanel = (props: VPCPanelProps) => {
                  </Box>
                }
              />
+              {assignPublicIPv4Address && nat_1_1Error && (


how can we test this error?

also, let's update this variable name so it's human readable, it's pretty unclear from reading the code

If the linode doesn't have an unused public IP available and the "Assign a public IPv4 address for this Linode" checkbox is checked, the following error gets returned:

One way to get it to be returned is to have an interface with 2 VPC slots and checking off that checkbox for both of them. You need an extra tag to be able to get to that state.

I'll rename the variable to publicIPv4Error or something to that effect.

jaalah-akamai · 2023-10-12T19:35:09Z

✅ For both the "Add Configuration" and "Edit Configuration" dialogs:

The section title should be "Networking" and there should be updated helper text that reflects the VPC option
There should be a "Primary Interface (Default Route)" dropdown
You should be able to successfully create a config with the designated interfaces
Field errors should be surfaced (example: uncheck the "Auto-assign a VPC IPv4 [...]" checkbox, input an invalid VPC IPv4, and observe the field error)
The dialog should reset when closed and re-opened

✅ For the "Edit Configuration" dialog:

Ensure the "Primary Interface" field displays the correct primary interface (the interface designated as "primary" based on the API payload should be in the eth position indicated by the "Primary Interface" field)
Changing the primary interface gets reflected in the payload submitted to the API, and after a successful submission, opening the dialog again for that config should reflect the changes you just made

abailly-akamai

Thanks for addressing the feedback and adding coverage - looks great!

Functionality and UI are good as far as my testing went. ✅
E2E failure seems unrelated

## Description 📝 Bug from #9709 On the prod env, we were unable to edit a config due to the config interface being deleted. This was due to the `regionHasVPCs` variable being coupled with `regionHasVLANS` in the if statement to delete config interfaces. The fix was to handle VPCs and VLANs separately. ## How to test 🧪 ### Prerequisites - Point to the prod environment ### Reproduction steps - Go to a Linode's details page and click on the `Configurations` tab - Click the edit button and scroll down to the Networking section - Select a VLAN and save changes ### Verification steps - The VLAN changes should be reflected in the Configurations table

…log (linode#9709)

## Description 📝 Bug from linode#9709 On the prod env, we were unable to edit a config due to the config interface being deleted. This was due to the `regionHasVPCs` variable being coupled with `regionHasVLANS` in the if statement to delete config interfaces. The fix was to handle VPCs and VLANs separately. ## How to test 🧪 ### Prerequisites - Point to the prod environment ### Reproduction steps - Go to a Linode's details page and click on the `Configurations` tab - Click the edit button and scroll down to the Networking section - Select a VLAN and save changes ### Verification steps - The VLAN changes should be reflected in the Configurations table

hana-akamai added the VPC Relating to VPC project label Sep 21, 2023

hana-akamai self-assigned this Sep 21, 2023

hana-akamai added 2 commits September 21, 2023 15:08

display vpc in the network interfaces column of the config table

168fa98

linode add config vpc state

e325b51

hana-akamai force-pushed the M3-7121-vpc-add-config-dialog-and-table branch from 2241b8e to e325b51 Compare September 21, 2023 19:09

hana-akamai and others added 16 commits September 22, 2023 11:19

fix label error on submit

21bc318

fix vpc select and errors not displaying

6e66e00

filter by region

8bb08e9

feature flag vpc option

2728840

fix vpc and subnet validation errors

307c089

fix vpc ipv4 input being overwritten

d87c0d3

clean up payload

0c2fed9

fix vlan ipam_address not nullable

3242a49

clean up error code

55bfffb

fix type errors

c1c4106

use VPCPanel

94cc92e

clean up and fix styling

fc008f2

Merge branch 'develop' into M3-7121-vpc-add-config-dialog-and-table

66015bf

Merge in latest develop and resolve conflicts

47a48cf

Fix styling of VPC panel in Linode Create flow

8e508c8

Formatting fixed for VPCPanel in Add/Edit Config dialog; Primary Inte…

0243356

…rface dropdown in Add/Edit Config dialog; casing adjustments for some variable names

dwiley-akamai changed the title ~~feat: [M3-7121] - Add VPC to Linode Add Config dialog~~ feat: [M3-7121 & M3-7122] - Support VPC in Linode Add/Edit Config dialog Sep 29, 2023

DevDW added 2 commits September 29, 2023 19:46

Refactors in LinodeConfigDialog.tsx; refactors in InterfaceSelect.tsx…

4f42031

… and fully fixed formatting

Move destructured props higher in function component body in Interfac…

8f919ce

…eSelect.tsx

dwiley-akamai reviewed Sep 30, 2023

View reviewed changes

packages/manager/src/features/Linodes/LinodesDetail/LinodeConfigs/LinodeConfigDialog.tsx Show resolved Hide resolved

DevDW added 2 commits October 2, 2023 13:36

Ensure primaryInterfaceIndex is 0 if we are in Create mode upon dialo…

8af257c

…g open; surface errors

Adjust useEffect() logic to handle for non-VPC cases to prevent VLAN bug

bc6ea6e

dwiley-akamai self-assigned this Oct 2, 2023

Don't display 'None' option for VPC interfaces in Config dialog

1c5d210

Fix Assign Linode bug; minor casing adjustment in variable name

6950b89

abailly-akamai requested changes Oct 6, 2023

View reviewed changes

packages/api-v4/src/linodes/types.ts Outdated Show resolved Hide resolved

packages/api-v4/src/linodes/types.ts Outdated Show resolved Hide resolved

Temporary solution for surfacing a couple of errors pending API fix

bfa3ea6

hana-akamai and others added 3 commits October 6, 2023 14:55

fix types

ccba866

Set custom breakpoint conditionally to stack elements sooner and avoi…

6922b0b

…d weird spacing in Add/Edit Linode Config dialog; make noWrap on VPC IPv4 checkbox text conditional so tooltip isn't obscured

Merge branch 'M3-7121-vpc-add-config-dialog-and-table' of https://git…

6d969d7

…hub.com/hana-linode/manager into M3-7121-vpc-add-config-dialog-and-table

abailly-akamai self-requested a review October 9, 2023 15:14

coliu-akamai self-requested a review October 9, 2023 18:19

dwiley-akamai mentioned this pull request Oct 10, 2023

fix: [M3-7251] Interface-level VPC IPv4 error in Add/Edit Linode Config dialog when VPC IPv4 checkbox is unchecked but VPC IPv4 not provided #9779

Closed

14 tasks

coliu-akamai approved these changes Oct 10, 2023

View reviewed changes

use invalidateQueries for VPCs if any of the config interfaces have a…

f3fbeac

… purpose === 'vpc'

abailly-akamai reviewed Oct 11, 2023

View reviewed changes

Merge branch 'develop' into M3-7121-vpc-add-config-dialog-and-table

d1a4830

jaalah-akamai approved these changes Oct 12, 2023

View reviewed changes

DevDW added 2 commits October 12, 2023 16:03

Feedback @abailly-akamai: type adjustments, unit tests

e9b411d

Add changesets

9ff5c34

dwiley-akamai requested a review from abailly-akamai October 12, 2023 21:25

abailly-akamai approved these changes Oct 16, 2023

View reviewed changes

dwiley-akamai merged commit 472cfa2 into linode:develop Oct 16, 2023
11 checks passed

hana-akamai mentioned this pull request Oct 17, 2023

fix: [M3-7270] - Edit Linode VLAN config interface #9798

Merged

abailly-akamai pushed a commit to abailly-akamai/manager that referenced this pull request Oct 17, 2023

feat: [M3-7121 & M3-7122] - Support VPC in Linode Add/Edit Config dia…

e777479

…log (linode#9709)

dwiley-akamai mentioned this pull request Oct 24, 2023

change: [M3-7250] – Properly surface VPC interface errors #9839

Merged

12 tasks

This was referenced Oct 26, 2023

Release v1.106.0 - release → staging #9842

Merged

Release v1.106.0 - staging → master #9850

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [M3-7121 & M3-7122] - Support VPC in Linode Add/Edit Config dialog #9709

feat: [M3-7121 & M3-7122] - Support VPC in Linode Add/Edit Config dialog #9709

hana-akamai commented Sep 21, 2023 •

edited by dwiley-akamai

Loading

dwiley-akamai commented Oct 2, 2023

dwiley-akamai commented Oct 5, 2023

dwiley-akamai commented Oct 6, 2023

coliu-akamai commented Oct 10, 2023 •

edited

Loading

coliu-akamai left a comment

abailly-akamai left a comment

abailly-akamai Oct 11, 2023

dwiley-akamai Oct 11, 2023

jaalah-akamai commented Oct 12, 2023

abailly-akamai left a comment •

edited

Loading

feat: [M3-7121 & M3-7122] - Support VPC in Linode Add/Edit Config dialog #9709

feat: [M3-7121 & M3-7122] - Support VPC in Linode Add/Edit Config dialog #9709

Conversation

hana-akamai commented Sep 21, 2023 • edited by dwiley-akamai Loading

Description 📝

Major Changes 🔄

How to test 🧪

dwiley-akamai commented Oct 2, 2023

dwiley-akamai commented Oct 5, 2023

dwiley-akamai commented Oct 6, 2023

coliu-akamai commented Oct 10, 2023 • edited Loading

coliu-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

abailly-akamai Oct 11, 2023

Choose a reason for hiding this comment

dwiley-akamai Oct 11, 2023

Choose a reason for hiding this comment

jaalah-akamai commented Oct 12, 2023

abailly-akamai left a comment • edited Loading

Choose a reason for hiding this comment

hana-akamai commented Sep 21, 2023 •

edited by dwiley-akamai

Loading

coliu-akamai commented Oct 10, 2023 •

edited

Loading

abailly-akamai left a comment •

edited

Loading