Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-05-10
15:10:18 jroll melwitt: ok cool, thanks
15:10:20 artom NIH?
15:10:24 mriedem melwitt: shhh
15:11:04 mriedem short answer on SB indexing in google is no
15:11:05 mriedem not yet
15:11:16 mriedem see -dev for details
15:13:35 melwitt mriedem: I didn't think we'd need a blueprint for this https://blueprints.launchpad.net/nova/+spec/add-documentation-rules-api-ref ?
15:15:13 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Migrate tempest-dsvm-multinode-live-migration job in-tree https://review.openstack.org/567617
15:15:14 mriedem melwitt: ha no
15:18:18 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: Migrate tempest-dsvm-multinode-live-migration job in-tree https://review.openstack.org/567619
15:19:57 mriedem melwitt: just leave a comment that you don't need a blueprint to write contributor docs and mark the bp obsolete since that's as close to invalid as we get in launchpad
15:20:18 melwitt mriedem: k thanks
15:20:30 mriedem i also don't really want to review said contributor doc additions, but...
15:20:57 mriedem simply linking from the contributor docs to the api-ref wiki would be good enough IMO
15:24:11 melwitt okay, I'll suggest that on the bp
15:39:16 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: libvirt: check image type before removing snapshots in _cleanup_resize https://review.openstack.org/567623
15:39:51 mdbooth mriedem: Thanks for the review, btw. Working on addressing your comments, without embarking on another big cleanup.
15:40:34 mriedem mdbooth: np; i thought i had figured out why the compute manager test in the 3rd or 4th patch in the series was failing (due to the fake driver wanting legacy bdms) but that didn't fix it locally for me
15:41:26 mdbooth Yeah, I got to the bottom of that. It's an annoying testing artifact.
15:43:33 mdbooth The initializer pre-populates the dict subclass with a bunch of fields. This isn't required in the new class as it has only proxy fields, and can't be done for all proxied fields because is_volume is a property and isn't writable.
15:43:37 dansmith mriedem: that stub_instance thing is because of a failure, not because I was being overly thorough :)
15:43:49 mdbooth Anyway, I'll come up with something. Thanks again.
15:43:50 dansmith mriedem: I'll remove and run tests to highlight which one it was
15:44:30 dansmith I think it's probably fping or something, which mocks something with the stub_instance that would have shadowed the inner call we make with cell_mappings
15:44:31 mriedem dansmith: ok, as noted i didn't run all tests, just a sample, but anyway
15:44:38 dansmith mriedem: yup
15:45:12 mriedem dansmith: do you plan on doing a cleanup followup? otherwise i'm happy to.
15:45:15 mdbooth I suspect I'm going to end up with some test-only code in block_device.py, though, because the tests need some way to serialise a whole object for comparison, whereas runtime code just accesses fields by name.
15:45:16 mriedem since they're my nits
15:45:37 dansmith mriedem: I haven't looked at all your comments, but if there are legit ones I'll do it yeah
15:46:21 mriedem mdbooth: we shouldn't need test-only code in block_device.py. if we need special serialize code for those types of objects, surely we could house that in test utils separately
15:46:32 mriedem e.g. we have special comparison methods for versioned objects
15:46:55 mdbooth mriedem: It might come to that, but that's a bit of big cleanup I was hoping to avoid.
15:47:15 mdbooth I think I'd end up having to poke a bunch of block_device tests.
15:47:39 mdbooth But yeah, that was my first thought.
15:47:56 mriedem ok i guess i don't know the root issue and what the fix would be to understand why a lot of stuff would have to change just for comparing local image bdms
15:48:58 mdbooth Well, there's already a bunch of test-only code in that module as it happens, it's just not obvious that it's test only.
15:49:07 mdbooth I guess I could sneak it in :)
15:50:00 mdbooth Basically anything which calls self.update() in _transform for a proxy attribute is test-only.
15:50:48 mdbooth But... I really don't want to clean it all up right now :)
15:50:50 mriedem ok, so can't you follow suit for the new local image bdm?
15:52:03 mdbooth mriedem: I can, yes, but I was going to be honest and mark it as test-only :)
15:52:14 mriedem https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L226 and https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L242 could clearly be changed to just use self._fields to avoid the duplicatoin
15:52:44 dansmith mriedem: 8 fails: https://pastebin.com/jUSj03qW
15:52:52 mdbooth mriedem: Yep.
15:53:07 mriedem gd fping
15:53:12 dansmith ikr
15:53:20 mriedem dansmith: looks like it's time i delete that code which i've been meaning to anyway
15:53:26 dansmith \o/
15:53:37 mriedem as a start for the nova-net REST API delete pattern
15:53:46 mriedem will do that in a bit
15:54:07 mriedem *someone
15:55:06 mriedem mtreinish: fyi, tox -e py27 -- <foo> doesn't work in nova in stable/pike,
15:55:09 mriedem i think because of ostestr?
15:55:31 mriedem or our tox.ini isn't passing the regex through properly
15:56:27 mtreinish mriedem: yeah I think someone bumped constraints on pike to pull in os-testr 1.0.0 which broke the loophole nova was using for that (that was the thing mikal was complaining about in denver)
15:56:38 mriedem https://github.com/openstack/nova/commit/bb7628fdf3b7481a0ca487cf6ede0babc4890ac5#diff-b91f3d5bd63fcd17221b267e851608e8 is in pike
15:56:48 mtreinish I'm not sure why that was bumped on a stable branch though, but I've been pinged by a few people because of things that broke because of it
15:57:07 mtreinish mriedem: in the short term just do tox -epy27 -- --regex <foo>
15:57:12 mriedem doing that now
15:58:43 mriedem that doesn't seem to work
15:58:49 mriedem unless my regex is wrong
15:59:49 mriedem tox -e py27 -- --regex test_cleanup_resize_not_same_host
16:00:10 mtreinish oh, it's probably the quotes around posargs in the tox.ini
16:00:30 mtreinish it probably is treating it as a single parameter with a space instead of a flag
16:00:39 mriedem f it, zuul can tell me if i screwed up
16:00:48 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: libvirt: check image type before removing snapshots in _cleanup_resize https://review.openstack.org/567625
16:06:08 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: libvirt: check image type before removing snapshots in _cleanup_resize https://review.openstack.org/567625
16:07:42 mriedem cfriesen_: i know you're using postgres, have you ever hit the issue described here? https://groups.google.com/forum/#!topic/sqlalchemy/sRgvpjUkzPU
16:07:56 mriedem looks like eventlet shenanigans with pg and sqla
16:08:11 cfriesen_ on a call, will take a look in a bit
16:17:40 openstackgerrit Matt Riedemann proposed openstack/nova stable/ocata: libvirt: check image type before removing snapshots in _cleanup_resize https://review.openstack.org/567630
16:22:16 tssurya mriedem: looks like the avz setting if its not specified by the user depends on CONF.default_schedule_zone (https://github.com/openstack/nova/blob/23fce8c13f8347a8ad862ab38ae9d8259db21349/nova/compute/api.py#L487)
16:22:59 mriedem tssurya: ok which defaults to None https://docs.openstack.org/nova/pike/configuration/config.html#DEFAULT.default_schedule_zone
16:23:09 tssurya mriedem: yes,
16:23:39 mriedem so, i kind of wonder how we didn't break the meaning / contract of None with the change in pike to always set AZ
16:23:43 mriedem "* None, which means that the instance can move from one availability zone to another during its lifetime if it is moved from one compute node to another."
16:25:23 mriedem i.e. before the change in pike, i could create an instance w/o specifying AZ and freely migrate to any compute node, but now if randomly land in AZ1, i'm bound to AZ1 even though i never specified that
16:25:38 mriedem which also likely means operators are going to use the force flag when live migrating those types of instances
16:26:12 tssurya mriedem: uh-huh, I see
16:26:56 mriedem well, i assume that's true anyway, i haven't tried it, but it should be easy to reproduce with a functional test
16:27:15 mriedem create 2 computes in separate AZs, create the instance w/o specifying AZ, and then live migrate it and see if scheduling fails
16:27:30 mriedem if that works in ocata but not > ocata then we regressed
16:27:41 tssurya mriedem: ack, I will look into this
16:37:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Move create_tap_dev to the new utility module. https://review.openstack.org/557835
16:37:24 openstackgerrit Matt Riedemann proposed openstack/nova master: Move set_vf_interface_vlan to the new utility module. https://review.openstack.org/558487
16:38:39 mriedem efried: i cleaned those up so tests pass now ^
16:38:42 openstackgerrit Dan Smith proposed openstack/nova master: Address feedback from instance_list smart-cell behavior https://review.openstack.org/567643
16:38:45 dansmith mriedem: ^
16:39:00 efried mriedem: ack
16:43:42 efried mriedem: It doesn't look like you actually fixed https://review.openstack.org/#/c/557835/8/nova/tests/unit/network/test_utils.py
16:43:54 efried mriedem: Unless I'm missing something obvious, you added the import but didn't change the offending references.
16:44:10 efried So now you'll fail pep8 *and* py27/35 :)
16:44:23 mriedem you're blind
16:44:26 mriedem everything passed locally
16:44:39 efried mriedem: sorry, I'm blind, ignore me.
16:46:30 mriedem efried: no joke https://review.openstack.org/#/c/558487/
16:46:34 mriedem sahid would be PISSED!
16:47:23 mriedem apparently that doesn't have any tests?
16:47:53 mriedem https://github.com/openstack/nova/commit/27614dc595d0fac2e57300c85bb34da266849a46

Earlier   Later