| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-09 | |||
| 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 | |
| 22:16:26 | sean-k-mooney | imacdonn: ok in that case on line 734 add if got_exceptions: return 2 | |
| 22:17:53 | sean-k-mooney | infact you could do the exception check on line 725 if you really wanted to exit fast | |
| 22:18:09 | imacdonn | that would be bad | |
| 22:18:29 | imacdonn | because some other migration may be a dependency for the first one to not fail, and you'd never get to run the second one | |
| 22:19:13 | sean-k-mooney | you jsut said you want to exit if there are excpetions | |
| 22:19:27 | imacdonn | nope, pretty sure I didn't | |
| 22:19:49 | imacdonn | I said they want the final exit status to be 2, if there were exceptions AND no migrations did any work | |
| 22:20:10 | sean-k-mooney | oh "*only if no migrations did work this time*" i misread that | |
| 22:20:20 | mriedem | sorrison: comments inline | |
| 22:21:01 | imacdonn | I guess "did work" is a confusing term ... "took effect"? ...... | |
| 22:21:16 | sean-k-mooney | mriedem: so we are gueessing about what you want return 2 to mean for https://review.openstack.org/#/c/608091/1/nova/cmd/manage.py@753 | |
| 22:21:29 | sorrison | Thanks mriedem :-) | |
| 22:21:34 | sean-k-mooney | mriedem: since your hear can you clarify for imacdonn | |
| 22:23:33 | sean-k-mooney | mriedem: based on the current code return 1 could have been returned if we passed --max-count and we had more the max-count migration so i think retrun 1 means some migrations ran with no errors and there are more to run and return 2 should be mean there were errors when runign migrations you better check out what went wrong | |
| 22:26:59 | imacdonn | sean-k-mooney: per dansmith, we should only return 2 if there were exceptions *and* we've determined that no other migrations did work (i.e. modified rows) | |
| 22:28:14 | sean-k-mooney | imacdonn: i dont see that in his comment on the review. was that in the schduler meeting? | |
| 22:28:19 | mriedem | it was in channel | |
| 22:28:21 | mriedem | several hours ago | |
| 22:28:31 | mriedem | and it sounds like you guys are talking about it all over again to come back to the same conclusion? | |
| 22:28:55 | sean-k-mooney | no there was a vlaid case to return 1 before | |
| 22:29:20 | mriedem | "(11:25:34 AM) dansmith: what if 2 means "I didn't do anything but there were exceptions", 1 means "I did things, maybe there were some exceptions too", 0 means "I didn't do anything, but no errors"" | |
| 22:29:30 | imacdonn | mriedem: the sticking point now is the meaning of '1' | |
| 22:29:33 | sean-k-mooney | we would have return one if we set a max count and there were more then max count migrations | |
| 22:30:27 | imacdonn | mriedem: In the current implementation, if it runs through all migrations (in batches), the final return code is 0, even though work was done | |
| 22:30:57 | sean-k-mooney | mriedem: so if there were 100 migrations to run and we set --max-count=90 before and all 90 ran sucessfully we would have retruned 1 to indicate there are more to run | |
| 22:31:02 | imacdonn | in my interpretation of the discussion this morning, it should be '1' if work was done, even if there is no work remaining to do | |
| 22:32:14 | sean-k-mooney | mriedem: 0 used to mean i ran all migrtions sucessfuly since 0 is sucess on bash | |
| 22:33:20 | mriedem | sorry but i'm past the point of having attention to think about this today | |
| 22:33:56 | sean-k-mooney | mriedem: no worries am ill leave a comment in the patch with what i understand the current logic is and you and dan can check when ye have had some rest | |
| 22:34:18 | sean-k-mooney | imacdonn: are you ok with waiting for them to look at this tomorrow? | |
| 22:34:44 | imacdonn | sean-k-mooney: sure | |
| 23:22:49 | openstackgerrit | sean mooney proposed openstack/nova master: add get_pci_requests_from_vifs to request.py https://review.openstack.org/609166 | |
| 23:56:48 | openstackgerrit | Chris Friesen proposed openstack/nova-specs master: Add support for emulated virtual TPM https://review.openstack.org/571111 | |
| #openstack-nova - 2018-10-10 | |||
| 00:09:29 | openstackgerrit | Merged openstack/nova master: api-ref: Replace non UUID string with UUID https://review.openstack.org/608854 | |
| 00:22:15 | openstackgerrit | Brin Zhang proposed openstack/nova master: Add compute version 36 to support ``volume_type`` https://review.openstack.org/579360 | |
| 00:34:16 | openstackgerrit | iain MacDonnell proposed openstack/nova master: Handle online_data_migrations exceptions https://review.openstack.org/608091 | |
| 01:17:29 | openstackgerrit | Sam Morrison proposed openstack/nova master: Allow ability for non admin users to list all flavors. https://review.openstack.org/608474 | |
| 02:30:06 | openstackgerrit | Jack Ding proposed openstack/nova master: Add I/O Semaphore to limit concurrent disk ops https://review.openstack.org/609180 | |
| 02:33:21 | openstackgerrit | huanhongda proposed openstack/nova master: api-ref: add two tables in the note of DELETE /os-services https://review.openstack.org/609186 | |
| 03:03:19 | openstackgerrit | Sam Morrison proposed openstack/nova master: Allow ability for non admin users to list all flavors. https://review.openstack.org/608474 | |
| 03:26:53 | openstackgerrit | Brin Zhang proposed openstack/nova master: Add compute API version for when a ``volume_type`` is requested https://review.openstack.org/605573 | |
| 04:04:45 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (7) https://review.openstack.org/571992 | |
| 04:05:05 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (8) https://review.openstack.org/571993 | |
| 04:05:31 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in virt/test_block_device.py https://review.openstack.org/566153 | |
| 04:27:35 | openstackgerrit | Sundar Nadathur proposed openstack/nova-specs master: Nova Cyborg interaction specification. https://review.openstack.org/603955 | |
| 05:55:47 | gmann | alex_xu hi, will you be there for API office hour ? | |
| 05:56:23 | alex_xu | gmann: yea | |
| 05:56:31 | gmann | cool, | |
| 06:01:53 | gmann | let's start | |