| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-25 | |||
| 19:08:48 | dansmith | mriedem: we're not running the _nw_info_build_network code | |
| 19:09:08 | melwitt | mriedem: thanks, will look | |
| 19:09:17 | mriedem | dansmith: hmm | |
| 19:09:23 | mriedem | i'm not sure why we wouldn't, i don't see anything stubbing that out | |
| 19:09:27 | dansmith | mriedem: that was my point above | |
| 19:09:30 | dansmith | and you shat upon it | |
| 19:09:31 | mriedem | oh wait, | |
| 19:09:42 | mriedem | we do have other things that stub some shit out in the nw api | |
| 19:09:51 | mriedem | and i bet that is getting stubbed in the parent test base class | |
| 19:10:23 | mriedem | in the air like you just don't care!? | |
| 19:10:52 | dansmith | like jan brady when marcia suggests the same thing she just did and now everyone thinks it's cool | |
| 19:11:58 | mriedem | ServersTestBase | |
| 19:12:03 | mriedem | fake_network.set_stub_network_methods(self) | |
| 19:12:17 | mriedem | thar she blar | |
| 19:13:13 | mriedem | so if stephenfin's test calls unset_stub_network_methods we should hit the cache rebuilder | |
| 19:14:00 | sean-k-mooney | so _nw_info_build_network is being called when weer are booting | |
| 19:14:00 | mriedem | i'm not sure why someone didn't figure this all out hours ago :P | |
| 19:14:19 | mriedem | sean-k-mooney: yes, at the end of allocate_for_instance, | |
| 19:14:26 | mriedem | but we don't get that far b/c of set_stub_network_methods | |
| 19:14:27 | sean-k-mooney | yes | |
| 19:14:37 | sean-k-mooney | its not called in the rebuild | |
| 19:15:01 | mriedem | it doesn't need to be, | |
| 19:15:14 | mriedem | b/c after the server is created, the nw info cache is persisted in the db with the instance | |
| 19:15:21 | mriedem | assuming we had something in the db, but we don't b/c of set_stub_network_methods | |
| 19:15:22 | dansmith | yep, that fixes it, I'll push this up | |
| 19:15:26 | mriedem | WOOT | |
| 19:15:49 | sean-k-mooney | dansmith: awsome :) | |
| 19:16:22 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Add additional functional tests for NUMA networks https://review.openstack.org/585385 | |
| 19:23:10 | dansmith | melwitt: mriedem: I'm assuming no cells meeting with the crunch and all | |
| 19:23:20 | melwitt | +1 | |
| 19:23:35 | mriedem | agree | |
| 19:31:49 | mriedem | oh sweet irony as soon as we approve artom's test_tagged_attachment debug patch, it fails on that test | |
| 19:31:53 | mriedem | http://logs.openstack.org/32/584032/5/check/nova-next/caee4e1/logs/testr_results.html.gz | |
| 19:39:06 | mriedem | doesn't show any of artom's new debug messages for the instance or volume | |
| 19:41:47 | sean-k-mooney | ok my server says i loged into it 12 hour and 21 minutes ago. my brain is fried so im going to call it a day. i have the functional test for dansmith updated version running local. if test_rebuild_server_network_changes still fails perhaps we should remove it given cold migrate and test_rebuild_server_no_network_changes pass | |
| 19:42:31 | dansmith | yeah the rebuild tests don't work, | |
| 19:42:35 | mriedem | dansmith: what do you want me to do re ^? otherwise i'm just watching zuul and need to book a flight to china. do we have more tests that need to be written and/or cleaned up in the patch? | |
| 19:43:15 | dansmith | I think we need the negative test | |
| 19:43:41 | mriedem | i'm not sure i know how to do that | |
| 19:43:50 | mriedem | that's where the nodes on the host are all claimed? | |
| 19:44:01 | dansmith | no, there's an easier way | |
| 19:44:12 | dansmith | jan will take a crack at it while marcia brushes her hair | |
| 19:44:33 | mriedem | alright, wfm | |
| 19:44:39 | mriedem | works. for. marcia. | |
| 19:44:58 | mriedem | oh btw i'm in full summer feather hair mode atm | |
| 19:45:09 | mriedem | but it will be shorn before denver | |
| 19:49:07 | sean-k-mooney | so ya that just finished running all the nova fuctional tests. nova.tests.functional.libvirt.test_numa_servers.NUMAServersWithNetworksTest.test_rebuild_server_network_changes is the only one that failed in dans version so its just the attch suff thats failing. | |
| 20:03:17 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Add additional functional tests for NUMA networks https://review.openstack.org/585385 | |
| 20:16:58 | artom | mriedem, wait, it failed and logged nothing at all o_O | |
| 20:19:49 | dansmith | mriedem: so you'll convert your +1 to +2 on the functional move patch? | |
| 20:21:50 | efried | Does he have to deny you three times, or something? | |
| 20:22:07 | efried | not that kind of conversion, maybe. | |
| 20:58:39 | openstackgerrit | Mathieu Gagné proposed openstack/nova master: Add support for multiple fixed-ips in metadata https://review.openstack.org/580742 | |
| 21:00:12 | mriedem | dansmith: yeah | |
| 21:00:27 | mriedem | artom: right | |
| 21:02:09 | melwitt | does anyone know anything about metadata API versions and what's valid? for the addition of 'ip_addresses' to network_data.json, it means we have a second version update this cycle and we apparently already used a date in the future 2018-08-27 https://review.openstack.org/#/c/580742/2/nova/api/metadata/base.py@77 | |
| 21:03:32 | mriedem | apparently we already did that once with NEWTON_TWO even though NEWTON_ONE was using the wrong date | |
| 21:03:44 | melwitt | what's proposed is 2018-08-27-2 which is a format we haven't done before | |
| 21:03:52 | mriedem | # correlate with OpenStack release dates, with all the changes for a given | |
| 21:03:52 | mriedem | # NOTE(mikal): think of these strings as version numbers. They traditionally | |
| 21:03:53 | mriedem | # from EC2, which this needs to be compatible with. | |
| 21:03:53 | mriedem | # required for testing purposes. We know this isn't great, but its inherited | |
| 21:03:53 | mriedem | # hidden from the listing, but can still be requested explicitly, which is | |
| 21:03:53 | mriedem | # release bundled into a single version. Note that versions in the future are | |
| 21:04:03 | mriedem | i would -1 on the fomrat | |
| 21:04:04 | mriedem | *format | |
| 21:04:24 | mriedem | # release bundled into a single version." we could lump the changes into the existing ROCKY version | |
| 21:04:24 | mriedem | # correlate with OpenStack release dates, with all the changes for a given | |
| 21:04:24 | mriedem | based on "They traditionally | |
| 21:04:42 | mriedem | which isn't CI/CD friendly, so if we cared, i'd just make the ROCKY_TWO a day later than ROCKY_ONE | |
| 21:04:48 | mriedem | i'm a big meh on either approach | |
| 21:05:10 | melwitt | okay, I was wondering about that. else we could use 2018-08-30 the actual rocky release date? I haven't been able to find examples of consumers using version strings to request versions | |
| 21:05:30 | mriedem | it says versions in the future are hidden from the listing, | |
| 21:05:38 | mriedem | so if that's true, rocky doesn't even show up today, | |
| 21:05:40 | mriedem | but would need to confirm | |
| 21:06:11 | melwitt | and it looks like this new version won't break anyone because it adds a field, doesn't change any fields. so cloud-init defaulting to latest (I think it does, based on the code) will still work with the new version with 'ip_addresses' in it | |
| 21:06:19 | melwitt | ohh, I see | |
| 21:06:34 | mriedem | i have a hell of a time ever knowing where the route code in this thing works | |
| 21:06:44 | melwitt | so rolling together should work on that basis. I didn't understand what "hidden" meant until you said that | |
| 21:07:00 | mriedem | well, i'd want to know where that hiding happens | |
| 21:07:04 | mriedem | i don't have a devstack handy to test this | |
| 21:07:09 | dansmith | right, should be rocky release date, and we shouldn't have multiple versions for rocky | |
| 21:07:26 | dansmith | I think that the date-based one is hidden, | |
| 21:07:32 | dansmith | but current takes you to it, | |
| 21:07:41 | mriedem | https://review.openstack.org/#/c/580742/2/nova/api/metadata/base.py@597 | |
| 21:07:49 | dansmith | and the idea is that until the release happens, it's not really codified as the date-based version, | |
| 21:07:51 | dansmith | so we can add stuff to it | |
| 21:07:56 | mriedem | yup, | |
| 21:07:58 | mriedem | found that code | |
| 21:10:17 | melwitt | a-ha, cool. thanks for all that info | |
| 21:12:49 | melwitt | fyi mgagne ^ (backscroll for more context on the latest review comment) | |
| 21:13:46 | mgagne | melwitt: so no new metadata api version and call it a day? | |
| 21:14:28 | melwitt | mgagne: yeah, the code mriedem highlighted on the review will hide version dates that are in the future, so that they stay 'unreleased' until the release date. so you can roll your changes into the existing ROCKY version | |
| 21:14:35 | mgagne | +1 | |
| 21:16:38 | openstackgerrit | Mathieu Gagné proposed openstack/nova master: Add support for multiple fixed-ips in metadata https://review.openstack.org/580742 | |
| 21:16:43 | artom | mriedem, yeah, am looking into it | |
| 21:16:46 | artom | Really weird | |
| 21:18:24 | mriedem | dansmith: so does that mean we're good with the move patch for my +2 since we have the functional WIP patch for the move ops? | |
| 21:18:27 | artom | mriedem, no chance we can merge and later revert really excessive logging? | |
| 21:18:35 | mriedem | like, are you ok approving the stack and cleaning up the functional patch test later or? | |