| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-09 | |||
| 20:11:29 | jaypipes | :) | |
| 20:11:29 | mriedem | so mgagne's use case can use the API exclusively and CERN can use the config exclusively | |
| 20:11:39 | mriedem | jaypipes: yes yikun is happy to pick it up, | |
| 20:11:43 | jaypipes | cool, thx | |
| 20:11:46 | mriedem | but he doesn't understand what the direction is... | |
| 20:11:48 | sean-k-mooney | mriedem: yes i think we said if you want to be api drive in the config you set the value to none or remvoe it | |
| 20:11:54 | mriedem | which is why i'm trying to be a middleman here | |
| 20:12:12 | sean-k-mooney | mriedem: if you want to be config driven you set the config value and dont touch it from the api | |
| 20:12:43 | jaypipes | mriedem: you are wonderful middleware. | |
| 20:13:19 | mriedem | and yikun had a question, "How to address the upgrade case? If we already have a 0.0 cpu ratio in db, should we change it to 16.0 first? online migration?" | |
| 20:13:55 | sean-k-mooney | mriedem: spefically you set cpu_allocation_ratio=None initall_cpu_allocation_ratio=16.0 if you want to set a default for new node but manage the actuall value form api | |
| 20:14:04 | mriedem | would we change the ComputeNode.*_allocation_ratio to the config value on read if the value in the db is 0.0? | |
| 20:14:13 | sean-k-mooney | and set cpu_allocation_ratio=x if you want to manage via config | |
| 20:15:48 | sean-k-mooney | mriedem: does that make sense? | |
| 20:16:42 | mriedem | yes i get that, | |
| 20:16:52 | mriedem | the question is upgrades https://review.openstack.org/#/c/552105/5/specs/stein/approved/initial-allocation-ratios.rst@114 | |
| 20:18:29 | sean-k-mooney | mriedem: i guess on upgrade if 0.0 is set in the db that would also imply that the resouce provider allocation_ratio or what ever is 0. | |
| 20:18:37 | sean-k-mooney | 0.0 also correct | |
| 20:20:09 | sean-k-mooney | mriedem: if the resouce provide exists and we get 0.0 from the db but placemetn has another vaule i would assuem we shoudl keep the placement value but not sure if that case can happen today | |
| 20:21:10 | sean-k-mooney | the compute node will just override placement with the value it gets from the resouce tracker today in update provider tree right? | |
| 20:25:04 | mriedem | the allocation ratio in placement can't be 0.0 | |
| 20:25:09 | mriedem | it will literally shit itself | |
| 20:26:16 | cfriesen | I assume it | |
| 20:26:21 | cfriesen | it's a divide by zero thing? | |
| 20:26:30 | sean-k-mooney | mriedem: ok in that case the logic is simple. on upgrade if placement provider exits and db value is 0 set db value to placement value. if not placement provider exeits set db to intiall_* value and create provider as normal | |
| 20:26:31 | cfriesen | should placement check for that? | |
| 20:27:20 | openstackgerrit | iain MacDonnell proposed openstack/nova master: Handle online_data_migrations exceptions https://review.openstack.org/608091 | |
| 20:27:36 | sean-k-mooney | cfriesen: its not actully a device by 0 but we multiply the available capasity by 0 and see if its larger then what we requested | |
| 20:28:05 | sean-k-mooney | cfriesen: so placement will not have a math error but you wont be able to get allocation against that resouce provider ever | |
| 20:28:12 | cfriesen | ah, thanks | |
| 20:50:31 | efried | mriedem: We talked about this in the sched meeting yesterday. jaypipes said he was about ready to abandon those two specs. We also discussed the possibility of generic inventory yaml leading to a solution. | |
| 20:51:21 | mriedem | efried: i know i was there and said i'd reach out to yikun to pick up the specs, | |
| 20:51:26 | mriedem | but he's confused about the direction, as am i | |
| 20:51:28 | mriedem | hence questions | |
| 20:51:43 | mriedem | i'm going through https://review.openstack.org/#/c/552105/ again now, | |
| 20:51:50 | mriedem | some of that is outdated given https://github.com/openstack/nova/commit/2588af87c862cfd02d860f6b860381e907b279ff | |
| 21:07:52 | mriedem | alright i've dumped comments in https://review.openstack.org/#/c/552105/ - i think i could probably update the spec at this point to cover the upgrade impact | |
| 21:08:11 | mriedem | i don't think we should leave the existing options defaulting to 0.0 like bauzas is asking for - that just prolongs the confusion of what those defaults mean | |
| 21:08:31 | mriedem | jaypipes: if you can skim my comments to see if they make sense i can take over updating the spec | |
| 21:20:49 | sean-k-mooney | mriedem: are you proposing defaulting them to None or 16.0/ what ever the real default is for that resource ? | |
| 21:21:08 | mriedem | what the spec says | |
| 21:21:21 | mriedem | change the *_allocation_ratio defaults from 0.0 to None | |
| 21:21:40 | mriedem | the initial_*_allocation_ratio defaults become what is in the ComputeNode object facade today | |
| 21:21:48 | mriedem | and we drop the facade | |
| 21:22:13 | sean-k-mooney | right that makes sense to me and inital_*_allocation_ratios will have per resouce type defaults correct | |
| 21:22:26 | mriedem | yes | |
| 21:22:53 | sean-k-mooney | ya that all sound sane to me. i have not read bauzas comment arguing for keeping 0.0 | |
| 21:23:30 | sean-k-mooney | current 0.0 has a special meaning right? e.g. use schduler/conductor values not compute node os somthing like that | |
| 21:24:02 | sean-k-mooney | i assume that is what his comment was related too. | |
| 21:26:02 | sean-k-mooney | the nova-status check makes sense but im not sure you can check the config as part of it | |
| 21:27:15 | mriedem | if the config explicitly sets the *_allocation_ratios to 0.0 when we have changed the defaults to None, that means their config mgmt system is setting that on purpose and is likely busted | |
| 21:29:03 | sean-k-mooney | mriedem: ture i just was thinking for FFU or in general the nova status command cant check the config on each compute unless you ran it on each compute | |
| 21:30:33 | sean-k-mooney | that said if you use oslo configs ablity to auto generate configs does it generate the config with all the values commeted out or set to there default. just trying to think if there was a resonable reason why it might be set to 0.0 | |
| 21:30:46 | mriedem | the allocation ratios aren't read on control plane services, so i think it's reasonable to assume if someone's config said 0.0 for those values when the defaults are None they are just setting the config globally and it's wrong | |
| 21:34:13 | sean-k-mooney | sorry i thnk i missed that bit where will these new values be read? scheduler/condctor or compute node | |
| 21:35:16 | sean-k-mooney | i had assuemed this was all config that was being read by the compute node? | |
| 21:38:19 | mriedem | the compute is what sets it | |
| 21:38:23 | mriedem | the scheduler will read it | |
| 21:38:26 | mriedem | from the compute node object | |
| 21:39:32 | mriedem | the only service that reads the config for these options is the compute service | |
| 21:41:35 | openstackgerrit | Adam Harwell proposed openstack/nova master: Add apply_cells to nova-manage https://review.openstack.org/568987 | |
| 21:42:14 | sean-k-mooney | mriedem: oh ok i was under the impression if the sechduler recived a 0.0 from the compute node it would read its own config and use the allocation ratio it got. was that how it used to work or am i just imagining things. | |
| 21:43:42 | sean-k-mooney | mriedem: by in anycase i think the nova-status check is sufficent. if there is a 0.0 in the db for a value the operator should first update there config and then upgrate/run online migration whatever is needed | |
| 21:44:40 | sean-k-mooney | i.e. im agreeing with your suggestion thanks for explaining :) | |
| 21:48:07 | imacdonn | mriedem dansmith: Fixing the migrations thing made grenade go boom ... there actually was another latent bug that I stumbled on, which was causing the exit code to be zero even though work had been done - with that fixed, grenade is not doing the right thing (repeating until exit status 0) | |
| 21:48:45 | sean-k-mooney | imacdonn: what is the other bug? | |
| 21:49:39 | mriedem | jaypipes: yikun: i've also gone through https://review.openstack.org/#/c/544683/ and left comments; i'm not on board with all it's proposing, but i think some of that is outdated now per the ptg discussions | |
| 21:49:44 | imacdonn | sean-k-mooney: "ran" gets reset to zero for each iteration of the loop here: https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L718 | |
| 21:49:55 | mriedem | i think the gist of ^ is that it's proposing to proxy aggregate allocation ratio metadata to placement, correct? | |
| 21:50:26 | imacdonn | sean-k-mooney: then it gets used later to determine if any migrations were ran/run at all here: https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L739 | |
| 21:51:08 | imacdonn | sean-k-mooney: so if the last iteration of the loop didn't do anything (even though a previous iteration did), it'd not count | |
| 21:52:05 | sean-k-mooney | imacdonn im not sure that is incorrect. if the last iteration did nothing it means migrations was empty so we break out of the while | |
| 21:52:36 | imacdonn | sean-k-mooney: yes, but the decision about whether or not any work had been done needs to consider ALL iterations of the loop | |
| 21:53:14 | sean-k-mooney | does it? why? | |
| 21:53:34 | imacdonn | sean-k-mooney: because, IIUC, that's what exit code 1 means (some migrations did work) | |
| 21:55:26 | imacdonn | I interpret that as "if ran is not zero, return 1, otherwise return 0" | |
| 21:56:34 | sean-k-mooney | imacdonn: yes but im trying to think what does that logically mean | |
| 21:56:58 | sean-k-mooney | is return 0 been used to indicate sucess like in bash or does 1 indicate sucess | |
| 21:58:05 | imacdonn | sean-k-mooney: it's complicated :) (again, IIUC)... zero means that there is no migration work remaining to be done, 1 means "some migrations did work, and there may be some more work that still needs to be done: | |
| 21:58:38 | imacdonn | so you're supposed to keep running the command until it doesn't return 1 | |
| 21:58:51 | sean-k-mooney | imacdonn: right in that cae you want ran to be 0 when all pending migrations have been processed so you want to reset it in the loop | |
| 21:59:49 | imacdonn | sean-k-mooney: no... you're supposed to re-run the command ... that's not what that loop is for | |
| 22:00:30 | sean-k-mooney | that is not how i read how it is currently written | |
| 22:01:13 | sean-k-mooney | from the current code it looks like its intent is to run all migration in batches up to max count and then exit when there are none left | |
| 22:02:09 | imacdonn | sean-k-mooney: yes, but there are scenarios where some of them will not work the first time (due to dependencies on others), so oit' | |
| 22:02:24 | imacdonn | it's necessary to iterate the whole command until you get a 0 | |
| 22:02:32 | imacdonn | .... is what I've been led to understand | |
| 22:02:45 | imacdonn | see comments on https://review.openstack.org/#/c/608091/ | |
| 22:04:21 | sean-k-mooney | this comment https://review.openstack.org/#/c/608091/1/nova/cmd/manage.py@753 or another one | |
| 22:04:56 | imacdonn | yes, those | |
| 22:05:01 | sorrison | mriedem I have another fun policy change https://review.openstack.org/#/c/608474/ | |
| 22:06:44 | sorrison | mriedem: I've been trying to figure out the whole admin/user context thing with tests and requests. I think I'm missing something | |
| 22:06:47 | sean-k-mooney | so from mriedem comment we did not raise exceptions before so while we said we coudl exit with a 1 error code we did not | |
| 22:08:04 | imacdonn | sean-k-mooney: I'm still not completely clear on what the exit code should be ... I thought I understood that '1' means "some migrations did work" | |
| 22:11:01 | sean-k-mooney | i think for mriedem comment 1 means some migrations ran but there were no errors and he was suggesting 2 for some migrations ran and there were errros and 0 means all migration ran and no errors | |
| 22:11:48 | imacdonn | I'm not sure how to distinguish between "some ran" and "all ran" | |
| 22:12:40 | sean-k-mooney | imacdonn: if you specify make count before your change and you havde more migration pending then max count it would have exited with 1 | |
| 22:13:19 | sean-k-mooney | so just change the retur on line 753 to 2 then be correct but we need a docs update to say what 2 means | |
| 22:14:32 | imacdonn | sean-k-mooney: the suggestion from today was to change it to return 2 *only if no migrations did work this time* | |
| 22:14:56 | sean-k-mooney | when --max-count is set unlimited is set to false so we break out of the loop on line 735 and ran is non 0 so " retrun ran and 1 or 0" returns 1 | |