| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-10 | |||
| 18:51:46 | dansmith | mriedem: was going to ask you this morning if you had fixed that yet :D | |
| 18:54:04 | dansmith | omg it works! | |
| 19:03:42 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: Handle volume API failure in post_live_migration https://review.openstack.org/609517 | |
| 19:08:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix NoneType error in _notify_volume_usage_detach https://review.openstack.org/609518 | |
| 19:13:36 | mriedem | dansmith: i needed something to do besides review code and/or specs | |
| 19:25:33 | openstackgerrit | Merged openstack/nova-specs master: Update blueprint name so spec matches launchpad https://review.openstack.org/607347 | |
| 19:27:59 | imacdonn | dansmith: I implemented what I understood from our discussion yesterday at https://review.openstack.org/608091 . It needs a little polish, but sean-k-mooney wants to use new the exit status if there are any exceptions, whether or not any migrations may still be pending. I don't think we can do that, because it can't be automated | |
| 19:41:09 | mriedem | hey gang, two easy +Ws https://review.openstack.org/#/c/608802/ https://review.openstack.org/#/c/609467/ | |
| 19:48:19 | melwitt | dansmith, mriedem: I linked my nova-consoleauth patch on L52 here that is ready for subteam review https://etherpad.openstack.org/p/stein-nova-subteam-tracking | |
| 19:50:07 | artom | I thought we had functional live migration tests? | |
| 19:50:49 | artom | Ah, nova/tests/functional/test_servers.py | |
| 19:50:53 | artom | Ignore me | |
| 19:51:26 | dansmith | imacdonn: I don't understand sean's concern or desire | |
| 19:53:57 | imacdonn | dansmith: thanks for commenting. One thing is still a bit fuzzy .. what does "work was done" mean? The way "ran" is currently implemented, it only counts how many rows were migrated *in the last batch*, so of you use the default of 50 at a time, ran will always end up as 0 | |
| 19:55:19 | imacdonn | ... because that's the only way it can break out of the loop | |
| 19:55:26 | dansmith | imacdonn: ran is the sum of all the "done" values from any migration it4eration right? | |
| 19:55:43 | dansmith | ran becoming nonzero is how you break out | |
| 19:55:45 | imacdonn | dansmith: no, only the last iteration | |
| 19:56:23 | imacdonn | dansmith: it gets reset to 0 at https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L718 | |
| 19:56:38 | dansmith | imacdonn: oh I see, but that's a bug I guess | |
| 19:56:47 | dansmith | from when this went from a fixed number to having an --until-done | |
| 19:56:49 | dansmith | or whatever | |
| 19:57:02 | dansmith | or the opposite, but you know what I mean | |
| 19:57:10 | sean-k-mooney | dansmith: we break out 1 of 2 way. ran becomes 0 or we pass --max-count in which case we do not loop as unlimited is false | |
| 19:57:11 | dansmith | so yeah you have to fix that for this to work | |
| 19:57:31 | imacdonn | dansmith: that's what I need to get nailed down ... I tried to fix that, in PS2, but made grenade blow up, becaused grenade needs the command to exit with status 0 | |
| 19:58:01 | dansmith | imacdonn: well, if this is wrong it's possible grenade is wrong | |
| 19:58:19 | jaypipes | mriedem: yesterday, which scheduler filter did you say already looked at instance metadata by querying the BuildRequest? | |
| 19:58:41 | mriedem | jaypipes: the one you're writing | |
| 19:58:42 | dansmith | oh wait, | |
| 19:58:45 | dansmith | maybe I'm remembering this now | |
| 19:58:54 | imacdonn | dansmith: it's possible, yes ... although it seems like if you run the command without --max-count, an outcome of exit status 0 (generally interpreted as success), is what would be expected | |
| 19:59:05 | dansmith | right | |
| 19:59:05 | dansmith | in the unlimited case, you really need the exit code to be zero | |
| 19:59:30 | dansmith | oof, this should be commented in here for sure | |
| 19:59:58 | imacdonn | so that's how I came up with the term "migrations may still be pending" .... when ran is not zero, which can only happen if you use --max-count | |
| 19:59:58 | dansmith | imacdonn: so maybe just keep the full count separate from ran and use that for the gate on exit 2 | |
| 20:00:08 | sean-k-mooney | dansmith: yes if you dont pass --max-count the exit 0 should mean all migration ran sucessfuly | |
| 20:00:14 | dansmith | imacdonn: like I say, you don't know what is pending or not really | |
| 20:00:26 | imacdonn | dansmith: that's why I used "may" :) | |
| 20:00:32 | dansmith | imacdonn: don't. | |
| 20:00:36 | sean-k-mooney | actully if you do pass --max-could exit 0 should still mean the same thing | |
| 20:00:44 | dansmith | imacdonn: so if you keep total_ran and use that then you're good right? | |
| 20:01:01 | dansmith | sean-k-mooney: no it shouldn't | |
| 20:01:16 | dansmith | sean-k-mooney: because you ran --max-count=50 and you get zero, you expect you're done | |
| 20:01:31 | dansmith | sean-k-mooney: if you don't pass --max-count, zero means all of them were completed, no errors so you're done | |
| 20:01:32 | sean-k-mooney | if there are 50 migrations and i pass --max-count=100 it should return 0 if all 50 ran successfully | |
| 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 | |