)]}'
{"neutron/agent/l3/agent.py":[{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"e6f8b07560d9793603dcd2d3811244bdd7f94a6a","unresolved":false,"context_lines":[{"line_number":542,"context_line":"                    self._resync_router(update)"},{"line_number":543,"context_line":"                    continue"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"            if not routers:"},{"line_number":546,"context_line":"                removed \u003d self._safe_router_removed(update.id)"},{"line_number":547,"context_line":"                if not removed:"},{"line_number":548,"context_line":"                    self._resync_router(update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_80525011","line":545,"range":{"start_line":545,"start_character":12,"end_line":545,"end_character":27},"updated":"2018-09-05 18:48:33.000000000","message":"This can also be set to \u0027if len(routers) \u003d\u003d 0:","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"95982218844c163b13306ec584948c048aa0bf7f","unresolved":false,"context_lines":[{"line_number":542,"context_line":"                    self._resync_router(update)"},{"line_number":543,"context_line":"                    continue"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"            if not routers:"},{"line_number":546,"context_line":"                removed \u003d self._safe_router_removed(update.id)"},{"line_number":547,"context_line":"                if not removed:"},{"line_number":548,"context_line":"                    self._resync_router(update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_d00fd9dd","line":545,"range":{"start_line":545,"start_character":19,"end_line":545,"end_character":26},"updated":"2018-09-04 15:34:30.000000000","message":"this won\u0027t get set in all cases, only if \u0027not router\u0027, perhaps just need tweak to L533 ?","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"633197125226f001fa3763b05eddf8dc63ab72b5","unresolved":false,"context_lines":[{"line_number":542,"context_line":"                    self._resync_router(update)"},{"line_number":543,"context_line":"                    continue"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"            if not routers:"},{"line_number":546,"context_line":"                removed \u003d self._safe_router_removed(update.id)"},{"line_number":547,"context_line":"                if not removed:"},{"line_number":548,"context_line":"                    self._resync_router(update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_74949236","line":545,"range":{"start_line":545,"start_character":12,"end_line":545,"end_character":27},"in_reply_to":"3f79a3b5_80525011","updated":"2018-09-05 21:32:57.000000000","message":"Never mind this might not work.","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"61917d666ef751ea4c36f819ad6b4ec037c88211","unresolved":false,"context_lines":[{"line_number":542,"context_line":"                    self._resync_router(update)"},{"line_number":543,"context_line":"                    continue"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"            if not routers:"},{"line_number":546,"context_line":"                removed \u003d self._safe_router_removed(update.id)"},{"line_number":547,"context_line":"                if not removed:"},{"line_number":548,"context_line":"                    self._resync_router(update)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_0fa2e1ae","line":545,"range":{"start_line":545,"start_character":19,"end_line":545,"end_character":26},"in_reply_to":"3f79a3b5_d00fd9dd","updated":"2018-09-06 12:28:25.000000000","message":"Changed above","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"95982218844c163b13306ec584948c048aa0bf7f","unresolved":false,"context_lines":[{"line_number":567,"context_line":"                        self._safe_router_removed(router[\u0027id\u0027])"},{"line_number":568,"context_line":"                except Exception:"},{"line_number":569,"context_line":"                    log_verbose_exc("},{"line_number":570,"context_line":"                        \"Failed to process compatible routers: %s\" % update.id,"},{"line_number":571,"context_line":"                        router)"},{"line_number":572,"context_line":"                    self._resync_router(update)"},{"line_number":573,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_f02d3546","line":570,"range":{"start_line":570,"start_character":54,"end_line":570,"end_character":61},"updated":"2018-09-04 15:34:30.000000000","message":"s/router\n\nsince it was just the one\n\nOr we need to change to mention both the router we\u0027re updating (update.id) and the router we looped on (router[\u0027id\u0027])","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"61917d666ef751ea4c36f819ad6b4ec037c88211","unresolved":false,"context_lines":[{"line_number":567,"context_line":"                        self._safe_router_removed(router[\u0027id\u0027])"},{"line_number":568,"context_line":"                except Exception:"},{"line_number":569,"context_line":"                    log_verbose_exc("},{"line_number":570,"context_line":"                        \"Failed to process compatible routers: %s\" % update.id,"},{"line_number":571,"context_line":"                        router)"},{"line_number":572,"context_line":"                    self._resync_router(update)"},{"line_number":573,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_af796d2f","line":570,"range":{"start_line":570,"start_character":54,"end_line":570,"end_character":61},"in_reply_to":"3f79a3b5_f02d3546","updated":"2018-09-06 12:28:25.000000000","message":"Done","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"99885268de8930a67f21075968441a8925517d76","unresolved":false,"context_lines":[{"line_number":567,"context_line":"                        self._safe_router_removed(router[\u0027id\u0027])"},{"line_number":568,"context_line":"                except Exception:"},{"line_number":569,"context_line":"                    log_verbose_exc("},{"line_number":570,"context_line":"                        \"Failed to process compatible routers: %s\" % update.id,"},{"line_number":571,"context_line":"                        router)"},{"line_number":572,"context_line":"                    self._resync_router(update)"},{"line_number":573,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_dd67d124","line":570,"range":{"start_line":570,"start_character":54,"end_line":570,"end_character":61},"in_reply_to":"3f79a3b5_f02d3546","updated":"2018-09-05 18:44:25.000000000","message":"In this case it makes sense to keep it as \u0027router\u0027 since we are going to report the state of the router in the router-list.\nThe first router failure here is going to cause an Exception and will trigger the resync","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"95982218844c163b13306ec584948c048aa0bf7f","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                        \"Failed to process compatible routers: %s\" % update.id,"},{"line_number":571,"context_line":"                        router)"},{"line_number":572,"context_line":"                    self._resync_router(update)"},{"line_number":573,"context_line":"                    continue"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":576,"context_line":"            rp.fetched_and_processed(update.timestamp)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_90c88155","line":573,"range":{"start_line":573,"start_character":20,"end_line":573,"end_character":28},"updated":"2018-09-04 15:34:30.000000000","message":"this used to continue in the outer loop after adding this update back to the queue, so the logic needs to change to keep that","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"61917d666ef751ea4c36f819ad6b4ec037c88211","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                        \"Failed to process compatible routers: %s\" % update.id,"},{"line_number":571,"context_line":"                        router)"},{"line_number":572,"context_line":"                    self._resync_router(update)"},{"line_number":573,"context_line":"                    continue"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":576,"context_line":"            rp.fetched_and_processed(update.timestamp)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_6fdcb5e0","line":573,"range":{"start_line":573,"start_character":20,"end_line":573,"end_character":28},"in_reply_to":"3f79a3b5_90c88155","updated":"2018-09-06 12:28:25.000000000","message":"Done","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"95982218844c163b13306ec584948c048aa0bf7f","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                    self._resync_router(update)"},{"line_number":573,"context_line":"                    continue"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":576,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def _process_routers_loop(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_108f912a","line":575,"range":{"start_line":575,"start_character":34,"end_line":575,"end_character":41},"updated":"2018-09-04 15:34:30.000000000","message":"s/router","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"99885268de8930a67f21075968441a8925517d76","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                    self._resync_router(update)"},{"line_number":573,"context_line":"                    continue"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":576,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def _process_routers_loop(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_3d24c5c5","line":575,"range":{"start_line":575,"start_character":34,"end_line":575,"end_character":41},"in_reply_to":"3f79a3b5_108f912a","updated":"2018-09-05 18:44:25.000000000","message":"This Log message should get into the For loop if it is an update of a single router.","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"61917d666ef751ea4c36f819ad6b4ec037c88211","unresolved":false,"context_lines":[{"line_number":572,"context_line":"                    self._resync_router(update)"},{"line_number":573,"context_line":"                    continue"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":576,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"    def _process_routers_loop(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3f79a3b5_afaded31","line":575,"range":{"start_line":575,"start_character":34,"end_line":575,"end_character":41},"in_reply_to":"3f79a3b5_3d24c5c5","updated":"2018-09-06 12:28:25.000000000","message":"Done","commit_id":"4d33600ca007048c9ca1989ef0b0d51d03f4b0b6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cab27f84965562b5c0678d4c594feb113e75b88f","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                continue"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"            LOG.debug(\"Finished a routers update for update event %s\","},{"line_number":573,"context_line":"                      update.id)"},{"line_number":574,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":"    def _process_routers_if_compatible(self, update_id, routers):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_1da56203","line":573,"updated":"2018-09-17 20:59:39.000000000","message":"I would put the log message after the call, in case it throws an exception.  It\u0027s also slightly different than L563, \u0027update event\u0027 added, just a nit.","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                continue"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"            LOG.debug(\"Finished a routers update for update event %s\","},{"line_number":573,"context_line":"                      update.id)"},{"line_number":574,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":"    def _process_routers_if_compatible(self, update_id, routers):"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_bc28e597","line":573,"in_reply_to":"3f79a3b5_1da56203","updated":"2018-09-18 20:42:36.000000000","message":"Done","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"94cedae0d693093a7df0bfb10d0d1a3a9ad70a5e","unresolved":false,"context_lines":[{"line_number":564,"context_line":"                continue"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"            try:"},{"line_number":567,"context_line":"                self._process_routers_if_compatible(update.id, routers)"},{"line_number":568,"context_line":"            except Exception:"},{"line_number":569,"context_line":"                self._resync_router(update)"},{"line_number":570,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_123bfd9a","line":567,"range":{"start_line":567,"start_character":63,"end_line":567,"end_character":70},"updated":"2018-09-18 14:03:42.000000000","message":"do you want to process this code if this is empty?","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9bff2a7dfcc694d65713e4f31cc185eb8541fd78","unresolved":false,"context_lines":[{"line_number":564,"context_line":"                continue"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"            try:"},{"line_number":567,"context_line":"                self._process_routers_if_compatible(update.id, routers)"},{"line_number":568,"context_line":"            except Exception:"},{"line_number":569,"context_line":"                self._resync_router(update)"},{"line_number":570,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_1403e50d","line":567,"range":{"start_line":567,"start_character":63,"end_line":567,"end_character":70},"in_reply_to":"3f79a3b5_123bfd9a","updated":"2018-09-18 17:54:26.000000000","message":"If it\u0027s empty we should do a continue on L564","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":564,"context_line":"                continue"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"            try:"},{"line_number":567,"context_line":"                self._process_routers_if_compatible(update.id, routers)"},{"line_number":568,"context_line":"            except Exception:"},{"line_number":569,"context_line":"                self._resync_router(update)"},{"line_number":570,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_5e609e35","line":567,"range":{"start_line":567,"start_character":63,"end_line":567,"end_character":70},"in_reply_to":"3f79a3b5_123bfd9a","updated":"2018-09-18 20:42:36.000000000","message":"No, if that\u0027s empty I don\u0027t want to process it. It\u0027s the same as it was before","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"94cedae0d693093a7df0bfb10d0d1a3a9ad70a5e","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                continue"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":573,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def _process_routers_if_compatible(self, update_id, routers):"},{"line_number":576,"context_line":"        for router in routers:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_92e06d29","line":573,"range":{"start_line":573,"start_character":34,"end_line":573,"end_character":41},"updated":"2018-09-18 14:03:42.000000000","message":"router\n\nin addition to this this may be printed in line 563 - so that could be confusing","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                continue"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":573,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def _process_routers_if_compatible(self, update_id, routers):"},{"line_number":576,"context_line":"        for router in routers:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_be4692a6","line":573,"range":{"start_line":573,"start_character":34,"end_line":573,"end_character":41},"in_reply_to":"3f79a3b5_92e06d29","updated":"2018-09-18 20:42:36.000000000","message":"In L563 it is \"router\" and here is \"routers\". Plural here is intentional as here it finishes to process all routers which came with \"update\" object. In L563 it is log for each of such routers.","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"94cedae0d693093a7df0bfb10d0d1a3a9ad70a5e","unresolved":false,"context_lines":[{"line_number":583,"context_line":"                    LOG.error(\"Removing incompatible router \u0027%s\u0027\","},{"line_number":584,"context_line":"                              router[\u0027id\u0027])"},{"line_number":585,"context_line":"                    self._safe_router_removed(router[\u0027id\u0027])"},{"line_number":586,"context_line":"            except Exception:"},{"line_number":587,"context_line":"                log_verbose_exc("},{"line_number":588,"context_line":"                    \"Failed to process compatible router: %s\" % update_id,"},{"line_number":589,"context_line":"                    router)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_f26e8194","line":586,"updated":"2018-09-18 14:03:42.000000000","message":"why not use with excutils.save_and_reraise_exception():?","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":583,"context_line":"                    LOG.error(\"Removing incompatible router \u0027%s\u0027\","},{"line_number":584,"context_line":"                              router[\u0027id\u0027])"},{"line_number":585,"context_line":"                    self._safe_router_removed(router[\u0027id\u0027])"},{"line_number":586,"context_line":"            except Exception:"},{"line_number":587,"context_line":"                log_verbose_exc("},{"line_number":588,"context_line":"                    \"Failed to process compatible router: %s\" % update_id,"},{"line_number":589,"context_line":"                    router)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_bdee9588","line":586,"in_reply_to":"3f79a3b5_f26e8194","updated":"2018-09-18 20:42:36.000000000","message":"Done","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9bff2a7dfcc694d65713e4f31cc185eb8541fd78","unresolved":false,"context_lines":[{"line_number":585,"context_line":"                    self._safe_router_removed(router[\u0027id\u0027])"},{"line_number":586,"context_line":"            except Exception:"},{"line_number":587,"context_line":"                log_verbose_exc("},{"line_number":588,"context_line":"                    \"Failed to process compatible router: %s\" % update_id,"},{"line_number":589,"context_line":"                    router)"},{"line_number":590,"context_line":"                raise"},{"line_number":591,"context_line":"            LOG.debug(\"Finished a router update for  %s\", router[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_f43669e7","line":588,"range":{"start_line":588,"start_character":64,"end_line":588,"end_character":73},"updated":"2018-09-18 17:54:26.000000000","message":"should also be router[\u0027id\u0027] right?  update_id would be the id of the router from the update but not the related one possibly.  then you can drop update_id from args i think","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":585,"context_line":"                    self._safe_router_removed(router[\u0027id\u0027])"},{"line_number":586,"context_line":"            except Exception:"},{"line_number":587,"context_line":"                log_verbose_exc("},{"line_number":588,"context_line":"                    \"Failed to process compatible router: %s\" % update_id,"},{"line_number":589,"context_line":"                    router)"},{"line_number":590,"context_line":"                raise"},{"line_number":591,"context_line":"            LOG.debug(\"Finished a router update for  %s\", router[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_7dcb1d1c","line":588,"range":{"start_line":588,"start_character":64,"end_line":588,"end_character":73},"in_reply_to":"3f79a3b5_f43669e7","updated":"2018-09-18 20:42:36.000000000","message":"Done","commit_id":"aa62a1e9205da9a75305c2cdb7c80e987fe341a0"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"1df3118004e8cdf24361aa998b316bb2216bf646","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                continue"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":573,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def _process_routers_if_compatible(self, routers):"},{"line_number":576,"context_line":"        for router in routers:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_5b5f370e","line":573,"updated":"2018-09-19 15:47:22.000000000","message":"(both nits)\n\n\"a\" before \u0027routers\u0027 looks unnecessary.\n\nIn addition, this message is similar to the message at L.591, but a value after \"for\" has different meaning. It is better to add some information to distinguish the meanings. For example, \"Finished routers update (update.id \u003d %s)\".","commit_id":"ce1f1154d616cce3e933775cd9708e65737aeada"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"69ff08e176b25d833e00fb0135d330b897dd7229","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                continue"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":573,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def _process_routers_if_compatible(self, routers):"},{"line_number":576,"context_line":"        for router in routers:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_e9575a59","line":573,"updated":"2018-09-19 18:51:20.000000000","message":"@haleyb I think that is the right way to go; if I was debugging a problem that is the kind of information I would want to see.","commit_id":"ce1f1154d616cce3e933775cd9708e65737aeada"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a3a09c65086b50be5d343dfe5d62292af3d1a2d8","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                continue"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":573,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def _process_routers_if_compatible(self, routers):"},{"line_number":576,"context_line":"        for router in routers:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_4163c8b1","line":573,"in_reply_to":"3f79a3b5_5b5f370e","updated":"2018-09-19 16:26:14.000000000","message":"Right.  In both cases the id is a router id, but this update could have triggered updated for other related routers.  So I guess in the \"normal\" case we could see two similar messages:\n\nFinished a router update for 1234\nFinished a routers update for 1234\n\nBut in this new case we could see:\n\nFinished a router update for 1234\nFinished a router update for 5678\nFinished a routers update for 1234\n\nThat could be confusing.\n\nMaybe in the code below code, we pass the update.id and only print sometimes:\n\nif update_id !\u003d router[\u0027id\u0027]:\n    LOG.debug(\"Finished a router update for %s (related router %s)\", router[\u0027id\u0027], update_id)\n\nThen just change this to \"router\" ?  Thoughts?","commit_id":"ce1f1154d616cce3e933775cd9708e65737aeada"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"636a9c5e86ae3c71d72446b23fe61597aa5577d0","unresolved":false,"context_lines":[{"line_number":570,"context_line":"                continue"},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":573,"context_line":"            LOG.debug(\"Finished a routers update for %s\", update.id)"},{"line_number":574,"context_line":""},{"line_number":575,"context_line":"    def _process_routers_if_compatible(self, routers):"},{"line_number":576,"context_line":"        for router in routers:"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_595aca1d","line":573,"in_reply_to":"3f79a3b5_e9575a59","updated":"2018-09-25 21:20:24.000000000","message":"Done","commit_id":"ce1f1154d616cce3e933775cd9708e65737aeada"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a3a09c65086b50be5d343dfe5d62292af3d1a2d8","unresolved":false,"context_lines":[{"line_number":587,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":588,"context_line":"                    log_verbose_exc("},{"line_number":589,"context_line":"                        \"Failed to process compatible router: %s\" %"},{"line_number":590,"context_line":"                        router[\u0027id\u0027], router)"},{"line_number":591,"context_line":"            LOG.debug(\"Finished a router update for  %s\", router[\u0027id\u0027])"},{"line_number":592,"context_line":""},{"line_number":593,"context_line":"    def _process_routers_loop(self):"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_816480b4","line":590,"updated":"2018-09-19 16:26:14.000000000","message":"I\u0027m wondering if this is the right thing to do.  Should we instead continue processing the other related routers, but then return the failure?  Either way we\u0027d do a resync I guess so maybe this is fine.","commit_id":"ce1f1154d616cce3e933775cd9708e65737aeada"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"636a9c5e86ae3c71d72446b23fe61597aa5577d0","unresolved":false,"context_lines":[{"line_number":587,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":588,"context_line":"                    log_verbose_exc("},{"line_number":589,"context_line":"                        \"Failed to process compatible router: %s\" %"},{"line_number":590,"context_line":"                        router[\u0027id\u0027], router)"},{"line_number":591,"context_line":"            LOG.debug(\"Finished a router update for  %s\", router[\u0027id\u0027])"},{"line_number":592,"context_line":""},{"line_number":593,"context_line":"    def _process_routers_loop(self):"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_79f8e6c2","line":590,"in_reply_to":"3f79a3b5_816480b4","updated":"2018-09-25 21:20:24.000000000","message":"Done","commit_id":"ce1f1154d616cce3e933775cd9708e65737aeada"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"86ccdea04088a1a0d51c0df715f8dec70b65c273","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            if update.action !\u003d DELETE_ROUTER and not routers:"},{"line_number":543,"context_line":"                try:"},{"line_number":544,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":545,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":546,"context_line":"                                                          [update.id])"},{"line_number":547,"context_line":"                except Exception:"},{"line_number":548,"context_line":"                    msg \u003d \"Failed to fetch router information for \u0027%s\u0027\""},{"line_number":549,"context_line":"                    LOG.exception(msg, update.id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_4cf9634c","line":546,"range":{"start_line":545,"start_character":0,"end_line":546,"end_character":70},"updated":"2018-09-25 01:26:43.000000000","message":"Here will get all raleated routers if they connection to one subnet by query one update_id (aka, router_id)?\n\nIf so, I have a race condition concern about this:\n(1) Let\u0027s say we have router_1 and router_2\n(2) Assuming router_1 is under updating, then here will get router_1 info and router_2 info.\n(3) (race condition), assuming that router_2 is doing updating currently in neutron server side, so an updating for router_2 will still be added here also. Because of the different router id (update_id), the router_2 will also be processing in other thread.\nSo what behavior will happen for router_2 ?","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"fa78233fc3ffe2b7e8986c9093b7db67990c9343","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            if update.action !\u003d DELETE_ROUTER and not routers:"},{"line_number":543,"context_line":"                try:"},{"line_number":544,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":545,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":546,"context_line":"                                                          [update.id])"},{"line_number":547,"context_line":"                except Exception:"},{"line_number":548,"context_line":"                    msg \u003d \"Failed to fetch router information for \u0027%s\u0027\""},{"line_number":549,"context_line":"                    LOG.exception(msg, update.id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_a85b1884","line":546,"range":{"start_line":545,"start_character":0,"end_line":546,"end_character":70},"in_reply_to":"3f79a3b5_4abdaed5","updated":"2018-09-26 15:29:08.000000000","message":"So when I went to change this code I ran into a problem.\n\nIf router1 enqueues an update for a related router, router2, then when that is processed it would also enqueue an update for related router, router1.  That looks like an infinite loop.\n\nWe\u0027ll have to think of a way around that, perhaps we just need another action variable to denote that.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"3e060a978c822eee2934059991f45f288e6f56ba","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            if update.action !\u003d DELETE_ROUTER and not routers:"},{"line_number":543,"context_line":"                try:"},{"line_number":544,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":545,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":546,"context_line":"                                                          [update.id])"},{"line_number":547,"context_line":"                except Exception:"},{"line_number":548,"context_line":"                    msg \u003d \"Failed to fetch router information for \u0027%s\u0027\""},{"line_number":549,"context_line":"                    LOG.exception(msg, update.id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_028f8476","line":546,"range":{"start_line":545,"start_character":0,"end_line":546,"end_character":70},"in_reply_to":"3f79a3b5_4cf9634c","updated":"2018-09-26 19:05:44.000000000","message":"Also will it be safe to process multiple routers on the server side and just send rpc calls for 1 router at a time to a particular host.\nWill that work?","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"23cc4bfa1b8ca8b2012769c3f47fa42491bf290a","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            if update.action !\u003d DELETE_ROUTER and not routers:"},{"line_number":543,"context_line":"                try:"},{"line_number":544,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":545,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":546,"context_line":"                                                          [update.id])"},{"line_number":547,"context_line":"                except Exception:"},{"line_number":548,"context_line":"                    msg \u003d \"Failed to fetch router information for \u0027%s\u0027\""},{"line_number":549,"context_line":"                    LOG.exception(msg, update.id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_e45e3b50","line":546,"range":{"start_line":545,"start_character":0,"end_line":546,"end_character":70},"in_reply_to":"3f79a3b5_4cf9634c","updated":"2018-09-25 13:44:28.000000000","message":"Liu,\n\nYes, you make a good point.  The way it works today with the ExclusiveResourceProcessor, any updates for a router ID will be processed by a single worker thread, and if updates arrive while it\u0027s processing, it will loop and process updates until they stop.\n\nPerhaps in this case we need to create a new ResourceUpdate for these associated routers and enqueue it, allowing for the case where there is already a worker processing the router ID.\n\nI think a change like that would still work correctly.\n\nSlawek - do you want me to update or does that make sense?","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a3219e07e55af4bdea67efbd35e82cbef147a07a","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            if update.action !\u003d DELETE_ROUTER and not routers:"},{"line_number":543,"context_line":"                try:"},{"line_number":544,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":545,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":546,"context_line":"                                                          [update.id])"},{"line_number":547,"context_line":"                except Exception:"},{"line_number":548,"context_line":"                    msg \u003d \"Failed to fetch router information for \u0027%s\u0027\""},{"line_number":549,"context_line":"                    LOG.exception(msg, update.id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_970434c7","line":546,"range":{"start_line":545,"start_character":0,"end_line":546,"end_character":70},"in_reply_to":"3f79a3b5_a85b1884","updated":"2018-09-26 18:18:19.000000000","message":"We actually might be Ok since if I create an update and set the resource as the router, L541 will initialize routers to [router], and we shouldn\u0027t call L545 which could possibly add to the list.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"636a9c5e86ae3c71d72446b23fe61597aa5577d0","unresolved":false,"context_lines":[{"line_number":542,"context_line":"            if update.action !\u003d DELETE_ROUTER and not routers:"},{"line_number":543,"context_line":"                try:"},{"line_number":544,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":545,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":546,"context_line":"                                                          [update.id])"},{"line_number":547,"context_line":"                except Exception:"},{"line_number":548,"context_line":"                    msg \u003d \"Failed to fetch router information for \u0027%s\u0027\""},{"line_number":549,"context_line":"                    LOG.exception(msg, update.id)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_4abdaed5","line":546,"range":{"start_line":545,"start_character":0,"end_line":546,"end_character":70},"in_reply_to":"3f79a3b5_e45e3b50","updated":"2018-09-25 21:20:24.000000000","message":"Thx Liu for very good catch :)\n@Brian: I will not have time today and tomorrow to do that so it would be great if You can continue work on it tomorrow. If not, I will continue this work on Thursday","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"86ccdea04088a1a0d51c0df715f8dec70b65c273","unresolved":false,"context_lines":[{"line_number":550,"context_line":"                    self._resync_router(update)"},{"line_number":551,"context_line":"                    continue"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"            if not routers:"},{"line_number":554,"context_line":"                removed \u003d self._safe_router_removed(update.id)"},{"line_number":555,"context_line":"                if not removed:"},{"line_number":556,"context_line":"                    self._resync_router(update)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_4c7c03b6","line":553,"range":{"start_line":553,"start_character":12,"end_line":553,"end_character":27},"updated":"2018-09-25 01:26:43.000000000","message":"Same here, what about router_2 is currently deleting?","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"937288c7d4b0717cf175b509322b0ae13a16fcb8","unresolved":false,"context_lines":[{"line_number":550,"context_line":"                    self._resync_router(update)"},{"line_number":551,"context_line":"                    continue"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"            if not routers:"},{"line_number":554,"context_line":"                removed \u003d self._safe_router_removed(update.id)"},{"line_number":555,"context_line":"                if not removed:"},{"line_number":556,"context_line":"                    self._resync_router(update)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_5f6f8ac3","line":553,"range":{"start_line":553,"start_character":12,"end_line":553,"end_character":27},"in_reply_to":"3f79a3b5_227ac076","updated":"2018-09-27 05:58:36.000000000","message":"We are talking about two routers, update one and delete another, since they are connected or can be queryed by one RPC.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"3e060a978c822eee2934059991f45f288e6f56ba","unresolved":false,"context_lines":[{"line_number":550,"context_line":"                    self._resync_router(update)"},{"line_number":551,"context_line":"                    continue"},{"line_number":552,"context_line":""},{"line_number":553,"context_line":"            if not routers:"},{"line_number":554,"context_line":"                removed \u003d self._safe_router_removed(update.id)"},{"line_number":555,"context_line":"                if not removed:"},{"line_number":556,"context_line":"                    self._resync_router(update)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_227ac076","line":553,"range":{"start_line":553,"start_character":12,"end_line":553,"end_character":27},"in_reply_to":"3f79a3b5_4c7c03b6","updated":"2018-09-26 19:05:44.000000000","message":"In the case of delete it is just one at a time, so the server side handles it.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"3384a103a5aee2ae04c3d32c828436b9980e5ce0","unresolved":false,"context_lines":[{"line_number":560,"context_line":"                    # processing queue (like events from fullsync) in order to"},{"line_number":561,"context_line":"                    # prevent deleted router re-creation"},{"line_number":562,"context_line":"                    rp.fetched_and_processed(update.timestamp)"},{"line_number":563,"context_line":"                LOG.debug(\"Finished router update for %s\", update.id)"},{"line_number":564,"context_line":"                continue"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"            if not self._process_routers_if_compatible(routers, update.id):"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_9cb6d0f9","line":563,"range":{"start_line":563,"start_character":36,"end_line":563,"end_character":42},"updated":"2018-09-24 20:49:01.000000000","message":"routers now ?","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bcf5682ffefe030f39730a23b8495bbe6c0aa1eb","unresolved":false,"context_lines":[{"line_number":560,"context_line":"                    # processing queue (like events from fullsync) in order to"},{"line_number":561,"context_line":"                    # prevent deleted router re-creation"},{"line_number":562,"context_line":"                    rp.fetched_and_processed(update.timestamp)"},{"line_number":563,"context_line":"                LOG.debug(\"Finished router update for %s\", update.id)"},{"line_number":564,"context_line":"                continue"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"            if not self._process_routers_if_compatible(routers, update.id):"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_db5a0bf1","line":563,"range":{"start_line":563,"start_character":36,"end_line":563,"end_character":42},"in_reply_to":"3f79a3b5_9cb6d0f9","updated":"2018-09-24 21:03:05.000000000","message":"I actually think this line shouldn\u0027t change at all - it\u0027s the case where there was an update for a router that is apparently no longer here.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"1669bd34b821eeb06ca90e3e7e2e5992dc5e50f3","unresolved":false,"context_lines":[{"line_number":560,"context_line":"                    # processing queue (like events from fullsync) in order to"},{"line_number":561,"context_line":"                    # prevent deleted router re-creation"},{"line_number":562,"context_line":"                    rp.fetched_and_processed(update.timestamp)"},{"line_number":563,"context_line":"                LOG.debug(\"Finished router update for %s\", update.id)"},{"line_number":564,"context_line":"                continue"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"            if not self._process_routers_if_compatible(routers, update.id):"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_0c8b6b17","line":563,"range":{"start_line":563,"start_character":36,"end_line":563,"end_character":42},"in_reply_to":"3f79a3b5_db5a0bf1","updated":"2018-09-25 00:50:16.000000000","message":"ah ok !","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"3384a103a5aee2ae04c3d32c828436b9980e5ce0","unresolved":false,"context_lines":[{"line_number":568,"context_line":"                continue"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":571,"context_line":"            LOG.debug(\"Finished routers update for %s\", update.id)"},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"    def _process_routers_if_compatible(self, routers, update_id):"},{"line_number":574,"context_line":"        process_result \u003d True"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_97cc69bc","line":571,"range":{"start_line":571,"start_character":12,"end_line":571,"end_character":66},"updated":"2018-09-24 20:49:01.000000000","message":"this is not needed after L591 ? also can the order of logging can be kept same it was before, it was called before rp.fetched_and_processed(update.timestamp)","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bcf5682ffefe030f39730a23b8495bbe6c0aa1eb","unresolved":false,"context_lines":[{"line_number":568,"context_line":"                continue"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":"            rp.fetched_and_processed(update.timestamp)"},{"line_number":571,"context_line":"            LOG.debug(\"Finished routers update for %s\", update.id)"},{"line_number":572,"context_line":""},{"line_number":573,"context_line":"    def _process_routers_if_compatible(self, routers, update_id):"},{"line_number":574,"context_line":"        process_result \u003d True"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_fb0027b4","line":571,"range":{"start_line":571,"start_character":12,"end_line":571,"end_character":66},"in_reply_to":"3f79a3b5_97cc69bc","updated":"2018-09-24 21:03:05.000000000","message":"Since we could possibly be updating many routers based on this single update, we decided to print the original ID here, but the individual ID at L591, that way we will see update messages for them all.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"86ccdea04088a1a0d51c0df715f8dec70b65c273","unresolved":false,"context_lines":[{"line_number":587,"context_line":"                    \"Failed to process compatible router: %s\" %"},{"line_number":588,"context_line":"                    router[\u0027id\u0027], router)"},{"line_number":589,"context_line":"                process_result \u003d False"},{"line_number":590,"context_line":"            if router[\u0027id\u0027] !\u003d update_id:"},{"line_number":591,"context_line":"                LOG.debug(\u0027Finished a router update for %(router_id)s \u0027"},{"line_number":592,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"},{"line_number":593,"context_line":"                          {\u0027router_id\u0027: router[\u0027id\u0027],"},{"line_number":594,"context_line":"                           \u0027related_router_id\u0027: update_id})"},{"line_number":595,"context_line":"        return process_result"},{"line_number":596,"context_line":""},{"line_number":597,"context_line":"    def _process_routers_loop(self):"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_8c721be4","line":594,"range":{"start_line":590,"start_character":0,"end_line":594,"end_character":59},"updated":"2018-09-25 01:26:43.000000000","message":"Please see my comments above.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"1aab43615e9f8cd941cff69b742791f7b057172e","unresolved":false,"context_lines":[{"line_number":577,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":578,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":579,"context_line":"                # on it already and we don\u0027t want to race."},{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_66b74c23","line":580,"updated":"2018-09-27 22:54:34.000000000","message":"Does the request id propagate in the message to the agent so the agent can log it as it processes this message?  That would ease the burden of tying events across between agent and server.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"937288c7d4b0717cf175b509322b0ae13a16fcb8","unresolved":false,"context_lines":[{"line_number":572,"context_line":""},{"line_number":573,"context_line":"    def _process_routers_if_compatible(self, routers, update):"},{"line_number":574,"context_line":"        process_result \u003d True"},{"line_number":575,"context_line":"        for router in routers:"},{"line_number":576,"context_line":"            if router[\u0027id\u0027] !\u003d update.id:"},{"line_number":577,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":578,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":579,"context_line":"                # on it already and we don\u0027t want to race."},{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_bf46de4f","line":580,"range":{"start_line":575,"start_character":0,"end_line":580,"end_character":63},"updated":"2018-09-27 05:58:36.000000000","message":"This will make the developer or operator really hard to do trouble shooting.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"37b5c0d72c72d24d419d0aad077e141213dbda23","unresolved":false,"context_lines":[{"line_number":577,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":578,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":579,"context_line":"                # on it already and we don\u0027t want to race."},{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_071be8ca","line":580,"in_reply_to":"3f79a3b5_66b74c23","updated":"2018-09-28 12:58:22.000000000","message":"No, any context from the RPC message is lost once it\u0027s processed and a ResourceUpdate is enqueued.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"1aab43615e9f8cd941cff69b742791f7b057172e","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_063458ab","line":583,"updated":"2018-09-27 22:54:34.000000000","message":"Since different neutron-server instances could be the ones enqueueing these updatesI think the only way to make sure that these events are sent with an enqueue-time timestamp and  track state on the agent side.  If the agent kept a record of all routers deleted in the past X minutes, and the timestamp each deletion was enqueued, when it could disregard add operations that have an earlier timestamp, and process ones with a later timestamp.  I can\u0027t really think of a better way to do it in a distributed, eventually-consistent system.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"937288c7d4b0717cf175b509322b0ae13a16fcb8","unresolved":false,"context_lines":[{"line_number":573,"context_line":"    def _process_routers_if_compatible(self, routers, update):"},{"line_number":574,"context_line":"        process_result \u003d True"},{"line_number":575,"context_line":"        for router in routers:"},{"line_number":576,"context_line":"            if router[\u0027id\u0027] !\u003d update.id:"},{"line_number":577,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":578,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":579,"context_line":"                # on it already and we don\u0027t want to race."},{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_2405e515","line":583,"range":{"start_line":576,"start_character":0,"end_line":583,"end_character":71},"updated":"2018-09-27 05:58:36.000000000","message":"What if a router_update for one router is adding to queue a little earlier at line 439? And that router_update is dis-connect the subnet. So after these two updates, should the router namespace created in this host or not?","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"37b5c0d72c72d24d419d0aad077e141213dbda23","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_2a126bfe","line":583,"in_reply_to":"3f79a3b5_063458ab","updated":"2018-09-28 12:58:22.000000000","message":"Yes, I thought about keeping state, I just worry about the complexity of storing router_ids and timestamps.\n\nAnother option might be to enqueue this update with an even higher priority than any new incoming RPC (update.priority-1) - that way it should be processed before an in-flight update.\n\nIn the end it\u0027s really up to the server to notify the agent.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"a652adc60606d9368add219fc5a9c1d7e8c818f8","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_a77f9eba","line":583,"in_reply_to":"3f79a3b5_0c91d9f7","updated":"2018-10-10 15:10:20.000000000","message":"Can You check to PS22 - I tried to address it somehow, if here we will add B to the queue, it will not get A again but only update B in such case.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"62e97bcdc059996679aaa057cd275a47eb73bb84","unresolved":false,"context_lines":[{"line_number":573,"context_line":"    def _process_routers_if_compatible(self, routers, update):"},{"line_number":574,"context_line":"        process_result \u003d True"},{"line_number":575,"context_line":"        for router in routers:"},{"line_number":576,"context_line":"            if router[\u0027id\u0027] !\u003d update.id:"},{"line_number":577,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":578,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":579,"context_line":"                # on it already and we don\u0027t want to race."},{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_9b00c137","line":583,"range":{"start_line":576,"start_character":0,"end_line":583,"end_character":71},"in_reply_to":"3f79a3b5_2405e515","updated":"2018-09-27 22:15:18.000000000","message":"Right, I suppose there is the window where this update creates a new queue entry, but that associated router gets deleted by another thread.  When the entry gets processed it will add the router back.  I\u0027m trying to think of a way to avoid that without adding even more state, suggestions are welcome.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"5845d5d29115aabc481a08fc6d1d6867454e6285","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_6cf9744c","line":583,"in_reply_to":"3f79a3b5_2a126bfe","updated":"2018-10-10 10:22:42.000000000","message":"Looking at https://github.com/openstack/neutron/blob/master/neutron/agent/common/resource_processing_queue.py#L42 it looks for me that using lower priority here should be good but we will have to use value lower than PRIORITY_RPC probably.\n\nThere is one more thing however which may lead us to problem. Imagine the case:\n1. Router A update is send, in L545 it gets also Router B as related to A,\n2. So it calls _process_routers_if_compatible() with 2 routers [A, B],\n3. Now router B is added to updates queue so will be processed in _process_router_update in same way how router A was,\n4. so for router B in L545 we will get list of routers with A and B again, right?\n5. If so, it will again add router A this time to updates queue...\n6. It will be endless loop IMO in such case\n\nor am I missing something and it will be stopped somehow in some place?","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"d09a6f9751cd45d94f07e1af09238af35457c5d4","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_0c91d9f7","line":583,"in_reply_to":"3f79a3b5_6cf9744c","updated":"2018-10-10 14:50:53.000000000","message":"Sounds like a true and long story, it can happen in some extreme case, and seems a fixed/variable higher priority can not prevent such scenario. Since every time A was processing it will add B in queue, every time B was processing it will add A in queue, and each will have a high level priority. So finally, it will block a delete or a normal router update.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"581e1deebd0312c7f1f9ddc2cb44014937c3f553","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_a3b1cb03","line":583,"in_reply_to":"3f79a3b5_796fea92","updated":"2018-10-10 19:55:24.000000000","message":"If I understand correctly Your first comment here, it looks for me that router added to queue in L439 will have lower priority than this update - so this will be done first, and after that update from L439 will be done - so finally subnet should be removed as expected, no?","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ed6ad309b7e64e9dc74a41d768fa8aff8cbe49ff","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_cece642b","line":583,"in_reply_to":"3f79a3b5_a3b1cb03","updated":"2018-10-11 08:42:10.000000000","message":"Line 439 enqueue router_update_1 and can dequeue (maybe we can call it as in processing state) quickly in line 533, since this update_2 here may not enqueue at all. Because such queue is not preemptive, it can not stop a running process procedure. So the update_1 will remove the router? this update_2 will add it back?","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ad6cba56ac92beb08a187e9db9eb7137e8c5b9ac","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_796fea92","line":583,"in_reply_to":"3f79a3b5_a77f9eba","updated":"2018-10-10 15:36:20.000000000","message":"Hmm, seems that the infinite loop can be avoid in some case.\nBut for my first question here in this comments thread, it can still happen.","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"e41c179fc8248c3bfd2ba35705109a57d87e2a93","unresolved":false,"context_lines":[{"line_number":580,"context_line":"                new_update \u003d queue.ResourceUpdate(router[\u0027id\u0027],"},{"line_number":581,"context_line":"                                                  update.priority,"},{"line_number":582,"context_line":"                                                  resource\u003drouter,"},{"line_number":583,"context_line":"                                                  action\u003dupdate.action)"},{"line_number":584,"context_line":"                self._queue.add(new_update)"},{"line_number":585,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":586,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f79a3b5_dac93aab","line":583,"in_reply_to":"3f79a3b5_cece642b","updated":"2018-10-11 11:15:55.000000000","message":"I don\u0027t think it will be added again. Such update will go to self.plugin_rpc.get_routers() in L545 and then, as this router is already deleted from this node, it should be returned back from neutron-server. If so, then it will not recreate it again. Or am I still missing something?","commit_id":"81a69d059d4a0e3198810ca2732b2214c1f209cc"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"be80751520e08f6134d70609cce30207d74eb2d1","unresolved":false,"context_lines":[{"line_number":71,"context_line":"PRIORITY_SYNC_ROUTERS_TASK \u003d 2"},{"line_number":72,"context_line":"PRIORITY_PD_UPDATE \u003d 3"},{"line_number":73,"context_line":"DELETE_ROUTER \u003d 2"},{"line_number":74,"context_line":"PD_UPDATE \u003d 3"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"def log_verbose_exc(message, router_payload):"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f79a3b5_8a7a8d22","line":74,"updated":"2018-10-11 15:56:14.000000000","message":"These last two are actions and not priorities, so don\u0027t need to change.","commit_id":"2a2f4bf59b95dcf88e1fce97b8fe2475ac148981"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"be80751520e08f6134d70609cce30207d74eb2d1","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":547,"context_line":"                                                          [update.id])"},{"line_number":548,"context_line":"                    if update.priority \u003d\u003d PRIORITY_RELATED_ROUTER_UPDATE:"},{"line_number":549,"context_line":"                        # NOTE(slaweq): in case if it was triggered by"},{"line_number":550,"context_line":"                        # _process_routers_if_compatible method, there is no"},{"line_number":551,"context_line":"                        # need to update all related routers again as this one"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f79a3b5_ca33a511","line":548,"updated":"2018-10-11 15:56:14.000000000","message":"Should this check for !\u003d and L546 is moved inside?\n\nThere should be a single router in the update already, will save the call and the filtering below.","commit_id":"2a2f4bf59b95dcf88e1fce97b8fe2475ac148981"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d6347a8e1b1275322d1d87587c3bf81f6156cef8","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":547,"context_line":"                                                          [update.id])"},{"line_number":548,"context_line":"                    if update.priority \u003d\u003d PRIORITY_RELATED_ROUTER_UPDATE:"},{"line_number":549,"context_line":"                        # NOTE(slaweq): in case if it was triggered by"},{"line_number":550,"context_line":"                        # _process_routers_if_compatible method, there is no"},{"line_number":551,"context_line":"                        # need to update all related routers again as this one"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f79a3b5_e5b5727e","line":548,"in_reply_to":"3f79a3b5_ca33a511","updated":"2018-10-11 16:20:08.000000000","message":"I think that we should call it to avoid the race condition decribed in comments to PS18 described - in case if router was already deleted by other thread, it should not get it here back and not reconfigure it if it\u0027s not necessary","commit_id":"2a2f4bf59b95dcf88e1fce97b8fe2475ac148981"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"850ad6e6b2df98fa1f0f2aea378a4dcc09d19579","unresolved":false,"context_lines":[{"line_number":580,"context_line":"    def _process_routers_if_compatible(self, routers, update):"},{"line_number":581,"context_line":"        process_result \u003d True"},{"line_number":582,"context_line":"        for router in routers:"},{"line_number":583,"context_line":"            if router[\u0027id\u0027] !\u003d update.id:"},{"line_number":584,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":585,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":586,"context_line":"                # on it already and we don\u0027t want to race."}],"source_content_type":"text/x-python","patch_set":23,"id":"3f79a3b5_2f6d76ca","line":583,"range":{"start_line":583,"start_character":0,"end_line":583,"end_character":41},"updated":"2018-10-12 10:27:06.000000000","message":"Sorry, I can produce the race condition (mentioned in Patch Set 18 [1]), after the testing, one router without any subnet connection can qrouter namespace in compute node, here is the step:\n\n\nstep 1: create the test scenario described in the LP bug description:\nopenstack network create AP1Site\nopenstack network create AP2Site\nopenstack network create Wan\nopenstack subnet create --gateway\u003d10.10.210.254 --network AP1Site --subnet-range 10.10.210.0/24 AP1_sub\nopenstack subnet create --gateway\u003d10.10.220.254 --network AP2Site --subnet-range 10.10.220.0/24 AP2_sub\nopenstack subnet create --gateway\u003d10.10.200.254 --network Wan --subnet-range 10.10.200.0/24 Wan_sub\nopenstack port create --network AP1Site --fixed-ip subnet\u003dAP1_sub,ip-address\u003d10.10.210.10 AP1Port-AP1Internal\nopenstack port create --network AP2Site --fixed-ip subnet\u003dAP2_sub,ip-address\u003d10.10.220.10 AP2Port-AP2Internal\nport_id_1\u003d`openstack port show AP1Port-AP1Internal|grep \"| id\"|awk \u0027{print $4}\u0027`\nnova boot --flavor nano --image cirros --nic port-id\u003d$port_id_1 AP1\nport_id_2\u003d`openstack port show AP2Port-AP2Internal|grep \"| id\"|awk \u0027{print $4}\u0027`\nnova boot --flavor nano --image cirros --nic port-id\u003d$port_id_2 AP2\nopenstack router create --distributed --no-ha AP2-RT\nopenstack router create --distributed --no-ha AP1-RT\nopenstack port create --network Wan --fixed-ip subnet\u003dWan_sub,ip-address\u003d10.10.200.10 AP1-WANPort\nopenstack port create --network Wan --fixed-ip subnet\u003dWan_sub,ip-address\u003d10.10.200.20 AP2-WANPort\nopenstack router add port AP2-RT AP2-WANPort\nopenstack router add port AP1-RT AP1-WANPort\nopenstack router add subnet AP2-RT AP2_sub\nopenstack router add subnet AP1-RT AP1_sub\n\nHere I just skip the custom route rules.\n\nStep 2： remove one router (AP1-RT) from private Site (AP1Site)\nRemove router AP1-RT interface of AP1_sub, you can still see 2 router namespace in the compute node.\n$ openstack router remove subnet AP1-RT AP1_sub\n\nFor this step, I have a question, sometimes I can see the AP1-RT qrouter-namespace in compute node, sometimes not. IMO, this router is still connected to WAN, it should have the namespace in compute node.\n\nStep 3: Concurrently adding one router (AP2-RT) to another Site (site_3) and removing router (AP1-RT) from Wan_sub, \nopenstack router add subnet \u003cAP2-RT\u003e \u003csite_3\u003e \u0026 openstack router remove subnet AP1-RT Wan_sub \u0026\n\nFor my test, I use the ID instead of name to prevent some query time of the openstack client,  in order to simulate the parallel API call.\n$ openstack router add subnet 9934e360-2ad6-4cb8-a315-7ae99f3fb2e3 77b01132-0341-42d3-8b0c-e20a433ce931 \u0026 openstack router remove subnet 1d47934d-c4c7-44bf-8002-2bc36a492053 c8b119d4-16dd-4467-8ea2-3273f342022b \u0026\n\nThen, you can see AP1-RT qrouter-namespace is stay in compute node without any subnet interface.\n\n[1] https://review.openstack.org/#/c/597567/18/neutron/agent/l3/agent.py@583","commit_id":"53b517408224792d54cac50d84ff18597e18d206"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"19c42e91581a09c6bd7b1b36dd1133be2c8757e8","unresolved":false,"context_lines":[{"line_number":580,"context_line":"    def _process_routers_if_compatible(self, routers, update):"},{"line_number":581,"context_line":"        process_result \u003d True"},{"line_number":582,"context_line":"        for router in routers:"},{"line_number":583,"context_line":"            if router[\u0027id\u0027] !\u003d update.id:"},{"line_number":584,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":585,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":586,"context_line":"                # on it already and we don\u0027t want to race."}],"source_content_type":"text/x-python","patch_set":23,"id":"3f79a3b5_e3c1d582","line":583,"range":{"start_line":583,"start_character":0,"end_line":583,"end_character":41},"in_reply_to":"3f79a3b5_2f6d76ca","updated":"2018-10-12 13:17:22.000000000","message":"I see problem which You pointed here but IMO it is even worst as it is not race condition but some bug in this patch. Even if I removed AP1-RT from WAN subnet, it wasn\u0027t removed from compute node (when it should be).\nI will investigate that...","commit_id":"53b517408224792d54cac50d84ff18597e18d206"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"67cccf3292a6f720026b152c742f7b36c0a8c8a1","unresolved":false,"context_lines":[{"line_number":580,"context_line":"    def _process_routers_if_compatible(self, routers, update):"},{"line_number":581,"context_line":"        process_result \u003d True"},{"line_number":582,"context_line":"        for router in routers:"},{"line_number":583,"context_line":"            if router[\u0027id\u0027] !\u003d update.id:"},{"line_number":584,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":585,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":586,"context_line":"                # on it already and we don\u0027t want to race."}],"source_content_type":"text/x-python","patch_set":23,"id":"3f79a3b5_d381dc4e","line":583,"range":{"start_line":583,"start_character":0,"end_line":583,"end_character":41},"in_reply_to":"3f79a3b5_78ed4eb6","updated":"2018-10-15 17:04:10.000000000","message":"Great, I will test the new patch set.","commit_id":"53b517408224792d54cac50d84ff18597e18d206"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"890a39aba8ddd0bb491716d02cd714d994b00c0e","unresolved":false,"context_lines":[{"line_number":580,"context_line":"    def _process_routers_if_compatible(self, routers, update):"},{"line_number":581,"context_line":"        process_result \u003d True"},{"line_number":582,"context_line":"        for router in routers:"},{"line_number":583,"context_line":"            if router[\u0027id\u0027] !\u003d update.id:"},{"line_number":584,"context_line":"                # Don\u0027t do the work here, instead create a new update and"},{"line_number":585,"context_line":"                # enqueue it, since there could be another thread working"},{"line_number":586,"context_line":"                # on it already and we don\u0027t want to race."}],"source_content_type":"text/x-python","patch_set":23,"id":"3f79a3b5_78ed4eb6","line":583,"range":{"start_line":583,"start_character":0,"end_line":583,"end_character":41},"in_reply_to":"3f79a3b5_e3c1d582","updated":"2018-10-15 12:27:53.000000000","message":"So I was doing some more tests on that and it looks for me that issue which You described was not related to race but simply because sending notification to other dvr routers that router\u0027s interface is removed is done in AFTER_DELETE notification. Then we already don\u0027t have any information that router A was connected with router B and that \"router_delete\" should be send to some hosts.\nI try to address it in PS24. With this change I couldn\u0027t reproduce issue which You described here.","commit_id":"53b517408224792d54cac50d84ff18597e18d206"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":547,"context_line":"                                                          [update.id])"},{"line_number":548,"context_line":"                    if update.priority \u003d\u003d PRIORITY_RELATED_ROUTER_UPDATE:"},{"line_number":549,"context_line":"                        # NOTE(slaweq): in case if it was triggered by"},{"line_number":550,"context_line":"                        # _process_routers_if_compatible method, there is no"},{"line_number":551,"context_line":"                        # need to update all related routers again as this one"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_dcf7455a","line":548,"updated":"2018-10-19 16:22:19.000000000","message":"Looking closer, can this ever happen?  For example, L587 creates a new update with this priority and resource\u003drouter, so \u0027routers\u0027 on L542 will always be something, not triggering L543 to fall-into this code.","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a162a6e2b231e61e3ede0be3b8724d6ed4bf718c","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":547,"context_line":"                                                          [update.id])"},{"line_number":548,"context_line":"                    if update.priority \u003d\u003d PRIORITY_RELATED_ROUTER_UPDATE:"},{"line_number":549,"context_line":"                        # NOTE(slaweq): in case if it was triggered by"},{"line_number":550,"context_line":"                        # _process_routers_if_compatible method, there is no"},{"line_number":551,"context_line":"                        # need to update all related routers again as this one"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_f5178eee","line":548,"in_reply_to":"3f79a3b5_6959788b","updated":"2018-10-22 17:20:58.000000000","message":"Liu, yes, this was getting all the routers, then removing the one from this update, I remember Slawek describing it to me.  Maybe because it was Friday :) I couldn\u0027t figure how it was every called though - if I made a mistake we can put it back.\n\nIf L587 creates an update with update.resource\u003drouter, then on L542 routers\u003d[update.resource], so L 542 will always be False.\n\nThat\u0027s why I originally thought this check was in the wrong place perhaps?  Has any of the testing shown an infinite loop?  I guess theoretically it is possible.","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f29066b32cf2089064f3528b56347c124b2fbfbd","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":547,"context_line":"                                                          [update.id])"},{"line_number":548,"context_line":"                    if update.priority \u003d\u003d PRIORITY_RELATED_ROUTER_UPDATE:"},{"line_number":549,"context_line":"                        # NOTE(slaweq): in case if it was triggered by"},{"line_number":550,"context_line":"                        # _process_routers_if_compatible method, there is no"},{"line_number":551,"context_line":"                        # need to update all related routers again as this one"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_d1efffaf","line":548,"in_reply_to":"3f79a3b5_7b4fec34","updated":"2018-10-24 19:32:15.000000000","message":"Right, but is the infinite loop something like this:\n\nUpdate for Router1, related router update queued for Router2\nRelate","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"e0e96d39b41adb080ebe813e678df56c096c9575","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":547,"context_line":"                                                          [update.id])"},{"line_number":548,"context_line":"                    if update.priority \u003d\u003d PRIORITY_RELATED_ROUTER_UPDATE:"},{"line_number":549,"context_line":"                        # NOTE(slaweq): in case if it was triggered by"},{"line_number":550,"context_line":"                        # _process_routers_if_compatible method, there is no"},{"line_number":551,"context_line":"                        # need to update all related routers again as this one"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_6959788b","line":548,"in_reply_to":"3f79a3b5_dcf7455a","updated":"2018-10-22 14:15:12.000000000","message":"According to the previous comments, I think this is to prevent the infinite loop for two connected routers processing scenario:\nhttps://review.openstack.org/#/c/597567/18/neutron/agent/l3/agent.py@583","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"c584171b9c33deda88141a3a14175ee81cb0af73","unresolved":false,"context_lines":[{"line_number":545,"context_line":"                    update.timestamp \u003d timeutils.utcnow()"},{"line_number":546,"context_line":"                    routers \u003d self.plugin_rpc.get_routers(self.context,"},{"line_number":547,"context_line":"                                                          [update.id])"},{"line_number":548,"context_line":"                    if update.priority \u003d\u003d PRIORITY_RELATED_ROUTER_UPDATE:"},{"line_number":549,"context_line":"                        # NOTE(slaweq): in case if it was triggered by"},{"line_number":550,"context_line":"                        # _process_routers_if_compatible method, there is no"},{"line_number":551,"context_line":"                        # need to update all related routers again as this one"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_7b4fec34","line":548,"in_reply_to":"3f79a3b5_f5178eee","updated":"2018-10-22 20:32:31.000000000","message":"I think, You are right Brian. It can\u0027t get here if \"routers\" variable is set to [update.resource] in L542 and it will always be like that if event is added to the queue in L587 in this PS (L581 in PS29)","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"656e25fe4128659992d7c1432dc0a6aca75efae3","unresolved":false,"context_lines":[{"line_number":609,"context_line":"                LOG.debug(\u0027Queued a router update for %(router_id)s \u0027"},{"line_number":610,"context_line":"                          \u0027(related router %(related_router_id)s)\u0027,"},{"line_number":611,"context_line":"                          {\u0027router_id\u0027: router[\u0027id\u0027],"},{"line_number":612,"context_line":"                           \u0027related_router_id\u0027: update.id})"},{"line_number":613,"context_line":"                continue"},{"line_number":614,"context_line":""},{"line_number":615,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":31,"id":"3f79a3b5_1300b402","line":612,"updated":"2018-10-26 21:00:12.000000000","message":"Slawek and I talked about adding \u0027action\u0027 to this debug message, so we know what we just created, since he did see a message like this from the loop above:\n\nStarting router update for 22b882f0-2847-43d7-a2a8-d633c4a6b855, action None, priority 0\n\nThat priority was set here, but action None was odd, maybe explained by my comment in the fullsync code below?","commit_id":"5cd790b6dab750ed9d8b4b175a4fcc4bce898d77"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"656e25fe4128659992d7c1432dc0a6aca75efae3","unresolved":false,"context_lines":[{"line_number":689,"context_line":"                        r[\u0027id\u0027],"},{"line_number":690,"context_line":"                        PRIORITY_SYNC_ROUTERS_TASK,"},{"line_number":691,"context_line":"                        resource\u003dr,"},{"line_number":692,"context_line":"                        timestamp\u003dtimestamp)"},{"line_number":693,"context_line":"                    self._queue.add(update)"},{"line_number":694,"context_line":"        except oslo_messaging.MessagingTimeout:"},{"line_number":695,"context_line":"            if self.sync_routers_chunk_size \u003e SYNC_ROUTERS_MIN_CHUNK_SIZE:"}],"source_content_type":"text/x-python","patch_set":31,"id":"3f79a3b5_d348fc8a","line":692,"updated":"2018-10-26 21:00:12.000000000","message":"I guess this should have \u0027action\u003dADD_UPDATE_ROUTER\u0027 to be complete, in case we somehow get here after the agent has started up.","commit_id":"5cd790b6dab750ed9d8b4b175a4fcc4bce898d77"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"ceb1b50f2bd313305fd94b0ab08e93781452dc8a","unresolved":false,"context_lines":[{"line_number":689,"context_line":"                        r[\u0027id\u0027],"},{"line_number":690,"context_line":"                        PRIORITY_SYNC_ROUTERS_TASK,"},{"line_number":691,"context_line":"                        resource\u003dr,"},{"line_number":692,"context_line":"                        timestamp\u003dtimestamp)"},{"line_number":693,"context_line":"                    self._queue.add(update)"},{"line_number":694,"context_line":"        except oslo_messaging.MessagingTimeout:"},{"line_number":695,"context_line":"            if self.sync_routers_chunk_size \u003e SYNC_ROUTERS_MIN_CHUNK_SIZE:"}],"source_content_type":"text/x-python","patch_set":31,"id":"3f79a3b5_65e7dfd8","line":692,"in_reply_to":"3f79a3b5_d348fc8a","updated":"2018-10-26 21:42:40.000000000","message":"Sounds reasonable.","commit_id":"5cd790b6dab750ed9d8b4b175a4fcc4bce898d77"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"91697403456bd4289a395dca9b0a7b7a23a37ac6","unresolved":false,"context_lines":[{"line_number":610,"context_line":"                          \u0027Original event action %(action)s, \u0027"},{"line_number":611,"context_line":"                          \u0027priority %(priority)s. \u0027"},{"line_number":612,"context_line":"                          \u0027New event action %(new_action)s, \u0027"},{"line_number":613,"context_line":"                          \u0027%(new_priority)s\u0027,"},{"line_number":614,"context_line":"                          {\u0027router_id\u0027: router[\u0027id\u0027],"},{"line_number":615,"context_line":"                           \u0027related_router_id\u0027: update.id,"},{"line_number":616,"context_line":"                           \u0027action\u0027: update.action,"}],"source_content_type":"text/x-python","patch_set":39,"id":"3f79a3b5_cc034575","line":613,"range":{"start_line":613,"start_character":27,"end_line":613,"end_character":28},"updated":"2018-10-31 01:36:34.000000000","message":"nit: missing word \u0027priority\u0027 here as L611","commit_id":"b3673aa7fdd1ea5d2c5e678c1f1f0acf7913758a"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"bcd883c66e73407aaee206e237102d4827861ac3","unresolved":false,"context_lines":[{"line_number":610,"context_line":"                          \u0027Original event action %(action)s, \u0027"},{"line_number":611,"context_line":"                          \u0027priority %(priority)s. \u0027"},{"line_number":612,"context_line":"                          \u0027New event action %(new_action)s, \u0027"},{"line_number":613,"context_line":"                          \u0027%(new_priority)s\u0027,"},{"line_number":614,"context_line":"                          {\u0027router_id\u0027: router[\u0027id\u0027],"},{"line_number":615,"context_line":"                           \u0027related_router_id\u0027: update.id,"},{"line_number":616,"context_line":"                           \u0027action\u0027: update.action,"}],"source_content_type":"text/x-python","patch_set":39,"id":"3f79a3b5_12bb87be","line":613,"range":{"start_line":613,"start_character":27,"end_line":613,"end_character":28},"in_reply_to":"3f79a3b5_cc034575","updated":"2018-10-31 14:45:48.000000000","message":"Done","commit_id":"b3673aa7fdd1ea5d2c5e678c1f1f0acf7913758a"}],"neutron/db/l3_dvr_db.py":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9b0056cb11bccafd3effd406e767e476aea0a615","unresolved":false,"context_lines":[{"line_number":537,"context_line":"            context, subnet_ids\u003d{ip[\u0027subnet_id\u0027] for ip in port[\u0027fixed_ips\u0027]})"},{"line_number":538,"context_line":"        router_hosts_after \u003d plugin._get_dvr_hosts_for_router("},{"line_number":539,"context_line":"            context, router_id)"},{"line_number":540,"context_line":"        removed_hosts \u003d set(router_hosts_for_removed) - set(router_hosts_after)"},{"line_number":541,"context_line":"        if removed_hosts:"},{"line_number":542,"context_line":"            agents \u003d plugin.get_l3_agents(context,"},{"line_number":543,"context_line":"                                          filters\u003d{\u0027host\u0027: removed_hosts})"}],"source_content_type":"text/x-python","patch_set":23,"id":"3f79a3b5_da2cb702","line":540,"updated":"2018-10-12 15:58:28.000000000","message":"It looks for me that when I delete this interface from one of routers, this removed_hosts set is empty so it\u0027s not going to L555 and not send notifications","commit_id":"53b517408224792d54cac50d84ff18597e18d206"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"5c78a55d58ba5a09898f4b23fb56903f3035e4c7","unresolved":false,"context_lines":[{"line_number":520,"context_line":"        return False"},{"line_number":521,"context_line":""},{"line_number":522,"context_line":"    @registry.receives(resources.ROUTER_INTERFACE, [events.BEFORE_DELETE])"},{"line_number":523,"context_line":"    def _cleanup_before_interface_removal(self, resource, event, trigger,"},{"line_number":524,"context_line":"                                          context, **kwargs):"},{"line_number":525,"context_line":"        router_id \u003d kwargs.get(\"router_id\")"},{"line_number":526,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_2e8bb469","line":523,"range":{"start_line":523,"start_character":8,"end_line":523,"end_character":41},"updated":"2018-10-18 18:44:48.000000000","message":"Do we need to have the function name as _cleanup, since it does not seem to be cleaning up anything here.\nRather it caches the router_hosts, before deleteing it.","commit_id":"184fe95b112837dd4e76b2abe88c22418a7aa705"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"5c78a55d58ba5a09898f4b23fb56903f3035e4c7","unresolved":false,"context_lines":[{"line_number":568,"context_line":"                if not is_this_snat_agent:"},{"line_number":569,"context_line":"                    self.l3plugin.l3_rpc_notifier.router_removed_from_agent("},{"line_number":570,"context_line":"                        context, router_id, agent[\u0027host\u0027])"},{"line_number":571,"context_line":"                    for connected_router_id in connected_dvr_routers:"},{"line_number":572,"context_line":"                        related_dvr_router_hosts \u003d set("},{"line_number":573,"context_line":"                            self._get_other_dvr_hosts("},{"line_number":574,"context_line":"                                context, connected_router_id))"},{"line_number":575,"context_line":"                        if agent[\u0027host\u0027] not in related_dvr_router_hosts:"},{"line_number":576,"context_line":"                            self.l3plugin.l3_rpc_notifier.\\"},{"line_number":577,"context_line":"                                router_removed_from_agent("},{"line_number":578,"context_line":"                                    context, connected_router_id,"},{"line_number":579,"context_line":"                                    agent[\u0027host\u0027])"},{"line_number":580,"context_line":"        self._cleanup_related_hosts_after_interface_removal("},{"line_number":581,"context_line":"            context, router_id, router_hosts_after)"},{"line_number":582,"context_line":"        # if subnet_id not in interface_info, request was to remove by port"},{"line_number":583,"context_line":"        sub_id \u003d (interface_info.get(\u0027subnet_id\u0027) or"},{"line_number":584,"context_line":"                  port[\u0027fixed_ips\u0027][0][\u0027subnet_id\u0027])"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_294e4e39","line":581,"range":{"start_line":571,"start_character":9,"end_line":581,"end_character":51},"updated":"2018-10-18 18:44:48.000000000","message":"I think there is some issue in this logic.\nHere is what I tested.\nI had 5 networks.\nI had 3 routers.\nnet1 -\u003e router1\nnet2 -\u003e router2\nnet3 -\u003e router3\nnet4 (ports) -\u003e router1 and router2  (connected)\nnet5 (ports) -\u003e router2 and router3  (connected).\n\nI had 1VM created on net1-(host1), 1VM Created on net2-(host2), and 2nd VM Created on net3-(host2).\n\nNow I try to remove the interfaces one by one.\n\n1. Removed net1 interface from router1 ( did see that all the router-namespace in Compute1 got cleared). ( VALID)\n2. Router1 namespace was still in Compute2, (VALID) since we still have an associated port on \u0027net4).\n3. Removed the port from router1 ( which is on net4). Now saw that the router namespace of router1 in Compute2 vanished. (VALID).\n4. Now removed the interface of Net3 from router3. ( INVALID)\n - What I saw was the router2 namespace was removed from HOST2 along with the interface that was in Router3 (net3).\nSo router2 still has VM in the HOST2, but the namespace was missing.","commit_id":"184fe95b112837dd4e76b2abe88c22418a7aa705"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"712de5c6dffc68db4932a7be051bca56e54c867f","unresolved":false,"context_lines":[{"line_number":568,"context_line":"                if not is_this_snat_agent:"},{"line_number":569,"context_line":"                    self.l3plugin.l3_rpc_notifier.router_removed_from_agent("},{"line_number":570,"context_line":"                        context, router_id, agent[\u0027host\u0027])"},{"line_number":571,"context_line":"                    for connected_router_id in connected_dvr_routers:"},{"line_number":572,"context_line":"                        related_dvr_router_hosts \u003d set("},{"line_number":573,"context_line":"                            self._get_other_dvr_hosts("},{"line_number":574,"context_line":"                                context, connected_router_id))"},{"line_number":575,"context_line":"                        if agent[\u0027host\u0027] not in related_dvr_router_hosts:"},{"line_number":576,"context_line":"                            self.l3plugin.l3_rpc_notifier.\\"},{"line_number":577,"context_line":"                                router_removed_from_agent("},{"line_number":578,"context_line":"                                    context, connected_router_id,"},{"line_number":579,"context_line":"                                    agent[\u0027host\u0027])"},{"line_number":580,"context_line":"        self._cleanup_related_hosts_after_interface_removal("},{"line_number":581,"context_line":"            context, router_id, router_hosts_after)"},{"line_number":582,"context_line":"        # if subnet_id not in interface_info, request was to remove by port"},{"line_number":583,"context_line":"        sub_id \u003d (interface_info.get(\u0027subnet_id\u0027) or"},{"line_number":584,"context_line":"                  port[\u0027fixed_ips\u0027][0][\u0027subnet_id\u0027])"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_c9823a00","line":581,"range":{"start_line":571,"start_character":9,"end_line":581,"end_character":51},"in_reply_to":"3f79a3b5_294e4e39","updated":"2018-10-18 18:53:40.000000000","message":"See the console log of what I did here.\nhttp://paste.openstack.org/show/732442/","commit_id":"184fe95b112837dd4e76b2abe88c22418a7aa705"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"9bacf11416c7a99211d5d6ebf40dc830838883fc","unresolved":false,"context_lines":[{"line_number":568,"context_line":"                if not is_this_snat_agent:"},{"line_number":569,"context_line":"                    self.l3plugin.l3_rpc_notifier.router_removed_from_agent("},{"line_number":570,"context_line":"                        context, router_id, agent[\u0027host\u0027])"},{"line_number":571,"context_line":"                    for connected_router_id in connected_dvr_routers:"},{"line_number":572,"context_line":"                        related_dvr_router_hosts \u003d set("},{"line_number":573,"context_line":"                            self._get_other_dvr_hosts("},{"line_number":574,"context_line":"                                context, connected_router_id))"},{"line_number":575,"context_line":"                        if agent[\u0027host\u0027] not in related_dvr_router_hosts:"},{"line_number":576,"context_line":"                            self.l3plugin.l3_rpc_notifier.\\"},{"line_number":577,"context_line":"                                router_removed_from_agent("},{"line_number":578,"context_line":"                                    context, connected_router_id,"},{"line_number":579,"context_line":"                                    agent[\u0027host\u0027])"},{"line_number":580,"context_line":"        self._cleanup_related_hosts_after_interface_removal("},{"line_number":581,"context_line":"            context, router_id, router_hosts_after)"},{"line_number":582,"context_line":"        # if subnet_id not in interface_info, request was to remove by port"},{"line_number":583,"context_line":"        sub_id \u003d (interface_info.get(\u0027subnet_id\u0027) or"},{"line_number":584,"context_line":"                  port[\u0027fixed_ips\u0027][0][\u0027subnet_id\u0027])"}],"source_content_type":"text/x-python","patch_set":27,"id":"3f79a3b5_24719346","line":581,"range":{"start_line":571,"start_character":9,"end_line":581,"end_character":51},"in_reply_to":"3f79a3b5_c9823a00","updated":"2018-10-18 19:49:46.000000000","message":"I will investigate that tomorrow morning. Thx for feedback","commit_id":"184fe95b112837dd4e76b2abe88c22418a7aa705"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":521,"context_line":"        return False"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"    @registry.receives(resources.ROUTER_INTERFACE, [events.BEFORE_DELETE])"},{"line_number":524,"context_line":"    def _cache_related_routers_info_before_interface_removal("},{"line_number":525,"context_line":"            self, resource, event, trigger, context, **kwargs):"},{"line_number":526,"context_line":"        router_id \u003d kwargs.get(\"router_id\")"},{"line_number":527,"context_line":""}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_dc7ae5b9","line":524,"range":{"start_line":524,"start_character":23,"end_line":524,"end_character":30},"updated":"2018-10-19 16:22:19.000000000","message":"super nit: s/routers/dvr_routers","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":633,"context_line":""},{"line_number":634,"context_line":"    def _cleanup_related_routers_after_interface_removal("},{"line_number":635,"context_line":"            self, context, router_id, router_hosts):"},{"line_number":636,"context_line":""},{"line_number":637,"context_line":"        related_dvr_routers_before \u003d self.related_dvr_router_routers.pop("},{"line_number":638,"context_line":"            router_id, set())"},{"line_number":639,"context_line":"        related_dvr_routers_after \u003d set("}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_df989777","line":636,"updated":"2018-10-19 16:22:19.000000000","message":"super nit: extra line","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"1c9feba9d39d394918f83e0dcc7df7ca091feec6","unresolved":false,"context_lines":[{"line_number":618,"context_line":"        related_dvr_router_hosts_after \u003d set(self._get_other_dvr_hosts("},{"line_number":619,"context_line":"            context, router_id))"},{"line_number":620,"context_line":"        related_dvr_router_hosts_before -\u003d set(router_hosts)"},{"line_number":621,"context_line":"        related_removed_hosts \u003d ("},{"line_number":622,"context_line":"            related_dvr_router_hosts_before - related_dvr_router_hosts_after)"},{"line_number":623,"context_line":"        if related_removed_hosts:"},{"line_number":624,"context_line":"            agents \u003d self.l3plugin.get_l3_agents("}],"source_content_type":"text/x-python","patch_set":36,"id":"3f79a3b5_f6686a22","line":621,"updated":"2018-10-29 20:41:19.000000000","message":"It looks that it still may happen that router will not be cleaned from host and for now I think that issue is somehow here.\nStill investigating it locally...","commit_id":"20e0f2450f99228dba7f7db70d9dafaf2b5c3a0d"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"20833219b94da55c2bddfd7e06165e4d9ddab473","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    necessary."},{"line_number":67,"context_line":"    \"\"\""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    related_dvr_router_hosts \u003d {}"},{"line_number":70,"context_line":"    related_dvr_router_routers \u003d {}"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"},{"line_number":73,"context_line":"    def l3plugin(self):"}],"source_content_type":"text/x-python","patch_set":40,"id":"3f79a3b5_9f59c92a","line":70,"range":{"start_line":69,"start_character":0,"end_line":70,"end_character":35},"updated":"2018-11-01 16:02:12.000000000","message":"I have a question about the cache here, what if a neutron-server is added to the cluster which never get the notification ever, so the following code will work fine too?","commit_id":"e4007453c88662fa647cacb6d9790afa8833a7de"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b38f0857556410bbf503d88d26f8851fc7d25d2","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    necessary."},{"line_number":67,"context_line":"    \"\"\""},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    related_dvr_router_hosts \u003d {}"},{"line_number":70,"context_line":"    related_dvr_router_routers \u003d {}"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @property"},{"line_number":73,"context_line":"    def l3plugin(self):"}],"source_content_type":"text/x-python","patch_set":40,"id":"3f79a3b5_b1a7190b","line":70,"range":{"start_line":69,"start_character":0,"end_line":70,"end_character":35},"in_reply_to":"3f79a3b5_9f59c92a","updated":"2018-11-01 17:01:56.000000000","message":"It will, this \"cache\" is filled only in _cache_related_dvr_routers_info_before_interface_removal which is called before router interface deletion (L524) and then cleared in \"_cache_related_dvr_routers_info_before_interface_removal\". \nSo it is such \"one shot cache\" only :)","commit_id":"e4007453c88662fa647cacb6d9790afa8833a7de"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a9d54ac1fc96dbc4f90a31d4672658669bee4332","unresolved":false,"context_lines":[{"line_number":531,"context_line":"            return"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        cache_key \u003d (router_id, subnet_id)"},{"line_number":534,"context_line":"        if router_id in self.related_dvr_router_hosts:"},{"line_number":535,"context_line":"            self.related_dvr_router_hosts[cache_key] |\u003d set("},{"line_number":536,"context_line":"                self._get_other_dvr_hosts(context, router_id))"},{"line_number":537,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":40,"id":"3f79a3b5_043490f9","line":534,"range":{"start_line":534,"start_character":11,"end_line":534,"end_character":20},"updated":"2018-11-01 15:57:35.000000000","message":"is this correct given cache_key is the index?","commit_id":"e4007453c88662fa647cacb6d9790afa8833a7de"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b38f0857556410bbf503d88d26f8851fc7d25d2","unresolved":false,"context_lines":[{"line_number":531,"context_line":"            return"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        cache_key \u003d (router_id, subnet_id)"},{"line_number":534,"context_line":"        if router_id in self.related_dvr_router_hosts:"},{"line_number":535,"context_line":"            self.related_dvr_router_hosts[cache_key] |\u003d set("},{"line_number":536,"context_line":"                self._get_other_dvr_hosts(context, router_id))"},{"line_number":537,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":40,"id":"3f79a3b5_11f12d01","line":534,"range":{"start_line":534,"start_character":11,"end_line":534,"end_character":20},"in_reply_to":"3f79a3b5_043490f9","updated":"2018-11-01 17:01:56.000000000","message":"changed","commit_id":"e4007453c88662fa647cacb6d9790afa8833a7de"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a9d54ac1fc96dbc4f90a31d4672658669bee4332","unresolved":false,"context_lines":[{"line_number":536,"context_line":"                self._get_other_dvr_hosts(context, router_id))"},{"line_number":537,"context_line":"        else:"},{"line_number":538,"context_line":"            self.related_dvr_router_hosts[cache_key] \u003d set("},{"line_number":539,"context_line":"                self._get_other_dvr_hosts(context, router_id))"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"        if router_id in self.related_dvr_router_routers:"},{"line_number":542,"context_line":"            self.related_dvr_router_routers[cache_key] |\u003d set("}],"source_content_type":"text/x-python","patch_set":40,"id":"3f79a3b5_a476fcd4","line":539,"updated":"2018-11-01 15:57:35.000000000","message":"Just a comment for a possible follow-on, as I might be completely wrong here.  Both the if and else legs are very similar here, does something like this work or has another set of issues?\n\ntry:\n    existing \u003d self.related_dvr_router_hosts[cache_key]\nexcept KeyError:\n    existing \u003d set()\n\nother \u003d self._get_other_dvr_hosts(context, router_id)\nself.related_dvr_router_hosts[cache_key] \u003d existing | other\n\nMaybe it\u0027s not less code, but close.","commit_id":"e4007453c88662fa647cacb6d9790afa8833a7de"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"0b38f0857556410bbf503d88d26f8851fc7d25d2","unresolved":false,"context_lines":[{"line_number":536,"context_line":"                self._get_other_dvr_hosts(context, router_id))"},{"line_number":537,"context_line":"        else:"},{"line_number":538,"context_line":"            self.related_dvr_router_hosts[cache_key] \u003d set("},{"line_number":539,"context_line":"                self._get_other_dvr_hosts(context, router_id))"},{"line_number":540,"context_line":""},{"line_number":541,"context_line":"        if router_id in self.related_dvr_router_routers:"},{"line_number":542,"context_line":"            self.related_dvr_router_routers[cache_key] |\u003d set("}],"source_content_type":"text/x-python","patch_set":40,"id":"3f79a3b5_317d4970","line":539,"in_reply_to":"3f79a3b5_a476fcd4","updated":"2018-11-01 17:01:56.000000000","message":"yes, that may be more readable solution. I changed it. Thx.","commit_id":"e4007453c88662fa647cacb6d9790afa8833a7de"}],"neutron/db/l3_dvrscheduler_db.py":[{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"efa11ab15e992f09a23ed9f15791f2c2f0ff6bb8","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from neutron.common import utils as n_utils"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"from neutron.db import agentschedulers_db"},{"line_number":31,"context_line":"from neutron.db import l3_db"},{"line_number":32,"context_line":"from neutron.db import l3_agentschedulers_db as l3agent_sch_db"},{"line_number":33,"context_line":"from neutron.db import l3_dvr_db"},{"line_number":34,"context_line":"from neutron.db import models_v2"}],"source_content_type":"text/x-python","patch_set":1,"id":"3f79a3b5_4cc3a339","line":31,"updated":"2018-08-29 19:16:03.000000000","message":"neutron.db.models import l3 as l3_db","commit_id":"c89437cefd93ce6acb3fc9a1a7ebe2950b6742de"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"f5abbee429101f788b8b3912640919dbb8d71a62","unresolved":false,"context_lines":[{"line_number":359,"context_line":"                                    list(subnet_ids))):"},{"line_number":360,"context_line":"                        result_set.add(router_id)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"            for dvr_router in dvr_routers:"},{"line_number":363,"context_line":"                result_set |\u003d set("},{"line_number":364,"context_line":"                    self._get_other_dvr_router_ids_connected_router("},{"line_number":365,"context_line":"                        context, dvr_router))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_3c075920","line":362,"updated":"2018-08-30 17:48:13.000000000","message":"This fix did not solve the problem.\nAlso it introduces additional problem, where the same router interfaces were created on both the compute nodes, irrespective of the VM ports attached subnet.","commit_id":"04f457b5a33ccae4d1290d8d7c39df2ef80fd588"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b1ceb680654e3734a4cea6655a6fa145154b7959","unresolved":false,"context_lines":[{"line_number":359,"context_line":"                                    list(subnet_ids))):"},{"line_number":360,"context_line":"                        result_set.add(router_id)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"            for dvr_router in dvr_routers:"},{"line_number":363,"context_line":"                result_set |\u003d set("},{"line_number":364,"context_line":"                    self._get_other_dvr_router_ids_connected_router("},{"line_number":365,"context_line":"                        context, dvr_router))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d1415ca8","line":362,"in_reply_to":"3f79a3b5_3c075920","updated":"2018-08-31 10:25:34.000000000","message":"I did simple test on multinode devstack with dvr.\nI used code from current master branch and I did exactly what is described in bug description. VMs doesn\u0027t ping between each other. Then I downloaded this patch to neutron server, restarted it and I also restarted neutron-l3 agents on both compute nodes. Then vms starts pinging properly.\n\nI\u0027m not an dvr expert here so I\u0027m probably wrong but what is the difference between the case if we would spawn also 3rd and 4th vm, connected to same networks (AP1Site and AP2Site from bug description) and put them on opposite hosts than vm AP1 and AP2? Then both routers will be created on both hosts and then this communication would also work properly. So if we just do it in case when dvr router has connection to other dvr router (not only in case when dvr router has connected vms on host) then it should be fine IMO.\nWhat I am missing here?","commit_id":"04f457b5a33ccae4d1290d8d7c39df2ef80fd588"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"8a80a02cf101b898f1605095caa83294fa723d91","unresolved":false,"context_lines":[{"line_number":359,"context_line":"                                    list(subnet_ids))):"},{"line_number":360,"context_line":"                        result_set.add(router_id)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"            for dvr_router in dvr_routers:"},{"line_number":363,"context_line":"                result_set |\u003d set("},{"line_number":364,"context_line":"                    self._get_other_dvr_router_ids_connected_router("},{"line_number":365,"context_line":"                        context, dvr_router))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_64a0bd43","line":362,"in_reply_to":"3f79a3b5_d1415ca8","updated":"2018-08-31 16:53:31.000000000","message":"Ok, let me recheck this patch again.","commit_id":"04f457b5a33ccae4d1290d8d7c39df2ef80fd588"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"35c72546d7ec69b33789d3011dc2f395796dcc9d","unresolved":false,"context_lines":[{"line_number":359,"context_line":"                                    list(subnet_ids))):"},{"line_number":360,"context_line":"                        result_set.add(router_id)"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"            for dvr_router in dvr_routers:"},{"line_number":363,"context_line":"                result_set |\u003d set("},{"line_number":364,"context_line":"                    self._get_other_dvr_router_ids_connected_router("},{"line_number":365,"context_line":"                        context, dvr_router))"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_5a01d869","line":362,"in_reply_to":"3f79a3b5_d1415ca8","updated":"2018-08-31 18:07:32.000000000","message":"This is what I am seeing when I create a VM on compute1/net1  and VM on compute2/net2.\n\nLog from compute1:\n\nstack@ubuntu-18-compute1:~/devstack$ sudo ip netns exec qrouter-901987b4-16e8-45f0-93b3-f77b5fc48ded bash\nroot@ubuntu-18-compute1:~/devstack# ifconfig\nlo: flags\u003d73\u003cUP,LOOPBACK,RUNNING\u003e  mtu 65536\n        inet 127.0.0.1  netmask 255.0.0.0\n        inet6 ::1  prefixlen 128  scopeid 0x10\u003chost\u003e\n        loop  txqueuelen 1000  (Local Loopback)\n        RX packets 0  bytes 0 (0.0 B)\n        RX errors 0  dropped 0  overruns 0  frame 0\n        TX packets 0  bytes 0 (0.0 B)\n        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0\n\nqr-0e070523-99: flags\u003d4163\u003cUP,BROADCAST,RUNNING,MULTICAST\u003e  mtu 1450\n        inet 10.2.0.1  netmask 255.255.255.0  broadcast 10.2.0.255\n        inet6 fe80::f816:3eff:fe5b:fd6f  prefixlen 64  scopeid 0x20\u003clink\u003e\n        ether fa:16:3e:5b:fd:6f  txqueuelen 1000  (Ethernet)\n        RX packets 17  bytes 1640 (1.6 KB)\n        RX errors 0  dropped 0  overruns 0  frame 0\n        TX packets 13  bytes 998 (998.0 B)\n        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0\n\nqr-adbf1e69-5b: flags\u003d4163\u003cUP,BROADCAST,RUNNING,MULTICAST\u003e  mtu 1450\n        inet 20.0.0.30  netmask 255.255.255.0  broadcast 20.0.0.255\n        inet6 fe80::f816:3eff:fea5:3f99  prefixlen 64  scopeid 0x20\u003clink\u003e\n        ether fa:16:3e:a5:3f:99  txqueuelen 1000  (Ethernet)\n        RX packets 7  bytes 512 (512.0 B)\n        RX errors 0  dropped 0  overruns 0  frame 0\n        TX packets 13  bytes 998 (998.0 B)\n        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0\n\nroot@ubuntu-18-compute1:~/devstack# exit\nexit\nstack@ubuntu-18-compute1:~/devstack$ sudo ip netns\nqrouter-901987b4-16e8-45f0-93b3-f77b5fc48ded (id: 0)\nstack@ubuntu-18-compute1:~/devstack$ sudo virsh console 1\nConnected to domain instance-00000001\nEscape character is ^]\n\nlogin as \u0027cirros\u0027 user. default password: \u0027cubswin:)\u0027. use \u0027sudo\u0027 for root.\ncirros login: cirros\nPassword: \n$ ifconfig eth0\neth0      Link encap:Ethernet  HWaddr FA:16:3E:D7:71:DD  \n          inet addr:10.1.0.6  Bcast:10.1.0.255  Mask:255.255.255.0\n          inet6 addr: fe80::f816:3eff:fed7:71dd/64 Scope:Link\n          UP BROADCAST RUNNING MULTICAST  MTU:1450  Metric:1\n          RX packets:19 errors:0 dropped:0 overruns:0 frame:0\n          TX packets:89 errors:0 dropped:0 overruns:0 carrier:0\n          collisions:0 txqueuelen:1000 \n          RX bytes:2025 (1.9 KiB)  TX bytes:4998 (4.8 KiB)\n\n$ \n\nWhat I am seeing here is even though my VM belongs to 10.1.0.6, the router namespace has interfaces pointing to 10.2.0.0 subnet.\n\nstack@ubuntu-18-ctlr-rocky:~/devstack$ neutron router-port-list test-router1\nneutron CLI is deprecated and will be removed in the future. Use openstack CLI instead.\n+--------------------------------------+------------------+----------------------------------+-------------------+----------------------------------------------------------------------------------+\n| id                                   | name             | tenant_id                        | mac_address       | fixed_ips                                                                        |\n+--------------------------------------+------------------+----------------------------------+-------------------+----------------------------------------------------------------------------------+\n| 06fc39a3-c45c-4419-9bcf-dd2f135e7df5 |                  | 6982578848b644b4ba98edb28be91940 | fa:16:3e:40:1f:8e | {\"subnet_id\": \"0ddbba89-07f4-4273-9c33-dcb4891f8462\", \"ip_address\": \"10.1.0.1\"}  |\n| 87558daa-908f-4c1c-81ce-7611983c0854 | wan-port-router1 | 6982578848b644b4ba98edb28be91940 | fa:16:3e:84:67:af | {\"subnet_id\": \"46d5fd18-a4eb-4211-957e-62c4f6ae67a9\", \"ip_address\": \"20.0.0.20\"} |\n+--------------------------------------+------------------+----------------------------------+-------------------+----------------------------------------------------------------------------------+\nstack@ubuntu-18-ctlr-rocky:~/devstack$ neutron router-port-list test-router2\nneutron CLI is deprecated and will be removed in the future. Use openstack CLI instead.\n+--------------------------------------+------------------+----------------------------------+-------------------+----------------------------------------------------------------------------------+\n| id                                   | name             | tenant_id                        | mac_address       | fixed_ips                                                                        |\n+--------------------------------------+------------------+----------------------------------+-------------------+----------------------------------------------------------------------------------+\n| 0e070523-995c-4392-ade7-db3de5d205cb |                  | 6982578848b644b4ba98edb28be91940 | fa:16:3e:5b:fd:6f | {\"subnet_id\": \"6461db54-f0a2-4ef9-bf1a-925d17b29893\", \"ip_address\": \"10.2.0.1\"}  |\n| adbf1e69-5ba8-4444-9fa0-44dd9d6454b7 | wan-port-router2 | 6982578848b644b4ba98edb28be91940 | fa:16:3e:a5:3f:99 | {\"subnet_id\": \"46d5fd18-a4eb-4211-957e-62c4f6ae67a9\", \"ip_address\": \"20.0.0.30\"} |\n+--------------------------------------+------------------+----------------------------------+-------------------+----------------------------------------------------------------------------------+","commit_id":"04f457b5a33ccae4d1290d8d7c39df2ef80fd588"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"0eb44e2f248cd782b25e03a871f0512f179ea335","unresolved":false,"context_lines":[{"line_number":83,"context_line":"    \"\"\""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    def dvr_handle_new_service_port(self, context, port,"},{"line_number":86,"context_line":"                                    dest_host\u003dNone, unbound_migrate\u003dFalse):"},{"line_number":87,"context_line":"        \"\"\"Handle new dvr service port creation."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"        When a new dvr service port is created, this function will"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_2119e278","line":86,"updated":"2018-09-06 22:40:49.000000000","message":"The logic to inform agents when a VM pops up on a routed subnet is in here.\nSo I think we should also capture the \u0027connected-routers\u0027 in here and its host and then send a notification to the agents.\n\nThis uses a different notification rpc api.","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"05eb7b7dbc7713613d54fe18a023302bf4c7641a","unresolved":false,"context_lines":[{"line_number":336,"context_line":"        router_subnets_query \u003d router_subnets_query.join(RouterPort.port)"},{"line_number":337,"context_line":"        router_subnets_query \u003d router_subnets_query.filter("},{"line_number":338,"context_line":"            RouterPort.port_type \u003d\u003d n_const.DEVICE_OWNER_DVR_INTERFACE)"},{"line_number":339,"context_line":"        router_subnets_query \u003d router_subnets_query.filter("},{"line_number":340,"context_line":"            RouterPort.router_id \u003d\u003d router_id)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        routers_query \u003d context.session.query("},{"line_number":343,"context_line":"            RouterPort.router_id).distinct()"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_e1068a86","line":340,"range":{"start_line":339,"start_character":7,"end_line":340,"end_character":46},"updated":"2018-09-06 23:24:42.000000000","message":"This query is returning all the subnet_ids and probably not filtering based on the router_id that we pass in.","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"0eb44e2f248cd782b25e03a871f0512f179ea335","unresolved":false,"context_lines":[{"line_number":341,"context_line":""},{"line_number":342,"context_line":"        routers_query \u003d context.session.query("},{"line_number":343,"context_line":"            RouterPort.router_id).distinct()"},{"line_number":344,"context_line":"        routers_query \u003d routers_query.join(models_v2.IPAllocation.port)"},{"line_number":345,"context_line":"        routers_query \u003d routers_query.filter("},{"line_number":346,"context_line":"            RouterPort.router_id !\u003d router_id)"},{"line_number":347,"context_line":"        routers_query \u003d routers_query.filter("}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_1e2be394","line":344,"updated":"2018-09-06 22:40:49.000000000","message":"Do we also need to filter here with \u0027RouterPort.port_type\u0027 as above in the router_subnets_query.","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"a1b13adb0b97c8f6754bb699f1f086de206794a6","unresolved":false,"context_lines":[{"line_number":346,"context_line":"            RouterPort.router_id !\u003d router_id)"},{"line_number":347,"context_line":"        routers_query \u003d routers_query.filter("},{"line_number":348,"context_line":"            models_v2.IPAllocation.subnet_id.in_(router_subnets_query))"},{"line_number":349,"context_line":"        return [item[0] for item in routers_query]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _get_router_ids_for_agent(self, context, agent_db, router_ids):"},{"line_number":352,"context_line":"        result_set \u003d set(super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_384d9699","line":349,"updated":"2018-09-06 21:26:14.000000000","message":"somewhere in those sql queries there is an issue and it returns me all routers instead of only those which are connected to same subnets as \"router_id\"","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"0eb44e2f248cd782b25e03a871f0512f179ea335","unresolved":false,"context_lines":[{"line_number":346,"context_line":"            RouterPort.router_id !\u003d router_id)"},{"line_number":347,"context_line":"        routers_query \u003d routers_query.filter("},{"line_number":348,"context_line":"            models_v2.IPAllocation.subnet_id.in_(router_subnets_query))"},{"line_number":349,"context_line":"        return [item[0] for item in routers_query]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _get_router_ids_for_agent(self, context, agent_db, router_ids):"},{"line_number":352,"context_line":"        result_set \u003d set(super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_3e51ff30","line":349,"in_reply_to":"3f79a3b5_384d9699","updated":"2018-09-06 22:40:49.000000000","message":"Yes this query is the one that returns the additional router.","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"7c5a98f08b886154379ba1f3869240afa4fff5fb","unresolved":false,"context_lines":[{"line_number":346,"context_line":"            RouterPort.router_id !\u003d router_id)"},{"line_number":347,"context_line":"        routers_query \u003d routers_query.filter("},{"line_number":348,"context_line":"            models_v2.IPAllocation.subnet_id.in_(router_subnets_query))"},{"line_number":349,"context_line":"        return [item[0] for item in routers_query]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _get_router_ids_for_agent(self, context, agent_db, router_ids):"},{"line_number":352,"context_line":"        result_set \u003d set(super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_787227e5","line":349,"in_reply_to":"3f79a3b5_3e51ff30","updated":"2018-09-07 06:10:55.000000000","message":"Probably we can use a query similar to this shown below\nhttps://github.com/openstack/neutron/blob/a388701ddfe628e9a5bd16a78422164799b11ef8/neutron/objects/router.py#L147\nWhere we can filter based on \u0027subnet-id\u0027 instead of subnet pool.\nThis will give the connected router-ids for the common subnet.","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"db17d4458f9bf7a11247937c2574cf219ede3382","unresolved":false,"context_lines":[{"line_number":346,"context_line":"            RouterPort.router_id !\u003d router_id)"},{"line_number":347,"context_line":"        routers_query \u003d routers_query.filter("},{"line_number":348,"context_line":"            models_v2.IPAllocation.subnet_id.in_(router_subnets_query))"},{"line_number":349,"context_line":"        return [item[0] for item in routers_query]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _get_router_ids_for_agent(self, context, agent_db, router_ids):"},{"line_number":352,"context_line":"        result_set \u003d set(super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_9fffdb64","line":349,"in_reply_to":"3f79a3b5_787227e5","updated":"2018-09-07 19:06:35.000000000","message":"This query seems to fix the problem, where it only returns the routers that are interconnected.\n    def _get_router_ids_by_subnet(self, context, router_id):\n        subnet_ids \u003d self.get_subnet_ids_on_router(context, router_id)\n        RouterPort \u003d l3_models.RouterPort\n        query \u003d context.session.query(RouterPort.router_id)\n        query \u003d query.join(models_v2.Port)\n        query \u003d query.join(\n            models_v2.Subnet,\n            models_v2.Subnet.network_id \u003d\u003d models_v2.Port.network_id)\n        query \u003d query.filter(\n            models_v2.Subnet.id.in_(subnet_ids),\n            RouterPort.port_type \u003d\u003d n_const.DEVICE_OWNER_DVR_INTERFACE).distinct()\n        return [item[0] for item in query]","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":346,"context_line":"            RouterPort.router_id !\u003d router_id)"},{"line_number":347,"context_line":"        routers_query \u003d routers_query.filter("},{"line_number":348,"context_line":"            models_v2.IPAllocation.subnet_id.in_(router_subnets_query))"},{"line_number":349,"context_line":"        return [item[0] for item in routers_query]"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"    def _get_router_ids_for_agent(self, context, agent_db, router_ids):"},{"line_number":352,"context_line":"        result_set \u003d set(super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_af15a29d","line":349,"in_reply_to":"3f79a3b5_9fffdb64","updated":"2018-09-18 20:42:36.000000000","message":"yes, thx a lot. It looks that this works fine","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"0eb44e2f248cd782b25e03a871f0512f179ea335","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                    l3plugin, context, port, address_pair)"},{"line_number":470,"context_line":"    l3plugin.delete_arp_entry_for_dvr_service_port(context, port)"},{"line_number":471,"context_line":"    removed_routers \u003d l3plugin.get_dvr_routers_to_remove(context, port)"},{"line_number":472,"context_line":"    for info in removed_routers:"},{"line_number":473,"context_line":"        l3plugin.l3_rpc_notifier.router_removed_from_agent("},{"line_number":474,"context_line":"            context, info[\u0027router_id\u0027], info[\u0027host\u0027])"},{"line_number":475,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3f79a3b5_e128ca86","line":472,"updated":"2018-09-06 22:40:49.000000000","message":"Also here when the VM is deleted, we should probably clean up the routers that we created based on the \u0027connected subnets\u0027","commit_id":"3bb74af55ab554b71d4b8c5dd79e36e78d4238ed"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cab27f84965562b5c0678d4c594feb113e75b88f","unresolved":false,"context_lines":[{"line_number":136,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":137,"context_line":"            for router_id in router_ids:"},{"line_number":138,"context_line":"                for host in hosts:"},{"line_number":139,"context_line":"                    LOG.debug(\u0027DVR: Handle new seuvice port, host %(host)s, \u0027"},{"line_number":140,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":141,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":142,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_ab93cba2","line":139,"range":{"start_line":139,"start_character":47,"end_line":139,"end_character":54},"updated":"2018-09-17 20:59:39.000000000","message":"s/service","commit_id":"a76ce22b616fbf3dead93ab6eaa40a74ec487cbe"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cab27f84965562b5c0678d4c594feb113e75b88f","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            for router_id in router_ids:"},{"line_number":138,"context_line":"                for host in hosts:"},{"line_number":139,"context_line":"                    LOG.debug(\u0027DVR: Handle new seuvice port, host %(host)s, \u0027"},{"line_number":140,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":141,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":142,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":143,"context_line":"                    context, router_ids, host)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_cbede724","line":140,"range":{"start_line":140,"start_character":38,"end_line":140,"end_character":41},"updated":"2018-09-17 20:59:39.000000000","message":"s/id","commit_id":"a76ce22b616fbf3dead93ab6eaa40a74ec487cbe"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cab27f84965562b5c0678d4c594feb113e75b88f","unresolved":false,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def get_hosts_to_notify(self, context, router_id):"},{"line_number":255,"context_line":"        \"\"\"Returns all hosts to send notification about router update\"\"\""},{"line_number":256,"context_line":"        context \u003d context.elevated()"},{"line_number":257,"context_line":"        hosts \u003d super(L3_DVRsch_db_mixin, self).get_hosts_to_notify("},{"line_number":258,"context_line":"            context, router_id)"},{"line_number":259,"context_line":"        router \u003d self.get_router(context, router_id)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_bdceae80","line":256,"updated":"2018-09-17 20:59:39.000000000","message":"Do you want to do this here and/or use the same variable name?","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":253,"context_line":""},{"line_number":254,"context_line":"    def get_hosts_to_notify(self, context, router_id):"},{"line_number":255,"context_line":"        \"\"\"Returns all hosts to send notification about router update\"\"\""},{"line_number":256,"context_line":"        context \u003d context.elevated()"},{"line_number":257,"context_line":"        hosts \u003d super(L3_DVRsch_db_mixin, self).get_hosts_to_notify("},{"line_number":258,"context_line":"            context, router_id)"},{"line_number":259,"context_line":"        router \u003d self.get_router(context, router_id)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_5c13d165","line":256,"in_reply_to":"3f79a3b5_bdceae80","updated":"2018-09-18 20:42:36.000000000","message":"I need elevated context only in get_router() call so I changed it.","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cab27f84965562b5c0678d4c594feb113e75b88f","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        dvr_hosts \u003d set()"},{"line_number":290,"context_line":"        connected_dvr_routers \u003d ("},{"line_number":291,"context_line":"            self._get_other_dvr_router_ids_connected_router("},{"line_number":292,"context_line":"                context, router_id))"},{"line_number":293,"context_line":"        for dvr_router in connected_dvr_routers:"},{"line_number":294,"context_line":"            dvr_hosts |\u003d set("},{"line_number":295,"context_line":"                self._get_dvr_hosts_for_router(context, dvr_router))"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_dd792a2f","line":292,"range":{"start_line":292,"start_character":16,"end_line":292,"end_character":23},"updated":"2018-09-17 20:59:39.000000000","message":"This is the only place you need elevated I think?  Perhaps just use a new local:\n\ns/context/context.elevated()","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        dvr_hosts \u003d set()"},{"line_number":290,"context_line":"        connected_dvr_routers \u003d ("},{"line_number":291,"context_line":"            self._get_other_dvr_router_ids_connected_router("},{"line_number":292,"context_line":"                context, router_id))"},{"line_number":293,"context_line":"        for dvr_router in connected_dvr_routers:"},{"line_number":294,"context_line":"            dvr_hosts |\u003d set("},{"line_number":295,"context_line":"                self._get_dvr_hosts_for_router(context, dvr_router))"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_9750620c","line":292,"range":{"start_line":292,"start_character":16,"end_line":292,"end_character":23},"in_reply_to":"3f79a3b5_dd792a2f","updated":"2018-09-18 20:42:36.000000000","message":"I will pass \"normal\" context and elevate where it is necessary","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cab27f84965562b5c0678d4c594feb113e75b88f","unresolved":false,"context_lines":[{"line_number":294,"context_line":"            dvr_hosts |\u003d set("},{"line_number":295,"context_line":"                self._get_dvr_hosts_for_router(context, dvr_router))"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        LOG.debug(\u0027Hosts for other DVR routers connected to router %s: %s\u0027,"},{"line_number":298,"context_line":"                  router_id, dvr_hosts)"},{"line_number":299,"context_line":"        return dvr_hosts"},{"line_number":300,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_3dd29e35","line":297,"updated":"2018-09-17 20:59:39.000000000","message":"nit: other places use a dict for arguments","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":294,"context_line":"            dvr_hosts |\u003d set("},{"line_number":295,"context_line":"                self._get_dvr_hosts_for_router(context, dvr_router))"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        LOG.debug(\u0027Hosts for other DVR routers connected to router %s: %s\u0027,"},{"line_number":298,"context_line":"                  router_id, dvr_hosts)"},{"line_number":299,"context_line":"        return dvr_hosts"},{"line_number":300,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_57088a25","line":297,"in_reply_to":"3f79a3b5_3dd29e35","updated":"2018-09-18 20:42:36.000000000","message":"Done","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cab27f84965562b5c0678d4c594feb113e75b88f","unresolved":false,"context_lines":[{"line_number":390,"context_line":"            for dvr_router in dvr_routers:"},{"line_number":391,"context_line":"                result_set |\u003d set("},{"line_number":392,"context_line":"                    self._get_other_dvr_router_ids_connected_router("},{"line_number":393,"context_line":"                        context, dvr_router))"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"        return list(result_set)"},{"line_number":396,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_9da412a5","line":393,"range":{"start_line":393,"start_character":24,"end_line":393,"end_character":31},"updated":"2018-09-17 20:59:39.000000000","message":"this is already an elevated context?","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"148a7758712b676bcfe5b6a22bd494eb1a54f4af","unresolved":false,"context_lines":[{"line_number":346,"context_line":"    def _get_other_dvr_router_ids_connected_router(self, context, router_id):"},{"line_number":347,"context_line":"        subnet_ids \u003d self.get_subnet_ids_on_router(context, router_id)"},{"line_number":348,"context_line":"        RouterPort \u003d l3_models.RouterPort"},{"line_number":349,"context_line":"        query \u003d context.elevated().session.query(RouterPort.router_id)"},{"line_number":350,"context_line":"        query \u003d query.join(models_v2.Port)"},{"line_number":351,"context_line":"        query \u003d query.join("},{"line_number":352,"context_line":"            models_v2.Subnet,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_80b25095","line":349,"updated":"2018-09-18 21:05:20.000000000","message":"This method and it\u0027s query construction should be in the RouterPort object: https://git.openstack.org/cgit/openstack/neutron/tree/neutron/objects/router.py#n130","commit_id":"deeef73b390ef466447c77e0cc6ac8b01cf025c4"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"636a9c5e86ae3c71d72446b23fe61597aa5577d0","unresolved":false,"context_lines":[{"line_number":346,"context_line":"    def _get_other_dvr_router_ids_connected_router(self, context, router_id):"},{"line_number":347,"context_line":"        subnet_ids \u003d self.get_subnet_ids_on_router(context, router_id)"},{"line_number":348,"context_line":"        RouterPort \u003d l3_models.RouterPort"},{"line_number":349,"context_line":"        query \u003d context.elevated().session.query(RouterPort.router_id)"},{"line_number":350,"context_line":"        query \u003d query.join(models_v2.Port)"},{"line_number":351,"context_line":"        query \u003d query.join("},{"line_number":352,"context_line":"            models_v2.Subnet,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_463be599","line":349,"in_reply_to":"3f79a3b5_80b25095","updated":"2018-09-25 21:20:24.000000000","message":"I agree that it should be moved to OVO class but I\u0027m not sure if it should be RouterPort or maybe Router class.\nAlso it would require to move get_subnet_ids_on_router() to OVO also or it would require to pass subnet_ids to this new method as argument.\nI\u0027m not sure if that should be done in this patch","commit_id":"deeef73b390ef466447c77e0cc6ac8b01cf025c4"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"148a7758712b676bcfe5b6a22bd494eb1a54f4af","unresolved":false,"context_lines":[{"line_number":355,"context_line":"            models_v2.Subnet.id.in_(subnet_ids),"},{"line_number":356,"context_line":"            RouterPort.port_type \u003d\u003d n_const.DEVICE_OWNER_DVR_INTERFACE"},{"line_number":357,"context_line":"                            ).distinct()"},{"line_number":358,"context_line":"        return [item[0] for item in query if item[0] !\u003d router_id]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def _get_router_ids_for_agent(self, context, agent_db, router_ids):"},{"line_number":361,"context_line":"        result_set \u003d set(super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_a06c2c07","line":358,"range":{"start_line":358,"start_character":42,"end_line":358,"end_character":66},"updated":"2018-09-18 21:05:20.000000000","message":"Couldn\u0027t this conditional be expressed in the SQL?  It seems that might be a touch more efficient.","commit_id":"deeef73b390ef466447c77e0cc6ac8b01cf025c4"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"636a9c5e86ae3c71d72446b23fe61597aa5577d0","unresolved":false,"context_lines":[{"line_number":355,"context_line":"            models_v2.Subnet.id.in_(subnet_ids),"},{"line_number":356,"context_line":"            RouterPort.port_type \u003d\u003d n_const.DEVICE_OWNER_DVR_INTERFACE"},{"line_number":357,"context_line":"                            ).distinct()"},{"line_number":358,"context_line":"        return [item[0] for item in query if item[0] !\u003d router_id]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def _get_router_ids_for_agent(self, context, agent_db, router_ids):"},{"line_number":361,"context_line":"        result_set \u003d set(super(L3_DVRsch_db_mixin,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_06dbad7c","line":358,"range":{"start_line":358,"start_character":42,"end_line":358,"end_character":66},"in_reply_to":"3f79a3b5_a06c2c07","updated":"2018-09-25 21:20:24.000000000","message":"Done","commit_id":"deeef73b390ef466447c77e0cc6ac8b01cf025c4"},{"author":{"_account_id":13995,"name":"Nate Johnston","email":"nate.johnston@redhat.com","username":"natejohnston"},"change_message_id":"148a7758712b676bcfe5b6a22bd494eb1a54f4af","unresolved":false,"context_lines":[{"line_number":500,"context_line":"            removed_routers \u003d l3plugin.get_dvr_routers_to_remove("},{"line_number":501,"context_line":"                context,"},{"line_number":502,"context_line":"                original_port,"},{"line_number":503,"context_line":"                get_related_hosts_info\u003dFalse)"},{"line_number":504,"context_line":"            if removed_routers:"},{"line_number":505,"context_line":"                removed_router_args \u003d {"},{"line_number":506,"context_line":"                    \u0027context\u0027: context,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_20ffbc02","line":503,"range":{"start_line":503,"start_character":16,"end_line":503,"end_character":44},"updated":"2018-09-18 21:05:20.000000000","message":"Under what circumstances would get_related_hosts_info be True?  It\u0027s not clear to me in this change where the other places it is getting called are, so it wouldn\u0027t have this False setting.","commit_id":"deeef73b390ef466447c77e0cc6ac8b01cf025c4"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"636a9c5e86ae3c71d72446b23fe61597aa5577d0","unresolved":false,"context_lines":[{"line_number":500,"context_line":"            removed_routers \u003d l3plugin.get_dvr_routers_to_remove("},{"line_number":501,"context_line":"                context,"},{"line_number":502,"context_line":"                original_port,"},{"line_number":503,"context_line":"                get_related_hosts_info\u003dFalse)"},{"line_number":504,"context_line":"            if removed_routers:"},{"line_number":505,"context_line":"                removed_router_args \u003d {"},{"line_number":506,"context_line":"                    \u0027context\u0027: context,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f79a3b5_c64e1538","line":503,"range":{"start_line":503,"start_character":16,"end_line":503,"end_character":44},"in_reply_to":"3f79a3b5_20ffbc02","updated":"2018-09-25 21:20:24.000000000","message":"In case when port will be removed not updated, then it will be called with True. It will go through _notify_port_delete() function above directly.","commit_id":"deeef73b390ef466447c77e0cc6ac8b01cf025c4"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"1df3118004e8cdf24361aa998b316bb2216bf646","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        if not agent_port_host_match:"},{"line_number":134,"context_line":"            hosts \u003d set([port_host])"},{"line_number":135,"context_line":"            for router_id in router_ids:"},{"line_number":136,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"            for host in hosts:"},{"line_number":139,"context_line":"                LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_46099e92","line":136,"updated":"2018-09-19 15:47:22.000000000","message":"Looking at [1], it seems efficient if we call get_l3_agents_hosting_routers() which takes a list of routers.\nHowever, we have some extra logic in [1], so I am not sure which is better.\n\n[1] https://github.com/openstack/neutron/blob/ef3344767a1367cf576344c798b7aa4e417962a7/neutron/db/l3_agentschedulers_db.py#L491-L496","commit_id":"ce1f1154d616cce3e933775cd9708e65737aeada"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"636a9c5e86ae3c71d72446b23fe61597aa5577d0","unresolved":false,"context_lines":[{"line_number":133,"context_line":"        if not agent_port_host_match:"},{"line_number":134,"context_line":"            hosts \u003d set([port_host])"},{"line_number":135,"context_line":"            for router_id in router_ids:"},{"line_number":136,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"            for host in hosts:"},{"line_number":139,"context_line":"                LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"3f79a3b5_59e72a10","line":136,"in_reply_to":"3f79a3b5_46099e92","updated":"2018-09-25 21:20:24.000000000","message":"I\u0027m not sure if get_l3_agents_hosting_routers() will return dvr L3 agents which are on compute nodes. I think such agents are not listed as \"agents hosting router\".","commit_id":"ce1f1154d616cce3e933775cd9708e65737aeada"},{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"3384a103a5aee2ae04c3d32c828436b9980e5ce0","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            for router_id in router_ids:"},{"line_number":136,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"            for host in hosts:"},{"line_number":139,"context_line":"                updated_routers \u003d set()"},{"line_number":140,"context_line":"                for router_id in router_ids:"},{"line_number":141,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_979f8901","line":138,"range":{"start_line":138,"start_character":12,"end_line":138,"end_character":30},"updated":"2018-09-24 20:49:01.000000000","message":"how about nesting this for loop under one at L134 so, L140 won\u0027t be needed ?","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"b88a7b22941b9b271755a5faec5f2314d83afefe","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":143,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":144,"context_line":"                    if self._check_for_rtr_serviceable_ports("},{"line_number":145,"context_line":"                        context.elevated(), router_id, host):"},{"line_number":146,"context_line":"                        updated_routers.add(router_id)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"                if updated_routers:"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_3cf45cd0","line":145,"range":{"start_line":145,"start_character":24,"end_line":145,"end_character":42},"updated":"2018-09-24 17:42:18.000000000","message":"elevated() does a copy.copy() of itself, so this should be outside the outer loop since it\u0027s only required once.\n\nnit: also, if this could be indented to be different from the code within that would be good, or maybe with a variable for the context this could be on previous line?","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"3384a103a5aee2ae04c3d32c828436b9980e5ce0","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        subnet_ids \u003d [ip[\u0027subnet_id\u0027] for ip in deleted_port[\u0027fixed_ips\u0027]]"},{"line_number":201,"context_line":"        router_ids \u003d self.get_dvr_routers_by_subnet_ids(admin_context,"},{"line_number":202,"context_line":"                                                        subnet_ids)"},{"line_number":203,"context_line":"        related_routers \u003d set()"},{"line_number":204,"context_line":"        for router_id in router_ids:"},{"line_number":205,"context_line":"            connected_dvr_routers \u003d set("},{"line_number":206,"context_line":"                self._get_other_dvr_router_ids_connected_router("}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_f25e0b12","line":203,"range":{"start_line":203,"start_character":8,"end_line":203,"end_character":31},"updated":"2018-09-24 20:49:01.000000000","message":"for simple code, I guess this can be removed and connected_dvr_routers can used at L236 ? and |\u003d with empty set() is same as \u003d","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bcf5682ffefe030f39730a23b8495bbe6c0aa1eb","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        subnet_ids \u003d [ip[\u0027subnet_id\u0027] for ip in deleted_port[\u0027fixed_ips\u0027]]"},{"line_number":201,"context_line":"        router_ids \u003d self.get_dvr_routers_by_subnet_ids(admin_context,"},{"line_number":202,"context_line":"                                                        subnet_ids)"},{"line_number":203,"context_line":"        related_routers \u003d set()"},{"line_number":204,"context_line":"        for router_id in router_ids:"},{"line_number":205,"context_line":"            connected_dvr_routers \u003d set("},{"line_number":206,"context_line":"                self._get_other_dvr_router_ids_connected_router("}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_3b50dfd5","line":203,"range":{"start_line":203,"start_character":8,"end_line":203,"end_character":31},"in_reply_to":"3f79a3b5_f25e0b12","updated":"2018-09-24 21:03:05.000000000","message":"No, since we need to OR-in possibly more than one connected DVR router.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"a71e7aab6b724ded0207a2051fd7a8ed9473fce8","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        subnet_ids \u003d [ip[\u0027subnet_id\u0027] for ip in deleted_port[\u0027fixed_ips\u0027]]"},{"line_number":201,"context_line":"        router_ids \u003d self.get_dvr_routers_by_subnet_ids(admin_context,"},{"line_number":202,"context_line":"                                                        subnet_ids)"},{"line_number":203,"context_line":"        related_routers \u003d set()"},{"line_number":204,"context_line":"        for router_id in router_ids:"},{"line_number":205,"context_line":"            connected_dvr_routers \u003d set("},{"line_number":206,"context_line":"                self._get_other_dvr_router_ids_connected_router("}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_feeb3578","line":203,"range":{"start_line":203,"start_character":8,"end_line":203,"end_character":31},"in_reply_to":"3f79a3b5_f25e0b12","updated":"2018-09-24 22:35:02.000000000","message":"Thanks for the comment, but it doesn\u0027t work that way.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"1669bd34b821eeb06ca90e3e7e2e5992dc5e50f3","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        subnet_ids \u003d [ip[\u0027subnet_id\u0027] for ip in deleted_port[\u0027fixed_ips\u0027]]"},{"line_number":201,"context_line":"        router_ids \u003d self.get_dvr_routers_by_subnet_ids(admin_context,"},{"line_number":202,"context_line":"                                                        subnet_ids)"},{"line_number":203,"context_line":"        related_routers \u003d set()"},{"line_number":204,"context_line":"        for router_id in router_ids:"},{"line_number":205,"context_line":"            connected_dvr_routers \u003d set("},{"line_number":206,"context_line":"                self._get_other_dvr_router_ids_connected_router("}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_ccad7385","line":203,"range":{"start_line":203,"start_character":8,"end_line":203,"end_character":31},"in_reply_to":"3f79a3b5_feeb3578","updated":"2018-09-25 00:50:16.000000000","message":"my bad! got it ! thanks for explanation","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"3384a103a5aee2ae04c3d32c828436b9980e5ce0","unresolved":false,"context_lines":[{"line_number":316,"context_line":"            self._get_other_dvr_router_ids_connected_router("},{"line_number":317,"context_line":"                context, router_id))"},{"line_number":318,"context_line":"        for dvr_router in connected_dvr_routers:"},{"line_number":319,"context_line":"            dvr_hosts |\u003d set("},{"line_number":320,"context_line":"                self._get_dvr_hosts_for_router(context, dvr_router))"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"        LOG.debug(\u0027Hosts for other DVR routers connected to router \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_3246c324","line":319,"range":{"start_line":319,"start_character":22,"end_line":319,"end_character":24},"updated":"2018-09-24 20:49:01.000000000","message":"as dvr_hosts is an empty set so \u0027\u003d\u0027 operator is also good here ?","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"bcf5682ffefe030f39730a23b8495bbe6c0aa1eb","unresolved":false,"context_lines":[{"line_number":316,"context_line":"            self._get_other_dvr_router_ids_connected_router("},{"line_number":317,"context_line":"                context, router_id))"},{"line_number":318,"context_line":"        for dvr_router in connected_dvr_routers:"},{"line_number":319,"context_line":"            dvr_hosts |\u003d set("},{"line_number":320,"context_line":"                self._get_dvr_hosts_for_router(context, dvr_router))"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"        LOG.debug(\u0027Hosts for other DVR routers connected to router \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_7bc17781","line":319,"range":{"start_line":319,"start_character":22,"end_line":319,"end_character":24},"in_reply_to":"3f79a3b5_3246c324","updated":"2018-09-24 21:03:05.000000000","message":"No, since we need to OR-in possibly more than one host.","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":17120,"name":"Manjeet Singh Bhatia","email":"manjeet.s.bhatia@intel.com","username":"manjeets"},"change_message_id":"1669bd34b821eeb06ca90e3e7e2e5992dc5e50f3","unresolved":false,"context_lines":[{"line_number":316,"context_line":"            self._get_other_dvr_router_ids_connected_router("},{"line_number":317,"context_line":"                context, router_id))"},{"line_number":318,"context_line":"        for dvr_router in connected_dvr_routers:"},{"line_number":319,"context_line":"            dvr_hosts |\u003d set("},{"line_number":320,"context_line":"                self._get_dvr_hosts_for_router(context, dvr_router))"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"        LOG.debug(\u0027Hosts for other DVR routers connected to router \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f79a3b5_acc4b742","line":319,"range":{"start_line":319,"start_character":22,"end_line":319,"end_character":24},"in_reply_to":"3f79a3b5_7bc17781","updated":"2018-09-25 00:50:16.000000000","message":"got it thanks !","commit_id":"6f725e97a1210f969f0e43bc3aea355d84d7623e"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"fe1c87aed14d47a143344c835b51b69b074aeae5","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        return subnet_ids"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def get_dvr_routers_to_remove(self, context, deleted_port,"},{"line_number":189,"context_line":"                                  get_related_hosts_info\u003dTrue):"},{"line_number":190,"context_line":"        \"\"\"Returns info about which routers should be removed"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        In case dvr serviceable port was deleted we need to check"}],"source_content_type":"text/x-python","patch_set":25,"id":"3f79a3b5_59872ab4","line":189,"range":{"start_line":189,"start_character":34,"end_line":189,"end_character":61},"updated":"2018-10-15 21:38:41.000000000","message":"Is this data being used in \u0027get_dvr_routers_to_remove\u0027?\nI meant is there code in here that handles \u0027get_related_hosts_info\u003dFalse\u0027.","commit_id":"2696d6fef4851bec3a52af621010e9cc1cb69832"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"b33e2168580244f6bac1e5ef8cca841421afd2b2","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        return subnet_ids"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def get_dvr_routers_to_remove(self, context, deleted_port,"},{"line_number":189,"context_line":"                                  get_related_hosts_info\u003dTrue):"},{"line_number":190,"context_line":"        \"\"\"Returns info about which routers should be removed"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        In case dvr serviceable port was deleted we need to check"}],"source_content_type":"text/x-python","patch_set":25,"id":"3f79a3b5_946267d8","line":189,"range":{"start_line":189,"start_character":34,"end_line":189,"end_character":61},"in_reply_to":"3f79a3b5_59872ab4","updated":"2018-10-15 21:42:39.000000000","message":"Yes, see e.g. L544 in this file","commit_id":"2696d6fef4851bec3a52af621010e9cc1cb69832"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"c0d6982fc9e8632da300d858ecc1ef7b60f90707","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        return subnet_ids"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def get_dvr_routers_to_remove(self, context, deleted_port,"},{"line_number":189,"context_line":"                                  get_related_hosts_info\u003dTrue):"},{"line_number":190,"context_line":"        \"\"\"Returns info about which routers should be removed"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"        In case dvr serviceable port was deleted we need to check"}],"source_content_type":"text/x-python","patch_set":25,"id":"3f79a3b5_abbe0f61","line":189,"range":{"start_line":189,"start_character":34,"end_line":189,"end_character":61},"in_reply_to":"3f79a3b5_946267d8","updated":"2018-10-16 17:18:48.000000000","message":"Yes I do see that L544 handles, it.\nI do see that \u0027get_related_hosts_info\u0027 is by default true in \u0027get_dvr_routers_to_remove\u0027.\nBut when \u0027get_related_hosts_info\u0027 is set to False, what part of the code in \u0027get_dvr_routers_to_remove\" is handling that case when it is set to False.\n\nI may be confused or not seeing the right part of the code.","commit_id":"2696d6fef4851bec3a52af621010e9cc1cb69832"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":143,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":144,"context_line":"                    if self._check_for_rtr_serviceable_ports("},{"line_number":145,"context_line":"                        context.elevated(), router_id, host):"},{"line_number":146,"context_line":"                        updated_routers.add(router_id)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"                if updated_routers:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_dfbf37a9","line":145,"updated":"2018-10-19 16:22:19.000000000","message":"nit: should be indented to make block obvious","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        subnet_ids \u003d [ip[\u0027subnet_id\u0027] for ip in deleted_port[\u0027fixed_ips\u0027]]"},{"line_number":201,"context_line":"        router_ids \u003d self.get_dvr_routers_by_subnet_ids(admin_context,"},{"line_number":202,"context_line":"                                                        subnet_ids)"},{"line_number":203,"context_line":"        related_routers \u003d set()"},{"line_number":204,"context_line":"        for router_id in router_ids:"},{"line_number":205,"context_line":"            connected_dvr_routers \u003d set("},{"line_number":206,"context_line":"                self._get_other_dvr_router_ids_connected_router("}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_022476c2","line":203,"range":{"start_line":203,"start_character":8,"end_line":203,"end_character":23},"updated":"2018-10-19 16:22:19.000000000","message":"nit: s/related_router_ids","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":205,"context_line":"            connected_dvr_routers \u003d set("},{"line_number":206,"context_line":"                self._get_other_dvr_router_ids_connected_router("},{"line_number":207,"context_line":"                    context, router_id))"},{"line_number":208,"context_line":"            related_routers |\u003d connected_dvr_routers"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        if not router_ids:"},{"line_number":211,"context_line":"            LOG.debug(\u0027No DVR routers for this DVR port %(port)s \u0027"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_25cbbc4b","line":208,"updated":"2018-10-19 16:22:19.000000000","message":"This block can actually move below, I\u0027ll update","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":230,"context_line":"                continue"},{"line_number":231,"context_line":"            if self._check_for_rtr_serviceable_ports("},{"line_number":232,"context_line":"                    admin_context, router_id, port_host):"},{"line_number":233,"context_line":"                host_has_serviceable_port \u003d True"},{"line_number":234,"context_line":"                continue"},{"line_number":235,"context_line":"            if not host_has_serviceable_port:"},{"line_number":236,"context_line":"                self._unbind_dvr_port_before_delete("}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_a59b4c77","line":233,"updated":"2018-10-19 16:22:19.000000000","message":"Same comment as below, once we set this we can stop checking, and we will never call the unbind code either.","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                        \u0027agent_id\u0027: str(agent.id)}"},{"line_number":240,"context_line":"                removed_router_info.append(info)"},{"line_number":241,"context_line":"                LOG.debug(\u0027Router %(router_id)s on host %(host)s to be \u0027"},{"line_number":242,"context_line":"                          \u0027deleted\u0027, info)"},{"line_number":243,"context_line":"        # Now collect the connected router info as well to remove"},{"line_number":244,"context_line":"        # it from the agent"},{"line_number":245,"context_line":"        for router_id in related_routers:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_225ad250","line":242,"updated":"2018-10-19 16:22:19.000000000","message":"We\u0027re logging here and L257, should remove this","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        for router_id in related_routers:"},{"line_number":246,"context_line":"            if router_id not in list(router_ids):"},{"line_number":247,"context_line":"                if self._check_for_rtr_serviceable_ports("},{"line_number":248,"context_line":"                    admin_context, router_id, port_host):"},{"line_number":249,"context_line":"                    host_has_serviceable_port \u003d True"},{"line_number":250,"context_line":"                    continue"},{"line_number":251,"context_line":"                if not host_has_serviceable_port:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_221f3272","line":248,"updated":"2018-10-19 16:22:19.000000000","message":"should be indented to make block obvious","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":246,"context_line":"            if router_id not in list(router_ids):"},{"line_number":247,"context_line":"                if self._check_for_rtr_serviceable_ports("},{"line_number":248,"context_line":"                    admin_context, router_id, port_host):"},{"line_number":249,"context_line":"                    host_has_serviceable_port \u003d True"},{"line_number":250,"context_line":"                    continue"},{"line_number":251,"context_line":"                if not host_has_serviceable_port:"},{"line_number":252,"context_line":"                    self._unbind_dvr_port_before_delete("}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_c581a84e","line":249,"updated":"2018-10-19 16:22:19.000000000","message":"Looks like we can optimize this a bit.  Once host_has_serviceable_port is True we shouldn\u0027t need to check again, since we will not set it False again.","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3a2f1ad0a04e685890a624431d6ffbbd9372a5f5","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                    admin_context, router_id, port_host):"},{"line_number":249,"context_line":"                    host_has_serviceable_port \u003d True"},{"line_number":250,"context_line":"                    continue"},{"line_number":251,"context_line":"                if not host_has_serviceable_port:"},{"line_number":252,"context_line":"                    self._unbind_dvr_port_before_delete("},{"line_number":253,"context_line":"                        context, router_id, port_host)"},{"line_number":254,"context_line":"                    info \u003d {\u0027router_id\u0027: router_id, \u0027host\u0027: port_host,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3f79a3b5_0587a05d","line":251,"updated":"2018-10-19 16:22:19.000000000","message":"This could also change on above comment.\n\nThe actual code that gets run shouldn\u0027t change.","commit_id":"ae1b489734fd1751abdb044f6f0c74af2fd20cc6"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"854af200f048bb365c755fe03171e5b6aee88c11","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        # After all serviceable ports in the host for the"},{"line_number":215,"context_line":"        # connected routers are deleted, then we can remove"},{"line_number":216,"context_line":"        # the router namespace."},{"line_number":217,"context_line":"        host_has_serviceable_port \u003d False"},{"line_number":218,"context_line":"        for router_id in router_ids:"},{"line_number":219,"context_line":"            if rb_obj.RouterL3AgentBinding.objects_exist(context,"},{"line_number":220,"context_line":"                                                         router_id\u003drouter_id,"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_76d72824","line":217,"updated":"2018-10-19 17:58:33.000000000","message":"I may have to test this block again, since there is more refactoring on this block.","commit_id":"ffd19362e3c651f2a893c270cb882cb4f377e398"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"c259321c30e927a28520d54878bceab3acc63ad1","unresolved":false,"context_lines":[{"line_number":214,"context_line":"        # After all serviceable ports in the host for the"},{"line_number":215,"context_line":"        # connected routers are deleted, then we can remove"},{"line_number":216,"context_line":"        # the router namespace."},{"line_number":217,"context_line":"        host_has_serviceable_port \u003d False"},{"line_number":218,"context_line":"        for router_id in router_ids:"},{"line_number":219,"context_line":"            if rb_obj.RouterL3AgentBinding.objects_exist(context,"},{"line_number":220,"context_line":"                                                         router_id\u003drouter_id,"}],"source_content_type":"text/x-python","patch_set":29,"id":"3f79a3b5_4be30dd8","line":217,"in_reply_to":"3f79a3b5_76d72824","updated":"2018-10-24 19:06:36.000000000","message":"As I suspected there is some issue with this refactor.\nIn my testing I am seeing some leftover router-namespaces even though there is no \u0027Serviceable\u0027 ports on that node.\n\nstack@ubuntu-18-ctlr-rocky:~/devstack$ neutron router-interface-delete test-router2 subnet4\nneutron CLI is deprecated and will be removed in the future. Use openstack CLI instead.\nRemoved interface from router test-router2.\nstack@ubuntu-18-ctlr-rocky:~/devstack$ neutron router-interface-delete test-router2 subnet2\nneutron CLI is deprecated and will be removed in the future. Use openstack CLI instead.\nRemoved interface from router test-router2.\nstack@ubuntu-18-ctlr-rocky:~/devstack$ neutron router-interface-delete test-router1 subnet1\nneutron CLI is deprecated and will be removed in the future. Use openstack CLI instead.\nRemoved interface from router test-router1.\nstack@ubuntu-18-ctlr-rocky:~/devstack$ neutron router-interface-delete test-router3 subnet3\nneutron CLI is deprecated and will be removed in the future. Use openstack CLI instead.\nRemoved interface from router test-router3.\n\nI still see this namespace leftover.\nstack@ubuntu-18-compute2:~/devstack$ sudo ip netns\nqrouter-98b57b8b-6a79-4e26-a018-c5b998b0d4fc (id: 2)","commit_id":"ffd19362e3c651f2a893c270cb882cb4f377e398"}],"neutron/tests/unit/agent/l3/test_agent.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"be80751520e08f6134d70609cce30207d74eb2d1","unresolved":false,"context_lines":[{"line_number":2508,"context_line":"        agent._queue \u003d mock.Mock()"},{"line_number":2509,"context_line":"        update \u003d mock.Mock()"},{"line_number":2510,"context_line":"        update.resource \u003d None"},{"line_number":2511,"context_line":"        update.action \u003d 2  # ROUTER_DELETED"},{"line_number":2512,"context_line":"        router_info \u003d mock.MagicMock()"},{"line_number":2513,"context_line":"        agent.router_info[update.id] \u003d router_info"},{"line_number":2514,"context_line":"        router_processor \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f79a3b5_45b1c644","line":2511,"updated":"2018-10-11 15:56:14.000000000","message":"should use l3_agent.ROUTER_DELETED","commit_id":"2a2f4bf59b95dcf88e1fce97b8fe2475ac148981"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"d6347a8e1b1275322d1d87587c3bf81f6156cef8","unresolved":false,"context_lines":[{"line_number":2508,"context_line":"        agent._queue \u003d mock.Mock()"},{"line_number":2509,"context_line":"        update \u003d mock.Mock()"},{"line_number":2510,"context_line":"        update.resource \u003d None"},{"line_number":2511,"context_line":"        update.action \u003d 2  # ROUTER_DELETED"},{"line_number":2512,"context_line":"        router_info \u003d mock.MagicMock()"},{"line_number":2513,"context_line":"        agent.router_info[update.id] \u003d router_info"},{"line_number":2514,"context_line":"        router_processor \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f79a3b5_052e6eaa","line":2511,"in_reply_to":"3f79a3b5_45b1c644","updated":"2018-10-11 16:20:08.000000000","message":"but it\u0027s not related change so I will not do it here","commit_id":"2a2f4bf59b95dcf88e1fce97b8fe2475ac148981"}],"neutron/tests/unit/scheduler/test_l3_agent_scheduler.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"cab27f84965562b5c0678d4c594feb113e75b88f","unresolved":false,"context_lines":[{"line_number":1235,"context_line":"                        return_value\u003d[agent_on_host]) as get_l3_agents,\\"},{"line_number":1236,"context_line":"                mock.patch.object("},{"line_number":1237,"context_line":"                        self.dut, \u0027get_hosts_to_notify\u0027,"},{"line_number":1238,"context_line":"                        return_value\u003d[\"other_host\"]):"},{"line_number":1239,"context_line":""},{"line_number":1240,"context_line":"            self.dut.dvr_handle_new_service_port("},{"line_number":1241,"context_line":"                self.adminContext, port)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_5de19acf","line":1238,"range":{"start_line":1238,"start_character":38,"end_line":1238,"end_character":50},"updated":"2018-09-17 20:59:39.000000000","message":"super nit: \u0027other_host\u0027 not \"\"","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"adb4e53c5066abe293c677682ba1202c4b32695c","unresolved":false,"context_lines":[{"line_number":1235,"context_line":"                        return_value\u003d[agent_on_host]) as get_l3_agents,\\"},{"line_number":1236,"context_line":"                mock.patch.object("},{"line_number":1237,"context_line":"                        self.dut, \u0027get_hosts_to_notify\u0027,"},{"line_number":1238,"context_line":"                        return_value\u003d[\"other_host\"]):"},{"line_number":1239,"context_line":""},{"line_number":1240,"context_line":"            self.dut.dvr_handle_new_service_port("},{"line_number":1241,"context_line":"                self.adminContext, port)"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_d741fa37","line":1238,"range":{"start_line":1238,"start_character":38,"end_line":1238,"end_character":50},"in_reply_to":"3f79a3b5_5de19acf","updated":"2018-09-18 20:42:36.000000000","message":"Done","commit_id":"606ea6ea2f492c01375cf8ffa860a78586a04db8"}]}