fix: [M3-8196] - Fix Linode Edit Config warning message when initially selecting a VPC as the primary interface #11424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
coliu-akamai
added
Bug
coliu-akamai
commented
Dec 16, 2024
There was a problem hiding this comment.
I'm not too familiar with configs, but could someone explain why if a config only has a public internet primary interface, configs.interface = [], but if a config has public internet and VPC interfaces, then configs.interfaces contains an interface with purpose = public?
vs
| @@ -1004,7 +1004,7 @@ export const LinodeConfigDialog = (props: Props) => { | |||
| disabled={isReadOnly} | |||
| label="Primary Interface (Default Route)" | |||
| options={getPrimaryInterfaceOptions(values.interfaces)} | |||
| value={primaryInterfaceOptions[primaryInterfaceIndex ?? 0]} | |||
There was a problem hiding this comment.
Users would see eth0 initially selected as the primary interface and be confused why the warning notice for 'selecting VPC as the primary interface' displays when they set eth0 to VPC. However, for the Edit Config flow, the notice was displayed bc primaryInterfaceIndex starts out as undefined, and stays undefined if there is no existing primary interface (lines 505-511).
(alternatively, could instead have an else case for lines 505-511 inside that useEffect --
if (indexOfExistingPrimaryInterface !== -1) {
setPrimaryInterfaceIndex(indexOfExistingPrimaryInterface);
} else {
setPrimaryInterfaceIndex(0);
}
bnussman-akamai
approved these changes
Dec 16, 2024
…lecting a VPC as the primary interface
coliu-akamai
force-pushed
the
m3-8196
branch
from
ad9b550 to
8837730
Compare
|
@coliu-akamai I don't know if this is 100% correct but I think configs with an empty interfaces array just implies that a default public eth0 interface will be used by the Linode |
coliu-akamai
changed the title
|
Coverage Report: ✅ |
Cloud Manager UI test results🎉 466 passing tests on test run #5 ↗︎
|
abailly-akamai
approved these changes
Dec 16, 2024
coliu-akamai
added
Approved
Cloud Manager E2E
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| Project |
Cloud Manager E2E
|
| Branch Review |
develop
|
| Run status |
|
| Run duration | 29m 53s |
| Commit |
|
| Committer | Connie Liu |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
2
|
Flaky
|
2
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
464
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
rebuild-linode.spec.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| rebuild linode > rebuilds a linode from Account StackScript |
Screenshots
Video
|
|
linode-config.spec.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Linode Config management > End-to-End > Clones a config |
Screenshots
Video
|
|
linodes/search-linodes.spec.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| Search Linodes > create a linode and make sure it shows up in the table and is searchable in main search tool |
Screenshots
Video
|
|
vpc/vpc-linodes-update.spec.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| VPC assign/unassign flows > can unassign Linode(s) from a VPC |
Screenshots
Video
|
|
4 tasks
This was referenced Jan 8, 2025
dmcintyr-akamai
pushed a commit
to dmcintyr-akamai/manager
that referenced
this pull request
Jan 9, 2025
…y selecting a VPC as the primary interface (linode#11424) * set initial primaryInterfaceIndex to 0 * extra space oops * some type cleanup * Added changeset: Linode Edit Config warning message when initially selecting a VPC as the primary interface
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description 📝
Changes 🔄
Target release date 🗓️
next release - 1/14?
Preview 📷
How to test 🧪
With a Linode not assigned to a VPC,
Reproduction steps
Verification steps
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
As an Author, before moving this PR from Draft to Open, I confirmed ✅