Earlier  
Posted Nick Remark
#openstack-nova - 2017-11-29
15:57:02 bauzas jaypipes: basically, I imagine a case where I create the object, save the object in DB, modify a field, deletes the record in DB, and then recreate it in the DB
15:57:19 jaypipes bauzas: I don't imagine that case :)
15:57:27 bauzas when I recreate in DB, the updates would only be that field
15:57:35 bauzas so we would only persist that field
15:57:59 bauzas again, it's unrelated to your change, so not a big deal for now
15:58:04 openstackgerrit Eric Berglund proposed openstack/nova master: WIP: PowerVM Driver: SEA https://review.openstack.org/523216
16:00:20 mriedem dansmith: on that cve errata, i'm thinking we maybe don't need to hold things up for https://review.openstack.org/#/c/521391/ which was the lesser regression; w/o that fix, we'll always run through the scheduler for rebuild of a volume-backed instance even if the image isn't changing, but with your change for RUN_ON_REBUILD, we shouldn't actually fail in the scheduler if the image is the same (unless the admin does something
16:00:20 mriedem onfig like changes isolated_images), so it's just an efficiency thing
16:00:31 mriedem still needs to be fixed, but i'm not sure that tristanC should hold up on the errata for it
16:01:07 dansmith mriedem: makes sense
16:01:57 mriedem since we're going to release your RUN_ON_REBUILD fix for newton before eol, we should still probably try to get that fixed and backported as well, since the regression was backported to newton also
16:04:06 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove vestigial extra_info update in PciDevice.save() https://review.openstack.org/523919
16:04:08 dansmith mriedem: why aren't you using is_volume_backed from compute utils?
16:04:21 mriedem dansmith: because we already have the stuff in scope for doing the same thing
16:04:26 mriedem the root_bdm
16:05:02 dansmith mriedem: yeah, just seems wrong to not use the util in case there's another detail we add later
16:05:08 dansmith but whatever
16:07:06 openstackgerrit Ed Leafe proposed openstack/nova master: placement: update client to set parent provider https://review.openstack.org/385693
16:07:48 openstackgerrit Ed Leafe proposed openstack/nova master: placement: adds REST API for nested providers https://review.openstack.org/384807
16:08:16 edleafe jaypipes: efried: ^^ Just fixed commit messages
16:08:29 jaypipes efried: ^^
16:08:36 jaypipes oops, sorry
16:08:38 efried ack
16:08:45 efried jaypipes ack
16:15:11 mriedem oomichi_afk: i've got some issues with the functional tests in https://review.openstack.org/#/c/408964/ which can be cleaned up separately, but +2 on that one if you want to review it again
16:20:32 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Object changes to support last-modified headers https://review.openstack.org/521639
16:20:32 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Add cache headers to placement api requests https://review.openstack.org/521640
16:20:38 cdent ^ rebase wars
16:24:47 openstackgerrit Matt Riedemann proposed openstack/nova master: Enable cold migration with target host(2/2) https://review.openstack.org/408964
16:53:02 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Enable limiting GET /allocation_candidates https://review.openstack.org/513526
16:56:52 cdent sigh, at microversin wars with myself
17:01:24 openstackgerrit Eric Fried proposed openstack/nova master: Make _Provider really private https://review.openstack.org/523932
17:01:29 efried jaypipes ^
17:01:52 efried jaypipes I didn't try to anticipate getters of _Provider attributes; we can add those as demand arises.
17:21:29 openstackgerrit Eric Fried proposed openstack/nova master: Make _Provider really private https://review.openstack.org/523932
17:21:29 openstackgerrit Eric Fried proposed openstack/nova master: ProviderTree.uuid_set() https://review.openstack.org/520243
17:21:55 efried jaypipes And there's uuid_set peeled out and modified as you suggested ^
17:22:02 efried (plus a silly UT fix on the base)
17:22:18 artom_ mriedem, is the topic set right for this series? https://review.openstack.org/#/c/521200/
17:22:46 artom_ I feel like bug/1732947 is the most obvious one
17:23:09 artom_ But I'd like to have related/1664931 somewhere, to keep track of all the changes that are happening because of that initial CVE fix
17:23:27 artom_ Hell, even create a (really weird) blueprint
17:23:47 mriedem artom_: the topic is set to the change on top
17:23:53 mriedem which is a different bug
17:24:21 mriedem track it in bug 1664931?
17:24:22 openstack bug 1664931 in nova (Ubuntu) "[OSSA-2017-005] nova rebuild ignores all image properties and scheduler filters (CVE-2017-16239)" [Undecided,New] https://launchpad.net/bugs/1664931
17:24:37 artom_ mriedem, I thought they were related?
17:24:59 mriedem bug 1732947 was a regression introduced by the fix for bug 1664931
17:25:01 openstack bug 1732947 in OpenStack Compute (nova) "volume-backed instance rebuild with no image change is still going through scheduler" [High,In progress] https://launchpad.net/bugs/1732947 - Assigned to Matt Riedemann (mriedem)
17:25:39 artom_ mriedem, right, that's what I meant
17:25:56 artom_ Ah, so only the first 2 changes in that series are fixes for 1664931 regressions?
17:26:55 mriedem melwitt: dansmith: are you aware of any functional test examples where we have multiple cells and a compute in each cell, and where fake.set_nodes works? i'm hitting something weird when trying to do a cold migration across 2 cells to assert it fails and hitting some weird stuff when i use fake.set_nodes
17:27:07 mriedem artom_: yes, the last change is a different bug
17:27:38 dansmith mriedem: no, but let me say it'd be nice if we could fix up that weird fake node behavior
17:27:38 artom_ mriedem, aha, thanks :)
17:27:38 mriedem could be related to the cells db fixture stuff being wonky
17:27:44 dansmith we basically work around it in all the tests that need that
17:27:56 mriedem artom: create an etherpad?
17:28:24 artom mriedem, yeah, not a bad idea
17:28:44 artom It's still a lot of clicking and checking branches, change IDs and topics
17:28:59 artom Since some (all?) of those got backported
17:29:35 melwitt mriedem: it might be related to the wonkiness I intended to fix with https://review.openstack.org/#/c/508432
17:29:53 melwitt because without that fix, every compute will write to the same cell db
17:30:14 mriedem i was going to see if putting your patch under this change fixes it
17:31:12 melwitt yeah, I think it would be worth trying but I'm not 100% it will solve your problem. what I'm thinking of is where compute services need to write records and they don't do it in a cell-aware way
17:31:53 dansmith yeah, any periodic will do the wrong thing I think
17:31:59 melwitt right
17:32:07 melwitt and during compute start
17:32:18 dansmith I think we need a more fundamental change before we can really be clean there
17:32:27 melwitt but I dunno what problem mriedem is hitting
17:32:39 mriedem i'll push it up in a sec
17:33:33 openstackgerrit Matt Riedemann proposed openstack/nova master: Enable cold migration with target host(2/2) https://review.openstack.org/408964
17:33:57 mriedem melwitt: this https://review.openstack.org/#/c/408964/111/nova/tests/functional/test_servers.py@3029
17:34:09 mriedem so takashi has this test_migrate_server_to_host_in_different_cell test,
17:34:16 mriedem which creates 1 host in different cells,
17:34:34 mriedem creates a server on one of the hosts and tries to force the migration to the other host in the other cell, which should fail
17:34:38 mriedem b/c no cross-cell migration
17:35:02 melwitt yeah, the compute node records won't go in the right db without my patch I think
17:35:04 mriedem it does fail with NoValidHost, but i wanted to test how good the test was, so i pulled it down and changed is so that both hosts would be in the same cell, and the test passed
17:35:12 melwitt they'll all go in the same cell, the default cell1
17:35:18 mriedem ok
17:35:40 mriedem so i found that if there is more than one host in a cell, like ServerTestV256RescheduleTestCase, then fake.set_nodes has to be used
17:35:55 mriedem but when i tried to use fake.set_nodes on hosts in different cells, things would fail
17:36:06 dansmith if the periodic runs to update the nodes, it'll put them back in the wrong cell though right?
17:36:18 dansmith er, create duplicates in the wrong cell I mean
17:36:26 melwitt I have seen that before doing single cell functional testing with multiple computes, that set_nodes is needed to have the scheduler consider both computes
17:36:55 mriedem all of our server moving tests in nova.tests.functional.test_servers create 2 computes in the same cell and have to use fake.set_nodes
17:37:03 mriedem but we don't have any multi-cell functional tests like that
17:37:25 mriedem this is really just a negative test, but i wanted to make sure it was actually failing for the right reasons
17:37:29 melwitt mriedem: it's not the set_nodes thing but the ComputeNode record itself that you'd want to target to separate cells and set_nodes won't do that. set_nodes doesn't write the compute node record
17:37:38 mriedem since you can't actually tell what the novalidhost was for on the api caller side
17:37:51 melwitt the compute node record is written by the fake compute service starting up
17:37:57 mriedem i'll pull your change down and put this on top and use fake.set_nodes and see what happens
17:38:03 melwitt okay
17:39:50 dansmith melwitt: won't your patch result in compute node records written into cell0 (which should never happen) if the periodic runs?
17:40:32 dansmith the resource tracker update periodic that creates and destroys compute nodes on the fly for things like ironic
17:40:49 melwitt dansmith: I think I make the db association when the fake service is created and wrapped all the service calls so that the periodics would run in the right cell. but let me look again, it's been a long time
17:40:49 dansmith it'll find that virt reports a compute node that it doesn't find in the database and create it
17:41:19 dansmith hmm, not sure how you could do that for periodics that run later
17:42:27 mriedem still fails

Earlier   Later