Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-04
19:22:08 sean-k-mooney smcginnis: or at least openstack wide lower-constraint
19:22:11 smcginnis sean-k-mooney: No, there is a new lower-constraint.txt file.
19:22:25 smcginnis sean-k-mooney: Well, I guess you could call that the new g-r.
19:23:01 smcginnis It's slightly different though: https://github.com/openstack/requirements/blob/master/lower-constraints.txt
19:23:13 sean-k-mooney smcginnis: oh ok is that updated automatically some how? just not sure what the delta is between it and g-r
19:23:36 sean-k-mooney oh its expcitly === with no ranges
19:24:12 smcginnis Right. It's saying "the minimum required is exactly this" rather than "it needs to be above this version, but not this one, etc."
19:24:15 sean-k-mooney so in theroy we could use it to test with minium supported version
19:24:27 smcginnis sean-k-mooney: Yep, I think that's the plan.
19:25:21 smcginnis It's a bit of a long read, but Doug wrote out the whole plan here: http://lists.openstack.org/pipermail/openstack-dev/2018-March/128352.html
19:26:29 sean-k-mooney smcginnis: oh good to know. i basically assume that since we only tested with what upperconstraties allowed that anything lower might work but not worth the heart ache of finding out
19:27:09 smcginnis sean-k-mooney: Hah, yeah. And I think in a lot of cases, our lower bound didn't/doesn't accurately reflect what really is the minimum required.
19:27:18 smcginnis This makes it plausible to have a test that can verify that.
19:28:16 sean-k-mooney smcginnis: well the distros. esspcially centos et al would be happy with knowing that a minium is actully checked before they start intergrating
19:28:28 sean-k-mooney ill give the ml post a read thanks
19:28:54 smcginnis sean-k-mooney: Pour yourselve a nice cup of tea first - it will take a while. ;)
19:52:45 openstackgerrit Lee Yarwood proposed openstack/nova master: libvirt: Block swapping to an encrypted volume when using QEMU to decrypt https://review.openstack.org/544238
19:59:44 openstackgerrit Chris Dent proposed openstack/nova master: Move test_report_client out of placement namespace https://review.openstack.org/558911
20:20:03 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Fix incorrect exception import https://review.openstack.org/558916
20:20:35 cdent efried, mriedem, dansmith : that's ^ a fun little bug fix that would be nice to have
20:24:06 openstackgerrit Surya Seetharaman proposed openstack/nova master: Add --enable and --disable options to nova-manage update_cell https://review.openstack.org/555416
20:24:57 mriedem cdent: we can't even have a simple unit test for that if gabbi won't cover it?
20:25:35 cdent mriedem: we can have a unit test for it but it would be...very mocky
20:25:49 mriedem that's fine, it's testing error handling
20:25:52 melwitt yeah, I was about to ask that. I'm not seeing unit tests in the tree for the aggregate handler, guess there hasn't been need of one yet
20:26:12 cdent melwitt, mriedem : as a general rule we haven't done unit tests for the handler code
20:26:16 melwitt that is, gabbit tests cover most of it
20:26:20 melwitt gabbi
20:26:22 mriedem cdent: i know
20:26:25 mriedem but...
20:26:30 mriedem clearly there is a need for it in some cases
20:26:36 melwitt aye
20:26:49 cdent I'm not saying I'm agin it, just that it hasn't happened yet
20:27:02 mriedem you can be a trailblazer here
20:27:19 cdent I was hoping to go to sleep instead
20:27:30 mriedem blaze that treasure trail in the morning
20:27:44 mriedem or lose sleep over it tonight :)
20:28:04 cdent I will lose sleep over trying to generate caring
20:29:00 cdent mriedem: so I can both think about it and not think about it, what is that you're hoping for here?
20:29:43 mriedem i'm -1 without a unit test
20:29:54 mriedem which can be dealt with whenever you feel like it i guess
20:30:00 cdent a test that confirms that the handler raises a 409 when it seens a ConcurrentUpdate, or that a ConcurrentUpdate happens when there is an increment generation failure that casues a ConcurrentUpdate
20:30:18 mriedem i'm fine with the former
20:30:20 mriedem seems easy enough
20:30:25 cdent one of the reasons we haven't done it in the past is because the answer to that ^ is unclear
20:30:35 melwitt +1 on the former
20:30:49 mriedem making sure we don't spew 500 out of the REST API is a simple enough thing to say is a good test
20:31:03 cdent hmmm. Would that even have caught this particular problem?
20:31:35 mriedem the exception moved,
20:31:37 cdent I suppose so, as the unit test itself would have had an import error when it tried to side effet
20:31:37 mriedem so yes it should
20:31:42 melwitt is ConcurrentUpdate a base nova exception too?
20:31:47 mriedem no
20:31:47 cdent not any more
20:31:48 mriedem it moved
20:31:58 melwitt okay, so you'd think it would blow up there
20:37:11 melwitt my concern is just let's patch that test coverage gap since we know it's there. so if that path breaks in the future, we'll catch it. whether that happens now or in a follow up is fine IMHO but I think it's worth doing
20:43:39 cdent I get the concern, my reluctance is mostly because we've done a good job of avoid mock madness in the tests associated with placement
20:43:59 openstackgerrit Merged openstack/nova master: network: add command to configure trusted mode for VFs https://review.openstack.org/458513
20:44:06 cdent I even added https://review.openstack.org/#/c/557355/
20:48:49 melwitt yeah, I understand. I do really like the gabbi testing of the placement APIs. I'm just not immediately seeing another way to cover this particular testing gap
20:49:01 openstackgerrit Tyler Blakeslee proposed openstack/nova master: Add __repr__ for NovaException https://review.openstack.org/555812
21:01:40 dansmith mriedem: melwitt cells meeting?
21:03:20 mriedem oh yeah
21:09:56 cdent efried: on https://review.openstack.org/#/c/548249/ do you remember why you are catching DBDuplicateError around set_aggregates?
21:10:06 efried ...
21:11:57 cdent efried: s/Error/Entry. That exception is handled in set_aggregates but with a pass, so I'm wondering if there's something else. it's not clear
21:12:00 efried cdent: Because Jay via https://review.openstack.org/#/c/548249/6/nova/api/openstack/placement/handlers/aggregate.py@100 pointed me to https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/handlers/inventory.py#L171-L174 which I copy/pasted.
21:13:33 cdent hrmm
21:14:47 efried cdent: Looking through a little bit, it's possible it's not necessary.
21:14:57 cdent will leave a NOTE next to it for the time being
21:15:04 efried ight.
21:23:38 openstackgerrit Matt Riedemann proposed openstack/nova master: Add nova-status check for ironic flavor migration https://review.openstack.org/527541
21:34:33 openstackgerrit Chris Dent proposed openstack/nova master: [placement] Fix incorrect exception import https://review.openstack.org/558916
21:35:06 cdent that version adds a unit test, but does it by extracting the problematic code to its own method (as suggested by https://docs.openstack.org/nova/latest/contributor/placement.html#testing )
21:42:25 mriedem melwitt: i've got a question in https://review.openstack.org/#/c/540258/
21:42:33 mriedem you and dan might have already covered that months ago though
21:46:12 mriedem so for initial scheduling, it seems necessary to hit all cells since we don't know which one the scheduler is going to pick for the instances in the create request, but for operations on an existing instance, it seems we should only need to care about hosts that are in the same cell as the instance, since we don't support move operations across cells
22:22:58 openstackgerrit Matt Riedemann proposed openstack/nova master: Add "activate_port_binding" neutron API method https://review.openstack.org/555947
22:22:58 openstackgerrit Matt Riedemann proposed openstack/nova master: Delete port bindings in setup_networks_on_host if teardown=True https://review.openstack.org/556333
22:22:59 openstackgerrit Matt Riedemann proposed openstack/nova master: Implement migrate_instance_start method for neutron https://review.openstack.org/556334
22:22:59 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: compute: use port binding extended API during live migration https://review.openstack.org/551371
22:23:00 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Port binding based on events during live migration https://review.openstack.org/434870
22:23:00 openstackgerrit Matt Riedemann proposed openstack/nova master: conductor: use port binding extended API in during live migrate https://review.openstack.org/522537
22:24:16 melwitt mriedem: thanks, looking. it's true that at present we don't support migrate across cells and we restrict it via request_spec.requested_destination.cell in the conductor task. someday when we do migrate across cells, we'll not want to restrict it
22:27:32 melwitt I'm trying to see if similar could be done with the instance group hosts query, if we can rely on request_spec.requested_destination.cell to limit it and put a NOTE on it that will remind us to remove that along with the others if we get cross-cell migration down the road
22:28:51 melwitt checking if task.execute happens before or after the setup_instance_group call
22:32:18 melwitt oh, it's _in_ execute. so yeah looks like we could read requested_destination.cell to know to limit it
22:34:05 melwitt for live migrate we'd have to move the setup_instance_group call down after the requested cell is set
22:35:42 mriedem efried: edleafe: fyi https://review.openstack.org/#/c/556529/
22:36:44 mriedem melwitt: so you mean from within the InstanceGroup.get_hosts() call, determine that you have a requested_destination.cell set and use it for the targeted context when doing InstanceList.get_by_filters?
22:36:51 mriedem or whatever the instance query method is,
22:37:18 mriedem you could do that, but you still have setup_instance_group doing a scatter/gather on the cells, so it would likely be redundant
22:39:45 melwitt mriedem: I was thinking in the setup_instance_group method, choose whether to scatter-gather based on whether request_spec.requested_destination.cell is set. (after making sure the setup_instance_group calls are moved until after .cell is set)
22:42:44 mriedem ah
22:42:46 mriedem yeah that might do it
22:43:38 mriedem one problem is InstanceGroup.get_hosts() doesn't take a context
22:43:51 mriedem but,
22:44:06 mriedem you could temporarily mutate it's _context to be the cell-targeted one from the RequestSpec
22:44:13 mriedem well...

Earlier   Later