| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-10 | |||
| 20:01:39 | dansmith | sean-k-mooney: no it shouldn't | |
| 20:01:49 | dansmith | sean-k-mooney: read the man page | |
| 20:01:50 | imacdonn | dansmith: are you saying exit 2 if exceptions and total_ran>0, and exit 1 if ran (not total_ran) > 0 ? | |
| 20:02:20 | sean-k-mooney | so if max is greater then total you dont want the same behavior as if there was no max | |
| 20:02:27 | dansmith | imacdonn: I'm saying leave the 0 and 1 the way they are, and exit 2 if total_ran=0 and exceptions | |
| 20:02:36 | dansmith | sean-k-mooney: it's about definition of done-ness | |
| 20:02:58 | imacdonn | dansmith: that's what I had in PS2, and it broke grenade, because it exits with 1 if any migrations were done | |
| 20:03:21 | imacdonn | dansmith: or maybe I didn't interpret your last statement right | |
| 20:03:22 | dansmith | imacdonn: I'm not talking about changing the definition of 1 | |
| 20:03:22 | sean-k-mooney | yes which is why im saying if i set a max larger then the amount to do and they all ran sucsessfully im done hence 0 | |
| 20:03:24 | dansmith | imacdonn: right | |
| 20:03:40 | dansmith | imacdonn: if you don't change the logic for 1 then grenade can't break, unless we're actually hiding exceptions | |
| 20:05:02 | imacdonn | dansmith: OK, I think that might work ... I'll have to try it ... sean-k-mooney, do you still have concerns ? | |
| 20:05:06 | sean-k-mooney | dansmith: we used to hide exceptions but my understand of 1 is there are still more migration to run i would prefer it to be "there are more migration to run and all the ones i just ran did not have error" | |
| 20:05:19 | dansmith | sean-k-mooney: that's not what 1 means | |
| 20:05:27 | dansmith | we can't really know if there are more unless we try and hit zero | |
| 20:05:36 | dansmith | without doing more database stuff to count separately from doing | |
| 20:05:41 | dansmith | which is not worth it, IMHO | |
| 20:06:04 | dansmith | so, while (rc==1) { doit } | |
| 20:06:22 | imacdonn | personally, I kinda wish there was a way to find out how many migrations are needed, before attempting any | |
| 20:06:33 | imacdonn | I might want to do this when planning an upgrade | |
| 20:06:58 | sean-k-mooney | ok so 1 just used to mean there was a partial update so we need to loop that i think makes sense | |
| 20:07:05 | sean-k-mooney | and 0 means we are done | |
| 20:07:13 | dansmith | sean-k-mooney: no, 1 means we did things, that's ALL it means :) | |
| 20:07:15 | sean-k-mooney | dansmith: what is the smantics of 2 in your case | |
| 20:07:16 | dansmith | might be partial, might be full | |
| 20:07:18 | mriedem | sorrison: if you wanted to policy something, i'd think having policy on the ability to create multiple servers in a single request would be a good one so you can avoid tenants killing your scheduler with large burst multi-create requests | |
| 20:07:25 | mriedem | especially if those users have higher than normal quota | |
| 20:07:40 | dansmith | sean-k-mooney: hopefully this can be the last time I say this, but 2 means we couldn't make any more progress, and exceptions were raised | |
| 20:07:41 | mriedem | we probably had a policy on the multi-create API extension at some point | |
| 20:08:08 | mriedem | make it 2.5.... | |
| 20:08:09 | mriedem | and i'm sold | |
| 20:08:14 | dansmith | sean-k-mooney: so you can distinguish between "can't make any more progress" and "can't make any more progress, but things seem unhappy" | |
| 20:08:19 | sean-k-mooney | dansmith: ok but we dont stop trying migration on the first exception | |
| 20:08:32 | dansmith | sean-k-mooney: exactly | |
| 20:09:18 | sean-k-mooney | my piont it 2 would only ever be retruned if all the migrations in the first iteration failed | |
| 20:09:25 | dansmith | no | |
| 20:09:39 | mriedem | sorrison: maybe that's just never been a problem b/c of quota restrictions, the multi-create thing i mean | |
| 20:10:05 | dansmith | sean-k-mooney: did you mean the last iteration? my answer is still no, but... | |
| 20:10:29 | dansmith | sean-k-mooney: with the suggestion I just made to imacdonn, we keep a total_run count and use that for 2 instead of ran, which is just "the last iteration" | |
| 20:10:29 | imacdonn | no, 2 gets returned if the only remaining possible migrations barfed for some reason | |
| 20:10:38 | dansmith | imacdonn: right | |
| 20:10:38 | openstack | Launchpad bug 1795982 in OpenStack Compute (nova) "/os-console-auth-tokens/{console_token} API doesn't handle the database backend" [High,In progress] - Assigned to melanie witt (melwitt) | |
| 20:10:38 | mriedem | melwitt: isn't this more than just the db backend for console auth? https://bugs.launchpad.net/nova/+bug/1795982 - it's been regressed since multi-cell support in pike | |
| 20:10:56 | dansmith | imacdonn: maybe just tweak and push that up and we can argue about it after there's something to see? | |
| 20:11:12 | imacdonn | dansmith: roger. will do after lunch | |
| 20:11:12 | dansmith | I'm getting kinda frustrated with this overly minute detail and really want to get on to other stuff before I end my day | |
| 20:11:16 | dansmith | imacdonn: thanks | |
| 20:11:30 | melwitt | mriedem: I don't think so because nova-consoleauth is global, you don't need to know anything about cells to query it | |
| 20:11:34 | sean-k-mooney | dansmith: yes i was referring to "total_run==0 and exceptions" for retrun 2 | |
| 20:12:10 | efried | dansmith, imacdonn: I don't intend to get super involved in this, but have we considered allowing behavior change(s) (e.g. where exceptions cause failure) based on an env var or CLI switch? (Sorry, this thought has been bouncing around in my head for a week, had to get it out.) | |
| 20:12:10 | melwitt | mriedem: that is, it's storage of console token auths is global across all cells | |
| 20:12:40 | dansmith | efried: I don't think we need a behavior change here | |
| 20:12:48 | mriedem | hmm, ok https://docs.openstack.org/nova/queens/user/cellsv2-layout.html#consoleauth-service-and-console-proxies | |
| 20:12:52 | efried | ...so that existing automations aren't affected, but new users can have the benefit of potentially improved UX | |
| 20:12:54 | mriedem | got confused about what's global or not | |
| 20:12:57 | dansmith | efried: if we did, then sure | |
| 20:13:04 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Handle volume API failure in post_live_migration https://review.openstack.org/609517 | |
| 20:13:39 | sean-k-mooney | dansmith: imacdonn you know what im fine with what ever you implement and ill read teh code after to understand what the exit code actully mean. | |
| 20:13:51 | efried | Okay. When I stopped looking, the patch was suggesting a behavior change that seemed fair and just to me, but was nacked for that reason. | |
| 20:14:57 | imacdonn | efried: we've introduced a new exit status, so the logic for the existing ones doesn't have to change .. I think it'll only "break" when there are exceptions and no more possible migrations, and we *want* it to break in that case | |
| 20:15:02 | dansmith | efried: the only thing that could make this more dreadful would be two behaviors in the same set of code :) | |
| 20:15:13 | dansmith | imacdonn: ++ | |
| 20:15:40 | efried | dansmith: Hm, we should introduce a microversioning system for that. | |
| 20:15:54 | dansmith | efried: we should microversion your butt. | |
| 20:16:22 | dansmith | if we ever have nova-manage cellv2 thingy --cli-version=2.123 | |
| 20:16:25 | dansmith | then just shoot me | |
| 20:16:54 | melwitt | nova-consoleauth service | |
| 20:16:54 | melwitt | mriedem: yeah, so the /os-console-auth-tokens/{console_token} API calls nova-consoleauth over RPC, and nova-consoleauth was made cell-aware sometime in the past, so all was working fine with multi-cell (unless there's a bug we don't know about). but when we moved to the database backend, that's what made it so that the /os-console-auth-tokens API would need to be able to talk to cell databases directly instead of going through the | |
| 20:17:37 | efried | what we really need is nested microversions | |
| 20:17:59 | sean-k-mooney | imacdonn: retruaning any new code will still requrie autoation scirpt to handel the new error case or manual intervention so the fact there is a behavior change or not is slight less important that said in the sucess case the codes should not change | |
| 20:18:01 | mriedem | god i bet my cross-cell resize stuff needs to recreate console auth tokens for the moved instance in the target cell db too... | |
| 20:18:03 | mriedem | like bdms and tags | |
| 20:18:13 | mriedem | and virtual_interfaces... | |
| 20:19:18 | melwitt | you could probably just punt that though, let them have to get a fresh console token after a cross-cell move | |
| 20:19:31 | imacdonn | sean-k-mooney: that's true, and that's why there's a release note for this, and it may not be backportable ... but the most common case will be running the command without --max-count and expecting a 0, or rerunning it until you don't get 1 | |
| 20:19:55 | melwitt | default TTL for console token auth is 10 minutes so they aren't designed to live long. operators can configure longer TTL but I'm not sure they'd expect you to solve for that | |
| 20:20:44 | imacdonn | sean-k-mooney: I guess the possible case where it could break automation is if the command is being rerun infinitely until it gets a zero, which would never happen if it's returning 2 every time | |
| 20:20:47 | sean-k-mooney | imacdonn: if i was writingin this in ansiable and i called it without --max-count i would have interpereted a non 0 result as an error just fyi | |
| 20:21:14 | imacdonn | sean-k-mooney: right, and we're not going to change that | |
| 20:21:45 | imacdonn | sean-k-mooney: without --max-count, you'll either get 0 (it worked), or 2 (something unexpectedly broke, and you need to figure out why) | |
| 20:22:42 | sean-k-mooney | imacdonn: if you gurarentee that in the code that is fine | |
| 20:23:15 | imacdonn | sean-k-mooney: I believe I can .. I'll post it this afternoon, and we can nit-pick :) | |
| 20:24:22 | sean-k-mooney | ill be offline by then but enjoy your lunch and ill take a look at it tomorow | |
| 20:24:33 | imacdonn | k, thanks! | |
| 20:34:46 | sean-k-mooney | have people see a 404 failing to retriva allocationf form resource provierded before | |
| 20:34:48 | sean-k-mooney | http://logs.openstack.org/84/602384/4/check/kuryr-kubernetes-tempest-daemon-octavia/33adb32/controller/logs/screen-n-cpu.txt.gz?#_Oct_10_19_02_09_953771 | |
| 20:36:36 | openstackgerrit | Jack Ding proposed openstack/nova-specs master: High Precision Event Timer (HPET) on x86 guests https://review.openstack.org/607989 | |
| 20:36:49 | openstackgerrit | Jack Ding proposed openstack/nova-specs master: High Precision Event Timer (HPET) on x86 guests https://review.openstack.org/607989 | |
| 20:56:04 | mriedem | melwitt: ok +2 | |
| 20:57:47 | melwitt | thanks | |
| 21:10:12 | mriedem | sean-k-mooney: yes it's a known bug | |
| 21:10:41 | openstack | Launchpad bug 1789998 in OpenStack Compute (nova) "ResourceProviderAllocationRetrievalFailed ERROR log message on fresh n-cpu startup" [Low,Triaged] | |
| 21:10:41 | mriedem | https://bugs.launchpad.net/nova/+bug/1789998 | |
| 21:10:50 | mriedem | happens on every start of a new compute | |
| 21:14:14 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Don't log an error if attachment_create fails https://review.openstack.org/609547 | |
| 21:23:03 | sorrison | mriedem: Missing some context RE: "maybe that's just never been a problem b/c of quota restrictions, the multi-create thing i mean" | |
| 21:23:30 | mriedem | meaning maybe no one has ever felt the need to restrict certain groups of users from being able to make multi-create requests | |
| 21:24:00 | mriedem | because multi-create can be abused, e.g. https://review.openstack.org/#/c/607735/ | |
| 21:24:41 | mriedem | sorrison: maybe a better question is, what is the highest any of your tenants have for instance quota? | |