| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-30 | |||
| 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 | mgagne | I opened the bug, I'm currently working on it https://bugs.launchpad.net/nova/+bug/1768107 | |
| 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: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 | melwitt | no worries | |
| 21:06:16 | mriedem | fwiw i think it's mostly easy stuff | |
| 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! | |
| 21:54:07 | arvindn05 | mriedem: glad you said that...i did exactly that i think....alex didnt want adding the image with required traits to the registry | |
| 21:54:10 | arvindn05 | https://review.openstack.org/#/c/560596/ | |
| 21:54:21 | arvindn05 | :( | |
| 21:54:47 | arvindn05 | it kindof makes sense....we dont want the registry to grow big with each of these changes | |
| 21:55:26 | arvindn05 | so wanted to do the same as how you guys handled flavor in the same tests....basically update a fake image with required traits | |
| 21:55:48 | mriedem | we don't need a fake service for flavors | |
| 21:56:13 | mriedem | _FakeImageService implements update() so why can't you just rely on that? | |
| 21:56:41 | mriedem | you should be able to just use this https://developer.openstack.org/api-ref/compute/#update-image-metadata | |
| 21:56:49 | mriedem | however, you have to make sure to use microversion < 2.39 | |
| 21:57:11 | mriedem | which is pretty easy though | |
| 21:57:26 | mriedem | with nova.utils.temporary_mutation(self.api.microversion, '2.1'): | |
| 21:57:31 | mriedem | ... update image metadata | |
| 21:58:13 | arvindn05 | you mean >2.39 right? | |
| 21:58:35 | mriedem | no | |
| 21:58:46 | mriedem | see the warning https://developer.openstack.org/api-ref/compute/#images-deprecated | |
| 21:59:08 | mriedem | also, i don't see where alex says to not create a new fake image in https://review.openstack.org/#/c/560596/ | |
| 21:59:11 | arvindn05 | oh...how d | |
| 21:59:25 | arvindn05 | he said not to update the registry | |
| 21:59:32 | mriedem | where? | |
| 21:59:34 | mriedem | link please | |
| 21:59:52 | arvindn05 | looking at this one of the alternatives | |
| 22:00:04 | arvindn05 | The API sample tests are for verify the API change, not sure we should use API sample test for other function/feature test. | |
| 22:01:12 | arvindn05 | on PS9 | |