Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-04
19:12:50 sean-k-mooney mriedem: i gues g-r keeps the minium for all pojects and nova requirments.txt has minium for nova
19:13:28 cdent mriedem: upper constraints remains the one true thing, even without a globally sync requirements.txt, maybe?
19:13:40 mriedem which means we could bump our minimum required version in nova for a library, but not care about bumping it in g-r
19:13:57 dansmith we still have to be below u-c
19:13:57 sean-k-mooney so if you dont deploy something that need the newer requirement you can still use version below g-r. that said our gate jobs always run with whatever is allowed by u-c
19:14:11 mriedem u-c bumps are automated
19:14:14 mriedem and merged in batches
19:14:25 openstackgerrit Eric Young proposed openstack/nova master: Support extending attached ScaleIO volumes https://review.openstack.org/554679
19:14:31 dansmith my feeling is that we'll still have some things where you can't actually use nova at the version pinned by cinder for a given thing
19:15:03 mriedem yeah. forget i asked. i need to find a hole to put my head.
19:15:57 mriedem the foot of snow we got yesterday doesn't help
19:16:15 dansmith better you than me
19:19:17 kashyap [nag] Since I'm awake at this random hour in front of the screen, might as well point out this if you are looking for something to +A: https://review.openstack.org/#/c/534384/
19:19:27 kashyap It already has a +2
19:20:29 smcginnis mriedem: I don't think g-r matters anymore and is probably going away at some point.
19:20:40 smcginnis Things just need to be between lower-constraint and upper-constraint.
19:21:52 sean-k-mooney smcginnis: isnt g-r the lower-constraint
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

Earlier   Later