)]}'
{"tempest/api/network/admin/test_l3_agent_scheduler.py":[{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"29da17ddc5847fd9a46c662231ced75d5ec59112","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            self.admin_client.add_router_to_l3_agent("},{"line_number":86,"context_line":"                self.agent[\u0027id\u0027],"},{"line_number":87,"context_line":"                router[\u0027router\u0027][\u0027id\u0027])"},{"line_number":88,"context_line":"        else:"},{"line_number":89,"context_line":"            body \u003d self.admin_client.list_l3_agents_hosting_router("},{"line_number":90,"context_line":"                router[\u0027router\u0027][\u0027id\u0027])"},{"line_number":91,"context_line":"        for agent in body[\u0027agents\u0027]:"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa963961_737375b2","line":88,"updated":"2015-03-27 18:04:10.000000000","message":"Aha, yes, this \u0027else\u0027 is redundant.","commit_id":"c1f4384107861ad0f7d4aca28c644e797c1e981e"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"3b675430f0910fdfad4601e2f4d72d64376a8f32","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                router[\u0027router\u0027][\u0027id\u0027])"},{"line_number":88,"context_line":"        else:"},{"line_number":89,"context_line":"            body \u003d self.admin_client.list_l3_agents_hosting_router("},{"line_number":90,"context_line":"                router[\u0027router\u0027][\u0027id\u0027])"},{"line_number":91,"context_line":"        for agent in body[\u0027agents\u0027]:"},{"line_number":92,"context_line":"            l3_agent_ids.append(agent[\u0027id\u0027])"},{"line_number":93,"context_line":"            self.assertIn(\u0027agent_type\u0027, agent)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa963961_065d21e9","line":90,"updated":"2015-03-27 02:53:42.000000000","message":"These new code makes me confusion:\n\n1) why we need run self.admin_client.list_l3_agents_hosting_router twice, if \u0027body\u0027 is valid, the second running should get the same result. right?\n\n2) if \u0027body\u0027 is invalid, in the if-branch, we just fix the router hosting, but not update the \u0027body\u0027, is it safe in the following process?","commit_id":"c1f4384107861ad0f7d4aca28c644e797c1e981e"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"624e9c4096abfebf735f361a9970f39e8243b85c","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                router[\u0027router\u0027][\u0027id\u0027])"},{"line_number":88,"context_line":"        else:"},{"line_number":89,"context_line":"            body \u003d self.admin_client.list_l3_agents_hosting_router("},{"line_number":90,"context_line":"                router[\u0027router\u0027][\u0027id\u0027])"},{"line_number":91,"context_line":"        for agent in body[\u0027agents\u0027]:"},{"line_number":92,"context_line":"            l3_agent_ids.append(agent[\u0027id\u0027])"},{"line_number":93,"context_line":"            self.assertIn(\u0027agent_type\u0027, agent)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa963961_06f0688f","line":90,"in_reply_to":"fa963961_065d21e9","updated":"2015-03-27 17:31:42.000000000","message":"Hi zongKail Li,\nthis is for a work around that causes the \"Icehouse\" code to fail.\nWe have a checken and egg problem in here.\n1. DVR routers from Juno and Kilo require a valid DVR serviceable port before it can be binded to an agent.\n2. So we create a valid port and then assign it to a router when we be bind to an agent it is fine.\n\nIn the case of Juno/Kilo even though if it is already binded it just logs a message saying that the router is already binded and continues to work.\n\nBut in the case of the Icehouse it \"raises\" an error stating that the router has already been binded.\n\nSo in order to solve this issue, if the  router has already been binded we don\u0027t call the \"add_router_to_l3_agent\". If not we call it.\n\nHope this explains.","commit_id":"c1f4384107861ad0f7d4aca28c644e797c1e981e"},{"author":{"_account_id":13667,"name":"ZongKai LI","email":"onionpiece@163.com","username":"lzklibj"},"change_message_id":"cba532eb39c6fa552549114eedaf505cc00be93c","unresolved":false,"context_lines":[{"line_number":87,"context_line":"                router[\u0027router\u0027][\u0027id\u0027])"},{"line_number":88,"context_line":"        else:"},{"line_number":89,"context_line":"            body \u003d self.admin_client.list_l3_agents_hosting_router("},{"line_number":90,"context_line":"                router[\u0027router\u0027][\u0027id\u0027])"},{"line_number":91,"context_line":"        for agent in body[\u0027agents\u0027]:"},{"line_number":92,"context_line":"            l3_agent_ids.append(agent[\u0027id\u0027])"},{"line_number":93,"context_line":"            self.assertIn(\u0027agent_type\u0027, agent)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fa963961_9320399d","line":90,"in_reply_to":"fa963961_06f0688f","updated":"2015-03-27 18:02:25.000000000","message":"Thanks, I get it. :) Good Job!","commit_id":"c1f4384107861ad0f7d4aca28c644e797c1e981e"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a3697b9d0f4cd1754a373c673ff48b47e38b626f","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    @test.attr(type\u003d\u0027smoke\u0027)"},{"line_number":59,"context_line":"    @test.idempotent_id(\u00279464e5e7-8625-49c3-8fd1-89c52be59d66\u0027)"},{"line_number":60,"context_line":"    def test_add_list_remove_router_on_l3_agent(self):"},{"line_number":61,"context_line":"        l3_agent_ids \u003d list()"},{"line_number":62,"context_line":"        # Create a port on a subnet and assign it to the router"},{"line_number":63,"context_line":"        # before binding it to an agent."}],"source_content_type":"text/x-python","patch_set":6,"id":"da9b358b_e3556f1d","line":60,"updated":"2015-03-30 22:43:34.000000000","message":"Albeit I agree with the intention, I think this approach is problematic because it does not solve the fundamental difference between centralized routers and distributed ones.\n\nFor instance in resource_setup we\u0027re listing L3 agents, but in multi-node jobs we\u0027re gonna have a snat_dvr and a dvr one; without selecting the snat_dvr explicitly we might end up failing if we select the DVR one by accident and try to add that to a router (since this is forbidden by design).\n\nThat aside, the test case\u0027s semantic below is for ever altered and we\u0027re bending the rule of black box testing.\n\nAll of this to say that perhaps we need to revise this test case and have it made router type aware. Unfortunately that \n\nThoughts?","commit_id":"095d07573b1aa16244954b915d521390ea13a292"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1dcc4322b96b7df35c2a98f5379ea8714ae9b809","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            msg \u003d \"L3 Agent Scheduler enabled in conf, but L3 Agent not found\""},{"line_number":66,"context_line":"            raise exceptions.InvalidConfiguration(msg)"},{"line_number":67,"context_line":"        cls.router \u003d cls.create_router(data_utils.rand_name(\u0027router\u0027))"},{"line_number":68,"context_line":"        if test.is_extension_enabled(\u0027dvr\u0027, \u0027network\u0027):"},{"line_number":69,"context_line":"            cls.network \u003d cls.create_network()"},{"line_number":70,"context_line":"            cls.create_subnet(cls.network)"},{"line_number":71,"context_line":"            cls.port \u003d cls.create_port(cls.network)"}],"source_content_type":"text/x-python","patch_set":7,"id":"baa041b7_d2db82ec","line":68,"updated":"2015-04-01 20:08:42.000000000","message":"Is it that the extension is enabled that we want to check?  Or, should it be whether the router that was just created is a DVR router?  The dvr extension will be enabled either way but what changes the agent binding logic is that the router is a dvr.","commit_id":"f66baf1a0839a639562fae8885691adfd825e907"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"ab39fe9b9a4827eb402b0a33f03a8bbd950bfed0","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            msg \u003d \"L3 Agent Scheduler enabled in conf, but L3 Agent not found\""},{"line_number":66,"context_line":"            raise exceptions.InvalidConfiguration(msg)"},{"line_number":67,"context_line":"        cls.router \u003d cls.create_router(data_utils.rand_name(\u0027router\u0027))"},{"line_number":68,"context_line":"        if test.is_extension_enabled(\u0027dvr\u0027, \u0027network\u0027):"},{"line_number":69,"context_line":"            cls.network \u003d cls.create_network()"},{"line_number":70,"context_line":"            cls.create_subnet(cls.network)"},{"line_number":71,"context_line":"            cls.port \u003d cls.create_port(cls.network)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9aa53dc9_8f7c8a73","line":68,"in_reply_to":"9aa53dc9_8cf3780a","updated":"2015-04-07 16:00:56.000000000","message":"Ok, after talking to you on irc I understand the intent here a bit more, but I think we need an inline comment here to explain what is going on, because this logic is very opaque to anyone reading the code. (which is the first step for debugging a test failure)","commit_id":"f66baf1a0839a639562fae8885691adfd825e907"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"64371701f9c6ee488400f1751fc24b2933a55575","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            msg \u003d \"L3 Agent Scheduler enabled in conf, but L3 Agent not found\""},{"line_number":66,"context_line":"            raise exceptions.InvalidConfiguration(msg)"},{"line_number":67,"context_line":"        cls.router \u003d cls.create_router(data_utils.rand_name(\u0027router\u0027))"},{"line_number":68,"context_line":"        if test.is_extension_enabled(\u0027dvr\u0027, \u0027network\u0027):"},{"line_number":69,"context_line":"            cls.network \u003d cls.create_network()"},{"line_number":70,"context_line":"            cls.create_subnet(cls.network)"},{"line_number":71,"context_line":"            cls.port \u003d cls.create_port(cls.network)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9aa53dc9_45278312","line":68,"in_reply_to":"9aa53dc9_8f7c8a73","updated":"2015-04-07 16:42:55.000000000","message":"Done","commit_id":"f66baf1a0839a639562fae8885691adfd825e907"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7d4683e24178307cd8d403bd7f81e4937857e516","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            msg \u003d \"L3 Agent Scheduler enabled in conf, but L3 Agent not found\""},{"line_number":66,"context_line":"            raise exceptions.InvalidConfiguration(msg)"},{"line_number":67,"context_line":"        cls.router \u003d cls.create_router(data_utils.rand_name(\u0027router\u0027))"},{"line_number":68,"context_line":"        if test.is_extension_enabled(\u0027dvr\u0027, \u0027network\u0027):"},{"line_number":69,"context_line":"            cls.network \u003d cls.create_network()"},{"line_number":70,"context_line":"            cls.create_subnet(cls.network)"},{"line_number":71,"context_line":"            cls.port \u003d cls.create_port(cls.network)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9aa53dc9_8cf3780a","line":68,"in_reply_to":"9aa53dc9_996df806","updated":"2015-04-07 15:48:32.000000000","message":"The attribute exists (but only when queried as admin), and available in the show response only from Juno onward. As mentioned, we could use \n\nis_distributed \u003d cls.admin_client.show_router(cls.router[\u0027id\u0027]).get(\u0027distributed\u0027, False)\n\nAnd test on the attribute; if you think it\u0027s better, then it should be a trivial change. I am always confused on to which approach to use, static vs dynamic and I erred on the side of caution.","commit_id":"f66baf1a0839a639562fae8885691adfd825e907"},{"author":{"_account_id":5196,"name":"Matthew Treinish","email":"mtreinish@kortar.org","username":"treinish"},"change_message_id":"b13e6a0e34887e67af8defe9a30d841ddf85386c","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            msg \u003d \"L3 Agent Scheduler enabled in conf, but L3 Agent not found\""},{"line_number":66,"context_line":"            raise exceptions.InvalidConfiguration(msg)"},{"line_number":67,"context_line":"        cls.router \u003d cls.create_router(data_utils.rand_name(\u0027router\u0027))"},{"line_number":68,"context_line":"        if test.is_extension_enabled(\u0027dvr\u0027, \u0027network\u0027):"},{"line_number":69,"context_line":"            cls.network \u003d cls.create_network()"},{"line_number":70,"context_line":"            cls.create_subnet(cls.network)"},{"line_number":71,"context_line":"            cls.port \u003d cls.create_port(cls.network)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9aa53dc9_996df806","line":68,"in_reply_to":"baa041b7_06208c9a","updated":"2015-04-07 15:40:13.000000000","message":"Well can the distributed attribute exist but not be used? I agree we should use config where possible for things, but if this is both a static config and a dynamic thing we should query the api too.","commit_id":"f66baf1a0839a639562fae8885691adfd825e907"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"0bedd1acd4418137114e862511862f93695bb674","unresolved":false,"context_lines":[{"line_number":65,"context_line":"            msg \u003d \"L3 Agent Scheduler enabled in conf, but L3 Agent not found\""},{"line_number":66,"context_line":"            raise exceptions.InvalidConfiguration(msg)"},{"line_number":67,"context_line":"        cls.router \u003d cls.create_router(data_utils.rand_name(\u0027router\u0027))"},{"line_number":68,"context_line":"        if test.is_extension_enabled(\u0027dvr\u0027, \u0027network\u0027):"},{"line_number":69,"context_line":"            cls.network \u003d cls.create_network()"},{"line_number":70,"context_line":"            cls.create_subnet(cls.network)"},{"line_number":71,"context_line":"            cls.port \u003d cls.create_port(cls.network)"}],"source_content_type":"text/x-python","patch_set":7,"id":"baa041b7_06208c9a","line":68,"in_reply_to":"baa041b7_d2db82ec","updated":"2015-04-01 21:44:29.000000000","message":"No, this is intentional, and it is done to avoid affecting the test outcome for those backends that do not support DVR.\n\nThe other approach would entail something like this:\n\n is_distributed \u003d cls.admin_client.show_router(cls.router[\u0027id\u0027]).get(\u0027distributed\u0027, False)\nif is_distributed:\n  # do extra setup.\n\nSince the presence of the router attribute is predicated on the existence of the extension, and since the approach in Tempest is usually to drive test behavior from static configuration rather than the dynamics enabled in the backend, I chose the former approach. Make sense?","commit_id":"f66baf1a0839a639562fae8885691adfd825e907"}]}