)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"59937f152d996536a054f83186206c762cd4894a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":8,"id":"4b664b3c_afc282a5","updated":"2026-05-26 09:47:12.000000000","message":"Please fix that imports in one of the files and it is good for me. But I am not FRR nor BGP EVPN expert so I will rely on the reviews from others for sure :)","commit_id":"cad7e2d4acd26ccc8fa4f2ee7b2f8975537808fd"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"dcd10be745c5fc2c7784dd07b97817a70ec14819","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b47276c3_8f3580b4","in_reply_to":"4b664b3c_afc282a5","updated":"2026-06-01 19:41:09.000000000","message":"Done","commit_id":"cad7e2d4acd26ccc8fa4f2ee7b2f8975537808fd"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"e727e9a3_6813d823","updated":"2026-06-02 14:45:57.000000000","message":"Some comments but overall looks great!","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"cbcfa1698749c84316770d58b9780b35cb90e013","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"95feee2a_38bd2957","updated":"2026-06-08 13:12:29.000000000","message":"I had to rebase","commit_id":"db76f4c6742c26f720d5e49f94a3665639443f38"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"6483620f18bdcd612aca4eb7ed856b47e2fd7d66","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"81fe91ed_d65a6cb3","updated":"2026-06-09 03:54:01.000000000","message":"parent changed so need to rebase once the changes their stabilizes","commit_id":"aa12b22084cf48482b7007fe6e23e5074d0c2c1f"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"3d94f74bf099fa92083e0f8947b974b8300dc2c9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"6bf9dced_5239ff37","updated":"2026-06-09 14:01:02.000000000","message":"recheck ovs job timeout https://bugs.launchpad.net/neutron/+bug/2155883","commit_id":"4bb1a29ddc50679ef94340c91dfd3d05bb8e51f4"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b535c55dafd02306835f68dbd21522da009ccf67","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"321ca7c0_14e1cf7c","updated":"2026-06-09 10:11:58.000000000","message":"recheck parent in gate again","commit_id":"4bb1a29ddc50679ef94340c91dfd3d05bb8e51f4"}],"devstack/etc/frr_with_evpn/daemons":[{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"ff2aa92586fe4c6e5c77001fee0a6c3853743c7c","unresolved":true,"context_lines":[{"line_number":15,"context_line":"fabricd\u003dno"},{"line_number":16,"context_line":"vrrpd\u003dno"},{"line_number":17,"context_line":"pathd\u003dno"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"#"},{"line_number":20,"context_line":"# If this option is set the /etc/init.d/frr script automatically loads"},{"line_number":21,"context_line":"# the config via \"vtysh -b\" when the servers are started."}],"source_content_type":"application/octet-stream","patch_set":17,"id":"e251db39_96f534ff","line":18,"updated":"2026-06-08 05:38:52.000000000","message":"nit: looks no addition daemons needed so can link to default frr daemons itself devstack/etc/frr/daemons","commit_id":"822c325368622a52707f2449527b39efd662852d"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"cbcfa1698749c84316770d58b9780b35cb90e013","unresolved":false,"context_lines":[{"line_number":15,"context_line":"fabricd\u003dno"},{"line_number":16,"context_line":"vrrpd\u003dno"},{"line_number":17,"context_line":"pathd\u003dno"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"#"},{"line_number":20,"context_line":"# If this option is set the /etc/init.d/frr script automatically loads"},{"line_number":21,"context_line":"# the config via \"vtysh -b\" when the servers are started."}],"source_content_type":"application/octet-stream","patch_set":17,"id":"3c6d9eb2_b15dded5","line":18,"in_reply_to":"e251db39_96f534ff","updated":"2026-06-08 13:12:29.000000000","message":"True, and when I first implemented it I wanted to do it that way. But then decided for a standalone demons. 1. We might want to enable some daemon or change some settings for \"Evpn\" test in the near future. 2. Similar reason, since this is for \"EVPN\" I wanted to keep it completely decoupled from the other `daemons` which are used for \"BGP core\". Technically, they will most likely be always the same, but still I like that this is completely independent.","commit_id":"822c325368622a52707f2449527b39efd662852d"}],"devstack/lib/frr":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6a92d10b83db13c0554acdc50df76d77b17f818e","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    # Clean the FRRt configuration dir"},{"line_number":62,"context_line":"    sudo rm -rf $FRR_CONF_DIR"},{"line_number":63,"context_line":"}"},{"line_number":64,"context_line":""}],"source_content_type":"application/x-shellscript","patch_set":13,"id":"3fdb9aac_5d65a160","side":"PARENT","line":64,"updated":"2026-06-03 17:32:45.000000000","message":"I didn\u0027t like this line either","commit_id":"af96a56aaf6c76cd6b08e1c24a8cdf003bc9264b"}],"neutron/agent/linux/evpn_router/frr/exceptions.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    def __init__(self, message, step\u003dNone, cause\u003dNone):"},{"line_number":21,"context_line":"        super().__init__(message)"},{"line_number":22,"context_line":"        self.step \u003d step"},{"line_number":23,"context_line":"        self.cause \u003d cause"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class FrrTemplateRenderError(FrrDriverError):"}],"source_content_type":"text/x-python","patch_set":11,"id":"f2e9d0aa_77c2defc","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":26},"updated":"2026-06-02 14:45:57.000000000","message":"Are these used anywhere?","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"83bb48f2e54f472b649626fc796de1b9e31174b1","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    def __init__(self, message, step\u003dNone, cause\u003dNone):"},{"line_number":21,"context_line":"        super().__init__(message)"},{"line_number":22,"context_line":"        self.step \u003d step"},{"line_number":23,"context_line":"        self.cause \u003d cause"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class FrrTemplateRenderError(FrrDriverError):"}],"source_content_type":"text/x-python","patch_set":11,"id":"0612a720_d559dfb0","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":26},"in_reply_to":"dcc5c8b5_b3bfed5f","updated":"2026-06-03 17:20:32.000000000","message":"Done","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":true,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"    def __init__(self, message, step\u003dNone, cause\u003dNone):"},{"line_number":21,"context_line":"        super().__init__(message)"},{"line_number":22,"context_line":"        self.step \u003d step"},{"line_number":23,"context_line":"        self.cause \u003d cause"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class FrrTemplateRenderError(FrrDriverError):"}],"source_content_type":"text/x-python","patch_set":11,"id":"dcc5c8b5_b3bfed5f","line":23,"range":{"start_line":22,"start_character":0,"end_line":23,"end_character":26},"in_reply_to":"f2e9d0aa_77c2defc","updated":"2026-06-02 21:27:28.000000000","message":"Yes they are used, notice call exception inherit from `FrrDriverError` then raise, e.g.\n```\n            raise frr_exceptions.FrrTemplateRenderError(\n                \"Failed to render FRR template context:\\n%s\" % context,\n                step\u003dtemplate_name,\n                cause\u003derr,\n```\n\nI started this form because I figured it would be nice in terms of calling this driver in the state machine. But I dont feel strongly about it, I can remove `step and cause` if you like","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"}],"neutron/agent/linux/evpn_router/frr/frr_driver.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ba1baeec1a0e488826f704a9a21f4565b2666697","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        return std_err in (\u0027\u0027, f\u0027Cannot find device \"{vrf_name}\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"class FrrVtyshDriver(interface.EVPNRouterDriver):"},{"line_number":155,"context_line":"    \"\"\"Apply EVPN configuration via vtysh."},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    Uses EVPNRouterConfig for portable fields."}],"source_content_type":"text/x-python","patch_set":1,"id":"efd71746_34e4230f","line":154,"updated":"2026-05-11 18:33:54.000000000","message":"This class seems almost stateless except the vrf_handler. The whole code structure seems to me rather procedural than OOP. I think there are some good design patterns to generate a config (file). Just by a quick look I think the most suitable is the Builder Pattern: https://en.wikipedia.org/wiki/Builder_pattern\n\nWith that, I think it can be easier to extend its functionality to be able to provide custom config files and the code would be more readable as it would look something like\n```\nFrrConfigBuilder().add_router(asn\u003d1234).add_address_family(param1, param2).add_interface()\n```\n\nThis would be more object-oriented and each section can be extended as we like.","commit_id":"e3e1484dee7922890ad17a2a86acf0c7f5697a35"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"2cff442abe7152aeba2e697b6372d9f278f408b0","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        return std_err in (\u0027\u0027, f\u0027Cannot find device \"{vrf_name}\u0027)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"class FrrVtyshDriver(interface.EVPNRouterDriver):"},{"line_number":155,"context_line":"    \"\"\"Apply EVPN configuration via vtysh."},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    Uses EVPNRouterConfig for portable fields."}],"source_content_type":"text/x-python","patch_set":1,"id":"08948af9_3c253ebe","line":154,"in_reply_to":"efd71746_34e4230f","updated":"2026-05-19 19:47:20.000000000","message":"Done","commit_id":"e3e1484dee7922890ad17a2a86acf0c7f5697a35"},{"author":{"_account_id":38298,"name":"Helen Chen","display_name":"Helen Chen","email":"ichen@redhat.com","username":"ingwherchen"},"change_message_id":"5405dde801345dea985fb758173f1ae5ff2d0938","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"a2bedd35_4a65968a","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"updated":"2026-05-21 20:18:48.000000000","message":"Is this still necessary if this is called from the FSM?","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":38298,"name":"Helen Chen","display_name":"Helen Chen","email":"ichen@redhat.com","username":"ingwherchen"},"change_message_id":"9eed2db638aa84df73331fd7ae708ca27e038d5e","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"2a9892a5_1eae9273","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"in_reply_to":"042137fb_6af7bf2b","updated":"2026-05-21 20:54:37.000000000","message":"Even if generic, this is also unnecessary, at least not for FRR\u0027s benefit.  FRR listens for VRF creation/deletion via netlink and can manage without the VRF existing.","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"d974a7b63ee6b834e43bffc05d047979c86d5a2f","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"800d4909_93833f40","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"in_reply_to":"2a9892a5_1eae9273","updated":"2026-05-21 23:43:38.000000000","message":"Through my experimentation I discovered that frr does care about the state of vrf on a system. For example, if you try to remove `evpnvrf-11` reference from frr.config but `evpnvrf-11` vrf still exists, you will get following error\n```\n1097741|zebra] done\n% Configuration failed.\n\ncommit failed session-id 15 on Unknown-FD-16 req-id 3 source-ds: candidate target-ds: running validate-only: 0: reason: \u0027Failed to create cfgdata: Only inactive VRFs can be deleted\u0027\n```\nI encountered similar error when for example I was trying to add vrf into frr.config but that vrf did not exists.\n\nThat is the reason I created the `EVPNRouterVrfHandler`. It basically defines a contract that you must pass to FrrVtyshDriver which will handle vrf.\n\nFor example, in functional testing I will do something like this(simplification):\n```\nclass FunctionalTestVRFHandler(interface.VRFHandler):\n    def ensure_vrf_exists(self, vrf_name):\n        utils.execute([ip, link, add, type, vrf, {vrf_name}])\n\n    def ensure_vrf_deleted(self, vrf_name):\n         utils.execute([ip, link, del, {vrf_name}])\n```\n\nIn FSM case, I might just create a dummy handler since the FSM state machine will make sure that `frr_driver.create_evpn_router` is always called after vrf netlink was detected\n\n```\nclass FSMEVPNRouterVrfHandler(interface.VRFHandle):\n    def ensure_vrf_exists(self, vrf_name):\n        # just pass, FSM state enforces that vrf present and active\n        pass\n\n    def ensure_vrf_deleted(self, vrf_name):\n        pass\n\n```\n\nThis implementation keeps my `FrrVtyshDriver` generic and can be used by anyone.\n\nBut I am open to other implementation, this is just my take on it","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"dcd10be745c5fc2c7784dd07b97817a70ec14819","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"79b9bebf_b38b2924","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"in_reply_to":"788e44fe_61180f7c","updated":"2026-06-01 19:41:09.000000000","message":"So some VRFHandler is still needed, right?\nAs you pointed out, the frr requires VRF in the kernel to be in a proper state. This `VRFHandler` just ensures that the caller handles that in whatever way it wants. Either, creating vrf using `ip command` or OVN creating it or whatever other way.\nAlso for a proper working order, the frr needs other things like vxlan interface. The `NamespacedVRFHandler` in the functional test is one example. In the state machine state that will be a class which will be a composition of OVN creating the vrf and the code you are implementing for SVD.\n\nWhat I am thinking now is to maybe just update the abstract class with the information your provided and rename the abstract methods so the purpose of the abstract class is clear?\n\n```\nclass EVPNRouterVrfHandler(abc.ABC):\n    \"\"\" EVPN Router VRF handler.\n    \n    FRR requires that referenced vrfs are in certain state for a proper\n    operation.\n    \n    \"\"\"\n\n    @abc.abstractmethod\n    def ensure_vrf_operational(self, vrf_name) -\u003e None:\n        \"\"\" Ensure vrf is in proper state\n        \n        FRR can manage the state of VRF in the kernel, but one must \n        ensure that VRF is created on the system. Creation of vrf does not need\n        to necessarily happen now, but it could. In most cases, creation of linux\n        interfaces is required for proper proper operation and thus vrf will be\n        created anyway. see frr doc link \u003cinsert\u003e\n                \n        \"\"\"\n        pass\n\n    @abc.abstractmethod\n    def ensure_vrf_deleted(self, vrf_name) -\u003e None:\n        \"\"\"\n        Ensure vrf is deleted from the kernel, otherwise frr will refuse to\n        delete it as it consider the VRF still active.\n        \"\"\"\n        pass\n```","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"ff2aa92586fe4c6e5c77001fee0a6c3853743c7c","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"89b8acb9_350ef683","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"in_reply_to":"79b9bebf_b38b2924","updated":"2026-06-08 05:38:52.000000000","message":"this seems to be unresolved yet, @mtomaska@redhat.com @ichen@redhat.com any action plan here ? or am missing something","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":38298,"name":"Helen Chen","display_name":"Helen Chen","email":"ichen@redhat.com","username":"ingwherchen"},"change_message_id":"25983533a3722bd314d2009a64a4bb8e852e1c72","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"788e44fe_61180f7c","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"in_reply_to":"800d4909_93833f40","updated":"2026-05-29 19:08:54.000000000","message":"FRR keeps two different states for a VRF, VRF_ACTIVE (in kernel) and VRF_CONFIGURED (in frr.conf).  FRR monitors VRF creation by listening for netlink messages.  When FRR learns through netlink that a VRF is created, FRR adds the vrf configuration.  FRR does not allow for the vrf configuration to be deleted if the VRF devices still exists in the kernel.","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":38298,"name":"Helen Chen","display_name":"Helen Chen","email":"ichen@redhat.com","username":"ingwherchen"},"change_message_id":"25983533a3722bd314d2009a64a4bb8e852e1c72","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1e7a92a0_847d177e","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"in_reply_to":"800d4909_93833f40","updated":"2026-05-29 19:08:54.000000000","message":"On my setup, when the VRF device does not exist in the kernel, FRR allows for VRF configuration to be added.\n\nHowever, there seems to be some asymmetry on delete.  For each VRF, FRR keeps two states, VRF_ACTIVE and VRF_CONFIGURED.  Both states need to be unset for the VRF configuration to be deleted.\n\nFRR monitors netlink messages for VRF create and delete.  Once FRR detects that a VRF is created, FRR adds the VRF configuration automatically, sets the VRF state to VRF_ACTIVE and only unsets it until the VRF device is deleted from the kernel. This VRF that FRR automatically adds to it configuration does not have the state VRF_CONFIGURED.  So, FRR complains when using configuration to delete the VRF at this point, when VRF_ACTIVE but not VRF_CONFIGURED.\n\nWhen the VRF device exists in the kernel, triggering FRR to automatically add the VRF configuration, and the user subsequently adds the VNI configuration to the VRF (first entering the VRF context and then adding VNI configuration), the VRF is considered VRF_CONFIGURED.  At this point, the VRF is both VRF_ACTIVE and VRF_CONFIGURED.  At this point, if FRR detects that the VRF device is removed, FRR does not complain but does not remove the VRF configuration.\n\nSo on my setup, create can occur in either order, VRF create in the kernel first or VRF create in FRR configuration first.  Clean delete can only occur in one order, VRF delete from the kernel first followed by VRF delete from FRR configuration.\n\nIMO, whether to check for VRF device in the kernel depends on what we want the code to be.  If this code is purely intended to transmit configuration to FRR, then it shouldn\u0027t check for VRF device.  If this code is intended configure certain things, then it should check for VRF device.","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"cbcfa1698749c84316770d58b9780b35cb90e013","unresolved":false,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"27670007_9737a840","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"in_reply_to":"89b8acb9_350ef683","updated":"2026-06-08 13:12:29.000000000","message":"I think we agreed to go with what I have for now. @ichen@redhat.com please correct me if I am wrong.","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"957f7d9356cd4c6d238b03555154d734de28fe81","unresolved":true,"context_lines":[{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def create_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":272,"context_line":"        LOG.debug(\"Creating EVPN router: %s\", config)"},{"line_number":273,"context_line":"        self.vrf_handler.ensure_vrf_exists(config.vrf_name)"},{"line_number":274,"context_line":"        self._create_bgp_router(config)"},{"line_number":275,"context_line":"        add_evpn_router_cmd \u003d self.cmd_builder.add_evpn_router_cmds(config)"},{"line_number":276,"context_line":"        self.executor.execute_cmds(add_evpn_router_cmd)"}],"source_content_type":"text/x-python","patch_set":4,"id":"042137fb_6af7bf2b","line":273,"range":{"start_line":273,"start_character":8,"end_line":273,"end_character":59},"in_reply_to":"a2bedd35_4a65968a","updated":"2026-05-21 20:20:46.000000000","message":"Although this driver will be used by FSM, I wanted to keep it generic unaware who will use it.","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":38298,"name":"Helen Chen","display_name":"Helen Chen","email":"ichen@redhat.com","username":"ingwherchen"},"change_message_id":"5405dde801345dea985fb758173f1ae5ff2d0938","unresolved":true,"context_lines":[{"line_number":277,"context_line":""},{"line_number":278,"context_line":"    def delete_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":279,"context_line":"        LOG.debug(\"Deleting EVPN router: %s\", config)"},{"line_number":280,"context_line":"        self.vrf_handler.ensure_vrf_deleted(config.vrf_name)"},{"line_number":281,"context_line":"        self.executor.execute_cmds("},{"line_number":282,"context_line":"            self.cmd_builder.delete_evpn_router_cmds(config))"}],"source_content_type":"text/x-python","patch_set":4,"id":"011fea84_a8669e53","line":280,"range":{"start_line":280,"start_character":8,"end_line":280,"end_character":60},"updated":"2026-05-21 20:18:48.000000000","message":"Same question. Is this still necessary if this is called from the FSM?","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"0e3d9b74609ab8f978dad7482d2f438e5f0ac00d","unresolved":false,"context_lines":[{"line_number":277,"context_line":""},{"line_number":278,"context_line":"    def delete_evpn_router(self, config: interface.EVPNRouterConfig) -\u003e None:"},{"line_number":279,"context_line":"        LOG.debug(\"Deleting EVPN router: %s\", config)"},{"line_number":280,"context_line":"        self.vrf_handler.ensure_vrf_deleted(config.vrf_name)"},{"line_number":281,"context_line":"        self.executor.execute_cmds("},{"line_number":282,"context_line":"            self.cmd_builder.delete_evpn_router_cmds(config))"}],"source_content_type":"text/x-python","patch_set":4,"id":"a4c9d0fd_c3fb77f7","line":280,"range":{"start_line":280,"start_character":8,"end_line":280,"end_character":60},"in_reply_to":"011fea84_a8669e53","updated":"2026-06-01 19:42:18.000000000","message":"see my response to your earlier `VRFHandler` class","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"1f7dfe067dd5f8d4f86cc62b32685516e6015a31","unresolved":true,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class FrrVtyshExecutor:"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def __init__(self, namespace: str | None \u003d None):"},{"line_number":166,"context_line":"        self._namespace \u003d namespace"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def _execute_vtysh(self, vtysh_args: list[str]) -\u003e str:"}],"source_content_type":"text/x-python","patch_set":5,"id":"c3eed4ad_5398960b","line":165,"range":{"start_line":165,"start_character":22,"end_line":165,"end_character":51},"updated":"2026-05-23 02:57:07.000000000","message":"I am undecided if passing namespace here is a good idea. The namespace here is purely because tests run frr in a namespace. In production we will run frr in the default namespace. I dont like mixing \"test business\" in production code. What is your thought about it.\n\nAnother idea would be to overwrite `FrrVtyshExecutor` class which just adds namespace, something like this\n\n```\nclass FrrVtyshExecutorNamespaced(FrrVtyshExecutor):\n      def __init__(self, namespace):\n          super().__init__(additional_args\u003d[\u0027-N\u0027, namespace])\n```\n\nor another idea is to simply define a generic method in `FrrVtyshExecutor` which takes any  additional arguments that get appended to each command.","commit_id":"80d22355030d5ea9618c296880c388d799f2f588"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"dcd10be745c5fc2c7784dd07b97817a70ec14819","unresolved":false,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class FrrVtyshExecutor:"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def __init__(self, namespace: str | None \u003d None):"},{"line_number":166,"context_line":"        self._namespace \u003d namespace"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def _execute_vtysh(self, vtysh_args: list[str]) -\u003e str:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5ea77266_b1ed26c6","line":165,"range":{"start_line":165,"start_character":22,"end_line":165,"end_character":51},"in_reply_to":"9c575085_d780a699","updated":"2026-06-01 19:41:09.000000000","message":"I think I will leave as is for now.","commit_id":"80d22355030d5ea9618c296880c388d799f2f588"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"59937f152d996536a054f83186206c762cd4894a","unresolved":true,"context_lines":[{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class FrrVtyshExecutor:"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def __init__(self, namespace: str | None \u003d None):"},{"line_number":166,"context_line":"        self._namespace \u003d namespace"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"    def _execute_vtysh(self, vtysh_args: list[str]) -\u003e str:"}],"source_content_type":"text/x-python","patch_set":5,"id":"9c575085_d780a699","line":165,"range":{"start_line":165,"start_character":22,"end_line":165,"end_character":51},"in_reply_to":"c3eed4ad_5398960b","updated":"2026-05-26 09:47:12.000000000","message":"I\u0027m fine either way","commit_id":"80d22355030d5ea9618c296880c388d799f2f588"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"59937f152d996536a054f83186206c762cd4894a","unresolved":true,"context_lines":[{"line_number":25,"context_line":"from neutron.agent.linux import utils as linux_utils"},{"line_number":26,"context_line":"from neutron_lib import exceptions"},{"line_number":27,"context_line":"from oslo_log import log as logging"},{"line_number":28,"context_line":"from oslo_serialization import jsonutils"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":8,"id":"b706aa77_c417289e","line":28,"updated":"2026-05-26 09:47:12.000000000","message":"please divide imports into 3 different groups: imports from stdlib, import of 3rd party libs and finally imports of neutron modules. This is common pattern we use in Neutron and other OpenStack projects also","commit_id":"cad7e2d4acd26ccc8fa4f2ee7b2f8975537808fd"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"0e3d9b74609ab8f978dad7482d2f438e5f0ac00d","unresolved":false,"context_lines":[{"line_number":25,"context_line":"from neutron.agent.linux import utils as linux_utils"},{"line_number":26,"context_line":"from neutron_lib import exceptions"},{"line_number":27,"context_line":"from oslo_log import log as logging"},{"line_number":28,"context_line":"from oslo_serialization import jsonutils"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":8,"id":"facc05be_67321fcf","line":28,"in_reply_to":"b706aa77_c417289e","updated":"2026-06-01 19:42:18.000000000","message":"Done","commit_id":"cad7e2d4acd26ccc8fa4f2ee7b2f8975537808fd"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"59937f152d996536a054f83186206c762cd4894a","unresolved":true,"context_lines":[{"line_number":191,"context_line":"            f.flush()"},{"line_number":192,"context_line":"            temp_path \u003d f.name"},{"line_number":193,"context_line":"            try:"},{"line_number":194,"context_line":"                self._execute_vtysh([\u0027--dryrun\u0027, \u0027-f\u0027, temp_path])"},{"line_number":195,"context_line":"            except exceptions.ProcessExecutionError as err:"},{"line_number":196,"context_line":"                raise frr_exceptions.FrrDryrunError("},{"line_number":197,"context_line":"                    \"FRR syntatic validity failed for \""}],"source_content_type":"text/x-python","patch_set":8,"id":"c611de1b_0e1a287e","line":194,"updated":"2026-05-26 09:47:12.000000000","message":"do we need to run that test always? Maybe only if debug is enabled or something like that?","commit_id":"cad7e2d4acd26ccc8fa4f2ee7b2f8975537808fd"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"dcd10be745c5fc2c7784dd07b97817a70ec14819","unresolved":false,"context_lines":[{"line_number":191,"context_line":"            f.flush()"},{"line_number":192,"context_line":"            temp_path \u003d f.name"},{"line_number":193,"context_line":"            try:"},{"line_number":194,"context_line":"                self._execute_vtysh([\u0027--dryrun\u0027, \u0027-f\u0027, temp_path])"},{"line_number":195,"context_line":"            except exceptions.ProcessExecutionError as err:"},{"line_number":196,"context_line":"                raise frr_exceptions.FrrDryrunError("},{"line_number":197,"context_line":"                    \"FRR syntatic validity failed for \""}],"source_content_type":"text/x-python","patch_set":8,"id":"1bc64d9e_385e5fb3","line":194,"in_reply_to":"c611de1b_0e1a287e","updated":"2026-06-01 19:41:09.000000000","message":"Initially, I had implementation where user was able to provide their own templates as well. In this case, the `dryrun` would act as a \"linter\" in order to not apply any bad syntax, which will make frr process to stop working. \nIn the current implementation, we are applying same static templates, so their syntax should always be correct. But I would still prefer to do this check just in case somehow some bad substitution has happened. At least the running config will continue to be operational.","commit_id":"cad7e2d4acd26ccc8fa4f2ee7b2f8975537808fd"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import tempfile"},{"line_number":17,"context_line":"from typing import Any"},{"line_number":18,"context_line":"from typing import cast"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from jinja2 import DictLoader"},{"line_number":21,"context_line":"from jinja2 import Environment"},{"line_number":22,"context_line":"from neutron_lib import exceptions"},{"line_number":23,"context_line":"from oslo_log import log as logging"},{"line_number":24,"context_line":"from oslo_serialization import jsonutils"}],"source_content_type":"text/x-python","patch_set":11,"id":"5613509b_59d71028","line":21,"range":{"start_line":17,"start_character":0,"end_line":21,"end_character":30},"updated":"2026-06-02 14:45:57.000000000","message":"import the modules, not objects: https://docs.openstack.org/charm-guide/victoria/coding-guidelines.html#import-style","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import tempfile"},{"line_number":17,"context_line":"from typing import Any"},{"line_number":18,"context_line":"from typing import cast"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from jinja2 import DictLoader"},{"line_number":21,"context_line":"from jinja2 import Environment"},{"line_number":22,"context_line":"from neutron_lib import exceptions"},{"line_number":23,"context_line":"from oslo_log import log as logging"},{"line_number":24,"context_line":"from oslo_serialization import jsonutils"}],"source_content_type":"text/x-python","patch_set":11,"id":"66f9a4f4_79139f4a","line":21,"range":{"start_line":17,"start_character":0,"end_line":21,"end_character":30},"in_reply_to":"5613509b_59d71028","updated":"2026-06-02 21:27:28.000000000","message":"Done","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"c0538d5608ab5d015a85cad571fdf4818637f276","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import tempfile"},{"line_number":17,"context_line":"from typing import Any"},{"line_number":18,"context_line":"from typing import cast"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from jinja2 import DictLoader"},{"line_number":21,"context_line":"from jinja2 import Environment"},{"line_number":22,"context_line":"from neutron_lib import exceptions"},{"line_number":23,"context_line":"from oslo_log import log as logging"},{"line_number":24,"context_line":"from oslo_serialization import jsonutils"}],"source_content_type":"text/x-python","patch_set":11,"id":"966ac87a_1ec8b82d","line":21,"range":{"start_line":17,"start_character":0,"end_line":21,"end_character":30},"in_reply_to":"6401796f_ca6a186a","updated":"2026-06-03 18:36:25.000000000","message":"Done","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6a92d10b83db13c0554acdc50df76d77b17f818e","unresolved":true,"context_lines":[{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import tempfile"},{"line_number":17,"context_line":"from typing import Any"},{"line_number":18,"context_line":"from typing import cast"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from jinja2 import DictLoader"},{"line_number":21,"context_line":"from jinja2 import Environment"},{"line_number":22,"context_line":"from neutron_lib import exceptions"},{"line_number":23,"context_line":"from oslo_log import log as logging"},{"line_number":24,"context_line":"from oslo_serialization import jsonutils"}],"source_content_type":"text/x-python","patch_set":11,"id":"6401796f_ca6a186a","line":21,"range":{"start_line":17,"start_character":0,"end_line":21,"end_character":30},"in_reply_to":"66f9a4f4_79139f4a","updated":"2026-06-03 17:32:45.000000000","message":"Still importing objects from `jinja2`","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":37,"context_line":"    def __init__(self):"},{"line_number":38,"context_line":"        self._env \u003d Environment("},{"line_number":39,"context_line":"            loader\u003dDictLoader({"},{"line_number":40,"context_line":"                \u0027add_bgp_router\u0027: frr_templates.ADD_BGP_ROUTER,"},{"line_number":41,"context_line":"                \u0027add_evpn_router\u0027: frr_templates.ADD_EVPN_ROUTER,"},{"line_number":42,"context_line":"                \u0027del_bgp_router\u0027: frr_templates.DEL_BGP_ROUTER,"},{"line_number":43,"context_line":"                \u0027del_evpn_router\u0027: frr_templates.DEL_EVPN_ROUTER,"}],"source_content_type":"text/x-python","patch_set":11,"id":"91e9c530_21771b52","line":40,"range":{"start_line":40,"start_character":16,"end_line":40,"end_character":32},"updated":"2026-06-02 14:45:57.000000000","message":"Should these be constants?","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"c0538d5608ab5d015a85cad571fdf4818637f276","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    def __init__(self):"},{"line_number":38,"context_line":"        self._env \u003d Environment("},{"line_number":39,"context_line":"            loader\u003dDictLoader({"},{"line_number":40,"context_line":"                \u0027add_bgp_router\u0027: frr_templates.ADD_BGP_ROUTER,"},{"line_number":41,"context_line":"                \u0027add_evpn_router\u0027: frr_templates.ADD_EVPN_ROUTER,"},{"line_number":42,"context_line":"                \u0027del_bgp_router\u0027: frr_templates.DEL_BGP_ROUTER,"},{"line_number":43,"context_line":"                \u0027del_evpn_router\u0027: frr_templates.DEL_EVPN_ROUTER,"}],"source_content_type":"text/x-python","patch_set":11,"id":"737a8f02_64050ca5","line":40,"range":{"start_line":40,"start_character":16,"end_line":40,"end_character":32},"in_reply_to":"111df32c_f8b8d13c","updated":"2026-06-03 18:36:25.000000000","message":"Done","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    def __init__(self):"},{"line_number":38,"context_line":"        self._env \u003d Environment("},{"line_number":39,"context_line":"            loader\u003dDictLoader({"},{"line_number":40,"context_line":"                \u0027add_bgp_router\u0027: frr_templates.ADD_BGP_ROUTER,"},{"line_number":41,"context_line":"                \u0027add_evpn_router\u0027: frr_templates.ADD_EVPN_ROUTER,"},{"line_number":42,"context_line":"                \u0027del_bgp_router\u0027: frr_templates.DEL_BGP_ROUTER,"},{"line_number":43,"context_line":"                \u0027del_evpn_router\u0027: frr_templates.DEL_EVPN_ROUTER,"}],"source_content_type":"text/x-python","patch_set":11,"id":"be602b3a_39a51b68","line":40,"range":{"start_line":40,"start_character":16,"end_line":40,"end_character":32},"in_reply_to":"91e9c530_21771b52","updated":"2026-06-02 21:27:28.000000000","message":"Sure, but I dont see benefit of it at this point. I would rather not increase amount of code. We can convert to constants in the future if there is need.","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6a92d10b83db13c0554acdc50df76d77b17f818e","unresolved":true,"context_lines":[{"line_number":37,"context_line":"    def __init__(self):"},{"line_number":38,"context_line":"        self._env \u003d Environment("},{"line_number":39,"context_line":"            loader\u003dDictLoader({"},{"line_number":40,"context_line":"                \u0027add_bgp_router\u0027: frr_templates.ADD_BGP_ROUTER,"},{"line_number":41,"context_line":"                \u0027add_evpn_router\u0027: frr_templates.ADD_EVPN_ROUTER,"},{"line_number":42,"context_line":"                \u0027del_bgp_router\u0027: frr_templates.DEL_BGP_ROUTER,"},{"line_number":43,"context_line":"                \u0027del_evpn_router\u0027: frr_templates.DEL_EVPN_ROUTER,"}],"source_content_type":"text/x-python","patch_set":11,"id":"111df32c_f8b8d13c","line":40,"range":{"start_line":40,"start_character":16,"end_line":40,"end_character":32},"in_reply_to":"be602b3a_39a51b68","updated":"2026-06-03 17:32:45.000000000","message":"There are arbitrary strings passed around as arguments. The benefit is that if you make a typo on L145, you\u0027ll not know until the point the add_bgp_router_cmds is called. As opposed to making a typo in a constant, the module import fails.","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":164,"context_line":"class FrrVtyshExecutor:"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    def __init__(self, namespace: str | None \u003d None):"},{"line_number":167,"context_line":"        self._namespace \u003d namespace"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _execute_vtysh(self, vtysh_args: list[str]) -\u003e str:"},{"line_number":170,"context_line":"        \"\"\"Execute any vtysh command args and return stdout.\"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"2ba96f51_9d57a9e6","line":167,"range":{"start_line":167,"start_character":0,"end_line":167,"end_character":35},"updated":"2026-06-02 14:45:57.000000000","message":"I assume this is just for the functional tests and not used in production. I\u0027d subclass the `FrrVtyshExecutor` class in the functional tests and override a method to create the cmd list. In the subclassed cmd you can just append the namespace.\n\nWith the current code we call `if self._namespace` in the production code for every vtysh command invocation just because we test Frr with namespaces. The tradeoff is that we will not really test the right executor but if the `_execute_vtysh` is split into two (create cmd, execute cmd) the change is very minimal.","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":164,"context_line":"class FrrVtyshExecutor:"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    def __init__(self, namespace: str | None \u003d None):"},{"line_number":167,"context_line":"        self._namespace \u003d namespace"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _execute_vtysh(self, vtysh_args: list[str]) -\u003e str:"},{"line_number":170,"context_line":"        \"\"\"Execute any vtysh command args and return stdout.\"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"a11bb181_703911f1","line":167,"range":{"start_line":167,"start_character":0,"end_line":167,"end_character":35},"in_reply_to":"2ba96f51_9d57a9e6","updated":"2026-06-02 21:27:28.000000000","message":"Yep, I opened comment about it 😉 , Slawek was ok with it, so I closed it\nhttps://review.opendev.org/c/openstack/neutron/+/988158/5..11/neutron/agent/linux/evpn_router/frr/frr_driver.py#b165\n\nBut I can make that change, such that `namespace` is not part of \"production code\"\n\nI implemented my initial idea. See latest update","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":221,"context_line":"        self.cmd_builder \u003d FrrCommandBuilder()"},{"line_number":222,"context_line":"        self.executor \u003d FrrVtyshExecutor(namespace\u003dnamespace)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def _does_bgp_router_exist(self, asn: int, bgp_router_id: str) -\u003e bool:"},{"line_number":225,"context_line":"        try:"},{"line_number":226,"context_line":"            std_out \u003d self.executor.execute_cli_cmd(\u0027show bgp summary json\u0027)"},{"line_number":227,"context_line":"        except frr_exceptions.FrrApplyError as err:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3927c65a_6076c2f7","line":224,"range":{"start_line":224,"start_character":9,"end_line":224,"end_character":13},"updated":"2026-06-02 14:45:57.000000000","message":"supernit: I\u0027d avoid extra word just for the correct grammar - `_bgp_router_exists` and it reads better as \"if bgp router exists\"","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":221,"context_line":"        self.cmd_builder \u003d FrrCommandBuilder()"},{"line_number":222,"context_line":"        self.executor \u003d FrrVtyshExecutor(namespace\u003dnamespace)"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def _does_bgp_router_exist(self, asn: int, bgp_router_id: str) -\u003e bool:"},{"line_number":225,"context_line":"        try:"},{"line_number":226,"context_line":"            std_out \u003d self.executor.execute_cli_cmd(\u0027show bgp summary json\u0027)"},{"line_number":227,"context_line":"        except frr_exceptions.FrrApplyError as err:"}],"source_content_type":"text/x-python","patch_set":11,"id":"1373a744_b476838a","line":224,"range":{"start_line":224,"start_character":9,"end_line":224,"end_character":13},"in_reply_to":"3927c65a_6076c2f7","updated":"2026-06-02 21:27:28.000000000","message":"Acknowledged","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"}],"neutron/agent/linux/evpn_router/frr/templates.py":[{"author":{"_account_id":38298,"name":"Helen Chen","display_name":"Helen Chen","email":"ichen@redhat.com","username":"ingwherchen"},"change_message_id":"5405dde801345dea985fb758173f1ae5ff2d0938","unresolved":true,"context_lines":[{"line_number":103,"context_line":"EVPN_AF_IPV4_UNICAST \u003d \"\"\"\\"},{"line_number":104,"context_line":" address-family ipv4 unicast"},{"line_number":105,"context_line":"  redistribute kernel"},{"line_number":106,"context_line":"  redistribute connected"},{"line_number":107,"context_line":" exit-address-family"},{"line_number":108,"context_line":"\"\"\""},{"line_number":109,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"c0e49424_1eb0da4d","line":106,"range":{"start_line":106,"start_character":2,"end_line":106,"end_character":24},"updated":"2026-05-21 20:18:48.000000000","message":"Is this necessary?","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"be2ac57741cad214fb7ae39d8d4d15200b8b76e5","unresolved":false,"context_lines":[{"line_number":103,"context_line":"EVPN_AF_IPV4_UNICAST \u003d \"\"\"\\"},{"line_number":104,"context_line":" address-family ipv4 unicast"},{"line_number":105,"context_line":"  redistribute kernel"},{"line_number":106,"context_line":"  redistribute connected"},{"line_number":107,"context_line":" exit-address-family"},{"line_number":108,"context_line":"\"\"\""},{"line_number":109,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"b78e12c2_26528785","line":106,"range":{"start_line":106,"start_character":2,"end_line":106,"end_character":24},"in_reply_to":"c0e49424_1eb0da4d","updated":"2026-06-02 01:09:27.000000000","message":"Acknowledged","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":38298,"name":"Helen Chen","display_name":"Helen Chen","email":"ichen@redhat.com","username":"ingwherchen"},"change_message_id":"5405dde801345dea985fb758173f1ae5ff2d0938","unresolved":true,"context_lines":[{"line_number":110,"context_line":"EVPN_AF_IPV6_UNICAST \u003d \"\"\"\\"},{"line_number":111,"context_line":" address-family ipv6 unicast"},{"line_number":112,"context_line":"  redistribute kernel"},{"line_number":113,"context_line":"  redistribute connected"},{"line_number":114,"context_line":" exit-address-family"},{"line_number":115,"context_line":"\"\"\""},{"line_number":116,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"4a3c32d7_a43a38e6","line":113,"range":{"start_line":113,"start_character":2,"end_line":113,"end_character":24},"updated":"2026-05-21 20:18:48.000000000","message":"Is this necessary?","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"be2ac57741cad214fb7ae39d8d4d15200b8b76e5","unresolved":false,"context_lines":[{"line_number":110,"context_line":"EVPN_AF_IPV6_UNICAST \u003d \"\"\"\\"},{"line_number":111,"context_line":" address-family ipv6 unicast"},{"line_number":112,"context_line":"  redistribute kernel"},{"line_number":113,"context_line":"  redistribute connected"},{"line_number":114,"context_line":" exit-address-family"},{"line_number":115,"context_line":"\"\"\""},{"line_number":116,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"791cf0af_ce88c18f","line":113,"range":{"start_line":113,"start_character":2,"end_line":113,"end_character":24},"in_reply_to":"4a3c32d7_a43a38e6","updated":"2026-06-02 01:09:27.000000000","message":"Done","commit_id":"a8fbd7681b2c290f3f06e17b7c06db1735551e1e"}],"neutron/tests/functional/agent/linux/evpn_router/frr/test_frr_driver.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":41,"context_line":"        self._vtep_ip \u003d vtep_ip"},{"line_number":42,"context_line":"        self._dstport \u003d dstport"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def _ns_exec(self, cmd, **kwargs):"},{"line_number":45,"context_line":"        return linux_utils.execute("},{"line_number":46,"context_line":"            [\u0027ip\u0027, \u0027netns\u0027, \u0027exec\u0027, self._namespace] + cmd,"},{"line_number":47,"context_line":"            run_as_root\u003dTrue, **kwargs)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    def _vni(self, vrf_name):"},{"line_number":50,"context_line":"        return int(vrf_name.split(\u0027-\u0027)[-1])"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _bridge_name(self, vni):"},{"line_number":53,"context_line":"        return \u0027br-%d\u0027 % vni"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _vxlan_name(self, vni):"},{"line_number":56,"context_line":"        return \u0027vxlan-%d\u0027 % vni"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _device_exists(self, dev_name):"},{"line_number":59,"context_line":"        _out, std_err \u003d self._ns_exec("},{"line_number":60,"context_line":"            [\u0027ip\u0027, \u0027link\u0027, \u0027show\u0027, dev_name],"},{"line_number":61,"context_line":"            check_exit_code\u003dFalse,"},{"line_number":62,"context_line":"            return_stderr\u003dTrue,"},{"line_number":63,"context_line":"            log_fail_as_error\u003dFalse)"},{"line_number":64,"context_line":"        return not str(std_err).strip()"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def _delete_device(self, dev_name, step):"},{"line_number":67,"context_line":"        try:"},{"line_number":68,"context_line":"            _out, std_err \u003d self._ns_exec("},{"line_number":69,"context_line":"                [\u0027ip\u0027, \u0027link\u0027, \u0027del\u0027, dev_name],"},{"line_number":70,"context_line":"                check_exit_code\u003dFalse,"},{"line_number":71,"context_line":"                return_stderr\u003dTrue,"},{"line_number":72,"context_line":"                log_fail_as_error\u003dFalse)"},{"line_number":73,"context_line":"        except exceptions.ProcessExecutionError as err:"},{"line_number":74,"context_line":"            raise frr_exceptions.FrrVrfError("},{"line_number":75,"context_line":"                \"Failed to delete %s\" % dev_name,"},{"line_number":76,"context_line":"                step\u003dstep,"},{"line_number":77,"context_line":"                cause\u003derr,"},{"line_number":78,"context_line":"            ) from err"},{"line_number":79,"context_line":"        std_err \u003d str(std_err).strip()"},{"line_number":80,"context_line":"        ok \u003d std_err in (\u0027\u0027, \u0027Cannot find device \"%s\"\u0027 % dev_name)"},{"line_number":81,"context_line":"        if not ok:"},{"line_number":82,"context_line":"            raise frr_exceptions.FrrVrfError("},{"line_number":83,"context_line":"                \"Failed to delete %s: %s\" % (dev_name, std_err),"},{"line_number":84,"context_line":"                step\u003dstep,"},{"line_number":85,"context_line":"            )"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def _ensure_vrf_created(self, vrf_name):"},{"line_number":88,"context_line":"        if self._device_exists(vrf_name):"},{"line_number":89,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":11,"id":"83ead871_824d961d","line":86,"range":{"start_line":44,"start_character":0,"end_line":86,"end_character":0},"updated":"2026-06-02 14:45:57.000000000","message":"This is all already implemented in the ip_lib in an OOP manner: https://opendev.org/openstack/neutron/src/branch/master/neutron/agent/linux/ip_lib.py","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        self._vtep_ip \u003d vtep_ip"},{"line_number":42,"context_line":"        self._dstport \u003d dstport"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def _ns_exec(self, cmd, **kwargs):"},{"line_number":45,"context_line":"        return linux_utils.execute("},{"line_number":46,"context_line":"            [\u0027ip\u0027, \u0027netns\u0027, \u0027exec\u0027, self._namespace] + cmd,"},{"line_number":47,"context_line":"            run_as_root\u003dTrue, **kwargs)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    def _vni(self, vrf_name):"},{"line_number":50,"context_line":"        return int(vrf_name.split(\u0027-\u0027)[-1])"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def _bridge_name(self, vni):"},{"line_number":53,"context_line":"        return \u0027br-%d\u0027 % vni"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def _vxlan_name(self, vni):"},{"line_number":56,"context_line":"        return \u0027vxlan-%d\u0027 % vni"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _device_exists(self, dev_name):"},{"line_number":59,"context_line":"        _out, std_err \u003d self._ns_exec("},{"line_number":60,"context_line":"            [\u0027ip\u0027, \u0027link\u0027, \u0027show\u0027, dev_name],"},{"line_number":61,"context_line":"            check_exit_code\u003dFalse,"},{"line_number":62,"context_line":"            return_stderr\u003dTrue,"},{"line_number":63,"context_line":"            log_fail_as_error\u003dFalse)"},{"line_number":64,"context_line":"        return not str(std_err).strip()"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def _delete_device(self, dev_name, step):"},{"line_number":67,"context_line":"        try:"},{"line_number":68,"context_line":"            _out, std_err \u003d self._ns_exec("},{"line_number":69,"context_line":"                [\u0027ip\u0027, \u0027link\u0027, \u0027del\u0027, dev_name],"},{"line_number":70,"context_line":"                check_exit_code\u003dFalse,"},{"line_number":71,"context_line":"                return_stderr\u003dTrue,"},{"line_number":72,"context_line":"                log_fail_as_error\u003dFalse)"},{"line_number":73,"context_line":"        except exceptions.ProcessExecutionError as err:"},{"line_number":74,"context_line":"            raise frr_exceptions.FrrVrfError("},{"line_number":75,"context_line":"                \"Failed to delete %s\" % dev_name,"},{"line_number":76,"context_line":"                step\u003dstep,"},{"line_number":77,"context_line":"                cause\u003derr,"},{"line_number":78,"context_line":"            ) from err"},{"line_number":79,"context_line":"        std_err \u003d str(std_err).strip()"},{"line_number":80,"context_line":"        ok \u003d std_err in (\u0027\u0027, \u0027Cannot find device \"%s\"\u0027 % dev_name)"},{"line_number":81,"context_line":"        if not ok:"},{"line_number":82,"context_line":"            raise frr_exceptions.FrrVrfError("},{"line_number":83,"context_line":"                \"Failed to delete %s: %s\" % (dev_name, std_err),"},{"line_number":84,"context_line":"                step\u003dstep,"},{"line_number":85,"context_line":"            )"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def _ensure_vrf_created(self, vrf_name):"},{"line_number":88,"context_line":"        if self._device_exists(vrf_name):"},{"line_number":89,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":11,"id":"0fab5a2c_259357df","line":86,"range":{"start_line":44,"start_character":0,"end_line":86,"end_character":0},"in_reply_to":"83ead871_824d961d","updated":"2026-06-02 21:27:28.000000000","message":"see my TODO comment in `NamespacedVRFHandler`. ip_lib is missing some things like setting `addrgenmode`. Helen Chen has a solution for it, so I will just follow up with another patch to address this.","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":196,"context_line":""},{"line_number":197,"context_line":"def add_routes_to_route_table(namespace, cidrs, device, table_id):"},{"line_number":198,"context_line":"    for cidr in cidrs:"},{"line_number":199,"context_line":"        ip_lib.add_ip_route(namespace, cidr, device\u003ddevice, table\u003dtable_id)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"def assert_routes(namespace, table_id, present\u003dNone, absent\u003dNone,"}],"source_content_type":"text/x-python","patch_set":11,"id":"a55333e4_9d827ec7","line":199,"updated":"2026-06-02 14:45:57.000000000","message":"I\u0027d add\n```\ntype\u003drtnl.rt_type[\u0027blackhole\u0027], scope\u003d0\n```\nto the parameters - I tested this creates a blackhole type route, which is what the ovn-controller creates.","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":196,"context_line":""},{"line_number":197,"context_line":"def add_routes_to_route_table(namespace, cidrs, device, table_id):"},{"line_number":198,"context_line":"    for cidr in cidrs:"},{"line_number":199,"context_line":"        ip_lib.add_ip_route(namespace, cidr, device\u003ddevice, table\u003dtable_id)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"def assert_routes(namespace, table_id, present\u003dNone, absent\u003dNone,"}],"source_content_type":"text/x-python","patch_set":11,"id":"c3a67ee2_a331b14e","line":199,"in_reply_to":"a55333e4_9d827ec7","updated":"2026-06-02 21:27:28.000000000","message":"Done","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":228,"context_line":"        super().setUp()"},{"line_number":229,"context_line":"        ns \u003d self.useFixture(net_helpers.NamespaceFixture())"},{"line_number":230,"context_line":"        self.namespace \u003d ns.name"},{"line_number":231,"context_line":"        self.useFixture(net_helpers.FrrFixture(namespace\u003dself.namespace))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        vrf_handler \u003d NamespacedVRFHandler(self.namespace)"},{"line_number":234,"context_line":"        self.driver \u003d frr_driver.FrrVtyshDriver("}],"source_content_type":"text/x-python","patch_set":11,"id":"afa734df_e8684833","line":231,"updated":"2026-06-02 14:45:57.000000000","message":"Do we need any waiter for the FRR to come up? Can it happen the frrinit starts the daemons but they are not ready by line 248 is executed?","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        super().setUp()"},{"line_number":229,"context_line":"        ns \u003d self.useFixture(net_helpers.NamespaceFixture())"},{"line_number":230,"context_line":"        self.namespace \u003d ns.name"},{"line_number":231,"context_line":"        self.useFixture(net_helpers.FrrFixture(namespace\u003dself.namespace))"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"        vrf_handler \u003d NamespacedVRFHandler(self.namespace)"},{"line_number":234,"context_line":"        self.driver \u003d frr_driver.FrrVtyshDriver("}],"source_content_type":"text/x-python","patch_set":11,"id":"3c07de7e_5d270f0d","line":231,"in_reply_to":"afa734df_e8684833","updated":"2026-06-02 21:27:28.000000000","message":"frrinit start does block until all enabled daemons are started.\nhttps://github.com/FRRouting/frr/blob/616d8073b173239f922e2986838651420d179c72/tools/frrcommon.sh.in#L156-L195\n\nI would rely on that for now.","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":244,"context_line":"            \u0027show running-config\u0027)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def test_create_evpn_router(self):"},{"line_number":247,"context_line":"        config \u003d make_evpn_config(100)"},{"line_number":248,"context_line":"        self.driver.create_evpn_router(config)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        running_config \u003d self._get_running_config()"}],"source_content_type":"text/x-python","patch_set":11,"id":"07866c2b_8d59f9df","line":247,"updated":"2026-06-02 14:45:57.000000000","message":"nit: it reads better if you do `vni\u003d100`","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":244,"context_line":"            \u0027show running-config\u0027)"},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"    def test_create_evpn_router(self):"},{"line_number":247,"context_line":"        config \u003d make_evpn_config(100)"},{"line_number":248,"context_line":"        self.driver.create_evpn_router(config)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        running_config \u003d self._get_running_config()"}],"source_content_type":"text/x-python","patch_set":11,"id":"e0f939f7_1821ad15","line":247,"in_reply_to":"07866c2b_8d59f9df","updated":"2026-06-02 21:27:28.000000000","message":"Acknowledged","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":308,"context_line":"            self.driver.delete_evpn_router, config)"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"class TestFrrVtyshDriverOperation(base.BaseSudoTestCase):"},{"line_number":312,"context_line":"    \"\"\"Functional tests for FRR EVPN router operation between two peers."},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    Topology::"}],"source_content_type":"text/x-python","patch_set":11,"id":"93df988f_aa432cd4","line":311,"updated":"2026-06-02 14:45:57.000000000","message":"++++ awesome!","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":308,"context_line":"            self.driver.delete_evpn_router, config)"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"class TestFrrVtyshDriverOperation(base.BaseSudoTestCase):"},{"line_number":312,"context_line":"    \"\"\"Functional tests for FRR EVPN router operation between two peers."},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    Topology::"}],"source_content_type":"text/x-python","patch_set":11,"id":"c7628fc7_2c2c62b9","line":311,"in_reply_to":"93df988f_aa432cd4","updated":"2026-06-02 21:27:28.000000000","message":"Acknowledged","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":328,"context_line":"        self.vtep_ip_a \u003d \u002710.0.0.1\u0027"},{"line_number":329,"context_line":"        self.vtep_ip_b \u003d \u002710.0.0.2\u0027"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        ns_a \u003d self.useFixture(net_helpers.NamespaceFixture(\u0027frr-a-\u0027))"},{"line_number":332,"context_line":"        self.ns_a \u003d ns_a.name"},{"line_number":333,"context_line":"        self.useFixture(net_helpers.FrrFixture(namespace\u003dself.ns_a))"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        ns_b \u003d self.useFixture(net_helpers.NamespaceFixture(\u0027frr-b-\u0027))"}],"source_content_type":"text/x-python","patch_set":11,"id":"4b7b7580_2f69b549","line":332,"range":{"start_line":331,"start_character":0,"end_line":332,"end_character":29},"updated":"2026-06-02 14:45:57.000000000","message":"nit: as the `ns_a` is unused, you can assign directly to `self.ns_a \u003d self.useFixture(net_helpers.NamespaceFixture(\u0027frr-a-\u0027)).name`\n\napplies to all fixtures below","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        self.vtep_ip_a \u003d \u002710.0.0.1\u0027"},{"line_number":329,"context_line":"        self.vtep_ip_b \u003d \u002710.0.0.2\u0027"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        ns_a \u003d self.useFixture(net_helpers.NamespaceFixture(\u0027frr-a-\u0027))"},{"line_number":332,"context_line":"        self.ns_a \u003d ns_a.name"},{"line_number":333,"context_line":"        self.useFixture(net_helpers.FrrFixture(namespace\u003dself.ns_a))"},{"line_number":334,"context_line":""},{"line_number":335,"context_line":"        ns_b \u003d self.useFixture(net_helpers.NamespaceFixture(\u0027frr-b-\u0027))"}],"source_content_type":"text/x-python","patch_set":11,"id":"3e33e067_2a8fcd1b","line":332,"range":{"start_line":331,"start_character":0,"end_line":332,"end_character":29},"in_reply_to":"4b7b7580_2f69b549","updated":"2026-06-02 21:27:28.000000000","message":"Acknowledged","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":342,"context_line":"        port_a_fixture \u003d self.useFixture("},{"line_number":343,"context_line":"            net_helpers.LinuxBridgePortFixture(bridge, self.ns_a))"},{"line_number":344,"context_line":"        self.port_a \u003d port_a_fixture.port"},{"line_number":345,"context_line":"        self.port_a.link.set_down()"},{"line_number":346,"context_line":"        self.port_a.link.set_name(\u0027port_a\u0027)"},{"line_number":347,"context_line":"        self.port_a.link.set_up()"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"        port_b_fixture \u003d self.useFixture("},{"line_number":350,"context_line":"            net_helpers.LinuxBridgePortFixture(bridge, self.ns_b))"}],"source_content_type":"text/x-python","patch_set":11,"id":"32620b88_2467dbb3","line":347,"range":{"start_line":345,"start_character":0,"end_line":347,"end_character":33},"updated":"2026-06-02 14:45:57.000000000","message":"what do we need this for?","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":342,"context_line":"        port_a_fixture \u003d self.useFixture("},{"line_number":343,"context_line":"            net_helpers.LinuxBridgePortFixture(bridge, self.ns_a))"},{"line_number":344,"context_line":"        self.port_a \u003d port_a_fixture.port"},{"line_number":345,"context_line":"        self.port_a.link.set_down()"},{"line_number":346,"context_line":"        self.port_a.link.set_name(\u0027port_a\u0027)"},{"line_number":347,"context_line":"        self.port_a.link.set_up()"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"        port_b_fixture \u003d self.useFixture("},{"line_number":350,"context_line":"            net_helpers.LinuxBridgePortFixture(bridge, self.ns_b))"}],"source_content_type":"text/x-python","patch_set":11,"id":"840216b6_7e07cd63","line":347,"range":{"start_line":345,"start_character":0,"end_line":347,"end_character":33},"in_reply_to":"32620b88_2467dbb3","updated":"2026-06-02 21:27:28.000000000","message":"it just set the port to that name. But I can just use the fixture generated name instead. I\u0027ll remove it","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"}],"neutron/tests/unit/agent/linux/evpn_router/frr/test_frr_driver.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":160,"context_line":"        )"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class TestFrrVtyshDriver(base.BaseTestCase):"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def setUp(self):"},{"line_number":166,"context_line":"        super().setUp()"}],"source_content_type":"text/x-python","patch_set":11,"id":"ec8d2ef2_3c494484","line":163,"updated":"2026-06-02 14:45:57.000000000","message":"Does this interact with the system at all? Perhaps the unittest fits better the nature of the tests below?","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"6a92d10b83db13c0554acdc50df76d77b17f818e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        )"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class TestFrrVtyshDriver(base.BaseTestCase):"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def setUp(self):"},{"line_number":166,"context_line":"        super().setUp()"}],"source_content_type":"text/x-python","patch_set":11,"id":"f2dbc941_303538d1","line":163,"in_reply_to":"65b75452_d513e6e1","updated":"2026-06-03 17:32:45.000000000","message":"My bad, I for some reason automatically assumed we\u0027re in `/tests/functional` directory","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        )"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"class TestFrrVtyshDriver(base.BaseTestCase):"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def setUp(self):"},{"line_number":166,"context_line":"        super().setUp()"}],"source_content_type":"text/x-python","patch_set":11,"id":"65b75452_d513e6e1","line":163,"in_reply_to":"ec8d2ef2_3c494484","updated":"2026-06-02 21:27:28.000000000","message":"No it does not actually call into frr. And this is unit test file, notice\n`neutron/tests/unit/agent/linux/evpn_router/frr/test_frr_driver.py`\n\nOr am I not understanding your comment?","commit_id":"8a2ba5abd72ab1e9fad26d0c3113e105c0470bae"}],"tools/configure_for_func_testing.sh":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"398b318e7be51e0a30fcb501c106426bc3b83f12","unresolved":true,"context_lines":[{"line_number":249,"context_line":"    _install_database"},{"line_number":250,"context_line":"    _install_rootwrap_sudoers"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    if [[ \"$INSTALL_FRR\" \u003d\u003d \"True\" ]]; then"},{"line_number":253,"context_line":"        install_package frr"},{"line_number":254,"context_line":"        if is_ubuntu; then"},{"line_number":255,"context_line":"            install_package linux-modules-extra-$(uname -r)"}],"source_content_type":"text/x-sh","patch_set":8,"id":"1de10a71_c551c6cc","line":252,"range":{"start_line":252,"start_character":0,"end_line":252,"end_character":43},"updated":"2026-06-02 14:45:57.000000000","message":"Can we move and re-use the `enable_frr` function instead of creating a new var?\n\nhttps://opendev.org/openstack/neutron/src/branch/master/devstack/plugin.sh#L35","commit_id":"cad7e2d4acd26ccc8fa4f2ee7b2f8975537808fd"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"f635338eb580d652b9a33f68f558fabfe83b0a72","unresolved":false,"context_lines":[{"line_number":249,"context_line":"    _install_database"},{"line_number":250,"context_line":"    _install_rootwrap_sudoers"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    if [[ \"$INSTALL_FRR\" \u003d\u003d \"True\" ]]; then"},{"line_number":253,"context_line":"        install_package frr"},{"line_number":254,"context_line":"        if is_ubuntu; then"},{"line_number":255,"context_line":"            install_package linux-modules-extra-$(uname -r)"}],"source_content_type":"text/x-sh","patch_set":8,"id":"2e62e9e3_15146a3f","line":252,"range":{"start_line":252,"start_character":0,"end_line":252,"end_character":43},"in_reply_to":"1de10a71_c551c6cc","updated":"2026-06-02 21:27:28.000000000","message":"question. Is","commit_id":"cad7e2d4acd26ccc8fa4f2ee7b2f8975537808fd"},{"author":{"_account_id":13861,"name":"yatin","email":"ykarel@redhat.com","username":"yatinkarel"},"change_message_id":"ff2aa92586fe4c6e5c77001fee0a6c3853743c7c","unresolved":true,"context_lines":[{"line_number":25,"context_line":"DATABASE_NAME\u003d${DATABASE_NAME:-openstack_citest}"},{"line_number":26,"context_line":"MEMORY_TRACKER\u003d${MEMORY_TRACKER:-False}"},{"line_number":27,"context_line":"MYSQL_REDUCE_MEMORY\u003d${MYSQL_REDUCE_MEMORY:-True}"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"if [[ \"$IS_GATE\" !\u003d \"True\" ]] \u0026\u0026 [[ \"$#\" -lt 1 ]]; then"},{"line_number":31,"context_line":"    \u003e\u00262 echo \"Usage: $0 /path/to/devstack [-i]"}],"source_content_type":"text/x-sh","patch_set":17,"id":"c4d65b22_2bdbb105","line":28,"updated":"2026-06-08 05:38:52.000000000","message":"nit: can add some default for ENABLE_FRR var explicit here","commit_id":"822c325368622a52707f2449527b39efd662852d"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"cbcfa1698749c84316770d58b9780b35cb90e013","unresolved":false,"context_lines":[{"line_number":25,"context_line":"DATABASE_NAME\u003d${DATABASE_NAME:-openstack_citest}"},{"line_number":26,"context_line":"MEMORY_TRACKER\u003d${MEMORY_TRACKER:-False}"},{"line_number":27,"context_line":"MYSQL_REDUCE_MEMORY\u003d${MYSQL_REDUCE_MEMORY:-True}"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"if [[ \"$IS_GATE\" !\u003d \"True\" ]] \u0026\u0026 [[ \"$#\" -lt 1 ]]; then"},{"line_number":31,"context_line":"    \u003e\u00262 echo \"Usage: $0 /path/to/devstack [-i]"}],"source_content_type":"text/x-sh","patch_set":17,"id":"65ba5775_5cd8b969","line":28,"in_reply_to":"c4d65b22_2bdbb105","updated":"2026-06-08 13:12:29.000000000","message":"Done","commit_id":"822c325368622a52707f2449527b39efd662852d"}]}