| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-07-28 | |||
| 19:03:38 | artom | I guess? | |
| 19:07:12 | lyarwood | artom: urgh this sucks | |
| 19:07:26 | artom | How so? | |
| 19:07:36 | lyarwood | artom: leaving it set to None breaks my use of loads of the helper methods from _IntegratedTestBase | |
| 19:07:45 | lyarwood | artom: _live_migrate etc | |
| 19:08:12 | sean-k-mooney | latest will break some tests | |
| 19:08:16 | lyarwood | feels weird writing functional tests against a version of the API we don't even support anymore tbh | |
| 19:08:30 | lyarwood | right so I've picked the max version in stable/queens | |
| 19:08:30 | sean-k-mooney | although those proably would be the ones testing older microverions | |
| 19:08:30 | artom | lyarwood, how does it break them? | |
| 19:08:47 | artom | lyarwood, my point is that microversio = 'latest' works just fine in queens | |
| 19:09:09 | sean-k-mooney | artom: what that technically means is use the latest version the nova client know about | |
| 19:09:21 | sean-k-mooney | not the latest version the api know about | |
| 19:09:23 | artom | sean-k-mooney, in func tests, so no novaclient, but yeah | |
| 19:09:25 | lyarwood | artom: latest in queens != latest in master | |
| 19:09:33 | artom | lyarwood, yep, so? | |
| 19:09:44 | artom | I fail to see the problem :) | |
| 19:10:02 | sean-k-mooney | lyarwood: artom means actully use "latest" not fine the latest microverion and use that number | |
| 19:10:16 | artom | Right, that | |
| 19:10:24 | sean-k-mooney | artom: does the "latest" string actully work in the api | |
| 19:10:30 | sean-k-mooney | i tought it was just in the client code | |
| 19:10:44 | artom | sean-k-mooney, https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/libvirt/test_numa_live_migration.py#L41 | |
| 19:10:53 | lyarwood | I'm writing this against master at the moment | |
| 19:11:01 | sean-k-mooney | cool so ya | |
| 19:11:21 | lyarwood | and I want to write it once and not have to update it with each backport as latest changes | |
| 19:11:30 | sean-k-mooney | microversion = 'latest' will be the latest version for any given release if you packport | |
| 19:11:59 | lyarwood | yeah but that's going to drop support for various things along the way meaning I'll need to constantly update the func test as I backport | |
| 19:12:16 | lyarwood | like the create args change loads between master and queens | |
| 19:12:19 | gmann | yes, 'latest' keyword is supported in API too | |
| 19:12:23 | lyarwood | networks for example | |
| 19:12:28 | sean-k-mooney | well you will have to do that anyway right | |
| 19:12:38 | gmann | it is api_version.max_api_version() | |
| 19:12:41 | lyarwood | not if I just picked the latest in queens when landing it in master | |
| 19:12:46 | artom | lyarwood, yeah, if you write your logic against APIs that only exist in U and up... | |
| 19:12:54 | artom | But I didn't see anything like that that jumped out | |
| 19:13:21 | sean-k-mooney | lyarwood: you could do that but you proably should also have a test for master then no? | |
| 19:13:47 | sean-k-mooney | what were you trying to test by the way | |
| 19:13:56 | lyarwood | nothing microversion specific | |
| 19:14:02 | lyarwood | this is just to keep the setup code sane | |
| 19:14:09 | lyarwood | and stop chrun in the backports | |
| 19:14:13 | sean-k-mooney | then use 2.1 if you want | |
| 19:14:33 | lyarwood | then I'm writing tests against a version of the API we don't even support anymore | |
| 19:15:20 | sean-k-mooney | we do support it because of microversion but my point was more dont set a microverison and only popluate the minium filed in the create request you need | |
| 19:15:49 | lyarwood | right sorry I mean with microversion = None | |
| 19:15:51 | sean-k-mooney | we teachnicaly have to support very micoversion untill we go to nova v4 | |
| 19:16:02 | lyarwood | yeah true | |
| 19:16:20 | sean-k-mooney | microversion = None is the same as 2.1 | |
| 19:16:22 | sean-k-mooney | i think | |
| 19:16:23 | gmann | functional tests are with 'latest' by default i think doing with 'latest' make sense | |
| 19:16:48 | lyarwood | gmann: it's set to None in _IntegratedTestBase | |
| 19:16:54 | sean-k-mooney | gmann: that is how i have always written them unless i was testing a feature | |
| 19:17:12 | artom | gmann, I think they're none, if I have the right code in front of me: https://opendev.org/openstack/nova/src/branch/master/nova/tests/functional/api/client.py#L135 | |
| 19:17:17 | sean-k-mooney | in which case i might use the microverion it was added in | |
| 19:18:00 | sean-k-mooney | lyarwood: is _IntegratedTestBase the new base of the functional test or the old one that we should not use anymore | |
| 19:18:18 | sean-k-mooney | there are a few baseclasses we can use in the funcational tests | |
| 19:18:31 | lyarwood | sean-k-mooney: the former I think, stephenfin is talking about replacing ProviderUsageBaseTestCase with it | |
| 19:18:53 | sean-k-mooney | waith isnte ProviderUsageBaseTestCase the thing we were ment to be moving too | |
| 19:18:59 | lyarwood | https://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L1062-L1063 | |
| 19:22:12 | gmann | ah right, it is in ProviderUsageBaseTestCase not base and many tests side too | |
| 19:22:16 | sean-k-mooney | oh sorry _IntegratedTestBase is not the one with the weird implematnion of _wait_for_server_allocations | |
| 19:22:39 | artom | sean-k-mooney, that got fixed a while ago | |
| 19:22:46 | artom | stephenfin unified all the things | |
| 19:22:53 | sean-k-mooney | ya i rememebr | |
| 19:23:04 | sean-k-mooney | i just was not sure if we still had some remnetes | |
| 19:23:46 | openstackgerrit | Lee Yarwood proposed openstack/nova master: func: Add live migration rollback volume attachment tests https://review.opendev.org/743534 | |
| 19:23:48 | openstackgerrit | Lee Yarwood proposed openstack/nova master: func: Add CinderFixture to _IntegratedTestBase https://review.opendev.org/743535 | |
| 19:23:48 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Don't delete the original attachment during pre LM rollback https://review.opendev.org/743319 | |
| 19:23:48 | openstack | bug 1889108 in OpenStack Compute (nova) "failures during driver.pre_live_migration remove source attachments during rollback" [High,In progress] https://launchpad.net/bugs/1889108 - Assigned to Lee Yarwood (lyarwood) | |
| 19:23:48 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Add regression tests for bug #1889108 https://review.opendev.org/743289 | |
| 19:23:50 | sean-k-mooney | what is confution me is that _IntegratedTestBase has a preceding underscore | |
| 19:24:02 | sean-k-mooney | and i tought we were not ment to be using it directly anymore | |
| 19:24:13 | sean-k-mooney | ane moving to the public base class | |
| 19:25:09 | sean-k-mooney | oh that todo was added 2 weeks ago https://github.com/openstack/nova/commit/c60f90cb2f8e2a30ffee5abeebd78f9eee3c6b25 | |
| 19:26:57 | sean-k-mooney | so ya one of the main deltas right now is microversion = None in _IntegratedTestBase and microversion = 'latest' in ProviderUsageBaseTestCase | |
| 19:27:29 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L978 vs https://github.com/openstack/nova/blob/master/nova/tests/functional/integrated_helpers.py#L1075 | |
| 19:29:55 | gmann | both are used in same amount so it is like half of functional tests run with 'None' and half with 'latest' | |
| 19:30:33 | sean-k-mooney | many other set it expcitly | |
| 19:31:14 | sean-k-mooney | or dont use either | |
| 19:31:40 | sean-k-mooney | the last regression i wrote did not use either of the base classes since the kept getting rewritten | |
| 19:31:43 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_1835822.py#L22-L23 | |
| 19:31:59 | gmann | running with 'latest' can give better coverage of latest code set and capture if something effecting functionality in latest (as new version on top of old one)code path | |
| 19:32:37 | sean-k-mooney | i think latest is generally more corerct unless you are testing older version explcitly | |
| 19:32:46 | sean-k-mooney | or testing something that is not version related | |
| 19:33:00 | sean-k-mooney | in which can none and latest are qually valid | |
| 19:33:17 | sean-k-mooney | if you use none it wont catch if a new cahnge alters the behavior | |
| 19:33:53 | sean-k-mooney | well it will if you alter it without actounting for microverison i guess | |
| 19:34:32 | gmann | correct. | |
| 20:52:08 | openstackgerrit | Merged openstack/python-novaclient master: Add a cleanup for a server in a functional test https://review.opendev.org/743589 | |
| 23:47:33 | brinzhang | gmann: we already introduced microversion in Ussuri, this is the patch https://review.opendev.org/#/c/696860/ | |
| 23:48:19 | brinzhang | gmann: I know this is still very simple and needs further optimization. If you have better suggestions, we are looking forward to it | |
| 23:52:50 | brinzhang | gmann: you are an expert in api, you can do the api change in cyborg too, and we will be very happy to see you improve this ^ | |
| #openstack-nova - 2020-07-29 | |||
| 01:56:50 | openstackgerrit | Tony Su proposed openstack/nova master: Provider Config File: YAML file loading and schema validation https://review.opendev.org/673341 | |
| 08:01:11 | tony_su | stephenfin: really appreciate your detailed review and I uploaded the latest patch reflecting the latest comments. | |
| 09:15:23 | lyarwood | stephenfin: https://review.opendev.org/#/q/topic:bug/1889108 updated for your reviewing pleasure btw | |
| 09:15:30 | stephenfin | ta | |
| 09:21:24 | stephenfin | melwitt, dansmith: Haven't investigated yet, but nova-ceph-multistore is failing again on some jobs. Could be nothing but just FYI https://review.opendev.org/#/c/673341/54 | |
| 09:25:34 | lyarwood | https://zuul.opendev.org/t/openstack/build/0709ecfb37c54cb8813afb42aefe9097/log/controller/logs/screen-g-api.txt#12286 | |
| 09:25:49 | lyarwood | rbd.OSError: [errno 116] error removing image | |
| 09:26:05 | lyarwood | [errno 2] error removing snapshot b'snap' from b'39db07f4-d56b-47f2-b7c2-81d3d4e8707e' Snapshot does not exist. | |
| 09:26:09 | lyarwood | that's a better error | |