Earlier  
Posted Nick Remark
#openstack-nova - 2018-04-27
14:02:01 bauzas I agree
14:02:16 bauzas I'm just saying we allow that, so communication is the key
14:03:13 sean-k-mooney in future we will likely want to narrow that to say if you run a out of tree driver it must call placement but there is not much add benifit to that vs the filter driver with just one custom filter
14:05:21 sean-k-mooney bauzas: the chance schduler could likely be ported to just a random weigher and a pass all filter for functional tests
14:09:56 openstackgerrit Merged openstack/nova master: placement: resource requests for nested providers https://review.openstack.org/554529
14:10:11 openstackgerrit Merged openstack/nova master: Fix tox -e docs https://review.openstack.org/564631
14:11:16 openstackgerrit Takahito Hirose proposed openstack/python-novaclient master: api_version decorator becomes an error in Python 3.5.0. https://review.openstack.org/564774
14:21:38 openstackgerrit Matt Riedemann proposed openstack/python-novaclient master: Make sure microversion < 2.62 does not show host(Id) for instance actions https://review.openstack.org/564782
14:30:13 sahid mriedem, superdan, when you have a moment i commented on a patch we have worked on
14:30:16 sahid https://review.openstack.org/#/c/497457/30/nova/virt/libvirt/driver.py@7384
14:30:38 sahid i think we will have to find a solution to address a specific case
14:43:18 sean-k-mooney mriedem: im going to be offlie for a while soon but ill try and respin https://review.openstack.org/#/c/553072/5 today.
14:45:12 sahid sean-k-mooney: don't set workflow -1, if you can't address the comments I will try next week
14:45:59 sean-k-mooney mriedem: ill also try and and set up a mixed lb and ovs devstack at the weekend/this evening to test https://review.openstack.org/#/c/551370/ and ill post the local.conf. i should be able to convert that into a ci job once i know what the local.conf need to look like.
14:46:23 sean-k-mooney sahid: well i set -1 since i plan to fix it myself later today once i get home.
14:46:43 mriedem sean-k-mooney: cool, thanks
14:49:57 kashyap mriedem: Fixed another typo in the config file: s/the you need to/then you need to/ in that change
14:50:03 openstackgerrit Kashyap Chamarthy proposed openstack/nova master: libvirt: Lift the restriction of choices for `cpu_model_extra_flags` https://review.openstack.org/563926
14:50:47 mriedem kashyap: did you see the comment from danpb?
14:50:50 mriedem on PS6
14:50:54 kashyap mriedem: Oh, let me see
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?

Earlier   Later