)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"1111147118c681b14d23b2e9354057fac7b0af73","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Clean lb crd status upon Load Balancer removal"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When a lb transitions to ERROR or the IP on the"},{"line_number":10,"context_line":"Service spec differs from the lb VIP, the lb is"},{"line_number":11,"context_line":"released and the CRD doesn\u0027t get updated,"},{"line_number":12,"context_line":"causing Not Found expections when handling the"},{"line_number":13,"context_line":"creation of others load balancer resources."},{"line_number":14,"context_line":"This commit fixes the issue by ensuring the clean"},{"line_number":15,"context_line":"up of the status field happens upon lb release."},{"line_number":16,"context_line":"Also, it adds protection in case we still get"},{"line_number":17,"context_line":"nonexistent lb on the CRD."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Closes-Bug: 1894758"},{"line_number":20,"context_line":"Change-Id: I484ece6a7b52b51d878f724bd4fad0494eb759d6"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9f560f44_38424e10","line":17,"range":{"start_line":9,"start_character":0,"end_line":17,"end_character":26},"updated":"2020-09-08 09:05:45.000000000","message":"50 characters column? I thought it should be around 72 (just like emails) :D","commit_id":"def60d3b2e0cea90d9cf15359ad2c47f3f1f7f0a"}],"kuryr_kubernetes/controller/drivers/lbaasv2.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"9ba66b66ac1578ee9a50467444c67de4cf71b89f","unresolved":false,"context_lines":[{"line_number":672,"context_line":"                                                  interval):"},{"line_number":673,"context_line":"            if not self._wait_for_provisioning("},{"line_number":674,"context_line":"                    loadbalancer, remaining, interval):"},{"line_number":675,"context_line":"                return None"},{"line_number":676,"context_line":"            try:"},{"line_number":677,"context_line":"                result \u003d self._ensure(create, find, obj, loadbalancer)"},{"line_number":678,"context_line":"                if result:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_44c5865b","line":675,"range":{"start_line":675,"start_character":23,"end_line":675,"end_character":27},"updated":"2020-09-09 12:54:52.000000000","message":"if it error, shouldn\u0027t we retry?","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2167ed90fb20efd03f01c466ac062e0dd2f2e6ac","unresolved":false,"context_lines":[{"line_number":672,"context_line":"                                                  interval):"},{"line_number":673,"context_line":"            if not self._wait_for_provisioning("},{"line_number":674,"context_line":"                    loadbalancer, remaining, interval):"},{"line_number":675,"context_line":"                return None"},{"line_number":676,"context_line":"            try:"},{"line_number":677,"context_line":"                result \u003d self._ensure(create, find, obj, loadbalancer)"},{"line_number":678,"context_line":"                if result:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_84895e27","line":675,"range":{"start_line":675,"start_character":23,"end_line":675,"end_character":27},"in_reply_to":"9f560f44_44c5865b","updated":"2020-09-09 13:01:45.000000000","message":"It will get handled in another event generated by the patching of the crd with status empty.","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"}],"kuryr_kubernetes/controller/handlers/loadbalancer.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"7a9d819d6f8e575cff5f26e4304e6f215b3be835","unresolved":false,"context_lines":[{"line_number":672,"context_line":""},{"line_number":673,"context_line":"            self._drv_lbaas.release_loadbalancer("},{"line_number":674,"context_line":"                loadbalancer\u003dlb)"},{"line_number":675,"context_line":"            utils.clean_lb_crd_status(lb)"},{"line_number":676,"context_line":"            lb \u003d None"},{"line_number":677,"context_line":"            changed \u003d True"},{"line_number":678,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_62eff91c","line":675,"range":{"start_line":675,"start_character":38,"end_line":675,"end_character":40},"updated":"2020-09-08 06:49:59.000000000","message":"lb[\u0027name\u0027]?","commit_id":"b6e3ef76427af273a3a0a8821f4116e230bf62ee"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"09d386f6885a604f4e348094afc0a135bb616943","unresolved":false,"context_lines":[{"line_number":672,"context_line":""},{"line_number":673,"context_line":"            self._drv_lbaas.release_loadbalancer("},{"line_number":674,"context_line":"                loadbalancer\u003dlb)"},{"line_number":675,"context_line":"            utils.clean_lb_crd_status(lb)"},{"line_number":676,"context_line":"            lb \u003d None"},{"line_number":677,"context_line":"            changed \u003d True"},{"line_number":678,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_e5ecb3f5","line":675,"range":{"start_line":675,"start_character":38,"end_line":675,"end_character":40},"in_reply_to":"9f560f44_62eff91c","updated":"2020-09-08 08:11:24.000000000","message":"yes, thanks!","commit_id":"b6e3ef76427af273a3a0a8821f4116e230bf62ee"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"1111147118c681b14d23b2e9354057fac7b0af73","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            # NOTE(ivc): deleting pool deletes its members"},{"line_number":136,"context_line":"            self._drv_lbaas.release_loadbalancer("},{"line_number":137,"context_line":"                loadbalancer\u003dloadbalancer_crd[\u0027status\u0027].get(\u0027loadbalancer\u0027))"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"            try:"},{"line_number":140,"context_line":"                pub_info \u003d loadbalancer_crd[\u0027status\u0027][\u0027service_pub_ip_info\u0027]"},{"line_number":141,"context_line":"            except KeyError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_d8feb2ad","side":"PARENT","line":138,"updated":"2020-09-08 09:05:45.000000000","message":"I would leave blank line for readability.","commit_id":"ffc0af30c6ccc8c4c8907eca1e541888fe9f80cc"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"1111147118c681b14d23b2e9354057fac7b0af73","unresolved":false,"context_lines":[{"line_number":672,"context_line":""},{"line_number":673,"context_line":"            self._drv_lbaas.release_loadbalancer("},{"line_number":674,"context_line":"                loadbalancer\u003dlb)"},{"line_number":675,"context_line":"            utils.clean_lb_crd_status(lb[\u0027name\u0027])"},{"line_number":676,"context_line":"            lb \u003d None"},{"line_number":677,"context_line":"            changed \u003d True"},{"line_number":678,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_58246260","line":675,"updated":"2020-09-08 09:05:45.000000000","message":"Also here, it would be good to have a separation with blank line.","commit_id":"def60d3b2e0cea90d9cf15359ad2c47f3f1f7f0a"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"9ba66b66ac1578ee9a50467444c67de4cf71b89f","unresolved":false,"context_lines":[{"line_number":686,"context_line":"                loadbalancer\u003dlb)"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"            lb \u003d {}"},{"line_number":689,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027pools\u0027] \u003d []"},{"line_number":690,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027listeners\u0027] \u003d []"},{"line_number":691,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027members\u0027] \u003d []"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        if not lb:"},{"line_number":694,"context_line":"            if loadbalancer_crd[\u0027spec\u0027].get(\u0027ip\u0027):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_c40a36cf","line":691,"range":{"start_line":689,"start_character":0,"end_line":691,"end_character":54},"updated":"2020-09-09 12:54:52.000000000","message":"do we need this if we are not upgrading the status here? can we simply do loadbalancer_crd[\u0027status\u0027] \u003d {}?","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"9f8ae0df60c732616caa0f3ff0f24c58fce57408","unresolved":false,"context_lines":[{"line_number":686,"context_line":"                loadbalancer\u003dlb)"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"            lb \u003d {}"},{"line_number":689,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027pools\u0027] \u003d []"},{"line_number":690,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027listeners\u0027] \u003d []"},{"line_number":691,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027members\u0027] \u003d []"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        if not lb:"},{"line_number":694,"context_line":"            if loadbalancer_crd[\u0027spec\u0027].get(\u0027ip\u0027):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_efe2b300","line":691,"range":{"start_line":689,"start_character":0,"end_line":691,"end_character":54},"in_reply_to":"9f560f44_44e0e697","updated":"2020-09-09 13:44:12.000000000","message":"I thought that If there was no changes of the fip specification, it was okay to not release the pub IP and just disassociate. But I realized the FIP never gets associated again if it\u0027s already filled in here https://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/controller/handlers/loadbalancer.py#L85-L87\n\nDo you think I should still move to line 684, or associate the FIP if existent in the link I shared?","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"230049dd59d24b2d0bc4540550d918bd0990521d","unresolved":false,"context_lines":[{"line_number":686,"context_line":"                loadbalancer\u003dlb)"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"            lb \u003d {}"},{"line_number":689,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027pools\u0027] \u003d []"},{"line_number":690,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027listeners\u0027] \u003d []"},{"line_number":691,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027members\u0027] \u003d []"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        if not lb:"},{"line_number":694,"context_line":"            if loadbalancer_crd[\u0027spec\u0027].get(\u0027ip\u0027):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_44e0e697","line":691,"range":{"start_line":689,"start_character":0,"end_line":691,"end_character":54},"in_reply_to":"9f560f44_64bc0abb","updated":"2020-09-09 13:10:38.000000000","message":"but the dissacociated_pub_ip is already done in lines 681-683, right? Perhaps that part must be done as part of the release here, instead of later in the current place, right?\n\nThe way I read it is that if if on line 693 is executed is either because we have cleaned up lb here, or because there is no loadbalancer (line 671) in which case there should be no FIP already, right? And to associated it later one the spec should be used instead of the status","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2167ed90fb20efd03f01c466ac062e0dd2f2e6ac","unresolved":false,"context_lines":[{"line_number":686,"context_line":"                loadbalancer\u003dlb)"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"            lb \u003d {}"},{"line_number":689,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027pools\u0027] \u003d []"},{"line_number":690,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027listeners\u0027] \u003d []"},{"line_number":691,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027members\u0027] \u003d []"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        if not lb:"},{"line_number":694,"context_line":"            if loadbalancer_crd[\u0027spec\u0027].get(\u0027ip\u0027):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_64bc0abb","line":691,"range":{"start_line":689,"start_character":0,"end_line":691,"end_character":54},"in_reply_to":"9f560f44_c40a36cf","updated":"2020-09-09 13:01:45.000000000","message":"In case the status has service_pub_ip_info, I guess we shouldn\u0027t clean it up as it will be handled at line 709.","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"9ba66b66ac1578ee9a50467444c67de4cf71b89f","unresolved":false,"context_lines":[{"line_number":705,"context_line":"                    service_type\u003dloadbalancer_crd[\u0027spec\u0027].get(\u0027type\u0027),"},{"line_number":706,"context_line":"                    provider\u003dself._lb_provider)"},{"line_number":707,"context_line":"                loadbalancer_crd[\u0027status\u0027][\u0027loadbalancer\u0027] \u003d lb"},{"line_number":708,"context_line":"                changed \u003d True"},{"line_number":709,"context_line":"            elif loadbalancer_crd[\u0027status\u0027].get(\u0027service_pub_ip_info\u0027):"},{"line_number":710,"context_line":"                self._drv_service_pub_ip.release_pub_ip("},{"line_number":711,"context_line":"                    loadbalancer_crd[\u0027status\u0027][\u0027service_pub_ip_info\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_e4c13a82","line":708,"range":{"start_line":708,"start_character":16,"end_line":708,"end_character":30},"updated":"2020-09-09 12:54:52.000000000","message":"if you are setting it at line 726, then it is not needed, right?","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2167ed90fb20efd03f01c466ac062e0dd2f2e6ac","unresolved":false,"context_lines":[{"line_number":705,"context_line":"                    service_type\u003dloadbalancer_crd[\u0027spec\u0027].get(\u0027type\u0027),"},{"line_number":706,"context_line":"                    provider\u003dself._lb_provider)"},{"line_number":707,"context_line":"                loadbalancer_crd[\u0027status\u0027][\u0027loadbalancer\u0027] \u003d lb"},{"line_number":708,"context_line":"                changed \u003d True"},{"line_number":709,"context_line":"            elif loadbalancer_crd[\u0027status\u0027].get(\u0027service_pub_ip_info\u0027):"},{"line_number":710,"context_line":"                self._drv_service_pub_ip.release_pub_ip("},{"line_number":711,"context_line":"                    loadbalancer_crd[\u0027status\u0027][\u0027service_pub_ip_info\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_24d33203","line":708,"range":{"start_line":708,"start_character":16,"end_line":708,"end_character":30},"in_reply_to":"9f560f44_e4c13a82","updated":"2020-09-09 13:01:45.000000000","message":"right.","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"9ba66b66ac1578ee9a50467444c67de4cf71b89f","unresolved":false,"context_lines":[{"line_number":710,"context_line":"                self._drv_service_pub_ip.release_pub_ip("},{"line_number":711,"context_line":"                    loadbalancer_crd[\u0027status\u0027][\u0027service_pub_ip_info\u0027])"},{"line_number":712,"context_line":"                loadbalancer_crd[\u0027status\u0027][\u0027service_pub_ip_info\u0027] \u003d None"},{"line_number":713,"context_line":"                changed \u003d True"},{"line_number":714,"context_line":""},{"line_number":715,"context_line":"            kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":716,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9f560f44_04c54e8e","line":713,"range":{"start_line":713,"start_character":16,"end_line":713,"end_character":30},"updated":"2020-09-09 12:54:52.000000000","message":"same here, not needed","commit_id":"86a3eb5406059de1258f28fa3d8d45bddd2a1567"},{"author":{"_account_id":30963,"name":"Sarka Scavnicka","display_name":"sscavnic","email":"scavnicka.sarka@gmail.com","username":"sarka_scavnicka"},"change_message_id":"066a08a652f446307b22c594d24702c7a867c294","unresolved":false,"context_lines":[{"line_number":685,"context_line":"            self._drv_lbaas.release_loadbalancer("},{"line_number":686,"context_line":"                loadbalancer\u003dlb)"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"            lb \u003d {}"},{"line_number":689,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027pools\u0027] \u003d []"},{"line_number":690,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027listeners\u0027] \u003d []"},{"line_number":691,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027members\u0027] \u003d []"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        if not lb:"},{"line_number":694,"context_line":"            if loadbalancer_crd[\u0027spec\u0027].get(\u0027ip\u0027):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_6f0983db","line":691,"range":{"start_line":688,"start_character":0,"end_line":691,"end_character":54},"updated":"2020-09-09 13:41:40.000000000","message":"This need to be change to loadbalancer_crd[\u0027status\u0027] \u003d {}, because there are required fields https://github.com/openstack/kuryr-kubernetes/blob/master/kubernetes_crds/kuryr_crds/kuryrloadbalancer.yaml#L118 so if we patch it like  loadbalancer_crd[\u0027status\u0027][\u0027listeners\u0027] \u003d [], it will failed.","commit_id":"84acf6e8e203f9f457cc7de0769a4ef58242c244"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5ca9bfdb18751b0f2a9a585655dea14cc1ee9119","unresolved":false,"context_lines":[{"line_number":685,"context_line":"            self._drv_lbaas.release_loadbalancer("},{"line_number":686,"context_line":"                loadbalancer\u003dlb)"},{"line_number":687,"context_line":""},{"line_number":688,"context_line":"            lb \u003d {}"},{"line_number":689,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027pools\u0027] \u003d []"},{"line_number":690,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027listeners\u0027] \u003d []"},{"line_number":691,"context_line":"            loadbalancer_crd[\u0027status\u0027][\u0027members\u0027] \u003d []"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        if not lb:"},{"line_number":694,"context_line":"            if loadbalancer_crd[\u0027spec\u0027].get(\u0027ip\u0027):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9f560f44_afe71be3","line":691,"range":{"start_line":688,"start_character":0,"end_line":691,"end_character":54},"in_reply_to":"9f560f44_6f0983db","updated":"2020-09-09 13:50:19.000000000","message":"I tested manually to edit the listeners field with [] and it worked fine. I guess those requirements would get applied if any listeners would exist on the list with one of those fields missing.","commit_id":"84acf6e8e203f9f457cc7de0769a4ef58242c244"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"a5548b5017d316864ef509de7da1d5269c1cec83","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            LOG.debug(\u0027KuryrLoadBalancer %s not found\u0027, svc_name)"},{"line_number":232,"context_line":"            return None"},{"line_number":233,"context_line":"        except k_exc.K8sUnprocessableEntity:"},{"line_number":234,"context_line":"            LOG.debug(\u0027KuryrLoadBalancer entity not processable\u0027,"},{"line_number":235,"context_line":"                      \u0027due to missing loadbalancer field.\u0027)"},{"line_number":236,"context_line":"            return None"},{"line_number":237,"context_line":"        except k_exc.K8sClientException:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_4eb58e7d","line":234,"range":{"start_line":234,"start_character":63,"end_line":234,"end_character":64},"updated":"2020-09-10 09:09:33.000000000","message":"Missing space.","commit_id":"f67d12410a4711ef4b1cb80522c50407b680d089"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"08467d18c540484050875506afce3ab3dedad0f2","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            LOG.debug(\u0027KuryrLoadBalancer %s not found\u0027, svc_name)"},{"line_number":232,"context_line":"            return None"},{"line_number":233,"context_line":"        except k_exc.K8sUnprocessableEntity:"},{"line_number":234,"context_line":"            LOG.debug(\u0027KuryrLoadBalancer entity not processable\u0027,"},{"line_number":235,"context_line":"                      \u0027due to missing loadbalancer field.\u0027)"},{"line_number":236,"context_line":"            return None"},{"line_number":237,"context_line":"        except k_exc.K8sClientException:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9f560f44_8c9e8e51","line":234,"range":{"start_line":234,"start_character":63,"end_line":234,"end_character":64},"in_reply_to":"9f560f44_4eb58e7d","updated":"2020-09-10 10:21:55.000000000","message":"and extra \",\"","commit_id":"f67d12410a4711ef4b1cb80522c50407b680d089"}]}
