)]}'
{"neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8ab5011365fd0f98b19f791662c2df7bc4eab187","unresolved":false,"context_lines":[{"line_number":1334,"context_line":"                    bridge_mappings[phys_net] \u003d bridge"},{"line_number":1335,"context_line":"        if bridge_mappings:"},{"line_number":1336,"context_line":"            sync \u003d True"},{"line_number":1337,"context_line":"            self.setup_physical_bridges(bridge_mappings)"},{"line_number":1338,"context_line":"        return sync"},{"line_number":1339,"context_line":""},{"line_number":1340,"context_line":"    def _check_bridge_datapath_id(self, bridge, datapath_ids_set):"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_177606da","line":1337,"range":{"start_line":1337,"start_character":17,"end_line":1337,"end_character":39},"updated":"2019-07-25 10:58:54.000000000","message":"I would also modify this method (see comment below). If, for any reason, in the \"bridge_mappings\" loop inside this function, we process correctly the first ones and then we stop, we\u0027ll have some bridges configured and other not.","commit_id":"b63809715a0b9b8ea0f354dfd6f1b3fd7a713352"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8ab5011365fd0f98b19f791662c2df7bc4eab187","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper()"},{"line_number":1372,"context_line":"        ovs \u003d ovs_lib.BaseOVS()"},{"line_number":1373,"context_line":"        ovs_bridges \u003d ovs.get_bridges()"},{"line_number":1374,"context_line":"        for physical_network, bridge in bridge_mappings.items():"},{"line_number":1375,"context_line":"            LOG.info(\"Mapping physical network %(physical_network)s to \""},{"line_number":1376,"context_line":"                     \"bridge %(bridge)s\","},{"line_number":1377,"context_line":"                     {\u0027physical_network\u0027: physical_network,"},{"line_number":1378,"context_line":"                      \u0027bridge\u0027: bridge})"},{"line_number":1379,"context_line":"            # setup physical bridge"},{"line_number":1380,"context_line":"            if bridge not in ovs_bridges:"},{"line_number":1381,"context_line":"                LOG.error(\"Bridge %(bridge)s for physical network \""},{"line_number":1382,"context_line":"                          \"%(physical_network)s does not exist. Agent \""},{"line_number":1383,"context_line":"                          \"terminated!\","},{"line_number":1384,"context_line":"                          {\u0027physical_network\u0027: physical_network,"},{"line_number":1385,"context_line":"                           \u0027bridge\u0027: bridge})"},{"line_number":1386,"context_line":"                sys.exit(1)"},{"line_number":1387,"context_line":"            br \u003d self.br_phys_cls(bridge)"},{"line_number":1388,"context_line":"            self._check_bridge_datapath_id(br, datapath_ids_set)"},{"line_number":1389,"context_line":""},{"line_number":1390,"context_line":"            # The bridge already exists, so create won\u0027t recreate it, but will"},{"line_number":1391,"context_line":"            # handle things like changing the datapath_type"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_d7586e57","line":1388,"range":{"start_line":1374,"start_character":0,"end_line":1388,"end_character":64},"updated":"2019-07-25 10:58:54.000000000","message":"We should first create a loop to check all phy bridges datapath. Once this loop is done (here is where the error described in the bug can happen), that means all bridges have a unique datapath ID. Then, we can start processing them using this ID.\n\n\nfor physical_network, bridge in bridge_mappings.items():\n  ...\n  self._check_bridge_datapath_id(br, datapath_ids_set)\n\n# Again, the same loop but without the previous check. Now we\n# know all bridges have a unique ID and can be accessed.\nfor physical_network, bridge in bridge_mappings.items():\n  ...\n  br.create()\n  br.set_secure_mode()\n  ...","commit_id":"b63809715a0b9b8ea0f354dfd6f1b3fd7a713352"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"9a34b84c18da78301c9d5559680f187edde322aa","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper()"},{"line_number":1372,"context_line":"        ovs \u003d ovs_lib.BaseOVS()"},{"line_number":1373,"context_line":"        ovs_bridges \u003d ovs.get_bridges()"},{"line_number":1374,"context_line":"        for physical_network, bridge in bridge_mappings.items():"},{"line_number":1375,"context_line":"            LOG.info(\"Mapping physical network %(physical_network)s to \""},{"line_number":1376,"context_line":"                     \"bridge %(bridge)s\","},{"line_number":1377,"context_line":"                     {\u0027physical_network\u0027: physical_network,"},{"line_number":1378,"context_line":"                      \u0027bridge\u0027: bridge})"},{"line_number":1379,"context_line":"            # setup physical bridge"},{"line_number":1380,"context_line":"            if bridge not in ovs_bridges:"},{"line_number":1381,"context_line":"                LOG.error(\"Bridge %(bridge)s for physical network \""},{"line_number":1382,"context_line":"                          \"%(physical_network)s does not exist. Agent \""},{"line_number":1383,"context_line":"                          \"terminated!\","},{"line_number":1384,"context_line":"                          {\u0027physical_network\u0027: physical_network,"},{"line_number":1385,"context_line":"                           \u0027bridge\u0027: bridge})"},{"line_number":1386,"context_line":"                sys.exit(1)"},{"line_number":1387,"context_line":"            br \u003d self.br_phys_cls(bridge)"},{"line_number":1388,"context_line":"            self._check_bridge_datapath_id(br, datapath_ids_set)"},{"line_number":1389,"context_line":""},{"line_number":1390,"context_line":"            # The bridge already exists, so create won\u0027t recreate it, but will"},{"line_number":1391,"context_line":"            # handle things like changing the datapath_type"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_630e98e6","line":1388,"range":{"start_line":1374,"start_character":0,"end_line":1388,"end_character":64},"in_reply_to":"7faddb67_28695fbd","updated":"2019-07-25 14:06:54.000000000","message":"Well, as you comment, in the second case (new bridge added), this is going to fail just for this new bridge (could be more but that\u0027s unlikely) and as you said, the operator will have the warn message.","commit_id":"b63809715a0b9b8ea0f354dfd6f1b3fd7a713352"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d133da883334d09daed220f7848b626db628ac15","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"        ip_wrapper \u003d ip_lib.IPWrapper()"},{"line_number":1372,"context_line":"        ovs \u003d ovs_lib.BaseOVS()"},{"line_number":1373,"context_line":"        ovs_bridges \u003d ovs.get_bridges()"},{"line_number":1374,"context_line":"        for physical_network, bridge in bridge_mappings.items():"},{"line_number":1375,"context_line":"            LOG.info(\"Mapping physical network %(physical_network)s to \""},{"line_number":1376,"context_line":"                     \"bridge %(bridge)s\","},{"line_number":1377,"context_line":"                     {\u0027physical_network\u0027: physical_network,"},{"line_number":1378,"context_line":"                      \u0027bridge\u0027: bridge})"},{"line_number":1379,"context_line":"            # setup physical bridge"},{"line_number":1380,"context_line":"            if bridge not in ovs_bridges:"},{"line_number":1381,"context_line":"                LOG.error(\"Bridge %(bridge)s for physical network \""},{"line_number":1382,"context_line":"                          \"%(physical_network)s does not exist. Agent \""},{"line_number":1383,"context_line":"                          \"terminated!\","},{"line_number":1384,"context_line":"                          {\u0027physical_network\u0027: physical_network,"},{"line_number":1385,"context_line":"                           \u0027bridge\u0027: bridge})"},{"line_number":1386,"context_line":"                sys.exit(1)"},{"line_number":1387,"context_line":"            br \u003d self.br_phys_cls(bridge)"},{"line_number":1388,"context_line":"            self._check_bridge_datapath_id(br, datapath_ids_set)"},{"line_number":1389,"context_line":""},{"line_number":1390,"context_line":"            # The bridge already exists, so create won\u0027t recreate it, but will"},{"line_number":1391,"context_line":"            # handle things like changing the datapath_type"}],"source_content_type":"text/x-python","patch_set":2,"id":"7faddb67_28695fbd","line":1388,"range":{"start_line":1374,"start_character":0,"end_line":1388,"end_character":64},"in_reply_to":"7faddb67_d7586e57","updated":"2019-07-25 13:15:35.000000000","message":"this method is used in 2 cases:\n- during start of agent - and then IMO if it will fail and agent will crash due to error, it\u0027s fine as operator should fix problem with bridges config,\n\n- when bridge will be removed and added again - this case this patch tries to address - in such case we know that bridge exists and agent was working fine before so we shouldn\u0027t crash whole agent but rather catch such RuntimeError, warn operator and try to reconfigure it again.\n\nI don\u0027t think that this would be an issue if agent will configure some of bridges and crash on one of them later. I can refactor this code a bit to avoid such situation but maybe it can be done in follow up patch? What do You think?","commit_id":"b63809715a0b9b8ea0f354dfd6f1b3fd7a713352"}]}