Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-13
14:26:10 mdbooth mnaser: Although, the current state of that code is the sum of a bunch of quick fixes...
14:26:13 kashyap stephenfin: Thanks for the fast +2 on that change!
14:26:17 openstackgerrit Balazs Gibizer proposed openstack/nova master: Add sample test for instance audit https://review.openstack.org/480955
14:26:43 stephenfin kashyap: np. Doc fixes like that easy-peasy
14:26:55 mnaser mdbooth: I think the huge challenge would be maintaining those two formats on disk
14:27:04 mnaser Essentially and possibly forever...
14:27:21 mdbooth Reliably, and maintainably
14:27:41 mnaser We can’t assume the instances which are cloned from local cache will eventually disappear because we’d block upgrades.
14:29:11 openstackgerrit Stephen Finucane proposed openstack/nova master: Add PCI NUMA policies https://review.openstack.org/527472
14:29:11 openstackgerrit Stephen Finucane proposed openstack/nova master: trivial: Modify signature of _filter_non_requested_pfs https://review.openstack.org/527473
14:29:30 kashyap stephenfin: I'll answer your question on the review
14:34:04 openstackgerrit Balazs Gibizer proposed openstack/nova master: Wait for live_migration_rollback.end notification https://review.openstack.org/527444
14:35:10 kashyap stephenfin: Is it just me, or the rendered page looks like full of "alerts" with the red colour -- https://docs.openstack.org/nova/pike/configuration/config.html
14:35:27 kashyap The whole config page looks like an "exceptions" page
14:35:55 stephenfin kashyap: Are you referring to the titles of each options or the 'warning' admonitions?
14:36:17 kashyap The 'warning' admonitions, and every option enumerated in red color
14:36:47 stephenfin The warning options are warranted - we don't want people using those as they're deprecated
14:36:51 kashyap (Looking at that really messing with my briain :-) I'll just stick to looking at the code.)
14:36:56 kashyap The warnings are fine
14:37:07 stephenfin and we've been removing deprecated options at a decent clip, though it's low priority
14:37:15 kashyap But the options should be changed to 'lime green' or something saner
14:37:23 stephenfin The other stuff is the fault of openstackdocstheme, I'm afraid
14:37:40 kashyap Okido; no worries.
14:37:42 stephenfin I'm not sure what colour I _would_ use though, tbh
14:42:43 mriedem at some point i (or someone) should follow the pattern yikun has here for the 2.57 samples and do those for the 2.1 and 2.51 samples
14:42:51 mriedem and use real operations rather than fakes
14:43:08 mriedem i've always hated that we use fakes for the instance action api samples
14:46:33 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove dead parameter from '_create_domain_and_network' https://review.openstack.org/512352
14:53:40 gibi mriedem: indeed it looks a lot cleaner now, I approved it
14:54:26 mriedem gibi: cool, thanks
14:54:28 mriedem one more down
15:04:28 openstackgerrit Matt Riedemann proposed openstack/nova master: DNM: see what else is doing useless migration context stuff https://review.openstack.org/471491
15:07:54 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Change CONF.my_ip to be IPOpt https://review.openstack.org/498095
15:14:16 niraj_singh efried_cya_jan: in cinder to nova interaction using service token. I am getting roles': [u'Member'], for service user in nova.
15:15:44 niraj_singh efried_cya_jan: I was logged in as admin. Is there anything need to do to assign role to service user.
15:17:04 jaypipes niraj_singh: Eric is out until January (thus his nick is efried_cya_jan) :)
15:17:23 niraj_singh ooh sorry
15:17:59 jaypipes niraj_singh: not a problem. you will likely need more roles than "Member" for the cinder service user.
15:19:21 niraj_singh jaypipes: can i assign role to service user explicitlely?
15:23:03 jaypipes niraj_singh: hmm, I *think* you should be able to do that, yes. via Keystone...
15:23:22 jaypipes niraj_singh: you'll want to restart the nova services after that, though, since the token will be cached in the Nova services.
15:23:55 niraj_singh jaypipes: ok. i will try this.
15:23:59 niraj_singh thanks
15:24:04 dansmith jaypipes: you gonna rev that patch that needs the comment added?
15:24:17 dansmith jaypipes: sounds like modulo those nits gibi and I are ready to send it to heaven
15:28:51 mriedem stephenfin: your comment in https://review.openstack.org/#/c/523919/ - i think the code your pointing at has nothing to do with what i'm removing in that patch
15:30:18 stephenfin mriedem: It's quite possible. The thing that put me onto that was that we _were_ storing 'self.extra_info' in the line you're removing, but once that's removed we don't appear to use it anywhere
15:30:35 stephenfin ergo, I figure the storing aspect should be removed to
15:30:49 stephenfin Maybe I missed something though. I'm looking at that commit you referenced now
15:31:08 mriedem stephenfin: it's called from PciDevice.create() https://review.openstack.org/#/c/523919/2/nova/objects/pci_device.py@234
15:31:13 mriedem which will dirty the extra_info field
15:31:20 mriedem so that it gets save()'d later
15:31:30 mriedem updates = self.obj_get_changes()
15:31:35 mriedem will contain extra_info
15:31:52 mriedem hence this note in update_device:
15:31:53 mriedem # NOTE(yjiang5): extra_info.update does not update
15:31:53 mriedem # obj_what_changed, set it explicitly
15:32:37 openstackgerrit Merged openstack/nova master: Stabilize test_live_migration_abort func test https://review.openstack.org/527440
15:34:59 mriedem dansmith: melwitt: here is the marker not found fix for the build_requests thing that came up yesterday https://review.openstack.org/#/c/527564/
15:35:12 openstackgerrit Balazs Gibizer proposed openstack/nova master: Transform instance.exists notification https://review.openstack.org/403660
15:35:21 openstackgerrit Balazs Gibizer proposed openstack/nova master: Add sample test for instance audit https://review.openstack.org/480955
15:36:01 openstackgerrit Balazs Gibizer proposed openstack/nova master: Add regression test for bug 1735407 https://review.openstack.org/526095
15:36:03 openstack bug 1735407 in OpenStack Compute (nova) "[Nova] Evacuation doesn't respect anti-affinity rules" [Medium,In progress] https://launchpad.net/bugs/1735407 - Assigned to Balazs Gibizer (balazs-gibizer)
15:36:03 dansmith mriedem: ack
15:36:08 openstackgerrit Balazs Gibizer proposed openstack/nova master: Add late server group policy check to rebuild https://review.openstack.org/525242
15:36:11 jaypipes dansmith: ya, lemme do that right quick. thanks for the reminder
15:37:28 mriedem dansmith: left a comment in there about something i was thinking about last night wrt getting the build request via the marker up front and then being smarter about getting the build requests that come *after* that marker, rather than getting all of the build requests first, filtering them and then throwing out a bunch - which seems really inefficient
15:37:40 mriedem but my brain hasn't quite been able to put together how that could work
15:38:08 mriedem dansmith: also, i wonder if some of this is why the build request stuff didn't play so nice with your new instance_list stuff + paging
15:38:10 dansmith well we have to filter them based on the json property so we kinda have to load them all right?
15:38:30 mriedem we can find the marker up front but i don't know if that helps us
15:38:30 dansmith mriedem: it's because of ^ that it was less easy to add
15:38:52 dansmith it doesn't because we still have to load them all to figure out where to start after the marker
15:38:57 mriedem sure,
15:39:03 mriedem how about this as an optimization though,
15:39:05 dansmith so could be one shortcut we could add maybe, but..
15:39:14 mriedem if we have a marker, get the build request up front and if not found, bail early
15:39:20 mriedem that would avoid the get_all())
15:39:38 mriedem chances are we aren't going to find a marker in the build request in most cases i wouldn't think
15:40:06 dansmith it's more round trips to the db, and getting the marker ahead of time doesn't really help us select a smaller get_all()
15:40:16 dansmith so I'm not sure it's really worth it, TBH, but.. if you think it's better
15:40:20 dansmith so imagine this scenario:
15:40:46 dansmith nova boot --min-instances=100, followed by a paged list of tens of instances per page
15:40:56 dansmith that could be a couple minutes where your markers are in the build requests
15:41:22 mriedem true
15:41:57 mriedem until the public cloud guys tell me in 18 months that we're spending too much time paging over build requests, i'll leave this alone :)
15:42:23 dansmith aye
15:42:39 stephenfin mriedem: OK, I know where I went wrong. Re-reviewed now
15:43:13 stephenfin dansmith: Mind reminding me why we don't do stuff like this with o.vo objects? https://review.openstack.org/#/c/523919/2/nova/objects/pci_device.py@177
15:43:24 mriedem stephenfin: cool thanks
15:43:59 stephenfin It was the reason for this whole series https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bug/1636338
15:44:06 mriedem stephenfin: could just be laziness on the part of the consuming code,
15:44:18 mriedem i.e. instead of the calling code checking "if 'extra_info' in pci_device'"
15:44:58 dansmith stephenfin: it means that objects default to something being set that may not match what is in the database. so, if I wanted to do something like this:
15:45:38 dansmith d = objects.PciDevice(id=123, label="new_label"); d.save(), we would overwrite extra_info in the database with an empty dict
15:46:09 dansmith may not apply directly to this situation, but that's the pattern we've got elsewhere and it's super confusing to have some of them work like that, and others have silent defaults for some fields
15:46:58 mriedem stephenfin: also without that self.extra_info = {} in __init__, if you called PciDevice.create(), we'd fail here https://review.openstack.org/#/c/523919/2/nova/objects/pci_device.py@168
15:47:47 stephenfin mriedem: yeah, you'd have to harden that to include 'if 'extra_info' in self:' or the likes
15:48:00 mriedem that should really probably be something like, "if 'extra_info' in self: extra_info = self.extra_info else: extra_info = {}"
15:48:01 stephenfin I'd to do sooo much of that in aforementioned series

Earlier   Later