Skip to content
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

Error when trying to attach a floating IP when an instance is starting #2361

Open
augustuswm opened this issue Aug 9, 2024 · 4 comments
Open
Assignees
Labels
Milestone

Comments

@augustuswm
Copy link

If you attempt to attach a floating IP while an instance is in the starting state you will get the following error:

image

This is an expected behavior, but in the future we should likely prevent this action until the instance is in a state that will accept the request.

@david-crespo
Copy link
Collaborator

We should try and get a better error message out the API there too. It’s possible that we are getting one and not displaying it because it’s a 503. Also is 503 really the right status code for this?

@augustuswm
Copy link
Author

This came up today (and I almost opened a new issue for it hah). I did confirm that we are getting back a 503 with an external message that mirrors the status code. So this is likely something that should be fixed in the API to provide more context to the error being sent back. I'm not sure there is a great error code to be returned here, but 503 fits decently close with its emphasis on it being temporary.

@david-crespo
Copy link
Collaborator

This would be pretty easy to fix by adding something like modifyIp: ['running', 'rebooting', 'stopped'] to the list here and disabling attach/detach in other other states:

console/app/api/util.ts

Lines 92 to 123 in efcaba7

const instanceActions: Record<string, InstanceState[]> = {
// NoVmm maps to to Stopped:
// https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-model/src/instance_state.rs#L55
// https://github.com/oxidecomputer/omicron/blob/0496637/nexus/src/app/instance.rs#L2064
start: ['stopped', 'failed'],
// https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/instance.rs#L865
delete: ['stopped', 'failed'],
// https://github.com/oxidecomputer/omicron/blob/3093818/nexus/db-queries/src/db/datastore/instance.rs#L1030-L1043
update: ['stopped', 'failed', 'creating'],
// reboot and stop are kind of weird!
// https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L790-L798
// https://github.com/oxidecomputer/propolis/blob/b278193/bin/propolis-server/src/lib/vm/request_queue.rs
// https://github.com/oxidecomputer/console/pull/2387#discussion_r1722368236
reboot: ['running'], // technically rebooting allowed but too weird to say it
// stopping a failed disk: https://github.com/oxidecomputer/omicron/blob/f0b804818b898bebdb317ac2b000618944c02457/nexus/src/app/instance.rs#L818-L830
stop: ['running', 'starting', 'rebooting', 'failed'],
// https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/disk.rs#L323-L327
detachDisk: ['creating', 'stopped', 'failed'],
// only Creating and NoVmm
// https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/disk.rs#L185-L188
attachDisk: ['creating', 'stopped'],
// primary nic: https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/network_interface.rs#L761-L765
// non-primary: https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/db-queries/src/db/datastore/network_interface.rs#L806-L810
updateNic: ['stopped'],
// https://github.com/oxidecomputer/omicron/blob/6dd9802/nexus/src/app/instance.rs#L1520-L1522
serialConsole: ['running', 'rebooting', 'migrating', 'repairing'],
}

The problem is I need to nail down exactly which states allow IP attach/detach. It looks like this is probably the relevant code. @gjcolombo or @FelixMcFelix, does ['running', 'rebooting', 'stopped'] sound good for states you're allowed to attach or detach IPs in?

@gjcolombo
Copy link

I think that's right, from my recollection of what the code you linked is trying to do (I agree that it's the relevant bit here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants