Skip to content

Conversation

charliepark
Copy link
Contributor

This adds an affordance to the instance networking tab, to prevent the deletion of a primary NIC when there are other NICs present.

As called out in #2652, we prevent the user from deleting the primary NIC when there are other NICs present. As Ben notes,

There is always zero or one primary NIC. There may zero or more secondary NICs (up to 7 today), but only if there is already a primary. The primary NIC is where we attach all the external networking state, like external addresses, and the VPC information like routes, subnet information, internet gateways, etc.

You may delete any secondary NIC. You may delete the primary NIC only if it's the only NIC (there are no secondary NICs). That's because we need a place to attach that external state -- if you delete a primary, we can only make an unambiguous choice of which secondary to promote if there is zero or one secondary NIC. Instead of trying to decide, and possibly getting it wrong, the user needs to pick.

In the linked issue, one approach was to add a modal, giving the user the ability to promote another NIC to primary. Happy to do a part 2 on this if we want to add that functionality, but wanted to at least disable the action for now.

Copy link

vercel bot commented Apr 22, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Apr 24, 2025 4:29pm

@david-crespo
Copy link
Collaborator

So weird for it to pass in Chrome but fail in FF and Safari. Maybe some accessible name implementation quirk.

@david-crespo
Copy link
Collaborator

As a test, I gave the PR diff and the test failure output to Gemini 2.5 Pro and it nailed the interpretation and the fix and explains it very nicely (conversation log).

  1. Blocking Element: The crucial part is: <button type="button" role="menuitem" ... data-disabled="" aria-disabled="true" ...>Delete ... intercepts pointer events This tells us that Playwright located the correct "Row actions" button for 'my-nic', but when it tried to physically perform the click, another element was in the way: the "Delete" menu item button (which is disabled).

I thought maybe the dropdown was actually covering the other row, like this

Screenshot 2025-04-23 at 11 38 01 AM

But it wasn't, it's just because it's a modal and I guess its overlay captures the pointer events. This is from the trace recorded by a local failure in Firefox:

Screenshot 2025-04-23 at 11 44 49 AM

@david-crespo
Copy link
Collaborator

david-crespo commented Apr 23, 2025

It's possible that with that fix in place (and maybe in a couple more spots) the other stuff in there dragging things out is no longer necessary. Just kidding, I was looking at one commit in isolation. I see you already took those changes out.

@charliepark
Copy link
Contributor Author

charliepark commented Apr 23, 2025

Good to know it found that, though! What's confounding me, I think, is that locally the tests are all working just peachy.

@david-crespo
Copy link
Collaborator

I'm guessing you were running it in Chrome only. I am able to repro the failure consistently in FF and Safari.

image

@david-crespo david-crespo merged commit 239e34b into main Apr 24, 2025
7 checks passed
@david-crespo david-crespo deleted the disable-primary-nic-delete branch April 24, 2025 16:48
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request May 7, 2025
oxidecomputer/console@99a1f00...46e244f

* [46e244fd](oxidecomputer/console@46e244fd) oxidecomputer/console#2818
* [9811a9c7](oxidecomputer/console@9811a9c7) element=null on root redirect route to silence warnings
* [b8e9f385](oxidecomputer/console@b8e9f385) oxidecomputer/console#2821
* [f097ec96](oxidecomputer/console@f097ec96) oxidecomputer/console#2819
* [585fe913](oxidecomputer/console@585fe913) oxidecomputer/console#2815
* [973d332a](oxidecomputer/console@973d332a) oxidecomputer/console#2817
* [ad61a081](oxidecomputer/console@ad61a081) oxidecomputer/console#2816
* [239e34b9](oxidecomputer/console@239e34b9) oxidecomputer/console#2806
* [376f172f](oxidecomputer/console@376f172f) oxidecomputer/console#2805
* [5c7c8b5b](oxidecomputer/console@5c7c8b5b) oxidecomputer/console#2746
* [1d8c3b76](oxidecomputer/console@1d8c3b76) bump playwright, use more cores to run e2es locally
* [f493cb35](oxidecomputer/console@f493cb35) oxidecomputer/console#2772
* [819b99a6](oxidecomputer/console@819b99a6) chore: type error after RR 7.5. this is why we make PRs
* [c1134cff](oxidecomputer/console@c1134cff) chore: react-router 7.5.0
* [c1f0d78b](oxidecomputer/console@c1f0d78b) chore: vite 6.3.0 + vitest 3.1.1
* [79c6bc53](oxidecomputer/console@79c6bc53) oxidecomputer/console#2803
* [f454f5ee](oxidecomputer/console@f454f5ee) oxidecomputer/console#2804
* [ccfa5a58](oxidecomputer/console@ccfa5a58) fix deploy-dogfood script when used with a tag
* [6196fa48](oxidecomputer/console@6196fa48) invalidate anti-affinity group member lists on instance action success
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request May 7, 2025
oxidecomputer/console@99a1f00...46e244f

* [46e244fd](oxidecomputer/console@46e244fd)
oxidecomputer/console#2818
* [9811a9c7](oxidecomputer/console@9811a9c7)
element=null on root redirect route to silence warnings
* [b8e9f385](oxidecomputer/console@b8e9f385)
oxidecomputer/console#2821
* [f097ec96](oxidecomputer/console@f097ec96)
oxidecomputer/console#2819
* [585fe913](oxidecomputer/console@585fe913)
oxidecomputer/console#2815
* [973d332a](oxidecomputer/console@973d332a)
oxidecomputer/console#2817
* [ad61a081](oxidecomputer/console@ad61a081)
oxidecomputer/console#2816
* [239e34b9](oxidecomputer/console@239e34b9)
oxidecomputer/console#2806
* [376f172f](oxidecomputer/console@376f172f)
oxidecomputer/console#2805
* [5c7c8b5b](oxidecomputer/console@5c7c8b5b)
oxidecomputer/console#2746
* [1d8c3b76](oxidecomputer/console@1d8c3b76)
bump playwright, use more cores to run e2es locally
* [f493cb35](oxidecomputer/console@f493cb35)
oxidecomputer/console#2772
* [819b99a6](oxidecomputer/console@819b99a6)
chore: type error after RR 7.5. this is why we make PRs
* [c1134cff](oxidecomputer/console@c1134cff)
chore: react-router 7.5.0
* [c1f0d78b](oxidecomputer/console@c1f0d78b)
chore: vite 6.3.0 + vitest 3.1.1
* [79c6bc53](oxidecomputer/console@79c6bc53)
oxidecomputer/console#2803
* [f454f5ee](oxidecomputer/console@f454f5ee)
oxidecomputer/console#2804
* [ccfa5a58](oxidecomputer/console@ccfa5a58)
fix deploy-dogfood script when used with a tag
* [6196fa48](oxidecomputer/console@6196fa48)
invalidate anti-affinity group member lists on instance action success
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants