opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack stable/1.0.0: Release 1.10.0 version https://review.opendev.org/c/openstack/ansible-collections-openstack/+/860223 | 09:07 |
---|---|---|
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Refactored floating_ip and floating_ip_info modules https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828613 | 09:11 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Replaced expensive get_server() and fixed issues in server module https://review.opendev.org/c/openstack/ansible-collections-openstack/+/852119 | 09:57 |
jm1 | gtema: o/ running into yet another issue with add_ips_to_server and i need your input on how to solve this. | 12:20 |
gtema | ok, I hear | 12:20 |
jm1 | gtema: suppose you have two private networks, a server which is attached to one of these networks and now you call add_ips_to_server(server) without any other arguments, hence auto_ip=True | 12:20 |
jm1 | gtema: you also have a router and a public network and neutron | 12:21 |
jm1 | gtema: i would assume that add_ips_to_server would attach a floating ip to the server | 12:21 |
jm1 | gtema: but actually it will fail here https://opendev.org/openstack/openstacksdk/src/commit/093e71e5a6050e6f7d47a03b5f03a62b8f60ca60/openstack/cloud/_floating_ip.py#L1129 | 12:22 |
gtema | jm1 - generally in any way this can work only: the net server is attached to is connected through router with public network | 12:22 |
jm1 | gtema: sure, the router is attached to public network and the private networks | 12:23 |
gtema | the place you mention is exactly one of the black holes | 12:23 |
jm1 | gtema: we also have subnets | 12:23 |
jm1 | gtema: the issue is, that this line here might return the "wrong" network https://opendev.org/openstack/openstacksdk/src/commit/093e71e5a6050e6f7d47a03b5f03a62b8f60ca60/openstack/cloud/_floating_ip.py#L1109 | 12:23 |
jm1 | gtema: it might return the one the server is connected to, but it could also return the other one | 12:24 |
gtema | because historically it searches for this "NAT network" which should be a private net server is connected to and connected through router to public net | 12:24 |
gtema | do you have logs (or how to reproduce? | 12:25 |
jm1 | gtema: https://6aef0912e1e7e067f94c-3957ec525977d9a3771da3eb17c87a1d.ssl.cf5.rackcdn.com/852119/18/check/ansible-collections-openstack-functional-devstack/593fd7f/job-output.txt | 12:25 |
jm1 | gtema: here is the culprit: https://opendev.org/openstack/openstacksdk/src/commit/093e71e5a6050e6f7d47a03b5f03a62b8f60ca60/openstack/cloud/_network_common.py#L180 | 12:26 |
jm1 | gtema: this will look though all subnets which will be returned even though the server might not be attached to that | 12:26 |
gtema | which which change it is (so that I reconstruct conditions)? | 12:26 |
jm1 | gtema: https://review.opendev.org/c/openstack/ansible-collections-openstack/+/852119/18 | 12:27 |
jm1 | gtema: my approach would be to return all possible nat destinations and let the caller choose what the best is | 12:27 |
gtema | actually there is clouds.yaml parameter for manual name of such net | 12:28 |
jm1 | gtema: but _nat_destination_port() can decide what nat destination to use without the user having to help the sdk | 12:29 |
gtema | I told you this is now all quite historical and not too much scalable, haven't I? | 12:30 |
jm1 | gtema: _nat_destination_port gets a server and has a list of ports. it just has to check which port is actually on a nat_destination network | 12:30 |
jm1 | gtema: hmm | 12:30 |
jm1 | gtema: so ignore? | 12:30 |
gtema | nope, I want to have a functest for that directly in SDK | 12:30 |
jm1 | gtema: the use case is pretty straight forward though: one has a server and wants it to have a floating ip. this should not fail just because the user has two networks with a gateway.. | 12:31 |
gtema | https://review.opendev.org/q/topic:better_tests is a chain where this would need to go | 12:31 |
gtema | this is clear, jm1. But I would need to create a proper functest for that | 12:31 |
jm1 | gtema: sure. question is, should i write a patch for this _nat_destination_port or is this going away anyway? | 12:32 |
gtema | no, I would first want to have a test that reproduces error and with that start reworking of the whole madness | 12:33 |
jm1 | gtema: ok sooo.. should i write a test? | 12:34 |
gtema | just describe to me detailed conditions | 12:35 |
jm1 | gtema: here or in a bug report? | 12:36 |
gtema | doesn't matter - even an email to me. Important is that I do not loose it | 12:36 |
gtema | and bugreport is exactly the place to loose everything once you close the tab | 12:36 |
jm1 | gtema: ok. devstack default setup with public network. 2 private networks with a subnet each. all three networks attached to the router. server attached to one private network. call to add_ips_to_server(server, auto_ip=True, reuse=False). sometimes fails with "No port on server... was found matching your NAT destination network" because _nat_destination_port() returns the other private network, not the one the server has been attached to. | 12:44 |
jm1 | gtema: plus links above. enough? | 12:44 |
gtema | oh, so actually all private nets attached to one router? that is a great fact to know | 12:46 |
jm1 | gtema: yes. although this would not change the bug because it lists all subnets anyway | 12:49 |
jm1 | gtema: have more than one network with a gateway? boom | 12:51 |
jm1 | gtema: or rather "potential boom" | 12:51 |
gtema | where is the connectivity between pub net and router? don't see it in the change | 12:52 |
jm1 | gtema: https://review.opendev.org/c/openstack/ansible-collections-openstack/+/852119/18/ci/roles/server/tasks/main.yml#37 | 12:53 |
gtema | ah, overseen | 12:53 |
gtema | ok, will sketch the test | 12:53 |
jm1 | gtema: while you are at it.. https://storyboard.openstack.org/#!/story/2010153 ;) | 12:56 |
jm1 | gtema: he also submitted a patch | 12:56 |
gtema | ugh, I don't know how to deal with patches into something that is so fragile | 12:57 |
jm1 | gtema: this patch actually looks good. it does not remove or change functionality, it just adds a missing parameter | 12:59 |
gtema | which works "magically" | 12:59 |
gtema | there are no really reasonable tests for this whole "chapter" | 13:00 |
jm1 | gtema: so you cannot break something with merging this | 13:00 |
gtema | lol - I can break you even further | 13:00 |
jm1 | gtema: not with this patch though ;) | 13:02 |
gtema | sure? | 13:02 |
jm1 | gtema: not more than what is broken already | 13:03 |
gtema | it doesn't look bad, I am just too skeptic since there are no real tests here | 13:03 |
gtema | okay, then I can go with merging it and restructure with it being added | 13:03 |
opendevreview | Artem Goncharov proposed openstack/openstacksdk master: support nat_destination when attaching existing floating_ip to a server https://review.opendev.org/c/openstack/openstacksdk/+/850115 | 13:06 |
jm1 | gtema: i know you hate storyboard but i need a place to track that error https://storyboard.openstack.org/#!/story/2010352 | 13:18 |
gtema | I do not hate it - I do not use it ;-) | 13:19 |
gtema | because it does not help, but rather disturb | 13:19 |
* jm1 gtema has been kicked due to inactivity on floating ips :P | 13:44 | |
gtema | really? okay, then I just don't push any other big changes ;-) | 13:45 |
opendevreview | Merged openstack/ansible-collections-openstack stable/1.0.0: Release 1.10.0 version https://review.opendev.org/c/openstack/ansible-collections-openstack/+/860223 | 13:56 |
gtema | you know jm1, while now reworking tests to be working in regular clouds faced already something fun errors ("Multiple possible networks found, use a Network ID to be more specific.") | 14:35 |
jm1 | gtema: i am feeling you 😬 | 14:39 |
opendevreview | Artem Goncharov proposed openstack/openstacksdk master: Whitelist cloud functional tests in acceptance https://review.opendev.org/c/openstack/openstacksdk/+/860009 | 15:19 |
opendevreview | Merged openstack/cliff master: Replace abc.abstractproperty with property and abc.abstractmethod https://review.opendev.org/c/openstack/cliff/+/852058 | 18:15 |
opendevreview | Merged openstack/cliff master: columns: Useful __str__, __repr__ implementation https://review.opendev.org/c/openstack/cliff/+/858550 | 18:15 |
opendevreview | Jakob Meng proposed openstack/ansible-collections-openstack master: Replaced expensive get_server() and fixed issues in server module https://review.opendev.org/c/openstack/ansible-collections-openstack/+/852119 | 19:05 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!