)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"df2138eff7458b268b4ddd3c38a9ce27e70557cf","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Make port binding attempt after agent is revived"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When agent sends heartbeat and is getting back to live,"},{"line_number":10,"context_line":"ML2 plugin will try to bind all ports with \"binding_failed\""},{"line_number":11,"context_line":"from this host."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I3bedb7c22312884cc28aa78aa0f8fbe418f97090"},{"line_number":14,"context_line":"Closes-Bug: #1794809"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_4927d51f","line":11,"range":{"start_line":9,"start_character":0,"end_line":11,"end_character":15},"updated":"2018-10-09 23:10:55.000000000","message":"This is a good explanation of what the new code does. I would like to include, though, a few lines explaining why this is necessary: L3 agent in HA scenario might start running before the L2 agent leading to port binding failures","commit_id":"d68549255ba15387129d441cfa87fe853e64d530"}],"neutron/objects/ports.py":[{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"82a466a50a2ac0a55c02d8dcb60aa876940e3bc5","unresolved":false,"context_lines":[{"line_number":529,"context_line":"        query \u003d query.filter("},{"line_number":530,"context_line":"            ml2_models.PortBinding.vif_type \u003d\u003d binding_type,"},{"line_number":531,"context_line":"            ml2_models.PortBinding.host \u003d\u003d host)"},{"line_number":532,"context_line":"        return [cls.get_object(context, id\u003dport_id[0])"},{"line_number":533,"context_line":"                for port_id in query.all()]"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_f1b2624a","line":532,"range":{"start_line":532,"start_character":20,"end_line":532,"end_character":30},"updated":"2018-10-19 19:11:45.000000000","message":"It looks this method executes two DB queries in total. One for querying the PortBinding table and the other for querying the Port table. Perhaps an optimization could be done to use one joined query instead of two? similar as what \u0027get_ports_by_router\u0027 does above.","commit_id":"6e1472fac2440d7e784cba75d70726ca6988de29"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"f99fcef40abcab1f5f0acdd9a174b8b6b4e0a3e2","unresolved":false,"context_lines":[{"line_number":529,"context_line":"        query \u003d query.filter("},{"line_number":530,"context_line":"            ml2_models.PortBinding.vif_type \u003d\u003d binding_type,"},{"line_number":531,"context_line":"            ml2_models.PortBinding.host \u003d\u003d host)"},{"line_number":532,"context_line":"        return [cls.get_object(context, id\u003dport_id[0])"},{"line_number":533,"context_line":"                for port_id in query.all()]"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_cff3e5e7","line":532,"range":{"start_line":532,"start_character":20,"end_line":532,"end_character":30},"in_reply_to":"3f79a3b5_f1b2624a","updated":"2018-10-22 16:44:18.000000000","message":"Done","commit_id":"6e1472fac2440d7e784cba75d70726ca6988de29"}],"neutron/plugins/ml2/plugin.py":[{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"a04a1d08fdd57c0b8c8524e53e00377e18f7590e","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        agent_revived \u003d (event \u003d\u003d events.AFTER_UPDATE and"},{"line_number":349,"context_line":"                         agent_status \u003d\u003d agent_consts.AGENT_REVIVED)"},{"line_number":350,"context_line":"        agent_new \u003d (event \u003d\u003d events.AFTER_CREATE and"},{"line_number":351,"context_line":"                     agent_status \u003d\u003d agent_consts.AGENT_NEW)"},{"line_number":352,"context_line":"        if not agent_revived and not agent_new:"},{"line_number":353,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_3f706009","line":350,"range":{"start_line":350,"start_character":37,"end_line":350,"end_character":49},"updated":"2018-09-28 21:22:38.000000000","message":"I couldn\u0027t find any subscribe for AFTER_CREATE so this check is unnecessary?","commit_id":"ca70f949d24b4b4ee5f1f6627788d9574b3eb562"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6bed2fcbc37bf7c74d5a25d8514e7f58d535a554","unresolved":false,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"        agent_revived \u003d (event \u003d\u003d events.AFTER_UPDATE and"},{"line_number":349,"context_line":"                         agent_status \u003d\u003d agent_consts.AGENT_REVIVED)"},{"line_number":350,"context_line":"        agent_new \u003d (event \u003d\u003d events.AFTER_CREATE and"},{"line_number":351,"context_line":"                     agent_status \u003d\u003d agent_consts.AGENT_NEW)"},{"line_number":352,"context_line":"        if not agent_revived and not agent_new:"},{"line_number":353,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_a5a1a53c","line":350,"range":{"start_line":350,"start_character":37,"end_line":350,"end_character":49},"in_reply_to":"3f79a3b5_3f706009","updated":"2018-10-01 17:51:43.000000000","message":"right, removed","commit_id":"ca70f949d24b4b4ee5f1f6627788d9574b3eb562"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"a04a1d08fdd57c0b8c8524e53e00377e18f7590e","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        agent_new \u003d (event \u003d\u003d events.AFTER_CREATE and"},{"line_number":351,"context_line":"                     agent_status \u003d\u003d agent_consts.AGENT_NEW)"},{"line_number":352,"context_line":"        if not agent_revived and not agent_new:"},{"line_number":353,"context_line":"            return"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        ports \u003d ports_obj.Port.get_ports_by_binding_type_and_host("},{"line_number":356,"context_line":"            context, portbindings.VIF_TYPE_BINDING_FAILED, host)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_7f53386d","line":353,"range":{"start_line":353,"start_character":12,"end_line":353,"end_character":18},"updated":"2018-09-28 21:22:38.000000000","message":"I assume we can return if the agent\u0027s admin_state_up is False?","commit_id":"ca70f949d24b4b4ee5f1f6627788d9574b3eb562"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6bed2fcbc37bf7c74d5a25d8514e7f58d535a554","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        agent_new \u003d (event \u003d\u003d events.AFTER_CREATE and"},{"line_number":351,"context_line":"                     agent_status \u003d\u003d agent_consts.AGENT_NEW)"},{"line_number":352,"context_line":"        if not agent_revived and not agent_new:"},{"line_number":353,"context_line":"            return"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        ports \u003d ports_obj.Port.get_ports_by_binding_type_and_host("},{"line_number":356,"context_line":"            context, portbindings.VIF_TYPE_BINDING_FAILED, host)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_0053a03b","line":353,"range":{"start_line":353,"start_character":12,"end_line":353,"end_character":18},"in_reply_to":"3f79a3b5_7f53386d","updated":"2018-10-01 17:51:43.000000000","message":"Done","commit_id":"ca70f949d24b4b4ee5f1f6627788d9574b3eb562"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"a04a1d08fdd57c0b8c8524e53e00377e18f7590e","unresolved":false,"context_lines":[{"line_number":352,"context_line":"        if not agent_revived and not agent_new:"},{"line_number":353,"context_line":"            return"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        ports \u003d ports_obj.Port.get_ports_by_binding_type_and_host("},{"line_number":356,"context_line":"            context, portbindings.VIF_TYPE_BINDING_FAILED, host)"},{"line_number":357,"context_line":"        for port in ports:"},{"line_number":358,"context_line":"            port_dict \u003d self._make_port_dict(port.db_obj)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_3f286096","line":355,"range":{"start_line":355,"start_character":26,"end_line":355,"end_character":30},"updated":"2018-09-28 21:22:38.000000000","message":"Just wondering. Do we have to re-bind all the ports. I guess simply re-binding all the ports belonging to L3 agent is sufficient to fix the bug?","commit_id":"ca70f949d24b4b4ee5f1f6627788d9574b3eb562"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6bed2fcbc37bf7c74d5a25d8514e7f58d535a554","unresolved":false,"context_lines":[{"line_number":352,"context_line":"        if not agent_revived and not agent_new:"},{"line_number":353,"context_line":"            return"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        ports \u003d ports_obj.Port.get_ports_by_binding_type_and_host("},{"line_number":356,"context_line":"            context, portbindings.VIF_TYPE_BINDING_FAILED, host)"},{"line_number":357,"context_line":"        for port in ports:"},{"line_number":358,"context_line":"            port_dict \u003d self._make_port_dict(port.db_obj)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_c57381e7","line":355,"range":{"start_line":355,"start_character":26,"end_line":355,"end_character":30},"in_reply_to":"3f79a3b5_3f286096","updated":"2018-10-01 17:51:43.000000000","message":"In bug report there was problem with router\u0027s port but why not try to rebind also ports which belongs to vms or others?","commit_id":"ca70f949d24b4b4ee5f1f6627788d9574b3eb562"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ef230244676e1086d875ba4d91b94313a66e1267","unresolved":false,"context_lines":[{"line_number":367,"context_line":"            mech_context \u003d driver_context.PortContext(self, context, port_dict,"},{"line_number":368,"context_line":"                                                      network,"},{"line_number":369,"context_line":"                                                      binding.db_obj, levels)"},{"line_number":370,"context_line":"            self._bind_port_if_needed(mech_context)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    def _clear_port_binding(self, mech_context, binding, port, original_host):"},{"line_number":373,"context_line":"        binding.vif_type \u003d portbindings.VIF_TYPE_UNBOUND"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_01c18de4","line":370,"updated":"2018-09-28 19:04:38.000000000","message":"My only comment is there should probably be a try/except around maybe these last 3 lines?  That way you could continue with the next port if this one failed.","commit_id":"ca70f949d24b4b4ee5f1f6627788d9574b3eb562"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6bed2fcbc37bf7c74d5a25d8514e7f58d535a554","unresolved":false,"context_lines":[{"line_number":367,"context_line":"            mech_context \u003d driver_context.PortContext(self, context, port_dict,"},{"line_number":368,"context_line":"                                                      network,"},{"line_number":369,"context_line":"                                                      binding.db_obj, levels)"},{"line_number":370,"context_line":"            self._bind_port_if_needed(mech_context)"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    def _clear_port_binding(self, mech_context, binding, port, original_host):"},{"line_number":373,"context_line":"        binding.vif_type \u003d portbindings.VIF_TYPE_UNBOUND"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_405d1846","line":370,"in_reply_to":"3f79a3b5_01c18de4","updated":"2018-10-01 17:51:43.000000000","message":"Done","commit_id":"ca70f949d24b4b4ee5f1f6627788d9574b3eb562"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c542463a6308179c0f1166bda276260bad80ec95","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                self._bind_port_if_needed(mech_context)"},{"line_number":384,"context_line":"            except Exception:"},{"line_number":385,"context_line":"                LOG.warning(\u0027Attempt to bind port %(port_id)s after agent \u0027"},{"line_number":386,"context_line":"                            \u0027%(agent_type)s on host %(host)s revived.\u0027,"},{"line_number":387,"context_line":"                            {\u0027port_id\u0027: port.id,"},{"line_number":388,"context_line":"                             \u0027agent_type\u0027: agent_type,"},{"line_number":389,"context_line":"                             \u0027host\u0027: host})"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_979e0d67","line":386,"range":{"start_line":386,"start_character":61,"end_line":386,"end_character":68},"updated":"2018-10-03 19:23:38.000000000","message":"s/revived failed","commit_id":"d1030c5d5ad56fadbcf57c1c1edfda47d84482e5"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c542463a6308179c0f1166bda276260bad80ec95","unresolved":false,"context_lines":[{"line_number":368,"context_line":"            context, portbindings.VIF_TYPE_BINDING_FAILED, host)"},{"line_number":369,"context_line":"        for port in ports:"},{"line_number":370,"context_line":"            port_dict \u003d self._make_port_dict(port.db_obj)"},{"line_number":371,"context_line":"            network \u003d self.get_network(context, port.network_id)"},{"line_number":372,"context_line":"            binding \u003d self._get_binding_for_host(port.bindings, host)"},{"line_number":373,"context_line":"            if not binding:"},{"line_number":374,"context_line":"                LOG.debug(\u0027No bindings found for port %(port_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_cd160a4b","line":371,"updated":"2018-10-03 19:23:38.000000000","message":"This call isn\u0027t necessary if there isn\u0027t a binding, so should move to after L377\n\nNot sure if that affects unit test.","commit_id":"96c69dbbe9970472f42dc9cebdcbaadf43b76869"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"82063e3cb579f1538894ab1cd0a1b141a602f362","unresolved":false,"context_lines":[{"line_number":368,"context_line":"            context, portbindings.VIF_TYPE_BINDING_FAILED, host)"},{"line_number":369,"context_line":"        for port in ports:"},{"line_number":370,"context_line":"            port_dict \u003d self._make_port_dict(port.db_obj)"},{"line_number":371,"context_line":"            network \u003d self.get_network(context, port.network_id)"},{"line_number":372,"context_line":"            binding \u003d self._get_binding_for_host(port.bindings, host)"},{"line_number":373,"context_line":"            if not binding:"},{"line_number":374,"context_line":"                LOG.debug(\u0027No bindings found for port %(port_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_34e6d0e9","line":371,"in_reply_to":"3f79a3b5_cd160a4b","updated":"2018-10-10 12:34:10.000000000","message":"right, I moved it now.\nBut in fact it shouldn\u0027t happen as in L367 we are looking for ports which already have bindings on specific host, so each of those ports should have such binding here.\n\nTBH I\u0027m not sure if this check shouldn\u0027t be removed from this loop :)","commit_id":"96c69dbbe9970472f42dc9cebdcbaadf43b76869"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"df2138eff7458b268b4ddd3c38a9ce27e70557cf","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        self.add_agent_status_check_worker(self.agent_health_check)"},{"line_number":229,"context_line":"        self.add_workers(self.mechanism_manager.get_workers())"},{"line_number":230,"context_line":"        self._verify_service_plugins_requirements()"},{"line_number":231,"context_line":"        registry.subscribe(self._retry_binding_revived_agents,"},{"line_number":232,"context_line":"                           resources.AGENT, events.AFTER_UPDATE)"},{"line_number":233,"context_line":"        LOG.info(\"Modular L2 Plugin initialization complete\")"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def _setup_rpc(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_0931dd59","line":232,"range":{"start_line":231,"start_character":0,"end_line":232,"end_character":64},"updated":"2018-10-09 23:10:55.000000000","message":"Let\u0027s not undo what Kevin did here: https://review.openstack.org/#/c/438489/. Can we use the receives decorator below?","commit_id":"d68549255ba15387129d441cfa87fe853e64d530"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"82063e3cb579f1538894ab1cd0a1b141a602f362","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        self.add_agent_status_check_worker(self.agent_health_check)"},{"line_number":229,"context_line":"        self.add_workers(self.mechanism_manager.get_workers())"},{"line_number":230,"context_line":"        self._verify_service_plugins_requirements()"},{"line_number":231,"context_line":"        registry.subscribe(self._retry_binding_revived_agents,"},{"line_number":232,"context_line":"                           resources.AGENT, events.AFTER_UPDATE)"},{"line_number":233,"context_line":"        LOG.info(\"Modular L2 Plugin initialization complete\")"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def _setup_rpc(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_f833fb5f","line":232,"range":{"start_line":231,"start_character":0,"end_line":232,"end_character":64},"in_reply_to":"3f79a3b5_0931dd59","updated":"2018-10-10 12:34:10.000000000","message":"Done","commit_id":"d68549255ba15387129d441cfa87fe853e64d530"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"df2138eff7458b268b4ddd3c38a9ce27e70557cf","unresolved":false,"context_lines":[{"line_number":349,"context_line":"                                new_mac\u003dport[\u0027mac_address\u0027])"},{"line_number":350,"context_line":"        return mac_change"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    def _retry_binding_revived_agents(self, resource, event, trigger,"},{"line_number":353,"context_line":"                                      **kwargs):"},{"line_number":354,"context_line":"        context \u003d kwargs[\u0027context\u0027]"},{"line_number":355,"context_line":"        host \u003d kwargs[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_2934d948","line":352,"updated":"2018-10-09 23:10:55.000000000","message":"Let\u0027s use the receives decorator with this method: https://review.openstack.org/#/c/438489/","commit_id":"d68549255ba15387129d441cfa87fe853e64d530"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"82063e3cb579f1538894ab1cd0a1b141a602f362","unresolved":false,"context_lines":[{"line_number":349,"context_line":"                                new_mac\u003dport[\u0027mac_address\u0027])"},{"line_number":350,"context_line":"        return mac_change"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"    def _retry_binding_revived_agents(self, resource, event, trigger,"},{"line_number":353,"context_line":"                                      **kwargs):"},{"line_number":354,"context_line":"        context \u003d kwargs[\u0027context\u0027]"},{"line_number":355,"context_line":"        host \u003d kwargs[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_983a0733","line":352,"in_reply_to":"3f79a3b5_2934d948","updated":"2018-10-10 12:34:10.000000000","message":"Done","commit_id":"d68549255ba15387129d441cfa87fe853e64d530"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"396363cff2e82a8c0a15a37a1828206927a77c24","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                mech_context \u003d driver_context.PortContext("},{"line_number":381,"context_line":"                    self, context, port_dict, network, binding.db_obj, levels)"},{"line_number":382,"context_line":"                self._bind_port_if_needed(mech_context)"},{"line_number":383,"context_line":"            except Exception:"},{"line_number":384,"context_line":"                LOG.warning(\u0027Attempt to bind port %(port_id)s after agent \u0027"},{"line_number":385,"context_line":"                            \u0027%(agent_type)s on host %(host)s revived.\u0027,"},{"line_number":386,"context_line":"                            {\u0027port_id\u0027: port.id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_e17251b0","line":383,"range":{"start_line":383,"start_character":19,"end_line":383,"end_character":28},"updated":"2018-10-11 08:44:37.000000000","message":"Log this exception?","commit_id":"d2013b5962874170d55b5b3dad485ea5415c3430"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ba80e63b1dc2e875168cab16161e7ca9725b0292","unresolved":false,"context_lines":[{"line_number":380,"context_line":"                mech_context \u003d driver_context.PortContext("},{"line_number":381,"context_line":"                    self, context, port_dict, network, binding.db_obj, levels)"},{"line_number":382,"context_line":"                self._bind_port_if_needed(mech_context)"},{"line_number":383,"context_line":"            except Exception:"},{"line_number":384,"context_line":"                LOG.warning(\u0027Attempt to bind port %(port_id)s after agent \u0027"},{"line_number":385,"context_line":"                            \u0027%(agent_type)s on host %(host)s revived.\u0027,"},{"line_number":386,"context_line":"                            {\u0027port_id\u0027: port.id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_9a3ae2c1","line":383,"range":{"start_line":383,"start_character":19,"end_line":383,"end_character":28},"in_reply_to":"3f79a3b5_e17251b0","updated":"2018-10-11 11:09:37.000000000","message":"Done","commit_id":"d2013b5962874170d55b5b3dad485ea5415c3430"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"59354ed477385edf19a6d4818c51b726b69df3a4","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        agent \u003d kwargs.get(\u0027agent\u0027, {})"},{"line_number":356,"context_line":"        agent_status \u003d agent.get(\u0027agent_status\u0027)"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        agent_revived \u003d (event \u003d\u003d events.AFTER_UPDATE and"},{"line_number":359,"context_line":"                         agent_status \u003d\u003d agent_consts.AGENT_REVIVED)"},{"line_number":360,"context_line":"        agent_type \u003d agent.get(\u0027agent_type\u0027)"},{"line_number":361,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_74d4991f","line":358,"range":{"start_line":358,"start_character":25,"end_line":358,"end_character":53},"updated":"2018-10-12 15:11:52.000000000","message":"Why do we need to check this?  Won\u0027t this only receive AFTER_UPDATE events?","commit_id":"7b81416e5e3c88f4d9b783d1e30e13a5c7705847"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ed71372fcbf3cb5860ed6345549e90c35c737529","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        agent \u003d kwargs.get(\u0027agent\u0027, {})"},{"line_number":356,"context_line":"        agent_status \u003d agent.get(\u0027agent_status\u0027)"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        agent_revived \u003d (event \u003d\u003d events.AFTER_UPDATE and"},{"line_number":359,"context_line":"                         agent_status \u003d\u003d agent_consts.AGENT_REVIVED)"},{"line_number":360,"context_line":"        agent_type \u003d agent.get(\u0027agent_type\u0027)"},{"line_number":361,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_b4cf9152","line":358,"range":{"start_line":358,"start_character":25,"end_line":358,"end_character":53},"in_reply_to":"3f79a3b5_74d4991f","updated":"2018-10-12 15:19:56.000000000","message":"right. I forgot to remove it. Sorry","commit_id":"7b81416e5e3c88f4d9b783d1e30e13a5c7705847"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0d105a9301901d03df555d90d9f2e245ac6c091b","unresolved":false,"context_lines":[{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        agent_type \u003d agent.get(\u0027agent_type\u0027)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        if (not agent_status !\u003d agent_consts.AGENT_REVIVED or"},{"line_number":361,"context_line":"                not agent.get(\u0027admin_state_up\u0027) or"},{"line_number":362,"context_line":"                agent_type not in self.agent_types_with_port_binding):"},{"line_number":363,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_3aae0b2f","line":360,"range":{"start_line":360,"start_character":12,"end_line":360,"end_character":31},"updated":"2018-10-12 16:04:55.000000000","message":"s/agent_status !\u003d\n\nI think the double negative would reverse the logic right?","commit_id":"0b3d3f7dee58bca3b1982c80259f29fc8da83042"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6643a3a289edd3d89df190babb4c17fb2573d910","unresolved":false,"context_lines":[{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        agent_type \u003d agent.get(\u0027agent_type\u0027)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        if (not agent_status !\u003d agent_consts.AGENT_REVIVED or"},{"line_number":361,"context_line":"                not agent.get(\u0027admin_state_up\u0027) or"},{"line_number":362,"context_line":"                agent_type not in self.agent_types_with_port_binding):"},{"line_number":363,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_7d20fd4b","line":360,"range":{"start_line":360,"start_character":12,"end_line":360,"end_character":31},"in_reply_to":"3f79a3b5_3aae0b2f","updated":"2018-10-12 16:22:28.000000000","message":"I really need this coming weekend :)","commit_id":"0b3d3f7dee58bca3b1982c80259f29fc8da83042"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"052305e9c99571d1768a782ea52841f514190880","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        return self._aliases"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"    @property"},{"line_number":191,"context_line":"    def agent_types_with_port_binding(self):"},{"line_number":192,"context_line":"        if not hasattr(self, \u0027_agent_types_with_port_binding\u0027):"},{"line_number":193,"context_line":"            self._agent_types_with_port_binding \u003d set([])"},{"line_number":194,"context_line":"            for mech_driver in self.mechanism_manager.mech_drivers.values():"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_e6078a6f","line":191,"updated":"2018-10-14 23:32:14.000000000","message":"Please see my comment to L351 below. This method should be adjusted based on that comment","commit_id":"40cbc96c2fb0188717bc2f16085c27594d5118be"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"052305e9c99571d1768a782ea52841f514190880","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        return mac_change"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    @registry.receives(resources.AGENT, [events.AFTER_UPDATE])"},{"line_number":351,"context_line":"    def _retry_binding_revived_agents(self, resource, event, trigger,"},{"line_number":352,"context_line":"                                      **kwargs):"},{"line_number":353,"context_line":"        context \u003d kwargs[\u0027context\u0027]"},{"line_number":354,"context_line":"        host \u003d kwargs[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_66d4daea","line":351,"updated":"2018-10-14 23:32:14.000000000","message":"IIUC from the bug (https://bugs.launchpad.net/neutron/+bug/1794809), we are seeing the need for this fix in the OVS agent. It seems to me that we lost the capability to recover from this kind of failure when we adopted the push notifications cache in OVS: https://review.openstack.org/#/c/410385, which is consistent with the fact that the bug refers to the Queens release. Prior to the adoption of this cache, the OVS agent, upon restarting, would find the devices plugged to br-int and would call get_devices_details_list_and_failed_devices (https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/rpc.py#L171), which in turn would call in L179 get_bound_ports_contexts in the ML2 plugin (https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/plugin.py#L1697). Please note get_bound_ports_contexts attempts to bind the ports discovered by the agent (https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/plugin.py#L1742) which is exactly what this patch does in L381 below.\n\nOther agents (Linuxbridge, Macvtap, sr-iov), though, don\u0027t use the push notifications cache: http://codesearch.openstack.org/?q\u003dCacheBackedPluginApi\u0026i\u003dnope\u0026files\u003d\u0026repos\u003d. When they discover devices in br-int, they still call the Neutron server directly by invoking get_devices_details_list (https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/agent/_common_agent.py#L218 and https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/mech_sriov/agent/sriov_nic_agent.py#L281). get_devices_details_list eventually leads to the invocation of get_bound_port_context which also attempts to bind the port of the device discovered by the agent: https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/plugin.py#L1693. So in the case of non ovs agents, this patch duplicates the re-binding of the ports. \n\nAs a consequence, I think we should limit the scope of this patch to situations where the OVS agent was revived and the ports remain unbound. Even then, we might be duplicating the rebinding of some ports, because from this method we don\u0027t really know which of the configured mechanism drivers will really do the binding. But at least we mitigate the problem. We should also document this as a current limitation of the recovery process","commit_id":"40cbc96c2fb0188717bc2f16085c27594d5118be"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"16fdff487d860991bd27f6f0ab94106555b206ea","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        return mac_change"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    @registry.receives(resources.AGENT, [events.AFTER_UPDATE])"},{"line_number":351,"context_line":"    def _retry_binding_revived_agents(self, resource, event, trigger,"},{"line_number":352,"context_line":"                                      **kwargs):"},{"line_number":353,"context_line":"        context \u003d kwargs[\u0027context\u0027]"},{"line_number":354,"context_line":"        host \u003d kwargs[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_3e701409","line":351,"in_reply_to":"3f79a3b5_026aabb3","updated":"2018-10-15 20:42:02.000000000","message":"My concern is that when we become aware here that the agent has revived, we go ahead and rebind the ports. For non ovs agents, given that they should be able to recover without the mechanism proposed in this patch, if we find unbound ports that may be indication that something else is wrong. That is why I want to exclude them from this mechanism. IN the case of OVS, given https://bugs.launchpad.net/neutron/+bug/1794809 and my previous comment, we have a compelling reason to suspect that the unbound ports are due to the agent reviving, although we are not a 100% certain. So I feel comfortable re-binding all the ports in that case, including compute owned and service ports. I don\u0027t think more granularity is warranted. Good question, though ;-)","commit_id":"40cbc96c2fb0188717bc2f16085c27594d5118be"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5b2086b92fed9838708ffc24265402b0d2c47b8a","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        return mac_change"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    @registry.receives(resources.AGENT, [events.AFTER_UPDATE])"},{"line_number":351,"context_line":"    def _retry_binding_revived_agents(self, resource, event, trigger,"},{"line_number":352,"context_line":"                                      **kwargs):"},{"line_number":353,"context_line":"        context \u003d kwargs[\u0027context\u0027]"},{"line_number":354,"context_line":"        host \u003d kwargs[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_eba40e4e","line":351,"in_reply_to":"3f79a3b5_3e701409","updated":"2018-10-16 13:17:09.000000000","message":"So I tested it today and You were right that for LB agent it is not necessary at all to try to rebind such ports. I reduced scope of this patch only to OVS agent then.\nFor LB agent I also created another bug https://bugs.launchpad.net/neutron/+bug/1798085 because fullstack test for rebind of router\u0027s port when LB agent is used is now failing. It is tests issue in fact.","commit_id":"40cbc96c2fb0188717bc2f16085c27594d5118be"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"c292c2a0a739cdb668bbe2a1cafa9ad25569908f","unresolved":false,"context_lines":[{"line_number":348,"context_line":"        return mac_change"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    @registry.receives(resources.AGENT, [events.AFTER_UPDATE])"},{"line_number":351,"context_line":"    def _retry_binding_revived_agents(self, resource, event, trigger,"},{"line_number":352,"context_line":"                                      **kwargs):"},{"line_number":353,"context_line":"        context \u003d kwargs[\u0027context\u0027]"},{"line_number":354,"context_line":"        host \u003d kwargs[\u0027host\u0027]"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_026aabb3","line":351,"in_reply_to":"3f79a3b5_66d4daea","updated":"2018-10-15 10:03:47.000000000","message":"Basically You are right but I added LB agent scenario to fullstack tests proposed in this patch and when I stopped executing this method, I had:\n\nneutron.tests.fullstack.test_ports_rebind.TestRouterPortRebind.test_vm_port_rebound_when_L2_agent_revived(Linux Bridge Agent) [191.068829s] ... FAILED\nneutron.tests.fullstack.test_ports_rebind.TestVmPortRebind.test_vm_port_rebound_when_L2_agent_revived(Linux Bridge Agent) [117.331773s] ... ok\n\nSo it basically looks that for some reason, port which belongs to router wasn\u0027t bound again after LB agent was revived.\nDo You think that we should do such granularity and check different types of ports for different L2 agents? Or maybe this issue with LB agent and router ports should be reported as separate bug and fixed in different way?","commit_id":"40cbc96c2fb0188717bc2f16085c27594d5118be"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"46eba0fc252328855122555be61ee4553da8b118","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            filter_validation._disable_extension_by_config(aliases)"},{"line_number":187,"context_line":"            self._aliases \u003d aliases"},{"line_number":188,"context_line":"        return self._aliases"},{"line_number":189,"context_line":"    _rebind_on_revive_agent_types \u003d [const.AGENT_TYPE_OVS]"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    def __new__(cls, *args, **kwargs):"},{"line_number":192,"context_line":"        model_query.register_hook("}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_f33391f5","line":189,"updated":"2018-10-17 02:00:41.000000000","message":"This just looks out of place here instead of up after L156, seems like it\u0027s part of the property","commit_id":"77c9a37e2b92f97d31ca61b403c2044e3c75ddb8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"26297fe204495aca4fd4efeac73ea100ae9bb0b1","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            filter_validation._disable_extension_by_config(aliases)"},{"line_number":187,"context_line":"            self._aliases \u003d aliases"},{"line_number":188,"context_line":"        return self._aliases"},{"line_number":189,"context_line":"    _rebind_on_revive_agent_types \u003d [const.AGENT_TYPE_OVS]"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"    def __new__(cls, *args, **kwargs):"},{"line_number":192,"context_line":"        model_query.register_hook("}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_926b17ef","line":189,"in_reply_to":"3f79a3b5_f33391f5","updated":"2018-10-18 08:44:45.000000000","message":"Yes, thx","commit_id":"77c9a37e2b92f97d31ca61b403c2044e3c75ddb8"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"82a466a50a2ac0a55c02d8dcb60aa876940e3bc5","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                levels \u003d db.get_binding_level_objs("},{"line_number":371,"context_line":"                    context, port.id, binding.host)"},{"line_number":372,"context_line":"                mech_context \u003d driver_context.PortContext("},{"line_number":373,"context_line":"                    self, context, port_dict, network, binding.db_obj, levels)"},{"line_number":374,"context_line":"                self._bind_port_if_needed(mech_context)"},{"line_number":375,"context_line":"            except Exception as e:"},{"line_number":376,"context_line":"                LOG.warning(\u0027Attempt to bind port %(port_id)s after agent \u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_b1738a1c","line":373,"range":{"start_line":373,"start_character":63,"end_line":373,"end_character":69},"updated":"2018-10-19 19:11:45.000000000","message":"Just a reminder. This will undo the effort done by: https://review.openstack.org/#/c/544206/25/neutron/plugins/ml2/driver_context.py . Perhaps we should set a dependency between these two patches or add a TODO note about that.","commit_id":"6e1472fac2440d7e784cba75d70726ca6988de29"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"f99fcef40abcab1f5f0acdd9a174b8b6b4e0a3e2","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                levels \u003d db.get_binding_level_objs("},{"line_number":371,"context_line":"                    context, port.id, binding.host)"},{"line_number":372,"context_line":"                mech_context \u003d driver_context.PortContext("},{"line_number":373,"context_line":"                    self, context, port_dict, network, binding.db_obj, levels)"},{"line_number":374,"context_line":"                self._bind_port_if_needed(mech_context)"},{"line_number":375,"context_line":"            except Exception as e:"},{"line_number":376,"context_line":"                LOG.warning(\u0027Attempt to bind port %(port_id)s after agent \u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_0fb1dd01","line":373,"range":{"start_line":373,"start_character":63,"end_line":373,"end_character":69},"in_reply_to":"3f79a3b5_b1738a1c","updated":"2018-10-22 16:44:18.000000000","message":"I think that second patch from those two (whichever will be merged first) will have to rebase and change that simply.\nBut I added simply todo to remember about it here","commit_id":"6e1472fac2440d7e784cba75d70726ca6988de29"}],"neutron/tests/fullstack/test_ports_rebind.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"46eba0fc252328855122555be61ee4553da8b118","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        \"\"\""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        if self.l2_agent_type \u003d\u003d constants.AGENT_TYPE_LINUXBRIDGE:"},{"line_number":163,"context_line":"            self.skip(\"Bug 1798085\")"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        gw_port \u003d self.safe_client.client.list_ports("},{"line_number":166,"context_line":"            device_id\u003dself.router[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_f36151d5","line":163,"range":{"start_line":163,"start_character":17,"end_line":163,"end_character":21},"updated":"2018-10-17 02:00:41.000000000","message":"is this supposed to be skipTest ?  I only see one occurrence of self.skip() which is maybe wrong?","commit_id":"77c9a37e2b92f97d31ca61b403c2044e3c75ddb8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"26297fe204495aca4fd4efeac73ea100ae9bb0b1","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        \"\"\""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"        if self.l2_agent_type \u003d\u003d constants.AGENT_TYPE_LINUXBRIDGE:"},{"line_number":163,"context_line":"            self.skip(\"Bug 1798085\")"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        gw_port \u003d self.safe_client.client.list_ports("},{"line_number":166,"context_line":"            device_id\u003dself.router[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_5256df2d","line":163,"range":{"start_line":163,"start_character":17,"end_line":163,"end_character":21},"in_reply_to":"3f79a3b5_f36151d5","updated":"2018-10-18 08:44:45.000000000","message":"Yes, skip() was deprecated some time ago. I forgot about it. Fixed now. Thx","commit_id":"77c9a37e2b92f97d31ca61b403c2044e3c75ddb8"},{"author":{"_account_id":27654,"name":"Hongbin Lu","email":"kira034@163.com","username":"hongbin.lu"},"change_message_id":"82a466a50a2ac0a55c02d8dcb60aa876940e3bc5","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        common_utils.wait_until_true(port_binding_failed)"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"class TestVmPortRebind(TestPortsRebind):"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    use_l3_agent \u003d False"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_b15cea80","line":78,"range":{"start_line":78,"start_character":10,"end_line":78,"end_character":12},"updated":"2018-10-19 19:11:45.000000000","message":"Super nit: \"VM\" will look better to me.","commit_id":"6e1472fac2440d7e784cba75d70726ca6988de29"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"f99fcef40abcab1f5f0acdd9a174b8b6b4e0a3e2","unresolved":false,"context_lines":[{"line_number":75,"context_line":"        common_utils.wait_until_true(port_binding_failed)"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"class TestVmPortRebind(TestPortsRebind):"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    use_l3_agent \u003d False"},{"line_number":81,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"3f79a3b5_4f859509","line":78,"range":{"start_line":78,"start_character":10,"end_line":78,"end_character":12},"in_reply_to":"3f79a3b5_b15cea80","updated":"2018-10-22 16:44:18.000000000","message":"Done","commit_id":"6e1472fac2440d7e784cba75d70726ca6988de29"}]}
