Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-11-29
15:56:04 mriedem setting self.uuid in save() does'nt get it into the updates dict wihch is passed to db.pci_device_update
15:56:14 bauzas jaypipes: I have to bail out, but I can explain later
15:56:23 jaypipes mriedem: sure it does...
15:56:24 mriedem so save() will call _from_db_object which will do the uuid migration and call save() again
15:56:28 mriedem jaypipes: how?
15:56:36 mriedem oh obj_get_changes
15:56:37 mriedem nvm
15:56:39 jaypipes mriedem: obj_get_changes() will pick up that uuid..
15:56:51 jaypipes bauzas: k
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 onfig like changes isolated_images), so it's just an efficiency thing
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: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] Add cache headers to placement api requests https://review.openstack.org/521640
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: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: ProviderTree.uuid_set() https://review.openstack.org/520243
17:21:29 openstackgerrit Eric Fried proposed openstack/nova master: Make _Provider really private https://review.openstack.org/523932
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 mriedem could be related to the cells db fixture stuff being wonky
17:27:38 artom_ mriedem, aha, thanks :)
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: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

Earlier   Later