| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-19 | |||
| 14:15:20 | leakypipes | fried_rice: of course. | |
| 14:15:34 | openstackgerrit | David Rabel proposed openstack/nova master: Fix format in flavors.rst https://review.openstack.org/535777 | |
| 14:15:55 | fried_rice | leakypipes But the documentation says the RP generation is ignored in alloc requests. | |
| 14:16:45 | fried_rice | leakypipes ...for PUT. And it appears to be entirely absent for POST. | |
| 14:17:18 | fried_rice | leakypipes Bug? | |
| 14:17:32 | leakypipes | fried_rice: https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L2085 | |
| 14:17:34 | artom | I can't even begin to think about the fix | |
| 14:17:47 | artom | We're setting a whole bunch of instance attributes in the compute API | |
| 14:17:54 | artom | And *then* doing the rebuild | |
| 14:17:55 | leakypipes | fried_rice: there is no POST. | |
| 14:18:02 | artom | With no checks whether it passed the scheduler or not | |
| 14:18:13 | fried_rice | leakypipes https://developer.openstack.org/api-ref/placement/#manage-allocations | |
| 14:18:16 | leakypipes | fried_rice: there is only PUT /allocations/{consumer_uuid} | |
| 14:18:34 | artom | Maybe the easiest would be to just revert the CVE "fix" and leave it as WONTFIX :/ | |
| 14:18:37 | TheJulia | fried_rice: my guess is some piece of compiled c is getting loaded in from the upgraded underlying modules, and the conductor goes sideways at that point | |
| 14:19:04 | fried_rice | TheJulia Fo sho. I was just being glib, though - I certainly haven't seen the behavior you describe. | |
| 14:19:07 | leakypipes | fried_rice: oh, that is for when we move instances. | |
| 14:19:30 | leakypipes | fried_rice: it is not used anywhere else. | |
| 14:19:44 | fried_rice | By nova. At the moment. But not the point. | |
| 14:20:23 | leakypipes | fried_rice: what is your point? | |
| 14:20:37 | fried_rice | leakypipes Getting there, finding code, one sec. | |
| 14:21:01 | fried_rice | leakypipes Yeah, so link above convinces me we're incrementing the generation when we do allocationey stuff, which is dandy. | |
| 14:21:18 | fried_rice | leakypipes But if the allocation APIs don't accept a generation, how are we detecting conflicts? | |
| 14:21:28 | fried_rice | leakypipes Or reporting them (409s)? | |
| 14:21:34 | rabel | can anyone please help me with quotas? is the default quota class used for anything? a GET call to /os-quota-sets/{tenant_id}/defaults does not seem to return it, but i'm not sure what it returns instead. | |
| 14:21:39 | leakypipes | fried_rice: we detect conflicts in terms of whether capacity is exceeded, that's all. | |
| 14:22:14 | leakypipes | fried_rice: i.e. does the original constraint that there was existing capacity for the allocated resources on these set of resource providers still hold. | |
| 14:23:07 | fried_rice | leakypipes That's cool. But if two threads are trying to e.g. modify allocations for the same consumer_uuid at the same time, the second one will "win". | |
| 14:23:54 | fried_rice | leakypipes Not a problem in today's world where everything is funneled through report client, and there's one compute node RP, and the consumer is an instance. | |
| 14:24:25 | leakypipes | fried_rice: no, that's not possible. one of those threads would get a 409 because the provider generation increment would fail. | |
| 14:24:43 | fried_rice | leakypipes How and why would the increment fail if we're not passing the generation into the API? | |
| 14:26:12 | leakypipes | fried_rice: because the provider generations are *read* at the start of the transaction that inserts/deletes allocation records. after those allocation records are changed, the provider generations are incremented for all providers involved in the transaction. if another thread had inserted/deleted allocation records (for that consumer or another consumer) against those same resource providers, the increment would fail. | |
| 14:26:57 | leakypipes | fried_rice: specifically, this would happen: https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L281-L282 | |
| 14:27:30 | fried_rice | leakypipes I'm not talking about multiple threads in the placement code at the same time. I'm talking about multiple consumer threads making single individual atomic calls to completion. | |
| 14:29:10 | leakypipes | fried_rice: I'm going to go back to reviewing the patch series I need to get reviewed today :) you are welcome to walk through the code paths in there and identify a particular concurrency bug. | |
| 14:30:03 | leakypipes | fried_rice: you are also welcome to put a discussion on all the concurrency topics up at the PTG. I think that would be a good discussion. it's just I need to focus today on these reviews. | |
| 14:30:39 | fried_rice | isk-less) payload and PUTs back. Placement happily replaces with B's version of the allocation, which has the vif but doesn't have the disk. | |
| 14:30:39 | fried_rice | leakypipes Here's the flow: Existing instance. Shared disk manager A wants to add some disk resource. sean-k-mooney's neutron thingy B wants to add a vif or whatever. A does a GET of the current allocations. B does a GET of the current allocations. A adds his disk and PUTs back. Placement happily replaces the allocations, which now include the disk, and increments the generation. B adds his vif to the original (d | |
| 14:30:59 | leakypipes | alex_xu: are you still around? that patch you wanted a review on is dependent on this one which needs a rebase: https://review.openstack.org/#/c/479776/. are you planning on getting to that? | |
| 14:32:18 | sean-k-mooney | fried_rice: why would you not request both the disk and vif in the same allocation request? | |
| 14:32:45 | fried_rice | sean-k-mooney Because different entities are "owning" those resources. | |
| 14:32:53 | leakypipes | alex_xu: never mind. it's not a dependent patch, it's just in the same topic. damn gerrit... :( | |
| 14:33:32 | sean-k-mooney | fried_rice: differnet entityes are creating the resouce providers but the nova sechduler/conductor will be claiming both on behalf of the instance | |
| 14:33:48 | fried_rice | sean-k-mooney Sure, you *could* have it set up that way. Or, you know, not. | |
| 14:33:58 | fried_rice | sean-k-mooney At the moment, this is not a danger. | |
| 14:34:05 | fried_rice | sean-k-mooney In the future.... | |
| 14:34:28 | fried_rice | Certainly for the general case usage of the placement API. | |
| 14:34:36 | sean-k-mooney | we could technically make the neutron port the owner of the vif which would then result in two owners | |
| 14:35:09 | sean-k-mooney | *vif allocation | |
| 14:35:22 | fried_rice | The solution would be fairly simple I think: just add the generation for each RP to PUT /allocations/{consumer_uuid} and POST /allocations | |
| 14:37:43 | openstackgerrit | Merged openstack/nova master: libvirt: create vGPU for instance https://review.openstack.org/528832 | |
| 14:38:05 | fried_rice | Basically, it works as long as all the allocations for a given consumer are always only managed by one thread at a time. Which is a constraint that happens to be true today, but certainly isn't documented as a limitation of using the API. | |
| 14:38:49 | sean-k-mooney | leakypipes: alex_xu https://review.openstack.org/#/c/535642 seams to add suport for required to get /allocation_candiates but i dont see support for the Resouce1..,Required1 syntax needed for different traits per resouce. is there a followup patch for that | |
| 14:39:16 | leakypipes | sean-k-mooney: yes, we're not targeting that for queens. fried_rice has a patch up for that. | |
| 14:40:17 | fried_rice | leakypipes Quit saying that. We *are* targeting it for Queens, until something changes. We have an approved blueprint and 95% of the code merged. | |
| 14:41:05 | leakypipes | fried_rice: I won't hold up non-granular requests with traits for granular requests. | |
| 14:41:15 | leakypipes | fried_rice: does that sound more accurate? | |
| 14:41:23 | sean-k-mooney | leakypipes: ya i saw fried_rice's patch i assumed it superceeded alex's instead of building on it.thats problematic but ok. ill cherry pick fried_rice patch if i need it. | |
| 14:41:24 | fried_rice | leakypipes Agree with that (see -dev ml) | |
| 14:42:20 | fried_rice | sean-k-mooney ditto (http://lists.openstack.org/pipermail/openstack-dev/2018-January/126393.html) | |
| 14:43:11 | sean-k-mooney | so how does required work wiht nrp | |
| 14:43:28 | fried_rice | sean-k-mooney The reason my patch doesn't build on alex_xu's is because mine is 2.5 months older :) | |
| 14:44:31 | fried_rice | sean-k-mooney You get candidates such that a) all resources come from providers in the same tree, and b) the union of all the traits on providers you actually get resources from is a superset of the traits you asked for. | |
| 14:45:11 | sean-k-mooney | fried_rice: ok the union is the important bit | |
| 14:45:13 | fried_rice | sean-k-mooney Which is obviously limited e.g. you can't say I want one VIF with trait CUSTOM_PHYSNET_A and one with CUSTOM_PHYSNET_B -- for that you would need granular, period. | |
| 14:46:17 | sean-k-mooney | yes but i can say one vif and one cpu with CUSTOM_PHYSNET_A and cpu_trait_x | |
| 14:46:43 | fried_rice | sean-k-mooney Yes. And you'd better hope that your CPU doesn't have trait CUSTOM_PHYSNET_A | |
| 14:46:54 | fried_rice | (silly example in this case) | |
| 14:47:12 | sean-k-mooney | why what would happen if it did? | |
| 14:47:19 | fried_rice | You could still get a vif from physnet B and placement will think it did its job. | |
| 14:47:28 | sean-k-mooney | ah | |
| 14:47:30 | sean-k-mooney | ok | |
| 14:48:29 | sean-k-mooney | well without traits at all we would get to the compute node try to bind the port and get a vif_binding_failed from neutron then hit the retry loop until we hit max retries or got lucky | |
| 14:48:43 | fried_rice | heh, yeah | |
| 14:49:01 | fried_rice | Oh, I'm not disputing that having traits support is better than not having traits support. | |
| 14:49:31 | fried_rice | Just that single-request-group has limitations no amount of trait-finagling will overcome. | |
| 14:49:49 | sean-k-mooney | fried_rice: ya i know but that example is precisly the one im trying to avoid in my poc as a first pass use of traits | |
| 14:50:04 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: WIP: hacky negotiation of ironic microversion https://review.openstack.org/535786 | |
| 14:50:04 | fried_rice | sean-k-mooney Well, that's why we need granular :) | |
| 14:50:40 | sean-k-mooney | the minimal support will work for the simple case e.g. all interfaces on same physnet which is enough for demos | |
| 14:50:46 | fried_rice | yuh | |
| 14:50:59 | fried_rice | on the same physnet and same everything else that matters. | |
| 14:51:36 | sean-k-mooney | well we can have multiple physnets in the cloud and show that placement found the correct node | |
| 14:51:48 | fried_rice | Yup. | |
| 14:51:59 | sean-k-mooney | we would have had to use availablity zones with out tratis to do the same thing | |
| 14:52:26 | sean-k-mooney | or atleast some host_aggregate based filter | |
| 14:52:32 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: QEMU native LUKS decryption for encrypted volumes https://review.openstack.org/523958 | |
| 14:52:45 | fried_rice | And you can *probably* even ask for multiple vifs on separate subnets in the same request. As long as you only want one of each. And don't share those traits with any other provider. Which is probably doable. | |
| 14:53:16 | fried_rice | brb | |
| 14:54:01 | sean-k-mooney | ill cross that bridge when i come to it but ya granular resouce requests in queens will support all of these usecase more cleanly/correctly | |
| 14:54:08 | sean-k-mooney | * in rocky | |
| 15:01:48 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: WIP: hacky negotiation of ironic microversion https://review.openstack.org/535786 | |
| 15:02:44 | leakypipes | alex_xu: review done. | |
| 15:14:59 | rabel | hm. why is a PUT to /os-quota-sets/default working? this does not make sense in my brain | |
| 15:25:52 | leakypipes | fried_rice: just confirming... mgoddard's patches for "Call get_traits() in the resource tracker" and "Add get_traits() method to ComputeDriver" are obseleted with the update_provider_tree() work, correct? | |
| 15:26:53 | fried_rice | leakypipes Eventually, but when I asked him about it, apparently he's wanting to move forward more quickly than that. | |
| 15:27:16 | fried_rice | leakypipes Right now he's working toward what's described in his blueprint. | |
| 15:27:33 | fried_rice | leakypipes And that stuff will coexist with update_provider_tree until it doesn't. | |
| 15:28:11 | leakypipes | hmm, ok | |
| 15:28:17 | fried_rice | leakypipes Though xen is counting on using update_provider_tree, and I just found out vmware is as well. | |