Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-11-29
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
17:42:46 melwitt dansmith: I think periodics are run by the service, so I did this https://review.openstack.org/#/c/508432/9/nova/test.py to target them to the associated cell
17:42:54 dansmith oh you patch periodic_tasks
17:43:13 dansmith not sure that's enough necessarily,
17:43:27 dansmith because anything that stores a context somewhere might not actually make that call in that path
17:43:49 melwitt it's better than what we have :P but yeah, I have been wanting people to review it to see what gaps could be addressed
17:44:11 dansmith well, it's not if we end up with spurious failures when we get unlucky and run a periodic in the middle of a slow test run
17:44:46 mriedem i left comments on the fixture patch
17:44:49 melwitt dansmith: what do you mean by "anything that stores a context somewhere"? the periodics in the compute service all use an anonymous admin context
17:45:16 dansmith melwitt: well, if something like RT has a context stored somewhere
17:46:07 melwitt dansmith: sure. I couldn't think of if/how it could be bullet-proofed
17:46:29 dansmith I think the thing we really should be doing is just make the compute manager grab a context on service start,
17:46:48 dansmith and wipe through compute/* and make sure nothing ever calls get_admin_context() or context.RequestContext() in there
17:46:53 dansmith sort of how we banned db access
17:47:18 melwitt yeah, that would be nicer
17:47:24 dansmith I think that also,
17:47:36 melwitt still, that doesn't stop anyone from doing something else, which is what you were pointing out
17:47:44 dansmith someone with "security" in their title would like to see that we can't just call foo.get_admin_credentials() anywhere
17:48:02 mriedem i think mikal is now head of security at aptira
17:48:13 mriedem securinaut or something
17:48:13 dansmith melwitt: well, it does if we actually clean that up to the point of making sure it won't work, like the db stuff
17:48:56 melwitt one thing I realized just now is, a context is handed off when the periodic tasks are kicked off once, and it reuses that. it's just that in the service code it uses get_admin_context for that
17:50:00 melwitt periodic_tasks is called once during service.start
17:50:02 openstackgerrit Matt Riedemann proposed openstack/nova master: Fix ValueError when loading old pci device record https://review.openstack.org/523914
17:50:02 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove vestigial extra_info update in PciDevice.save() https://review.openstack.org/523919
17:50:37 dansmith yeah, but your wrapping of it ensures that anyway
17:51:07 melwitt but I thought you were saying "what if compute manager uses a different context" and it can't
17:51:08 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Enable limiting GET /allocation_candidates https://review.openstack.org/513526
17:51:10 melwitt https://github.com/openstack/nova/blob/master/nova/service.py#L294-L295
17:51:35 dansmith melwitt: I'm saying if we have another context we got from not the periodic one we were handed, not in the scope of the periodic itself
17:51:44 dansmith we have contexts everywhere, on every object, etc
17:51:56 dansmith I thought we actually stored on in the RT too but I don't think we do
17:52:07 melwitt okay, yeah I thought you were talking only about periodics
17:52:25 dansmith ah in the claim we do, that's probably what I'm thinking of
17:52:43 dansmith melwitt: I mean a periodic that accesses one created outside the scope of the periodic, like claim for example
17:52:54 melwitt oh, I see
17:52:57 melwitt yeah
17:57:52 melwitt mriedem: I think you might have to not do your own _setup_compute_service
17:58:38 cdent jaypipes, efried_rollin : is n-r-p stack clean and reviewable? Last I checked last night might rebase was in progress.
18:07:20 openstackgerrit melanie witt proposed openstack/nova master: qemu-img do not use cache=none if no O_DIRECT support https://review.openstack.org/523554
18:12:15 dansmith stephenfin: https://review.openstack.org/#/c/523914
18:13:45 melwitt mriedem: I pulled down your patch to see if I can find anything
18:15:53 mriedem dansmith: that root_bdm thing with using compute_utils.is_volume_backed opens up some funny doors
18:16:06 mriedem probably nothing we'd ever hit in real life
18:16:06 dansmith mriedem: yeah?
18:16:24 mriedem is_volume_backed_instance returns True if there is no root bdm but the image_ref is ''
18:16:36 mriedem so "volume_id = root_bdm.volume_id" could result in a NoneType
18:17:05 mriedem nova.tests.unit.compute.test_compute.ComputeAPITestCase.test_rebuild_no_image apparently tickles that path
18:17:58 mriedem in that case i guess i just log an error or something, not sure
18:18:02 mriedem shouldn't really happen
18:29:21 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Refactor encryptor attach and detach calls https://review.openstack.org/460243
18:29:21 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Introduce disk encryption config classes https://review.openstack.org/464008
18:29:22 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP libvirt: Use QEMU's native LUKS support https://review.openstack.org/523958
18:37:53 openstackgerrit Merged openstack/nova master: Updated from global requirements https://review.openstack.org/523765
18:38:58 INeedaUUID Hi everyone. Can anyone tell me how best to modify a systems' uuid using openstack methods?
18:39:03 INeedaUUID Does openstack support this? (I know it's a stupid request, but an application depends on them being set)
18:39:11 INeedaUUID Ocata/KVM
18:39:47 jaypipes cdent: efried_rollin is currently rebasing./
18:40:12 jaypipes cdent: ooh, hold up, he may have pushed a fresh series.
18:45:54 mriedem gdi, that moment when you rebase a series and forget git add / git rebase --continue on the last change, and did git commit -a --amend instead, thus squashing the last change and ruining everything

Earlier   Later