)]}'
{"octavia/controller/worker/v1/tasks/amphora_driver_tasks.py":[{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"094d07e02232f0df7c4febb0e5426f16229840d8","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                LOG.warning(\u0027Failed to reload listeners on amphora %s. \u0027"},{"line_number":135,"context_line":"                            \u0027Skipping this amphora as it is failing to \u0027"},{"line_number":136,"context_line":"                            \u0027reload due to: %s\u0027, amphora_id, str(e))"},{"line_number":137,"context_line":"                self.amphora_repo.update(db_apis.get_session(), amphora_id,"},{"line_number":138,"context_line":"                                         status\u003dconstants.ERROR)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_728e277a","line":137,"updated":"2020-09-15 07:04:47.000000000","message":"When an amp in an active-standby topology is in ERROR, the associated LB operating status should be DEGRADED. This is important for visibility/SLA reporting to non-admin users who do not have read permission to amphora objects (hence neither to their statuses).","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"},{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"3c61980f4bc9f760936647bab1ded22e6bd49799","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                LOG.warning(\u0027Failed to reload listeners on amphora %s. \u0027"},{"line_number":135,"context_line":"                            \u0027Skipping this amphora as it is failing to \u0027"},{"line_number":136,"context_line":"                            \u0027reload due to: %s\u0027, amphora_id, str(e))"},{"line_number":137,"context_line":"                self.amphora_repo.update(db_apis.get_session(), amphora_id,"},{"line_number":138,"context_line":"                                         status\u003dconstants.ERROR)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_cb2acc2d","line":137,"in_reply_to":"9f560f44_48d407bc","updated":"2020-09-17 12:11:36.000000000","message":"My suggestion is to set the *operating* status to DEGRADED. That status value exists already today: https://docs.openstack.org/api-ref/load-balancer/v2/#operating-status-codes\n\nThe provisioning status should be left untouched. Its status should already be PENDING_UPDATE when this code runs.","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"cd56f62f6a27d0659cf456150295f02fccfee7a9","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                LOG.warning(\u0027Failed to reload listeners on amphora %s. \u0027"},{"line_number":135,"context_line":"                            \u0027Skipping this amphora as it is failing to \u0027"},{"line_number":136,"context_line":"                            \u0027reload due to: %s\u0027, amphora_id, str(e))"},{"line_number":137,"context_line":"                self.amphora_repo.update(db_apis.get_session(), amphora_id,"},{"line_number":138,"context_line":"                                         status\u003dconstants.ERROR)"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_48d407bc","line":137,"in_reply_to":"9f560f44_728e277a","updated":"2020-09-15 21:13:30.000000000","message":"That would be a big change to our current status information. We currently do not have a \"DEGRADED\" provisioning status: https://docs.openstack.org/api-ref/load-balancer/v2/index.html#provisioning-status-codes\nI\u0027m not saying that isn\u0027t an option/idea, but would be best in it\u0027s own feature patch.","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"},{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"094d07e02232f0df7c4febb0e5426f16229840d8","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            LOG.error(\u0027Failed to start VRRP on amphora %s. \u0027"},{"line_number":404,"context_line":"                      \u0027Skipping this amphora as it is failing to start due \u0027"},{"line_number":405,"context_line":"                      \u0027to: %s\u0027, amphora_id, str(e))"},{"line_number":406,"context_line":"            self.amphora_repo.update(db_apis.get_session(), amphora_id,"},{"line_number":407,"context_line":"                                     status\u003dconstants.ERROR)"},{"line_number":408,"context_line":"            return"},{"line_number":409,"context_line":"        LOG.debug(\"Started VRRP on amphora %s.\", amphorae[amphora_index].id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_d2f453f4","line":406,"updated":"2020-09-15 07:04:47.000000000","message":"ditto","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"}],"octavia/controller/worker/v2/tasks/amphora_driver_tasks.py":[{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"094d07e02232f0df7c4febb0e5426f16229840d8","unresolved":false,"context_lines":[{"line_number":195,"context_line":"                LOG.warning(\u0027Failed to reload listeners on amphora %s. \u0027"},{"line_number":196,"context_line":"                            \u0027Skipping this amphora as it is failing to \u0027"},{"line_number":197,"context_line":"                            \u0027reload due to: %s\u0027, amphora_id, str(e))"},{"line_number":198,"context_line":"                self.amphora_repo.update(db_apis.get_session(), amphora_id,"},{"line_number":199,"context_line":"                                         status\u003dconstants.ERROR)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_12d6cb54","line":198,"updated":"2020-09-15 07:04:47.000000000","message":"ditto","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"},{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"094d07e02232f0df7c4febb0e5426f16229840d8","unresolved":false,"context_lines":[{"line_number":528,"context_line":"            LOG.error(\u0027Failed to start VRRP on amphora %s. \u0027"},{"line_number":529,"context_line":"                      \u0027Skipping this amphora as it is failing to start due \u0027"},{"line_number":530,"context_line":"                      \u0027to: %s\u0027, amphora_id, str(e))"},{"line_number":531,"context_line":"            self.amphora_repo.update(db_apis.get_session(), amphora_id,"},{"line_number":532,"context_line":"                                     status\u003dconstants.ERROR)"},{"line_number":533,"context_line":"            return"},{"line_number":534,"context_line":"        LOG.debug(\"Started VRRP on amphora %s.\","}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_72e3c735","line":531,"updated":"2020-09-15 07:04:47.000000000","message":"ditto","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"}],"octavia/network/drivers/neutron/allowed_address_pairs.py":[{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"094d07e02232f0df7c4febb0e5426f16229840d8","unresolved":false,"context_lines":[{"line_number":715,"context_line":"                try:"},{"line_number":716,"context_line":"                    self._get_amp_net_configs(amp, amp_configs,"},{"line_number":717,"context_line":"                                              vip_subnet, vip_port)"},{"line_number":718,"context_line":"                except Exception as e:"},{"line_number":719,"context_line":"                    LOG.warning(\u0027Getting network configurations for amphora \u0027"},{"line_number":720,"context_line":"                                \u0027%(amp)s failed due to %(err)s.\u0027,"},{"line_number":721,"context_line":"                                {\u0027amp\u0027: amp.id, \u0027err\u0027: str(e)})"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_92bdbbd9","line":718,"updated":"2020-09-15 07:04:47.000000000","message":"amp_configs will be returned empty and post_vip_plugin will raise an AttributeError: \u0027NoneType\u0027 object has no attribute \u0027vip_subnet/vrrp_port\u0027.","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"b6630c2e7448b6c4f935916dd954e19526524b49","unresolved":false,"context_lines":[{"line_number":715,"context_line":"                try:"},{"line_number":716,"context_line":"                    self._get_amp_net_configs(amp, amp_configs,"},{"line_number":717,"context_line":"                                              vip_subnet, vip_port)"},{"line_number":718,"context_line":"                except Exception as e:"},{"line_number":719,"context_line":"                    LOG.warning(\u0027Getting network configurations for amphora \u0027"},{"line_number":720,"context_line":"                                \u0027%(amp)s failed due to %(err)s.\u0027,"},{"line_number":721,"context_line":"                                {\u0027amp\u0027: amp.id, \u0027err\u0027: str(e)})"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_2631d60b","line":718,"in_reply_to":"9f560f44_0ba8448a","updated":"2020-09-17 20:54:33.000000000","message":"The point of this change is to not revert the flow if one of the ports is missing from neutron. We want it to passively fail (marking the amp as ERROR) such that if this is a two amphora load balancer one can be repaired even if the other is missing in neutron.\nYou will note that the other two tasks that call this method all pass in an amphora, so they will not go down this code path and will still raise the exception in the above if block. Only the GetAmphoraeNetworkConfigs task, which is used in the VRRP subflow only, will not pass an amphora in.\n\nGranted, this is a subtlety in the code. I guess another option is to split get_network_configs into two methods, one that iterates over the amps and one that doesn\u0027t. Do you have a thought on how to make this more clear?","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"},{"author":{"_account_id":6469,"name":"Carlos Gonçalves","display_name":"Carlos Goncalves","email":"cgoncalves@redhat.com","username":"cgoncalves"},"change_message_id":"3c61980f4bc9f760936647bab1ded22e6bd49799","unresolved":false,"context_lines":[{"line_number":715,"context_line":"                try:"},{"line_number":716,"context_line":"                    self._get_amp_net_configs(amp, amp_configs,"},{"line_number":717,"context_line":"                                              vip_subnet, vip_port)"},{"line_number":718,"context_line":"                except Exception as e:"},{"line_number":719,"context_line":"                    LOG.warning(\u0027Getting network configurations for amphora \u0027"},{"line_number":720,"context_line":"                                \u0027%(amp)s failed due to %(err)s.\u0027,"},{"line_number":721,"context_line":"                                {\u0027amp\u0027: amp.id, \u0027err\u0027: str(e)})"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_0ba8448a","line":718,"in_reply_to":"9f560f44_2838b3ec","updated":"2020-09-17 12:11:36.000000000","message":"I did not empirically produce this.\n\n1. AmphoraIndexVRRPUpdate in VRRP subflow requires AMPHORAE_NETWORK_CONFIG provided by GetAmphoraeNetworkConfigs just before in the flow (it calls this method).\n\n2. The case of neutron losing a port in-between is unlikely to happen, yes. Though, if get_network_configs raises an exception (e.g. due to momentary connectivity glitch with Neutron API), the flow will only fail later in AmphoraPostVIPPlug.\n\nSo you are relying on amp_configs-consuming tasks such as AmphoraIndexVRRPUpdate and AmphoraPostVIPPlug to revert the flow when the returned amp_configs from this task empty. I think we should trigger a flow revert from this task if the returning amps_configs is an empty dict.","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"cd56f62f6a27d0659cf456150295f02fccfee7a9","unresolved":false,"context_lines":[{"line_number":715,"context_line":"                try:"},{"line_number":716,"context_line":"                    self._get_amp_net_configs(amp, amp_configs,"},{"line_number":717,"context_line":"                                              vip_subnet, vip_port)"},{"line_number":718,"context_line":"                except Exception as e:"},{"line_number":719,"context_line":"                    LOG.warning(\u0027Getting network configurations for amphora \u0027"},{"line_number":720,"context_line":"                                \u0027%(amp)s failed due to %(err)s.\u0027,"},{"line_number":721,"context_line":"                                {\u0027amp\u0027: amp.id, \u0027err\u0027: str(e)})"}],"source_content_type":"text/x-python","patch_set":3,"id":"9f560f44_2838b3ec","line":718,"in_reply_to":"9f560f44_92bdbbd9","updated":"2020-09-15 21:13:30.000000000","message":"How did you produce that? It\u0027s called only in two places\n1 The VRRP subflow, which is after post_vip_plug.\n2. The get_amp_net_subflow where the port was just created in the same flow (and the amp is passed in, so it will be the only result). If neutron loses a port between two points in the same flow, I think revert is probably the only option right?","commit_id":"66f3a63f442bb790dea5c529aa56a462d9515363"}]}