Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-11
14:41:27 mriedem which reminds me, you can only see faults for ERROR or DELETED servers, so that's another reason to set the server to ERROR in this case so they can see the fault
14:41:56 mriedem so as long as the NoValidHost has a reasonable message we should be fine
14:42:17 mriedem that would be a good wrinkle for the functional test though right?
14:43:06 openstackgerrit do3meli proposed openstack/nova master: docs: add nova host-evacuate command to evacuate documentation https://review.openstack.org/578040
14:44:08 mriedem why is all this shared provider stuff in this change? https://review.openstack.org/#/c/569498/12/nova/tests/functional/libvirt/test_shared_resource_provider.py
14:44:49 efried mriedem: I asked for it.
14:45:14 efried mriedem: Because it's important that we're checking all and only providers involved in the allocation.
14:45:39 efried mriedem: So we should also be checking nested, but letting that slide for now because we don't have any scenarios that actually use nested yet.
14:46:33 mriedem hmm, ok.
14:46:41 mriedem and those unit tests look to be completely redundant for conductor
14:46:47 mriedem redundant with the functional tests
14:46:51 mriedem we don't need both for the same scenarios
14:47:17 mriedem https://review.openstack.org/#/c/569498/12/nova/tests/functional/libvirt/test_shared_resource_provider.py is just a really weird location for rebuild tests...
14:48:28 efried if it's testing how shared resource providers are handled by rebuild by libvirt, it seems as good as anything.
14:49:34 mriedem the validation being added to conductor is virt-agnostic
14:49:42 mriedem so it's a weird place to have those tests
14:49:43 mriedem is all
14:50:17 efried but libvirt is the only driver that's doing shared providers (properly) at the moment.
14:50:31 mriedem yeah i know, but that doesn't really matter here does it?
14:50:39 mriedem it's just a convenient place to put the tests given the setUp
14:51:01 mriedem we could have just as easily written a functional test that populate providers and allocations in placement and then run rebuild
14:51:07 mriedem anyway, i'm not -1 on it, it's just weird
14:51:23 efried Agree the main motivator was probably convenience as you say.
14:57:49 mriedem i'd like to nuke all of these checks http://codesearch.openstack.org/?q=We%20need%20to%20mock%20that%20the%20old%20way&i=nope&files=&repos=
14:57:52 mriedem seems we can do that now right?
14:58:16 mriedem oh nvm, we haven't dropped the migrate_instances_add_request_spec online data migration yet
14:59:17 mriedem dansmith: it's probably not very possible to do a blocker migration for the request spec migration is it?
14:59:31 mriedem since it's api db + multi-cell
14:59:46 mriedem we could add a nova-status check pretty easily
14:59:59 mriedem iterate all cells and make sure all non-deleted instances have a request spec
15:00:00 dansmith for which migration?
15:00:04 dansmith ah
15:00:05 dansmith yeah
15:00:07 mriedem migrate_instances_add_request_spec
15:00:21 mriedem yeah - i want to kill all of these "if not request_spec: populate from garbage" in the code
15:00:34 dansmith hah yeah
15:00:42 mriedem we can't do a blocker db migration in db sync
15:00:54 mriedem well, shouldn't
15:00:55 mriedem ?
15:01:08 mriedem but nova-status could do the job, i mean this migration has been around since newton
15:01:13 dansmith I think people have also expressed displeasure with the blocker migrations, so any time we're not enforcing some really structural thing (like a column being non-null or something) a nova-status would be better
15:01:15 mriedem so nova-status in rocky and drop in stein
15:01:34 mriedem drop compat in stein i mean
15:01:47 dansmith aye
15:02:23 openstackgerrit Chris Dent proposed openstack/nova master: Add placement.concurrent_udpate to generation pre-checks https://review.openstack.org/581771
15:20:20 tssurya mriedem: I was trying to understand the marker logic you have in _heal_instances_in_cell (want to do something similar for the migration tool for updating queued_for_delete), wanted to confirm something: every time the command is run it will loop through all the instances which might have been processed before as well right ? like there is no way to check and skip the ones we have already covered. All we can ensure is each time it does m
15:21:50 openstackgerrit Matt Riedemann proposed openstack/nova-specs master: Fix nits in the handling down cell spec https://review.openstack.org/581243
15:22:34 mriedem tssurya: heal_allocations doesn't use a global marker like map_instances
15:22:46 mriedem so yes today it will hit all instances,
15:22:50 tssurya mriedem: hmm yea
15:23:07 mriedem it's only using a per-cell marker while iterating instances in the cell when we have a limit specified (default of 50)
15:23:09 tssurya because I have two online migration tools which are like this needing a marker logic
15:23:24 mriedem migrate_instances_add_request_spec uses a global marker
15:24:10 mriedem so i'd follow that pattern
15:25:19 mriedem map_instances is slightly different
15:25:33 mriedem it uses a sentinel project_id and munges the last real instance mapping uuid
15:26:00 tssurya yea that I am aware , but I don't want to insert a new row and all
15:26:35 mriedem you're going to have to either way
15:27:07 mriedem otherwise how would you find the marker record between runs of the command?
15:27:39 tssurya ah just looked at migrate_instances_add_request_spec; it is using the same logic
15:27:43 tssurya with a FAKE_UUID
15:27:53 mriedem yes
15:28:00 tssurya what if more than 1 tool has this FAKE_UUID ?
15:28:17 mriedem i don't care about out of tree tools
15:28:47 mriedem the alternative is you do like map_instances and use a fake project_id and munge the uuid from an existing mapping
15:29:04 mriedem but some people hate that too
15:29:08 mriedem since it's not a real uuid
15:29:42 tssurya uh-huh yea, well I guess it might be better than starting a series of FAKE_UUIDs
15:30:26 mriedem the sentinel uuid is always the same 00000000-0000-0000-0000-000000000000
15:30:47 mriedem we don't have anything else in nova, as far as i know, that inserts instance mappings with that uuid
15:31:30 tssurya oh okay
15:31:35 mriedem between migrate_instances_add_request_spec and map_instances i'd just personally follow what was done in migrate_instances_add_request_spec
15:31:47 mriedem it's nearly the same thing you need
15:32:09 mriedem minus creating a request spec and all that - so your thing will be much lighter
15:32:48 mriedem one thing about migrate_instances_add_request_spec is that it's not multi-cell aware
15:33:06 mriedem but i think we said in vancouver that running db sync and online data migrations per cell is not a big deal
15:33:19 tssurya mriedem: I was just getting to that actually
15:33:33 tssurya thinking if my marker should be in instance_mappings or instances
15:34:00 mriedem L39 https://etherpad.openstack.org/p/YVR18-cellsv2-migration-sync-with-operators
15:34:17 tssurya as I will be fetching deleted (or soft_deleted) instances and then updating only those instance_mappings to true
15:34:36 mriedem migrate_instances_add_request_spec puts the marker in the request_specs table
15:34:41 mriedem so again, i'd just follow that pattern :)
15:34:53 mriedem since i will assume people thought about all of this when migrate_instances_add_request_spec was written
15:35:27 tssurya mriedem: okay then! I will go for the same consistency and follow the migrate_instances_add_request_spec stuff
15:37:32 tssurya dansmith: are we having cells meeting today ? (I don't have anything new we haven't discussed and have the needed feedback for the spec to work on)
15:38:03 dansmith tssurya: I don't have anything either, mriedem, melwitt ?
15:43:13 sapd1 Hi everyone, How can I remove an object from BlockDeviceMappingList object.
15:45:31 mriedem dansmith: nothing pressing, just starting reviews on tssurya's changes and need reviews on this bug fix which is related to cells v2 (build requests and quota counting): https://review.openstack.org/#/q/topic:bug/1780373+(status:open+OR+status:merged)
15:46:09 dansmith ack\
15:46:38 mriedem sapd1: can you provide more context?
15:47:05 openstackgerrit do3meli proposed openstack/nova master: docs: add nova host-evacuate command to evacuate documentation https://review.openstack.org/578040
15:48:21 sapd1 mriedem: I would like to customize bdms list. In this function: https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2752
15:50:41 mriedem sapd1: because you're trying to rebuild a volume-backed instance?
15:51:23 mriedem sapd1: if so, you should read this spec first https://review.openstack.org/#/c/532407/
15:52:47 sapd1 mriedem: I'm trying, I don't find any change which can work, So I'm trying to make it run.
15:54:15 mriedem well read the spec and discussion in there
15:54:28 mriedem or if you're going to fork it anyway, you could look at https://review.openstack.org/#/c/528740/ but i wouldn't use that method
15:55:26 mriedem there are in fact several attempts at this in https://bugs.launchpad.net/nova/+bug/1378689
15:55:27 openstack Launchpad bug 1378689 in OpenStack Compute (nova) "error when rebuilding a instance booted from volume" [Undecided,In progress] - Assigned to Jie Li (ramboman)
16:02:32 sapd1 mriedem: This feature is sound like have too many bugs :D

Earlier   Later