| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-10 | |||
| 12:39:38 | jaypipes | like SR-IOV VFs are some of the resource types we do *not* currently track using the placement service. our work on nested resource providers, granular request groups, and the "reshaper" effort are paving the way to supporting these types of resources in the placement service. I predict that we'll have most of that work done by the end of the "T" release series, at which point the quotas-in-placement work can become a reality. | |
| 12:39:38 | jaypipes | mago_: we've been planning on moving the queries for calculation of resource usage out of the nova-api service and towards the placement service for a while (https://review.openstack.org/#/c/509042/) but until we are tracking all the different types of resources using the placement service consistently, such an effort is only ever going to yield a small benefit. PCI devices, NUMA topology and dedicated CPUs, and certain networking resources | |
| 12:39:39 | efried | mdbooth: There's an open bug and proposed fix for this. | |
| 12:39:50 | mdbooth | efried: Yeah, we discussed it earlier. | |
| 12:40:11 | efried | mdbooth: Okay, upon rereading I figured that's not what you were getting at :) | |
| 12:40:27 | mdbooth | But even knowing the bug, I'm still surprised by the behaviour in practise. | |
| 12:40:56 | efried | mdbooth: You mean that there's some apparent predictability to the unpredictability of the hash ordering? | |
| 12:41:11 | mdbooth | I mean, I'd expect potentially different ordering if we were creating objects in a different order, or running things in a different order, or doing *anything* different. | |
| 12:41:38 | mdbooth | But in 2 consecutive absolutely identical test runs I'm surprised to see a difference in behaviour unless we're intentionally perturbing it somehow. | |
| 12:41:44 | gibi | mdbooth: try the same PYTHONHASHSEED | |
| 12:41:54 | mago_ | jaypipes, efried thank you very much for the explanations. | |
| 12:43:08 | gibi | mdbooth: I think if PYTHONHASHSEED=0 then you will get the same order every time | |
| 12:43:19 | gibi | mdbooth: but by default PYTHONHASHSEED=random | |
| 12:44:06 | mdbooth | gibi: PYTHONHASHSEED=1 ./.tox/lower-constraints/bin/python -m testtools.run nova.tests.unit.virt.libvirt.test_driver.LibvirtConnTestCase.test_pre_live_migration_volume_backed_encrypted | |
| 12:44:13 | mdbooth | Still inconsistent | |
| 12:44:22 | mdbooth | Meh, I was just curious | |
| 12:44:30 | mdbooth | This doesn't actually matter in the slightest :) | |
| 12:44:48 | gibi | :) | |
| 12:46:00 | jaypipes | mago_: np. sorry to not have great news! | |
| 12:53:30 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: [WIP/POC] Updating queued_for_delete from instance_destroy() https://review.openstack.org/566813 | |
| 12:53:35 | jangutter | jaypipes: for SR-IOV VF's there's a rough a proxy: Neutron Ports can under quota.... but it's very very very leaky at best. | |
| 12:54:22 | jangutter | jaypipes: very difficult to see how that model would translate to GPU, though. | |
| 12:57:38 | efried | cdent, jaypipes: +A on https://review.openstack.org/#/c/543262/ -- great stuff. | |
| 12:58:13 | efried | johnthetubaguy: FYI ^ | |
| 13:03:27 | cdent | efried: On self.compute thing, it's been long enoug since I made that change that I can't remember the details :( | |
| 13:03:49 | efried | no worries; gate-bound in any case. If it passes zuul, it's good enough for me. | |
| 13:04:33 | efried | cdent: But as I'm sure you're aware, mock.patch.object(actual object, 'method') is not quite the same thing as mock.patch.object(path to lib containing method, 'method') | |
| 13:04:47 | efried | cdent: But in this case I think both things were actually pointing to the lib ultimately. | |
| 13:04:53 | efried | so in this case I think it *was* the same. | |
| 13:05:08 | cdent | yes, that's why it ended up as it did, because the indirection of self.compute was meaningless | |
| 13:12:19 | openstackgerrit | Merged openstack/os-vif master: add noop plugin https://review.openstack.org/567924 | |
| 13:13:27 | tssurya | mriedem, dansmith: do you have some time now or shall I ping you back later ? | |
| 13:14:41 | mriedem | i'm here | |
| 13:15:50 | tssurya | mriedem: so is it documented somewhere that when a user does nova list --deleted, it silently ignores this | |
| 13:15:57 | tssurya | without complaining about policy ? | |
| 13:16:10 | mriedem | for non-admins? | |
| 13:16:21 | tssurya | yes | |
| 13:16:43 | mriedem | i'm not aware of something saying it's not an error for non-admins | |
| 13:16:54 | mriedem | https://docs.openstack.org/python-novaclient/latest/cli/nova.html#nova-list says (admin only) | |
| 13:17:02 | tssurya | hmm okay then I will open a bug | |
| 13:17:11 | mriedem | bug for what? | |
| 13:17:28 | tssurya | because currently if you do nova list --deleted as a non-admin | |
| 13:17:32 | tssurya | it just ignores the filter | |
| 13:17:39 | mriedem | it's been that way forever | |
| 13:17:39 | tssurya | and gives you all the active instances | |
| 13:18:03 | mriedem | changing that would likely be a non-trivial backward incompatible behavior change in the api | |
| 13:18:26 | tssurya | oh so why do the rest of the admin only filters give policy errors | |
| 13:18:32 | mriedem | see remove_invalid_options() | |
| 13:18:59 | mriedem | which filters for example? | |
| 13:19:09 | mriedem | also, you should probably include alex_xu and/or gmann in this discussion | |
| 13:19:16 | tssurya | --user or --all-tenants etcc | |
| 13:19:29 | tssurya | I mean all those filters which say (admin only) | |
| 13:19:38 | tssurya | shouldn't they all behave in the same way ? | |
| 13:19:59 | mriedem | i see | |
| 13:20:00 | mriedem | raise exc.HTTPForbidden(explanation=msg) | |
| 13:20:00 | mriedem | msg = _("Only administrators may list deleted instances") | |
| 13:20:24 | mriedem | but looks like that's only if you're filtering on vm_state='deleted' | |
| 13:20:39 | tssurya | yea | |
| 13:20:48 | openstackgerrit | Merged openstack/nova master: Remove duplicate parameter in API sample documents https://review.openstack.org/581203 | |
| 13:20:49 | tssurya | doing --status DELETED is caught correctly | |
| 13:21:00 | tssurya | thanks alex_xu | |
| 13:21:38 | mriedem | tssurya: it's also possible today to just pass random query params w/o an error | |
| 13:21:42 | mriedem | GET /servers?foo=bar | |
| 13:21:54 | mriedem | for backward compatibility reasons | |
| 13:22:03 | tssurya | mriedem: ah okay | |
| 13:22:15 | mriedem | if we made the query parameter whitelist a true whitelist and failure for unknown parameters, that would be a microversion | |
| 13:23:17 | tssurya | mriedem: hmm, yea I don't know if it needs to change, I noticed this when I was playing around with policy/filters | |
| 13:23:22 | alex_xu | the API behaviour is ignoring any invalid parameters | |
| 13:23:37 | mriedem | https://github.com/openstack/nova/blob/b67a9a111ab0cf68084f62017a74f9a41377244f/nova/api/openstack/compute/schemas/servers.py#L599 | |
| 13:24:26 | openstackgerrit | Matthew Booth proposed openstack/nova master: Avoid redundant initialize_connection on source post live migration https://review.openstack.org/551302 | |
| 13:24:27 | openstackgerrit | Matthew Booth proposed openstack/nova master: Restore connection_info after live migration rollback https://review.openstack.org/551349 | |
| 13:26:08 | openstackgerrit | Silvan Kaiser proposed openstack/nova master: Exec systemd-run with privileges in Quobyte driver https://review.openstack.org/554195 | |
| 13:27:35 | mriedem | also, for example, 2.26 adds the *tags* query params but they are only effective if using >= 2.26, if you pass tags=1,2 with 2.1, they are just ignored | |
| 13:27:39 | tssurya | alex_xu, mriedem: thanks for the pointer to the code/confirmation! it was just strange that nova list --deleted and nova list --status DELETED behaved differently. | |
| 13:27:41 | mriedem | because 'additionalProperties': True, | |
| 13:28:31 | tssurya | right | |
| 13:28:52 | mriedem | it also looks like if you pass deleted=cookies it will accept that and default deleted=False | |
| 13:29:10 | tssurya | heh yea | |
| 13:29:17 | mriedem | https://github.com/openstack/nova/blob/b67a9a111ab0cf68084f62017a74f9a41377244f/nova/api/openstack/compute/servers.py#L187 | |
| 13:30:15 | mriedem | why we treat --status DELETED and --deleted differently, idk | |
| 13:30:24 | mriedem | looks like that was pre-microversions | |
| 13:30:36 | tssurya | yea , well is this documented somewhere outside the code ? | |
| 13:31:05 | tssurya | (I couldn't find it) | |
| 13:31:10 | mriedem | tssurya: i'm not aware of something about this being documented off the top of my head - i'd check the compute API guide since that's where i'd expect something like this to exist, if at all | |
| 13:31:24 | alex_xu | tssurya: mriedem in the before, we have a proposal to change the all_tenants policy https://review.openstack.org/#/c/527019/3/specs/ocata/implemented/add-whitelist-for-server-list-filter-sort-parameters.rst@68 | |
| 13:31:30 | alex_xu | but we didn't implement it | |
| 13:31:46 | openstackgerrit | Silvan Kaiser proposed openstack/nova master: Exec systemd-run with privileges in Quobyte driver https://review.openstack.org/554195 | |
| 13:32:13 | tssurya | mriedem, alex_xu: ack | |
| 13:37:28 | mriedem | https://review.openstack.org/#/c/11469/ added the ability to do nova list --status DELETED | |
| 13:37:32 | mriedem | and fails for non-admins | |
| 13:38:12 | mriedem | which is so long ago there would have been no consideration of consistency with failing a non-admin passing deleted=True | |
| 13:39:16 | mriedem | it feels like changing that now to fail for non-admins would require a microversion but i could see the argument both ways | |
| 13:39:50 | mriedem | there is something to be said for explicit api behavior, i.e. if i'm a non-admin doing nova list --deleted and not getting back deleted instances, i might not realize i'm using the wrong rc file and think it's a bug | |
| 13:40:12 | mriedem | i'm sure we have crap like this all over the api | |
| 13:40:23 | tssurya_ | Haha | |
| 13:40:37 | mriedem | there are lots of "make this strict in a new microversion" notes in lots of that code | |
| 13:40:50 | mriedem | for example, lots of the schema validation that has additionalProperties: true | |
| 13:40:59 | mriedem | and fixing error response codes | |
| 13:41:26 | mriedem | if someone had the energy and care to wrangle a lot of those things into a spec we could fix them in a single microversion | |
| 13:42:16 | mriedem | validation just gets hard when you have 80 *known* query params and some of those have behavior characteristics depending on microversion and/or what other parameters are passed | |