)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":24245,"name":"Harald Jensås","email":"hjensas@redhat.com","username":"harald.jensas"},"change_message_id":"82fe35142f207518e435941a4cfaa3d915b77211","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Remove vifs upon teardown"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Since we removed the ability for nova to cleanly remove"},{"line_number":10,"context_line":"the vif during teardown because that created created a race"},{"line_number":11,"context_line":"condition, the removal of all vif attachment records only"},{"line_number":12,"context_line":"seems to be the right thing to do since we can\u0027t realistically"},{"line_number":13,"context_line":"change nova try harder, and functionally we are otherwise"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bf659307_72b88ec7","line":10,"range":{"start_line":10,"start_character":45,"end_line":10,"end_character":53},"updated":"2018-03-27 12:56:25.000000000","message":"s/created//","commit_id":"68e16ed27900c81ca37f3c63170b1a0f54390e2b"}],"ironic/common/network.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c7ad34a9acc0a73a2e0610fac1e7266fd347172c","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    \"\"\""},{"line_number":122,"context_line":"    vifs \u003d task.driver.network.vif_list(task)"},{"line_number":123,"context_line":"    for vif_entry in vifs:"},{"line_number":124,"context_line":"        if \u0027id\u0027 in vif_entry:"},{"line_number":125,"context_line":"            vif \u003d vif_entry[\u0027id\u0027]"},{"line_number":126,"context_line":"        else:"},{"line_number":127,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_ef0f2c2b","line":124,"updated":"2018-04-04 13:30:57.000000000","message":"I don\u0027t quite get this part. If vif_entry does not have \u0027id\u0027, then what is it? If it\u0027s just the ID itself, then using \u0027in\u0027 on it is a bit confusing. Did you essentially mean\n\n if isinstance(vif_entry, dict)","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e1bc342bdd3d3652615cf0965e92b898aa625ef","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    \"\"\""},{"line_number":122,"context_line":"    vifs \u003d task.driver.network.vif_list(task)"},{"line_number":123,"context_line":"    for vif_entry in vifs:"},{"line_number":124,"context_line":"        if \u0027id\u0027 in vif_entry:"},{"line_number":125,"context_line":"            vif \u003d vif_entry[\u0027id\u0027]"},{"line_number":126,"context_line":"        else:"},{"line_number":127,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_795f5172","line":124,"in_reply_to":"bf659307_50f58282","updated":"2018-04-09 14:12:52.000000000","message":"Done","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":6637,"name":"Sam Betts","email":"sam@code-smash.net","username":"sambetts"},"change_message_id":"2ede2e993ac4569059380fde49ea8edec19415d7","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    \"\"\""},{"line_number":122,"context_line":"    vifs \u003d task.driver.network.vif_list(task)"},{"line_number":123,"context_line":"    for vif_entry in vifs:"},{"line_number":124,"context_line":"        if \u0027id\u0027 in vif_entry:"},{"line_number":125,"context_line":"            vif \u003d vif_entry[\u0027id\u0027]"},{"line_number":126,"context_line":"        else:"},{"line_number":127,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_50f58282","line":124,"in_reply_to":"bf659307_6d8d5d29","updated":"2018-04-05 09:28:41.000000000","message":"The API for vif detach is that it just takes just the VIF ID, and vif_list should be returning a dict with each entry containing an ID otherwise it will break the API. With those two rules in mind I personally think if we want to be safe about it we should be doing something like:\n\n vif_id \u003d vif_entry.get(\u0027id\u0027)\n if not vif_id:\n     LOG.warn(\"Unable to remove VIF because it has no ID\")\n     continue\n\nBut IMO if the network drivers are implemented correctly, then vif_entry[\u0027id\u0027] *should* always work, and we can expect there to be key error thrown if someone loads a broken network driver.","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"f33b29d498ac9ba6d1cffc6aaa542b2a2191451c","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    \"\"\""},{"line_number":122,"context_line":"    vifs \u003d task.driver.network.vif_list(task)"},{"line_number":123,"context_line":"    for vif_entry in vifs:"},{"line_number":124,"context_line":"        if \u0027id\u0027 in vif_entry:"},{"line_number":125,"context_line":"            vif \u003d vif_entry[\u0027id\u0027]"},{"line_number":126,"context_line":"        else:"},{"line_number":127,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_6d8d5d29","line":124,"in_reply_to":"bf659307_ef0f2c2b","updated":"2018-04-04 20:18:00.000000000","message":"if it doesn\u0027t have an id, then it is likely a unit test.\n\nJust checking if it is a dictionary is insufficent because there would then be a keyerror otherwise. I mean, we could just catch keyerror and pass in that event...","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c7ad34a9acc0a73a2e0610fac1e7266fd347172c","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            LOG.warning(\u0027While removing records of VIF attachments from node \u0027"},{"line_number":132,"context_line":"                        \u0027%(node)s, we recieved indication that %(vif)s is \u0027"},{"line_number":133,"context_line":"                        \u0027no longer attached. There should not happen under \u0027"},{"line_number":134,"context_line":"                        \u0027 normal circumstances.\u0027,"},{"line_number":135,"context_line":"                        {\u0027node\u0027: task.node.uuid,"},{"line_number":136,"context_line":"                         \u0027vif\u0027: vif})"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_8f1a306c","line":134,"updated":"2018-04-04 13:30:57.000000000","message":"nit: redundant space before \u0027normal\u0027","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e1bc342bdd3d3652615cf0965e92b898aa625ef","unresolved":false,"context_lines":[{"line_number":131,"context_line":"            LOG.warning(\u0027While removing records of VIF attachments from node \u0027"},{"line_number":132,"context_line":"                        \u0027%(node)s, we recieved indication that %(vif)s is \u0027"},{"line_number":133,"context_line":"                        \u0027no longer attached. There should not happen under \u0027"},{"line_number":134,"context_line":"                        \u0027 normal circumstances.\u0027,"},{"line_number":135,"context_line":"                        {\u0027node\u0027: task.node.uuid,"},{"line_number":136,"context_line":"                         \u0027vif\u0027: vif})"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_3951d963","line":134,"in_reply_to":"bf659307_8f1a306c","updated":"2018-04-09 14:12:52.000000000","message":"Done","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c7ad34a9acc0a73a2e0610fac1e7266fd347172c","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        except exception.NetworkError as e:"},{"line_number":139,"context_line":"            LOG.error(\u0027An error has been encountered while removing all \u0027"},{"line_number":140,"context_line":"                      \u0027VIF records for  %(node)s. Error: %(error)s\u0027,"},{"line_number":141,"context_line":"                      {\u0027node\u0027: task.node.uuid,"},{"line_number":142,"context_line":"                       \u0027error\u0027: e.message})"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_af15b43e","line":140,"updated":"2018-04-04 13:30:57.000000000","message":"nit: redundant space before %(node)s","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e1bc342bdd3d3652615cf0965e92b898aa625ef","unresolved":false,"context_lines":[{"line_number":137,"context_line":""},{"line_number":138,"context_line":"        except exception.NetworkError as e:"},{"line_number":139,"context_line":"            LOG.error(\u0027An error has been encountered while removing all \u0027"},{"line_number":140,"context_line":"                      \u0027VIF records for  %(node)s. Error: %(error)s\u0027,"},{"line_number":141,"context_line":"                      {\u0027node\u0027: task.node.uuid,"},{"line_number":142,"context_line":"                       \u0027error\u0027: e.message})"}],"source_content_type":"text/x-python","patch_set":3,"id":"bf659307_59cfed28","line":140,"in_reply_to":"bf659307_af15b43e","updated":"2018-04-09 14:12:52.000000000","message":"Done","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a06036c2230c0fadd37494c18728cfb82b8bd73e","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    \"\"\""},{"line_number":122,"context_line":"    vifs \u003d task.driver.network.vif_list(task)"},{"line_number":123,"context_line":"    for vif_entry in vifs:"},{"line_number":124,"context_line":"        vif \u003d vif_entry.get(\u0027id\u0027, None)"},{"line_number":125,"context_line":"        if not vif:"},{"line_number":126,"context_line":"            LOG.warning(\u0027Incorrect vif entry for %(node)s lacks an ID field, \u0027"},{"line_number":127,"context_line":"                        \u0027and is thus unsupported.\u0027, {\u0027node\u0027: task.node.uuid})"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf659307_c42eb4f9","line":124,"updated":"2018-04-05 12:52:41.000000000","message":"nit: \"None\" is the default already","commit_id":"c5f192f46e179e8cc337a299fcc57d852b668ad8"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a06036c2230c0fadd37494c18728cfb82b8bd73e","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        vif \u003d vif_entry.get(\u0027id\u0027, None)"},{"line_number":125,"context_line":"        if not vif:"},{"line_number":126,"context_line":"            LOG.warning(\u0027Incorrect vif entry for %(node)s lacks an ID field, \u0027"},{"line_number":127,"context_line":"                        \u0027and is thus unsupported.\u0027, {\u0027node\u0027: task.node.uuid})"},{"line_number":128,"context_line":"            continue"},{"line_number":129,"context_line":"            vif \u003d vif_entry[\u0027id\u0027]"},{"line_number":130,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf659307_64332864","line":127,"updated":"2018-04-05 12:52:41.000000000","message":"nit: log the whole entry?","commit_id":"c5f192f46e179e8cc337a299fcc57d852b668ad8"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a06036c2230c0fadd37494c18728cfb82b8bd73e","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            LOG.warning(\u0027Incorrect vif entry for %(node)s lacks an ID field, \u0027"},{"line_number":127,"context_line":"                        \u0027and is thus unsupported.\u0027, {\u0027node\u0027: task.node.uuid})"},{"line_number":128,"context_line":"            continue"},{"line_number":129,"context_line":"            vif \u003d vif_entry[\u0027id\u0027]"},{"line_number":130,"context_line":"        try:"},{"line_number":131,"context_line":"            task.driver.network.vif_detach(task, vif)"},{"line_number":132,"context_line":"        except exception.VifNotAttached as e:"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf659307_8438bc3d","line":129,"updated":"2018-04-05 12:52:41.000000000","message":"code after continue is unreachable. remove?","commit_id":"c5f192f46e179e8cc337a299fcc57d852b668ad8"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e1bc342bdd3d3652615cf0965e92b898aa625ef","unresolved":false,"context_lines":[{"line_number":126,"context_line":"            LOG.warning(\u0027Incorrect vif entry for %(node)s lacks an ID field, \u0027"},{"line_number":127,"context_line":"                        \u0027and is thus unsupported.\u0027, {\u0027node\u0027: task.node.uuid})"},{"line_number":128,"context_line":"            continue"},{"line_number":129,"context_line":"            vif \u003d vif_entry[\u0027id\u0027]"},{"line_number":130,"context_line":"        try:"},{"line_number":131,"context_line":"            task.driver.network.vif_detach(task, vif)"},{"line_number":132,"context_line":"        except exception.VifNotAttached as e:"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf659307_44686420","line":129,"in_reply_to":"bf659307_8438bc3d","updated":"2018-04-09 14:12:52.000000000","message":"Well, that is if there is a failure to match, the rest will actually trigger the detach.","commit_id":"c5f192f46e179e8cc337a299fcc57d852b668ad8"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"58c0de9b0378358048e0c34452d3661c2b8fd099","unresolved":false,"context_lines":[{"line_number":142,"context_line":"            LOG.error(\u0027An error has been encountered while removing all \u0027"},{"line_number":143,"context_line":"                      \u0027VIF records for %(node)s. Error: %(error)s\u0027,"},{"line_number":144,"context_line":"                      {\u0027node\u0027: task.node.uuid,"},{"line_number":145,"context_line":"                       \u0027error\u0027: e.message})"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_4053f417","line":145,"updated":"2018-04-11 12:09:45.000000000","message":"This isn\u0027t all, this is a single vif. Also, we should probably log which vif failed to remove.","commit_id":"f55ec740c6c65494b22bf428478c8b662085261b"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"4b10e122ba1c7e0efe560d94a9cf2b76f4ab672c","unresolved":false,"context_lines":[{"line_number":142,"context_line":"            LOG.error(\u0027An error has been encountered while removing all \u0027"},{"line_number":143,"context_line":"                      \u0027VIF records for %(node)s. Error: %(error)s\u0027,"},{"line_number":144,"context_line":"                      {\u0027node\u0027: task.node.uuid,"},{"line_number":145,"context_line":"                       \u0027error\u0027: e.message})"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_5052d73c","line":145,"in_reply_to":"bf659307_4053f417","updated":"2018-04-11 18:53:32.000000000","message":"Good point, I\u0027ll update the text. The exception that is logged from unbind_neutron_port which would include the vif, so I\u0027m not sure there is value in double logging.","commit_id":"f55ec740c6c65494b22bf428478c8b662085261b"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"adda08e236b23857bb6b8dd5c5e645f177bea77a","unresolved":false,"context_lines":[{"line_number":132,"context_line":"            task.driver.network.vif_detach(task, vif)"},{"line_number":133,"context_line":"        except exception.VifNotAttached as e:"},{"line_number":134,"context_line":"            LOG.warning(\u0027While removing records of VIF attachments from node \u0027"},{"line_number":135,"context_line":"                        \u0027%(node)s, we recieved indication that %(vif)s is \u0027"},{"line_number":136,"context_line":"                        \u0027no longer attached. There should not happen under \u0027"},{"line_number":137,"context_line":"                        \u0027normal circumstances.\u0027,"},{"line_number":138,"context_line":"                        {\u0027node\u0027: task.node.uuid,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9f6a8fd7_0e5fbb0e","line":135,"updated":"2018-04-18 03:06:40.000000000","message":"s/recieved/received/","commit_id":"3a4e259a3714cedcad567e229e983f1b559c2668"}],"ironic/conductor/manager.py":[{"author":{"_account_id":6637,"name":"Sam Betts","email":"sam@code-smash.net","username":"sambetts"},"change_message_id":"0a7a14eba59082ff0f44df13f206545c44ff3573","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            driver_internal_info \u003d node.driver_internal_info"},{"line_number":929,"context_line":"            driver_internal_info.pop(\u0027instance\u0027, None)"},{"line_number":930,"context_line":"            node.driver_internal_info \u003d driver_internal_info"},{"line_number":931,"context_line":"            task.driver.network.clear_vif_attachments(task)"},{"line_number":932,"context_line":"            node.save()"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":"        # Begin cleaning"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf659307_34cbc21e","line":931,"updated":"2018-03-28 10:46:47.000000000","message":"This should be calling vif_detach for each vif that is currently attached to this node, detaching a vif often has more actions than just clearing the the record we store about it.","commit_id":"9715f80915c124b47c4463d7af4d505c5bedab9b"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"ed529b6db50a43c470b2d3e2a13bde278e5eb929","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            driver_internal_info \u003d node.driver_internal_info"},{"line_number":929,"context_line":"            driver_internal_info.pop(\u0027instance\u0027, None)"},{"line_number":930,"context_line":"            node.driver_internal_info \u003d driver_internal_info"},{"line_number":931,"context_line":"            task.driver.network.clear_vif_attachments(task)"},{"line_number":932,"context_line":"            node.save()"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":"        # Begin cleaning"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf659307_83db72eb","line":931,"in_reply_to":"bf659307_34cbc21e","updated":"2018-03-28 14:08:45.000000000","message":"I disagree. This is post clean-up before we\u0027re going into cleaning. Where as as vif_detach orchestrates a neutron disconnect.\n\nI don\u0027t think we need to do that since the tear down process calls unconfigure_tenant_networks. We could put conditional logic deep down inside of unconfigure tenant networks, but this is the end of life of the deployed node.","commit_id":"9715f80915c124b47c4463d7af4d505c5bedab9b"},{"author":{"_account_id":6637,"name":"Sam Betts","email":"sam@code-smash.net","username":"sambetts"},"change_message_id":"5e67a9b78f6981413be7c5a98e347cbd6b5462ca","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            driver_internal_info \u003d node.driver_internal_info"},{"line_number":929,"context_line":"            driver_internal_info.pop(\u0027instance\u0027, None)"},{"line_number":930,"context_line":"            node.driver_internal_info \u003d driver_internal_info"},{"line_number":931,"context_line":"            task.driver.network.clear_vif_attachments(task)"},{"line_number":932,"context_line":"            node.save()"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":"        # Begin cleaning"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf659307_bd93195b","line":931,"in_reply_to":"bf659307_3f55d21d","updated":"2018-03-29 09:20:07.000000000","message":"Yup I think that proposal fits in with what I was looking for, only change I would make is I think the neutron and flat network interfaces should try to unbind the neutron port regardless of the node\u0027s state to make sure we catch all scenarios we\u0027ve not thought about.","commit_id":"9715f80915c124b47c4463d7af4d505c5bedab9b"},{"author":{"_account_id":6637,"name":"Sam Betts","email":"sam@code-smash.net","username":"sambetts"},"change_message_id":"f292333ca00902e61e93f118e3590822d071c64d","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            driver_internal_info \u003d node.driver_internal_info"},{"line_number":929,"context_line":"            driver_internal_info.pop(\u0027instance\u0027, None)"},{"line_number":930,"context_line":"            node.driver_internal_info \u003d driver_internal_info"},{"line_number":931,"context_line":"            task.driver.network.clear_vif_attachments(task)"},{"line_number":932,"context_line":"            node.save()"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":"        # Begin cleaning"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf659307_b57ee968","line":931,"in_reply_to":"bf659307_83db72eb","updated":"2018-03-28 14:18:00.000000000","message":"vif detach in the neutron driver is doing a neutron disconnect to handle the case of a vif detach with an active node, but that is a network driver implementation detail.\n\nThe network interface API specifies vif detach is called to detach a vif, which is different from unbinding a vif from the node which is what unconfigure_tenant_networks does.\n\nAn example of this is the case of certain hardware that can support a dynamic number of vnics. In this case when a vif is attached or detached, calls can be made to the hardware to add or remove vnic from the hardware. This current patch will circumvent / require the network driver re implement that logic.","commit_id":"9715f80915c124b47c4463d7af4d505c5bedab9b"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"b7b681c5c7914478264b3184970b78750d601d1e","unresolved":false,"context_lines":[{"line_number":928,"context_line":"            driver_internal_info \u003d node.driver_internal_info"},{"line_number":929,"context_line":"            driver_internal_info.pop(\u0027instance\u0027, None)"},{"line_number":930,"context_line":"            node.driver_internal_info \u003d driver_internal_info"},{"line_number":931,"context_line":"            task.driver.network.clear_vif_attachments(task)"},{"line_number":932,"context_line":"            node.save()"},{"line_number":933,"context_line":""},{"line_number":934,"context_line":"        # Begin cleaning"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf659307_3f55d21d","line":931,"in_reply_to":"bf659307_b57ee968","updated":"2018-03-28 17:38:22.000000000","message":"So, lets start over, because this is even more complex than we\u0027re both arguing.\n\n1) the neutron network interface will perform an unbind\n2) the flat network interface just does pass.\n3) vif_detach, which requires the vif_id to be known for the call\n4) vif_detach _only_ unbinds the port if the node state is active.\n5) The node state is not active at this point.\n6) The clear_vif_attachments code merely does the same thing as vif_detach except it enumerates through all ports.\n\nSo, a proposal:\n\n1) We create a helper outside of the network interface mdoel that collects all vif records, and calls vif_detach.\n2) We update vif_detach to attempt to unbind the neutron port while the node is in DELETING state, with try/catch logic to handle the fact that the port may already be gone.","commit_id":"9715f80915c124b47c4463d7af4d505c5bedab9b"}],"ironic/drivers/modules/network/common.py":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"86a24f09d293a52b3d56ff4fd492aec1f2050013","unresolved":false,"context_lines":[{"line_number":377,"context_line":"                p_obj.internal_info.get(\u0027rescuing_vif_port_id\u0027) or"},{"line_number":378,"context_line":"                self._get_vif_id_by_port_like_obj(p_obj) or None)"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    def clear_vif_attachments(self, task):"},{"line_number":381,"context_line":"        \"\"\"Remove all vif ports from a node."},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"        :param task: A TaskManager instance."}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_ec98481c","line":380,"updated":"2018-03-27 12:31:52.000000000","message":"Let\u0027s also add an empty clear_vif_attechments call to the base NetworkInterface, so that out-of-tree drivers don\u0027t cause AttributeError. Maybe include a warning there, something like \"Clearing VIF attachments is not implemented for network interface %(iface)s, orphaned VIFs may be left on node %(node)s\"","commit_id":"68e16ed27900c81ca37f3c63170b1a0f54390e2b"},{"author":{"_account_id":360,"name":"Dan Prince","email":"dprince@redhat.com","username":"dan-prince"},"change_message_id":"63f04aca889e09e0eb59effa2258c02f404dd980","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        :param task: A TaskManager instance."},{"line_number":384,"context_line":"        \"\"\""},{"line_number":385,"context_line":"        for port_like_obj in task.portgroups + task.ports:"},{"line_number":386,"context_line":"            self._clear_vif_from_port_like_obj(port_like_obj)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"class NeutronVIFPortIDMixin(VIFPortIDMixin):"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_95bf4ccf","line":386,"updated":"2018-03-27 13:33:00.000000000","message":"Any chance for exception here? Do we need to try/except around this?","commit_id":"68e16ed27900c81ca37f3c63170b1a0f54390e2b"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"04e234a45ddc556c55293645422e096bf5bb2e99","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        :param task: A TaskManager instance."},{"line_number":384,"context_line":"        \"\"\""},{"line_number":385,"context_line":"        for port_like_obj in task.portgroups + task.ports:"},{"line_number":386,"context_line":"            self._clear_vif_from_port_like_obj(port_like_obj)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"class NeutronVIFPortIDMixin(VIFPortIDMixin):"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_620aee8a","line":386,"in_reply_to":"bf659307_7514f8a0","updated":"2018-03-27 23:49:51.000000000","message":"I think the only chance of an exception is upon the object save, which would only be possible if someone manually deleted the port/portgroup from the database. If performed via the API, the delete would be blocked due to the node lock as the task is not shared.\n\ntl;dr I\u0027m not convinced it is necessary.","commit_id":"68e16ed27900c81ca37f3c63170b1a0f54390e2b"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"6ffcbdd5a2a6d335b4a47cc7de3892bb2f0f1a32","unresolved":false,"context_lines":[{"line_number":383,"context_line":"        :param task: A TaskManager instance."},{"line_number":384,"context_line":"        \"\"\""},{"line_number":385,"context_line":"        for port_like_obj in task.portgroups + task.ports:"},{"line_number":386,"context_line":"            self._clear_vif_from_port_like_obj(port_like_obj)"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"class NeutronVIFPortIDMixin(VIFPortIDMixin):"}],"source_content_type":"text/x-python","patch_set":1,"id":"bf659307_7514f8a0","line":386,"in_reply_to":"bf659307_95bf4ccf","updated":"2018-03-27 13:43:09.000000000","message":"++","commit_id":"68e16ed27900c81ca37f3c63170b1a0f54390e2b"}],"ironic/tests/unit/common/test_network.py":[{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"58c0de9b0378358048e0c34452d3661c2b8fd099","unresolved":false,"context_lines":[{"line_number":169,"context_line":"        with task_manager.acquire(self.context, self.node.uuid) as task:"},{"line_number":170,"context_line":"            result \u003d network.get_node_vif_ids(task)"},{"line_number":171,"context_line":"        self.assertEqual({}, result[\u0027ports\u0027])"},{"line_number":172,"context_line":"        self.assertEqual({}, result[\u0027portgroups\u0027])"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"class GetPortgroupByIdTestCase(db_base.DbTestCase):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_00a73c0d","line":172,"updated":"2018-04-11 12:09:45.000000000","message":"Can we test the invalid/errored cases as well? I imagine a case with two vifs where the first is invalid/fails and we ensure the second is still removed.","commit_id":"f55ec740c6c65494b22bf428478c8b662085261b"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"4b10e122ba1c7e0efe560d94a9cf2b76f4ab672c","unresolved":false,"context_lines":[{"line_number":169,"context_line":"        with task_manager.acquire(self.context, self.node.uuid) as task:"},{"line_number":170,"context_line":"            result \u003d network.get_node_vif_ids(task)"},{"line_number":171,"context_line":"        self.assertEqual({}, result[\u0027ports\u0027])"},{"line_number":172,"context_line":"        self.assertEqual({}, result[\u0027portgroups\u0027])"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"class GetPortgroupByIdTestCase(db_base.DbTestCase):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_6b3290f4","line":172,"in_reply_to":"bf659307_00a73c0d","updated":"2018-04-11 18:53:32.000000000","message":"Make sense.","commit_id":"f55ec740c6c65494b22bf428478c8b662085261b"},{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"19db339d2ae4eb5a003eff1f06471f101ee75935","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        db_utils.create_test_portgroup("},{"line_number":193,"context_line":"            node_id\u003dself.node.id, address\u003d\u0027dd:ee:ff:aa:bb:cc\u0027,"},{"line_number":194,"context_line":"            internal_info\u003d{driver_common.TENANT_VIF_KEY: \u0027test-vif-B\u0027})"},{"line_number":195,"context_line":"        print(self.node)"},{"line_number":196,"context_line":"        mock_unbind.side_effect \u003d [exception.NetworkError, None]"},{"line_number":197,"context_line":"        with task_manager.acquire(self.context, self.node.uuid) as task:"},{"line_number":198,"context_line":"            network.remove_vifs_from_node(task)"}],"source_content_type":"text/x-python","patch_set":6,"id":"bf659307_55d2b75d","line":195,"updated":"2018-04-12 08:05:41.000000000","message":"I wonder if we should remove it because usually print() is not used in unit tests.","commit_id":"9416f4a5e46be1ec417a18605e27da09c7796529"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"945d416a200b96bdec794be8b6497048c7f476ae","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        db_utils.create_test_portgroup("},{"line_number":193,"context_line":"            node_id\u003dself.node.id, address\u003d\u0027dd:ee:ff:aa:bb:cc\u0027,"},{"line_number":194,"context_line":"            internal_info\u003d{driver_common.TENANT_VIF_KEY: \u0027test-vif-B\u0027})"},{"line_number":195,"context_line":"        print(self.node)"},{"line_number":196,"context_line":"        mock_unbind.side_effect \u003d [exception.NetworkError, None]"},{"line_number":197,"context_line":"        with task_manager.acquire(self.context, self.node.uuid) as task:"},{"line_number":198,"context_line":"            network.remove_vifs_from_node(task)"}],"source_content_type":"text/x-python","patch_set":6,"id":"bf659307_996b97b5","line":195,"in_reply_to":"bf659307_2a98fdbd","updated":"2018-04-12 20:43:02.000000000","message":"Doh, thanks! I put it in while troubleshooting some unit test issues while I was writing the test and forgot abou tit.","commit_id":"9416f4a5e46be1ec417a18605e27da09c7796529"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"ad76320f7762b9898444146c9fa614ef22ba2fa3","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        db_utils.create_test_portgroup("},{"line_number":193,"context_line":"            node_id\u003dself.node.id, address\u003d\u0027dd:ee:ff:aa:bb:cc\u0027,"},{"line_number":194,"context_line":"            internal_info\u003d{driver_common.TENANT_VIF_KEY: \u0027test-vif-B\u0027})"},{"line_number":195,"context_line":"        print(self.node)"},{"line_number":196,"context_line":"        mock_unbind.side_effect \u003d [exception.NetworkError, None]"},{"line_number":197,"context_line":"        with task_manager.acquire(self.context, self.node.uuid) as task:"},{"line_number":198,"context_line":"            network.remove_vifs_from_node(task)"}],"source_content_type":"text/x-python","patch_set":6,"id":"bf659307_2a98fdbd","line":195,"in_reply_to":"bf659307_55d2b75d","updated":"2018-04-12 12:18:30.000000000","message":"+1, just going to do it now.","commit_id":"9416f4a5e46be1ec417a18605e27da09c7796529"}],"releasenotes/notes/remove-vifs-on-teardown-707c8e40c46b6e64.yaml":[{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"86a24f09d293a52b3d56ff4fd492aec1f2050013","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds the behavior to remove all records of VIF attachments upon"},{"line_number":5,"context_line":"    the teardown of a deployed node. This is in order to resolve issues"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf659307_8c9f4c02","line":2,"updated":"2018-03-27 12:31:52.000000000","message":"do we also need a few words in the \"upgrade\" section? like, \"If you are relying on VIFs staying after the node is torn down, this has never been intentional. Please update your tooling to re-add them expicitly.\" Not sure about wording, something like this.","commit_id":"68e16ed27900c81ca37f3c63170b1a0f54390e2b"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"04e234a45ddc556c55293645422e096bf5bb2e99","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Adds the behavior to remove all records of VIF attachments upon"},{"line_number":5,"context_line":"    the teardown of a deployed node. This is in order to resolve issues"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf659307_f5dc8875","line":2,"in_reply_to":"bf659307_8c9f4c02","updated":"2018-03-27 23:49:51.000000000","message":"I was thinking about that last night after pushing the patch. Likely so, I\u0027ll add in my next revision.","commit_id":"68e16ed27900c81ca37f3c63170b1a0f54390e2b"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"86a24f09d293a52b3d56ff4fd492aec1f2050013","unresolved":false,"context_lines":[{"line_number":4,"context_line":"    Adds the behavior to remove all records of VIF attachments upon"},{"line_number":5,"context_line":"    the teardown of a deployed node. This is in order to resolve issues"},{"line_number":6,"context_line":"    related to where it is operationally impossible in some circumstances"},{"line_number":7,"context_line":"    to remove a VIF attachment while a node is being undeplyed as the"},{"line_number":8,"context_line":"    Compute service will only attempt to remove the VIF for five minutes."},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"    See `bug 1743652 \u003chttps://bugs.launchpad.net/ironic/+bug/1743652\u003e`_ for more details."}],"source_content_type":"text/x-yaml","patch_set":1,"id":"bf659307_aca2504a","line":7,"updated":"2018-03-27 12:31:52.000000000","message":"typo: undeployed","commit_id":"68e16ed27900c81ca37f3c63170b1a0f54390e2b"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c7ad34a9acc0a73a2e0610fac1e7266fd347172c","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The behavior for retention of VIF interface attachments has changed."},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    If your use of the BareMetal service is reliant upon the behvior of"},{"line_number":7,"context_line":"    the VIFs being retained, which was introduced as a behavior change"},{"line_number":8,"context_line":"    during the Ocata cycle, then you must update your tooling to explicitly"},{"line_number":9,"context_line":"    re-add the the VIF attachments prior to deployment."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"bf659307_4f2ad87e","line":6,"updated":"2018-04-04 13:30:57.000000000","message":"typo: behavior","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"1e1bc342bdd3d3652615cf0965e92b898aa625ef","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The behavior for retention of VIF interface attachments has changed."},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"    If your use of the BareMetal service is reliant upon the behvior of"},{"line_number":7,"context_line":"    the VIFs being retained, which was introduced as a behavior change"},{"line_number":8,"context_line":"    during the Ocata cycle, then you must update your tooling to explicitly"},{"line_number":9,"context_line":"    re-add the the VIF attachments prior to deployment."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"bf659307_d9e21da1","line":6,"in_reply_to":"bf659307_4f2ad87e","updated":"2018-04-09 14:12:52.000000000","message":"Done","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c7ad34a9acc0a73a2e0610fac1e7266fd347172c","unresolved":false,"context_lines":[{"line_number":9,"context_line":"    re-add the the VIF attachments prior to deployment."},{"line_number":10,"context_line":"fixes:"},{"line_number":11,"context_line":"  - |"},{"line_number":12,"context_line":"    Adds the behavior to remove all records of VIF attachments upon"},{"line_number":13,"context_line":"    the teardown of a deployed node. This is in order to resolve issues"},{"line_number":14,"context_line":"    related to where it is operationally impossible in some circumstances"},{"line_number":15,"context_line":"    to remove a VIF attachment while a node is being undeployed as the"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"bf659307_6f2d5c93","line":12,"updated":"2018-04-04 13:30:57.000000000","message":"nit: s/Adds the behavior to remove/Removes/","commit_id":"d71eb6d472531aa1f8562b6c8ce47fdbc033861c"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"adda08e236b23857bb6b8dd5c5e645f177bea77a","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    If your use of the BareMetal service is reliant upon the behavior of"},{"line_number":7,"context_line":"    the VIFs being retained, which was introduced as a behavior change"},{"line_number":8,"context_line":"    during the Ocata cycle, then you must update your tooling to explicitly"},{"line_number":9,"context_line":"    re-add the the VIF attachments prior to deployment."},{"line_number":10,"context_line":"fixes:"},{"line_number":11,"context_line":"  - |"},{"line_number":12,"context_line":"    Removes all records of VIF attachments upon the teardown of a deployed"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"9f6a8fd7_ee6367db","line":9,"updated":"2018-04-18 03:06:40.000000000","message":"s/the the/the/","commit_id":"3a4e259a3714cedcad567e229e983f1b559c2668"}]}