Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-20
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 cdent mriedem: then we get into strange issues with ordering of middleware
15:16:32 sdague mriedem: we can now sometimes, that's really new though
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 so, does this seem weird to anyone else?
15:22:51 mriedem if inv_data == curr.get('inventories', {}):
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 sneti alex_xu, can you please review this patch when you have time: https://review.openstack.org/#/c/396420/
15:43:34 efried Is that a bug in the mock lib?
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
15:50:39 EmilienM mriedem: ovb-nonha or ovb-ha
15:50:59 mriedem i see the other queue now
15:51:31 EmilienM mriedem: gate-tripleo-ci-centos-7-ovb-nonha or gate-tripleo-ci-centos-7-ovb-ha or gate-tripleo-ci-centos-7-ovb-updates
15:51:39 EmilienM they are the one who actually test Ironic / Nova
16:00:43 sfinucan mriedem: I have those sphinx issues all fixed in this series https://review.openstack.org/#/c/445906/
16:01:08 openstackgerrit ChangBo Guo(gcb) proposed openstack/nova master: Fix wrong unit test about config option enabled_apis https://review.openstack.org/446898
16:01:18 sfinucan Just fyi
16:01:24 mriedem ok cool
16:04:07 snecklifter Hello #openstack-nova
16:04:21 snecklifter Would it be possible to get a review on this: https://review.openstack.org/#/c/393489/
16:04:37 snecklifter CI is green except for a couple of non-voters
16:12:39 mriedem snecklifter: you're missing a version bump on the ImageMetaProps object
16:12:55 mriedem that test which checks the hash indicates that the version needs a bump

Earlier   Later