Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-20
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
16:25:18 dansmith zzzeek: I tried server_default at jaypipes' suggestion anyway, and it causes other problems for us in the oslo.db migration checker
16:25:19 dansmith zzzeek: ah, definitely can't have that then
16:25:23 mriedem CERN has what, 10K+ compute nodes?
16:25:49 zzzeek dansmith: i'd agree it's not "slow" but it is def. a variable-length operation, i would think
16:26:10 zzzeek dansmith: problems in oslo.db should be fixed in oslo.db , feel free to post a launchpad
16:26:33 dansmith zzzeek: okay, what I meant was, jaypipes said it wouldn't require a rewrite
16:26:41 dansmith it really doesn't matter though,
16:26:51 dansmith we can handle this on read just as easily and not have to walk the table
16:26:55 jaypipes zzzeek: I don't think anyone's suggesting there's something wrong with oslo.db.
16:27:10 zzzeek dansmith: ok jaypipes knows mysql internals more than I do but in my experience, when you have server deafult in a migration w/ nullable=false, it has to write data to every row
16:27:26 zzzeek jaypipes: dansmith just referred to some problem with oslo.db's migration checker
16:27:31 jaypipes zzzeek: yes, that's correct.
16:27:31 mriedem snecklifter: yes
16:27:55 openstackgerrit Claudiu Belu proposed openstack/nova master: POC: tests: autospecs all the mock.patch usages https://review.openstack.org/447505
16:27:57 jaypipes zzzeek: ah, the migration checker thing... sorry, yeah
16:29:04 claudiub efried: mriedem: ^ in case you're curious about the autospec stuff. :)
16:29:57 claudiub still experimental, and it's just the 1st step.
16:31:03 mriedem dansmith: why even set mapped=0 in fake compute node records in https://review.openstack.org/#/c/427901/ ?
16:32:00 EmilienM mriedem: I have updates
16:32:15 EmilienM 1. the patch that dansmith sent to have more logging, is still running so I'll give update later on logs
16:32:23 dansmith mriedem: because we create from that and we don't have it optional on create
16:32:32 EmilienM 2. the patch that revert jaypipes's patch is fixing our problem, I see it in the logs
16:32:35 dansmith mriedem: go run without that and I think you'll see it tries to lazy-load on create
16:33:02 EmilienM so I confirm the issue is related to this patch: https://review.openstack.org/#/c/441544/
16:33:09 openstackgerrit Christopher Brown proposed openstack/nova master: Add lan9118 as valid nic for hw_vif_model property for qemu https://review.openstack.org/393489
16:33:40 mriedem dansmith: create of the object or the db record?
16:33:41 EmilienM [overcloud.AllNodesDeploySteps.CephStorageDeployment_Step3]: CREATE_COMPLETE state changed ---- It means that overcloud nodes were deployed successfuly (and now deployment is moving forward)
16:34:12 dansmith mriedem: ComputeNode.create()
16:34:47 dansmith mriedem: even still, we're adding something as nullable purely for speed, we don't want to treat it as optional going forward
16:35:43 mriedem i don't see how create() will fail when _from_db_object has a check for null: value = 0 if value is None else value

Earlier   Later