)]}'
{"kuryr_kubernetes/controller/drivers/base.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"79be7d447caa95cc91732c8b4c8a7e69dabbb9a2","unresolved":false,"context_lines":[{"line_number":725,"context_line":"        \"\"\""},{"line_number":726,"context_line":"        raise NotImplementedError()"},{"line_number":727,"context_line":""},{"line_number":728,"context_line":"    @abc.abstractmethod"},{"line_number":729,"context_line":"    def is_tagging_supported(self):"},{"line_number":730,"context_line":"        \"\"\"Returns if tagging LBaaS resources is supported\"\"\""},{"line_number":731,"context_line":"        raise NotImplementedError()"},{"line_number":732,"context_line":""},{"line_number":733,"context_line":""},{"line_number":734,"context_line":"@six.add_metaclass(abc.ABCMeta)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_56c9074f","line":731,"range":{"start_line":728,"start_character":0,"end_line":731,"end_character":35},"updated":"2019-11-27 16:36:06.000000000","message":"This won\u0027t be required after my other comment is addressed.","commit_id":"011547515ec1c3a96920176aa5bc22326985a805"}],"kuryr_kubernetes/controller/drivers/lbaasv2.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"3d06c9e946ad7bc95e3ddb6da0c66ef0b1baf13f","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    def get_loadbalancer_pool_name(self, loadbalancer, namespace, svc_name):"},{"line_number":86,"context_line":"        return \"%s/%s/%s\" % (loadbalancer.name, namespace, svc_name)"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    def add_tags(self, resource, req):"},{"line_number":89,"context_line":"        if CONF.neutron_defaults.resource_tags:"},{"line_number":90,"context_line":"            if self._octavia_tags:"},{"line_number":91,"context_line":"                req[\u0027tags\u0027] \u003d CONF.neutron_defaults.resource_tags"},{"line_number":92,"context_line":"            else:"},{"line_number":93,"context_line":"                if resource in (\u0027loadbalancer\u0027, \u0027listener\u0027, \u0027pool\u0027,"},{"line_number":94,"context_line":"                                \u0027l7policy\u0027):"},{"line_number":95,"context_line":"                    req[\u0027description\u0027] \u003d \u0027,\u0027.join("},{"line_number":96,"context_line":"                        CONF.neutron_defaults.resource_tags)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def ensure_loadbalancer(self, name, project_id, subnet_id, ip,"},{"line_number":99,"context_line":"                            security_groups_ids\u003dNone, service_type\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_78d15756","line":96,"range":{"start_line":88,"start_character":0,"end_line":96,"end_character":60},"updated":"2019-12-03 08:55:44.000000000","message":"as we have another method for adding tags on controller/driver/utils, not sure if better to add it there instead of here","commit_id":"a704c3c5d9ac579ffe66b7e1afb0b16c7ae646c3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"9eb37fa6e16096b13f904897f8a7729185c3ad15","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    def get_loadbalancer_pool_name(self, loadbalancer, namespace, svc_name):"},{"line_number":86,"context_line":"        return \"%s/%s/%s\" % (loadbalancer.name, namespace, svc_name)"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    def add_tags(self, resource, req):"},{"line_number":89,"context_line":"        if CONF.neutron_defaults.resource_tags:"},{"line_number":90,"context_line":"            if self._octavia_tags:"},{"line_number":91,"context_line":"                req[\u0027tags\u0027] \u003d CONF.neutron_defaults.resource_tags"},{"line_number":92,"context_line":"            else:"},{"line_number":93,"context_line":"                if resource in (\u0027loadbalancer\u0027, \u0027listener\u0027, \u0027pool\u0027,"},{"line_number":94,"context_line":"                                \u0027l7policy\u0027):"},{"line_number":95,"context_line":"                    req[\u0027description\u0027] \u003d \u0027,\u0027.join("},{"line_number":96,"context_line":"                        CONF.neutron_defaults.resource_tags)"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def ensure_loadbalancer(self, name, project_id, subnet_id, ip,"},{"line_number":99,"context_line":"                            security_groups_ids\u003dNone, service_type\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_cef49458","line":96,"range":{"start_line":88,"start_character":0,"end_line":96,"end_character":60},"in_reply_to":"3fa7e38b_78d15756","updated":"2019-12-15 19:00:33.000000000","message":"The \u0027add_tags\u0027 method is only adding the tags to a request body, regardless of the action (GET/POST/PUT) performed with that body. While, the \u0027tag_neutron_resources\u0027 method on controller/driver/utils sets the tags of a specific resource. Or were you referring to another method?","commit_id":"a704c3c5d9ac579ffe66b7e1afb0b16c7ae646c3"}],"kuryr_kubernetes/controller/handlers/lbaas.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"00df0b6dee2a72148cecbf1a817121b7a36cbb13","unresolved":false,"context_lines":[{"line_number":635,"context_line":""},{"line_number":636,"context_line":"    def _cleanup_leftover_lbaas(self):"},{"line_number":637,"context_line":"        lbaas_client \u003d clients.get_loadbalancer_client()"},{"line_number":638,"context_line":"        loadbalancers \u003d lbaas_client.load_balancers()"},{"line_number":639,"context_line":"        services \u003d driver_utils.get_services().get(\u0027items\u0027)"},{"line_number":640,"context_line":"        services_dict \u003d {\u0027{}/{}\u0027.format("},{"line_number":641,"context_line":"            service[\u0027metadata\u0027][\u0027namespace\u0027],"}],"source_content_type":"text/x-python","patch_set":2,"id":"3fa7e38b_692ca892","line":638,"range":{"start_line":638,"start_character":8,"end_line":638,"end_character":53},"updated":"2019-11-22 12:52:00.000000000","message":"you need to ensure you only pick loadbalancers generated by kuryr (with the right tagging)","commit_id":"534619fc56e0d3ae17e5846da36ff3ff435fef99"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"79be7d447caa95cc91732c8b4c8a7e69dabbb9a2","unresolved":false,"context_lines":[{"line_number":643,"context_line":"            service[\u0027metadata\u0027][\u0027name\u0027]): service"},{"line_number":644,"context_line":"            for service in services}"},{"line_number":645,"context_line":"        lbaas_spec \u003d {}"},{"line_number":646,"context_line":"        tags \u003d CONF.neutron_defaults.resource_tags"},{"line_number":647,"context_line":"        if tags:"},{"line_number":648,"context_line":"            if self._drv_lbaas.is_tagging_supported():"},{"line_number":649,"context_line":"                lbaas_spec[\u0027tags\u0027] \u003d tags"},{"line_number":650,"context_line":"            else:"},{"line_number":651,"context_line":"                lbaas_spec[\u0027description\u0027] \u003d tags"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        loadbalancers \u003d lbaas_client.load_balancers(**lbaas_spec)"},{"line_number":654,"context_line":"        for loadbalancer in loadbalancers:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_d605f7c2","line":651,"range":{"start_line":646,"start_character":0,"end_line":651,"end_character":48},"updated":"2019-11-27 16:36:06.000000000","message":"If there are no tags you\u0027ll put \"[]\" into the description. Why not just use LBaaSv2Driver._add_tags() (possibly renaming it and making public)?","commit_id":"011547515ec1c3a96920176aa5bc22326985a805"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"805b5752de9e83534bca8d975fb40cea6061a4ce","unresolved":false,"context_lines":[{"line_number":653,"context_line":"        loadbalancers \u003d lbaas_client.load_balancers(**lbaas_spec)"},{"line_number":654,"context_line":"        for loadbalancer in loadbalancers:"},{"line_number":655,"context_line":"            if not services_dict.get(loadbalancer.name):"},{"line_number":656,"context_line":"                self._drv_lbaas.release_loadbalancer(loadbalancer)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_84e1a62e","line":656,"range":{"start_line":656,"start_character":16,"end_line":656,"end_character":66},"updated":"2019-11-26 08:20:47.000000000","message":"this probably should try/catch the not found exception. Also, it should get the ResourceNotReady exception and retry the deletion (otherwise some leftovers may still happen. Or at least that is what I got for namespaces while implementing the cleanup there)","commit_id":"011547515ec1c3a96920176aa5bc22326985a805"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"63093f7011b7063585a414af7d783b2ff395de73","unresolved":false,"context_lines":[{"line_number":653,"context_line":"        loadbalancers \u003d lbaas_client.load_balancers(**lbaas_spec)"},{"line_number":654,"context_line":"        for loadbalancer in loadbalancers:"},{"line_number":655,"context_line":"            if not services_dict.get(loadbalancer.name):"},{"line_number":656,"context_line":"                self._drv_lbaas.release_loadbalancer(loadbalancer)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_8ace7d8d","line":656,"range":{"start_line":656,"start_character":16,"end_line":656,"end_character":66},"in_reply_to":"3fa7e38b_84e1a62e","updated":"2019-11-27 09:57:13.000000000","message":"When a NotFound exception is raised the deletion of the resources is ignored[1][2], so I don\u0027t think there\u0027s a need to catch it here.\nWhy should I retry the deletion, if when resourceNotReady exception is raised all the event will be retried?\n\n[1] https://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/controller/drivers/lbaasv2.py#L725-L736\n[2] https://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/controller/drivers/lbaasv2.py#L115-L135","commit_id":"011547515ec1c3a96920176aa5bc22326985a805"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"1528e8ed889bd2b68873fdf413451499fce2990f","unresolved":false,"context_lines":[{"line_number":653,"context_line":"        loadbalancers \u003d lbaas_client.load_balancers(**lbaas_spec)"},{"line_number":654,"context_line":"        for loadbalancer in loadbalancers:"},{"line_number":655,"context_line":"            if not services_dict.get(loadbalancer.name):"},{"line_number":656,"context_line":"                self._drv_lbaas.release_loadbalancer(loadbalancer)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_aa633919","line":656,"range":{"start_line":656,"start_character":16,"end_line":656,"end_character":66},"in_reply_to":"3fa7e38b_8ace7d8d","updated":"2019-11-27 10:23:40.000000000","message":"Right about the notFound. \nBut for the retry... This is not a on_present or on_delete event, but something triggered while creating the handler object, so I don\u0027t think it is retry. Plus if you have a long list you don\u0027t want to wait for the retries to delete the others as it can take a long time","commit_id":"011547515ec1c3a96920176aa5bc22326985a805"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"79be7d447caa95cc91732c8b4c8a7e69dabbb9a2","unresolved":false,"context_lines":[{"line_number":653,"context_line":"        loadbalancers \u003d lbaas_client.load_balancers(**lbaas_spec)"},{"line_number":654,"context_line":"        for loadbalancer in loadbalancers:"},{"line_number":655,"context_line":"            if not services_dict.get(loadbalancer.name):"},{"line_number":656,"context_line":"                self._drv_lbaas.release_loadbalancer(loadbalancer)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa7e38b_16c2af54","line":656,"range":{"start_line":656,"start_character":16,"end_line":656,"end_character":66},"in_reply_to":"3fa7e38b_aa633919","updated":"2019-11-27 16:36:06.000000000","message":"Also I think this method probably expects this object, not the one from the client: https://github.com/openstack/kuryr-kubernetes/blob/dfa9a392f1d3b777a68adda9f5e06d36575bf5a1/kuryr_kubernetes/objects/lbaas.py#L24\n\nI think Luis is right about ResourceNotFound - spawning a green thread will give it a new stack, so ResourceNotFound will just go to the top of it and will never be caught by where you\u0027ve run eventlet.spawn from.","commit_id":"011547515ec1c3a96920176aa5bc22326985a805"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"705ef80b5aded6b3646a5c2b3a1c908e6ade6847","unresolved":false,"context_lines":[{"line_number":647,"context_line":"            lb_obj \u003d obj_lbaas.LBaaSLoadBalancer(**loadbalancer)"},{"line_number":648,"context_line":"            if not services_dict.get(lb_obj.name):"},{"line_number":649,"context_line":"                retry \u003d True"},{"line_number":650,"context_line":"                while retry:"},{"line_number":651,"context_line":"                    try:"},{"line_number":652,"context_line":"                        self._drv_lbaas.release_loadbalancer(lb_obj)"},{"line_number":653,"context_line":"                        retry \u003d False"},{"line_number":654,"context_line":"                    except k_exc.ResourceNotReady:"},{"line_number":655,"context_line":"                        LOG.debug(\"Cleanup of loadbalancer %s failed. A retry \""},{"line_number":656,"context_line":"                                  \"will be triggered.\", lb_obj.name)"},{"line_number":657,"context_line":"                        retry \u003d True"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_9ed8e652","line":657,"range":{"start_line":650,"start_character":0,"end_line":657,"end_character":36},"updated":"2019-11-28 16:35:19.000000000","message":"If we want to do it this way, this got to be exponential and have a timeout. There\u0027s a helper function [1] and a bit overcomplicated example on how to use it [2].\n\n[1] https://github.com/openstack/kuryr-kubernetes/blob/1b97158c89918501e573ed42e1550ec7353d33b4/kuryr_kubernetes/utils.py#L111\n[2] https://github.com/openstack/kuryr-kubernetes/blob/94bb9f88044a8c7f60d45a524245441f65d59c6f/kuryr_kubernetes/watcher.py#L175","commit_id":"af34b18c8f1c1ba73ded4528c710d7759eac4421"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"f08f237133b94ea8765e0dd3cd7429f9c001acb1","unresolved":false,"context_lines":[{"line_number":647,"context_line":"            lb_obj \u003d obj_lbaas.LBaaSLoadBalancer(**loadbalancer)"},{"line_number":648,"context_line":"            if not services_dict.get(lb_obj.name):"},{"line_number":649,"context_line":"                retry \u003d True"},{"line_number":650,"context_line":"                while retry:"},{"line_number":651,"context_line":"                    try:"},{"line_number":652,"context_line":"                        self._drv_lbaas.release_loadbalancer(lb_obj)"},{"line_number":653,"context_line":"                        retry \u003d False"},{"line_number":654,"context_line":"                    except k_exc.ResourceNotReady:"},{"line_number":655,"context_line":"                        LOG.debug(\"Cleanup of loadbalancer %s failed. A retry \""},{"line_number":656,"context_line":"                                  \"will be triggered.\", lb_obj.name)"},{"line_number":657,"context_line":"                        retry \u003d True"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_54000601","line":657,"range":{"start_line":650,"start_character":0,"end_line":657,"end_character":36},"in_reply_to":"3fa7e38b_030b3ad5","updated":"2019-11-29 12:27:48.000000000","message":"I didn\u0027t see this comment, sry. That\u0027s a good point, thanks!","commit_id":"af34b18c8f1c1ba73ded4528c710d7759eac4421"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"f93c9b217ff6d5e6fddac17c7267ddff27caa472","unresolved":false,"context_lines":[{"line_number":647,"context_line":"            lb_obj \u003d obj_lbaas.LBaaSLoadBalancer(**loadbalancer)"},{"line_number":648,"context_line":"            if not services_dict.get(lb_obj.name):"},{"line_number":649,"context_line":"                retry \u003d True"},{"line_number":650,"context_line":"                while retry:"},{"line_number":651,"context_line":"                    try:"},{"line_number":652,"context_line":"                        self._drv_lbaas.release_loadbalancer(lb_obj)"},{"line_number":653,"context_line":"                        retry \u003d False"},{"line_number":654,"context_line":"                    except k_exc.ResourceNotReady:"},{"line_number":655,"context_line":"                        LOG.debug(\"Cleanup of loadbalancer %s failed. A retry \""},{"line_number":656,"context_line":"                                  \"will be triggered.\", lb_obj.name)"},{"line_number":657,"context_line":"                        retry \u003d True"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_94ec3ebc","line":657,"range":{"start_line":650,"start_character":0,"end_line":657,"end_character":36},"in_reply_to":"3fa7e38b_54000601","updated":"2019-11-29 13:32:02.000000000","message":"Actually, with the latest version of this PS all the lbaas will have a chance to get deleted, as it\u0027s being retried for a certain amount, and if the number max of attempts is reached it will proceed to the next lbaas.","commit_id":"af34b18c8f1c1ba73ded4528c710d7759eac4421"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d06ea76f6c3639eb644fa02c7bd0c2887ff5a912","unresolved":false,"context_lines":[{"line_number":647,"context_line":"            lb_obj \u003d obj_lbaas.LBaaSLoadBalancer(**loadbalancer)"},{"line_number":648,"context_line":"            if not services_dict.get(lb_obj.name):"},{"line_number":649,"context_line":"                retry \u003d True"},{"line_number":650,"context_line":"                while retry:"},{"line_number":651,"context_line":"                    try:"},{"line_number":652,"context_line":"                        self._drv_lbaas.release_loadbalancer(lb_obj)"},{"line_number":653,"context_line":"                        retry \u003d False"},{"line_number":654,"context_line":"                    except k_exc.ResourceNotReady:"},{"line_number":655,"context_line":"                        LOG.debug(\"Cleanup of loadbalancer %s failed. A retry \""},{"line_number":656,"context_line":"                                  \"will be triggered.\", lb_obj.name)"},{"line_number":657,"context_line":"                        retry \u003d True"}],"source_content_type":"text/x-python","patch_set":8,"id":"3fa7e38b_030b3ad5","line":657,"range":{"start_line":650,"start_character":0,"end_line":657,"end_character":36},"in_reply_to":"3fa7e38b_9ed8e652","updated":"2019-11-29 08:42:35.000000000","message":"I would handle the retry in the outer loop, so that you first do a try to remove as many LBs as possible, and in the next loop you iterate over the remaining ones. This way, if octavia has problems removing one lbaas (stuck in PENDING_UPDATE), all the rest that won\u0027t have problems will be left behind","commit_id":"af34b18c8f1c1ba73ded4528c710d7759eac4421"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"3d06c9e946ad7bc95e3ddb6da0c66ef0b1baf13f","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            if lb_obj.name not in services_set:"},{"line_number":650,"context_line":"                self._ensure_release_lbaas(lb_obj)"},{"line_number":651,"context_line":""},{"line_number":652,"context_line":"    def _ensure_release_lbaas(self, lb_obj):"},{"line_number":653,"context_line":"        attempts \u003d 0"},{"line_number":654,"context_line":"        deadline \u003d 0"},{"line_number":655,"context_line":"        retry \u003d True"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_9869b38d","line":652,"range":{"start_line":652,"start_character":0,"end_line":652,"end_character":44},"updated":"2019-12-03 08:55:44.000000000","message":"I still think it is better to iterate on the loadbalancer first and then only retry the failing ones, rather than having to wait until the failing one timeout on the number of retries and then proceed with the next one. That said, no strong opinion! Perhaps we can also create threads when running _ensure_release_lbaas so that those are run concurrently","commit_id":"a704c3c5d9ac579ffe66b7e1afb0b16c7ae646c3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"9eb37fa6e16096b13f904897f8a7729185c3ad15","unresolved":false,"context_lines":[{"line_number":649,"context_line":"            if lb_obj.name not in services_set:"},{"line_number":650,"context_line":"                self._ensure_release_lbaas(lb_obj)"},{"line_number":651,"context_line":""},{"line_number":652,"context_line":"    def _ensure_release_lbaas(self, lb_obj):"},{"line_number":653,"context_line":"        attempts \u003d 0"},{"line_number":654,"context_line":"        deadline \u003d 0"},{"line_number":655,"context_line":"        retry \u003d True"}],"source_content_type":"text/x-python","patch_set":10,"id":"3fa7e38b_6e8fa0d3","line":652,"range":{"start_line":652,"start_character":0,"end_line":652,"end_character":44},"in_reply_to":"3fa7e38b_9869b38d","updated":"2019-12-15 19:00:33.000000000","message":"Done","commit_id":"a704c3c5d9ac579ffe66b7e1afb0b16c7ae646c3"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"8bc8fd45f818deed101dea7399ccd914afff1e5b","unresolved":false,"context_lines":[{"line_number":640,"context_line":"        try:"},{"line_number":641,"context_line":"            services \u003d driver_utils.get_services().get(\u0027items\u0027)"},{"line_number":642,"context_line":"        except k_exc.K8sClientException:"},{"line_number":643,"context_line":"            LOG.debug(\"Skipping cleanup of leftover lbaas.\""},{"line_number":644,"context_line":"                      \"Error retriving Kubernetes services\")"},{"line_number":645,"context_line":"            return"},{"line_number":646,"context_line":"        services_set \u003d set(\u0027{}/{}\u0027.format("}],"source_content_type":"text/x-python","patch_set":11,"id":"3fa7e38b_4e6e4b2d","line":643,"range":{"start_line":643,"start_character":57,"end_line":643,"end_character":58},"updated":"2019-12-17 09:56:08.000000000","message":"Missing space here.","commit_id":"d2e3aea728eb19d8df378a493f8fb0f6f1768c9c"}]}