Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-20
15:13:16 cdent if there's something we can borrow from nova glance interactions, great
15:13:23 jroll EmilienM: nice, I'll go find some typo fixes
15:13:34 mriedem i don't see anything between nova and glance
15:13:45 sdague mriedem: it does keep coming up, there is no generic solution
15:13:56 dansmith okay I thought that was sorted already
15:14:13 sdague there was going to be some logging of both at some point, but it remains unsolved
15:14:13 cdent is it safe for service to accent an incoming request id?
15:14:20 cdent wow, my typing
15:14:21 cdent sorry
15:14:23 cdent accept!
15:14:24 sdague cdent: that is the sticking point
15:14:31 sdague some people get really concerned about that
15:14:51 sdague it would all be super easy if we could get past that
15:14:55 mriedem what's the concern? the request id is read-only isn't it?
15:15:10 sdague mriedem: the concern is accepting request id from the user
15:15:12 jroll so weird to me, today it's always wrong, if we accept one then worst case it's wrong
15:15:18 dansmith mriedem: attackers confusing the ops' logs I imagine
15:15:37 jroll reject anything that isn't 'req-' + uuid_regex
15:15:44 dansmith accepting it but only using it if config says to seems reasonable
15:15:54 cdent jroll: then send all the same uuid to fudge up the logs
15:15:55 dansmith for ops that prefer correlation with some risk
15:16:06 mriedem can we tell if it's a service user request rather than a user user?
15:16:17 sdague yeh, so honestly the service user / service token bits might make this better
15:16:18 mriedem and if so, use the provided request id
15:16:32 sdague mriedem: we can now sometimes, that's really new though
15:16:32 cdent mriedem: then we get into strange issues with ordering of middleware
15:16:33 mriedem right, we're already doing the re-auth thing
15:16:44 jroll cdent: I guess, if I was an operator I'd just ignore support requests and bugs for that request id :P
15:17:11 sdague mriedem: I think there is probably a path forward that wouldn't freak everyone out based on service token
15:17:13 mriedem anywho, total tangent
15:17:15 sdague yep
15:17:45 mriedem sounds like something for lbragstad and the vm+baremetal+totally+not+exclusive+club+platform team
15:17:56 cdent i've always assumed we want the thing which logs begin and end of request to the first and last pieces of middleware that get touched, thus before any token analysis
15:18:48 mriedem so i'm trying to go back to what could possibly change with https://review.openstack.org/#/c/441544/
15:19:03 mriedem and how self.scheduler_client.update_compute_node(compute_node) works if get_inventory isn't there
15:19:18 dansmith mriedem: yeah, I don't know either, but if we're getting back zero from placement, I'm not sure what else to blame
15:19:20 mriedem if the node is disabled, they both do the same thing, they return {} inventory which tells the report client to delete inventory for that provider
15:19:28 dansmith mriedem: so the log result plus the revert test of that should be interesting
15:22:51 mriedem if inv_data == curr.get('inventories', {}):
15:22:51 mriedem so, does this seem weird to anyone else?
15:23:16 mriedem https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L495
15:23:52 dansmith if the thing we're going to report is equal to what we have, then don't report?
15:23:59 bauzas mriedem: that just means "heh look, we already have that"
15:24:07 dansmith and if what is being reported is nothing and nothing is already stored, then.. don't do anything either
15:24:18 bauzas dansmith: mriedem: FWIW, we already have that in place in the RT
15:24:31 mriedem is the dict compare always good in py27 even with random key order and whatnot?
15:24:33 bauzas old-world-ish of course
15:24:59 dansmith sure
15:25:18 lbragstad mriedem sdague just finished parsing the scroll back - kinda sounds similar to what jamielennox was doing with service users and validating expired tokens
15:25:19 mriedem bauzas: if you're referring to this https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L730 it's a bit different
15:25:22 jroll >>> {'a': 'b', 'c': 'd'} == {'c': 'd', 'a': 'b'}
15:25:24 jroll True
15:25:26 jroll mriedem: ^
15:25:27 bauzas mriedem: https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L730
15:25:33 bauzas yeah
15:26:17 openstackgerrit Sujitha proposed openstack/nova master: Add helper method to add additional data about policy rule. https://review.openstack.org/434842
15:26:22 mriedem ok nvm then, that just surprised me given how many times assertEqual on dicts has failed b/c of random hash seeds
15:27:17 jroll well, assertEqual isn't so smart iirc
15:27:22 siva_krishnan mriedem: can you remove -2 for this patch https://review.openstack.org/#/c/308198/ ?
15:27:57 dansmith I think it's more assertEqual where we've pulled things out of a dict in random order anyway, right?
15:29:26 mriedem siva_krishnan: the bp isn't approved
15:31:14 sdague dansmith: right, it's not actually that 2 dicts aren't equal, it's that items() happens in different orders
15:31:23 dansmith yeah
15:34:14 mriedem ok here are the calls for one resource provider, it's created here:
15:34:15 mriedem http://logs.openstack.org/periodic/periodic-tripleo-ci-centos-7-ovb-nonha/12d034e/logs/undercloud/var/log/nova/nova-compute.txt.gz#_2017-03-19_07_10_44_256
15:34:16 siva_krishnan mriedem: makes sense. it's kind of an POC, I thought reviewers can suggest any changes to spec or code if they feel so.
15:34:32 mriedem 4 minutes later we finally get the real resource view from ironic
15:34:40 mriedem http://logs.openstack.org/periodic/periodic-tripleo-ci-centos-7-ovb-nonha/12d034e/logs/undercloud/var/log/nova/nova-compute.txt.gz#_2017-03-19_07_14_42_794
15:35:04 mriedem and right after that, same second, we update inventory for that provider
15:35:05 mriedem http://logs.openstack.org/periodic/periodic-tripleo-ci-centos-7-ovb-nonha/12d034e/logs/undercloud/var/log/nova/nova-compute.txt.gz#_2017-03-19_07_14_42_928
15:36:48 mriedem we never update inventory for that provider after that, because it never changes
15:37:13 jaypipes mriedem: yep.
15:37:37 mriedem the old way, we would have called driver.get_available_resource and updated the compute node record with the values (if they changed)
15:37:46 openstackgerrit Sujitha proposed openstack/nova master: Add description to policies in admin_actions. https://review.openstack.org/440829
15:37:58 mriedem and the compute node would be passed to self.scheduler_client.update_compute_node(compute_node) to update the rp inventory
15:38:04 mriedem if it changed
15:38:47 mriedem new style we call get_inventory which in this case returns a dict with 3 keys, for vcpu/disk_gb/memory_mb
15:38:49 jaypipes mriedem: well, I moved compute_node.save() into the RT's _update() method.
15:39:37 jaypipes mriedem: so regardless of whether it's get_inventory() or update_compute_node() that is called, we still update the compute node DB record.
15:39:47 mriedem i know
15:39:53 jaypipes ok, sorry
15:39:55 mriedem the save() doesn't really matter
15:40:11 mriedem update_compute_node pulls the values off the ComputeNode object
15:40:17 jaypipes sorry, I thought you were suggesting that the compute_nodes table wasn't being updated.
15:40:31 mriedem which is updated in _init_compute_node
15:43:19 efried mriedem jroll Okay, here's an update. I _am_ using the mock library; and mock _does_ have those methods (assert_not_called, assert_called_once) UNLESS I use mock.patch AND the target is a function (as opposed to a class).
15:43:34 efried Is that a bug in the mock lib?
15:43:34 sneti alex_xu, can you please review this patch when you have time: https://review.openstack.org/#/c/396420/
15:43:57 mriedem efried: no idea, i'd ask lifeless
15:44:12 efried Here, or a different channel?
15:44:18 mriedem he's here
15:44:22 mriedem but new zealand
15:44:26 mriedem so not here
15:45:02 efried Ight. I guess I can open a bug, and it'll be bounced if it's invalid. Thanks.
15:45:18 mriedem efried: bug where? the mock lib/
15:45:19 mriedem ?
15:45:30 efried Yeah.
15:45:32 mriedem EmilienM: which jobs are we looking for in https://review.openstack.org/#/c/447522/ ?
15:45:40 mriedem because some passed and one failed
15:50:31 EmilienM mriedem: the ovb jobs

Earlier   Later