Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-30
17:55:13 openstackgerrit Merged openstack/nova master: Add additional functional tests for NUMA networks https://review.openstack.org/585385
17:59:20 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Use of set_middleware_defaults https://review.openstack.org/587183
18:00:32 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Use own set_middleware_defaults https://review.openstack.org/587183
18:32:33 efried Hey folks, we're back with https://bugs.launchpad.net/nova/+bug/1737711
18:32:33 openstack Launchpad bug 1737711 in OpenStack Compute (nova) "nova boot failed when use the chinese metadata key and value" [Undecided,In progress] - Assigned to huanhongda (hongda)
18:32:59 efried They've restricted the patch https://review.openstack.org/#/c/536236/ to just metadata keys (not extra_specs keys anymore).
18:34:20 efried The bug still doesn't give a whole lot of detail as to the use case for this, but maybe it makes more sense to y'all, dansmith mriedem jaypipes
18:38:21 melwitt efried: fwiw, my initial read on that is we have more than one place where unicode keys won't work (as you mentioned extra_specs is another one) and IMHO it feels like it would be a small spec to propose to change all of them together in a consistent way to support unicode
18:38:39 melwitt is the schema validation really the only thing preventing unicode from working?
18:39:14 efried melwitt: Good question. No idea if they've tried... whatever it is they wanted to try, with their fix in place.
18:39:59 efried melwitt: Anyway, my position on this is that I don't have the background to be able to make a compelling argument to shut down the bug, or ask for it to be converted to a bp, or whatever. I need cavalry support (that would be you).
18:40:41 jaypipes efried: I'm curious why the patch includes a change to add maxLength: 255
18:41:40 efried jaypipes: Yeah, that's redundant (already covered by the regex), but not really important to the question of whether we should do it at all...
18:42:25 efried jaypipes: I added a comment for ya :)
18:42:36 jaypipes efried: right, but AFAICT, the regex doesn't change.
18:42:49 mriedem stephenfin: where does the libvirt driver actually translate hw_cpu_policy and hw_cpu_thread_policy into something that goes in the guest xml?
18:42:58 efried jaypipes: Right, that part of the delta is them making sure the extra_specs pattern does *not* change.
18:43:16 mriedem stephenfin: _get_guest_cpu_config ?
18:44:11 efried jaypipes: Actually, they've correctly carried over the redundant maxLength from the metadata key pattern...
18:44:13 openstackgerrit karim proposed openstack/nova master: Updated AggregateImagePropertiesIsolation filter illustration https://review.openstack.org/586317
18:44:45 efried jaypipes: I suppose if you were being really particular, you could say they did the right thing, because you could get back a different schema error.
18:46:49 efried jaypipes: But it's not about whether the patch is correct (it clearly isn't quite, yet). It's about whether we should allow it at all. Or if we do, if it needs to have microversion paperwork, a bp, a spec, etc.
18:47:12 melwitt efried: understood. at the very least I'd think we'd want tempest tests to cover unicode metadata key cases to verify it for now and keep it working in the future. and I'd think we'd want to change all of the user-settable keys to support unicode at the same time (rather than pick only one). and that's a bigger change and one I'd expect to have a spec about that explains all of the changes needed to properly support unicode in keys
18:47:28 dansmith and tests to cover metadata server and configdrive, amirite?
18:47:35 melwitt that's just my opinion. any other opinions?
18:47:53 dansmith personally I do not think unicode is worth the change to this, no surprises there
18:48:06 efried you anglocentric, you
18:49:03 mriedem i wouldn't ask me, i'd ask alex_xu, ken'ichi, takashi and Kevin_Zheng
18:49:20 mriedem i asked kevin long ago if huawei had any need for unicode in flavor extra specs and i believe he said no
18:49:38 mriedem passing unicode metadata values through to the guest is probably an easier case to make
18:49:49 melwitt I didn't mean opinions on the validity of the change, just whether it would need a spec and review
18:49:50 dansmith resources:<pileofpoo>=1
18:50:57 mriedem normal microversion behavior state diagram says that if i can make a request to cloud A and it works and make the same request to cloud B and it doesn't work, w/o microversions, it should probably be versioned - unless it's just a bug / something broken in existing support
18:51:15 mriedem but this has never worked as far as i know
18:51:22 mriedem so it's not a bug/regression, it's an RFE
18:51:25 dansmith what do you do for keys that are unicode if retrieved from an older microversion?
18:51:30 dansmith detect if they have unicode and exclude them?
18:51:56 mriedem i thought you couldn't do that b/c of the existing schema?
18:52:06 mriedem oh you mean,
18:52:15 efried hypothetically if this change was made, even with a microversion
18:52:16 mriedem set with unicode on newer microversion, retrieve with older microversion?
18:52:23 dansmith mriedem: yeah
18:52:37 mriedem dansmith: idk, detecting that would suck
18:52:42 dansmith mriedem: yup
18:52:46 jaypipes efried: I asked a question on the patch.
18:52:47 dansmith mriedem: and be super confusing for people
18:53:28 mriedem this is generally why we have specs for api behavior changes.... :)
18:53:39 dansmith ahyup
18:53:43 efried jaypipes: In the discussion from a few weeks ago (linked in the bug report) we talked about it not being a good idea for extra specs. I think it was in reaction to that that they reverted that part.
18:53:45 jaypipes although I do like dansmith's pile of poo resource.
18:53:56 dansmith jaypipes: one pile of poo please, affined to numa node #2
18:54:07 jaypipes side of fries with that, dansmith?
18:54:17 dansmith jaypipes: only after some hand sanitizer
18:54:21 jaypipes :)
18:54:29 efried affinitizations for 914 points
18:55:25 sean-k-mooney mriedem: does the api activly reject unicode?
18:55:46 efried sean-k-mooney: We're talking about in metadata/extra_specs keys, where the schema is patters
18:55:51 efried pattern-limited to ascii.
18:55:58 efried so yeah
18:56:24 melwitt efried: I'm going to comment on the patch
18:56:41 efried melwitt: Okay, I was about to update the bug.
18:56:42 sean-k-mooney efried: oh ok i was going to say we dont mandate a coralation type for the db so someone could have created a db with utf-8 set and would be able to store it
18:56:59 mriedem sean-k-mooney: can you answer this question to stephen? "where does the libvirt driver actually translate hw_cpu_policy and hw_cpu_thread_policy into something that goes in the guest xml?"
18:57:04 melwitt efried: feel free to do that
18:57:13 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Use oslotest CaptureOutput fixture https://review.openstack.org/587129
18:57:14 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Use a non-nova log capture fixture https://review.openstack.org/587130
18:57:15 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Use a simplified WarningsFixture https://review.openstack.org/587131
18:57:35 efried melwitt: Procedurally, if we've deemed this to need a bp/spec, do I set the bug to Won't Fix?
18:57:55 mriedem i don't think so, it would be a wishlist bug
18:58:01 mriedem invalid -> wishlist or something
18:58:05 sean-k-mooney mriedem: let me see if i can find it. i can i can give you the relevent xml snipit it generates
18:58:12 efried ight
18:58:37 mriedem all i mostly see is the giant hardware.py methods,
18:58:43 mriedem but can't link those up to where it's used by a driver
18:59:14 mriedem maybe it's not directly set in the guest xml? maybe it's just used to determine which cpus to pin?
19:00:01 sean-k-mooney mriedem: its burried in the numa code
19:00:19 sean-k-mooney mriedem: yes it just used to determin the pinning
19:00:21 efried melwitt, mriedem: Do we have a helpful contributor link to the bp/spec process?
19:00:28 mriedem yes
19:00:34 sean-k-mooney it never gets into the xml itself
19:00:40 mriedem https://docs.openstack.org/nova/latest/contributor/blueprints.html
19:00:48 mriedem sean-k-mooney: ok then, that answers that, thanks
19:01:11 efried ack
19:02:14 sean-k-mooney mriedem: bassicaly we generate teh pinning here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L4471-L4480
19:07:13 MultipleCrashes Anyone free to take up this review further : https://review.openstack.org/#/c/563418/
19:21:04 openstackgerrit Eric Fried proposed openstack/nova master: Updated AggregateImagePropertiesIsolation filter illustration https://review.openstack.org/586317
19:25:45 openstackgerrit karim proposed openstack/nova master: Updated AggregateImagePropertiesIsolation filter illustration https://review.openstack.org/586317
19:26:55 melwitt MultipleCrashes: are you asking for review or help with updating the patch or both?
19:28:10 MultipleCrashes I am new to it, in my knowledge we need a +2 for a merge ..mostly looking for a possibility of merge
19:30:37 sean-k-mooney MultipleCrashes: just looking at the bug you are getting a keysonte error form calling neutronport delete in a bulk delete of nova instnaces.
19:31:01 sean-k-mooney this almost looks like we are ddosing the neuron api with too many requests at once
19:31:24 sean-k-mooney retry is certenly one want to solve it but perhaps we should be ratelimiting
19:33:04 MultipleCrashes yeah , apparently this happens when too many instances are simultaneously deleted , if we try a rate limiting there is possibility that the task of deleting the VMs might get interrupted.
19:33:38 MultipleCrashes Eg:lets say we are deleting 1000 instances and the problem occurs at 550th (say) instance
19:34:21 MultipleCrashes we would still like to continue the process, probably rate limit might stop further execution
19:34:30 melwitt MultipleCrashes: the last comment on the review suggests a change to avoid logging error per retry and instead log info for the retry and then if all retries have failed, log error. otherwise the operator gets a false log error if one of the retries succeeds
19:35:14 melwitt and by false I mean, operators consider "error" to mean action should be taken
19:36:06 sean-k-mooney MultipleCrashes: well the error is stemming from data = neutron.list_ports(**search_opts). so in your case we would be doing 1000 concurrent requets to netron to list the ports for the 1000 instnaces.
19:36:11 MultipleCrashes yeah, since with the retries we would also be having instance ids, so we would be able to figure out that the retires are being done for this particular intance
19:37:25 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix formatting for vcpu_pin_set and reserved_huge_pages https://review.openstack.org/587206

Earlier   Later