| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-10 | |||
| 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 | mriedem | melwitt: i know, see https://bugs.launchpad.net/nova/+bug/1768876 | |
| 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: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 | |
| 18:38:07 | efried | melwitt: roger that. YHO is what I was asking for. Will leave them for other cores to scrutinize. | |
| 18:39:00 | melwitt | mriedem: guh, another regression | |
| 18:40:18 | openstackgerrit | Merged openstack/nova master: Add CellMapping.get_by_project_id() query method https://review.openstack.org/509002 | |
| 18:47:02 | openstackgerrit | Dan Smith proposed openstack/nova master: Change the field type of InstanceMapping.instance_uuid https://review.openstack.org/567669 | |
| 18:51:42 | arvindn05 | mriedem: good call on checking for scheduling volume backed instances | |
| 18:52:06 | clutchbeyers | thanks for that explanation. we have a use-case where whatever az the scheduler chooses, we do not want a resize to migrate across az's. Sounds like the team has made the decision to keep that logic, and I support that. We can fork our own logic for that | |
| 18:52:33 | arvindn05 | for volume back instances the volume meta data is provided as image meta data. see https://github.com/openstack/nova/blob/master/nova/compute/api.py#L1060 | |
| 18:53:08 | mriedem | clutchbeyers: if you look at my comments in https://bugs.launchpad.net/nova/+bug/1768876 - starting in pike you can't migrate across az's | |
| 18:53:08 | openstack | Launchpad bug 1768876 in OpenStack Compute (nova) queens "metadata-api fails to get availability zone for instances created before pike" [High,Triaged] | |
| 18:53:15 | mriedem | which is actually a regression | |
| 18:53:16 | jmccarthy | mriedem: You about ? Trying to get 'test_cleanup_resize_not_same_host' to pass locally with tox, I don't really know this area - any hints how to resolve this conflict ? https://paste.fedoraproject.org/paste/ntZxhXGlWmzNiL--RZU~Ww | |
| 18:53:30 | mriedem | jmccarthy: see my backports? | |
| 18:53:40 | mriedem | https://review.openstack.org/#/q/Ib10081150e125961cba19cfa821bddfac4614408 | |
| 18:54:23 | jmccarthy | mriedem: Oh ! Ok cool lemme check that out | |
| 18:54:59 | mriedem | arvindn05: i could be wrong but i don't think that actually gets stored in the RequestSpec.image field | |
| 18:55:38 | jmccarthy | mreidem: Perfect - Thanks again ! Quite what to do there was giving me a headache lol | |
| 18:55:59 | arvindn05 | from what i saw...the volume meta data is treated exactly the same as image meta data,...the requestSpec.image has volume meta data in the case | |
| 18:56:36 | mriedem | arvindn05: i'm not so sure https://github.com/openstack/nova/commit/0a7427dc58eeb271646a962604106e0e84b870b6 | |
| 18:56:36 | arvindn05 | the only difference is that for volume backed instance the "id" field in the RequestSpec.image is not present... | |
| 18:56:40 | mriedem | but i don't know what you're seeing | |
| 18:56:54 | mriedem | oh that might be true | |
| 18:57:40 | arvindn05 | yup...the code there looks at the id field as well to determine...so looks like its consistent | |
| 18:57:45 | mriedem | oh i guess we use boot_meta here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L882 | |
| 18:58:28 | mriedem | https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py#L132 | |
| 18:58:30 | arvindn05 | now the only question is should we honor that metadata for volume boot? | |
| 18:59:42 | mriedem | idk | |
| 19:00:00 | arvindn05 | https://blueprints.launchpad.net/python-cinderclient/+spec/support-modify-volume-image-metadata an old blueprint discussion | |
| 19:00:10 | mriedem | i'm not sure how much precedent we have for honoring image meta during scheduling that comes from a volume | |
| 19:00:11 | arvindn05 | 20:32:54 <DuncanT> fmaldonado_: volume glance meta is for all of the same things glance properties are for | |
| 19:01:58 | arvindn05 | it seems that within the code, the volume meta data is treated exactly the same as image meta data...so i am leaning towards leaving it this way and adding few more unit and functional test cases | |
| 19:04:07 | clutchbeyers | mriedem: thats great to hear on the cross-az migration, we handle that in newton by use of the nova filters and extra specs that dont allow things to cross. But our specific use case is in the resize. If one spawns an instance without giving it an az, a resize will cold migrate (shutdown) and treat it like a new vm spin up and select another az with new flavors. Are you saying this behavior also does not allow for cross- | |
| 19:05:22 | mriedem | clutchbeyers: the change in pike would apply to any move operation through the scheduler, | |
| 19:05:32 | mriedem | so resize, live migrate, unshelve and evacuate | |
| 19:09:04 | clutchbeyers | mriedem: thank you for the clarification! | |
| 19:10:40 | mriedem | clutchbeyers: note that we could consider this a regression in pike. it's in your favor i guess, but depending on how we handle this bug we might change it back. | |
| 19:11:02 | mriedem | dansmith: maybe something we need to put in your forum session on cells... | |
| 19:13:08 | dansmith | mriedem: I'm not sure what you're saying is a regression | |
| 19:13:11 | dansmith | or what it has to do with cells | |
| 19:13:31 | dansmith | that you can't migrate across az boundaries/ | |
| 19:13:38 | mriedem | dansmith: https://bugs.launchpad.net/nova/+bug/1768876 | |
| 19:13:38 | openstack | Launchpad bug 1768876 in OpenStack Compute (nova) queens "metadata-api fails to get availability zone for instances created before pike" [High,Triaged] | |
| 19:14:02 | clutchbeyers | mriedem: it my honest opinion, if a tenant does not specify an az on vm provision, then they arent too concnered about controlling their fault domain. So when they do decide to resize and it moves to another az, they theoretically shouldn't care | |
| 19:14:33 | clutchbeyers | But for those that specifically call out an az, they have a fault domain architecture in mind and wouldnt want to disrupt that | |
| 19:14:56 | dansmith | mriedem: okay, I think we talked about that in dublin, about potentially just forcing instance.az to whatever we chose if you didn't ask for one, keeping you in whatever az you happened to land in | |
| 19:15:26 | melwitt | I think that's what the original behavior was -- only allows migrating across AZ if none was selected deliberately | |