Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-05-10
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
16:48:23 efried mriedem: I clearly looked into it https://review.openstack.org/#/c/558487/2/nova/network/linux_net.py@a1813
16:48:39 efried mriedem: Why doesn't my comment show up with yours? Seems like a gerrit ui bug.
16:48:42 sean-k-mooney mriedem: we should proably port that to os-vif at some point rather then doing it in the libvirt vif.py
16:48:55 mriedem sean-k-mooney: yes, both jaypipes and i asked for that in the original patch
16:48:56 efried anyway, IIRC what I discovered was that that method had been removed somewhere else.
16:49:44 mriedem so in his first change, he means to move create_tap_dev, but also moved the trusted vf tests, and then in the 2nd patch deletes the trusted_vf code but not the tests....
16:50:02 sean-k-mooney mriedem: adding a new pluging wont be two hard ill open a bug for it to track it. we also need to move a few others
16:50:03 efried and yeah, I see that method in master.
16:50:49 mriedem which is also what was causing the pep8/unit test failures
16:50:51 mriedem so i'll fix this up
16:59:41 openstackgerrit Dan Smith proposed openstack/nova master: Address feedback from instance_list smart-cell behavior https://review.openstack.org/567643
17:08:32 openstackgerrit Matt Riedemann proposed openstack/nova master: Move create_tap_dev to the new utility module. https://review.openstack.org/557835
17:08:33 openstackgerrit Matt Riedemann proposed openstack/nova master: Move set_vf_interface_vlan to the new utility module. https://review.openstack.org/558487
17:09:59 mriedem efried: ok those 2 should be golden now
17:10:08 efried mriedem: ack
17:21:21 efried mriedem: +2 on the first one, but didn't +W it - question inline. Can you confirm that that test case was added as part of this patch?
17:21:35 efried (I don't see it in master or in the base)
17:22:32 cfriesen_ mriedem: regarding the DB thing. Doesn't ring a bell, but I'm pretty sure we take down the DB, then switch the VIP, then bring up the DB again on the new side.
17:24:48 cfriesen_ cfriesen: we use shared storage rather than a master/slave postgres setup
17:25:08 cfriesen_ mriedem: ^ (apparently I like talking to myself)
17:32:48 cfriesen is there a reason why we default to "novalocal" as the dhcp_domain even when using neutron? It means the hostname retrieved via metadata has ".novalocal" appended to it by default.

Earlier   Later