Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-14
15:11:51 gmann stephenfin: mriedem or you can use d-g var for legacy if not writing directly on localrc - DEVSTACK_GATE_USE_PYTHON3. But moving the devstack default to py3 and keep py2 setting in devstack on demand if anyone needed is something we should do first. I will review mriedem patch (not sure it does the same).
15:14:06 dansmith mriedem: yeah it wasn't showing up on dash for me at the time, but is now.. hadn't run anything so must've just been queued or something
15:14:27 openstackgerrit Dan Smith proposed openstack/nova master: Add cache_images() to conductor https://review.opendev.org/687139
15:14:27 openstackgerrit Dan Smith proposed openstack/nova master: Add image caching API for aggregates https://review.opendev.org/687140
15:15:11 dansmith mriedem: conductor patch was just rebased to kick it out of the gate right away and get in line behind the 2.80 one ^
15:15:14 mriedem efried: no, there is still a novaclient change
15:15:20 gmann stephenfin: mriedem FYI. fully dropping py2 from devstack or other common tooling/lib/oslo etc will be the last once all projects drop py2. We discussed the plan in TC for that and i will send the same on ML also, further and final plan/discussion will be on 24th TC office hour.
15:15:37 efried mriedem: ack thx
15:16:12 mriedem gmann: can't change the default on DEVSTACK_GATE_USE_PYTHON3 in d-g since it's branchless, unless we add branch logic in d-g for that like we did for using neutron by default back in i think newton or ocata
15:16:44 mriedem this https://github.com/openstack/devstack-gate/blob/master/devstack-vm-gate-wrap.sh#L199
15:18:28 gmann mriedem: yeah, legacy jobs and d-g need branch handling.
15:18:35 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove unused 'nova-dsvm-base' job https://review.opendev.org/688389
15:18:36 openstackgerrit Stephen Finucane proposed openstack/nova master: zuul: Make functional job inherit from openstack parents https://review.opendev.org/688425
15:19:03 mriedem one question is going to be grenade, since if grenade train side starts with py2 and upgrades to ussuri and starts using py3 packages, but https://review.opendev.org/#/c/649097/ should have hit that
15:19:30 dansmith mriedem: is that what we'll do? or just go py3-py3 on both sides?
15:19:54 dansmith I feel like staying on py3 on both sides is more in line with what we've done before with ubuntu upgrades, but not very realistic of couse
15:20:04 mriedem since https://review.opendev.org/#/c/649097/ isn't doing anything else i expect grenade starts with py2 and the new side is py3
15:20:16 mriedem yeah it's definitely not realisitc,
15:20:19 mriedem *realistic
15:20:39 mriedem we - or someone that cares enough, like a distro - should make sure that people can upgrade from py2 in train to py3 in ussuri
15:21:18 mriedem this came up a long time ago of course when the tc was talking about py2 deprecation and the only thing i remember really coming up was maybe some oslo libraries, like messaging, maybe having some compat issues
15:21:24 dansmith mriedem: why is that patch causing a switch?
15:21:32 gmann that should be tested as past of py2.7 jobs + py3 testing on train
15:21:51 mriedem dansmith: because the old side of grenade in that run would run with py2 by default - since it's running from stable/n-1
15:21:59 dansmith mriedem: oh you mean the grenade testing of that patch in isolation, not once that patch is merged?
15:21:59 gmann mean both version testing make sure train code can run on both py version so does upgrade is no issue
15:22:08 mriedem dansmith: both
15:22:32 mriedem gmann: not really
15:22:47 mriedem say you have multi-node grenade where one compute is left at n-1 (train),
15:22:59 gmann mriedem: but we will keep only grenade-py3 in ussuri which will be py3->py3
15:23:04 mriedem i would expect that job to start with py2, upgrade controller services and one compute to py3 ussuri and leave the n-1 compute at py2
15:23:09 mriedem and everything, like rpc, should continue to work
15:23:18 dansmith mriedem: oh, devstack has branched, d-g does not I see, yeah
15:23:23 mriedem yup
15:23:55 mriedem gmann: if we don't have something like what i described above with multinode grenade we're not really testing upgrades from py2 to py3
15:24:26 mriedem e.g. what happens if a py3 conductor sends serialized json rpc to a py2 compute?
15:24:34 gmann how that will be different than testing compute node for both version in separate jobs
15:24:51 gmann ohk
15:25:10 dansmith mriedem: one *could* argue that we don't care to test that sort of thing
15:25:15 mriedem sure
15:25:25 mriedem this isn't going to be my fight in ussuri
15:25:27 dansmith mriedem: as you say, leave it to thedistros, but not specifically test or care about a platform change related problem
15:25:31 mriedem if it blows up i'll laugh
15:25:38 gmann this came up in denver PTG (previous one) also
15:26:00 mriedem gmann: lots of things come up
15:26:16 mriedem idk if anyone (the tc?) documented test plans for this
15:26:22 mriedem or statements about testing this kind of upgrade
15:26:38 mriedem if there was previous agreement from the tc and/or qa or whatever that we as a community don't care about testing that scenario, then fine
15:27:05 gmann http://lists.openstack.org/pipermail/openstack-dev/2018-September/134592.html
15:27:32 mriedem yeah that's the thread i was thinking about when i said this came up earlier
15:30:41 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove unused 'nova-dsvm-base' job https://review.opendev.org/688389
15:30:41 openstackgerrit Stephen Finucane proposed openstack/nova master: Stop testing Python 2 https://review.opendev.org/687954
15:30:42 openstackgerrit Stephen Finucane proposed openstack/nova master: zuul: Make functional job inherit from openstack parents https://review.opendev.org/688425
15:30:47 gmann mriedem: i think there was agreement 'community don't care ' but documented anywhere
15:31:08 mriedem that's what doug seems to be saying http://lists.openstack.org/pipermail/openstack-dev/2018-September/134655.html
15:31:14 mriedem so *shrug*
15:31:28 gmann * not documented anywhere
15:33:44 gmann I think we can add that in TC doc/resolution. i will add that part as part of 24th TC office hour discussion
15:48:25 openstackgerrit Chris Dent proposed openstack/nova master: single pass instance info fetch in host manager https://review.opendev.org/623558
15:58:05 melwitt mriedem: I thought you might be interested in this, I wrote a regression test for the "run online_data_migrations twice fails" in the post test hook and a different fix on top https://review.opendev.org/688205
15:58:49 mriedem that would get around the mysql in functional test thing yeah
15:59:05 mriedem get your power back?
16:03:18 melwitt yeah, restored friday evening
16:03:50 melwitt nice to have internet again
16:03:54 mriedem heh, i bet
16:06:46 melwitt and yeah, I couldn't find another way to exercise the bug and explained briefly why in the commit message. lmk if you think the explanation could use more info, or if there are alternatives that I missed
16:07:14 openstackgerrit Merged openstack/nova master: Avoid using image with kernel in BDM large request func test https://review.opendev.org/688132
16:08:45 efried stephenfin: Does https://review.opendev.org/#/c/688249/ look right to you?
16:10:04 stephenfin sure does
16:11:44 efried stephenfin: cool, I've got a few similar ones...
16:11:46 efried thanks
16:13:18 melwitt zzzeek: I would appreciate your review on this fix I proposed related to the questions I asked you on friday https://review.opendev.org/688206
16:13:38 zzzeek melwitt: yah, saw the link, will look now
16:13:46 melwitt (and thanks again for helping me out with that)
16:15:02 efried https://review.opendev.org/#/c/688250/ https://review.opendev.org/#/c/688235/ https://review.opendev.org/#/c/688087/ if you care stephenfin
16:17:22 cdent mriedem: my to do list has just reminded me to prompt folk about https://review.opendev.org/#/c/623558/ which is jay's patch to improve instance info fetch in the host manager. A while back I added some unit tests and did the tweaks that eric requested
16:18:49 openstackgerrit Merged openstack/os-resource-classes master: Bump the openstackdocstheme extension to 1.20 https://review.opendev.org/688249
16:24:10 zzzeek melwitt: before I look silly on the review because I'm missing something, do i read correctly that security_group_ensure_default is called twice inside of instance_create() ?
16:25:02 melwitt zzzeek: that is correct. the first time, it intends to create it if it doesn't exist, the second time, it's really only doing it to read it back (was my conclusion)
16:25:36 zzzeek melwitt: what condition would casue the return value to have changed in the intervening 15 or so lines of code?
16:25:39 melwitt are you thinking another way would be to just use the returned object from the first time, later on?
16:25:55 openstackgerrit Eric Fried proposed openstack/nova-specs master: Add 'Feature Liaison' spec process https://review.opendev.org/685857
16:25:56 openstackgerrit Eric Fried proposed openstack/nova-specs master: Add spec for vm scoped sriov numa affinity https://review.opendev.org/683174
16:26:01 efried bauzas: ^
16:26:19 zzzeek melwitt: beacsue i dont know the concepts being worked with here, from what im seeing i dont immediately see why a. the top call should use the context, not independent transaction and b. what's wrong with default_gruop assigned at the top there
16:26:58 openstackgerrit Ghanshyam Mann proposed openstack/nova-specs master: Re-propose policy-defaults-refresh spec for Ussuri https://review.opendev.org/686058
16:27:02 zzzeek melwitt: then there's this _Get_Sec_group_models closure, which appears to be called only once, three lines later, no recursion, no passing it onwards, so why is it like that
16:27:02 gmann melwitt: ^^^ re-proposed policy spec for ussuri
16:27:04 mriedem cdent: without digging into the code change yet, i dropped a few comments in the commit message
16:27:15 mriedem soft -1, i know it's an adopted patch
16:27:17 zzzeek melwitt: it makes it look like this used to be very different and was refactored, but not all the way
16:27:21 cdent mriedem: thanks
16:27:50 zzzeek melwitt: if you agree thatim reading it right i can post my comments to this effect and folks can discuss
16:28:36 melwitt zzzeek: indeed, I could only assume similar because I'm not _that_ familiar with the method and definitely not familiar with the history. I'm not quite parsing your question a) though, why the top call should not use a separate transcation? did you mean why does it?
16:29:10 mriedem note that we don't even need that call if using neutron https://review.opendev.org/#/c/653065/
16:29:20 melwitt zzzeek: and I agree that it looks partially refactored
16:29:22 zzzeek melwitt: yes, got my negatives reversed.
16:29:29 zzzeek melwitt: yes why use a separte trans there
16:30:37 zzzeek melwitt: when the separate trans is used, that means that you either want the thing you're doing to modify the DB even if your method fails, or you want the thing you're doing to modify the DB while your mehtod is still running (like logging a slow operation), or, otehrwise you're trying to read some state that occurs within the middle of your ongoing, slow transaction
16:31:26 zzzeek melwitt: this instance_create doesn't look like a slow function and it looks like the originator of the transaction that it commits. but i dont know

Earlier   Later