| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-19 | |||
| 13:43:51 | priteau | Could you please review? | |
| 14:23:12 | opendevreview | sean mooney proposed openstack/nova master: Force api to exit if exceptions are uncaught https://review.opendev.org/c/openstack/nova/+/944933 | |
| 14:24:02 | opendevreview | Bogdan Dobrelya proposed openstack/nova master: [DNM] Hard OS exit instead of SysExit exception https://review.opendev.org/c/openstack/nova/+/945011 | |
| 14:26:42 | opendevreview | sean mooney proposed openstack/nova master: Force api to exit if exceptions are uncaught https://review.opendev.org/c/openstack/nova/+/944933 | |
| 14:31:03 | opendevreview | Bogdan Dobrelya proposed openstack/nova master: [DNM] Hard OS exit instead of SysExit exception https://review.opendev.org/c/openstack/nova/+/945011 | |
| 15:10:59 | dansmith | sean-k-mooney: so one approach is to try to exit really really hard to get apache/mod_wsgi to notice... but, | |
| 15:11:31 | dansmith | what about us making sure we never raise and just return 500s if we would have otherwise failed? | |
| 15:11:48 | sean-k-mooney | hum | |
| 15:11:50 | dansmith | even without a dedicated healthcheck that would be easy to arrange to not be healthy | |
| 15:12:09 | sean-k-mooney | we would have to kind of hack it | |
| 15:12:10 | dansmith | curl /versions or whatever, always returning 500 would leave a pod unsuitable for traffic | |
| 15:12:23 | sean-k-mooney | but we could set a module level global i guess | |
| 15:12:30 | sean-k-mooney | and check that really early | |
| 15:12:30 | dansmith | sure, we'd need some way to record it and continue logging the config error or whatever so the operator saw it in the logs | |
| 15:12:47 | dansmith | sean-k-mooney: or just make application() return a different stub object if we failed to init the real one | |
| 15:13:12 | sean-k-mooney | perhaps. the issue is it reloading that applciation into the exisitng interperter | |
| 15:13:36 | sean-k-mooney | so we keep all the module level state like that partly parsed oslo config global | |
| 15:13:45 | dansmith | do we need to? | |
| 15:14:09 | sean-k-mooney | do we need to keep that state? | |
| 15:14:19 | dansmith | we could just have a safety proxy that sits around the real one and keeps trying to load the real one, eventually proxying to it if it loads, or returning the 500s if not | |
| 15:14:23 | sean-k-mooney | i was condiring trying to reset it so that we woul parse the config and raise the excption again | |
| 15:14:48 | dansmith | I kinda don't think we need to worry about the partial state | |
| 15:15:07 | sean-k-mooney | well that why its getting to the microverison endpoint when its reloaded | |
| 15:15:27 | dansmith | I imagine everyone doing this in a modern way is going to fix the config and restart a container when they do, so just having it be a terminal state if config is broken seems fine to me | |
| 15:15:27 | sean-k-mooney | just enough fo the state is thare that we get passed where the excption woudl be raised | |
| 15:15:55 | sean-k-mooney | right im just wondering how to make it terminal | |
| 15:16:29 | dansmith | I mean if mod_wsgi reloads us and we have to try initializing again and still fail, who cares? | |
| 15:16:35 | sean-k-mooney | im totally fine with changing the guts of the decorator to do something like record if it ever raised an excption | |
| 15:16:46 | sean-k-mooney | and then keep raising that if invoked in the future | |
| 15:16:51 | sean-k-mooney | that would be pretty clean i think | |
| 15:17:17 | sean-k-mooney | dansmith: right now when we are reloaded we dont fail on the init the second time | |
| 15:17:20 | dansmith | yeah, just set a global if we failed, stop trying, just return all the 500s, wait for them to fix config and restart | |
| 15:17:21 | sean-k-mooney | becuase of the leaked state | |
| 15:17:36 | dansmith | sean-k-mooney: ahh, I see, then yeah, some global to prevent us from trying | |
| 15:17:49 | sean-k-mooney | dansmith: lets wait for the current ci job to complete | |
| 15:18:05 | sean-k-mooney | ill refactor to raise 500 if we ever raised before and see what that looks like | |
| 15:18:07 | dansmith | I just feel like trying to sys.exit() harder and harder is sort of missing the point | |
| 15:18:12 | bogdando | we also have one testing the os._exit "fork" | |
| 15:18:25 | sean-k-mooney | ya i dont like calling that ^ | |
| 15:18:35 | dansmith | we should just detect, log, and signal (which for a web application is a 500 healthcheck, IMHO) | |
| 15:18:48 | dansmith | sean-k-mooney: agree about os._exit, that's what I mean by "harder" :) | |
| 15:19:23 | sean-k-mooney | ya so i dont know if you looked at my most recent version wich follows the reload docs | |
| 15:19:36 | sean-k-mooney | i choose to use sig kill instead of sig interupt | |
| 15:19:51 | sean-k-mooney | but either i think woudl work | |
| 15:20:12 | bogdando | sys exit is trying the door, but apache won't let us out | |
| 15:20:20 | sean-k-mooney | dansmith: i think your suggestion is worth trying as it shoudl be pretty clean. | |
| 15:20:26 | bogdando | os.exit - trying a window | |
| 15:20:38 | bogdando | 500 code - sitting in the room arguing with the door guard | |
| 15:21:04 | sean-k-mooney | 500s are more polite | |
| 15:21:13 | dansmith | sean-k-mooney: to me, the kill/reload thing is what we should do if we need to retry, but for bad config or almost all of our situations, we don't | |
| 15:21:20 | dansmith | so doing kill there just means we crashloop | |
| 15:21:44 | dansmith | starting up and returning 500 will signal that we're not happy instead of constantly looking like we're restarting | |
| 15:21:49 | sean-k-mooney | yep | |
| 15:22:02 | dansmith | I much prefer the "unhealthy" marker instead of the constant "starting..." status where you're not sure if you're still supposed to wait | |
| 15:22:10 | sean-k-mooney | long term either implemeting helthchecks or hooking the system ready socket is cleaner | |
| 15:22:26 | bogdando | the problem is we need a hard stop, no half measures | |
| 15:22:27 | sean-k-mooney | i.e. for knowing if we started properly | |
| 15:22:43 | bogdando | letting the service operating half-configured even a fraction of a second could be destructive | |
| 15:23:15 | sean-k-mooney | well if my current patch works it wont | |
| 15:23:21 | bogdando | (if we got unlucky enough) | |
| 15:23:25 | sean-k-mooney | and if we do the 500 thing it wont either | |
| 15:23:30 | dansmith | it's not going to operate half-configured either way if we never return application | |
| 15:23:32 | dansmith | right | |
| 15:23:33 | bogdando | ok, makes sense | |
| 15:23:46 | bogdando | what about a tail of rpc calls? | |
| 15:24:00 | dansmith | not sure what you mean | |
| 15:24:08 | bogdando | it won't accept new API requests, but what about a pending work | |
| 15:24:24 | sean-k-mooney | it gets dropped on the floor | |
| 15:24:25 | bogdando | woudln't it heppily pick it up after crash looping? | |
| 15:24:27 | bogdando | ok | |
| 15:24:39 | sean-k-mooney | the http request wont get into the applciation logic and it will get a 500 | |
| 15:24:54 | opendevreview | ribaudr proposed openstack/nova master: FUP Remove unnecessary PCI check https://review.opendev.org/c/openstack/nova/+/944105 | |
| 15:24:55 | opendevreview | ribaudr proposed openstack/nova master: FUP improve and add integration tests for PCI SR-IOV servers https://review.opendev.org/c/openstack/nova/+/944106 | |
| 15:24:55 | opendevreview | ribaudr proposed openstack/nova master: FUP Add a warning to make non-explicit live migration request debugging easier https://review.opendev.org/c/openstack/nova/+/944133 | |
| 15:24:56 | opendevreview | ribaudr proposed openstack/nova master: FUP Update pci-passthrough and virtual-gpu documentation https://review.opendev.org/c/openstack/nova/+/944153 | |
| 15:26:39 | bogdando | IIRC, there is a logic for recovering ongoing rpc calls (long running ops) after restarting the process. Wouldn't restarting half-configured introduce more problems that it solves, even if never taking new API requests? | |
| 15:26:56 | sean-k-mooney | no | |
| 15:27:21 | sean-k-mooney | config errors should not allow the api to ever come up | |
| 15:27:27 | dansmith | if we never return application, we can never start rpc calls | |
| 15:27:28 | sean-k-mooney | so you shoudl nto be able to start rpcs | |
| 15:27:29 | dansmith | I'm not sure what the concern is | |
| 15:27:49 | bogdando | oh ok then, sorry | |
| 15:27:51 | opendevreview | ribaudr proposed openstack/nova master: FUP Remove unnecessary PCI check https://review.opendev.org/c/openstack/nova/+/944105 | |
| 15:27:52 | opendevreview | ribaudr proposed openstack/nova master: FUP improve comment accuracy and variable naming for tag removal https://review.opendev.org/c/openstack/nova/+/943124 | |
| 15:27:52 | opendevreview | ribaudr proposed openstack/nova master: FUP Add a warning to make non-explicit live migration request debugging easier https://review.opendev.org/c/openstack/nova/+/944133 | |
| 15:27:53 | sean-k-mooney | also if you restart the api and it has done an rpc the reply is dropped on the floor | |
| 15:27:53 | opendevreview | ribaudr proposed openstack/nova master: FUP Update pci-passthrough and virtual-gpu documentation https://review.opendev.org/c/openstack/nova/+/944153 | |
| 15:27:53 | opendevreview | ribaudr proposed openstack/nova master: FUP improve and add integration tests for PCI SR-IOV servers https://review.opendev.org/c/openstack/nova/+/944106 | |
| 15:28:03 | dansmith | also, the 500 approach specifically avoids restarting if we know we're not going to be able to do so successfull | |
| 15:29:13 | sean-k-mooney | it will be about 2 hours until the ci job finishes | |
| 15:29:27 | sean-k-mooney | ill see if i can do a poc of the 500 approch this evenign or early tomorrow | |
| 15:29:47 | sean-k-mooney | i mean i can push one now | |
| 15:29:59 | bogdando | what about conductor, it service no API, right? but we cannot afford it restarted half-configured for some moment? sorry I am fully noob in Nova services architecture | |
| 15:30:00 | sean-k-mooney | but i guess i can triger a job via a second dnm change | |
| 15:30:11 | sean-k-mooney | the conductor is not a wsig process | |
| 15:30:11 | bogdando | it serves no api* | |
| 15:30:18 | sean-k-mooney | this si only a problem because of mod_wsgi | |
| 15:30:24 | bogdando | oh yes, indeed | |
| 15:30:25 | sean-k-mooney | all the rest exit on config error | |
| 15:30:27 | dansmith | yeah I'm not sure what conductor has to do with this.. it can use the usual systemd stuff | |