Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-03-16
15:06:57 mriedem kevinbenton: does neutron have a concept of provider rules for security groups that the tenant user can't change?
15:07:58 mriedem Jack_Iv: i think that's probably the thread you need to pull on though
15:08:14 mriedem listen for notifications from nova for the instance build lifecycle, and then adjust rules via the networking API as needed
15:09:41 mriedem no vmware people around huh
15:09:51 mriedem their CI results come in, if at all, about a week late
15:15:50 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: Move detach logic from manager into driver BDM https://review.openstack.org/439520
15:15:51 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: Only destroy BDMs after successful detach call https://review.openstack.org/440693
15:16:13 gibi dansmith: I'm +1 on https://review.openstack.org/#/c/446053
15:16:35 dansmith gibi: awesome, thanks for looking at that
15:16:40 dansmith gibi: on the other patch,
15:16:44 lyarwood mdbooth: ^ updated removing some of the nits, just to confirm the LM rollback issue isn't new right?
15:17:02 mdbooth lyarwood: No, the LM rollback issue looks new
15:17:13 dansmith gibi: the change I made prevents projects from showing up on actual flavor create/delete, so I'm fixing that by just forcing projects to be loaded in the api before doing the notification
15:17:23 mdbooth The problem is you've combined 2 functions in your move that were previously separate
15:17:31 dansmith gibi: just fyi.. i'll push that up a bit later, it's not a huge rush
15:17:32 mdbooth And remove_volume_connection only called one of them
15:18:10 mdbooth The problem is the interaction of remove_volume_connection and the code from _detach_volume, which we previously didn't call
15:18:38 gibi dansmith: that sounds OK to me. I will keep an eye on that review
15:19:17 dansmith gibi: cool, thanks a bunch :)
15:19:59 mdbooth Incidentally, I really don't like the proliferation of do_all_the_things(but_not_this_thing=True, dont_not_do_that_thing=Maybe, do_this_extra_thing='True')
15:23:13 mdbooth lyarwood: So, I feel like the old _driver_detach_volume is what needs to move, with perhaps a couple of volume_api calls thrown in. Ideally, though, the code in block_device won't concern itself with CONF.host. It'll be called, or it won't be called.
15:25:03 mdbooth lyarwood: That's not 100% thought through, though. There may be practical reasons for dont_do_these_things=['foo', 'bar', 7, None, {}]
15:28:16 lyarwood mdbooth: sorry just went over the diff again, I see your point but just to confirm, instance.host is going to point to the source during a rollback right?
15:28:28 mriedem there doesn't seem to be much they're not willing to do in their company
15:28:29 mdbooth Not during rollback_at_destination
15:28:31 mriedem mdbooth: ^
15:28:48 mdbooth mriedem: Hehe.
15:30:42 mdbooth lyarwood: Sorry, yes. instance.host *will* be the source. I didn't read you right there.
15:30:56 mdbooth However, it's executing on the destination, which is the problem.
15:31:11 mdbooth context, instance, bdm.volume_id, dest)
15:31:11 mdbooth self.compute_rpcapi.remove_volume_connection(
15:31:20 mdbooth In _rollback_live_migration
15:31:25 lyarwood mdbooth: haha right, I just couldn't find the instance.host update
15:31:43 mdbooth So CONF.host != instance.host
15:31:52 mdbooth BUT, we still need to tear it down
15:32:07 mdbooth So I feel like that's the sort of logic that lives in ComputeManager
15:32:12 lyarwood well, we are going to call destroy that's going to do that anyway
15:32:37 lyarwood but sure, I get your point
15:33:53 lyarwood mriedem / johnthetubaguy ; if you have anytime before the cinder meeting the uuid and attachment_id change could really use core review - https://review.openstack.org/#/q/topic:bp/cinder-new-attach-apis & https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bug/1489581
15:34:01 mriedem dansmith: sdague: melwitt: can we get https://review.openstack.org/#/c/444106/ and the test patch below it in so we can move forward with the backport? it would also help ironic ci that is failing on this.
15:34:02 bauzas dansmith: have you seen my comment on archive_deleted_rows ?
15:34:18 mriedem lyarwood: ok, i meant to go through those again today
15:34:23 dansmith bauzas: you said you didn't think you could use it, but didn't say why (or that I saw)
15:34:39 bauzas dansmith: because the logic in there is looping over the cell DB tables
15:34:46 dansmith bauzas: obviously some refactoring will be needed, but I just would like to avoid having to add yet another command to run cleanup
15:34:56 dansmith mriedem: yeah
15:34:57 bauzas to push them in their shadow one
15:35:23 mdbooth lyarwood: In case you didn't notice, I thought the problem in _from_db_object() here was pretty severe: https://review.openstack.org/#/c/242603/23/nova/objects/block_device.py
15:35:30 bauzas dansmith: the main point is that we provide the number of rows per table archived
15:36:01 dansmith bauzas: not per table, but that applies to the thing we need to do in reqspec too right?
15:36:12 bauzas dansmith: so I should say how many rows for the request_spec table are 'archived', ie. killed?
15:36:25 bauzas dansmith: nope, this is per table AFAICS
15:36:37 bauzas you return either 0 or 1
15:36:38 dansmith bauzas: in fact, you have to look at the cell dbs to know what you can remove from reqspec, right?
15:36:38 openstackgerrit Moshe Levi proposed openstack/os-vif master: HW offload support for openvswitch https://review.openstack.org/398277
15:36:39 sdague mriedem: ... once I finish updating the limits spec
15:36:46 openstackgerrit Matt Riedemann proposed openstack/nova master: libvirt: add debug logging in detach_device_with_retry https://review.openstack.org/446601
15:36:47 mriedem lyarwood: ^ is the debug patch for detach
15:37:00 bauzas but the verbose prints that
15:37:11 dansmith bauzas: are you talking about the parameter or what gets printed?
15:37:16 bauzas dansmith: what's printed
15:37:24 lyarwood mdbooth: yeah sorry I did miss that, are you sure that doesn't result in a save straight away?
15:37:38 bauzas dansmith: for looking at which record to remove, I was rather thinking on looking at InstanceMapping
15:37:39 mdbooth lyarwood: If it does, I don't know how
15:37:46 bauzas oh wait
15:37:54 dansmith bauzas: you need to purge those too
15:37:57 bauzas dansmith: we don't remove yet also InstanceMapping records?
15:38:02 bauzas a-ha, yeah
15:38:13 dansmith we don't in the case of soft delete, but otherwise we do
15:38:19 bauzas erf
15:38:30 bauzas okay, then I need to look at all the cells, yeah
15:38:50 bauzas I should also purging the instance_mappings too then
15:38:56 lyarwood mdbooth: ah I might be mixing this up with the driver_bdm dict updates that result in a save
15:39:04 bauzas well, okay, I'll see what I can do to use the same command
15:40:12 mdbooth lyarwood: Also, in the previous patch I didn't like the unnecessary argument type ambiguity. But I think I was ok with everything else.
15:44:04 lyarwood mdbooth: the uuid vs bdm_uuid issue?
15:44:12 mdbooth lyarwood: No, just a sec
15:44:40 lyarwood mdbooth: ah the cellv1 values thing
15:44:54 mdbooth https://review.openstack.org/#/c/242602/22/nova/db/sqlalchemy/api.py
15:45:03 mdbooth block_device_mapping_update
15:45:47 mdbooth The caller has full context, and there are multiple ways to pass it in unambiguously
15:46:32 lyarwood mdbooth: right and at the moment that's always the id so we can just drop this for now
15:47:05 mdbooth Well then you'd have to drop the change in update_or_create
15:47:52 mdbooth It shouldn't be a complex fix.
15:48:09 lyarwood ah yeah got it
15:48:30 dansmith bauzas: remind me, we don't delete reqspec at all right now, but will in the case of non-soft delete soon right?
15:48:47 dansmith bauzas: so this command will really only be needed if operators have soft delete enabled, right?
15:49:02 bauzas dansmith: yup, we don't delete the spec yety
15:49:45 bauzas dansmith: if someone hard-deletes an instance (ie. the instance now longer exists), then we should purge the entry
15:49:52 mriedem mdbooth: lyarwood: in case you haven't noticed yet, the original patches to add bdm.uuid were for a cellsv1 race issue where update_or_create happened at the api cell and things got racey and crazy,
15:50:05 mriedem mdbooth: lyarwood: so that's why there is the update_or_create stuff
15:50:08 dansmith bauzas: you're saying you will soon delete reqspec at normal instance delete time, yes?
15:50:09 bauzas dansmith: tbc, I was planning to iterate over all the spec records, and lookup over all cells to see if some instance exists
15:50:18 bauzas for the purge command
15:50:31 dansmith not talking about purge, I'm talking about normal api delete..
15:50:35 mriedem mdbooth: lyarwood: but i believe that update_or_create is also still called from the compute api (and/or conductor now) when creating the bdms
15:50:49 bauzas dansmith: ah, that, I wasn't planning to introduce it given mriedem's point
15:50:59 dansmith bauzas: his point about what?
15:51:16 bauzas dansmith: the fact that we shouldn't delete the spec record in case this is an instance soft-delete
15:51:19 mdbooth mriedem: IIRC that's correct. The cellsv1 caller was unique in that it passed in a versioned object instead of a list of update values, though.

Earlier   Later