)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":31664,"name":"Omer Schwartz","email":"oschwart@redhat.com","username":"oschwart"},"change_message_id":"d73b904397496d7664ee9fd98f7c9aecee0316f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"be73808b_be17301e","updated":"2022-11-02 17:03:25.000000000","message":"Apart from those, I tested the fix and it worked fine on my devstack environment.","commit_id":"55f1d8ce096e415af204a9296125e423f5e704fa"},{"author":{"_account_id":27442,"name":"Quentin GROLLEAU","email":"quentin.grolleau@corp.ovh.com","username":"QG"},"change_message_id":"a765503eba665c8aabdb46b6e209deaa1088bff9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d6d56067_4d0ee229","updated":"2022-11-14 17:23:12.000000000","message":"I just tested the patch and i think we missed one case,\nI\u0027m still able to reproduce the stack trace when i delete the loadbalancer with --cascade.\nWe are entering in this : https://github.com/openstack/octavia/blob/master/octavia/api/drivers/utils.py#L258\n ","commit_id":"55f1d8ce096e415af204a9296125e423f5e704fa"},{"author":{"_account_id":31664,"name":"Omer Schwartz","email":"oschwart@redhat.com","username":"oschwart"},"change_message_id":"6144a33609a415d0f1df7e486860bda6eafce582","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"55015548_45420c9d","updated":"2022-11-02 17:05:39.000000000","message":"Not sure if a -1 needed, eventually the fix works and was tested.","commit_id":"55f1d8ce096e415af204a9296125e423f5e704fa"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"c56030b99be28d1d0f78f4f285988c68c9d4dc8a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"72c4e30c_517f3134","in_reply_to":"d6d56067_4d0ee229","updated":"2022-11-23 09:59:57.000000000","message":"Thanks, I think it is fixed now, BTW I also fixed a similar issue with TLS-enabled pools.","commit_id":"55f1d8ce096e415af204a9296125e423f5e704fa"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"3de9612b22a1d666a48b73ff80e3c989ae3a89ce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e70d4710_7bb387e7","updated":"2022-11-23 18:44:21.000000000","message":"I have some questions","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"},{"author":{"_account_id":31664,"name":"Omer Schwartz","email":"oschwart@redhat.com","username":"oschwart"},"change_message_id":"00f376f082afd39269913f15c9c94a1b75573001","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f60c9a87_03ad3122","updated":"2023-01-20 11:53:21.000000000","message":"I tested the following scenario with this patch:\nI created an LB + TLS_TERMINATED listener\nI deleted the secret\nI called the failover API and it failed, it set the LB on ERROR provisioning_status but then the LB becomes immutable and it is stuck in this way.\n\nI see that deleting the listener resulted with \nLoad Balancer \u003cLB id\u003e is immutable and cannot be updated. (HTTP 409)\n\nTraceback (most recent call last):\n  File \"/opt/stack/octavia/octavia/controller/worker/v2/controller_worker.py\", line 976, in failover_amphora\n    provider_lb_dict \u003d (provider_utils.\n  File \"/opt/stack/octavia/octavia/api/drivers/utils.py\", line 151, in db_loadbalancer_to_provider_loadbalancer\n    new_loadbalancer_dict \u003d lb_dict_to_provider_dict(\n  File \"/opt/stack/octavia/octavia/api/drivers/utils.py\", line 144, in lb_dict_to_provider_dict\n    new_lb_dict[\u0027listeners\u0027] \u003d db_listeners_to_provider_listeners(\n  File \"/opt/stack/octavia/octavia/api/drivers/utils.py\", line 168, in db_listeners_to_provider_listeners \n    provider_listener \u003d db_listener_to_provider_listener(\n  File \"/opt/stack/octavia/octavia/api/drivers/utils.py\", line 182, in db_listener_to_provider_listener\n    new_listener_dict \u003d listener_dict_to_provider_dict(\n  File \"/opt/stack/octavia/octavia/api/drivers/utils.py\", line 266, in listener_dict_to_provider_dict\n    cert_dict \u003d {}\n  File \"/usr/local/lib/python3.9/site-packages/oslo_utils/excutils.py\", line 227, in __exit__\n    self.force_reraise()\n  File \"/usr/local/lib/python3.9/site-packages/oslo_utils/excutils.py\", line 200, in force_reraise\n    raise self.value\n  File \"/opt/stack/octavia/octavia/api/drivers/utils.py\", line 258, in listener_dict_to_provider_dict\n    cert_dict \u003d cert_parser.load_certificates_data(cert_manager,\n  File \"/opt/stack/octavia/octavia/common/tls_utils/cert_parser.py\", line 365, in load_certificates_data  \n    raise exceptions.CertificateRetrievalException(\noctavia.common.exceptions.CertificateRetrievalException: Could not retrieve certificate: http://192.168.122.153/key-manager/v1/secrets/5a401732-da6f-4ed9-b4c1-fa68619b329b\n\nBut openstack loadbalancer delete --cascade did delete both.\n","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"},{"author":{"_account_id":27442,"name":"Quentin GROLLEAU","email":"quentin.grolleau@corp.ovh.com","username":"QG"},"change_message_id":"8c23c3d64013d9ef8c14391aa63bef021df1f58e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cb2e0a33_18d3685e","updated":"2022-11-23 18:33:48.000000000","message":"LGTM, tested on Yoga and works perfect !","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"},{"author":{"_account_id":31664,"name":"Omer Schwartz","email":"oschwart@redhat.com","username":"oschwart"},"change_message_id":"f64a79be46ea068ee5e106569afaf412745c482d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a292ef09_ffac0196","in_reply_to":"54d73568_b0e64a47","updated":"2023-01-20 12:05:44.000000000","message":"Oh great\nI tested this patch again because I wanted to rebase Michael\u0027s patch on top of it (I wasn\u0027t aware of the fact that secret consumers will solve it too)\n\nI will test the barbican consumer patch directly then.\n\nBTW, in the scenario I ran the LB was on ERROR, yet I received that immutable error (it wasn\u0027t on pending_X provisioning_status but it was immutable).\n\nI will test the same scenario with the secret consumer patch","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"84015b6e6077133246d1905d0b4ce157609c666a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"54d73568_b0e64a47","in_reply_to":"f60c9a87_03ad3122","updated":"2023-01-20 11:56:43.000000000","message":"Thanks,\na failover is not expected to work here, however the LB should not be in an immutable state (PENDING_UPDATE), it should go into ERROR.\n\nFYI Michael has worked on the barbican secret consumer (https://review.opendev.org/c/openstack/octavia/+/864308), with this patch, a user should not be able to delete a secret that is used by a listener, so it means that it would fix this issue.\nMaybe we don\u0027t need my patch anymore.","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"}],"octavia/amphorae/drivers/haproxy/rest_api_driver.py":[{"author":{"_account_id":28722,"name":"Anton Kurbatov","email":"anton.kurbatov@acronis.com","username":"akurbatov"},"change_message_id":"376dcdca8c1e282f143cb5706a811c0c12b34c98","unresolved":true,"context_lines":[{"line_number":351,"context_line":"        # Check if there\u0027s any certs that we need to delete"},{"line_number":352,"context_line":"        try:"},{"line_number":353,"context_line":"            certs \u003d self._process_tls_certificates(listener)"},{"line_number":354,"context_line":"        except exceptions.CertificateRetrievalException:"},{"line_number":355,"context_line":"            # Certificates might have been already removed from the store"},{"line_number":356,"context_line":"            certs \u003d None"},{"line_number":357,"context_line":"            LOG.error(\"Cannot retrieve certificate, skipping \""}],"source_content_type":"text/x-python","patch_set":1,"id":"a717f0df_ad7061a3","line":354,"updated":"2022-09-27 09:07:10.000000000","message":"a CertificateRetrievalException is returned even if, say, the barbican service is unavailable (503). It seems to me that ideally we need to introduce a separate type of error for 404 codes from the barbican.\nHowever, this patch fixes my problem. Thanks","commit_id":"55f1d8ce096e415af204a9296125e423f5e704fa"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"3de9612b22a1d666a48b73ff80e3c989ae3a89ce","unresolved":true,"context_lines":[{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        # Check if there\u0027s any certs that we need to delete"},{"line_number":352,"context_line":"        try:"},{"line_number":353,"context_line":"            certs \u003d self._process_tls_certificates(listener)"},{"line_number":354,"context_line":"        except exceptions.CertificateRetrievalException:"},{"line_number":355,"context_line":"            # Certificates might have been already removed from the store"},{"line_number":356,"context_line":"            certs \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"517c98ad_b6fe3230","line":353,"updated":"2022-11-23 18:44:21.000000000","message":"Why are we even calling this and round tripping to Barbican. Don\u0027t we already have the required IDs in the database?","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"5467c0c73f05ac9607eaeaff67c74118ebf139c3","unresolved":true,"context_lines":[{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        # Check if there\u0027s any certs that we need to delete"},{"line_number":352,"context_line":"        try:"},{"line_number":353,"context_line":"            certs \u003d self._process_tls_certificates(listener)"},{"line_number":354,"context_line":"        except exceptions.CertificateRetrievalException:"},{"line_number":355,"context_line":"            # Certificates might have been already removed from the store"},{"line_number":356,"context_line":"            certs \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"3c9aa9ab_15a4707c","line":353,"in_reply_to":"517c98ad_b6fe3230","updated":"2023-02-15 08:21:49.000000000","message":"Hmm I didn\u0027t change this part, I only added the try/except block.\nMaybe we can revisit it in the future","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"3de9612b22a1d666a48b73ff80e3c989ae3a89ce","unresolved":true,"context_lines":[{"line_number":367,"context_line":"            # Delete them (they\u0027ll be recreated before the reload if they are"},{"line_number":368,"context_line":"            # needed for other listeners anyway)"},{"line_number":369,"context_line":"            for cert_id in certs_to_delete:"},{"line_number":370,"context_line":"                self.clients[amphora.api_version].delete_cert_pem("},{"line_number":371,"context_line":"                    amphora, listener.load_balancer.id,"},{"line_number":372,"context_line":"                    \u0027{id}.pem\u0027.format(id\u003dcert_id))"},{"line_number":373,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"a1a3224d_2e47fa99","line":370,"updated":"2022-11-23 18:44:21.000000000","message":"This all seems a bit wasteful, can\u0027t we just add an amphora API call that deletes them all? Why not send over a list to delete?","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"},{"author":{"_account_id":29244,"name":"Gregory Thiemonge","email":"gthiemon@redhat.com","username":"gthiemonge"},"change_message_id":"5467c0c73f05ac9607eaeaff67c74118ebf139c3","unresolved":true,"context_lines":[{"line_number":367,"context_line":"            # Delete them (they\u0027ll be recreated before the reload if they are"},{"line_number":368,"context_line":"            # needed for other listeners anyway)"},{"line_number":369,"context_line":"            for cert_id in certs_to_delete:"},{"line_number":370,"context_line":"                self.clients[amphora.api_version].delete_cert_pem("},{"line_number":371,"context_line":"                    amphora, listener.load_balancer.id,"},{"line_number":372,"context_line":"                    \u0027{id}.pem\u0027.format(id\u003dcert_id))"},{"line_number":373,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d7477fbe_fa1b3018","line":370,"in_reply_to":"a1a3224d_2e47fa99","updated":"2023-02-15 08:21:49.000000000","message":"I didn\u0027t change this part, I only added the try/except block and a if block.\nMaybe we can revisit it in the future","commit_id":"45703b05c8f688070f71e77d5df06488f437438c"}],"octavia/tests/unit/amphorae/drivers/haproxy/test_rest_api_driver_1_0.py":[{"author":{"_account_id":31664,"name":"Omer Schwartz","email":"oschwart@redhat.com","username":"oschwart"},"change_message_id":"d73b904397496d7664ee9fd98f7c9aecee0316f6","unresolved":true,"context_lines":[{"line_number":600,"context_line":"        mock_load_crt.side_effect \u003d ("},{"line_number":601,"context_line":"            exceptions.CertificateRetrievalException(ref\u003d\"error\"),"},{"line_number":602,"context_line":"            {\u0027tls_cert\u0027: self.sl.default_tls_container, \u0027sni_certs\u0027: sconts},"},{"line_number":603,"context_line":"            {\u0027tls_cert\u0027: None, \u0027sni_certs\u0027: []})"},{"line_number":604,"context_line":"        self.driver.jinja_combo.build_config.side_effect \u003d [\u0027fake_config\u0027]"},{"line_number":605,"context_line":"        # Execute driver method"},{"line_number":606,"context_line":"        self.driver.delete(sl)"}],"source_content_type":"text/x-python","patch_set":1,"id":"8552369b_518ab186","line":603,"range":{"start_line":603,"start_character":12,"end_line":603,"end_character":47},"updated":"2022-11-02 17:03:25.000000000","message":"This is not being used (on my environment, mock_load_crt was called 2 times)","commit_id":"55f1d8ce096e415af204a9296125e423f5e704fa"},{"author":{"_account_id":31664,"name":"Omer Schwartz","email":"oschwart@redhat.com","username":"oschwart"},"change_message_id":"d73b904397496d7664ee9fd98f7c9aecee0316f6","unresolved":true,"context_lines":[{"line_number":605,"context_line":"        # Execute driver method"},{"line_number":606,"context_line":"        self.driver.delete(sl)"},{"line_number":607,"context_line":""},{"line_number":608,"context_line":"        # Certificates is not reachable, we cannot delete it from the amphora"},{"line_number":609,"context_line":"        self.driver.clients[API_VERSION].delete_cert_pem.assert_not_called()"},{"line_number":610,"context_line":""},{"line_number":611,"context_line":"        # Now just make sure we did an update and not a delete"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf54498d_e1bf57b4","line":608,"range":{"start_line":608,"start_character":10,"end_line":608,"end_character":25},"updated":"2022-11-02 17:03:25.000000000","message":"typo Certificates is","commit_id":"55f1d8ce096e415af204a9296125e423f5e704fa"}],"releasenotes/notes/fix-deletion-of-listeners-with-deleted-certificates-da6e7a9f718206b3.yaml":[{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"01503fc686155181e7b3313d0231cc6aa3f4508c","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Fix a bug that prevented from deleting an HTTPS_TERMINATED listener with"},{"line_number":5,"context_line":"    already deleted certificates."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"3cb6432b_966ab235","line":4,"range":{"start_line":4,"start_character":4,"end_line":4,"end_character":7},"updated":"2022-09-29 09:01:40.000000000","message":"Nit: Considering the fact that release notes list what changes have been made I think release notes should use past tense normally (unlike commit messages which are use present tense usually)","commit_id":"55f1d8ce096e415af204a9296125e423f5e704fa"}]}