)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"8539628240e30d0c2bd146bbd6812c01a168478c","unresolved":false,"context_lines":[{"line_number":19,"context_line":"could delete all the related resources in one API call and doesn\u0027t"},{"line_number":20,"context_line":"involve communication between Octavia controller and amphora instance."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"This patch deletes the api/etcd load balancers (if applicable) before"},{"line_number":23,"context_line":"deleting Heat stack, making the cluster deletion process more robust."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"[1]: https://docs.openstack.org/api-ref/load-balancer/v2/index.html?expanded\u003dremove-a-load-balancer-detail#remove-a-load-balancer"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"df33271e_cefeab69","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":69},"updated":"2020-04-02 11:02:07.000000000","message":"This is because we don\u0027t trust heat will do it and it will get stuck?","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"7fad0119d6f7396c10a25bf153d8c4c7e9c96a35","unresolved":false,"context_lines":[{"line_number":19,"context_line":"could delete all the related resources in one API call and doesn\u0027t"},{"line_number":20,"context_line":"involve communication between Octavia controller and amphora instance."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"This patch deletes the api/etcd load balancers (if applicable) before"},{"line_number":23,"context_line":"deleting Heat stack, making the cluster deletion process more robust."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"[1]: https://docs.openstack.org/api-ref/load-balancer/v2/index.html?expanded\u003dremove-a-load-balancer-detail#remove-a-load-balancer"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"df33271e_83ee0c89","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":69},"in_reply_to":"df33271e_59d5391b","updated":"2020-04-04 17:32:23.000000000","message":"I\u0027m ok with deleting the LBs in magnum. Although it feels more like a requirement a patched magnum might have, which is not using heat to deploy the control-plane.\n\nPlease make this opt-in. Deployments that use heat only will rely on this. Other magnum drivers can use this new mechanism.","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"30562e1d34e15fccab943d8a8c89553d18e6760a","unresolved":false,"context_lines":[{"line_number":19,"context_line":"could delete all the related resources in one API call and doesn\u0027t"},{"line_number":20,"context_line":"involve communication between Octavia controller and amphora instance."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"This patch deletes the api/etcd load balancers (if applicable) before"},{"line_number":23,"context_line":"deleting Heat stack, making the cluster deletion process more robust."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"[1]: https://docs.openstack.org/api-ref/load-balancer/v2/index.html?expanded\u003dremove-a-load-balancer-detail#remove-a-load-balancer"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"df33271e_c3d5a645","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":69},"in_reply_to":"df33271e_83ee0c89","updated":"2020-04-05 19:16:56.000000000","message":"\u003e I\u0027m ok with deleting the LBs in magnum. Although it feels more like\n \u003e a requirement a patched magnum might have, which is not using heat\n \u003e to deploy the control-plane.\n\nI was terribly wrong. With this change, magnum queries heat to get the load balancers.\nhttps://review.opendev.org/#/c/716930/1/magnum/common/octavia.py@100\n\nI am really sorry, my apologies!\n\n \u003e \n \u003e Please make this opt-in. Deployments that use heat only will rely\n \u003e on this. Other magnum drivers can use this new mechanism.\n\nIt will still be a good idea to make this opt-in. The Octavia API is implemented by Tungsten Fabric but cascade delete is not supported yet.\n\nPlease move ahead if it improves the API calls. Less stress in the APIs is more than welcome for every cloud using Octavia.","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"7cd303c5f05f6269c2066824cb586c7c95198c8b","unresolved":false,"context_lines":[{"line_number":19,"context_line":"could delete all the related resources in one API call and doesn\u0027t"},{"line_number":20,"context_line":"involve communication between Octavia controller and amphora instance."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"This patch deletes the api/etcd load balancers (if applicable) before"},{"line_number":23,"context_line":"deleting Heat stack, making the cluster deletion process more robust."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"[1]: https://docs.openstack.org/api-ref/load-balancer/v2/index.html?expanded\u003dremove-a-load-balancer-detail#remove-a-load-balancer"},{"line_number":26,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"df33271e_59d5391b","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":69},"in_reply_to":"df33271e_cefeab69","updated":"2020-04-02 21:39:20.000000000","message":"We trust Heat, but the problem is Heat is doing multiple API calls in order to delete a whole load balancer stack based on its implementation, that\u0027s the Heat mechanism we can\u0027t change.","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"4b8b3a109011ea660893a5b244f607d13a108f95","unresolved":false,"context_lines":[{"line_number":23,"context_line":"deleting Heat stack, making the cluster deletion process more robust."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"[1]: https://docs.openstack.org/api-ref/load-balancer/v2/index.html?expanded\u003dremove-a-load-balancer-detail#remove-a-load-balancer"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Change-Id: Ibe8f788559d0977475d0991fc99ad91ccfd7dca7"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"ff570b3c_7648c9cb","line":26,"updated":"2020-05-12 08:20:25.000000000","message":"story: 2007657\ntask: 39743","commit_id":"73459ebe537d6b5d1c933692fcb5035485e1b109"}],"magnum/common/neutron.py":[{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"62c7327f5176b2e855776e85641b9a0948dc0cd1","unresolved":false,"context_lines":[{"line_number":47,"context_line":"        id \u003d fips[\"floatingips\"][0][\"id\"]"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        if re.match(pattern, desc):"},{"line_number":50,"context_line":"            LOG.info(\"Deleting floating ip %s for cluster %s\", id,"},{"line_number":51,"context_line":"                     cluster.uuid)"},{"line_number":52,"context_line":"            n_client.delete_floatingip(id)"},{"line_number":53,"context_line":"    except Exception as e:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_2d754496","line":50,"updated":"2020-04-15 08:50:09.000000000","message":"+1","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"}],"magnum/common/octavia.py":[{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"85c94af328dacf11447ee93c5ba986856b57ef50","unresolved":false,"context_lines":[{"line_number":47,"context_line":"            break"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        if (time.time() - timeout) \u003e start_time:"},{"line_number":50,"context_line":"            raise Exception(\"Timeout waiting for the load balancers \""},{"line_number":51,"context_line":"                            \"%s to be deleted.\" % deleted_lbs)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_6e195fd5","line":50,"updated":"2020-04-02 11:06:16.000000000","message":"In case we move forward or in another patch,\nCould we also change this to an ERROR log instead of an \"aggressive\" exception?","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"7cd303c5f05f6269c2066824cb586c7c95198c8b","unresolved":false,"context_lines":[{"line_number":47,"context_line":"            break"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        if (time.time() - timeout) \u003e start_time:"},{"line_number":50,"context_line":"            raise Exception(\"Timeout waiting for the load balancers \""},{"line_number":51,"context_line":"                            \"%s to be deleted.\" % deleted_lbs)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_d9d14925","line":50,"in_reply_to":"df33271e_6e195fd5","updated":"2020-04-02 21:39:20.000000000","message":"The exception is needed because it\u0027s catched outside (Line 129) and raise another \"official\" exception","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"7fad0119d6f7396c10a25bf153d8c4c7e9c96a35","unresolved":false,"context_lines":[{"line_number":47,"context_line":"            break"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        if (time.time() - timeout) \u003e start_time:"},{"line_number":50,"context_line":"            raise Exception(\"Timeout waiting for the load balancers \""},{"line_number":51,"context_line":"                            \"%s to be deleted.\" % deleted_lbs)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"        time.sleep(1)"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_c3e8147e","line":50,"in_reply_to":"df33271e_d9d14925","updated":"2020-04-04 17:32:23.000000000","message":"We need this, I will propose another patch that can *optionally* go around this. It is out of the scope of this patch.","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"fdefa9e4c71b78ac2f46e210c2f847d8e3a47394","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        time.sleep(1)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def delete_loadbalancers(context, cluster):"},{"line_number":57,"context_line":"    \"\"\"Delete loadbalancers for the cluster."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    The following load balancers should be deleted:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_d9979739","line":56,"updated":"2020-04-02 11:50:12.000000000","message":"pep8: C901 \u0027delete_loadbalancers\u0027 is too complex (14)","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"8cad0c8c14d2e127eea5c2cc8cc4adb5f8982587","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        time.sleep(1)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def delete_loadbalancers(context, cluster):"},{"line_number":57,"context_line":"    \"\"\"Delete loadbalancers for the cluster."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    The following load balancers should be deleted:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_df6bf7ed","line":56,"in_reply_to":"df33271e_d9979739","updated":"2020-04-02 13:15:10.000000000","message":"+1","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"7cd303c5f05f6269c2066824cb586c7c95198c8b","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        time.sleep(1)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def delete_loadbalancers(context, cluster):"},{"line_number":57,"context_line":"    \"\"\"Delete loadbalancers for the cluster."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    The following load balancers should be deleted:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_f997ed43","line":56,"in_reply_to":"df33271e_df6bf7ed","updated":"2020-04-02 21:39:20.000000000","message":"I will merge some code in the next patchset if the idea is good to go","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"7fad0119d6f7396c10a25bf153d8c4c7e9c96a35","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        time.sleep(1)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def delete_loadbalancers(context, cluster):"},{"line_number":57,"context_line":"    \"\"\"Delete loadbalancers for the cluster."},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    The following load balancers should be deleted:"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_43fc043e","line":56,"in_reply_to":"df33271e_f997ed43","updated":"2020-04-04 17:32:23.000000000","message":"+1","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"85c94af328dacf11447ee93c5ba986856b57ef50","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        wait_for_lb_deleted(octavia_client, candidates)"},{"line_number":129,"context_line":"    except Exception as e:"},{"line_number":130,"context_line":"        raise exception.PreDeletionFailed(cluster_uuid\u003dcluster.uuid,"},{"line_number":131,"context_line":"                                          msg\u003dstr(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_8e1663a7","line":130,"updated":"2020-04-02 11:06:16.000000000","message":"In case we move forward or in another patch,\nCould we also change this to an ERROR log instead of an \"aggressive\" exception?","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"7cd303c5f05f6269c2066824cb586c7c95198c8b","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        wait_for_lb_deleted(octavia_client, candidates)"},{"line_number":129,"context_line":"    except Exception as e:"},{"line_number":130,"context_line":"        raise exception.PreDeletionFailed(cluster_uuid\u003dcluster.uuid,"},{"line_number":131,"context_line":"                                          msg\u003dstr(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_d9a4a9a8","line":130,"in_reply_to":"df33271e_8e1663a7","updated":"2020-04-02 21:39:20.000000000","message":"I didn\u0027t follow, do you mean we change this to an ERROR log or we change the msg to something more explicit rather than str(e)?\n\nIf the load balancer deletion fails, we need this exception to stop the cluster deletion process.","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"7fad0119d6f7396c10a25bf153d8c4c7e9c96a35","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        wait_for_lb_deleted(octavia_client, candidates)"},{"line_number":129,"context_line":"    except Exception as e:"},{"line_number":130,"context_line":"        raise exception.PreDeletionFailed(cluster_uuid\u003dcluster.uuid,"},{"line_number":131,"context_line":"                                          msg\u003dstr(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"df33271e_6398281b","line":130,"in_reply_to":"df33271e_d9a4a9a8","updated":"2020-04-04 17:32:23.000000000","message":"We need this as you say, I will propose another patch that can *optionally* go around this. It is out of the scope of this patch.","commit_id":"bc8a122cd0f37b2482568d1e76176d3017184bfe"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"f2e6d0490930c4d52b1324042dbbf340e256a786","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        time.sleep(1)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def _cascade_delete(context, lbs, cluster, octavia_client, remove_fip\u003dFalse):"},{"line_number":57,"context_line":"    candidates \u003d set()"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    for lb in lbs:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f493fa4_5ef1ed6a","line":56,"range":{"start_line":56,"start_character":4,"end_line":56,"end_character":19},"updated":"2020-04-28 10:46:55.000000000","message":"Can we name this _delete_loadbalancers and pass cascade as an argument?\n\nI propose this because this method loops over the list creates another and deletes a fip too.\n\nWhen we add support for non-cascade the call delete will be factored out to an even smaller method which will wrap cascade delete and the alternative will be to delete all resources.","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"d84e9e3501c9e49768505de5f6340c787fcc5eb4","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        time.sleep(1)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def _cascade_delete(context, lbs, cluster, octavia_client, remove_fip\u003dFalse):"},{"line_number":57,"context_line":"    candidates \u003d set()"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    for lb in lbs:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f493fa4_b3b6038a","line":56,"range":{"start_line":56,"start_character":4,"end_line":56,"end_character":19},"in_reply_to":"1f493fa4_5ef1ed6a","updated":"2020-05-04 01:54:02.000000000","message":"Done","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"62c7327f5176b2e855776e85641b9a0948dc0cd1","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        if status not in [\"PENDING_DELETE\", \"DELETED\"]:"},{"line_number":62,"context_line":"            LOG.info(\"Deleting load balancer %s for cluster %s\","},{"line_number":63,"context_line":"                     lb[\"id\"], cluster.uuid)"},{"line_number":64,"context_line":"            octavia_client.load_balancer_delete(lb[\"id\"], cascade\u003dTrue)"},{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_2daa64d3","line":64,"range":{"start_line":64,"start_character":58,"end_line":64,"end_character":70},"updated":"2020-04-15 08:50:09.000000000","message":"Could we make this configurable (I don\u0027t mind default to true)?\n\nSome octavia API implementations may not support this.\n\nOur LBaaS octavia API is backed by Tungsten Fabric which doesn\u0027t support cascade delete.","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"cb71939dff6dcae3c51a6a6b5c0d093d4d18e4d9","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        if status not in [\"PENDING_DELETE\", \"DELETED\"]:"},{"line_number":62,"context_line":"            LOG.info(\"Deleting load balancer %s for cluster %s\","},{"line_number":63,"context_line":"                     lb[\"id\"], cluster.uuid)"},{"line_number":64,"context_line":"            octavia_client.load_balancer_delete(lb[\"id\"], cascade\u003dTrue)"},{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_2df1a8ce","line":64,"range":{"start_line":64,"start_character":58,"end_line":64,"end_character":70},"in_reply_to":"3f4c43b2_149841be","updated":"2020-04-16 14:21:48.000000000","message":"I have to check. I think it won\u0027t fail but probably it will leave things behind.","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"b99ca62e4916345af6c8b2dc82ccbef3efd36117","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        if status not in [\"PENDING_DELETE\", \"DELETED\"]:"},{"line_number":62,"context_line":"            LOG.info(\"Deleting load balancer %s for cluster %s\","},{"line_number":63,"context_line":"                     lb[\"id\"], cluster.uuid)"},{"line_number":64,"context_line":"            octavia_client.load_balancer_delete(lb[\"id\"], cascade\u003dTrue)"},{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f493fa4_1f14074e","line":64,"range":{"start_line":64,"start_character":58,"end_line":64,"end_character":70},"in_reply_to":"3f4c43b2_1ddb044f","updated":"2020-04-21 23:43:49.000000000","message":"Yes, this patch is ready for review, I\u0027ve also tested in my DevStack environment.","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"9bbc9eb4f8b250da95ab82d4cb63e0657bf88899","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        if status not in [\"PENDING_DELETE\", \"DELETED\"]:"},{"line_number":62,"context_line":"            LOG.info(\"Deleting load balancer %s for cluster %s\","},{"line_number":63,"context_line":"                     lb[\"id\"], cluster.uuid)"},{"line_number":64,"context_line":"            octavia_client.load_balancer_delete(lb[\"id\"], cascade\u003dTrue)"},{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_63d75bc1","line":64,"range":{"start_line":64,"start_character":58,"end_line":64,"end_character":70},"in_reply_to":"3f4c43b2_2daa64d3","updated":"2020-04-15 10:43:49.000000000","message":"Sure, will add a new config option","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"dd4eb54bc72ec48ceb737d35fdf322f2478fe439","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        if status not in [\"PENDING_DELETE\", \"DELETED\"]:"},{"line_number":62,"context_line":"            LOG.info(\"Deleting load balancer %s for cluster %s\","},{"line_number":63,"context_line":"                     lb[\"id\"], cluster.uuid)"},{"line_number":64,"context_line":"            octavia_client.load_balancer_delete(lb[\"id\"], cascade\u003dTrue)"},{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_1ddb044f","line":64,"range":{"start_line":64,"start_character":58,"end_line":64,"end_character":70},"in_reply_to":"3f4c43b2_2df1a8ce","updated":"2020-04-20 10:11:38.000000000","message":"We can leave this out atm. For TungstenFabric or something else that doesn\u0027t support cascade deletion, all resources have to be deleted. For the API endpoints, this is not a problem. Even without this patch, the pre-delete hook, will not work for APIs that don\u0027t support cascade delete.\n\nLingxian, let\u0027s move forward without this option. It doesn\u0027t make sense. Work needs to be done to support non-cascade deletion. Since we won\u0027t add this option, is this patch ready?","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"03536fa6c9ab2a9b010e45232dc1dae98be460dc","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        if status not in [\"PENDING_DELETE\", \"DELETED\"]:"},{"line_number":62,"context_line":"            LOG.info(\"Deleting load balancer %s for cluster %s\","},{"line_number":63,"context_line":"                     lb[\"id\"], cluster.uuid)"},{"line_number":64,"context_line":"            octavia_client.load_balancer_delete(lb[\"id\"], cascade\u003dTrue)"},{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_a38fa3ca","line":64,"range":{"start_line":64,"start_character":58,"end_line":64,"end_character":70},"in_reply_to":"3f4c43b2_63d75bc1","updated":"2020-04-15 10:46:00.000000000","message":"Thanks, I really appreciate it","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"22979d5c1888e4fb38ec108470fe928837f229e6","unresolved":false,"context_lines":[{"line_number":61,"context_line":"        if status not in [\"PENDING_DELETE\", \"DELETED\"]:"},{"line_number":62,"context_line":"            LOG.info(\"Deleting load balancer %s for cluster %s\","},{"line_number":63,"context_line":"                     lb[\"id\"], cluster.uuid)"},{"line_number":64,"context_line":"            octavia_client.load_balancer_delete(lb[\"id\"], cascade\u003dTrue)"},{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_149841be","line":64,"range":{"start_line":64,"start_character":58,"end_line":64,"end_character":70},"in_reply_to":"3f4c43b2_a38fa3ca","updated":"2020-04-15 21:24:08.000000000","message":"I have a question, Spyros, if your Octavia doesn\u0027t support cascade delete load balancer, will directly delete a load balancer(without removing all sub-resources) fail?","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"f2e6d0490930c4d52b1324042dbbf340e256a786","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"},{"line_number":68,"context_line":"                neutron.delete_floatingip(context, lb[\"vip_port_id\"], cluster)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    return candidates"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f493fa4_de3fbd22","line":68,"updated":"2020-04-28 10:46:55.000000000","message":"If the LB doesn\u0027t have a fip, will this fail?","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"d84e9e3501c9e49768505de5f6340c787fcc5eb4","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            candidates.add(lb[\"id\"])"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            if remove_fip:"},{"line_number":68,"context_line":"                neutron.delete_floatingip(context, lb[\"vip_port_id\"], cluster)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    return candidates"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1f493fa4_d3ac6f37","line":68,"in_reply_to":"1f493fa4_de3fbd22","updated":"2020-05-04 01:54:02.000000000","message":"nope, see https://github.com/openstack/magnum/blob/master/magnum/common/neutron.py#L41","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"16dfe083b9f5916ebde77666e5083f448b69d7f5","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        # Get load balancers created for service/ingress"},{"line_number":96,"context_line":"        lbs \u003d octavia_client.load_balancer_list().get(\"loadbalancers\", [])"},{"line_number":97,"context_line":"        lbs \u003d [lb for lb in lbs if re.match(pattern, lb[\"description\"])]"},{"line_number":98,"context_line":"        deleted \u003d _cascade_delete(context, lbs, cluster, octavia_client_adm,"},{"line_number":99,"context_line":"                                  remove_fip\u003dTrue)"},{"line_number":100,"context_line":"        candidates.update(deleted)"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f493fa4_2e344ade","line":98,"updated":"2020-05-04 02:32:39.000000000","message":"pep8: F821 undefined name \u0027_cascade_delete\u0027","commit_id":"92af0513dd1158c1572d23c500f2830180c89c2b"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"16dfe083b9f5916ebde77666e5083f448b69d7f5","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                lbs.append(lb)"},{"line_number":112,"context_line":"            except osc_exc.NotFound:"},{"line_number":113,"context_line":"                continue"},{"line_number":114,"context_line":"        deleted \u003d _cascade_delete(context, lbs, cluster, octavia_client_adm,"},{"line_number":115,"context_line":"                                  remove_fip\u003dFalse)"},{"line_number":116,"context_line":"        candidates.update(deleted)"},{"line_number":117,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f493fa4_0e318eed","line":114,"updated":"2020-05-04 02:32:39.000000000","message":"pep8: F821 undefined name \u0027_cascade_delete\u0027","commit_id":"92af0513dd1158c1572d23c500f2830180c89c2b"}],"magnum/tests/unit/common/test_octavia.py":[{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"2716ed82960902deb9b0d6c3415c4b2c77a0a28b","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        mock_octavie_client.load_balancer_list.side_effect \u003d ["},{"line_number":68,"context_line":"            mock_lbs, {\"loadbalancers\": []}"},{"line_number":69,"context_line":"        ]"},{"line_number":70,"context_line":"        mock_octavie_client.load_balancer_show.return_value \u003d {"},{"line_number":71,"context_line":"            \u0027id\u0027: \u0027heat_lb_id\u0027,"},{"line_number":72,"context_line":"            \u0027provisioning_status\u0027: \u0027ACTIVE\u0027"},{"line_number":73,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_883ba6cf","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":20},"updated":"2020-04-15 09:00:39.000000000","message":"nit: s/octavie/octavia in the whole script","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"9bbc9eb4f8b250da95ab82d4cb63e0657bf88899","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        mock_octavie_client.load_balancer_list.side_effect \u003d ["},{"line_number":68,"context_line":"            mock_lbs, {\"loadbalancers\": []}"},{"line_number":69,"context_line":"        ]"},{"line_number":70,"context_line":"        mock_octavie_client.load_balancer_show.return_value \u003d {"},{"line_number":71,"context_line":"            \u0027id\u0027: \u0027heat_lb_id\u0027,"},{"line_number":72,"context_line":"            \u0027provisioning_status\u0027: \u0027ACTIVE\u0027"},{"line_number":73,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f4c43b2_03f01767","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":20},"in_reply_to":"3f4c43b2_883ba6cf","updated":"2020-04-15 10:43:49.000000000","message":"Nice catch, will update, thanks!","commit_id":"d3161e83977a9e5ebd51215edd6dca3450551b8b"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"fd7558decb921b975344c7c854dc52ce97cc757c","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        mock_octavie_client.load_balancer_list.side_effect \u003d ["},{"line_number":68,"context_line":"            mock_lbs, {\"loadbalancers\": []}"},{"line_number":69,"context_line":"        ]"},{"line_number":70,"context_line":"        mock_octavie_client.load_balancer_show.return_value \u003d {"},{"line_number":71,"context_line":"            \u0027id\u0027: \u0027heat_lb_id\u0027,"},{"line_number":72,"context_line":"            \u0027provisioning_status\u0027: \u0027ACTIVE\u0027"},{"line_number":73,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_76e3bfa5","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":20},"updated":"2020-05-04 15:43:43.000000000","message":"s/octavie/octavia/g","commit_id":"477e31651d34cba2803569a7f9073c0c63bc8e3f"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"0e45429ad4609aff2cf24a9271f8b0baacaf919b","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        mock_octavie_client.load_balancer_list.side_effect \u003d ["},{"line_number":68,"context_line":"            mock_lbs, {\"loadbalancers\": []}"},{"line_number":69,"context_line":"        ]"},{"line_number":70,"context_line":"        mock_octavie_client.load_balancer_show.return_value \u003d {"},{"line_number":71,"context_line":"            \u0027id\u0027: \u0027heat_lb_id\u0027,"},{"line_number":72,"context_line":"            \u0027provisioning_status\u0027: \u0027ACTIVE\u0027"},{"line_number":73,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_79b7303c","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":20},"in_reply_to":"1f493fa4_360c9741","updated":"2020-05-04 15:56:33.000000000","message":"I see, it is wrong everywhere. Please update all occurences of octavie with octavia.","commit_id":"477e31651d34cba2803569a7f9073c0c63bc8e3f"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"38d528b6ab1b1725899a8aa535101e3d3f7dcd3f","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        mock_octavie_client.load_balancer_list.side_effect \u003d ["},{"line_number":68,"context_line":"            mock_lbs, {\"loadbalancers\": []}"},{"line_number":69,"context_line":"        ]"},{"line_number":70,"context_line":"        mock_octavie_client.load_balancer_show.return_value \u003d {"},{"line_number":71,"context_line":"            \u0027id\u0027: \u0027heat_lb_id\u0027,"},{"line_number":72,"context_line":"            \u0027provisioning_status\u0027: \u0027ACTIVE\u0027"},{"line_number":73,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_d9f644fa","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":20},"in_reply_to":"1f493fa4_360c9741","updated":"2020-05-04 15:57:26.000000000","message":"Looks like mock_octavie_client is used in this file only and the typo has always been there and internally consistent... I do not wish to block over this if the consensus is that this is not an issue but this feels like a good opportunity to fix something like this.","commit_id":"477e31651d34cba2803569a7f9073c0c63bc8e3f"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"927c3c26db9d1fc83ce3906295ec1f6512771a25","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        mock_octavie_client.load_balancer_list.side_effect \u003d ["},{"line_number":68,"context_line":"            mock_lbs, {\"loadbalancers\": []}"},{"line_number":69,"context_line":"        ]"},{"line_number":70,"context_line":"        mock_octavie_client.load_balancer_show.return_value \u003d {"},{"line_number":71,"context_line":"            \u0027id\u0027: \u0027heat_lb_id\u0027,"},{"line_number":72,"context_line":"            \u0027provisioning_status\u0027: \u0027ACTIVE\u0027"},{"line_number":73,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_360c9741","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":20},"in_reply_to":"1f493fa4_76e3bfa5","updated":"2020-05-04 15:52:26.000000000","message":"@bharat and Lingxian since my +2 is invalid now.\n\nPlease, investigate, why the CI is not breaking by this typo. I imagine this was spotting by reading the code.","commit_id":"477e31651d34cba2803569a7f9073c0c63bc8e3f"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"decc18a6ba4bc14e6fddbd84520222d4a2f2dcb1","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        mock_octavie_client.load_balancer_list.side_effect \u003d ["},{"line_number":68,"context_line":"            mock_lbs, {\"loadbalancers\": []}"},{"line_number":69,"context_line":"        ]"},{"line_number":70,"context_line":"        mock_octavie_client.load_balancer_show.return_value \u003d {"},{"line_number":71,"context_line":"            \u0027id\u0027: \u0027heat_lb_id\u0027,"},{"line_number":72,"context_line":"            \u0027provisioning_status\u0027: \u0027ACTIVE\u0027"},{"line_number":73,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_b9ba58de","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":20},"in_reply_to":"1f493fa4_d92be42d","updated":"2020-05-04 16:03:25.000000000","message":"It would be great if we are ready to merge when the typo is fixed.","commit_id":"477e31651d34cba2803569a7f9073c0c63bc8e3f"},{"author":{"_account_id":20498,"name":"Spyros Trigazis","email":"spyridon.trigazis@cern.ch","username":"strigazi"},"change_message_id":"a5c33d56ccbcfaea301016f1de9c42208ab1e2ff","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        mock_octavie_client.load_balancer_list.side_effect \u003d ["},{"line_number":68,"context_line":"            mock_lbs, {\"loadbalancers\": []}"},{"line_number":69,"context_line":"        ]"},{"line_number":70,"context_line":"        mock_octavie_client.load_balancer_show.return_value \u003d {"},{"line_number":71,"context_line":"            \u0027id\u0027: \u0027heat_lb_id\u0027,"},{"line_number":72,"context_line":"            \u0027provisioning_status\u0027: \u0027ACTIVE\u0027"},{"line_number":73,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":4,"id":"1f493fa4_d92be42d","line":70,"range":{"start_line":70,"start_character":13,"end_line":70,"end_character":20},"in_reply_to":"1f493fa4_d9f644fa","updated":"2020-05-04 16:02:08.000000000","message":"The mistake was merged by me, Feilong and Lingxian anyway.\nhttps://review.opendev.org/#/c/497144/\n\nI\u0027m sorry, but once it is reported and we are embaraced by just reading the code. We must fix it.","commit_id":"477e31651d34cba2803569a7f9073c0c63bc8e3f"},{"author":{"_account_id":28022,"name":"Bharat Kunwar","email":"brtknr@bath.edu","username":"brtknr"},"change_message_id":"fd95d71fb6422514bfd1e423f7c56083a402c07b","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @mock.patch(\"magnum.common.neutron.delete_floatingip\")"},{"line_number":93,"context_line":"    @mock.patch(\u0027magnum.common.clients.OpenStackClients\u0027)"},{"line_number":94,"context_line":"    def test_delete_loadbalancers_with_invalid_lb(self, mock_clients,"},{"line_number":95,"context_line":"                                                  mock_delete_fip):"},{"line_number":96,"context_line":"        osc \u003d mock.MagicMock()"},{"line_number":97,"context_line":"        mock_clients.return_value \u003d osc"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f493fa4_089e8a06","side":"PARENT","line":94,"range":{"start_line":94,"start_character":0,"end_line":94,"end_character":69},"updated":"2020-05-05 05:40:55.000000000","message":"What is the justification for removing this test?","commit_id":"dd4b79263f37deaa56d551b1daaa82ab19867fd4"},{"author":{"_account_id":6732,"name":"Lingxian Kong","email":"anlin.kong@gmail.com","username":"kong"},"change_message_id":"d7a0728460f8454377821d8ef900f877c41441f7","unresolved":false,"context_lines":[{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @mock.patch(\"magnum.common.neutron.delete_floatingip\")"},{"line_number":93,"context_line":"    @mock.patch(\u0027magnum.common.clients.OpenStackClients\u0027)"},{"line_number":94,"context_line":"    def test_delete_loadbalancers_with_invalid_lb(self, mock_clients,"},{"line_number":95,"context_line":"                                                  mock_delete_fip):"},{"line_number":96,"context_line":"        osc \u003d mock.MagicMock()"},{"line_number":97,"context_line":"        mock_clients.return_value \u003d osc"}],"source_content_type":"text/x-python","patch_set":5,"id":"1f493fa4_537dd16c","side":"PARENT","line":94,"range":{"start_line":94,"start_character":0,"end_line":94,"end_character":69},"in_reply_to":"1f493fa4_089e8a06","updated":"2020-05-05 21:41:18.000000000","message":"If you take a look at the code, we removed the invalid (status) lb check, because:\n\n1. It\u0027s redundant, if octavia doesn\u0027t allow to delete, the magnum-conductor will raise exception.PreDeletionFailed() with proper message.\n2. In case some cloud providers allow adm user to delete pending lbs.","commit_id":"dd4b79263f37deaa56d551b1daaa82ab19867fd4"}]}