)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"6bddcf024f68eaaa52e689272f4a5282dd71a608","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"62ed5a3e_718d6402","updated":"2023-03-20 11:51:16.000000000","message":"This needs a functional test to prove it works and avoid regressions. It should relatively easy to add one to [1]. Please ask if you need guidance.\n\n[1] openstack/tests/functional/cloud/test_floating_ip.py","commit_id":"d87ac457ec0aba95023e5e9468e0a58dfc390bcf"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"950120338f43cec39562c5b60346be43594e6596","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1f7ba1e3_74c021a7","updated":"2023-03-10 09:51:19.000000000","message":"recheck","commit_id":"d87ac457ec0aba95023e5e9468e0a58dfc390bcf"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"f676abb9c1aed78e13c626a8a31810a4038690d6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e7268b8f_a66b7e5b","updated":"2023-03-03 07:36:47.000000000","message":"unrelated openstack.tests.functional.load_balancer.v2.test_load_balancer.TestLoadBalancer failed","commit_id":"d87ac457ec0aba95023e5e9468e0a58dfc390bcf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"062db2b82bef5e967b178b86accf94d320704016","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e4ee20ae_317db7f8","in_reply_to":"62ed5a3e_718d6402","updated":"2023-03-20 11:52:31.000000000","message":"To be clear, you probably want to test \u0027available_floating_ip\u0027 rather than the inner function...","commit_id":"d87ac457ec0aba95023e5e9468e0a58dfc390bcf"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"c83ba790fce0686807fbe95c413a53769ec7d2df","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"77e767e0_63e98d15","in_reply_to":"e4ee20ae_317db7f8","updated":"2023-04-20 08:49:54.000000000","message":"Done","commit_id":"d87ac457ec0aba95023e5e9468e0a58dfc390bcf"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4f33d99f6d365051776d5bce722e7ec2866cd0e8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2fb4a4da_00de0c36","updated":"2023-05-25 09:45:00.000000000","message":"I missed something","commit_id":"fa6ac8e2cfce79058e11ab61d26bae943a3261db"},{"author":{"_account_id":8745,"name":"Lars Kellogg-Stedman","email":"lars@redhat.com","username":"lars"},"change_message_id":"da94c965424a2b9f31a576afd6c9b840bc9a0ea7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a63aa5d2_734e855c","updated":"2023-05-25 03:44:37.000000000","message":"I ran into exactly this issue in an environmen with strict quotas on floating ips, and this fix is exactly the fix necessary to allow available_floating_ips to reuse ips rather than always allocating new ones.","commit_id":"fa6ac8e2cfce79058e11ab61d26bae943a3261db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ece2e6d33014b4441a38744e86f17199b363d43f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4f22bfb0_d8a0b465","updated":"2023-05-25 12:05:03.000000000","message":"Fixed the issue and added follow-ups to prevent it recurring","commit_id":"dc52fd4978424ae9f55b3a602572dce6a291407b"},{"author":{"_account_id":8745,"name":"Lars Kellogg-Stedman","email":"lars@redhat.com","username":"lars"},"change_message_id":"f7acfdd8bbaa1fbe0596d3799ec441f2c436f024","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"2e4f4d54_e5546191","updated":"2023-05-26 20:03:23.000000000","message":"Oops, I ran into a problem using this in practice -- as written, `available_floating_ip` appears to search both free **and assigned** floating ips, so it *stole* a floating ip that was already assigned to one server. I *think* this is not the intended behavior.\n\n\nA reproducer for that problem looks like:\n\n```\nimport time\nimport openstack\n\nconn \u003d openstack.connect()\n\nservers \u003d []\nfor servername in [\"test1\", \"test2\"]:\n    print(f\"create server {servername}\")\n    servers.append(\n        conn.create_server(\n            name\u003dservername,\n            flavor\u003d\"mem-a.1\",\n            image\u003d\"fedora-38-x86_64\",\n            auto_ip\u003dFalse,\n            wait\u003dTrue,\n        )\n    )\n\ntry:\n    f1 \u003d conn.available_floating_ip()\n    if not f1:\n        raise RuntimeError(\"failed to acquire floating ip\")\n    print(f\"adding {f1.floating_ip_address} to server {servers[0].name}\")\n    conn.add_ips_to_server(servers[0], ips\u003d[f1.floating_ip_address])\n\n    while True:\n        server \u003d conn.get_server_by_id(servers[0].id)\n        if server.public_v4:\n            break\n\n    f2 \u003d conn.available_floating_ip()\n    if not f2:\n        raise RuntimeError(\"failed to acquire floating ip\")\n    if f2.floating_ip_address \u003d\u003d f1.floating_ip_address:\n        print(\"Oh, no! Existing address was re-assigned!\")\n\n    print(f\"adding {f1.floating_ip_address} to server {servers[1].name}\")\n    conn.add_ips_to_server(servers[1], ips\u003d[f2.floating_ip_address])\n    while True:\n        server \u003d conn.get_server_by_id(servers[1].id)\n        if server.public_v4:\n            break\n\n    print(f\"waiting for server {servers[0].name} to lose public ip\")\n    for i in range(10):\n        server \u003d conn.get_server_by_id(servers[0].id)\n        assert server.public_v4 !\u003d \"\", f\"server {server.name} lost its floating ip\"\n        time.sleep(1)\nfinally:\n    for server in servers:\n        conn.delete_server(server.id, delete_ips\u003dTrue)\n```\n\nRunning this will result in something like:\n\n```\ncreate server test1\n/home/lars/projects/jupyterhub-openstack-spawner/.venv/lib64/python3.11/site-packages/openstack/cloud/_floating_ip.py:68: UserWarning: search_floating_ips is deprecated. Use search_resource instead.\n  warnings.warn(\ncreate server test2\nadding 199.94.60.238 to server test1\nOh, no! Existing address was re-assigned!\nadding 199.94.60.238 to server test2\nwaiting for server test1 to lose public ip\nTraceback (most recent call last):\n  File \"/home/lars/projects/jupyterhub-openstack-spawner/repro.py\", line 47, in \u003cmodule\u003e\n    assert server.public_v4 !\u003d \"\", f\"server {server.name} lost its floating ip\"\n           ^^^^^^^^^^^^^^^^^^^^^^\nAssertionError: server test1 lost its floating ip\n```","commit_id":"dc52fd4978424ae9f55b3a602572dce6a291407b"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"7f3ca5f16a2a84c6db8895d0f871a14882c711fb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d115fe3b_8f25fd98","updated":"2023-05-26 08:53:40.000000000","message":"Thanks!","commit_id":"dc52fd4978424ae9f55b3a602572dce6a291407b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"2d88bb6275df28fb6fbd906ae80be82c42798ffb","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5dd07251_6fdcf1f6","in_reply_to":"2e4f4d54_e5546191","updated":"2023-05-29 09:37:42.000000000","message":"\u003e Oops, I ran into a problem using this in practice -- as written, `available_floating_ip` appears to search both free **and assigned** floating ips, so it *stole* a floating ip that was already assigned to one server. I *think* this is not the intended behavior.\n\nAre you certain you tested this with the latest revision? I\u0027m not able to get this to fail locally, whether I have 0, 1, 2 or more FIPs already created. I\u0027d be surprised if I could get it to fail also as we\u0027re filtering on `port_id`, which will be set the moment a FIP is assigned to an instance. We only fixed that in the latest revision, however.\n\nCan you double check that your local copy has replaced `port` with `port_id`?\n\nPS: There\u0027s a typo in the script below but it only affects the output. \n\n\u003e \n\u003e \n\u003e A reproducer for that problem looks like:\n\u003e \n\u003e ```\n\u003e import time\n\u003e import openstack\n\u003e \n\u003e conn \u003d openstack.connect()\n\u003e \n\u003e servers \u003d []\n\u003e for servername in [\"test1\", \"test2\"]:\n\u003e     print(f\"create server {servername}\")\n\u003e     servers.append(\n\u003e         conn.create_server(\n\u003e             name\u003dservername,\n\u003e             flavor\u003d\"mem-a.1\",\n\u003e             image\u003d\"fedora-38-x86_64\",\n\u003e             auto_ip\u003dFalse,\n\u003e             wait\u003dTrue,\n\u003e         )\n\u003e     )\n\u003e \n\u003e try:\n\u003e     f1 \u003d conn.available_floating_ip()\n\u003e     if not f1:\n\u003e         raise RuntimeError(\"failed to acquire floating ip\")\n\u003e     print(f\"adding {f1.floating_ip_address} to server {servers[0].name}\")\n\u003e     conn.add_ips_to_server(servers[0], ips\u003d[f1.floating_ip_address])\n\u003e \n\u003e     while True:\n\u003e         server \u003d conn.get_server_by_id(servers[0].id)\n\u003e         if server.public_v4:\n\u003e             break\n\u003e \n\u003e     f2 \u003d conn.available_floating_ip()\n\u003e     if not f2:\n\u003e         raise RuntimeError(\"failed to acquire floating ip\")\n\u003e     if f2.floating_ip_address \u003d\u003d f1.floating_ip_address:\n\u003e         print(\"Oh, no! Existing address was re-assigned!\")\n\u003e \n\u003e     print(f\"adding {f1.floating_ip_address} to server {servers[1].name}\")\n\u003e ```\n\nTypo here. Should read \u0027f2.floating_ip_address\u0027.\n\n\u003e ```\n\u003e     conn.add_ips_to_server(servers[1], ips\u003d[f2.floating_ip_address])\n\u003e     while True:\n\u003e         server \u003d conn.get_server_by_id(servers[1].id)\n\u003e         if server.public_v4:\n\u003e             break\n\u003e \n\u003e     print(f\"waiting for server {servers[0].name} to lose public ip\")\n\u003e     for i in range(10):\n\u003e         server \u003d conn.get_server_by_id(servers[0].id)\n\u003e         assert server.public_v4 !\u003d \"\", f\"server {server.name} lost its floating ip\"\n\u003e         time.sleep(1)\n\u003e finally:\n\u003e     for server in servers:\n\u003e         conn.delete_server(server.id, delete_ips\u003dTrue)\n\u003e ```\n\u003e \n\u003e Running this will result in something like:\n\u003e \n\u003e ```\n\u003e create server test1\n\u003e /home/lars/projects/jupyterhub-openstack-spawner/.venv/lib64/python3.11/site-packages/openstack/cloud/_floating_ip.py:68: UserWarning: search_floating_ips is deprecated. Use search_resource instead.\n\u003e   warnings.warn(\n\u003e create server test2\n\u003e adding 199.94.60.238 to server test1\n\u003e Oh, no! Existing address was re-assigned!\n\u003e adding 199.94.60.238 to server test2\n\u003e waiting for server test1 to lose public ip\n\u003e Traceback (most recent call last):\n\u003e   File \"/home/lars/projects/jupyterhub-openstack-spawner/repro.py\", line 47, in \u003cmodule\u003e\n\u003e     assert server.public_v4 !\u003d \"\", f\"server {server.name} lost its floating ip\"\n\u003e            ^^^^^^^^^^^^^^^^^^^^^^\n\u003e AssertionError: server test1 lost its floating ip\n\u003e ```","commit_id":"dc52fd4978424ae9f55b3a602572dce6a291407b"}],"openstack/cloud/_floating_ip.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4f33d99f6d365051776d5bce722e7ec2866cd0e8","unresolved":true,"context_lines":[{"line_number":278,"context_line":"            floating_network_id \u003d self._get_floating_network_id()"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        filters \u003d {"},{"line_number":281,"context_line":"            \u0027port\u0027: None,"},{"line_number":282,"context_line":"            \u0027floating_network_id\u0027: floating_network_id,"},{"line_number":283,"context_line":"            \u0027location\u0027: {\u0027project\u0027: {\u0027id\u0027: project_id}},"},{"line_number":284,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":2,"id":"13bf3f92_8b7027dc","line":281,"updated":"2023-05-25 09:45:00.000000000","message":"This needs to be updated to \u0027port_id\u0027 also, as seen at [1]. Testing this we can see it\u0027s also the case.\n\n    \u003e\u003e\u003e import openstack\n    \u003e\u003e\u003e conn \u003d openstack.connect()\n    \u003e\u003e\u003e fip \u003d conn._list_floating_ips()[0]  # I already have some FIPs\n    \u003e\u003e\u003e hasattr(fip, \u0027network\u0027)\n    False\n    \u003e\u003e\u003e hasattr(fip, \u0027floating_network_id\u0027)\n    True\n    \u003e\u003e\u003e hasattr(fip, \u0027port\u0027)\n    False\n    \u003e\u003e\u003e hasattr(fip, \u0027port_id\u0027)\n    True\n\n[1] https://review.opendev.org/c/openstack/openstacksdk/+/874540/1/openstack/cloud/_floating_ip.py#283","commit_id":"fa6ac8e2cfce79058e11ab61d26bae943a3261db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"941c6a2513aac3d317e6eb2fc2e1a712dd58b9a7","unresolved":false,"context_lines":[{"line_number":278,"context_line":"            floating_network_id \u003d self._get_floating_network_id()"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        filters \u003d {"},{"line_number":281,"context_line":"            \u0027port\u0027: None,"},{"line_number":282,"context_line":"            \u0027floating_network_id\u0027: floating_network_id,"},{"line_number":283,"context_line":"            \u0027location\u0027: {\u0027project\u0027: {\u0027id\u0027: project_id}},"},{"line_number":284,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":2,"id":"4d6e2933_479913b5","line":281,"in_reply_to":"13bf3f92_8b7027dc","updated":"2023-05-26 12:54:22.000000000","message":"Done","commit_id":"fa6ac8e2cfce79058e11ab61d26bae943a3261db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4f33d99f6d365051776d5bce722e7ec2866cd0e8","unresolved":true,"context_lines":[{"line_number":280,"context_line":"        filters \u003d {"},{"line_number":281,"context_line":"            \u0027port\u0027: None,"},{"line_number":282,"context_line":"            \u0027floating_network_id\u0027: floating_network_id,"},{"line_number":283,"context_line":"            \u0027location\u0027: {\u0027project\u0027: {\u0027id\u0027: project_id}},"},{"line_number":284,"context_line":"        }"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        floating_ips \u003d self._list_floating_ips()"}],"source_content_type":"text/x-python","patch_set":2,"id":"52a0ca64_532807bb","line":283,"updated":"2023-05-25 09:45:00.000000000","message":"Same as the above change, we can probably simplify this to \u0027project_id\u0027:\n\n    \u003e\u003e\u003e hasattr(fip, \u0027location\u0027)\n    True\n    \u003e\u003e\u003e hasattr(fip, \u0027project_id\u0027)\n    True","commit_id":"fa6ac8e2cfce79058e11ab61d26bae943a3261db"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"941c6a2513aac3d317e6eb2fc2e1a712dd58b9a7","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        filters \u003d {"},{"line_number":281,"context_line":"            \u0027port\u0027: None,"},{"line_number":282,"context_line":"            \u0027floating_network_id\u0027: floating_network_id,"},{"line_number":283,"context_line":"            \u0027location\u0027: {\u0027project\u0027: {\u0027id\u0027: project_id}},"},{"line_number":284,"context_line":"        }"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        floating_ips \u003d self._list_floating_ips()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7de9bf68_eed48f9f","line":283,"in_reply_to":"52a0ca64_532807bb","updated":"2023-05-26 12:54:22.000000000","message":"Done","commit_id":"fa6ac8e2cfce79058e11ab61d26bae943a3261db"}]}
