Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-13
10:15:57 johnthetubaguy cool, I was wondering if I was going down a crazy thought path
10:17:11 mdbooth dakhmetov: Incidentally, if you're close to users who are actually relying on this, I'd be interested to know what your take is on how to fix the changing username issue.
10:17:38 johnthetubaguy +1 thats what I was fishing for really
10:18:40 mdbooth dakhmetov: Is the issue that people have scripted this, and scripts will require updating?
10:19:10 johnthetubaguy FWIW, this change breaks those folks too
10:19:28 mdbooth johnthetubaguy: Really? I missed that.
10:19:43 johnthetubaguy all existing instance, once modified will now have no username or projectname, we are missing a unit test for that
10:20:42 mdbooth johnthetubaguy: Ah, yes. It's a fix only for future instances, I'd glossed over that.
10:21:07 mdbooth We could save the info before deleting it...
10:21:28 mdbooth Meh, lets get this done before gold plating it.
10:25:14 johnthetubaguy dakhmetov: I updated my comments to try summarise what I am thinking now, basically I would split this into two patches, and the second patch needs an extra unit test.
10:25:16 mdbooth dakhmetov: Can you quickly write the test case johnthetubaguy has asked for? Should be simple enough and probably just works, but we should definitely be testing that.
10:32:39 alex_xu Qiming: gmann, yea, i mean it even worse, they have similar name, but doing different thing
10:40:19 dakhmetov mdbooth: johnthetubaguy: reading your comments
10:41:35 dakhmetov as workaround for outdated names in instance's system_metadata: names can be updated in mysql
10:42:28 dakhmetov it's dirty hack, but anyway solves the possible issue
10:44:03 mdbooth dakhmetov: I think the safer option is to test that having nothing there at least doesn't break existing instances. That means that we're not regressing. Saving the info before rewriting the xml is better, but also a game of whack-a-mole, so best be safe first I think.
10:49:48 kashyap lyarwood: Isn't melwitt right here -- https://review.openstack.org/#/c/441204/5/nova/tests/unit/virt/libvirt/test_guest.py
10:50:01 dakhmetov mdbooth: So, what I need is to handle the situation with missing owner_user_name and owner_project_name in instance's metadata, as well as add test for this. Right?
10:50:05 kashyap Specifically her question about 'live=False'
10:50:36 johnthetubaguy dakhmetov: as long as we test for that, thats my main worry out the way
10:50:47 mdbooth dakhmetov: I suspect it already handles that, but we should definitely test as johnthetubaguy says.
10:50:56 mdbooth If it doesn't work, that would be a regression.
10:52:12 johnthetubaguy dakhmetov: yeah, I think the get() is OK, but deep down, I would rather we didn't populate the names at all myself, hence me being tempted to do this as two patches, one that just gets user_id and project_id from the instance, sets the others to None, then a second patch that adds in your cache of the names.
10:54:05 mdbooth johnthetubaguy: How about resolving username and projectname from keystone at the time we rewrite the metadata?
10:54:15 mdbooth Rather than storing it in system meta
10:54:18 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/nova master: Read NIC features in libvirt https://review.openstack.org/444415
10:54:24 mdbooth Would that be so bad?
10:56:02 openstackgerrit Stephen Finucane proposed openstack/nova-specs master: Reserve NUMA nodes with PCI devices attached https://review.openstack.org/364468
10:57:16 dakhmetov mdbooth: if we open Keystone session to resolve ids to names in libvirtxmdriver with token from request context, it may not have permissions to resolve names in keystone
10:57:46 dakhmetov we may not have permissions to resolve IDs to names in keystone even if we always open admin session
10:57:52 dakhmetov it's how I see it
10:57:57 mdbooth dakhmetov: Explain
11:00:19 mdbooth dakhmetov: I'm not being funny, I just don't understand that interaction all that well.
11:01:44 dakhmetov mdbooth: keystone policies are adjustable in a wide range
11:01:47 dakhmetov https://github.com/openstack/keystone/blob/master/etc/policy.json
11:02:17 mdbooth dakhmetov: Where did the lookups come from originally?
11:02:55 mdbooth Are you saying that the policy context is different between creation and subsequent lookup, or that somebody might have changed the policy?
11:03:04 dakhmetov mdbooth: so, listing users or projects can even be prohibited
11:04:18 mdbooth dakhmetov: It would be a pretty odd policy which denied an authenticated user permission to lookup their own username though, right?
11:05:07 mdbooth "identity:get_project": "rule:admin_required or project_id:%(target.project.id)s",
11:05:17 mdbooth "identity:get_user": "rule:admin_or_owner",
11:05:43 johnthetubaguy dakhmetov: I don't think we should call out to keystone in there
11:06:26 mdbooth johnthetubaguy: I know that came up earlier in the review, but are you thinking of performance or robustness when you say that?
11:06:33 johnthetubaguy dakhmetov: I would rather we get the patch in that uses instance.project_id and instance.user_id first, thats an important fix (we could use the context)
11:07:02 johnthetubaguy mdbooth: a combination of all of those really, plus security around every compute node having those permissions, and setting that up correctly
11:07:56 johnthetubaguy mdbooth: matching uuids to current names feels like something the monitoring systems could do
11:08:07 openstackgerrit jichenjc proposed openstack/nova master: Trivial Fix https://review.openstack.org/442310
11:09:49 openstackgerrit jichenjc proposed openstack/nova master: Add description for Image location in snapshot https://review.openstack.org/440379
11:16:32 openstackgerrit jichenjc proposed openstack/nova master: check hw:mem_page_size when change extra_spec https://review.openstack.org/408610
11:18:17 bauzas johnthetubaguy: oh, just saw https://review.openstack.org/#/c/442387/1/resolutions/20170307-compute-platform.rst
11:18:42 bauzas johnthetubaguy: I haven't seen any ML discussion around this
11:18:57 bauzas johnthetubaguy: is this something you discussed in ATL ?
11:19:09 johnthetubaguy bauzas: I haven't started any, its something I have been wanting to suggest for a while
11:19:32 bauzas johnthetubaguy: oh okay
11:19:44 bauzas fine by me, just wanted to make sure people were happy with it
11:20:03 johnthetubaguy bauzas: I suspect people hate it, but I think its needed
11:20:45 cdent johnthetubaguy: it's gonna rub some people funny, but I agree it is important
11:21:51 johnthetubaguy I think we keep avoiding difficult conversations for two long on this kind of thing
11:22:04 bauzas ++
11:22:13 bauzas anyway, /me goes offline
11:22:26 cdent johnthetubaguy: hell yes
11:23:13 johnthetubaguy cdent: I like your idea, frame the problem, not the people, I am going to go do that now
11:24:04 cdent that's mostly for conflict avoidance. I think everyone is aware there is a problem...
11:24:37 johnthetubaguy yeha
11:32:51 dakhmetov johnthetubaguy: mdbooth: guys, thank you for the advice! I'll add unit test for missing keys in system metadata.
11:35:46 VAhl Openstack [Newton]: When deploying new instance on Nova. I got the following error. Running privsep helper: ['sudo', 'nova-rootwrap', '/etc/nova/rootwrap.conf', 'privsep-helper', '--config-file', '/etc/nova/nova.conf', '--config-file', '/etc/nova/nova-compute.conf', '--privsep_context', 'os_brick.privileged.default', '--privsep_sock_path', '/tmp/tmpcWGEib/privsep.sock']
11:35:46 VAhl etc/nova/rootwrap.conf is own by root:root. etc/nova/nova.conf + nova-compute.conf is own by nova:nov
11:43:35 sfinucan gmann: Comments left on https://review.openstack.org/#/c/431038/
11:44:09 sfinucan alex_xu: If you're looking for easy config-opt patches to review, you might look this way --> https://review.openstack.org/#/c/431039/ :)
11:45:31 alex_xu sfinucan: thanks :)
12:23:06 artom sfinucan, thanks for the tagged attach reviews :) I'll get to websocket proxy security, I promise. In my defense, it's harder to get full context for the latter than the former, and I don't feel like just reviewing patch by patch and writing nits
12:23:59 artom I had an idea about removing the tag=None param to the driver interface, let me respin that first
12:25:21 artom Building device metadata is probably not a huge deal, optimization-wise, but the DB call hidden in instance.save definitely is. So I'm thinking build the new device metadata, compare it to the old one, and only save if there's a difference
12:25:33 artom Get rid of the driver interface change, still no useless DB queries
12:26:29 artom Wait...
12:26:40 artom Doesn't save() call obj_what_changed() or whatever it's called?
12:26:54 artom Does it still call the database if nothing has changed?
12:28:45 sfinucan artom: Yeah, no (immediate) panic :) the websocket proxy stuff /is/ tougher to review than other stuff, seeing as it's not just glue like other nova code
12:29:09 sfinucan I'd be interested in seeing the approach to removing the tag parameter though, if it works out
12:29:19 sfinucan Steadily working through the rest of the series now
12:29:40 sfinucan sahid: RE: https://review.openstack.org/#/c/400875/, this is the issue I have with it https://review.openstack.org/#/c/400875/23/nova/virt/hardware.py@848
12:29:44 artom Based on ^^^ I think we can just remove with no other changes/repercussions?
12:29:58 sfinucan For N calls of _numa_fit_instance_cell_with_pinning, you will only ever have one where num_cpu_reserved is anything but 0
12:30:04 sfinucan That, to me, is overloading
12:30:18 artom dansmith can confirm when he wakes up, but if instance.save() is smart about how it calls the database, which I think it is, we're good
12:30:31 sfinucan artom: looking
12:32:35 openstackgerrit Sergey Nikitin proposed openstack/nova master: [WIP] PIC NUMA policy PoC https://review.openstack.org/390520
12:35:33 artom https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L721 https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L663
12:36:19 artom ^^ that pretty conclusively indicates that if device_metadata has not changed, there will be no useless DB calls
12:42:42 sfinucan artom: That would be reading of it to
12:42:44 sfinucan Good spot
12:42:47 sfinucan So
12:43:30 sfinucan We can just call it every single time with negligible performance impact
12:46:04 artom sfinucan, pretty sure
12:46:51 artom sfinucan, the little ones are waking up, let me drive them to daycare and I'll continue after
12:47:02 sfinucan artom: (y)
12:56:11 erol hi everyone, I wonder Is there any way to flush Nova Schedular RetryFilter count? I hit max attempt. What can i do now?
12:56:16 sean-k-mooney sc68cal: if you get a chance can you review https://review.openstack.org/#/c/444788 it will fix our third party ci after https://review.openstack.org/#/c/435575/ added a test requirement that break ovs compilation
12:57:01 sean-k-mooney sc68cal: it should also give a speed up in the ci jobs as less stuff needs to be downloaded which is a nice side effect

Earlier   Later