| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-10 | |||
| 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. | |
| 17:36:44 | openstackgerrit | Eric Fried proposed openstack/nova master: Add tests for alloc_cands with member_of https://review.openstack.org/561399 | |
| 17:40:34 | mriedem | cfriesen: ack | |
| 17:41:10 | mriedem | cfriesen: https://www.openstack.org/summit/vancouver-2018/summit-schedule/events/21754/novaneutron-ops-cross-project-session | |
| 17:49:40 | mriedem | efried: yes it's new and it also looks wrong | |
| 17:50:07 | mriedem | because ex.assert_has_calls(calls) looks like the wrong assertion, since execute should have been called twice | |
| 17:50:18 | mriedem | so that should have probably been ex.return_value.assert_has_calls | |
| 17:52:14 | mriedem | oh nvm, that's on set_device_mtu which is unrelated | |
| 17:52:20 | efried | mriedem: ... yeah. | |
| 17:52:28 | efried | so like, where tf did this test case come from? | |
| 17:52:37 | efried | It's not wrong, it's just... random. | |
| 17:53:24 | mriedem | probably rebase damage from I9872056c440a706b89dd51d3b9f2418951723efa | |
| 17:53:49 | mriedem | yeah it was dropped here https://review.openstack.org/#/c/557543/1/nova/tests/unit/network/test_linux_net.py | |
| 17:54:17 | mriedem | i'll update | |
| 17:54:31 | efried | mriedem: Yup, I see it. | |
| 17:54:42 | efried | mriedem: No need to update, what are you going to do, split it into a separate patch? | |
| 17:54:48 | mriedem | yeah | |
| 17:55:14 | efried | mriedem: Okay. FWIW I'd be fine just commenting in the review that it was accidentally dropped in a bad rebase, add that link ^, and be done. | |
| 17:55:22 | efried | but if you prefer splitting, soit. | |
| 17:57:39 | openstackgerrit | Alvaro Lopez Garcia proposed openstack/nova master: Ensure that periodic reclaim cleans DB deleted instances https://review.openstack.org/323250 | |
| 17:57:47 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Move create_tap_dev to the new utility module. https://review.openstack.org/557835 | |
| 17:57:48 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Move set_vf_interface_vlan to the new utility module. https://review.openstack.org/558487 | |
| 17:57:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add test_set_device_mtu_default back in https://review.openstack.org/567659 | |
| 17:58:50 | efried | mriedem: +2 all the way up. Don't know if you still feel good about approving the bottom one... | |
| 17:58:56 | mriedem | i feel great | |
| 17:59:23 | mriedem | you could also just fast approve https://review.openstack.org/#/c/567659/ | |
| 17:59:27 | mriedem | it's just adding a unit test back in | |
| 18:00:09 | efried | mriedem: done. | |
| 18:13:35 | clutchbeyers | This is a post-newton question: Does anyone know if we set availability_zone in spec_obj on vm provision if the user does not select an availability_zone? | |
| 18:14:23 | clutchbeyers | set in the db after launch* | |
| 18:28:45 | mriedem | clutchbeyers: i know that starting in pike we set the instance.availability_zone based on the compute host that was selected by the scheduler https://review.openstack.org/#/c/446053/ - not sure if that extends to the request spec | |
| 18:28:54 | mriedem | the request spec is really about what the user requested | |
| 18:29:08 | mriedem | so i don't think setting reqspec.az if the user didn't request one would be correct there | |
| 18:31:15 | melwitt | before that change, instance.availability_zone used to get set to the "default availability zone" if one wasn't requested at boot time | |
| 18:36:43 | openstack | Launchpad bug 1768876 in OpenStack Compute (nova) queens "metadata-api fails to get availability zone for instances created before pike" [High,Triaged] | |
| 18:36:43 | mriedem | melwitt: i know, see https://bugs.launchpad.net/nova/+bug/1768876 | |
| 18:36:46 | efried | mriedem, melwitt: Do y'all have a problem if I fast-approve takashin's mox removal patches? They're test-only; if they pass zuul they clearly work; and I'd like to save other reviewers the pain of combing over these (they're pretty tedious). | |
| 18:37:35 | melwitt | efried: passing zuul is the easy part -- reviewing to ensure test coverage is not lost isn't. so I've not thought of them as easy, one person reviews | |
| 18:37:47 | melwitt | but that's just MHO | |