)]}'
{".zuul.yaml":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f5b0355758ad60f9c88fe1bb1217ff716884f178","unresolved":true,"context_lines":[{"line_number":29,"context_line":"    name: kuryr-kubernetes-tempest-wallaby"},{"line_number":30,"context_line":"    parent: kuryr-kubernetes-tempest"},{"line_number":31,"context_line":"    override-checkout: stable/wallaby"},{"line_number":32,"context_line":"    vars:"},{"line_number":33,"context_line":"      tempest_plugins:"},{"line_number":34,"context_line":"        - octavia-tempest-plugin"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"- job:"},{"line_number":37,"context_line":"    name: kuryr-kubernetes-tempest-victoria"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"23a7f683_34d380ce","line":34,"range":{"start_line":32,"start_character":0,"end_line":34,"end_character":32},"updated":"2021-08-04 12:40:23.000000000","message":"not the right place for this, should be on kuryr-kubernetes-tempest","commit_id":"2a53bdc08fa6aabd0bcb109cf95a9c6c0753324c"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f3a10e140fc812aaa439751f2430e5c37280efc2","unresolved":true,"context_lines":[{"line_number":29,"context_line":"    name: kuryr-kubernetes-tempest-wallaby"},{"line_number":30,"context_line":"    parent: kuryr-kubernetes-tempest"},{"line_number":31,"context_line":"    override-checkout: stable/wallaby"},{"line_number":32,"context_line":"    "},{"line_number":33,"context_line":"- job:"},{"line_number":34,"context_line":"    name: kuryr-kubernetes-tempest-victoria"},{"line_number":35,"context_line":"    parent: kuryr-kubernetes-tempest"}],"source_content_type":"text/x-yaml","patch_set":41,"id":"1da6b6b6_d46de0e5","line":32,"updated":"2021-09-02 07:16:26.000000000","message":"remove spaces added","commit_id":"cd60638fea49126236eaf372d4d9156085c8ae7e"}],"kuryr_tempest_plugin/config.py":[{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"df8720984861aed56f26c70610e19e9dd84b3d02","unresolved":true,"context_lines":[{"line_number":99,"context_line":"    cfg.BoolOpt(\"test_configurable_listener_timeouts\", default\u003dFalse,"},{"line_number":100,"context_line":"                help\u003d\"Whether or not listener timeout values are \""},{"line_number":101,"context_line":"                \"configurable\"),"},{"line_number":102,"context_line":"    cfg.BoolOpt(\"enable_reconciliation\", default\u003dTrue,"},{"line_number":103,"context_line":"                help\u003d\"Whether or not reconciliation is enabled \"),"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":26,"id":"fd7f588e_3bd3357e","line":102,"range":{"start_line":102,"start_character":49,"end_line":102,"end_character":53},"updated":"2021-08-18 16:27:15.000000000","message":"The default should be set to False as the kuryr-kubernetes patch is enabling it","commit_id":"924c5a549807d9d3ab5c82461832de2ea86d8c42"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d41ba49337580c851673b7e21180101a00a7544c","unresolved":true,"context_lines":[{"line_number":99,"context_line":"    cfg.BoolOpt(\"test_configurable_listener_timeouts\", default\u003dFalse,"},{"line_number":100,"context_line":"                help\u003d\"Whether or not listener timeout values are \""},{"line_number":101,"context_line":"                \"configurable\"),"},{"line_number":102,"context_line":"    cfg.BoolOpt(\"enable_reconciliation\", default\u003dFalse,"},{"line_number":103,"context_line":"                help\u003d\"Whether or not reconciliation is enabled\"),"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":69,"id":"94b173b1_980cc5b3","line":104,"range":{"start_line":102,"start_character":1,"end_line":104,"end_character":0},"updated":"2021-09-16 11:41:50.000000000","message":"seems you have removed the lb_members_change_timeout option","commit_id":"721f6c82fb156f14c80ad4e4a8d66684b3debb3a"}],"kuryr_tempest_plugin/tests/scenario/base.py":[{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"131bf66ee880738c53c224585cd5ab5234cffc41","unresolved":true,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class BaseKuryrScenarioTest(manager.NetworkScenarioTest):"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, \u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    @classmethod"},{"line_number":68,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":1,"id":"453859c6_a17affa9","line":65,"updated":"2021-08-02 18:44:59.000000000","message":"This should be:\ncredentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]\nThis creates a credential \u0027lb_admin\u0027 in tempest with the \u0027load-balancer_admin\u0027 role in keystone.","commit_id":"c32de4ff6c89fbfc1eaeb4a53a7b84250cc54a0f"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"131bf66ee880738c53c224585cd5ab5234cffc41","unresolved":true,"context_lines":[{"line_number":79,"context_line":"    def setup_clients(cls):"},{"line_number":80,"context_line":"        super(BaseKuryrScenarioTest, cls).setup_clients()"},{"line_number":81,"context_line":"        cls.k8s_client \u003d k8s_client"},{"line_number":82,"context_line":"        cls.admin_lb_client \u003d cls.os_roles_lb_admin.load_balancer_v2.LoadbalancerClient(),"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"fd8ea353_68ac936b","line":82,"updated":"2021-08-02 18:44:59.000000000","message":"You may want to move all of this(Meaning, every change in this file) into the octavia specifc test suite as not all jobs may have the octavia-tempest-plugin loaded into tempest.","commit_id":"c32de4ff6c89fbfc1eaeb4a53a7b84250cc54a0f"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"5735a6a2f97509f0ff03d0488c24426151ab905d","unresolved":true,"context_lines":[{"line_number":1464,"context_line":"        self.assertEqual(annotated_values, lb_status_values)"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":""},{"line_number":1467,"context_line":"def wait_for_deleted_status_or_not_found("},{"line_number":1468,"context_line":"        show_client, id, status_key, check_interval, check_timeout,"},{"line_number":1469,"context_line":"        root_tag\u003dNone, **kwargs):"},{"line_number":1470,"context_line":"    \"\"\"Waits for an object to reach a DELETED status or be not found (404)."}],"source_content_type":"text/x-python","patch_set":44,"id":"3a436cb4_148ce2dd","line":1467,"range":{"start_line":1467,"start_character":0,"end_line":1467,"end_character":41},"updated":"2021-09-03 06:47:21.000000000","message":"this is not being used, and not needed based on the loop you have on the main method, so you can remove this","commit_id":"4f88495f983c8932ca70b345371a5f1d69e10802"}],"kuryr_tempest_plugin/tests/scenario/consts.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"a45c557121e8ab2723cededb7ebf6d2302675b7b","unresolved":true,"context_lines":[{"line_number":21,"context_line":"POD_STATUS_RETRIES \u003d 240"},{"line_number":22,"context_line":"NS_TIMEOUT \u003d 600"},{"line_number":23,"context_line":"LB_TIMEOUT \u003d 1200"},{"line_number":24,"context_line":"LB_RECONCILE_TIMEOUT \u003d 1200"}],"source_content_type":"text/x-python","patch_set":12,"id":"d42f2266_e827e5f1","line":24,"range":{"start_line":24,"start_character":23,"end_line":24,"end_character":27},"updated":"2021-08-10 13:56:53.000000000","message":"You\u0027re sure 20 minutes is required? How often is reconciliation run by default?","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"5466274e6e3aafb2d583dd85613a11b46175cc8c","unresolved":true,"context_lines":[{"line_number":21,"context_line":"POD_STATUS_RETRIES \u003d 240"},{"line_number":22,"context_line":"NS_TIMEOUT \u003d 600"},{"line_number":23,"context_line":"LB_TIMEOUT \u003d 1200"},{"line_number":24,"context_line":"LB_RECONCILE_TIMEOUT \u003d 1200"}],"source_content_type":"text/x-python","patch_set":12,"id":"b83a8e6f_fd89a5d6","line":24,"range":{"start_line":24,"start_character":23,"end_line":24,"end_character":27},"in_reply_to":"d42f2266_e827e5f1","updated":"2021-08-10 14:21:25.000000000","message":"By default, reconciliation runs every 10mins","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"79ee33a0b20c85dfed2b7e2fd51a7f53f0f3992c","unresolved":true,"context_lines":[{"line_number":21,"context_line":"POD_STATUS_RETRIES \u003d 240"},{"line_number":22,"context_line":"NS_TIMEOUT \u003d 600"},{"line_number":23,"context_line":"LB_TIMEOUT \u003d 1200"},{"line_number":24,"context_line":"LB_RECONCILE_TIMEOUT \u003d 1200"}],"source_content_type":"text/x-python","patch_set":13,"id":"c16555d7_4bd16b0c","line":24,"updated":"2021-08-12 10:53:42.000000000","message":"reconciliation sleep is 600 seconds, why timeout is twice as long? do we expect, that restoring loadbalancers takes that long?","commit_id":"1f4e2c80dc768790f94cd336abdf1a4851c0bcc3"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"5af42810a62f22be32de52f4ddcaf62bd76e8869","unresolved":true,"context_lines":[{"line_number":21,"context_line":"POD_STATUS_RETRIES \u003d 240"},{"line_number":22,"context_line":"NS_TIMEOUT \u003d 600"},{"line_number":23,"context_line":"LB_TIMEOUT \u003d 1200"},{"line_number":24,"context_line":"LB_RECONCILE_TIMEOUT \u003d 1200"}],"source_content_type":"text/x-python","patch_set":13,"id":"dbf13ee4_f90d31ef","line":24,"in_reply_to":"c16555d7_4bd16b0c","updated":"2021-08-12 11:32:38.000000000","message":"\u003e reconciliation sleep is 600 seconds, why timeout is twice as long? do we expect, that restoring loadbalancers takes that long?\n\nNot really, I feel I can slightly make the timeout to be more than the reconciliation sleep. For instance, if the reconciliation loop just runs now, it would run in the next 600 seconds, so I expect the timeout to come after the reconciliation loop is completed, maybe we could try to make it 600 seconds as well?","commit_id":"1f4e2c80dc768790f94cd336abdf1a4851c0bcc3"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"5735a6a2f97509f0ff03d0488c24426151ab905d","unresolved":true,"context_lines":[{"line_number":22,"context_line":"NS_TIMEOUT \u003d 600"},{"line_number":23,"context_line":"LB_TIMEOUT \u003d 1200"},{"line_number":24,"context_line":"LB_RECONCILE_TIMEOUT \u003d 1200"},{"line_number":25,"context_line":"PROVISIONING_STATUS \u003d \u0027provisioning_status\u0027"},{"line_number":26,"context_line":"DELETED \u003d \u0027DELETED\u0027"}],"source_content_type":"text/x-python","patch_set":44,"id":"ee16285e_b481e6a7","line":26,"range":{"start_line":25,"start_character":0,"end_line":26,"end_character":19},"updated":"2021-09-03 06:47:21.000000000","message":"you don\u0027t need this","commit_id":"4f88495f983c8932ca70b345371a5f1d69e10802"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2d60020da34f75ef8894a52bd07b236efa1b6e97","unresolved":true,"context_lines":[{"line_number":21,"context_line":"POD_STATUS_RETRIES \u003d 240"},{"line_number":22,"context_line":"NS_TIMEOUT \u003d 600"},{"line_number":23,"context_line":"LB_TIMEOUT \u003d 1200"},{"line_number":24,"context_line":"LB_RECONCILE_TIMEOUT \u003d 1500"}],"source_content_type":"text/x-python","patch_set":45,"id":"c137671e_9165dc7e","line":24,"range":{"start_line":24,"start_character":23,"end_line":24,"end_character":27},"updated":"2021-09-06 09:00:34.000000000","message":"Shouldn\u0027t it be 1800 or 1200 to attempt reconciliation at least 3 or 2 times as it\u0027s set to run every 600 seconds?","commit_id":"cb8dbf03cd37fb077abf9c7c9b344b1562fc7895"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"17aaf88f35f6dd6309396158478f8107c6317853","unresolved":true,"context_lines":[{"line_number":21,"context_line":"POD_STATUS_RETRIES \u003d 240"},{"line_number":22,"context_line":"NS_TIMEOUT \u003d 600"},{"line_number":23,"context_line":"LB_TIMEOUT \u003d 1200"},{"line_number":24,"context_line":"LB_RECONCILE_TIMEOUT \u003d 1800"}],"source_content_type":"text/x-python","patch_set":66,"id":"d7ef3d7f_0ced273c","line":24,"range":{"start_line":24,"start_character":23,"end_line":24,"end_character":27},"updated":"2021-09-13 15:15:11.000000000","message":"should this be 600? as that is the reconciliation timeout being used on kuryr-kubernetes? https://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/controller/service.py#L167","commit_id":"1f6923e1c47be3e40bf7a9e233679b6bcb410871"}],"kuryr_tempest_plugin/tests/scenario/test_service.py":[{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"131bf66ee880738c53c224585cd5ab5234cffc41","unresolved":true,"context_lines":[{"line_number":176,"context_line":"            service_name\u003d\"kuryr-listener-demo\")"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"class TestLoadBalancerReconciliation(base.BaseKuryrScenarioTest):"},{"line_number":179,"context_line":""},{"line_number":180,"context_line":"    @classmethod"},{"line_number":181,"context_line":"    def skip_checks(cls):"},{"line_number":182,"context_line":"        super(TestLoadBalancerReconciliation, cls).skip_checks()"}],"source_content_type":"text/x-python","patch_set":1,"id":"8aead692_0772b7cb","line":179,"updated":"2021-08-02 18:44:59.000000000","message":"You could move all of the changes from base to here.\n\nI.e. you can create a setup_clients and credentials here. See this example in Designate:\nhttps://github.com/openstack/designate-tempest-plugin/blob/master/designate_tempest_plugin/tests/api/v2/test_pool.py#L36","commit_id":"c32de4ff6c89fbfc1eaeb4a53a7b84250cc54a0f"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"45258cf3313952e504967626d7a83ac4e9525dd7","unresolved":true,"context_lines":[{"line_number":187,"context_line":"            raise cls.skipException(\"Service tests are not enabled\")"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"    @classmethod"},{"line_number":190,"context_line":"    def setup_credentials(cls):"},{"line_number":191,"context_line":"        super(TestLoadBalancerReconciliation, cls).setup_credentials()"},{"line_number":192,"context_line":""},{"line_number":193,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":3,"id":"d5806ae2_31dfe697","line":190,"updated":"2021-08-02 21:27:23.000000000","message":"This shouldn\u0027t be necessary as all you are calling here is super, which will run anyway.","commit_id":"a811c2545af53a347fa86b0eb15510e3f3568001"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"28bbefed92f3f1ca230cbb3eeee8fccafaa72ec2","unresolved":true,"context_lines":[{"line_number":201,"context_line":"            service_name\u003d\"kuryr-loadbalancer-demo\")"},{"line_number":202,"context_line":"        lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":203,"context_line":"        lbs_id \u003d [lb[\u0027id\u0027] for lb in lbs]"},{"line_number":204,"context_line":"        for lb_id in lbs_id:"},{"line_number":205,"context_line":"            self.lbaas.delete_loadbalancer(lb_id, cascade\u003dTrue)"},{"line_number":206,"context_line":"        #poll to wait for the delete event before checking"},{"line_number":207,"context_line":"        service_lb \u003d [lb for lb in lbs if lb[\u0027id\u0027] in lbs_id]"},{"line_number":208,"context_line":"        self.assertEmpty(service_lb)"}],"source_content_type":"text/x-python","patch_set":4,"id":"2213c64d_d694ae2f","line":205,"range":{"start_line":204,"start_character":0,"end_line":205,"end_character":63},"updated":"2021-08-03 07:35:35.000000000","message":"you don\u0027t need to remove all the loadbalancers! just the one created for the kuryr-loadbalancer-demo","commit_id":"4a37081b9b19b9cd85b43f7a68882f5d3ffabe77"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"28bbefed92f3f1ca230cbb3eeee8fccafaa72ec2","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        for lb_id in lbs_id:"},{"line_number":205,"context_line":"            self.lbaas.delete_loadbalancer(lb_id, cascade\u003dTrue)"},{"line_number":206,"context_line":"        #poll to wait for the delete event before checking"},{"line_number":207,"context_line":"        service_lb \u003d [lb for lb in lbs if lb[\u0027id\u0027] in lbs_id]"},{"line_number":208,"context_line":"        self.assertEmpty(service_lb)"},{"line_number":209,"context_line":"        # check if there is a connectivity to the reconcile loop"}],"source_content_type":"text/x-python","patch_set":4,"id":"9850e339_3db76778","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":61},"updated":"2021-08-03 07:35:35.000000000","message":"you would need to call again to obtain the list of loadbalancer.\n\nActually, I perhaps would not care that much about it, and instead do:\n- Create service  (+loadbalancer)\n- Check connectivity works\n- Delete the loadbalancer associated to the service\n- Check connectivity does not work\n- Wait for the reconciliation to happen\n- Check connectivity works again","commit_id":"4a37081b9b19b9cd85b43f7a68882f5d3ffabe77"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"f08c487e8ab4917ee42abb42900a09b08598f151","unresolved":true,"context_lines":[{"line_number":204,"context_line":"        for lb_id in lbs_id:"},{"line_number":205,"context_line":"            self.lbaas.delete_loadbalancer(lb_id, cascade\u003dTrue)"},{"line_number":206,"context_line":"        #poll to wait for the delete event before checking"},{"line_number":207,"context_line":"        service_lb \u003d [lb for lb in lbs if lb[\u0027id\u0027] in lbs_id]"},{"line_number":208,"context_line":"        self.assertEmpty(service_lb)"},{"line_number":209,"context_line":"        # check if there is a connectivity to the reconcile loop"}],"source_content_type":"text/x-python","patch_set":4,"id":"2553d2fa_04fe268a","line":207,"range":{"start_line":207,"start_character":8,"end_line":207,"end_character":61},"in_reply_to":"9850e339_3db76778","updated":"2021-08-03 07:40:23.000000000","message":"\u003e you would need to call again to obtain the list of loadbalancer.\n\u003e \n\u003e Actually, I perhaps would not care that much about it, and instead do:\n\u003e - Create service  (+loadbalancer)\n\u003e - Check connectivity works\n\u003e - Delete the loadbalancer associated to the service\n\u003e - Check connectivity does not work\n\u003e - Wait for the reconciliation to happen\n\u003e - Check connectivity works again\n\nThanks Luis. Please could you assist share quickstart links/resources about checking connectivity works implementation in case.\nOkay I need to only delete the loadbalancer created in \u0027kuryr-loadbalancer-demo\u0027","commit_id":"4a37081b9b19b9cd85b43f7a68882f5d3ffabe77"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f5b0355758ad60f9c88fe1bb1217ff716884f178","unresolved":true,"context_lines":[{"line_number":203,"context_line":"        self.create_setup_for_service_test("},{"line_number":204,"context_line":"            service_name\u003dservice_name)"},{"line_number":205,"context_line":"        self.check_service_internal_connectivity(service_name)"},{"line_number":206,"context_line":"        lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":207,"context_line":"        lb_id \u003d [lb[\u0027id\u0027] for lb in lbs if service_name in lb[\u0027name\u0027]]"},{"line_number":208,"context_line":"        self.lbaas.delete_loadbalancer(lb_id[0], cascade\u003dTrue)"},{"line_number":209,"context_line":"        start \u003d time.time()"},{"line_number":210,"context_line":"        while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":211,"context_line":"            time.sleep(5)"}],"source_content_type":"text/x-python","patch_set":7,"id":"c3fda9aa_742c747b","line":208,"range":{"start_line":206,"start_character":0,"end_line":208,"end_character":62},"updated":"2021-08-04 12:40:23.000000000","message":"perhaps better to obtain the loadbalancer id from the CRD status.\n\nAlso, you should check there is no connectivity after removing the loadbalancer","commit_id":"2a53bdc08fa6aabd0bcb109cf95a9c6c0753324c"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f5b0355758ad60f9c88fe1bb1217ff716884f178","unresolved":true,"context_lines":[{"line_number":206,"context_line":"        lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":207,"context_line":"        lb_id \u003d [lb[\u0027id\u0027] for lb in lbs if service_name in lb[\u0027name\u0027]]"},{"line_number":208,"context_line":"        self.lbaas.delete_loadbalancer(lb_id[0], cascade\u003dTrue)"},{"line_number":209,"context_line":"        start \u003d time.time()"},{"line_number":210,"context_line":"        while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":211,"context_line":"            time.sleep(5)"},{"line_number":212,"context_line":"        self.check_service_internal_connectivity(service_name)"},{"line_number":213,"context_line":"        self.assertNotEmpty(lbs)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bdfed9b0_82d3a77d","line":211,"range":{"start_line":209,"start_character":0,"end_line":211,"end_character":25},"updated":"2021-08-04 12:40:23.000000000","message":"no need to sleep from 5 to 5. this could be replaced simply by time.sleep(LB_RECONCILE_TIMEOUT)","commit_id":"2a53bdc08fa6aabd0bcb109cf95a9c6c0753324c"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"a45c557121e8ab2723cededb7ebf6d2302675b7b","unresolved":true,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"class TestLoadBalancerReconciliation(base.BaseKuryrScenarioTest):"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @classmethod"},{"line_number":186,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":12,"id":"b70c0ced_8b57ba32","line":183,"range":{"start_line":183,"start_character":0,"end_line":183,"end_character":75},"updated":"2021-08-10 13:56:53.000000000","message":"Where are those taken from? I\u0027m probably misunderstanding how Tempest clients work?","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"f719c9038bfe336fa7ead3b0f2326905f6919bb8","unresolved":true,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"class TestLoadBalancerReconciliation(base.BaseKuryrScenarioTest):"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @classmethod"},{"line_number":186,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":12,"id":"051f7671_6848de7b","line":183,"range":{"start_line":183,"start_character":0,"end_line":183,"end_character":75},"in_reply_to":"8f1c1a87_c1f06711","updated":"2021-08-11 20:58:00.000000000","message":"Octavia, by default, uses advanced RBAC (service specific member, reader, admin, etc.) roles (not to be confused with \"secure RBAC\"). lb_admin is a tempest credential, that can be used with the service clients, that is created with the load-balancer_admin role.","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"d4f2cc42af7cab0caf86d6752eafe25c9c368bdb","unresolved":true,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"class TestLoadBalancerReconciliation(base.BaseKuryrScenarioTest):"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    @classmethod"},{"line_number":186,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":12,"id":"8f1c1a87_c1f06711","line":183,"range":{"start_line":183,"start_character":0,"end_line":183,"end_character":75},"in_reply_to":"b70c0ced_8b57ba32","updated":"2021-08-11 18:37:11.000000000","message":"\u003e Where are those taken from? I\u0027m probably misunderstanding how Tempest clients work?\n\nThose are credentials necessary to call Octavia according to the Octavia-tempest-plugin credentials setup","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"a161087eeddc8b0e50c4059f154c18b7165cbb73","unresolved":true,"context_lines":[{"line_number":204,"context_line":"            service_name\u003dservice_name)"},{"line_number":205,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("},{"line_number":206,"context_line":"            service_name\u003dservice_name)"},{"line_number":207,"context_line":"        lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":208,"context_line":"        lb_id \u003d [lb[\u0027id\u0027] for lb in lbs if service_name in lb[\u0027name\u0027]]"},{"line_number":209,"context_line":"        self.lbaas.delete_loadbalancer(lb_id[0], cascade\u003dTrue)"},{"line_number":210,"context_line":"        time.sleep(30)"},{"line_number":211,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("}],"source_content_type":"text/x-python","patch_set":12,"id":"3e5bc28c_8da8ce52","line":208,"range":{"start_line":207,"start_character":0,"end_line":208,"end_character":70},"updated":"2021-08-10 11:00:29.000000000","message":"maybe the list_loadbalancers call can already filter the lb by name? which in this case would have the \"\u003cservice-namespace\u003e/\u003cservice-name\u003e\"","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"f719c9038bfe336fa7ead3b0f2326905f6919bb8","unresolved":true,"context_lines":[{"line_number":204,"context_line":"            service_name\u003dservice_name)"},{"line_number":205,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("},{"line_number":206,"context_line":"            service_name\u003dservice_name)"},{"line_number":207,"context_line":"        lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":208,"context_line":"        lb_id \u003d [lb[\u0027id\u0027] for lb in lbs if service_name in lb[\u0027name\u0027]]"},{"line_number":209,"context_line":"        self.lbaas.delete_loadbalancer(lb_id[0], cascade\u003dTrue)"},{"line_number":210,"context_line":"        time.sleep(30)"},{"line_number":211,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("}],"source_content_type":"text/x-python","patch_set":12,"id":"f657af34_6dce7099","line":208,"range":{"start_line":207,"start_character":0,"end_line":208,"end_character":70},"in_reply_to":"3e5bc28c_8da8ce52","updated":"2021-08-11 20:58:00.000000000","message":"Yes, list_loadbalancers(query_params\u003d{\"name\": service_name})","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"a161087eeddc8b0e50c4059f154c18b7165cbb73","unresolved":true,"context_lines":[{"line_number":207,"context_line":"        lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":208,"context_line":"        lb_id \u003d [lb[\u0027id\u0027] for lb in lbs if service_name in lb[\u0027name\u0027]]"},{"line_number":209,"context_line":"        self.lbaas.delete_loadbalancer(lb_id[0], cascade\u003dTrue)"},{"line_number":210,"context_line":"        time.sleep(30)"},{"line_number":211,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("},{"line_number":212,"context_line":"            service_name\u003dservice_name)"},{"line_number":213,"context_line":"        self.assertEmpty(pod_name)"}],"source_content_type":"text/x-python","patch_set":12,"id":"43bce9ff_8527a50f","line":210,"range":{"start_line":210,"start_character":8,"end_line":210,"end_character":22},"updated":"2021-08-10 11:00:29.000000000","message":"instead it might be safer to keep checking for the existence of the load balancer with a \"while\" till a certain period and timeout if the lb is still there after x seconds","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"a161087eeddc8b0e50c4059f154c18b7165cbb73","unresolved":true,"context_lines":[{"line_number":208,"context_line":"        lb_id \u003d [lb[\u0027id\u0027] for lb in lbs if service_name in lb[\u0027name\u0027]]"},{"line_number":209,"context_line":"        self.lbaas.delete_loadbalancer(lb_id[0], cascade\u003dTrue)"},{"line_number":210,"context_line":"        time.sleep(30)"},{"line_number":211,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("},{"line_number":212,"context_line":"            service_name\u003dservice_name)"},{"line_number":213,"context_line":"        self.assertEmpty(pod_name)"},{"line_number":214,"context_line":"        start \u003d time.time()"},{"line_number":215,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"}],"source_content_type":"text/x-python","patch_set":12,"id":"8de73c7c_1025bf34","line":212,"range":{"start_line":211,"start_character":0,"end_line":212,"end_character":38},"updated":"2021-08-10 11:00:29.000000000","message":"maybe the connectivity should be checked once it was verified the LB was created again.","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"a45c557121e8ab2723cededb7ebf6d2302675b7b","unresolved":true,"context_lines":[{"line_number":208,"context_line":"        lb_id \u003d [lb[\u0027id\u0027] for lb in lbs if service_name in lb[\u0027name\u0027]]"},{"line_number":209,"context_line":"        self.lbaas.delete_loadbalancer(lb_id[0], cascade\u003dTrue)"},{"line_number":210,"context_line":"        time.sleep(30)"},{"line_number":211,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("},{"line_number":212,"context_line":"            service_name\u003dservice_name)"},{"line_number":213,"context_line":"        self.assertEmpty(pod_name)"},{"line_number":214,"context_line":"        start \u003d time.time()"},{"line_number":215,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"}],"source_content_type":"text/x-python","patch_set":12,"id":"815817f6_5c987ae8","line":212,"range":{"start_line":211,"start_character":0,"end_line":212,"end_character":38},"in_reply_to":"8de73c7c_1025bf34","updated":"2021-08-10 13:56:53.000000000","message":"Yup, this will most likely fail with an assertion error because deeper in that function we do assertEqual.","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"a45c557121e8ab2723cededb7ebf6d2302675b7b","unresolved":true,"context_lines":[{"line_number":211,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("},{"line_number":212,"context_line":"            service_name\u003dservice_name)"},{"line_number":213,"context_line":"        self.assertEmpty(pod_name)"},{"line_number":214,"context_line":"        start \u003d time.time()"},{"line_number":215,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":216,"context_line":"            time.sleep(consts.LB_RECONCILE_TIMEOUT)"},{"line_number":217,"context_line":"        reconciled_lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":218,"context_line":"        self.assertNotEmpty(reconciled_lbs)"}],"source_content_type":"text/x-python","patch_set":12,"id":"4ecb0822_3667bdf8","line":216,"range":{"start_line":214,"start_character":0,"end_line":216,"end_character":51},"updated":"2021-08-10 13:56:53.000000000","message":"This doesn\u0027t make sense, the loop will be run exactly once, so it\u0027s equivalent of just running time.sleep(const.LB_RECONCILE_TIMEOUT). In each iteration the test should check if the LBs is recreated.","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"a45c557121e8ab2723cededb7ebf6d2302675b7b","unresolved":true,"context_lines":[{"line_number":214,"context_line":"        start \u003d time.time()"},{"line_number":215,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":216,"context_line":"            time.sleep(consts.LB_RECONCILE_TIMEOUT)"},{"line_number":217,"context_line":"        reconciled_lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":218,"context_line":"        self.assertNotEmpty(reconciled_lbs)"}],"source_content_type":"text/x-python","patch_set":12,"id":"8813388b_b4cb8e5a","line":217,"range":{"start_line":217,"start_character":54,"end_line":217,"end_character":56},"updated":"2021-08-10 13:56:53.000000000","message":"You can try filtering by LB name here.","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"a45c557121e8ab2723cededb7ebf6d2302675b7b","unresolved":true,"context_lines":[{"line_number":215,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":216,"context_line":"            time.sleep(consts.LB_RECONCILE_TIMEOUT)"},{"line_number":217,"context_line":"        reconciled_lbs \u003d self.lbaas.list_loadbalancers()"},{"line_number":218,"context_line":"        self.assertNotEmpty(reconciled_lbs)"}],"source_content_type":"text/x-python","patch_set":12,"id":"885420b3_6c00ae77","line":218,"range":{"start_line":218,"start_character":0,"end_line":218,"end_character":43},"updated":"2021-08-10 13:56:53.000000000","message":"The tests are run in parallel, so you need to search for that exact LB namespace and name, otherwise if other test created a service and LB the check will pass even if reconcile haven\u0027t worked.","commit_id":"840d61733c298d43f0e99c5cd44eb4b1bb5a484c"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"79ee33a0b20c85dfed2b7e2fd51a7f53f0f3992c","unresolved":true,"context_lines":[{"line_number":189,"context_line":"        if not CONF.kuryr_kubernetes.service_tests_enabled:"},{"line_number":190,"context_line":"            raise cls.skipException(\"Service tests are not enabled\")"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    @classmethod"},{"line_number":193,"context_line":"    def setup_credentials(cls):"},{"line_number":194,"context_line":"        super(TestLoadBalancerReconciliation, cls).setup_credentials()"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    @classmethod"},{"line_number":197,"context_line":"    def setup_clients(cls):"}],"source_content_type":"text/x-python","patch_set":13,"id":"993ecbfe_06c9e067","line":194,"range":{"start_line":192,"start_character":0,"end_line":194,"end_character":70},"updated":"2021-08-12 10:53:42.000000000","message":"Probably, you don\u0027t need this.","commit_id":"1f4e2c80dc768790f94cd336abdf1a4851c0bcc3"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"c6c01e9f698b184144f2d106871692780b7a1637","unresolved":true,"context_lines":[{"line_number":189,"context_line":"        if not CONF.kuryr_kubernetes.service_tests_enabled:"},{"line_number":190,"context_line":"            raise cls.skipException(\"Service tests are not enabled\")"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    @classmethod"},{"line_number":193,"context_line":"    def setup_credentials(cls):"},{"line_number":194,"context_line":"        super(TestLoadBalancerReconciliation, cls).setup_credentials()"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    @classmethod"},{"line_number":197,"context_line":"    def setup_clients(cls):"}],"source_content_type":"text/x-python","patch_set":13,"id":"16bd1334_bcad3d57","line":194,"range":{"start_line":192,"start_character":0,"end_line":194,"end_character":70},"in_reply_to":"993ecbfe_06c9e067","updated":"2021-08-13 08:06:43.000000000","message":"\u003e Probably, you don\u0027t need this.\n\nI noticed that without this, I get this error: AttributeError: \u0027Manager\u0027 object has no attribute \u0027load_balancer_v2\u0027","commit_id":"1f4e2c80dc768790f94cd336abdf1a4851c0bcc3"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"232b74bfef8613de31a1bdea1775156a7643d6f1","unresolved":true,"context_lines":[{"line_number":208,"context_line":"                                                      \u0027status\u0027, {}).get("},{"line_number":209,"context_line":"                                                      \u0027loadbalancer\u0027, {}).get("},{"line_number":210,"context_line":"                                                      \u0027id\u0027, {})"},{"line_number":211,"context_line":"        self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":212,"context_line":"        start \u003d time.time()"},{"line_number":213,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":214,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":13,"id":"ee9cc819_a1a0ebdc","line":211,"range":{"start_line":211,"start_character":0,"end_line":211,"end_character":64},"updated":"2021-08-12 10:57:28.000000000","message":"you need to check connectivity before removing the loadbalancer to ensure it is working","commit_id":"1f4e2c80dc768790f94cd336abdf1a4851c0bcc3"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"232b74bfef8613de31a1bdea1775156a7643d6f1","unresolved":true,"context_lines":[{"line_number":210,"context_line":"                                                      \u0027id\u0027, {})"},{"line_number":211,"context_line":"        self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":212,"context_line":"        start \u003d time.time()"},{"line_number":213,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":214,"context_line":"            try:"},{"line_number":215,"context_line":"                time.sleep(10)"},{"line_number":216,"context_line":"                reconciled_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"8a852344_6ca0dca6","line":213,"range":{"start_line":213,"start_character":43,"end_line":213,"end_character":64},"updated":"2021-08-12 10:57:28.000000000","message":"this consts looks misleading... we just need to wait until the LB is deleted, not related to the reconcile tiemput","commit_id":"1f4e2c80dc768790f94cd336abdf1a4851c0bcc3"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"232b74bfef8613de31a1bdea1775156a7643d6f1","unresolved":true,"context_lines":[{"line_number":223,"context_line":"            msg \u003d (\u0027Timed out waiting for LoadBalancer %s deletion\u0027 %"},{"line_number":224,"context_line":"                   klb_crd_id)"},{"line_number":225,"context_line":"            raise lib_exc.TimeoutException(msg)"},{"line_number":226,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("},{"line_number":227,"context_line":"            service_name\u003dservice_name)"},{"line_number":228,"context_line":"        self.assertNotEmpty(pod_name)"},{"line_number":229,"context_line":"        reconciled_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":230,"context_line":"        self.assertEqual(reconciled_lb[\u0027id\u0027], klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":13,"id":"cca749fa_51dee960","line":228,"range":{"start_line":226,"start_character":0,"end_line":228,"end_character":37},"updated":"2021-08-12 10:57:28.000000000","message":"here you need to do two things:\n- Check there is no connectivity\n- Wait LB REcONCILTE TIMEOUT\n- check there is connectivity now","commit_id":"1f4e2c80dc768790f94cd336abdf1a4851c0bcc3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"d6ec245a0d6bcec4b7609d8271e4c7e92a984723","unresolved":true,"context_lines":[{"line_number":215,"context_line":"                                                          {}).get("},{"line_number":216,"context_line":"                                                          \u0027loadbalancer\u0027,"},{"line_number":217,"context_line":"                                                          {}).get(\u0027id\u0027, {})"},{"line_number":218,"context_line":"        except kubernetes.client.rest.ApiException:"},{"line_number":219,"context_line":"            return None, None"},{"line_number":220,"context_line":"        try:"},{"line_number":221,"context_line":"            self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":222,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":22,"id":"62cba46c_87c42e6d","line":219,"range":{"start_line":218,"start_character":0,"end_line":219,"end_character":29},"updated":"2021-08-16 10:05:40.000000000","message":"If there was a failure around the Kubernetes API this would silently ignore it, it might better to not handle the Exception so we have an idea of what might have happened.","commit_id":"e716511890b1f80dd8c1472e9d2bcd016b33e0f7"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"d6ec245a0d6bcec4b7609d8271e4c7e92a984723","unresolved":true,"context_lines":[{"line_number":219,"context_line":"            return None, None"},{"line_number":220,"context_line":"        try:"},{"line_number":221,"context_line":"            self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":222,"context_line":"        except Exception:"},{"line_number":223,"context_line":"            LOG.debug(\"Error deleting LoadBalancer %s\", klb_crd_id)"},{"line_number":224,"context_line":"            return"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        if not self.check_service_internal_connectivity("},{"line_number":227,"context_line":"                service_name\u003dservice_name):"}],"source_content_type":"text/x-python","patch_set":22,"id":"71aa3a53_bcb04d74","line":224,"range":{"start_line":222,"start_character":0,"end_line":224,"end_character":18},"updated":"2021-08-16 10:05:40.000000000","message":"ditto","commit_id":"e716511890b1f80dd8c1472e9d2bcd016b33e0f7"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"d6ec245a0d6bcec4b7609d8271e4c7e92a984723","unresolved":true,"context_lines":[{"line_number":230,"context_line":"                try:"},{"line_number":231,"context_line":"                    time.sleep(10)"},{"line_number":232,"context_line":"                    os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":233,"context_line":"                    if os_lb[\u0027id\u0027] \u003d\u003d klb_crd_id:"},{"line_number":234,"context_line":"                        break"},{"line_number":235,"context_line":"                except Exception as e:"},{"line_number":236,"context_line":"                    if e.code \u003d\u003d 404:"},{"line_number":237,"context_line":"                        continue"}],"source_content_type":"text/x-python","patch_set":22,"id":"47aab09d_46b92c99","line":234,"range":{"start_line":233,"start_character":0,"end_line":234,"end_character":29},"updated":"2021-08-16 10:05:40.000000000","message":"Should this be the other way around? if the lb id is equal to the one defined on the CR it should \"continue\" and only break when the load-balancer is not found","commit_id":"e716511890b1f80dd8c1472e9d2bcd016b33e0f7"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"d6ec245a0d6bcec4b7609d8271e4c7e92a984723","unresolved":true,"context_lines":[{"line_number":245,"context_line":"        # if there is a connectivity now, that means the LoadBalancer"},{"line_number":246,"context_line":"        # is reconciled"},{"line_number":247,"context_line":"        self.check_service_internal_connectivity(service_name\u003dservice_name)"},{"line_number":248,"context_line":"        reconciled_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":249,"context_line":"        self.assertEqual(reconciled_lb[\u0027id\u0027], klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":22,"id":"74e62b6b_ba234cee","line":249,"range":{"start_line":248,"start_character":0,"end_line":249,"end_character":57},"updated":"2021-08-16 10:05:40.000000000","message":"Seems the same load-balancer since the beginning of the test is being checked. Instead we should fetch the CRD again and see if the LB has a different ID.","commit_id":"e716511890b1f80dd8c1472e9d2bcd016b33e0f7"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"df8720984861aed56f26c70610e19e9dd84b3d02","unresolved":true,"context_lines":[{"line_number":190,"context_line":"        if not CONF.kuryr_kubernetes.service_tests_enabled:"},{"line_number":191,"context_line":"            raise cls.skipException(\"Service tests are not enabled\")"},{"line_number":192,"context_line":"        if not CONF.kuryr_kubernetes.enable_reconciliation:"},{"line_number":193,"context_line":"            raise cls.skipException(\"Reconciliation is not enabled)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    @classmethod"},{"line_number":196,"context_line":"    def setup_credentials(cls):"}],"source_content_type":"text/x-python","patch_set":26,"id":"7de86256_a5454652","line":193,"range":{"start_line":193,"start_character":35,"end_line":193,"end_character":67},"updated":"2021-08-18 16:27:15.000000000","message":"missing \"","commit_id":"924c5a549807d9d3ab5c82461832de2ea86d8c42"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2028c63de78c82b735e18585f249d26a5c2de03c","unresolved":true,"context_lines":[{"line_number":183,"context_line":"class TestLoadBalancerReconciliation(base.BaseKuryrScenarioTest):"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":186,"context_line":"    CONF.kuryr_kubernetes.enable_reconciliation \u003d True"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    @classmethod"},{"line_number":189,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":28,"id":"7b12ba74_1bc54ebc","line":186,"range":{"start_line":186,"start_character":4,"end_line":186,"end_character":54},"updated":"2021-08-19 14:36:47.000000000","message":"this is always setting the configuration to true and consequently running the tests on some gates that didn\u0027t have it enabled","commit_id":"c8f49d1f1cd49ce3d008c7e2495e8f03bb371ec1"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"dc5fae5d6b71ebe3d89eb0340a32ef21f495d9a4","unresolved":true,"context_lines":[{"line_number":213,"context_line":"                                                      \u0027loadbalancer\u0027,"},{"line_number":214,"context_line":"                                                      {}).get(\u0027id\u0027, {})"},{"line_number":215,"context_line":"        deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":216,"context_line":"                                                    ignore_errors\u003dTrue,"},{"line_number":217,"context_line":"                                                    cascade\u003dTrue)"},{"line_number":218,"context_line":"        if not deleted_lb:"},{"line_number":219,"context_line":"            os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":28,"id":"aa4273e6_d884c5df","line":216,"range":{"start_line":216,"start_character":52,"end_line":216,"end_character":70},"updated":"2021-08-19 14:56:39.000000000","message":"why ignore errors?","commit_id":"c8f49d1f1cd49ce3d008c7e2495e8f03bb371ec1"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"dc5fae5d6b71ebe3d89eb0340a32ef21f495d9a4","unresolved":true,"context_lines":[{"line_number":217,"context_line":"                                                    cascade\u003dTrue)"},{"line_number":218,"context_line":"        if not deleted_lb:"},{"line_number":219,"context_line":"            os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":220,"context_line":"            while os_lb:"},{"line_number":221,"context_line":"                LOG.debug(\u0027Waiting for the loadbalancer %s to be deleted\u0027,"},{"line_number":222,"context_line":"                          service_name)"},{"line_number":223,"context_line":"                os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":224,"context_line":"                if os_lb:"},{"line_number":225,"context_line":"                    continue"},{"line_number":226,"context_line":"                else:"},{"line_number":227,"context_line":"                    break"},{"line_number":228,"context_line":"        else:"},{"line_number":229,"context_line":"            pass"},{"line_number":230,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("}],"source_content_type":"text/x-python","patch_set":28,"id":"33beb91b_d51b5af6","line":227,"range":{"start_line":220,"start_character":0,"end_line":227,"end_character":25},"updated":"2021-08-19 14:56:39.000000000","message":"Using a timeout similar to line 234 might be safer just in case octavia api is down this test  would hang forever. And the condition could be simplified to something in the lines of:\n\nif not os_lb:\n    break","commit_id":"c8f49d1f1cd49ce3d008c7e2495e8f03bb371ec1"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"4a906108229d6110aa0b993d58e1d7febe36198c","unresolved":true,"context_lines":[{"line_number":217,"context_line":"                                                    cascade\u003dTrue)"},{"line_number":218,"context_line":"        if not deleted_lb:"},{"line_number":219,"context_line":"            os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":220,"context_line":"            while os_lb:"},{"line_number":221,"context_line":"                LOG.debug(\u0027Waiting for the loadbalancer %s to be deleted\u0027,"},{"line_number":222,"context_line":"                          service_name)"},{"line_number":223,"context_line":"                os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":224,"context_line":"                if os_lb:"},{"line_number":225,"context_line":"                    continue"},{"line_number":226,"context_line":"                else:"},{"line_number":227,"context_line":"                    break"},{"line_number":228,"context_line":"        else:"},{"line_number":229,"context_line":"            pass"},{"line_number":230,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("}],"source_content_type":"text/x-python","patch_set":28,"id":"da3a2a80_57657be6","line":227,"range":{"start_line":220,"start_character":0,"end_line":227,"end_character":25},"in_reply_to":"33beb91b_d51b5af6","updated":"2021-08-19 16:42:55.000000000","message":"\u003e Using a timeout similar to line 234 might be safer just in case octavia api is down this test  would hang forever. And the condition could be simplified to something in the lines of:\n\u003e \n\u003e if not os_lb:\n\u003e     break\n\nCool!","commit_id":"c8f49d1f1cd49ce3d008c7e2495e8f03bb371ec1"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"dc5fae5d6b71ebe3d89eb0340a32ef21f495d9a4","unresolved":true,"context_lines":[{"line_number":229,"context_line":"            pass"},{"line_number":230,"context_line":"        pod_name \u003d self.check_service_internal_connectivity("},{"line_number":231,"context_line":"            service_name\u003dservice_name)"},{"line_number":232,"context_line":"        if not pod_name:"},{"line_number":233,"context_line":"            start \u003d time.time()"},{"line_number":234,"context_line":"            while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":235,"context_line":"                try:"}],"source_content_type":"text/x-python","patch_set":28,"id":"1fbc3bc7_9a7e60f9","line":232,"range":{"start_line":232,"start_character":0,"end_line":232,"end_character":23},"updated":"2021-08-19 14:56:39.000000000","message":"seems this function always returns pod_name so there is no need for this check","commit_id":"c8f49d1f1cd49ce3d008c7e2495e8f03bb371ec1"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"dc5fae5d6b71ebe3d89eb0340a32ef21f495d9a4","unresolved":true,"context_lines":[{"line_number":234,"context_line":"            while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":235,"context_line":"                try:"},{"line_number":236,"context_line":"                    time.sleep(10)"},{"line_number":237,"context_line":"                    os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":238,"context_line":"                    if os_lb[\u0027id\u0027] \u003d\u003d klb_crd_id:"},{"line_number":239,"context_line":"                        continue"},{"line_number":240,"context_line":"                    elif not os_lb:"}],"source_content_type":"text/x-python","patch_set":28,"id":"2cf301bd_12d2be85","line":237,"range":{"start_line":237,"start_character":20,"end_line":237,"end_character":68},"updated":"2021-08-19 14:56:39.000000000","message":"this is the same ID of the loadbalancer that was deleted, there is no need to check again if it\u0027s present as loop on line 220 already checked it.\n\nInstead the CRD could be constantly retrieved till the lb_id on the status is different than the old one(deleted) and then check the connectivity.","commit_id":"c8f49d1f1cd49ce3d008c7e2495e8f03bb371ec1"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":183,"context_line":"class TestLoadBalancerReconciliation(base.BaseKuryrScenarioTest):"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":186,"context_line":"    CONF.kuryr_kubernetes.enable_reconciliation \u003d True"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    @classmethod"},{"line_number":189,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":29,"id":"bc050108_a72b9287","line":186,"range":{"start_line":186,"start_character":4,"end_line":186,"end_character":54},"updated":"2021-08-20 09:19:19.000000000","message":"Can this line be remove so we can see if the branch specific gates will be green again?","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"3425fef00e97342f1a058903047de77ea80734a8","unresolved":true,"context_lines":[{"line_number":183,"context_line":"class TestLoadBalancerReconciliation(base.BaseKuryrScenarioTest):"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":186,"context_line":"    CONF.kuryr_kubernetes.enable_reconciliation \u003d True"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    @classmethod"},{"line_number":189,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":29,"id":"4faf74aa_ae44dcfe","line":186,"range":{"start_line":186,"start_character":4,"end_line":186,"end_character":54},"in_reply_to":"bc050108_a72b9287","updated":"2021-08-20 10:22:20.000000000","message":"\u003e Can this line be remove so we can see if the branch specific gates will be green again?\n\nIf this line is removed, the test get skipped due to line #194. Hope that would work?","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":205,"context_line":"        self.create_setup_for_service_test(service_name\u003dservice_name)"},{"line_number":206,"context_line":"        self.check_service_internal_connectivity(service_name\u003dservice_name)"},{"line_number":207,"context_line":"        # if there is a connectivity"},{"line_number":208,"context_line":"        LOG.debug(\"Retrieving the loadbalancer from Kubernetes\")"},{"line_number":209,"context_line":"        klb_crd_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"},{"line_number":210,"context_line":"                                                      namespace).get("},{"line_number":211,"context_line":"                                                      \u0027status\u0027,"}],"source_content_type":"text/x-python","patch_set":29,"id":"b577d663_37c86eb2","line":208,"range":{"start_line":208,"start_character":34,"end_line":208,"end_character":62},"updated":"2021-08-20 09:19:19.000000000","message":"loadbalancer ID from KuryrLoadBalancer CR","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":214,"context_line":"                                                      {}).get(\u0027id\u0027, {})"},{"line_number":215,"context_line":"        deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":216,"context_line":"        LOG.debug(\"Deleting the loadbalancer from Octavia %s \", deleted_lb)"},{"line_number":217,"context_line":"        if not deleted_lb:"},{"line_number":218,"context_line":"            os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":219,"context_line":"            start \u003d time.time()"},{"line_number":220,"context_line":"            while time.time() - start \u003c consts.LB_TIMEOUT:"}],"source_content_type":"text/x-python","patch_set":29,"id":"a5845db0_8f16ee8d","line":217,"range":{"start_line":217,"start_character":8,"end_line":217,"end_character":26},"updated":"2021-08-20 09:19:19.000000000","message":"it seems that this check in not needed, it\u0027s ok to go directly to loadbalancer show","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":215,"context_line":"        deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":216,"context_line":"        LOG.debug(\"Deleting the loadbalancer from Octavia %s \", deleted_lb)"},{"line_number":217,"context_line":"        if not deleted_lb:"},{"line_number":218,"context_line":"            os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":219,"context_line":"            start \u003d time.time()"},{"line_number":220,"context_line":"            while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":221,"context_line":"                LOG.debug(\u0027Waiting for LoadBalancer %s to be deleted\u0027,"}],"source_content_type":"text/x-python","patch_set":29,"id":"98ec7b1e_19e23749","line":218,"range":{"start_line":218,"start_character":12,"end_line":218,"end_character":60},"updated":"2021-08-20 09:19:19.000000000","message":"looks like this can be removed as you\u0027re retrieving the lb again inside the loop","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":222,"context_line":"                          klb_crd_id)"},{"line_number":223,"context_line":"                time.sleep(10)"},{"line_number":224,"context_line":"                os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":225,"context_line":"                if os_lb:"},{"line_number":226,"context_line":"                    continue"},{"line_number":227,"context_line":"                else:"},{"line_number":228,"context_line":"                    break"},{"line_number":229,"context_line":"        else:"},{"line_number":230,"context_line":"            LOG.debug(\"TimedOut waiting for LoadBalancer %s to be deleted\","},{"line_number":231,"context_line":"                      klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":29,"id":"bbf7f94d_eafd9e75","line":228,"range":{"start_line":225,"start_character":0,"end_line":228,"end_character":25},"updated":"2021-08-20 09:19:19.000000000","message":"maybe this could be just:\n\nif not os_lb:\n    break","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":227,"context_line":"                else:"},{"line_number":228,"context_line":"                    break"},{"line_number":229,"context_line":"        else:"},{"line_number":230,"context_line":"            LOG.debug(\"TimedOut waiting for LoadBalancer %s to be deleted\","},{"line_number":231,"context_line":"                      klb_crd_id)"},{"line_number":232,"context_line":"            return"},{"line_number":233,"context_line":"        self.check_service_internal_connectivity(service_name\u003dservice_name)"},{"line_number":234,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":29,"id":"ac4b0af1_81d0ff3d","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":33},"updated":"2021-08-20 09:19:19.000000000","message":"this log would fit in the following condition:\n\nif os_lb:\n    Log.debug ...","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":234,"context_line":"        start \u003d time.time()"},{"line_number":235,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":236,"context_line":"            try:"},{"line_number":237,"context_line":"                time.sleep(10)"},{"line_number":238,"context_line":"                klb_crd_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"},{"line_number":239,"context_line":"                                                              namespace).get("},{"line_number":240,"context_line":"                                                              \u0027status\u0027,"}],"source_content_type":"text/x-python","patch_set":29,"id":"1c1330b9_fa195651","line":237,"range":{"start_line":237,"start_character":16,"end_line":237,"end_character":30},"updated":"2021-08-20 09:19:19.000000000","message":"is this sleep needed considering that the timeout is in place?","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":235,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":236,"context_line":"            try:"},{"line_number":237,"context_line":"                time.sleep(10)"},{"line_number":238,"context_line":"                klb_crd_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"},{"line_number":239,"context_line":"                                                              namespace).get("},{"line_number":240,"context_line":"                                                              \u0027status\u0027,"},{"line_number":241,"context_line":"                                                              {}).get("}],"source_content_type":"text/x-python","patch_set":29,"id":"dd4e0a75_e0a4c226","line":238,"range":{"start_line":238,"start_character":16,"end_line":238,"end_character":26},"updated":"2021-08-20 09:19:19.000000000","message":"this could be renamed to new_lb_id","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":241,"context_line":"                                                              {}).get("},{"line_number":242,"context_line":"                                                              \u0027loadbalancer\u0027,"},{"line_number":243,"context_line":"                                                              {}).get(\u0027id\u0027, {})"},{"line_number":244,"context_line":"                if deleted_lb \u003d\u003d klb_crd_id:"},{"line_number":245,"context_line":"                    continue"},{"line_number":246,"context_line":"                else:"},{"line_number":247,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":29,"id":"45337e36_af564868","line":244,"range":{"start_line":244,"start_character":16,"end_line":244,"end_character":44},"updated":"2021-08-20 09:19:19.000000000","message":"and then here you could compare new_lb_id \u003d\u003d klb_crd_id","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":245,"context_line":"                    continue"},{"line_number":246,"context_line":"                else:"},{"line_number":247,"context_line":"                    break"},{"line_number":248,"context_line":"            except Exception as e:"},{"line_number":249,"context_line":"                if e.code \u003d\u003d 404:"},{"line_number":250,"context_line":"                    continue"},{"line_number":251,"context_line":"        else:"},{"line_number":252,"context_line":"            msg \u003d (\u0027Timed out waiting for LoadBalancer %s deletion\u0027,"},{"line_number":253,"context_line":"                   klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":29,"id":"4c4005a0_edd31e6c","line":250,"range":{"start_line":248,"start_character":0,"end_line":250,"end_character":28},"updated":"2021-08-20 09:19:19.000000000","message":"Looks like this exception could be removed, if such failure happen we would want the user to be aware","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bbca1437702b8cff3c58ab507538b86703b1e735","unresolved":true,"context_lines":[{"line_number":255,"context_line":"        # if there is a connectivity now, that means the LoadBalancer"},{"line_number":256,"context_line":"        # is reconciled"},{"line_number":257,"context_line":"        self.check_service_internal_connectivity(service_name\u003dservice_name)"},{"line_number":258,"context_line":"        klb_crd_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"},{"line_number":259,"context_line":"                                                      namespace).get("},{"line_number":260,"context_line":"                                                      \u0027status\u0027,"},{"line_number":261,"context_line":"                                                      {}).get("},{"line_number":262,"context_line":"                                                      \u0027loadbalancer\u0027,"},{"line_number":263,"context_line":"                                                      {}).get(\u0027id\u0027, {})"},{"line_number":264,"context_line":"        reconciled_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":265,"context_line":"        self.assertEqual(reconciled_lb[\u0027id\u0027], klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":29,"id":"5e437b59_066b0581","line":265,"range":{"start_line":258,"start_character":0,"end_line":265,"end_character":57},"updated":"2021-08-20 09:19:19.000000000","message":"there is no need to fetch the CRD and check the ID again as that seems to be done on line 244","commit_id":"044b35ebcefa1af29859085e5ef3d721b115ed45"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"60d3a76012e0ba44c39b046a646bed76953f7b91","unresolved":true,"context_lines":[{"line_number":214,"context_line":"                                                      \u0027loadbalancer\u0027,"},{"line_number":215,"context_line":"                                                      {}).get(\u0027id\u0027, {})"},{"line_number":216,"context_line":"        deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":217,"context_line":"                                                    ignore_errors\u003dTrue,"},{"line_number":218,"context_line":"                                                    cascade\u003dTrue)"},{"line_number":219,"context_line":"        if not deleted_lb:"},{"line_number":220,"context_line":"            # NOTE(digitalsimboja: We need to await for DELETE to"}],"source_content_type":"text/x-python","patch_set":32,"id":"517334fd_39dc877a","line":217,"range":{"start_line":217,"start_character":52,"end_line":217,"end_character":70},"updated":"2021-08-23 08:51:54.000000000","message":"Is there any error popping up here?","commit_id":"45ebec443ab6c4f764733aeafa5e4dc576df7928"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"60d3a76012e0ba44c39b046a646bed76953f7b91","unresolved":true,"context_lines":[{"line_number":228,"context_line":"                if not os_lb:"},{"line_number":229,"context_line":"                    break"},{"line_number":230,"context_line":"            else:"},{"line_number":231,"context_line":"                if os_lb:"},{"line_number":232,"context_line":"                    LOG.debug(\"TimedOut waiting for LoadBalancer %s deletion\","},{"line_number":233,"context_line":"                              klb_crd_id)"},{"line_number":234,"context_line":"                return"},{"line_number":235,"context_line":"        else:"},{"line_number":236,"context_line":"            LOG.debug(\"Unable to delete loadbalancer %s \", klb_crd_id)"},{"line_number":237,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":32,"id":"c3e2c8a8_e21f9f03","line":234,"range":{"start_line":231,"start_character":0,"end_line":234,"end_character":22},"updated":"2021-08-23 08:51:54.000000000","message":"Instead of silently returning an exception could be raised with a message:\n\nraise lib_exc.TimeoutException(\"TimedOut waiting for LoadBalancer deletion\")","commit_id":"45ebec443ab6c4f764733aeafa5e4dc576df7928"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"60d3a76012e0ba44c39b046a646bed76953f7b91","unresolved":true,"context_lines":[{"line_number":235,"context_line":"        else:"},{"line_number":236,"context_line":"            LOG.debug(\"Unable to delete loadbalancer %s \", klb_crd_id)"},{"line_number":237,"context_line":"            return"},{"line_number":238,"context_line":"        self.check_service_internal_connectivity(service_name\u003dservice_name)"},{"line_number":239,"context_line":"        start \u003d time.time()"},{"line_number":240,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":241,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":32,"id":"2932356e_ef77a9cd","line":238,"range":{"start_line":238,"start_character":8,"end_line":238,"end_character":75},"updated":"2021-08-23 08:51:54.000000000","message":"If the load-balancer is gone at this point is there a need to check connectivity? I would expect the test to fail here as curl wouldn\u0027t work","commit_id":"45ebec443ab6c4f764733aeafa5e4dc576df7928"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2863cf564a49026c6475a34acbdb5e17ddbe3714","unresolved":true,"context_lines":[{"line_number":227,"context_line":"                    # NOTE(digitalsimboja: We need to await for DELETE to"},{"line_number":228,"context_line":"                    # complete on Octavia)"},{"line_number":229,"context_line":"                    start \u003d time.time()"},{"line_number":230,"context_line":"                    while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":231,"context_line":"                        LOG.debug(\u0027Waiting for LoadBalancer ID %s to be\u0027"},{"line_number":232,"context_line":"                                  \u0027deleted\u0027, klb_crd_id)"},{"line_number":233,"context_line":"                        os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":234,"context_line":"                        if not os_lb:"},{"line_number":235,"context_line":"                            break"},{"line_number":236,"context_line":"                        else:"},{"line_number":237,"context_line":"                            if os_lb:"},{"line_number":238,"context_line":"                                LOG.debug(\"TimedOut waiting for LoadBalancer\""},{"line_number":239,"context_line":"                                          \"%s deletion\", klb_crd_id)"},{"line_number":240,"context_line":"                            return"},{"line_number":241,"context_line":"            except Exception:"},{"line_number":242,"context_line":"                continue"},{"line_number":243,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":35,"id":"54b7a4d6_f77068ca","line":240,"range":{"start_line":230,"start_character":0,"end_line":240,"end_character":34},"updated":"2021-08-24 18:19:47.000000000","message":"It\u0027s better to have this inner loop after the first loop at line 246","commit_id":"08556c728ddcea80e944318deb0f2b74a00b5db2"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2863cf564a49026c6475a34acbdb5e17ddbe3714","unresolved":true,"context_lines":[{"line_number":238,"context_line":"                                LOG.debug(\"TimedOut waiting for LoadBalancer\""},{"line_number":239,"context_line":"                                          \"%s deletion\", klb_crd_id)"},{"line_number":240,"context_line":"                            return"},{"line_number":241,"context_line":"            except Exception:"},{"line_number":242,"context_line":"                continue"},{"line_number":243,"context_line":"        else:"},{"line_number":244,"context_line":"            LOG.debug(\"Unable to delete loadbalancer %s \", klb_crd_id)"},{"line_number":245,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for LoadBalancer\""}],"source_content_type":"text/x-python","patch_set":35,"id":"c55e96bc_cf993f41","line":242,"range":{"start_line":241,"start_character":1,"end_line":242,"end_character":24},"updated":"2021-08-24 18:19:47.000000000","message":"why any exception needs to be handled? I assume that if any issue happened we would want to be aware of that and not to continue","commit_id":"08556c728ddcea80e944318deb0f2b74a00b5db2"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"29cbfefac327b0c24b0d19280bf20c855c08b261","unresolved":true,"context_lines":[{"line_number":223,"context_line":"                deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":224,"context_line":"                                                            cascade\u003dTrue)"},{"line_number":225,"context_line":"                LOG.debug(\"Delete response from octavia\", deleted_lb)"},{"line_number":226,"context_line":"                if deleted_lb.get(\u0027status\u0027, {}) \u003d\u003d 204:"},{"line_number":227,"context_line":"                    break"},{"line_number":228,"context_line":"                else:"},{"line_number":229,"context_line":"                    continue"},{"line_number":230,"context_line":"            except Exception:"},{"line_number":231,"context_line":"                raise lib_exc.TimeoutException(\"TimedOut waiting for \""},{"line_number":232,"context_line":"                                               \"LoadBalancer deletion\")"}],"source_content_type":"text/x-python","patch_set":36,"id":"b6e13311_f9b33fc4","line":229,"range":{"start_line":226,"start_character":0,"end_line":229,"end_character":28},"updated":"2021-08-25 08:56:58.000000000","message":"could be simplified to:\nif deleted_lb.get(\u0027status\u0027, {}) !\u003d 204:\n    break","commit_id":"2b7fc1bb3dee36f89dd9a79515d815dc53808f78"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"29cbfefac327b0c24b0d19280bf20c855c08b261","unresolved":true,"context_lines":[{"line_number":227,"context_line":"                    break"},{"line_number":228,"context_line":"                else:"},{"line_number":229,"context_line":"                    continue"},{"line_number":230,"context_line":"            except Exception:"},{"line_number":231,"context_line":"                raise lib_exc.TimeoutException(\"TimedOut waiting for \""},{"line_number":232,"context_line":"                                               \"LoadBalancer deletion\")"},{"line_number":233,"context_line":"        # NOTE(digitalsimboja: We need to await for DELETE to"},{"line_number":234,"context_line":"        # complete on Octavia)"},{"line_number":235,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":36,"id":"2d44a012_b7defd6d","line":232,"range":{"start_line":230,"start_character":0,"end_line":232,"end_character":71},"updated":"2021-08-25 08:56:58.000000000","message":"this catching of exception doesn\u0027t look needed, the timeout could be raised on an else statement after the while, similar to what you did on line 242.","commit_id":"2b7fc1bb3dee36f89dd9a79515d815dc53808f78"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"29cbfefac327b0c24b0d19280bf20c855c08b261","unresolved":true,"context_lines":[{"line_number":239,"context_line":"            os_lb \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":240,"context_line":"            if not os_lb:"},{"line_number":241,"context_line":"                break"},{"line_number":242,"context_line":"            else:"},{"line_number":243,"context_line":"                if os_lb:"},{"line_number":244,"context_line":"                    LOG.debug(\"TimedOut waiting for LoadBalancer\""},{"line_number":245,"context_line":"                              \"%s deletion\", klb_crd_id)"},{"line_number":246,"context_line":"                    return"},{"line_number":247,"context_line":"        else:"},{"line_number":248,"context_line":"            LOG.debug(\"Unable to delete loadbalancer %s \", klb_crd_id)"},{"line_number":249,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for LoadBalancer\""}],"source_content_type":"text/x-python","patch_set":36,"id":"8d072a9f_7b8e33b5","line":246,"range":{"start_line":242,"start_character":0,"end_line":246,"end_character":26},"updated":"2021-08-25 08:56:58.000000000","message":"this doesn\u0027t seem needed as the timeout on line 249 would take care of that.","commit_id":"2b7fc1bb3dee36f89dd9a79515d815dc53808f78"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"29cbfefac327b0c24b0d19280bf20c855c08b261","unresolved":true,"context_lines":[{"line_number":258,"context_line":"                                                             {}).get("},{"line_number":259,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":260,"context_line":"                                                             {}).get(\u0027id\u0027, {})"},{"line_number":261,"context_line":"                if new_lb_id \u003d\u003d klb_crd_id:"},{"line_number":262,"context_line":"                    continue"},{"line_number":263,"context_line":"                else:"},{"line_number":264,"context_line":"                    break"},{"line_number":265,"context_line":"            except kubernetes.client.rest.ApiException:"},{"line_number":266,"context_line":"                return None, None"},{"line_number":267,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":36,"id":"22ef6d6b_d043d9aa","line":264,"range":{"start_line":261,"start_character":0,"end_line":264,"end_character":25},"updated":"2021-08-25 08:56:58.000000000","message":"could be simplified to:\n\nif new_lb_id !\u003d klb_crd_id:\n    break","commit_id":"2b7fc1bb3dee36f89dd9a79515d815dc53808f78"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"ffdf1e206492fc2f21f76b579b2eb96a9a26a420","unresolved":true,"context_lines":[{"line_number":222,"context_line":"            deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":223,"context_line":"                                                        cascade\u003dTrue)"},{"line_number":224,"context_line":"            LOG.debug(\"Delete response from octavia\", deleted_lb)"},{"line_number":225,"context_line":"            if deleted_lb.get(\u0027status\u0027, {}) !\u003d 204:"},{"line_number":226,"context_line":"                break"},{"line_number":227,"context_line":"        else:"},{"line_number":228,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for \""}],"source_content_type":"text/x-python","patch_set":37,"id":"52f08332_f913201e","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":51},"updated":"2021-08-26 16:19:09.000000000","message":"why is this look breaking when the status is different than 204? shouldn\u0027t it be the contrary?","commit_id":"3eabb755214ca039976da643aa357bc7559007d6"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"0e23c3cab5bec4f33db349bda1595add2abe0554","unresolved":true,"context_lines":[{"line_number":222,"context_line":"            deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":223,"context_line":"                                                        cascade\u003dTrue)"},{"line_number":224,"context_line":"            LOG.debug(\"Delete response from octavia\", deleted_lb)"},{"line_number":225,"context_line":"            if deleted_lb.get(\u0027status\u0027, {}) !\u003d 204:"},{"line_number":226,"context_line":"                break"},{"line_number":227,"context_line":"        else:"},{"line_number":228,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for \""}],"source_content_type":"text/x-python","patch_set":37,"id":"f12ab0a2_d33487a7","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":51},"in_reply_to":"22ac4926_e44568d5","updated":"2021-08-27 10:31:43.000000000","message":"\u003e As ignore_erros was not set I would expect the response status code to be returned[0] and in case of no content (status 204) to break the loop.\n\u003e \n\u003e [0] https://github.com/openstack/octavia-tempest-plugin/blob/master/octavia_tempest_plugin/services/load_balancer/v2/loadbalancer_client.py#L261-L262\n\u003e \n\u003e Perhaps the condition needs to be adapted to be \"deleted_lb.get(\u0027status\u0027, {}) \u003d\u003d 204:\"\n\nSure! I got response \u003d\u003d 204 and the execution continued thereafter","commit_id":"3eabb755214ca039976da643aa357bc7559007d6"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"a9ac1186e63d86912a1953ca399cc522068504d7","unresolved":true,"context_lines":[{"line_number":222,"context_line":"            deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":223,"context_line":"                                                        cascade\u003dTrue)"},{"line_number":224,"context_line":"            LOG.debug(\"Delete response from octavia\", deleted_lb)"},{"line_number":225,"context_line":"            if deleted_lb.get(\u0027status\u0027, {}) !\u003d 204:"},{"line_number":226,"context_line":"                break"},{"line_number":227,"context_line":"        else:"},{"line_number":228,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for \""}],"source_content_type":"text/x-python","patch_set":37,"id":"a003d8e0_c587a2a7","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":51},"in_reply_to":"52f08332_f913201e","updated":"2021-08-26 16:33:52.000000000","message":"\u003e why is this look breaking when the status is different than 204? shouldn\u0027t it be the contrary?\n\nPlease clarify, you mean you tested with status different than 204 and it breaks? From the API,the status code is returned if \u0027ignore-errors\u0027 is set to True, if the delete is success it returns 204","commit_id":"3eabb755214ca039976da643aa357bc7559007d6"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"84b8af1c07f83b2b2903a76db00b09469e6545ed","unresolved":true,"context_lines":[{"line_number":222,"context_line":"            deleted_lb \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":223,"context_line":"                                                        cascade\u003dTrue)"},{"line_number":224,"context_line":"            LOG.debug(\"Delete response from octavia\", deleted_lb)"},{"line_number":225,"context_line":"            if deleted_lb.get(\u0027status\u0027, {}) !\u003d 204:"},{"line_number":226,"context_line":"                break"},{"line_number":227,"context_line":"        else:"},{"line_number":228,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for \""}],"source_content_type":"text/x-python","patch_set":37,"id":"22ac4926_e44568d5","line":225,"range":{"start_line":225,"start_character":12,"end_line":225,"end_character":51},"in_reply_to":"a003d8e0_c587a2a7","updated":"2021-08-27 09:42:05.000000000","message":"As ignore_erros was not set I would expect the response status code to be returned[0] and in case of no content (status 204) to break the loop.\n\n[0] https://github.com/openstack/octavia-tempest-plugin/blob/master/octavia_tempest_plugin/services/load_balancer/v2/loadbalancer_client.py#L261-L262\n\nPerhaps the condition needs to be adapted to be \"deleted_lb.get(\u0027status\u0027, {}) \u003d\u003d 204:\"","commit_id":"3eabb755214ca039976da643aa357bc7559007d6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"3b43933aa848dcc11d3dbd01bbf7230ea817c24f","unresolved":true,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from oslo_log import log as logging"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"import kubernetes"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from kuryr_tempest_plugin.tests.scenario import base"},{"line_number":22,"context_line":"from kuryr_tempest_plugin.tests.scenario import consts"}],"source_content_type":"text/x-python","patch_set":39,"id":"de04804b_300d6b33","line":19,"range":{"start_line":19,"start_character":0,"end_line":19,"end_character":17},"updated":"2021-08-27 16:17:48.000000000","message":"This should be in the same section as oslo_log. Please review Import order template in\nOpenStack Style Guide [1].\n\n[1] https://docs.openstack.org/hacking/latest/user/hacking.html#imports","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"3b43933aa848dcc11d3dbd01bbf7230ea817c24f","unresolved":true,"context_lines":[{"line_number":21,"context_line":"from kuryr_tempest_plugin.tests.scenario import base"},{"line_number":22,"context_line":"from kuryr_tempest_plugin.tests.scenario import consts"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from tempest import config"},{"line_number":25,"context_line":"from tempest.lib import decorators"},{"line_number":26,"context_line":"from tempest.lib import exceptions as lib_exc"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":29,"context_line":"CONF \u003d config.CONF"}],"source_content_type":"text/x-python","patch_set":39,"id":"7e928e56_82568942","line":26,"range":{"start_line":24,"start_character":0,"end_line":26,"end_character":45},"updated":"2021-08-27 16:17:48.000000000","message":"I think those still should be in the section with oslo_config.","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ca64e2a7f33a5c8221742c65ff9aa1e12826fd26","unresolved":true,"context_lines":[{"line_number":216,"context_line":"                                                         {}).get(\u0027id\u0027, {})"},{"line_number":217,"context_line":"        except kubernetes.client.rest.ApiException:"},{"line_number":218,"context_line":"            return"},{"line_number":219,"context_line":"        start \u003d time.time()"},{"line_number":220,"context_line":"        while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":221,"context_line":"            response \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":222,"context_line":"                                                      cascade\u003dTrue)"},{"line_number":223,"context_line":"            LOG.debug(\"Delete response from octavia %s\", response)"},{"line_number":224,"context_line":"            if response \u003d\u003d 204:"},{"line_number":225,"context_line":"                LOG.info(\"Breaking from the DELETE loop\")"}],"source_content_type":"text/x-python","patch_set":39,"id":"ab1da804_7c8ab4bd","line":222,"range":{"start_line":219,"start_character":0,"end_line":222,"end_character":67},"updated":"2021-08-31 06:40:44.000000000","message":"delete_loadbalancer should be out of the loop, and then wait until there is no connectivity to the service","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"fd968a3d0f74e930ff4006c9e0c38fea042b4548","unresolved":true,"context_lines":[{"line_number":216,"context_line":"                                                         {}).get(\u0027id\u0027, {})"},{"line_number":217,"context_line":"        except kubernetes.client.rest.ApiException:"},{"line_number":218,"context_line":"            return"},{"line_number":219,"context_line":"        start \u003d time.time()"},{"line_number":220,"context_line":"        while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":221,"context_line":"            response \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":222,"context_line":"                                                      cascade\u003dTrue)"},{"line_number":223,"context_line":"            LOG.debug(\"Delete response from octavia %s\", response)"},{"line_number":224,"context_line":"            if response \u003d\u003d 204:"},{"line_number":225,"context_line":"                LOG.info(\"Breaking from the DELETE loop\")"}],"source_content_type":"text/x-python","patch_set":39,"id":"8e012239_0e42f2ae","line":222,"range":{"start_line":219,"start_character":0,"end_line":222,"end_character":67},"in_reply_to":"ab1da804_7c8ab4bd","updated":"2021-08-31 07:13:30.000000000","message":"\u003e delete_loadbalancer should be out of the loop, and then wait until there is no connectivity to the service\n\nOn Octavia, they await the LoadBalancer to be completely gone. So I am trying to mirror what they have here:\nhttps://github.com/openstack/octavia-tempest-plugin/blob/058ceaf0e7df0a911a6b07eff74f068c2e54502f/octavia_tempest_plugin/tests/api/v2/test_load_balancer.py#L154","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"3b43933aa848dcc11d3dbd01bbf7230ea817c24f","unresolved":true,"context_lines":[{"line_number":217,"context_line":"        except kubernetes.client.rest.ApiException:"},{"line_number":218,"context_line":"            return"},{"line_number":219,"context_line":"        start \u003d time.time()"},{"line_number":220,"context_line":"        while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":221,"context_line":"            response \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":222,"context_line":"                                                      cascade\u003dTrue)"},{"line_number":223,"context_line":"            LOG.debug(\"Delete response from octavia %s\", response)"},{"line_number":224,"context_line":"            if response \u003d\u003d 204:"},{"line_number":225,"context_line":"                LOG.info(\"Breaking from the DELETE loop\")"},{"line_number":226,"context_line":"                break"},{"line_number":227,"context_line":"        else:"},{"line_number":228,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for \""},{"line_number":229,"context_line":"                                           \"LoadBalancer deletion\")"}],"source_content_type":"text/x-python","patch_set":39,"id":"4247f995_d9c3f674","line":226,"range":{"start_line":220,"start_character":0,"end_line":226,"end_character":21},"updated":"2021-08-27 16:17:48.000000000","message":"Is this piece be supposed to wait for LB to be completely gone from the Octavia API? In such case you\u0027d need to query for it normally, not just wait for 204 which doesn\u0027t guarantee the LB is already gone.\n\nOr maybe goal is different? Is it to retry when Octavia is blocking deletion when LB is in PENDING_UPDATE?","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"74ccfe69bbd2cd7e192962fc8f4c966bcb8867d2","unresolved":true,"context_lines":[{"line_number":217,"context_line":"        except kubernetes.client.rest.ApiException:"},{"line_number":218,"context_line":"            return"},{"line_number":219,"context_line":"        start \u003d time.time()"},{"line_number":220,"context_line":"        while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":221,"context_line":"            response \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":222,"context_line":"                                                      cascade\u003dTrue)"},{"line_number":223,"context_line":"            LOG.debug(\"Delete response from octavia %s\", response)"},{"line_number":224,"context_line":"            if response \u003d\u003d 204:"},{"line_number":225,"context_line":"                LOG.info(\"Breaking from the DELETE loop\")"},{"line_number":226,"context_line":"                break"},{"line_number":227,"context_line":"        else:"},{"line_number":228,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for \""},{"line_number":229,"context_line":"                                           \"LoadBalancer deletion\")"}],"source_content_type":"text/x-python","patch_set":39,"id":"361ec72a_9f572ee3","line":226,"range":{"start_line":220,"start_character":0,"end_line":226,"end_character":21},"in_reply_to":"4247f995_d9c3f674","updated":"2021-08-27 17:02:52.000000000","message":"\u003e Is this piece be supposed to wait for LB to be completely gone from the Octavia API? In such case you\u0027d need to query for it normally, not just wait for 204 which doesn\u0027t guarantee the LB is already gone.\n\u003e \n\u003e Or maybe goal is different? Is it to retry when Octavia is blocking deletion when LB is in PENDING_UPDATE?\n\nIn previous solutions, we noticed that trying to list loadbalancer immediately after calling DELETE results in failure response. So the idea is to wait for loadbalancer to be completely gone. I could try to query for it but Octavia returns 204 if DELETE is a success","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"ca64e2a7f33a5c8221742c65ff9aa1e12826fd26","unresolved":true,"context_lines":[{"line_number":227,"context_line":"        else:"},{"line_number":228,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for \""},{"line_number":229,"context_line":"                                           \"LoadBalancer deletion\")"},{"line_number":230,"context_line":"        # NOTE(digitalsimboja: We need to await for DELETE to"},{"line_number":231,"context_line":"        # complete on Octavia)"},{"line_number":232,"context_line":"        LOG.debug(\"Continuing after Delete response from octavia %s\", response)"},{"line_number":233,"context_line":"        # wait for reconciliation"},{"line_number":234,"context_line":"        start \u003d time.time()"},{"line_number":235,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":236,"context_line":"            try:"},{"line_number":237,"context_line":"                LOG.debug(\"Checking for LoadBalancers Reconciliation\")"},{"line_number":238,"context_line":"                new_lb_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"}],"source_content_type":"text/x-python","patch_set":39,"id":"99df063c_6245a41c","line":235,"range":{"start_line":230,"start_character":3,"end_line":235,"end_character":64},"updated":"2021-08-31 06:40:44.000000000","message":"here you should wait the LB_RECONCILE_LOOP instead, and proceed with the checking of different IDs for the loadbalancer with a while loop that longs LB_TIMEOUT (i.e., the time to create an octavia load balancer","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"3b43933aa848dcc11d3dbd01bbf7230ea817c24f","unresolved":true,"context_lines":[{"line_number":240,"context_line":"                                                             \u0027status\u0027,"},{"line_number":241,"context_line":"                                                             {}).get("},{"line_number":242,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":243,"context_line":"                                                             {}).get(\u0027id\u0027, {})"},{"line_number":244,"context_line":"                if new_lb_id !\u003d klb_crd_id:"},{"line_number":245,"context_line":"                    LOG.info(\"LoadBalancer reconciliation completed %s\","},{"line_number":246,"context_line":"                             new_lb_id)"}],"source_content_type":"text/x-python","patch_set":39,"id":"f105dbec_0b4684c9","line":243,"range":{"start_line":243,"start_character":75,"end_line":243,"end_character":77},"updated":"2021-08-27 16:17:48.000000000","message":"nit: None would be better here.","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"3b43933aa848dcc11d3dbd01bbf7230ea817c24f","unresolved":true,"context_lines":[{"line_number":241,"context_line":"                                                             {}).get("},{"line_number":242,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":243,"context_line":"                                                             {}).get(\u0027id\u0027, {})"},{"line_number":244,"context_line":"                if new_lb_id !\u003d klb_crd_id:"},{"line_number":245,"context_line":"                    LOG.info(\"LoadBalancer reconciliation completed %s\","},{"line_number":246,"context_line":"                             new_lb_id)"},{"line_number":247,"context_line":"                    break"}],"source_content_type":"text/x-python","patch_set":39,"id":"95cced19_549484e5","line":244,"range":{"start_line":244,"start_character":19,"end_line":244,"end_character":42},"updated":"2021-08-27 16:17:48.000000000","message":"What if you\u0027ll hit the moment that `status` is removed from the KLB? new_lb_id becomes {} then and this condition will be True. Is that a success of the test even though the LB is not created yet? I think you should put this here:\n\n  if new_lb_id and new_lb_id !\u003d klb_crd_id:","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"3b43933aa848dcc11d3dbd01bbf7230ea817c24f","unresolved":true,"context_lines":[{"line_number":246,"context_line":"                             new_lb_id)"},{"line_number":247,"context_line":"                    break"},{"line_number":248,"context_line":"            except kubernetes.client.rest.ApiException:"},{"line_number":249,"context_line":"                return None, None"},{"line_number":250,"context_line":"        else:"},{"line_number":251,"context_line":"            msg \u003d (\u0027Timed out waiting for LoadBalancer %s reconciliation\u0027,"},{"line_number":252,"context_line":"                   klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":39,"id":"14f5739b_51e19aff","line":249,"range":{"start_line":249,"start_character":16,"end_line":249,"end_character":33},"updated":"2021-08-27 16:17:48.000000000","message":"It shouldn\u0027t be a success when K8s API broke, this line should rather do `continue`.","commit_id":"fa7a13fba9defc38b37d7ef61cdb8ab8308298b6"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f3a10e140fc812aaa439751f2430e5c37280efc2","unresolved":true,"context_lines":[{"line_number":221,"context_line":"        start \u003d time.time()"},{"line_number":222,"context_line":"        while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":223,"context_line":"            LOG.debug(\"Waiting for LoadBalancer to be completedly deleted\")"},{"line_number":224,"context_line":"            pod_name \u003d self.check_service_internal_connectivity("},{"line_number":225,"context_line":"                        service_name\u003dservice_name)"},{"line_number":226,"context_line":"            if not pod_name:"},{"line_number":227,"context_line":"                break"},{"line_number":228,"context_line":"            else:"},{"line_number":229,"context_line":"                continue"},{"line_number":230,"context_line":"        else:"},{"line_number":231,"context_line":"            raise lib_exc.TimeoutException(\"TimedOut waiting for \""},{"line_number":232,"context_line":"                                           \"LoadBalancer deletion\")"}],"source_content_type":"text/x-python","patch_set":41,"id":"cbd96f8e_195ff30f","line":229,"range":{"start_line":224,"start_character":0,"end_line":229,"end_character":24},"updated":"2021-09-02 07:16:26.000000000","message":"I don\u0027t think this is the right way. What is this pod_name being return? we need t o loop until the service connectivity is lost (or until the loadbalancer is deleted in openstack side)\n\nAlso, please put a sleep between iterations... otherwise this is going to be a lot of extra effort for the \"poor\" gates 😄","commit_id":"cd60638fea49126236eaf372d4d9156085c8ae7e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f3a10e140fc812aaa439751f2430e5c37280efc2","unresolved":true,"context_lines":[{"line_number":234,"context_line":"        LOG.debug(\"Continuing after Delete response from octavia\")"},{"line_number":235,"context_line":"        # wait for reconciliation"},{"line_number":236,"context_line":"        start \u003d time.time()"},{"line_number":237,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":238,"context_line":"            try:"},{"line_number":239,"context_line":"                LOG.debug(\"Checking for LoadBalancers Reconciliation\")"},{"line_number":240,"context_line":"                new_lb_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"}],"source_content_type":"text/x-python","patch_set":41,"id":"6d5bf855_281dd038","line":237,"range":{"start_line":237,"start_character":8,"end_line":237,"end_character":64},"updated":"2021-09-02 07:16:26.000000000","message":"it is possible that we will need to wait here for LB_RECONCILE_TIMEOUT (to give time to kuryr to start the recreation) + LB_TIMEOUT (to give time octavia to actually create and wire the loadbalancer afte the reconciliation)","commit_id":"cd60638fea49126236eaf372d4d9156085c8ae7e"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"7471b7fda146e8e5849418f06e0395e85780a4bf","unresolved":true,"context_lines":[{"line_number":234,"context_line":"        LOG.debug(\"Continuing after Delete response from octavia\")"},{"line_number":235,"context_line":"        # wait for reconciliation"},{"line_number":236,"context_line":"        start \u003d time.time()"},{"line_number":237,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":238,"context_line":"            try:"},{"line_number":239,"context_line":"                LOG.debug(\"Checking for LoadBalancers Reconciliation\")"},{"line_number":240,"context_line":"                new_lb_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"}],"source_content_type":"text/x-python","patch_set":41,"id":"ed085796_8c3059ed","line":237,"range":{"start_line":237,"start_character":8,"end_line":237,"end_character":64},"in_reply_to":"6d5bf855_281dd038","updated":"2021-09-03 05:55:47.000000000","message":"\u003e it is possible that we will need to wait here for LB_RECONCILE_TIMEOUT (to give time to kuryr to start the recreation) + LB_TIMEOUT (to give time octavia to actually create and wire the loadbalancer afte the reconciliation)\n\nPlease clarify this further. My understanding is increasing the wait time you mean?","commit_id":"cd60638fea49126236eaf372d4d9156085c8ae7e"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"a6bcb55e7bccf52e4c087308540ffd71d9c18481","unresolved":true,"context_lines":[{"line_number":234,"context_line":"        LOG.debug(\"Continuing after Delete response from octavia\")"},{"line_number":235,"context_line":"        # wait for reconciliation"},{"line_number":236,"context_line":"        start \u003d time.time()"},{"line_number":237,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":238,"context_line":"            try:"},{"line_number":239,"context_line":"                LOG.debug(\"Checking for LoadBalancers Reconciliation\")"},{"line_number":240,"context_line":"                new_lb_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"}],"source_content_type":"text/x-python","patch_set":41,"id":"32ec4f6a_3e241f56","line":237,"range":{"start_line":237,"start_character":8,"end_line":237,"end_character":64},"in_reply_to":"ed085796_8c3059ed","updated":"2021-09-03 06:04:22.000000000","message":"\u003e \u003e it is possible that we will need to wait here for LB_RECONCILE_TIMEOUT (to give time to kuryr to start the recreation) + LB_TIMEOUT (to give time octavia to actually create and wire the loadbalancer afte the reconciliation)\n\u003e \n\u003e Please clarify this further. My understanding is increasing the wait time you mean?\n\nI can make LB_RECONCILE_TIMEOUT to be 1500 seconds as the LB_TIMEOUT \u003d\u003d 1200 seconds. With this at least the loadbalancer would have been rebuilt and we can wait extra 5mins to be sure?","commit_id":"cd60638fea49126236eaf372d4d9156085c8ae7e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f3a10e140fc812aaa439751f2430e5c37280efc2","unresolved":true,"context_lines":[{"line_number":257,"context_line":"        # if there is a connectivity now, that means the LoadBalancer"},{"line_number":258,"context_line":"        # is reconciled"},{"line_number":259,"context_line":"        LOG.info(\"LoadBalancer successfully reconciled\")"},{"line_number":260,"context_line":"        self.assertEqual(new_lb_id, klb_crd_id)"},{"line_number":261,"context_line":"        self.check_service_internal_connectivity(service_name\u003dservice_name)"}],"source_content_type":"text/x-python","patch_set":41,"id":"c8fdc6fa_951630d9","line":260,"range":{"start_line":260,"start_character":8,"end_line":260,"end_character":47},"updated":"2021-09-02 07:16:26.000000000","message":"this should be assertNotEqual, right? it should be a new Octavia load balancer, which will have a different ID","commit_id":"cd60638fea49126236eaf372d4d9156085c8ae7e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"5735a6a2f97509f0ff03d0488c24426151ab905d","unresolved":true,"context_lines":[{"line_number":218,"context_line":"        # NOTE(digitalsimboja: We need to await for DELETE to"},{"line_number":219,"context_line":"        # complete on Octavia)"},{"line_number":220,"context_line":"        self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":221,"context_line":"        retries \u003d 120"},{"line_number":222,"context_line":"        while True:"},{"line_number":223,"context_line":"            try:"},{"line_number":224,"context_line":"                time.sleep(1)"},{"line_number":225,"context_line":"                self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":226,"context_line":"                retries -\u003d 1"},{"line_number":227,"context_line":"                self.assertNotEqual(0, retries,"},{"line_number":228,"context_line":"                                    \"Timed out waiting for loadbalancer %s to\""},{"line_number":229,"context_line":"                                    \" be deleted\" % service_name)"},{"line_number":230,"context_line":"            except Exception as e:"},{"line_number":231,"context_line":"                if e.status \u003d\u003d 404:"},{"line_number":232,"context_line":"                    LOG.info(\"LoadBalancer sucessfully deleted\")"},{"line_number":233,"context_line":"                    break"},{"line_number":234,"context_line":"        LOG.info(\"Wait for reconciliation after deletion of loadbalancer\")"},{"line_number":235,"context_line":"        start \u003d time.time()"},{"line_number":236,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"}],"source_content_type":"text/x-python","patch_set":44,"id":"9800665a_defdb6ee","line":233,"range":{"start_line":221,"start_character":0,"end_line":233,"end_character":25},"updated":"2021-09-03 06:47:21.000000000","message":"I would use the time instead of retries. And I would not try every second, perhaps every 5 seconds","commit_id":"4f88495f983c8932ca70b345371a5f1d69e10802"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"5735a6a2f97509f0ff03d0488c24426151ab905d","unresolved":true,"context_lines":[{"line_number":233,"context_line":"                    break"},{"line_number":234,"context_line":"        LOG.info(\"Wait for reconciliation after deletion of loadbalancer\")"},{"line_number":235,"context_line":"        start \u003d time.time()"},{"line_number":236,"context_line":"        while time.time() - start \u003c consts.LB_RECONCILE_TIMEOUT:"},{"line_number":237,"context_line":"            try:"},{"line_number":238,"context_line":"                LOG.debug(\"Checking for LoadBalancers Reconciliation\")"},{"line_number":239,"context_line":"                new_lb_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"}],"source_content_type":"text/x-python","patch_set":44,"id":"5de68e85_e9992907","line":236,"range":{"start_line":236,"start_character":4,"end_line":236,"end_character":64},"updated":"2021-09-03 06:47:21.000000000","message":"What I meant on the previous comment was that you need to wait here for two things: first for the reconciliation to happen, i.e., for triggering the recovery of the loadbalancer (LB_RECONCILE_TIMEOUT), but then you also need to wait for octavia to actually create the loadbalancer (i.e., addd an extra LB_TIMEOUT). Since, there are two steps:\n1) patch the status, which is when the reconciliation is triggered and the action to recreate the loadbalancer starts\n2) the recreation of the loadbalancer itself on the OpenStack side of things. And this takes time on the gates where this is run, so we need LB_TIMEOUT (the timeout we wait for the loadbalancer to be created)","commit_id":"4f88495f983c8932ca70b345371a5f1d69e10802"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2d60020da34f75ef8894a52bd07b236efa1b6e97","unresolved":true,"context_lines":[{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    @decorators.idempotent_id(\u0027da9bd886-e895-4869-b356-228c92a4da7f\u0027)"},{"line_number":202,"context_line":"    def test_loadbalancers_reconcilation(self):"},{"line_number":203,"context_line":"        service_name \u003d \"kuryr-mm-demo\""},{"line_number":204,"context_line":"        namespace \u003d \"default\""},{"line_number":205,"context_line":"        self.create_setup_for_service_test(service_name\u003dservice_name)"},{"line_number":206,"context_line":"        self.check_service_internal_connectivity(service_name\u003dservice_name)"}],"source_content_type":"text/x-python","patch_set":45,"id":"4ce66de5_2c827d07","line":203,"range":{"start_line":203,"start_character":24,"end_line":203,"end_character":37},"updated":"2021-09-06 09:00:34.000000000","message":"what \u0027mm\u0027 stands for? maybe something in the lines of kuryr-test might be better","commit_id":"cb8dbf03cd37fb077abf9c7c9b344b1562fc7895"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2d60020da34f75ef8894a52bd07b236efa1b6e97","unresolved":true,"context_lines":[{"line_number":237,"context_line":"                    break"},{"line_number":238,"context_line":"        LOG.info(\"Wait for reconciliation after deletion of loadbalancer\")"},{"line_number":239,"context_line":"        start \u003d time.time()"},{"line_number":240,"context_line":"        timeout \u003d consts.LB_RECONCILE_TIMEOUT + consts.LB_TIMEOUT"},{"line_number":241,"context_line":"        while time.time() - start \u003c timeout:"},{"line_number":242,"context_line":"            try:"},{"line_number":243,"context_line":"                LOG.debug(\"Checking for LoadBalancers Reconciliation\")"}],"source_content_type":"text/x-python","patch_set":45,"id":"ce7b4c2f_42559609","line":240,"range":{"start_line":240,"start_character":8,"end_line":240,"end_character":65},"updated":"2021-09-06 09:00:34.000000000","message":"Nice!","commit_id":"cb8dbf03cd37fb077abf9c7c9b344b1562fc7895"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2d60020da34f75ef8894a52bd07b236efa1b6e97","unresolved":true,"context_lines":[{"line_number":261,"context_line":"        # if there is a connectivity now, that means the LoadBalancer"},{"line_number":262,"context_line":"        # is reconciled"},{"line_number":263,"context_line":"        LOG.info(\"LoadBalancer successfully reconciled\")"},{"line_number":264,"context_line":"        self.assertNotEqual(new_lb_id, klb_crd_id)"},{"line_number":265,"context_line":"        self.check_service_internal_connectivity(service_name\u003dservice_name)"}],"source_content_type":"text/x-python","patch_set":45,"id":"12c50f7e_c11def43","line":264,"range":{"start_line":264,"start_character":8,"end_line":264,"end_character":50},"updated":"2021-09-06 09:00:34.000000000","message":"It doesn\u0027t seems needed, as line 251 already asserts that.","commit_id":"cb8dbf03cd37fb077abf9c7c9b344b1562fc7895"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"15121ce714cbb2ddfe018962e2f5a4c9e3e94842","unresolved":true,"context_lines":[{"line_number":227,"context_line":"                time.sleep(5)"},{"line_number":228,"context_line":"                self.lbaas.show_loadbalancer(klb_crd_id,"},{"line_number":229,"context_line":"                                             return_object_only\u003dTrue)"},{"line_number":230,"context_line":"                start -\u003d 5"},{"line_number":231,"context_line":"                self.assertNotEqual(0, start,"},{"line_number":232,"context_line":"                                    \"Timed out waiting for loadbalancer %s to\""},{"line_number":233,"context_line":"                                    \" be deleted\" % service_name)"},{"line_number":234,"context_line":"            except Exception as e:"},{"line_number":235,"context_line":"                if e.status \u003d\u003d 404:"},{"line_number":236,"context_line":"                    LOG.info(\"LoadBalancer sucessfully deleted\")"}],"source_content_type":"text/x-python","patch_set":46,"id":"c09455a0_ca1c576a","line":233,"range":{"start_line":230,"start_character":0,"end_line":233,"end_character":65},"updated":"2021-09-07 06:17:58.000000000","message":"this is a weird mix between using iterations and time to break the while loop. Please choose one or the other (I\u0027m more into the time one) to implement when the loop need to be stopped (e.g., when actual time \u003e start_time + LB_TIMEOUT","commit_id":"53add08b3e8634aafc32a9da89e76260ff283bf5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"15121ce714cbb2ddfe018962e2f5a4c9e3e94842","unresolved":true,"context_lines":[{"line_number":218,"context_line":"        # NOTE(digitalsimboja: We need to await for DELETE to"},{"line_number":219,"context_line":"        # complete on Octavia)"},{"line_number":220,"context_line":"        start \u003d time.time()"},{"line_number":221,"context_line":"        while True:"},{"line_number":222,"context_line":"            try:"},{"line_number":223,"context_line":"                delete_status \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":224,"context_line":"                                                               cascade\u003dTrue)"},{"line_number":225,"context_line":"                if delete_status \u003d\u003d 204:"},{"line_number":226,"context_line":"                    break"},{"line_number":227,"context_line":"                time.sleep(5)"},{"line_number":228,"context_line":"                self.lbaas.show_loadbalancer(klb_crd_id,"},{"line_number":229,"context_line":"                                             return_object_only\u003dTrue)"},{"line_number":230,"context_line":"                start -\u003d 5"},{"line_number":231,"context_line":"                self.assertNotEqual(0, start,"},{"line_number":232,"context_line":"                                    \"Timed out waiting for loadbalancer %s to\""},{"line_number":233,"context_line":"                                    \" be deleted\" % service_name)"},{"line_number":234,"context_line":"            except Exception as e:"},{"line_number":235,"context_line":"                if e.status \u003d\u003d 404:"},{"line_number":236,"context_line":"                    LOG.info(\"LoadBalancer sucessfully deleted\")"},{"line_number":237,"context_line":"                    break"},{"line_number":238,"context_line":"        LOG.info(\"Wait for reconciliation after deletion of loadbalancer\")"},{"line_number":239,"context_line":"        start \u003d time.time()"},{"line_number":240,"context_line":"        timeout \u003d consts.LB_RECONCILE_TIMEOUT + consts.LB_TIMEOUT"}],"source_content_type":"text/x-python","patch_set":46,"id":"7e95cdd4_46d1ddc3","line":237,"range":{"start_line":221,"start_character":0,"end_line":237,"end_character":25},"updated":"2021-09-07 06:17:58.000000000","message":"as we said before, the delete_loadbalancer should not be inside the loop, that should be triggered only once","commit_id":"53add08b3e8634aafc32a9da89e76260ff283bf5"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"9777bdb29c4f74c3a0c5be05eedc2daab85e6eec","unresolved":true,"context_lines":[{"line_number":218,"context_line":"        # NOTE(digitalsimboja: We need to await for DELETE to"},{"line_number":219,"context_line":"        # complete on Octavia)"},{"line_number":220,"context_line":"        start \u003d time.time()"},{"line_number":221,"context_line":"        while True:"},{"line_number":222,"context_line":"            try:"},{"line_number":223,"context_line":"                delete_status \u003d self.lbaas.delete_loadbalancer(klb_crd_id,"},{"line_number":224,"context_line":"                                                               cascade\u003dTrue)"},{"line_number":225,"context_line":"                if delete_status \u003d\u003d 204:"},{"line_number":226,"context_line":"                    break"},{"line_number":227,"context_line":"                time.sleep(5)"},{"line_number":228,"context_line":"                self.lbaas.show_loadbalancer(klb_crd_id,"},{"line_number":229,"context_line":"                                             return_object_only\u003dTrue)"},{"line_number":230,"context_line":"                start -\u003d 5"},{"line_number":231,"context_line":"                self.assertNotEqual(0, start,"},{"line_number":232,"context_line":"                                    \"Timed out waiting for loadbalancer %s to\""},{"line_number":233,"context_line":"                                    \" be deleted\" % service_name)"},{"line_number":234,"context_line":"            except Exception as e:"},{"line_number":235,"context_line":"                if e.status \u003d\u003d 404:"},{"line_number":236,"context_line":"                    LOG.info(\"LoadBalancer sucessfully deleted\")"},{"line_number":237,"context_line":"                    break"},{"line_number":238,"context_line":"        LOG.info(\"Wait for reconciliation after deletion of loadbalancer\")"},{"line_number":239,"context_line":"        start \u003d time.time()"},{"line_number":240,"context_line":"        timeout \u003d consts.LB_RECONCILE_TIMEOUT + consts.LB_TIMEOUT"}],"source_content_type":"text/x-python","patch_set":46,"id":"543c090b_7ca098cc","line":237,"range":{"start_line":221,"start_character":0,"end_line":237,"end_character":25},"in_reply_to":"7e95cdd4_46d1ddc3","updated":"2021-09-07 06:58:06.000000000","message":"\u003e as we said before, the delete_loadbalancer should not be inside the loop, that should be triggered only once\n\nI think we need to recheck this. It seems keeping the delete_loadbalancer outside the loop causes the code to stop executing on returning status code 204. I have been going back and forth with this and maybe you could take a look further.","commit_id":"53add08b3e8634aafc32a9da89e76260ff283bf5"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"64b1707bc7b8e5d17e3fe44b87b5e322a7b1e6bf","unresolved":true,"context_lines":[{"line_number":219,"context_line":"        self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":220,"context_line":"        start \u003d time.time()"},{"line_number":221,"context_line":"        while time.time() - start \u003c consts.LB_TIMEOUT:"},{"line_number":222,"context_line":"            LOG.info(\"Wait for loadbalancer to be completely gone\")"},{"line_number":223,"context_line":"            try:"},{"line_number":224,"context_line":"                loadbalancer \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":225,"context_line":"                if loadbalancer[\u0027status\u0027] \u003d\u003d 404:"},{"line_number":226,"context_line":"                    break"},{"line_number":227,"context_line":"            except Exception as e:"},{"line_number":228,"context_line":"                if e.status \u003d\u003d 404:"},{"line_number":229,"context_line":"                    LOG.info(\"LoadBalancer sucessfully deleted\")"},{"line_number":230,"context_line":"                    break"},{"line_number":231,"context_line":"        else:"},{"line_number":232,"context_line":"            LOG.info(\"TimedOut waiting for loadbalancer %s to be completely\""},{"line_number":233,"context_line":"                     \" deleted\", klb_crd_id)"}],"source_content_type":"text/x-python","patch_set":50,"id":"f1050ac4_538f4edb","line":230,"range":{"start_line":222,"start_character":0,"end_line":230,"end_character":25},"updated":"2021-09-07 15:22:29.000000000","message":"this needs a time.sleep, not to be executing this too many times","commit_id":"6a4382bfd8098ffa6b725de5a5d2f7f21288f464"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"64b1707bc7b8e5d17e3fe44b87b5e322a7b1e6bf","unresolved":true,"context_lines":[{"line_number":236,"context_line":"        start \u003d time.time()"},{"line_number":237,"context_line":"        timeout \u003d consts.LB_RECONCILE_TIMEOUT + consts.LB_TIMEOUT"},{"line_number":238,"context_line":"        while time.time() - start \u003c timeout:"},{"line_number":239,"context_line":"            try:"},{"line_number":240,"context_line":"                LOG.debug(\"Checking for LoadBalancers Reconciliation\")"},{"line_number":241,"context_line":"                new_lb_id \u003d self.get_kuryr_loadbalancer_crds(service_name,"},{"line_number":242,"context_line":"                                                             namespace).get("},{"line_number":243,"context_line":"                                                             \u0027status\u0027,"},{"line_number":244,"context_line":"                                                             {}).get("},{"line_number":245,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":246,"context_line":"                                                             {}).get(\u0027id\u0027,"},{"line_number":247,"context_line":"                                                                     None)"},{"line_number":248,"context_line":"                if new_lb_id and new_lb_id !\u003d klb_crd_id:"},{"line_number":249,"context_line":"                    LOG.info(\"LoadBalancer reconciliation completed %s\","},{"line_number":250,"context_line":"                             new_lb_id)"},{"line_number":251,"context_line":"                    break"},{"line_number":252,"context_line":"            except kubernetes.client.rest.ApiException:"},{"line_number":253,"context_line":"                continue"},{"line_number":254,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":50,"id":"ef740c56_83066c01","line":251,"range":{"start_line":239,"start_character":0,"end_line":251,"end_character":25},"updated":"2021-09-07 15:22:29.000000000","message":"ditto, given the waiting time is long, I would not check this more frequently than once a minute","commit_id":"6a4382bfd8098ffa6b725de5a5d2f7f21288f464"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"9f4b371d95b8c114a17b7907e97787c441b814dd","unresolved":true,"context_lines":[{"line_number":224,"context_line":"                time.sleep(5)"},{"line_number":225,"context_line":"                LOG.info(\"Waiting for loadbalancer to be completely gone\")"},{"line_number":226,"context_line":"                loadbalancer \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":227,"context_line":"                if loadbalancer[\u0027status\u0027] \u003d\u003d 404:"},{"line_number":228,"context_line":"                    break"},{"line_number":229,"context_line":"            except lib_exc.NotFound:"},{"line_number":230,"context_line":"                LOG.info(\"LoadBalancer sucessfully deleted\")"},{"line_number":231,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":56,"id":"68d3fa51_7869493c","line":228,"range":{"start_line":227,"start_character":0,"end_line":228,"end_character":25},"updated":"2021-09-08 12:54:27.000000000","message":"not needed, just show_loadbalancer (I would increase the time.sleep to 20 or 30 seconds too), we are waiting for the exception, don\u0027t care about the returned value","commit_id":"04cbc87abaca3bf7ea627e6452c40b54829c2f12"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"687849455b50fe31de3fa59a3f11f1d2ea6ed951","unresolved":true,"context_lines":[{"line_number":224,"context_line":"                time.sleep(5)"},{"line_number":225,"context_line":"                LOG.info(\"Waiting for loadbalancer to be completely gone\")"},{"line_number":226,"context_line":"                loadbalancer \u003d self.lbaas.show_loadbalancer(klb_crd_id)"},{"line_number":227,"context_line":"                if loadbalancer[\u0027status\u0027] \u003d\u003d 404:"},{"line_number":228,"context_line":"                    break"},{"line_number":229,"context_line":"            except lib_exc.NotFound:"},{"line_number":230,"context_line":"                LOG.info(\"LoadBalancer sucessfully deleted\")"},{"line_number":231,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":56,"id":"108d8ecd_cdf1e34d","line":228,"range":{"start_line":227,"start_character":0,"end_line":228,"end_character":25},"in_reply_to":"68d3fa51_7869493c","updated":"2021-09-08 13:03:36.000000000","message":"\u003e not needed, just show_loadbalancer (I would increase the time.sleep to 20 or 30 seconds too), we are waiting for the exception, don\u0027t care about the returned value\n\nAlright got it.","commit_id":"04cbc87abaca3bf7ea627e6452c40b54829c2f12"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"28a046baf42cab0a7a1f0479df83c79bea5c7143","unresolved":true,"context_lines":[{"line_number":229,"context_line":"        else:"},{"line_number":230,"context_line":"            LOG.info(\"TimedOut waiting for loadbalancer %s to be completely\""},{"line_number":231,"context_line":"                     \" deleted\", klb_crd_id)"},{"line_number":232,"context_line":"            return"},{"line_number":233,"context_line":"        start \u003d time.time()"},{"line_number":234,"context_line":"        timeout \u003d consts.LB_RECONCILE_TIMEOUT + consts.LB_TIMEOUT"},{"line_number":235,"context_line":"        while time.time() - start \u003c timeout:"}],"source_content_type":"text/x-python","patch_set":60,"id":"7f667d6e_32f24d64","line":232,"range":{"start_line":232,"start_character":12,"end_line":232,"end_character":18},"updated":"2021-09-09 11:56:59.000000000","message":"instead of just returning could you raise a TimeoutException here, it will give better indication of where the issue is.","commit_id":"f1db7198f792cff9bf05a1b60e271c83bc24bdea"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"4b2354fbb3987dea8db7d2623003402cd5b40882","unresolved":true,"context_lines":[{"line_number":211,"context_line":"                                                         \u0027status\u0027,"},{"line_number":212,"context_line":"                                                         {}).get("},{"line_number":213,"context_line":"                                                         \u0027loadbalancer\u0027,"},{"line_number":214,"context_line":"                                                         {}).get(\u0027id\u0027, None)"},{"line_number":215,"context_line":"        except kubernetes.client.rest.ApiException:"},{"line_number":216,"context_line":"            return"},{"line_number":217,"context_line":"        # NOTE(digitalsimboja: We need to await for DELETE to"}],"source_content_type":"text/x-python","patch_set":61,"id":"b33ab250_538701f1","line":214,"range":{"start_line":214,"start_character":71,"end_line":214,"end_character":75},"updated":"2021-09-09 12:54:04.000000000","message":"ultranit: you don\u0027t need to have None here, since\n\n  {}.get(\u0027foo\u0027)\n\nwill return None, i.e. if dictionary doesn\u0027t contain requested key, it will return None by default.","commit_id":"464ec99c7136a4f91b73bc83a1d8cfe5c5c558c2"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"4b2354fbb3987dea8db7d2623003402cd5b40882","unresolved":true,"context_lines":[{"line_number":242,"context_line":"                                                             {}).get("},{"line_number":243,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":244,"context_line":"                                                             {}).get(\u0027id\u0027,"},{"line_number":245,"context_line":"                                                                     None)"},{"line_number":246,"context_line":"                if new_lb_id and new_lb_id !\u003d klb_crd_id:"},{"line_number":247,"context_line":"                    LOG.debug(\"LoadBalancer reconciliation completed %s\","},{"line_number":248,"context_line":"                              new_lb_id)"}],"source_content_type":"text/x-python","patch_set":61,"id":"5f64f49c_d32a4359","line":245,"range":{"start_line":245,"start_character":69,"end_line":245,"end_character":73},"updated":"2021-09-09 12:54:04.000000000","message":"Ditto.","commit_id":"464ec99c7136a4f91b73bc83a1d8cfe5c5c558c2"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"83b5e2ddafb21e0892bf8d0383c63740d50283e4","unresolved":true,"context_lines":[{"line_number":243,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":244,"context_line":"                                                             {}).get(\u0027id\u0027,"},{"line_number":245,"context_line":"                                                                     None)"},{"line_number":246,"context_line":"                if new_lb_id and new_lb_id !\u003d klb_crd_id:"},{"line_number":247,"context_line":"                    LOG.debug(\"LoadBalancer reconciliation completed %s\","},{"line_number":248,"context_line":"                              new_lb_id)"},{"line_number":249,"context_line":"                    break"},{"line_number":250,"context_line":"            except kubernetes.client.rest.ApiException:"},{"line_number":251,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":61,"id":"b859e749_6b3c0890","line":248,"range":{"start_line":246,"start_character":16,"end_line":248,"end_character":40},"updated":"2021-09-09 12:45:51.000000000","message":"I know, that I\u0027m late to the party, although there is a (stupid) convention, that every test should have some sort of assert method in it. And those tests aren\u0027t much different from unit tests, they just do stuff from different perspective.\n\nAnyhow, this check might be a perfect candidate to change it to the assert* method.\n\nWDYT?","commit_id":"464ec99c7136a4f91b73bc83a1d8cfe5c5c558c2"},{"author":{"_account_id":13692,"name":"Roman Dobosz","email":"gryf73@gmail.com","username":"gryf"},"change_message_id":"8046d362d85f723d5be9b12297dd34231530815e","unresolved":true,"context_lines":[{"line_number":243,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":244,"context_line":"                                                             {}).get(\u0027id\u0027,"},{"line_number":245,"context_line":"                                                                     None)"},{"line_number":246,"context_line":"                if new_lb_id and new_lb_id !\u003d klb_crd_id:"},{"line_number":247,"context_line":"                    LOG.debug(\"LoadBalancer reconciliation completed %s\","},{"line_number":248,"context_line":"                              new_lb_id)"},{"line_number":249,"context_line":"                    break"},{"line_number":250,"context_line":"            except kubernetes.client.rest.ApiException:"},{"line_number":251,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":61,"id":"ebe676cf_b2fec1ce","line":248,"range":{"start_line":246,"start_character":16,"end_line":248,"end_character":40},"in_reply_to":"2e7aeeb0_e14f2d5b","updated":"2021-09-09 17:10:08.000000000","message":"I\u0027m not sure if I understand your intentions. Although. Given the fact, that if there is not loadbalancer ready for specified service and namespace, get_kuryr_loadbalancer_crds.get(…).get(…) will return None, otherwise it will return an loadbalancer ID, right? So the flow could be as follows:\n\n  if new_lb_id is None:\n    continue\n  else:\n    self.assertNotEqual(new_lb_id, klb_crd_id)\n    break\n\nSomething like this. Not sure if you can get same ID at the beginning of the loop, so you might get me corrected :)","commit_id":"464ec99c7136a4f91b73bc83a1d8cfe5c5c558c2"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"1b8338495e097dae04de923f8ac5eed8407df0a7","unresolved":true,"context_lines":[{"line_number":243,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":244,"context_line":"                                                             {}).get(\u0027id\u0027,"},{"line_number":245,"context_line":"                                                                     None)"},{"line_number":246,"context_line":"                if new_lb_id and new_lb_id !\u003d klb_crd_id:"},{"line_number":247,"context_line":"                    LOG.debug(\"LoadBalancer reconciliation completed %s\","},{"line_number":248,"context_line":"                              new_lb_id)"},{"line_number":249,"context_line":"                    break"},{"line_number":250,"context_line":"            except kubernetes.client.rest.ApiException:"},{"line_number":251,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":61,"id":"2e7aeeb0_e14f2d5b","line":248,"range":{"start_line":246,"start_character":16,"end_line":248,"end_character":40},"in_reply_to":"b859e749_6b3c0890","updated":"2021-09-09 13:31:27.000000000","message":"\u003e I know, that I\u0027m late to the party, although there is a (stupid) convention, that every test should have some sort of assert method in it. And those tests aren\u0027t much different from unit tests, they just do stuff from different perspective.\n\u003e \n\u003e Anyhow, this check might be a perfect candidate to change it to the assert* method.\n\u003e \n\u003e WDYT?\n\nI am thinking of another way of breaking out of the loop without the if control. If self.assertNotEqual(new_lb_id, klb_crd_id):\nthen break\nHow does this sound?","commit_id":"464ec99c7136a4f91b73bc83a1d8cfe5c5c558c2"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"839ff31e2310fcbf9129df2d2b83cb6769c4ae66","unresolved":true,"context_lines":[{"line_number":243,"context_line":"                                                             \u0027loadbalancer\u0027,"},{"line_number":244,"context_line":"                                                             {}).get(\u0027id\u0027,"},{"line_number":245,"context_line":"                                                                     None)"},{"line_number":246,"context_line":"                if new_lb_id and new_lb_id !\u003d klb_crd_id:"},{"line_number":247,"context_line":"                    LOG.debug(\"LoadBalancer reconciliation completed %s\","},{"line_number":248,"context_line":"                              new_lb_id)"},{"line_number":249,"context_line":"                    break"},{"line_number":250,"context_line":"            except kubernetes.client.rest.ApiException:"},{"line_number":251,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":61,"id":"dd46fb6a_292daf79","line":248,"range":{"start_line":246,"start_character":16,"end_line":248,"end_character":40},"in_reply_to":"ebe676cf_b2fec1ce","updated":"2021-09-09 17:25:29.000000000","message":"\u003e I\u0027m not sure if I understand your intentions. Although. Given the fact, that if there is not loadbalancer ready for specified service and namespace, get_kuryr_loadbalancer_crds.get(…).get(…) will return None, otherwise it will return an loadbalancer ID, right? So the flow could be as follows:\n\u003e \n\u003e   if new_lb_id is None:\n\u003e     continue\n\u003e   else:\n\u003e     self.assertNotEqual(new_lb_id, klb_crd_id)\n\u003e     break\n\u003e \n\u003e Something like this. Not sure if you can get same ID at the beginning of the loop, so you might get me corrected :)\n\nOkay this is cool, just wondering if this is more appropriate:\n if not new_lb_id:\n   continue\n\n?","commit_id":"464ec99c7136a4f91b73bc83a1d8cfe5c5c558c2"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b2f26e65f56c55b04ad637ad185c5c62e70aa447","unresolved":true,"context_lines":[{"line_number":213,"context_line":"                                                         \u0027loadbalancer\u0027,"},{"line_number":214,"context_line":"                                                         {}).get(\u0027id\u0027)"},{"line_number":215,"context_line":"        except kubernetes.client.rest.ApiException:"},{"line_number":216,"context_line":"            return"},{"line_number":217,"context_line":"        # NOTE(digitalsimboja: We need to await for DELETE to"},{"line_number":218,"context_line":"        # complete on Octavia)"},{"line_number":219,"context_line":"        self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":63,"id":"d2576cf3_df3459ee","line":216,"range":{"start_line":216,"start_character":1,"end_line":216,"end_character":18},"updated":"2021-09-10 07:47:42.000000000","message":"this should raise an error on the test, right?","commit_id":"84f5172c6a52d8a0c9ad3e0e5975077e81d88d80"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b2f26e65f56c55b04ad637ad185c5c62e70aa447","unresolved":true,"context_lines":[{"line_number":231,"context_line":"                   \" deleted\", klb_crd_id)"},{"line_number":232,"context_line":"            raise lib_exc.TimeoutException(msg)"},{"line_number":233,"context_line":"        start \u003d time.time()"},{"line_number":234,"context_line":"        timeout \u003d consts.LB_RECONCILE_TIMEOUT + consts.LB_TIMEOUT"},{"line_number":235,"context_line":"        while time.time() - start \u003c timeout:"},{"line_number":236,"context_line":"            try:"},{"line_number":237,"context_line":"                time.sleep(60)"}],"source_content_type":"text/x-python","patch_set":63,"id":"e53d7c5b_1353d102","line":234,"range":{"start_line":234,"start_character":1,"end_line":234,"end_character":65},"updated":"2021-09-10 07:47:42.000000000","message":"one comment here about why to add both constants would help to remember why it was done","commit_id":"84f5172c6a52d8a0c9ad3e0e5975077e81d88d80"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"6302706db34172704d0bfebed9f068dd8d35d618","unresolved":true,"context_lines":[{"line_number":183,"context_line":"class TestLoadBalancerReconciliationScenario(base.BaseKuryrScenarioTest):"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":186,"context_line":"    CONF.kuryr_kubernetes.enable_reconciliation \u003d True"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    @classmethod"},{"line_number":189,"context_line":"    def skip_checks(cls):"}],"source_content_type":"text/x-python","patch_set":64,"id":"6082dc7a_c996b92d","line":186,"range":{"start_line":186,"start_character":0,"end_line":186,"end_character":54},"updated":"2021-09-13 06:57:06.000000000","message":"remove this line!!! Otherwise this is always executed!","commit_id":"6be152922c67920a5e456e9e098913e4cfc5a351"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"423849033a64f4927e54cad07c308a33a0750c5d","unresolved":true,"context_lines":[{"line_number":214,"context_line":"                                                         {}).get(\u0027id\u0027)"},{"line_number":215,"context_line":"        except kubernetes.client.rest.ApiException:"},{"line_number":216,"context_line":"            raise lib_exc.ServerFault"},{"line_number":217,"context_line":"        # NOTE(digitalsimboja: We need to await for DELETE to"},{"line_number":218,"context_line":"        # complete on Octavia)"},{"line_number":219,"context_line":"        self.lbaas.delete_loadbalancer(klb_crd_id, cascade\u003dTrue)"},{"line_number":220,"context_line":"        LOG.debug(\"Waiting for loadbalancer to be completely gone\")"},{"line_number":221,"context_line":"        start \u003d time.time()"}],"source_content_type":"text/x-python","patch_set":65,"id":"a987214d_2daf253b","line":218,"range":{"start_line":217,"start_character":0,"end_line":218,"end_character":30},"updated":"2021-09-13 12:21:32.000000000","message":"super nit, should be something like this instead:\n        # NOTE(digitalsimboja): We need to await for DELETE to\n        # complete on Octavia","commit_id":"6a60f5e30bc2d5aaf80d08add5918c8d118ba5ff"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"643e0b15b4e88a4049413010d4419b5a6d594acd","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.check_updated_listener_timeout("},{"line_number":183,"context_line":"            service_name\u003d\"kuryr-listener-demo\")"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"\u003c\u003c\u003c\u003c\u003c\u003c\u003c HEAD"},{"line_number":187,"context_line":"class TestDeployment(base.BaseKuryrScenarioTest):"},{"line_number":188,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":189,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":68,"id":"e54c6ff4_33585b2a","line":186,"range":{"start_line":185,"start_character":0,"end_line":186,"end_character":12},"updated":"2021-09-16 09:25:21.000000000","message":"Seems the rebase was not successful","commit_id":"6037d758dd3b68b7a6e79ead7ab9168c93ff61e2"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"bcc013e35a259121123b24ec85ed3158bd5c1bd9","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.check_updated_listener_timeout("},{"line_number":183,"context_line":"            service_name\u003d\"kuryr-listener-demo\")"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"\u003c\u003c\u003c\u003c\u003c\u003c\u003c HEAD"},{"line_number":187,"context_line":"class TestDeployment(base.BaseKuryrScenarioTest):"},{"line_number":188,"context_line":"    credentials \u003d [\u0027admin\u0027, \u0027primary\u0027, [\u0027lb_admin\u0027, \u0027load-balancer_admin\u0027]]"},{"line_number":189,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":68,"id":"e292b7af_ac4761e8","line":186,"range":{"start_line":185,"start_character":0,"end_line":186,"end_character":12},"in_reply_to":"e54c6ff4_33585b2a","updated":"2021-09-16 14:02:32.000000000","message":"\u003e Seems the rebase was not successful\n\nOhh!","commit_id":"6037d758dd3b68b7a6e79ead7ab9168c93ff61e2"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d41ba49337580c851673b7e21180101a00a7544c","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"        self.scale_deployment(0, deployment_name)"},{"line_number":244,"context_line":"        self.check_lb_members(pool_id, 0)"},{"line_number":245,"context_line":"        self.check_lb_members(pool_id, 0)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"class TestLoadBalancerReconciliationScenario(base.BaseKuryrScenarioTest):"}],"source_content_type":"text/x-python","patch_set":69,"id":"2f29b8f1_b2e2f8d8","line":245,"range":{"start_line":245,"start_character":0,"end_line":245,"end_character":41},"updated":"2021-09-16 11:41:50.000000000","message":"this is duplicated","commit_id":"721f6c82fb156f14c80ad4e4a8d66684b3debb3a"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"bcc013e35a259121123b24ec85ed3158bd5c1bd9","unresolved":true,"context_lines":[{"line_number":242,"context_line":""},{"line_number":243,"context_line":"        self.scale_deployment(0, deployment_name)"},{"line_number":244,"context_line":"        self.check_lb_members(pool_id, 0)"},{"line_number":245,"context_line":"        self.check_lb_members(pool_id, 0)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"class TestLoadBalancerReconciliationScenario(base.BaseKuryrScenarioTest):"}],"source_content_type":"text/x-python","patch_set":69,"id":"f1216576_f7a04fef","line":245,"range":{"start_line":245,"start_character":0,"end_line":245,"end_character":41},"in_reply_to":"2f29b8f1_b2e2f8d8","updated":"2021-09-16 14:02:32.000000000","message":"\u003e this is duplicated\n\nfixed on patchset 70","commit_id":"721f6c82fb156f14c80ad4e4a8d66684b3debb3a"}],"requirements.txt":[{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"aebeb2bf1acdd32b1cc3b91d9572fd6ed5b23de6","unresolved":true,"context_lines":[{"line_number":13,"context_line":"kubernetes\u003e\u003d5.0.0 # Apache-2.0"},{"line_number":14,"context_line":"oslo.concurrency\u003e\u003d3.26.0 # Apache-2.0"},{"line_number":15,"context_line":"netaddr\u003e\u003d0.7.19 # BSD"},{"line_number":16,"context_line":"octavia-tempest-plugin # Apache-2.0"}],"source_content_type":"text/plain","patch_set":3,"id":"7add87a0_75bf2dbc","line":16,"updated":"2021-08-02 23:48:04.000000000","message":"Ok, I spoke with some of the upstream QA team about what the final decision was on how to do this.\nIt\u0027s not in requirements.txt. They decided to setup a zuul variable to list the required plugins.\nSee this for an example:\nhttps://opendev.org/openstack/heat/src/branch/master/.zuul.yaml#L29\n\nYou can remove this from requirements.txt and add \"- octavia-tempest-plugin\" to your zuul job definition variables. This will fix both the requirements job issues and the \"manager\" issue.","commit_id":"a811c2545af53a347fa86b0eb15510e3f3568001"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"62af141e4e6c4054be35135a757c4caa721db14f","unresolved":true,"context_lines":[{"line_number":13,"context_line":"kubernetes\u003e\u003d5.0.0 # Apache-2.0"},{"line_number":14,"context_line":"oslo.concurrency\u003e\u003d3.26.0 # Apache-2.0"},{"line_number":15,"context_line":"netaddr\u003e\u003d0.7.19 # BSD"},{"line_number":16,"context_line":"octavia-tempest-plugin # Apache-2.0"}],"source_content_type":"text/plain","patch_set":3,"id":"e656e11f_5528aa36","line":16,"in_reply_to":"7add87a0_75bf2dbc","updated":"2021-08-03 03:10:21.000000000","message":"\u003e Ok, I spoke with some of the upstream QA team about what the final decision was on how to do this.\n\u003e It\u0027s not in requirements.txt. They decided to setup a zuul variable to list the required plugins.\n\u003e See this for an example:\n\u003e https://opendev.org/openstack/heat/src/branch/master/.zuul.yaml#L29\n\u003e \n\u003e You can remove this from requirements.txt and add \"- octavia-tempest-plugin\" to your zuul job definition variables. This will fix both the requirements job issues and the \"manager\" issue.\n\nOkay got it! I will amend thus. Thanks","commit_id":"a811c2545af53a347fa86b0eb15510e3f3568001"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"4824cbcb54da5e6ecf64f495398728c3f5795930","unresolved":true,"context_lines":[{"line_number":13,"context_line":"kubernetes\u003e\u003d5.0.0 # Apache-2.0"},{"line_number":14,"context_line":"oslo.concurrency\u003e\u003d3.26.0 # Apache-2.0"},{"line_number":15,"context_line":"netaddr\u003e\u003d0.7.19 # BSD"},{"line_number":16,"context_line":"octavia-tempest-plugin # Apache-2.0"}],"source_content_type":"text/plain","patch_set":3,"id":"37b55645_3ef8f038","line":16,"in_reply_to":"e656e11f_5528aa36","updated":"2021-08-04 06:57:52.000000000","message":"\u003e \u003e Ok, I spoke with some of the upstream QA team about what the final decision was on how to do this.\n\u003e \u003e It\u0027s not in requirements.txt. They decided to setup a zuul variable to list the required plugins.\n\u003e \u003e See this for an example:\n\u003e \u003e https://opendev.org/openstack/heat/src/branch/master/.zuul.yaml#L29\n\u003e \u003e \n\u003e \u003e You can remove this from requirements.txt and add \"- octavia-tempest-plugin\" to your zuul job definition variables. This will fix both the requirements job issues and the \"manager\" issue.\n\u003e \n\u003e Okay got it! I will amend thus. Thanks\n\nPlease take a look further, Zuul is still complaining on the \u0027Manager\u0027 issue https://zuul.opendev.org/t/openstack/build/b117d3eee5e1464f98cd60e8b6b9e429","commit_id":"a811c2545af53a347fa86b0eb15510e3f3568001"}]}