Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-20
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
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
16:13:29 openstackgerrit Gábor Antal proposed openstack/nova master: Add json style checking for sample notifications https://review.openstack.org/443677
16:20:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove unused VIFModel._get_legacy method https://review.openstack.org/447577
16:21:02 mriedem zzzeek: is there no way to add a column to an existing table and make it nullable=False by providing a default value?
16:21:11 mriedem zzzeek: i.e. https://review.openstack.org/#/c/427901/3/nova/db/sqlalchemy/migrate_repo/versions/358_add_compute_node_mapped.py
16:21:44 zzzeek mriedem: yes if you give it a server side default it will add. however, that's one of the "slow" migrations for a big table
16:22:40 zzzeek mriedem: the code example is incorrect, it needs to be server_default="0"
16:23:06 mriedem because "A FetchedValue instance, str, Unicode or text() construct representing the DDL DEFAULT value for the column."
16:23:08 mriedem ?
16:23:44 zzzeek "0" is a str
16:23:45 snecklifter mriedem: thanks
16:23:59 mriedem zzzeek: yeah so i'm not sure what code example is incorrect
16:24:18 zzzeek mriedem: this one: https://review.openstack.org/#/c/427901/3/nova/db/sqlalchemy/migrate_repo/versions/358_add_compute_node_mapped.py
16:24:29 zzzeek "server_default" vs. "default"
16:24:30 snecklifter mriedem: so change 1.16 to 1.17?
16:24:45 dansmith zzzeek: ah, jay said it wasn't slow
16:25:06 zzzeek dansmith: well if you have 1M rows it has to popualte 1M rows w/ a value and probably do some locking too. depends on your table

Earlier   Later