Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-27
14:51:47 kashyap mriedem: Darn
14:51:58 kashyap In the original series, I _did_ canonicalize
14:52:11 kashyap (s/series/patch/)
14:52:36 sean-k-mooney kashyap: ya i was about to check if it was done in the previous patch
14:52:58 kashyap sean-k-mooney: In the original backported version we ignore the case
14:53:01 kashyap Hmm
14:53:47 sean-k-mooney kashyap: in that case you proably want a patch to just lowercase the flags then rebase https://review.openstack.org/#/c/563926/ on top
14:54:04 sean-k-mooney then we can backport the lowercsing seperatly
14:54:35 kashyap sean-k-mooney: Hmm, I'm a bit torn
14:54:46 kashyap Whether to just do the canonicalization to lowercase in this patch
14:54:52 kashyap (Which seems cleaner)
14:54:54 sahid sean-k-mooney: ok thanks
14:55:09 kashyap But there's the "What about backports" thing
14:55:12 sean-k-mooney kashyap: actully we have a choices element in the backport right?
14:55:40 sean-k-mooney kashyap: e.g. the backported verion only allows pcid
14:55:41 kashyap sean-k-mooney: We do, and also have: 'ignore_case=True'
14:55:54 kashyap sean-k-mooney: Yes, only 'pcid' was allowed
14:56:27 sean-k-mooney so for the backport if we always emit lowercase pcid then its fine to jsut canonicalise the names in https://review.openstack.org/#/c/563926
14:56:55 kashyap sean-k-mooney: No, in the backport too we ignore casing.
14:57:00 leakypipes TheJulia: FYI, I'm reviewing the ironic instance_uuid prep_for_spawn() patch. Might take a little while.
14:57:46 TheJulia leakypipes: thanks, no worries :) I just don't want it to fall off radars :)
14:57:56 leakypipes yup, no worries.
15:01:56 sean-k-mooney kashyap: https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L160-L164 this should lowercase it for you
15:02:33 sean-k-mooney oh wait no
15:02:46 kashyap Not really
15:02:49 kashyap Something like:
15:02:50 sean-k-mooney it returns value not processed_value
15:02:50 kashyap extra_flags = set([flag.lower() for flag in CONF.libvirt.cpu_model_extra_flags]
15:04:09 sean-k-mooney kashyap: ya that will solve it i was just hopping that the sting value would convert it to the canonical form pecifed in the choices field on your behalf
15:11:26 openstackgerrit Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766
15:11:27 openstackgerrit Chris Dent proposed openstack/nova master: Isolate placement database config https://review.openstack.org/541435
15:11:28 openstackgerrit Chris Dent proposed openstack/nova master: WIP: Ensure that os-traits sync is attempted only at start of process https://review.openstack.org/553857
15:12:10 openstackgerrit Chris Dent proposed openstack/nova master: WIP: Add PLACEMENT_DB_ENABLED=True to the nova-next job https://review.openstack.org/564067
15:21:51 openstackgerrit Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766
15:21:52 openstackgerrit Chris Dent proposed openstack/nova master: Isolate placement database config https://review.openstack.org/541435
15:21:53 openstackgerrit Chris Dent proposed openstack/nova master: WIP: Ensure that os-traits sync is attempted only at start of process https://review.openstack.org/553857
15:22:09 openstackgerrit Chris Dent proposed openstack/nova master: WIP: Add PLACEMENT_DB_ENABLED=True to the nova-next job https://review.openstack.org/564067
15:24:23 leakypipes jroll, TheJulia: from bug #1766301's report: "This can be preproduced fairly easily with TripleO and a lack of a resource class defined on the flavor."
15:24:23 openstack bug 1766301 in OpenStack Compute (nova) "ironic baremetal node ownership not checked with early vif plugging" [Undecided,In progress] https://launchpad.net/bugs/1766301 - Assigned to Julia Kreger (juliaashleykreger)
15:24:48 leakypipes jroll, TheJulia: from bug #1766301's report: I didn't think we allowed "disabling resource class defined on the flavor" for Ironic?
15:25:16 TheJulia leakypipes: turns out tons of people didn't get the memo that resource classes were required for baremetal
15:26:26 leakypipes TheJulia: the issue I have with the patch is that the solution for this problem (needing an atomic claim of resources) *was* the placement claiming resources for an instance.
15:26:48 leakypipes TheJulia: can this be reproduced *without* "disabling resource classes in flavors"?
15:27:38 jgwentworth when we talked about this last time, it can also happen if other things other than nova are calling ironic API
15:27:41 dtantsur leakypipes: imagine a rare case of nova co-existing with non-nova using ironic
15:28:00 dtantsur (which may not be actually rare)
15:28:05 jgwentworth so it seemed valid to us to go ahead and do the node reserve first
15:28:43 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Lift the restriction of choices for `cpu_model_extra_flags` https://review.openstack.org/563926
15:29:04 leakypipes jgwentworth: other than Ironic, what virt driver needs to "reserve a node"?
15:29:52 jgwentworth leakypipes: none, but it doesn't seem unreasonable to offer virt driver "pre-spawn" and "cleanup-pre-spawn". we already did it, we just named it "setup_network_*" instead
15:30:08 TheJulia leakypipes: I think so, just have two consumers of ironic's api try to deploy the same hardware at the same time
15:30:12 leakypipes jgwentworth: for the record, "reserving a node" is precisely what the claim_resources() is doing in the scheduler when Ironic is involved in the request.
15:30:22 jgwentworth I suggested changing the name of the methods to be more generic and just use them for reserve and unreserve
15:30:27 kashyap mriedem: Does that look better: https://review.openstack.org/#/c/563926/8/nova/virt/libvirt/driver.py@3817
15:30:49 kashyap mriedem: But it won't fix stable branches.
15:31:18 kashyap mriedem: How about sean-k-mooney's idea: Make the "casing normalization" patch on _top_ of the existing change
15:31:27 kashyap So that it can be backported to stable branches too?
15:32:26 jgwentworth leakypipes: yeah, but that won't help if something outside of nova is also using the ironic API, right
15:32:34 jgwentworth which is valid
15:32:59 leakypipes jgwentworth: seems like a poor reason to essentially put in a new locking mechanism into the virt driver API.
15:34:10 leakypipes TheJulia, jgwentworth: why do two consumers of Ironic's API try to deploy the same hardware at the same time?
15:34:25 jgwentworth well, it's not going to be "start locking mechanism" and "release locking mechanism", it should be generic like "pre spawn" "cleanup pre spawn". I thought that's a thing that virt drivers could want to do
15:35:08 TheJulia leakypipes: manual orchestration, disconnected orchestration, single hardware resource pools shared amongst teams using different tooling.
15:35:09 leakypipes TheJulia, jgwentworth: why not have those consumers reserve a node before they start deploying to it? if this is truly out of Nova, I don't see why Nova should have a change to its virt driver API to handle this.
15:36:00 TheJulia how long would it take for nova to pickup that instance_uuid is set and mark the node as in use?
15:36:28 leakypipes TheJulia: I thought you have been discussing Nova *not* being in use here?
15:36:45 mriedem kashyap: if we need to fix stable branches you're going to have to make that a separate change
15:36:46 TheJulia leakypipes: but the flag for reservation/use of a node is quite literally the instance_uuid field
15:37:02 leakypipes TheJulia: let's back up a sec here.
15:37:16 TheJulia hence why we needed to move it earlier because nodes were running out of network ports and failing any network related issue as "block device mapping" failures
15:37:19 leakypipes TheJulia: what is the thing that is the "consumer of Ironic's API"?
15:37:23 jgwentworth leakypipes: they do. the problem is ironic needs to get the IP of an instance before attaching a volume because some volume backends require it, and they have to plug vifs to do it, and they're currently doing that outside of node reserve in their virt driver, because they had to do node reserve in spawn, because there isn't a virt driver method that let's them use it to do node reserve
15:38:03 kashyap mriedem: I damn well tried both upper and lower in my functional test; I didn't catch it there. Bizarre
15:38:14 kashyap mriedem: I'll do another test and then make a comment on the change
15:38:32 leakypipes jgwentworth, TheJulia: can we do a hangout please?
15:38:36 TheJulia leakypipes: any number of nova-compute services, humans with playbooks, humans with python-ironcclient who know exactly what they want and if they honor and place the instance_uuid, the placement stuff doesn't pickup on that until later. The assertion of the instance_uuid field to the ironic API will fail if it is already set which allows nova to know "oh, this can be rescheduled"
15:38:45 TheJulia leakypipes: sure
15:38:50 jgwentworth okay
15:39:29 TheJulia who is making a room?
15:39:42 leakypipes TheJulia, jgwentworth: https://hangouts.google.com/call/v-fWz1j1qrJCQ4mjKR-cAAEE
15:43:15 openstackgerrit Lance Bragstad proposed openstack/nova master: Simplify logic in get_enforcer https://review.openstack.org/531008
15:54:42 openstackgerrit Lance Bragstad proposed openstack/nova master: Deprecate rule:context_is_admin https://review.openstack.org/564349
16:01:57 mriedem finucannot: off the top of your head,
16:02:31 mriedem does hw:cpu_policy=shared just mean vcpus can float across pcpus in any number? like, do we have a way to say that the vcpus must be evenly distributed?
16:03:00 finucannot what do you mean, in any number?
16:03:13 mriedem i think i might be looking for
16:03:14 mriedem FLAVOR-CORES: (comma-separated list of integers) A list of instance vCPUs to map to instance NUMA node N. If not specified, vCPUs are evenly divided among available NUMA nodes.
16:03:17 mriedem from https://docs.openstack.org/nova/latest/user/flavors.html
16:03:51 finucannot Unless you're specifying guest NUMA topologies, I don't think that's what you're looking for
16:04:01 finucannot *requesting
16:04:27 mriedem Kevin_Zheng: around?
16:04:35 finucannot That lets you say "I want 2 cores to go to _guest_ NUMA node 0 and 6 cores to _guest_ NUMA node 1"
16:04:56 finucannot (as an example)
16:05:03 Kevin_Zheng Yeah
16:05:09 mriedem i was thinking if i have a flavor with 8 vcpu and i want those divided evently across numa node 0 and 1
16:05:16 mriedem *evenly
16:05:38 finucannot mriedem: hw:numa_nodes=2
16:05:51 finucannot nova won't split guest NUMA nodes across host NUMA nodes
16:06:23 finucannot (nor will it allow two guest NUMA nodes be placed on the same host NUMA node. I think that's a mistake, but that's neither here nor there)
16:06:45 sean-k-mooney finucannot: that is a limitation of the libvirt dirver

Earlier   Later