| openstackgerrit | Tetsuro Nakamura proposed openstack/nova-specs master: Spec: allocation candidates in tree https://review.openstack.org/603585 | 01:07 |
|---|---|---|
| *** tetsuro has joined #openstack-placement | 01:07 | |
| *** mriedem_away has quit IRC | 01:08 | |
| openstackgerrit | Sundar Nadathur proposed openstack/nova-specs master: Nova Cyborg interaction specification. https://review.openstack.org/603955 | 02:33 |
| *** tetsuro has quit IRC | 04:06 | |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova-specs master: Spec for API inconsistency cleanup https://review.openstack.org/603969 | 04:16 |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova-specs master: Spec for API inconsistency cleanup https://review.openstack.org/603969 | 04:28 |
| *** tetsuro has joined #openstack-placement | 06:19 | |
| *** belmoreira has joined #openstack-placement | 06:39 | |
| *** e0ne has joined #openstack-placement | 07:16 | |
| *** helenafm has joined #openstack-placement | 07:51 | |
| *** tetsuro has quit IRC | 09:28 | |
| *** helenafm has quit IRC | 09:30 | |
| *** belmoreira has quit IRC | 09:38 | |
| *** helenafm has joined #openstack-placement | 10:05 | |
| *** takashin has quit IRC | 10:13 | |
| openstackgerrit | Yikun Jiang proposed openstack/placement master: WIP: Add nova database migration script(psql version) https://review.openstack.org/604028 | 10:17 |
| openstackgerrit | Yikun Jiang proposed openstack/placement master: WIP: Add nova database migration script(psql version) https://review.openstack.org/604031 | 10:22 |
| openstackgerrit | Yikun Jiang proposed openstack/placement master: WIP: Add nova database migration script(psql version) https://review.openstack.org/604028 | 10:25 |
| *** takashin has joined #openstack-placement | 10:44 | |
| openstackgerrit | John Garbutt proposed openstack/nova-specs master: WIP: Spec for API policy updates https://review.openstack.org/547850 | 10:47 |
| *** belmoreira has joined #openstack-placement | 10:57 | |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova-specs master: Spec for API inconsistency cleanup https://review.openstack.org/603969 | 10:58 |
| *** tetsuro has joined #openstack-placement | 10:59 | |
| *** s10 has joined #openstack-placement | 11:06 | |
| *** tssurya has joined #openstack-placement | 11:39 | |
| *** mriedem has joined #openstack-placement | 13:06 | |
| *** cdent has joined #openstack-placement | 13:15 | |
| *** e0ne has quit IRC | 13:48 | |
| cdent | mriedem: I was going to look into during the grenade changes for nova -> placement upgrade but a) i'm way sick and stupid so not up to the immediate challenge of teasing that out, b) on pto first three days of next week so... ? | 13:51 |
| mriedem | that's fine, | 13:55 |
| mriedem | i can do it (or someone else if they beat me) but i wanted to test out dan's db migration script in a devstack first, | 13:55 |
| mriedem | which i haven't had time to do yet this week | 13:56 |
| cdent | I ran dan's db migration on a plain devstack yesterday and it was happy. was then thinking I'd spin up a placement-devstack with https://review.openstack.org/#/c/600162/ but didn't get that far and wasn't sure what the point was: the mova data from here ... to here part is tight. as long as things are identified properly | 13:58 |
| *** jaypipes is now known as jaypipes-ooo | 14:03 | |
| belmoreira | I'm running into a weird situation with placement... can't schedule a VM because placement doesn't retrieve any allocation candidate | 14:11 |
| belmoreira | Doing some debug with the os_placement client and getting: | 14:12 |
| belmoreira | openstack allocation candidate list --resource MEMORY_MB=60000 | 14:12 |
| belmoreira | | MEMORY_MB=60000 | 90799f16-9a4a-4001-868b-335dbce6b5df | MEMORY_MB=60000/122966 | | 14:12 |
| belmoreira | openstack allocation candidate list --resource MEMORY_MB=60000 --resource VCPU=1 | 14:12 |
| belmoreira | | VCPU=1,MEMORY_MB=60000 | 90799f16-9a4a-4001-868b-335dbce6b5df | VCPU=32/1584,MEMORY_MB=60000/122966 | | 14:12 |
| belmoreira | openstack allocation candidate list --resource MEMORY_MB=60000 --resource VCPU=32 | 14:12 |
| belmoreira | The node is not in the list! | 14:12 |
| belmoreira | Is there any know issue in placement that can justify this? | 14:13 |
| cdent | belmoreira: are min_unit, max_unit or step_size set to anything weird on the inventories on 90799f16-9a4a-4001-868b-335dbce6b5df ? | 14:14 |
| edleafe | belmoreira: Are there 32 VCPU available (total-reserved-allocated)? | 14:14 |
| belmoreira | cdent: that's it! | 14:16 |
| cdent | \o/ | 14:16 |
| belmoreira | max_unit is set to 16 that is the number of available cores | 14:16 |
| cdent | yeah, that was a decision that was made at some point that was a bit controversial but made sense at the time | 14:16 |
| belmoreira | I guess that it can't be changed | 14:17 |
| cdent | on vcpu: max_unit gets set to the physical max because it seemed wrong to cause immediate cpu contention with just one workload | 14:17 |
| cdent | not without changing code | 14:17 |
| cdent | it would get reset | 14:18 |
| * cdent looks at the code | 14:18 | |
| belmoreira | the issue is that with the L1TF mitigations SMT was disabled. meaning that the inventory was updated to half of the cores which causes issues with already defined flavors | 14:19 |
| cdent | ah, yeah. | 14:19 |
| belmoreira | I think we should discuss to allow "max_unit" configurable | 14:20 |
| efried | you can tweak your allocation ratio | 14:20 |
| cdent | that won't help | 14:20 |
| cdent | max_unit controls the max individual ask | 14:20 |
| efried | oright | 14:20 |
| efried | we're asking for 32 VCPU? | 14:20 |
| belmoreira | efried: I'm doing that. My allocation ration for CPUs is 99. We just scheduler based in RAM | 14:21 |
| cdent | if the flavor wants 32 on a (now) 16 core machine, no chance | 14:21 |
| efried | true story. | 14:21 |
| efried | I thought we set max_unit to MAXINT | 14:21 |
| efried | no? total? That seems weird, in light of allocation ratio. | 14:22 |
| cdent | that's the default if it is not set when creating the inventory | 14:22 |
| cdent | but the resource tracker sets max_unit in _compute_node_to_inventory_dict | 14:22 |
| cdent | belmoreira: what's the outcome that would be perfect? | 14:23 |
| efried | cdent: I think if we're not going to default to MAXINT, we should default to total * allocation_ratio | 14:24 |
| cdent | presumably if SMT is disabled, then your hardware capacity has in fact halved by some vague definitions. are you wanting to be able to ignore that? | 14:24 |
| efried | total is just... wrong. | 14:24 |
| belmoreira | for my use case allowing the "max_unit" being dynamic considering the allocation ratio | 14:24 |
| cdent | efried: I think we're talking about two different things at once and we should pick your topic up after we've figure out what belmoreira needs to have happy | 14:24 |
| * efried stfu | 14:24 | |
| belmoreira | cdent: I agree with efried (total * allocation_ratio) | 14:25 |
| efried | belmoreira: Easy patch if you want to try it in your env. | 14:26 |
| cdent | isn't that unsafe? it means that we're allowing (for disk, ram and vcpu) a single workload to consume not just all the physical resources, but also all the overcommited resources | 14:26 |
| belmoreira | cdent: answering the "hardware capacity has in fact halved" question. It's a deployment decision in my opinion considering the workloads | 14:27 |
| efried | cdent: IMO setting max_unit to total is just as arbitrary. | 14:27 |
| efried | belmoreira: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6478 | 14:27 |
| efried | If you remove that line, I think the placement side will actually default to MAXINT. | 14:28 |
| efried | checking... | 14:28 |
| efried | yup | 14:28 |
| efried | which is effectively the same as total * allocation_ratio, because that'd be the most you would ever be able to allocate anyway. | 14:28 |
| cdent | efried, belmoreira: I guess my understanding of how hardware works is confused, but making max_unit greater than total is _really_ weird to me. It _can_ work for vcpus, but for disk and ram it sounds downright messy | 14:30 |
| efried | cdent: The operator shouldn't have to think about max_unit. | 14:30 |
| cdent | I agree, which is why it defaults to a safe option | 14:30 |
| cdent | it's there to prevent a single workload clobbering a machine | 14:31 |
| efried | I'm safer if I never leave my house. | 14:31 |
| efried | I don't agree that that one-size-fits-all decision is appropriate across the board. | 14:31 |
| cdent | if you allow two 32 vcpu workloads on a 16 cpu machine, what happens? | 14:31 |
| belmoreira | cdent: The operator is responsible for the available flavors. I agree that can lead to unsafe options but sane confiugration shouldn't the enforced | 14:32 |
| belmoreira | s/the/be | 14:32 |
| efried | guess it depends what overcommit actually means based on how the system is set up. | 14:32 |
| cdent | hmm, we should probably get jaypipes-ooo and dansmith in this conversation when they are available because I remember them in this conversation when it first came up | 14:33 |
| efried | I don't know what ^ is on libvirt, but e.g. on power with micropartitioning, if I have my micropartition size set to .05, each vcpu is actually 1/20 of a physical CPU, so I really do have 20x total. | 14:33 |
| cdent | I agree that max_unit should be changeable | 14:33 |
| belmoreira | cdent: in some of our cells we have a CPU overcommit ratio of 3 | 14:33 |
| cdent | efried: the way power and vmware manage the concept of "vcpu" is much different from libvirt, as I understand things | 14:34 |
| dansmith | I think I brought this up as an issue a couple months ago | 14:34 |
| cdent | you did, which is why I pinged you | 14:35 |
| dansmith | but I'm on two meetings right now, so can't really discuss for another hour plus | 14:35 |
| dansmith | ack | 14:35 |
| efried | cdent: Which is cool, because this would be a change in the libvirt driver specifically. I.e. we can tailor this particular change to whatever makes sense specifically for libvirt. | 14:35 |
| efried | The placement side is MAXINT, which is sane. | 14:35 |
| cdent | belmoreira: is the goal of that overcommit ratio to allow "single workloads that are bigger than the physical constraints of this machine" or to allow "many workloads" | 14:35 |
| cdent | efried: I don't think there's any question that we need to make some changes. We do. What I'm trying to figure out is the more general picture of the goals. | 14:36 |
| jaypipes-ooo | lemme read back | 14:36 |
| cdent | The reason for the original set up was basically to have a "safe" default. However since that default isn't actually changeable, we've introduced a problem. | 14:37 |
| belmoreira | cdent: I was never thinking in these terms... initially definitely to allow "many workloads" but now that SMT is off we need "single workloads that are bigger than the physical constraints of this machine" otherwise all flavors/flavor allocation need to be reviewed | 14:37 |
| cdent | It's not a "default", it's a lock in. | 14:37 |
| efried | I'm just really not a fan of adding more conf opts in the spirit of the allocation ratio ones, which are a tangled freakin mess. | 14:37 |
| cdent | efried: I was trying to avoid going into solution space yet. | 14:38 |
| dansmith | belmoreira: so you want max_unit to go higher not because it makes sense but because you don't want your flavors to change? | 14:38 |
| dansmith | purely because you turned off HT for meltdown I assume? | 14:38 |
| efried | cdent: Argument about mem being allowed to exceed total - that makes sense to me as well, for thin provisioning. | 14:39 |
| efried | sorry, disk, not mem | 14:39 |
| dansmith | cpu is the most legit use of overcommit, fwiw | 14:39 |
| dansmith | like the most straightforward and the most "doable without completely crazy results" | 14:39 |
| belmoreira | dansmith: "not because it makes" :) I think it makes sense... but ran into this problem because SMT off, yes | 14:40 |
| jaypipes-ooo | efried: total is not "just wrong". | 14:40 |
| jaypipes-ooo | this has been this way since before Kilo. | 14:40 |
| jaypipes-ooo | it used to be called limit['vcpus'] but it was always the number of physical CPU cores on the box. | 14:41 |
| dansmith | belmoreira: what I meant was "you wouldn't have chosen this by default, but now you need it because of HT" | 14:41 |
| belmoreira | dansmith: I didn't have this use case until now | 14:43 |
| dansmith | right, that's what I'm trying to get at | 14:43 |
| jaypipes-ooo | belmoreira: ftr, this would have happened without placement. this has been the behaviour of Nova for as long as I remember. it used to be that the scheduler passed a "limits" dict that contained the max number of host CPU processors and would prevent (even with overcommit) an instance from consuming more than that amount. | 14:44 |
| dansmith | when I brought this up last time, I was worried that someone would be specifically designing for this | 14:44 |
| belmoreira | I think it's a good default, but should not be enforced | 14:44 |
| dansmith | jaypipes-ooo: pretty sure that's not the case | 14:44 |
| dansmith | jaypipes-ooo: because we had some tests that had to change because of this, which is why I brought this up last time | 14:44 |
| efried | belmoreira: So what would actually happen if you *were* able to get past the allocation candidates here? You would get into the virt driver's spawn method, but would libvirt actually be able to create a VM with 32 VCPU on this system? | 14:44 |
| jaypipes-ooo | dansmith: we have had limits['vcpu'] set to the max host CPU processors for a long time. | 14:45 |
| dansmith | jaypipes-ooo: at the very least if you didn't have the cpu filter enabled, then you wouldn't get a cpu limit | 14:45 |
| jaypipes-ooo | dansmith: oh, yes, that's true indeed. | 14:45 |
| dansmith | jaypipes-ooo: and cpu filter was not enabled by default ever, AFAIK | 14:47 |
| cdent | belmoreira: do you want "should not be enforced" or "should be changeable" | 14:48 |
| dansmith | https://github.com/openstack/nova/blob/stable/ocata/nova/conf/scheduler.py#L268-L276 | 14:48 |
| belmoreira | efried: That's a good question... never tried. I just went from understanding why placement was not retrieving the allocation candidates to all these open questions | 14:48 |
| efried | well | 14:48 |
| belmoreira | efried: because in the past my flavors were designed considering the max_unit | 14:49 |
| efried | well | 14:50 |
| efried | designed considering the allocation ratio | 14:50 |
| efried | belmoreira: Can you apply this patch and see if you actually succeed in spawning? WIP: libvirt: Turn off max_unit https://review.openstack.org/604110 | 14:50 |
| efried | I suspect you'll actually fail (just later). | 14:50 |
| cdent | efried: why do you say [t 1rID]? | 14:51 |
| purplerbot | <efried> designed considering the allocation ratio [2018-09-20 14:50:11.520176] [n 1rID] | 14:51 |
| jaypipes-ooo | dansmith: we removed CoreFilter from default enabled_filters in Ocata AFAICT: https://docs.openstack.org/releasenotes/nova/ocata.html#upgrade-notes | 14:51 |
| efried | cdent: Maybe that was a wrong assumption. | 14:51 |
| belmoreira | efried: actually libvirt can define 32vcpus in one VM (in 16 cores nodes) Because already existing VMs (with 32 vcpus) are running | 14:52 |
| dansmith | jaypipes-ooo: was pretty sure we never had that on by default | 14:52 |
| dansmith | let me look further back | 14:52 |
| efried | I don't understand how that works, but okay. | 14:52 |
| dansmith | jaypipes-ooo: not in there in newton either: https://github.com/openstack/nova/blob/newton-eol/nova/conf/scheduler.py#L114 | 14:52 |
| belmoreira | efried: at least users didn't complain. I will review them anyway | 14:54 |
| cdent | belmoreira: presumably those are vms that existed before the shut down to get the compute node having the new SMT settings. I wonder if creating a new one and starting and existing one is any different. I would assume not, but these are murky waters. | 14:55 |
| jaypipes-ooo | dansmith: ack. | 14:55 |
| jaypipes-ooo | efried: for the record... https://github.com/openstack/nova/blob/liberty-eol/nova/scheduler/filters/core_filter.py#L48-L51 | 14:55 |
| jaypipes-ooo | efried: the use of total vcpus reported by the host has been the limit/max_unit for a long time. dansmith is correct that the CoreFilter has not been in the enabled filter defaults, though. | 14:56 |
| jaypipes-ooo | which surprises me, actually... | 14:56 |
| jaypipes-ooo | considering we count vcpu quotas by default, but whatever. | 14:56 |
| efried | jaypipes-ooo: Look at line 46 though - that limit is being set to total * allocation ratio | 14:56 |
| *** e0ne has joined #openstack-placement | 14:56 | |
| efried | which was one of the first things we said made more sense than just total. | 14:57 |
| jaypipes-ooo | efried: that's not the right limit. | 14:57 |
| dansmith | so for me, I want to focus on whether or not it makes sense to have max_unit over total for real, separate from belmoreira's rather synthetic problem here | 14:57 |
| jaypipes-ooo | efried: sorry, gave you wrong line numbers: https://github.com/openstack/nova/blob/liberty-eol/nova/scheduler/filters/core_filter.py#L53-L55 | 14:58 |
| belmoreira | cdent: I also assume not. But I will test this | 14:58 |
| dansmith | I'm not sure that it really does make sense | 14:58 |
| jaypipes-ooo | to quote the code... | 14:58 |
| jaypipes-ooo | # Only provide a VCPU limit to compute if the virt driver is reporting | 14:58 |
| jaypipes-ooo | # an accurate count of installed VCPUs. (XenServer driver does not) | 14:58 |
| jaypipes-ooo | and | 14:58 |
| jaypipes-ooo | # Do not allow an instance to overcommit against itself, only | 14:58 |
| jaypipes-ooo | # against other instances. | 14:58 |
| efried | dansmith: I contend that at least disk overcommit makes sense if thin provisioning is in play. | 15:00 |
| dansmith | efried: disagree, because you definitely don't want one single allocation to be larger than the actual size of yourdisk | 15:00 |
| dansmith | thin or not | 15:00 |
| efried | shrug | 15:00 |
| dansmith | this is about max_unit, right/ | 15:00 |
| efried | IMO that shouldn't be hardcoded | 15:00 |
| efried | should be up to the operator | 15:00 |
| cdent | yeah, we need to make sure we're focusing on the meaning of max_unit here, not "overcommit" these are two separate things | 15:00 |
| dansmith | well, the point of placement is to provide a sane theoretical model, IMHO | 15:01 |
| efried | Placement is doing the right thing here. | 15:01 |
| cdent | I'm not sure it should be up to the operator, but it should be up to the virt driver | 15:01 |
| efried | It's the driver - libvirt in this case - that's imposing max_unit = total. | 15:01 |
| dansmith | I thought the point was that max_unit can't be higher than total in placement? | 15:01 |
| efried | nope | 15:01 |
| efried | max_unit defaults to MAXINT if not specified. | 15:02 |
| dansmith | can it be? | 15:02 |
| dansmith | hrm | 15:02 |
| efried | absolutely | 15:02 |
| *** takashin has left #openstack-placement | 15:02 | |
| dansmith | I thought it couldn't.. I thought that was kindof the point | 15:02 |
| jaypipes-ooo | dansmith: efried may be talking about the JSONSchema in placement API. | 15:03 |
| jaypipes-ooo | dansmith: but AFAIK, we never set max_unit > total anywhere in the virt drivers. | 15:03 |
| efried | I'm talking about nova/api/openstack/placement/handlers/inventory.py:45 | 15:04 |
| cdent | that's basically pre-schema default management | 15:04 |
| jaypipes-ooo | dansmith: and the default value we use for max_unit is in fact total for libvirt virt driver in all supported resource classes (cpu, ram, disk and vgpu) | 15:05 |
| cdent | and we chose maxint because we didn't want placement to a) need to know anything or guess anything, b) have built in relationships/smartness between those various fields: we wanted the client side to control (as nova has done) | 15:05 |
| efried | We definitely don't do any numeric calc/limitation in the schema | 15:05 |
| efried | "max_unit": { | 15:05 |
| efried | "type": "integer", | 15:05 |
| efried | "maximum": db_const.MAX_INT, | 15:05 |
| efried | "minimum": 1 | 15:05 |
| efried | }, | 15:05 |
| jaypipes-ooo | cdent: right, which is the correct thing, IMHO. | 15:06 |
| cdent | i agree | 15:06 |
| efried | ++ | 15:06 |
| jaypipes-ooo | cdent: this is client-side logic, not server-side logic. | 15:06 |
| efried | ^ | 15:06 |
| cdent | I know | 15:06 |
| jaypipes-ooo | each client can and probably should decide its own for max_unit-ness... | 15:06 |
| cdent | I agree | 15:06 |
| efried | ++ | 15:06 |
| efried | Earlier I gave an example of why powervm would definitely not want max_unit = total | 15:07 |
| cdent | the debate appears to be over what actor in the client should be making the decision: the individual virt drivers, nova a whole, or the operator/deployer | 15:07 |
| cdent | where in this case "client" == "nova" | 15:07 |
| efried | Is it not sufficient to constrain maximum VCPUs via the flavors themselves? | 15:08 |
| efried | Or is this a thing where, because flavors can be shared among different-size hosts, we want an additional cap to make sure one instance can't ever overcommit? | 15:09 |
| cdent | you don't want huge flavors going to small servers | 15:09 |
| efried | right, and that's not enforced any other way? | 15:09 |
| cdent | it's what max_unit is _for_ | 15:09 |
| efried | Right, I mean, I guess if we look at this from that angle, being able to set a max_unit per host makes sense. | 15:10 |
| belmoreira | cdent: I don't agree. if it's bellow the allocation ratio should be allowed. It's a operator decision | 15:10 |
| *** helenafm has quit IRC | 15:10 | |
| efried | jaypipes-ooo: Did your generic provider override yaml spec allow for this? Setting overrides for any field of any inventory on any provider on the host? | 15:11 |
| cdent | belmoreira: I was speaking in terms of the design of placement: the reason max_unit exists is to define the size of maximum workload for a resource provider | 15:11 |
| cdent | how we control that value is up for debate | 15:11 |
| cdent | as far I can tell, however, it's purpose/meaning is not | 15:11 |
| cdent | its | 15:12 |
| efried | agree with that | 15:12 |
| belmoreira | I need to leave. Thanks for helping me understand the issue | 15:13 |
| cdent | belmoreira: thanks for brining it. If you're able to come back at some point with the results of the experiments menionted above, that would be great | 15:14 |
| cdent | I'm not sure how this discussion is going to resolve | 15:14 |
| jaypipes-ooo | efried: pretty sure it did. | 15:15 |
| jaypipes-ooo | efried: yeah, it did: https://review.openstack.org/#/c/550244/2/specs/rocky/approved/provider-config-file.rst@122 | 15:16 |
| efried | jaypipes-ooo: Not sure how that would play for nrp | 15:17 |
| efried | oh, that's what gibi is saying in that comment. | 15:17 |
| efried | Like, basically, the file needs some way to identify which provider it's mucking with. And sometimes you wouldn't have a UUID or even a name until the provider gets created by nova. Chicken/egg. | 15:18 |
| jaypipes-ooo | efried: which is why I answered gibi the way I did. | 15:18 |
| efried | e.g. if what I wanted to do was muck with the VCPU allocation ratio for my NUMA nodes... | 15:19 |
| efried | In any case, where I was going with that is, I'm thinking it makes sense to allow the op to override max_unit, but it would be crazy to try doing that via conf options for exactly this reason: we need a way to identify the provider and the resource. Which we can mostly do sanely today, but it won't hold up into the future with nrp. | 15:22 |
| efried | so if we're going to allow such an override, we should try to resurrect this yaml deal - but we need to address the `identification` issue. | 15:23 |
| *** s10 has quit IRC | 15:23 | |
| efried | I don't know if it makes sense from an op point of view to bring up the compute, let it auto-populate providers, dump some placement info, and use that to compose the yaml file (identifying providers by the generated names/UUIDs). Then the next periodic picks up the changes and we're good to go. | 15:25 |
| cdent | that makes it hard to manage the compute nodes from something like ansible | 15:26 |
| efried | Would work okay for messing with things like reserved, min/max units, and allocation ratios on existing resources on existing providers. Not so well for adding/removing providers, adding/removing whole resources, etc. But I'm not sure the latter are sane use cases anyway. | 15:27 |
| efried | cdent: Not sure there's any mechanism that would allow us to do this kind of tweaking more easily from ansible. | 15:28 |
| cdent | what i mean is that you've defined a two stage process. which people don't want. | 15:28 |
| cdent | it may be that it is not possible to do it some other way, but that concern should be remembered | 15:29 |
| efried | The deployer (human or tool) really needs to understand how the provider tree is built. | 15:29 |
| cdent | ideally the compute node itself should be a "cloud native app" as much as possible | 15:29 |
| efried | Though I guess that's sort of the case for constructing flavors anyway. | 15:29 |
| efried | If the providers in the tree are named predictably, you could reduce to single-stage. But that requires the deployer to duplicate a lot of the logic of e.g. the virt driver and/or neutron/cyborg/cinder/whoever is responsible for creating and naming providers. | 15:30 |
| efried | how else? | 15:31 |
| mriedem | cdent: you have a spec for being able to list resource providers as having some kind of inventory right? | 15:32 |
| cdent | wow | 15:32 |
| cdent | jinxness | 15:32 |
| efried | mriedem: https://review.openstack.org/#/c/600016/ | 15:32 |
| mriedem | yeah ok i thought so | 15:33 |
| mriedem | i remember you commenting on that in a nova fix | 15:33 |
| efried | Does that help here? Or were you starting a new topic mriedem? | 15:33 |
| mriedem | i wasn't following along, | 15:34 |
| mriedem | just interjecting | 15:34 |
| cdent | efried: it's from a conversation in #openstack-sdks | 15:34 |
| cdent | efried: in response to "how else?". I'm not really sure. I think we might need to solve small problems one at a time rather than coming up with Solutions™ because we haven't got the bandwidth or brains to make good enough choices. | 15:37 |
| efried | cdent: Yeah, what I fear is that we'll wind up adding ${resource_class}_max_unit conf options, and then as soon as those resources are no longer on the compute node, we end up with a godsawful mess like what we're going through now with allocation ratios. | 15:38 |
| cdent | Yes, I agree that nested providers are difficult to manage. | 15:39 |
| efried | we at least know we're moving toward a future where we can't count on resources - any resources, really - being on the compute node RP. And when they're not, trying to use some constant value for settings thereon won't make sense. | 15:40 |
| efried | Good luck trying to define a conf option that indicates | 15:41 |
| efried | vcpu_max_unit = $total * $allocation_ratio for whatever inventory you find VCPU on | 15:41 |
| cdent | i wonder if we need the same kind of "allow operator to set and don't override if they do" logic that allocation ratios (will) have, for *_unit. Or inventory in general. It occurs to me (only now) that we have a way to check for a new inventory: the updated_at field is null | 15:47 |
| cdent | but we don't expose that externally anywhere | 15:47 |
| cdent | efried: heads up in case I forget to mention it otherwise: i'm out m-w this coming week | 15:55 |
| efried | Not sure how the updated_at thing is germane. Agree we need "allow operator to set" with some kind of sane default (which IMO would still be MAXINT, but for backward compat will probably need to be `total` instead). But that doesn't address the problem of *how* to set it - how to target a specific max_unit on a specific resource class on a specific provider. | 15:55 |
| efried | ack | 15:55 |
| cdent | efried: I was going back to belmoreira's needs, not the nested stuff. | 15:56 |
| efried | yeah, I'm sayin, we can satsify belmoreira's needs right now with simple $rc_max_unit conf options, but that's going to bite our ass later. | 15:57 |
| cdent | right, I'm trying to say: we can avoid that | 15:57 |
| cdent | if we want | 15:57 |
| cdent | allow the existing code to do its thing | 15:57 |
| cdent | but make it so it only sets new inventory | 15:58 |
| cdent | allow the op to use the api to set inventory that the RT won't clobber | 15:58 |
| cdent | I don't like that very much, but it's a way that satisfies your constraint | 15:58 |
| cdent | however | 15:58 |
| cdent | I still don't think we've really satisfied the underlying debate there on how/what max_unit means and who should controls it, so I'm not really sure why I'm talking ... :) | 15:59 |
| edleafe | thinking out loud? | 16:00 |
| cdent | guess so | 16:00 |
| cdent | feverish? | 16:00 |
| efried | well | 16:01 |
| efried | I think the op needs to be ultimately able to declare that their instances aren't limited to max_unit = total. | 16:02 |
| efried | but I recognize that changing the default at this stage is not likely to be an option | 16:03 |
| efried | which means we need something configurable (if we're going to solve it at all) | 16:03 |
| cdent | when you say "configurable" do you mean "conf or other files on the compute node" or does that also include "manipulating the inventory over the api from somewhere else" | 16:04 |
| efried | the latter would be okay I suppose | 16:06 |
| efried | tricky to implement tho | 16:06 |
| efried | easiest with new support from the API | 16:07 |
| efried | PATCH /rp/{u}/inventories :P | 16:07 |
| cdent | how does that make any difference? | 16:08 |
| efried | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L6478 <== delete this line, and instead of getting MAXINT as you would today, overriding whatever the user set manually, it leaves whatever's already set in max_unit alone | 16:10 |
| efried | otherwise you have to GET first, check/set max_unit explicitly | 16:11 |
| efried | which I guess since we're caching, we can do... | 16:12 |
| efried | initial setup is tricky in any case | 16:13 |
| efried | guess that's where you're talking about trying to use updated_at | 16:14 |
| cdent | you probably want to GET first anyway, because you need the right generation, at which point a PUT is straightforward | 16:14 |
| cdent | in general blind PATCH is not recommended | 16:14 |
| efried | mm | 16:15 |
| efried | though that's in spirit what we're doing now (blind PUT) | 16:16 |
| efried | cache contains the latest generation, but we're blindly overriding the contents with whatever upt tells us | 16:17 |
| cdent | In my experience PATCH encourages a lot of weird behaviors so is not something I'd want the API to expose. What nova chooses to do on its side in its use of the API is entirely up to nova (and that's a good thing). | 16:17 |
| *** e0ne has quit IRC | 16:41 | |
| openstackgerrit | Lee Yarwood proposed openstack/placement master: wsgi: Always reset conf.CONF when starting the application https://review.openstack.org/604167 | 17:11 |
| lyarwood | cdent: ^ re your comment in https://review.openstack.org/#/c/603372/ , was that more what you had in mind? | 17:12 |
| * lyarwood isn't sure that's even valid, just trying to resolve this as it keeps coming up downstream | 17:12 | |
| cdent | lyarwood: I hadn't got around to think about it deeply yet. What does reset() actually do? Because by name you'd think that was clearing the configuration which is not what we'd want at that point | 17:13 |
| openstackgerrit | Lee Yarwood proposed openstack/placement master: wsgi: Always reset conf.CONF when starting the application https://review.openstack.org/604167 | 17:14 |
| lyarwood | cdent: https://github.com/openstack/oslo.config/blob/master/oslo_config/cfg.py#L2665 - yeah it's clearing everything from CONF before we reload it all again in init_application, it's heavy handed but should workaround things in the short term. | 17:15 |
| cdent | in that case it needs to be _before_ _parse_args | 17:16 |
| cdent | otherwise you are clearing the conf before it is being used to configure the db | 17:16 |
| lyarwood | cdent: yeah sorry just moved it in the second PS above | 17:17 |
| cdent | or setup the wsgi app | 17:17 |
| cdent | in that case, seems sane to me. have you tried it? | 17:17 |
| cdent | i mean with the database related thing | 17:17 |
| lyarwood | I'll try it now | 17:18 |
| openstackgerrit | Lee Yarwood proposed openstack/placement master: wsgi: Always reset conf.CONF when starting the application https://review.openstack.org/604167 | 17:18 |
| *** tssurya has quit IRC | 17:20 | |
| cdent | lyarwood: tomorrow I'll make sure it does make uwsgi-based stuff blow up | 17:28 |
| lyarwood | cdent: doesn't* ? | 17:50 |
| cdent | yes, sorry. contractions are my nemesis | 17:51 |
| lyarwood | cdent: but yeah this appears to work in devstack, I'll try it out with a downstream build under httpd tomorrow and remove the reference to httpd in the change | 17:51 |
| cdent | yay! | 17:51 |
| *** e0ne has joined #openstack-placement | 18:10 | |
| *** cdent has quit IRC | 18:44 | |
| *** e0ne has quit IRC | 19:03 | |
| *** e0ne has joined #openstack-placement | 19:30 | |
| *** e0ne has quit IRC | 19:34 | |
| *** belmoreira has quit IRC | 19:35 | |
| *** e0ne has joined #openstack-placement | 19:43 | |
| *** tetsuro has quit IRC | 21:35 | |
| *** e0ne has quit IRC | 21:36 | |
| *** tetsuro has joined #openstack-placement | 21:37 | |
| *** takashin has joined #openstack-placement | 21:50 | |
| *** mriedem is now known as mriedem_away | 23:10 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!