| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-30 | |||
| 17:53:12 | mgagne | kilo -> mitaka. I lost the user_id when moving from filter_properties to spec_obj | |
| 17:53:19 | mriedem | ok mitaka is definitely not "latest" nova | |
| 17:53:27 | mgagne | =) | |
| 17:53:37 | mriedem | why can't you use reqspec.instance.user_id? | |
| 17:53:50 | mriedem | oh nvm | |
| 17:53:51 | mgagne | spec_obj.project_id is available, not spec_obj.user_id | |
| 17:54:37 | mriedem | RequestSpec.from_primitives uses filter_properties to build the RequestSpec from the instance, and doesn't save off the user_id | |
| 17:54:41 | mgagne | I didn't look much into it, I saw that spec_obj.project_id was available and used elsewhere and tried spec_obj.user_id | |
| 17:56:09 | mriedem | so https://review.openstack.org/#/c/197912/ was the regression for you in mitaka | |
| 17:57:22 | mgagne | yes | |
| 17:57:49 | mgagne | I'm just gonna contribute my patch and see where it goes from there | |
| 17:59:06 | mriedem | wfm, should probably report a bug as well | |
| 17:59:22 | mgagne | bug is enough? will open then | |
| 17:59:35 | mriedem | regardless of the custom out of tree scheduler filter dependency, it makes sense to have user_id to know the user that originally requested to create the instance vs another user (in the same project) to move an instance | |
| 17:59:53 | mriedem | especially if we already have project_id in RequestSpec | |
| 18:00:15 | melwitt | fwiw, I agree. we have the same mismatch (project_id but no user_id) in InstanceMapping too | |
| 18:00:20 | mriedem | maybe others feel differently, but this seems ok to me as a regression fix, albeit for out of tree things | |
| 18:00:55 | mriedem | instance mappings are pretty special purpose though, | |
| 18:01:04 | mriedem | and not tied to any pluggable interfaces as far as i know | |
| 18:03:32 | melwitt | no, but it was weird to see part of the owner info there and not the other when I looked at it for counting instances. I don't know what it uses project_id for currently | |
| 18:03:46 | mriedem | allocation claims in placement at least | |
| 18:04:07 | mriedem | AggregateMultiTenancyIsolation also uses it | |
| 18:04:09 | mgagne | https://github.com/openstack/nova/blob/master/nova/scheduler/filters/aggregate_multitenancy_isolation.py#L41 | |
| 18:04:10 | mgagne | yes | |
| 18:04:30 | mgagne | that's where I found spec_obj.project_id was available | |
| 18:04:37 | melwitt | I mean I don't know what the InstanceMapping.project_id is used for currently | |
| 18:05:01 | melwitt | i.e. why it needed project_id but not user_id | |
| 18:05:31 | dansmith | melwitt: so you can list instances :) | |
| 18:08:32 | dansmith | melwitt: for this: https://review.openstack.org/#/c/509003/8 | |
| 18:08:34 | melwitt | oh okay, so it doesn't just do a scatter-gather query instances by project_id and instead does separate queries per InstanceMapping? I didn't realize that | |
| 18:09:01 | dansmith | well, it was to avoid hitting cells you don't need during list.. I don't remember if we ever implemented that before my list thing, | |
| 18:09:10 | dansmith | but we never merged that bit of my newer thing either | |
| 18:09:25 | dansmith | because of the BR thing I never finished | |
| 18:09:56 | melwitt | oh, I see | |
| 18:10:21 | melwitt | so it could pre-check if there are any cells that do not contain that project_id | |
| 18:11:42 | dansmith | it just got a unique list of cells that you have mappings for | |
| 18:11:57 | dansmith | https://review.openstack.org/#/c/509002/8/nova/objects/cell_mapping.py | |
| 18:12:21 | dansmith | we never implemented that before (I just checked) with the old stuff because we were just using the global cell mapping list | |
| 18:12:37 | dansmith | it's a perf tradeoff | |
| 18:12:52 | dansmith | for two cells and even distribution, it's faster to just use the global list, | |
| 18:13:05 | dansmith | if you're cern, it makes sense to figure out which cells we should even ask during list | |
| 18:13:40 | dansmith | might should have a perf tunable in that last patch | |
| 18:14:44 | openstack | Launchpad bug 1768107 in OpenStack Compute (nova) "The user_id field is missing from RequestSpec" [Undecided,In progress] - Assigned to Mathieu Gagné (mgagne) | |
| 18:14:44 | mgagne | I opened the bug, I'm currently working on it https://bugs.launchpad.net/nova/+bug/1768107 | |
| 18:15:21 | melwitt | yeah, understood | |
| 18:37:52 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Implement granular policy rules for placement https://review.openstack.org/524425 | |
| 18:37:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Cleanup placement policy generator docs https://review.openstack.org/565225 | |
| 18:55:42 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: Object changes for granular https://review.openstack.org/564351 | |
| 18:55:43 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: placement: Granular GET /allocation_candidates https://review.openstack.org/517757 | |
| 18:55:44 | openstackgerrit | Eric Fried proposed openstack/nova master: Get anchors for sharing providers https://review.openstack.org/565279 | |
| 19:17:27 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Return resources of entire trees in Placement https://review.openstack.org/559466 | |
| 19:39:43 | openstackgerrit | Brianna Poulos proposed openstack/nova stable/pike: conf: Do not inherit image signature props with snapshots https://review.openstack.org/565288 | |
| 19:42:22 | openstackgerrit | Brianna Poulos proposed openstack/nova stable/pike: conf: Do not inherit image signature props with snapshots https://review.openstack.org/565288 | |
| 19:52:39 | openstackgerrit | Brianna Poulos proposed openstack/nova master: Add notification support for trusted_certs https://review.openstack.org/563269 | |
| 19:58:04 | openstackgerrit | Eric Fried proposed openstack/nova master: Get anchors for sharing providers https://review.openstack.org/565279 | |
| 19:58:05 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: Object changes for granular https://review.openstack.org/564351 | |
| 19:58:06 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: placement: Granular GET /allocation_candidates https://review.openstack.org/517757 | |
| 20:59:19 | mriedem | dansmith: i think https://review.openstack.org/#/c/325414/ (bottom patch in the console auth token series) is ready to go, comments up the stack on the other changes though | |
| 20:59:46 | mriedem | dansmith: this is also likely a question for you https://review.openstack.org/#/c/333990/32/nova/console/websocketproxy.py@43 | |
| 21:03:49 | dansmith | mriedem: answered that question.. not sure I'll get to much more before EOD | |
| 21:05:49 | mriedem | melwitt: fyi, comments in your console auth token series | |
| 21:05:56 | mriedem | i see the runway slot ends tomorrow | |
| 21:05:59 | mriedem | sorry for being late on those | |
| 21:06:12 | melwitt | mriedem: replies in progress | |
| 21:06:16 | mriedem | fwiw i think it's mostly easy stuff | |
| 21:06:16 | melwitt | no worries | |
| 21:06:17 | openstackgerrit | Mathieu Gagné proposed openstack/nova master: Add user_id to RequestSpec https://review.openstack.org/565340 | |
| 21:06:50 | melwitt | thanks for the reviews | |
| 21:29:09 | melwitt | mriedem: do you happen to know what's the process for moving methods into os-vif plugins for plug/unplug? does there have to be a deprecation period or requirement for how the plugin has to be able to be installed? this is a change for moving bigswitch methods to a plugin: https://review.openstack.org/534371 | |
| 21:30:24 | mriedem | last time i looked at that patch i asked some questions https://review.openstack.org/#/c/534371/4//COMMIT_MSG@12 | |
| 21:30:30 | openstackgerrit | Mathieu Gagné proposed openstack/nova master: Add user_id to RequestSpec https://review.openstack.org/565340 | |
| 21:31:10 | melwitt | oh, I'm blind | |
| 21:32:12 | mriedem | as for what the process is, i don't think we really have one, but if we have some vif type impl in tree and then it moves to an out of tree neutron plugin, operators would obviously need to make sure the neutron agent is using that new package to provide that vif type support before they upgrade nova-compute | |
| 21:32:28 | mgagne | mriedem: just to make sure, I'm looking at above test for user_id and they don't cover use case you are suggesting. so I'm a bit confused about why in that case, I should test for it. | |
| 21:32:58 | mriedem | mgagne: see my latest reply? | |
| 21:33:09 | mgagne | mriedem: yes, hence my question | |
| 21:33:32 | mriedem | mgagne: because we have unit tests with false positives i think for some object compat tests | |
| 21:33:54 | mriedem | so asserting that something is still in the primitive after the compat conversion is just good practice to make sure the test is not asserting a false positive | |
| 21:34:11 | mgagne | ok so it's a new use case we should add that doesn't exist currently | |
| 21:34:56 | mriedem | mgagne: here is one example https://github.com/openstack/nova/blob/master/nova/tests/unit/objects/test_migrate_data.py#L74 | |
| 21:34:57 | mgagne | because there is no way I could have guess that this kind of test was a good idea by looking at previous tests above. | |
| 21:35:22 | melwitt | mriedem: okay, makes sense | |
| 21:35:23 | mgagne | yes, I know the principle just wondering why now it's important and not before | |
| 21:35:35 | mriedem | i think this test might be broken https://github.com/openstack/nova/blob/master/nova/tests/unit/objects/test_migrate_data.py#L226 | |
| 21:35:59 | mriedem | mgagne: because reviewers might not have thought about it before when reviewing these types of tests, | |
| 21:36:12 | mriedem | and dansmith busted me on making the same mistake once in something i was adding that did the same thing, so now i'm conscious of it | |
| 21:36:26 | mgagne | mriedem: ok, will add those tests. In that case, I suggest we retrofit the other tests too | |
| 21:36:44 | mriedem | mgagne: i don't think that needs to happen in your change - retrofitting the existing tests i mean | |
| 21:36:51 | mgagne | ok | |
| 21:38:37 | openstackgerrit | Mathieu Gagné proposed openstack/nova master: Add user_id to RequestSpec https://review.openstack.org/565340 | |
| 21:43:11 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Make `cpu_model_extra_flags` case-insensitive for real https://review.openstack.org/565043 | |
| 21:47:40 | arvindn05 | i need to add a patch method to the client under tests.functional.api.client.TestOpenStackClient | |
| 21:48:34 | arvindn05 | methods like tests.functional.api.client.TestOpenStackClient#post_image invoke tests.unit.image.fake._FakeImageService | |
| 21:48:53 | arvindn05 | how is this mapping made? | |
| 21:51:44 | arvindn05 | in other words, how can i ensure a specific method is called when PATCH is called on the FakeImageService? | |
| 21:52:55 | mriedem | why do you need to add a patch method? | |
| 21:53:04 | mriedem | to update an existing fake image with required traits? | |
| 21:53:08 | arvindn05 | yup | |
| 21:53:24 | mriedem | just add a new fake image in the 'registry' with the required traits, hard-code the uuid and refer to that image id in your tests | |
| 21:53:26 | mriedem | done! | |