| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2019-10-04 | |||
| 14:26:24 | mordred | dtroyer: is this https://762ea4c4c4fb203eceec-1a0f674a3837e35101e72ddbc77fd8ba.ssl.cf2.rackcdn.com/686589/1/check/osc-functional-devstack-tips/de578de/testr_results.html.gz | |
| 14:26:33 | mordred | something we should be worried about and/or know about already? | |
| 14:37:49 | dtroyer | mordred: I do not recall seeing that before. volume tests are historically timing sensitive but we've eliminated most of them (I think). Thats a compute test but volumes are involved. Have you seen more than one of these? | |
| 14:43:29 | mordred | dtroyer: no - I should probably just recheck then | |
| 15:02:44 | efried | mordred: reviewing that test change, I wonder if has_service ought to return the endpoint(_data) so we don't wind up duplicating calls... | |
| 15:03:49 | mordred | efried: well - the reason to change to has_service is that we don't actually need to do the full endpoint_data call (which results in a discovery api call) in a lot of cases | |
| 15:04:03 | openstackgerrit | Merged openstack/os-client-config master: Update the constraints url https://review.opendev.org/685581 | |
| 15:04:04 | efried | yah, I get that | |
| 15:04:11 | mordred | oh - but ... maybe make has_service also take a mv arg? | |
| 15:04:33 | efried | I'm looking for usages of has_service to see if any of the are directly followed by something that does discovery | |
| 15:04:42 | efried | I would think that in most cases the discovery is cached already one way or another. | |
| 15:04:43 | mordred | nod | |
| 15:04:48 | mordred | yeah | |
| 15:04:54 | mordred | once it's done - it should be cached | |
| 15:05:25 | efried | yeah, nothing jumps out. Probably n/a then. Ignore me. | |
| 15:15:34 | efried | mordred: I think the dep chain might be busted. Care to join me in -requirements ? | |
| 15:30:12 | openstackgerrit | Monty Taylor proposed openstack/keystoneauth master: Fetch discovery documents with auth when needed https://review.opendev.org/685042 | |
| 15:40:21 | openstackgerrit | Monty Taylor proposed openstack/keystoneauth master: Fetch discovery documents with auth when needed https://review.opendev.org/685042 | |
| 15:40:45 | efried | wfm, thanks mordred | |
| 15:42:32 | efried | dangit, where tf is the cloud data sdk tests with? | |
| 15:43:57 | efried | k, found the discovery documents. Looking for the flavors it loads up... | |
| 15:44:10 | mordred | efried: right there! | |
| 15:45:41 | efried | mordred: Sorry, help please, still not seeing where flavor data comes from | |
| 15:45:54 | efried | i.e. self.flavor here https://review.opendev.org/#/c/685999/7/openstack/tests/functional/cloud/test_inventory.py | |
| 15:47:38 | efried | mebbe openstack.tests.fakes.make_fake_flavor ? | |
| 15:47:52 | mordred | no - that's a functional test | |
| 15:48:03 | mordred | it's the flavor that we get find from devstack | |
| 15:48:22 | mordred | efried: from openstack.tests.functional.cloud.util import pick_flavor | |
| 15:48:42 | efried | ...from devstack? | |
| 15:49:09 | efried | Are these "functional" tests running in a devstack?? | |
| 15:49:43 | mordred | efried: yes. that's how the sdk functional tests work. | |
| 15:49:45 | mordred | https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/tests/functional/cloud/util.py#L23-L43 | |
| 15:50:01 | mordred | efried: the idea is that we point sdk at the devstack and then we use that devstack as a remote cloud | |
| 15:50:09 | efried | whoah. | |
| 15:50:14 | efried | I mean, cool, but... whoah. | |
| 15:50:22 | mordred | :) | |
| 15:50:28 | mordred | it's a fun set of tests | |
| 15:50:35 | efried | so what I don't get is why the deltas in that file are necessary/appropriate. | |
| 15:51:19 | mordred | because what the ksa patch fixed was microversion detection - so we now correctly detect the available microversion, which means we're now fetching Server at the mv declared in the server resource | |
| 15:51:26 | mordred | which means we get the new form of flavor now | |
| 15:51:39 | mordred | this change SHOULD have gone into the change last april that bumped our microversion | |
| 15:51:59 | efried | okay, the test is way more magic than I was seeing | |
| 15:52:01 | efried | thanks. | |
| 15:52:02 | mordred | but we didn;t notice a that point that the behavior changed because the mv discovery was silently broke | |
| 15:52:46 | mordred | it's actually super un-magic- it's just actually creating a server on an openstack and checking that it works. easy peasy right? | |
| 15:54:05 | efried | mordred: next point of confusion... | |
| 15:54:12 | efried | the references to 'host' in this test code... | |
| 15:54:18 | efried | are those actualy *instances*?? | |
| 15:54:36 | efried | based on OpenStackInventory.list_hosts, it... kinda looks like they might be | |
| 15:54:49 | efried | because that's doing compute.list_servers | |
| 15:55:43 | mordred | yes indeed | |
| 15:55:46 | mordred | those are instances | |
| 15:56:06 | efried | what do we call hosts?? | |
| 15:56:13 | mordred | the word is host here because this is a file in support of ansible dynamic inventory | |
| 15:56:41 | efried | tribal. knowledge. | |
| 15:57:08 | mordred | but in this file a "host" is a record describing how to connect to a remote compute - and in this case those remote computers are vms managed by openstack with nova | |
| 15:57:32 | efried | wow | |
| 15:57:33 | efried | neat | |
| 15:57:34 | efried | mordred: so given all that, I dropped a question in https://review.opendev.org/#/c/685999/ for a moar bettr test. LMK if you agree. | |
| 16:01:58 | cmorpheus | i guess i need to do things? | |
| 16:02:49 | efried | cmorpheus: I think you're just on the hook for this un: https://review.opendev.org/#/c/685042/ | |
| 16:03:10 | cmorpheus | mmk | |
| 16:03:22 | efried | aside from zuul, both gtema and I have tested it locally. | |
| 16:08:28 | cmorpheus | so that one depends-on https://review.opendev.org/685999 is there a tldr why that's okay? that makes it seem very much backwards incompatible | |
| 16:12:08 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Fix server for later microversion https://review.opendev.org/685999 | |
| 16:12:33 | mordred | cmorpheus: the tl;dr is that the fix to ksa exposes a bug in sdk | |
| 16:12:43 | mordred | so we need to land the bugfix in sdk and release it first | |
| 16:14:27 | mordred | efried: o - you know what - I agreed with your comment - but then really I'm wrong in my latest iteration on that - because removing the id references is important - the main new thing is that their existence is now undefinable | |
| 16:14:32 | mordred | let me do one more quick respin | |
| 16:15:15 | efried | mordred: oh, you mean because the id will exist iff the original flavor hasn't been deleted? | |
| 16:20:15 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Fix server for later microversion https://review.opendev.org/685999 | |
| 16:20:16 | openstackgerrit | Monty Taylor proposed openstack/openstacksdk master: Re-add functional tests on flavor content https://review.opendev.org/686765 | |
| 16:20:39 | mordred | efried: no - I mean until ksa releases we're still on the old mv | |
| 16:20:54 | efried | ugh | |
| 16:21:11 | efried | but... that's okay because of the dep? | |
| 16:21:13 | mordred | efried: so we can neither assert id is in or not in the flavor record - because of the underlying mv shift - it's impossible to have a consistent payload | |
| 16:21:14 | efried | oh | |
| 16:21:19 | efried | the dep is backwards | |
| 16:21:20 | mordred | like - it's impossible in the real world | |
| 16:21:44 | efried | can we tell whath mv we used? | |
| 16:21:54 | efried | (not that I think we necessarily should, just wondering) | |
| 16:22:20 | mordred | maybe? actually - I don't think there is a good way to do that right now | |
| 16:22:27 | mordred | but - we could potentially cook something up | |
| 16:24:29 | efried | mordred: so now we actually *do* want a l-c bump from sdk to ksa when the latter is released, so we can ... oic you did that in the fup patch ++ | |
| 16:26:48 | mordred | cmorpheus: responded - my answer is sort of an existential one ... | |
| 16:27:08 | mordred | efried: yeah. perhaps that followup makes the weird sequence a little clearer - or not :) | |
| 16:27:49 | efried | mordred: +2s on the stuff we can do now. -1 to get matching l-c on that fup. Also wouldn't be a bad idea to make the commit message voice less future-y, since in git history it's going to be past-y. | |
| 16:28:24 | mordred | efried: yeah - I'll respin it once ksa releases - and will fix the l-c thing then too | |
| 16:28:30 | efried | ++ | |
| 16:28:31 | mordred | agree on the voice | |
| 16:29:04 | efried | Thanks for working through this mess mordred | |
| 16:30:31 | cmorpheus | mordred: lol | |
| 16:32:34 | mordred | efried: likewise. this would have bit someone more unexpectedly at some point - at least now it comes with a release note | |
| 16:33:09 | cmorpheus | mordred: are you wanting to release both of these today? today as in friday? friday the week before final RCs? | |
| 16:33:28 | mordred | cmorpheus: no - I think we figured out earlier today that we do not need to rush a fix out the door | |
| 16:33:40 | cmorpheus | okay awesome | |
| 16:33:41 | mordred | they've got a workaround for it in cyborg | |
| 16:33:53 | mordred | so I'm thinking point release post release :) | |
| 16:34:30 | efried | whereupon the version numbers in the renos will need to be changed in the backports | |
| 16:36:35 | openstackgerrit | Monty Taylor proposed openstack/keystoneauth master: Fetch discovery documents with auth when needed https://review.opendev.org/685042 | |