)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"73cf203d7b94dc030dace736ebc3515c5e607f01","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Stop leaking ports on port_create timeout"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Sometimes neutron can take a long time to create a port, as its an"},{"line_number":10,"context_line":"expensive operation at the best of times, and sometimes it can take so"},{"line_number":11,"context_line":"long that the HTTP request times out. Usually this timeout doesn\u0027t stop"},{"line_number":12,"context_line":"the port from being created."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"ba2be162_d97df4e8","line":9,"range":{"start_line":9,"start_character":60,"end_line":9,"end_character":63},"updated":"2017-02-28 18:26:38.000000000","message":"it\u0027s","commit_id":"2e8c51a5862397957d907ed583509849c5e13d23"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"73cf203d7b94dc030dace736ebc3515c5e607f01","unresolved":false,"context_lines":[{"line_number":14,"context_line":"To avoid us leaking ports, we move to query neutron for the list of"},{"line_number":15,"context_line":"ports that have the device_id set (this can only be the ones we have"},{"line_number":16,"context_line":"created, at this point), and delete those. This will include the ports"},{"line_number":17,"context_line":"that are created after the timeout succeeds."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Note: the fact the port_create times out, shouldn\u0027t mean the list ports"},{"line_number":20,"context_line":"and delete ports calls will also timeout. Of course they could fail, but"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"ba2be162_59510462","line":17,"range":{"start_line":17,"start_character":9,"end_line":17,"end_character":43},"updated":"2017-02-28 18:26:38.000000000","message":"...created after the timeout expires?","commit_id":"2e8c51a5862397957d907ed583509849c5e13d23"}],"nova/network/neutronv2/api.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"50ac4a3b34948c5f83f9c29d6bbe257378fc1f79","unresolved":false,"context_lines":[{"line_number":667,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":668,"context_line":"                    if created_port_ids:"},{"line_number":669,"context_line":"                        self._delete_ports("},{"line_number":670,"context_line":"                            neutron, instance, created_port_ids)"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"                    # NOTE(johngarbutt) if we hit a timeout, we will not"},{"line_number":673,"context_line":"                    # know the port_uuid, but we still need to delete it."}],"source_content_type":"text/x-python","patch_set":2,"id":"dada55a8_ea707574","line":670,"updated":"2016-07-26 10:00:57.000000000","message":"I think we should probably not do both cleanups, once we have this tested properly.","commit_id":"88bebb457aab24676b800dc4895a932f0026bf20"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"b5a120b131ccfdb31a16eb8c6b7f7536497e680a","unresolved":false,"context_lines":[{"line_number":679,"context_line":"                    data \u003d neutron.list_ports(**search_opts)"},{"line_number":680,"context_line":"                    port_list \u003d data.get(\u0027ports\u0027, [])"},{"line_number":681,"context_line":"                    timedout_port_ids \u003d [port[\u0027id\u0027] for port in port_list]"},{"line_number":682,"context_line":"                    if timedout_port_ids:"},{"line_number":683,"context_line":"                        self._delete_ports("},{"line_number":684,"context_line":"                            neutron, instance, timedout_port_ids)"},{"line_number":685,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_66d9aa83","line":682,"updated":"2016-08-16 02:17:54.000000000","message":"Wouldn\u0027t we delete all ports even for those that were successfully created? Imagine the case where there are multiple NICs involved and we fail only at the N-th round of this for loop. Wouldn\u0027t we delete all the ports that were successfully created the during the previous N-1 rounds?\n\nThis sounds a bit inefficient and I fear for the cumulative effect that multiple requests may have. We should probably diff this with requests_and_created_ports.","commit_id":"88bebb457aab24676b800dc4895a932f0026bf20"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7298d3cb74810c33956e9be3fac3c85cdbaa6fe8","unresolved":false,"context_lines":[{"line_number":679,"context_line":"                    data \u003d neutron.list_ports(**search_opts)"},{"line_number":680,"context_line":"                    port_list \u003d data.get(\u0027ports\u0027, [])"},{"line_number":681,"context_line":"                    timedout_port_ids \u003d [port[\u0027id\u0027] for port in port_list]"},{"line_number":682,"context_line":"                    if timedout_port_ids:"},{"line_number":683,"context_line":"                        self._delete_ports("},{"line_number":684,"context_line":"                            neutron, instance, timedout_port_ids)"},{"line_number":685,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3ac371cc_41ff3867","line":682,"in_reply_to":"3ac371cc_66d9aa83","updated":"2016-08-16 02:53:00.000000000","message":"Um, discard my comment, I misread this diff as it looks you\u0027re reraising anyway.","commit_id":"88bebb457aab24676b800dc4895a932f0026bf20"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"d1a926ce885354d1ae834c009419e86008c7a899","unresolved":false,"context_lines":[{"line_number":793,"context_line":"                    request, created_port_id))"},{"line_number":794,"context_line":""},{"line_number":795,"context_line":"            except Exception:"},{"line_number":796,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":797,"context_line":"                    # NOTE(johngarbutt) if we hit a timeout, we will not"},{"line_number":798,"context_line":"                    # know the port_uuid, but we still need to delete it."},{"line_number":799,"context_line":"                    search_opts \u003d {"},{"line_number":800,"context_line":"                        \u0027tenant_id\u0027: context.project_id,"},{"line_number":801,"context_line":"                        \u0027device_id\u0027: instance.uuid,"},{"line_number":802,"context_line":"                        \u0027fields\u0027: [\u0027id\u0027]"},{"line_number":803,"context_line":"                    }"},{"line_number":804,"context_line":"                    data \u003d neutron.list_ports(**search_opts)"},{"line_number":805,"context_line":"                    port_list \u003d data.get(\u0027ports\u0027, [])"},{"line_number":806,"context_line":"                    timedout_port_ids \u003d [port[\u0027id\u0027] for port in port_list]"},{"line_number":807,"context_line":"                    if timedout_port_ids:"},{"line_number":808,"context_line":"                        self._delete_ports("},{"line_number":809,"context_line":"                            neutron, instance, timedout_port_ids)"},{"line_number":810,"context_line":""},{"line_number":811,"context_line":"        return requests_and_created_ports"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a95cdbc_c3bd8a69","line":809,"range":{"start_line":796,"start_character":16,"end_line":809,"end_character":65},"updated":"2016-10-07 13:37:51.000000000","message":"why not to retry first in that case? what if list_ports fails too","commit_id":"42fe176132e333f60c44caec214783856c23adc8"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"83a49470dcd5e6bc61ee46defb99b1986d60fea5","unresolved":false,"context_lines":[{"line_number":793,"context_line":"                    request, created_port_id))"},{"line_number":794,"context_line":""},{"line_number":795,"context_line":"            except Exception:"},{"line_number":796,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":797,"context_line":"                    # NOTE(johngarbutt) if we hit a timeout, we will not"},{"line_number":798,"context_line":"                    # know the port_uuid, but we still need to delete it."},{"line_number":799,"context_line":"                    search_opts \u003d {"},{"line_number":800,"context_line":"                        \u0027tenant_id\u0027: context.project_id,"},{"line_number":801,"context_line":"                        \u0027device_id\u0027: instance.uuid,"},{"line_number":802,"context_line":"                        \u0027fields\u0027: [\u0027id\u0027]"},{"line_number":803,"context_line":"                    }"},{"line_number":804,"context_line":"                    data \u003d neutron.list_ports(**search_opts)"},{"line_number":805,"context_line":"                    port_list \u003d data.get(\u0027ports\u0027, [])"},{"line_number":806,"context_line":"                    timedout_port_ids \u003d [port[\u0027id\u0027] for port in port_list]"},{"line_number":807,"context_line":"                    if timedout_port_ids:"},{"line_number":808,"context_line":"                        self._delete_ports("},{"line_number":809,"context_line":"                            neutron, instance, timedout_port_ids)"},{"line_number":810,"context_line":""},{"line_number":811,"context_line":"        return requests_and_created_ports"},{"line_number":812,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a95cdbc_940e238a","line":809,"range":{"start_line":796,"start_character":16,"end_line":809,"end_character":65},"in_reply_to":"1a95cdbc_c3bd8a69","updated":"2016-10-10 08:52:35.000000000","message":"If we retry, we are likely going to end up with extra ports. That should be a totally separate change.\n\nIf list_ports fails, we could fall back to the created_port_ids, but I think at that point, things all get too complicated.\n\nBut at a minimum, we should log any errors we hit during this tidy up with a warning log, so its possible to trace failures in this code.","commit_id":"42fe176132e333f60c44caec214783856c23adc8"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2c7fd534429004bc59f0140df3da67d78d17e917","unresolved":false,"context_lines":[{"line_number":808,"context_line":"                    data \u003d neutron.list_ports(**search_opts)"},{"line_number":809,"context_line":"                    port_list \u003d data.get(\u0027ports\u0027, [])"},{"line_number":810,"context_line":"                    timedout_port_ids \u003d [port[\u0027id\u0027] for port in port_list]"},{"line_number":811,"context_line":"                    if timedout_port_ids:"},{"line_number":812,"context_line":"                        self._delete_ports("},{"line_number":813,"context_line":"                            neutron, instance, timedout_port_ids)"},{"line_number":814,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"ba5da102_1845ba16","line":811,"updated":"2016-11-04 11:46:32.000000000","message":"Hum that looks little bit ugly, so neutron timeedout and we are going to call it again, what will happen if it timeout an other time ? I don\u0027t think adding a retry would be a good solution because that is going to increase the time of the process...\n\nI think we should have a periodic_task for such things, so at this step we should just add \u0027instance.uuid\u0027 in a queue and reraise the exception. In an other job we loop on them and call neutron to delete the ports.","commit_id":"302edde25e2e0e12246ec39d96f4375d0f91ec36"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48503561b34aea8b4c92691301aafa6d2eba6ce3","unresolved":false,"context_lines":[{"line_number":808,"context_line":"                        timedout_port_ids \u003d [port[\u0027id\u0027] for port in port_list]"},{"line_number":809,"context_line":"                        if timedout_port_ids:"},{"line_number":810,"context_line":"                            self._delete_ports("},{"line_number":811,"context_line":"                                neutron, instance, timedout_port_ids)"},{"line_number":812,"context_line":"                    except Exception:"},{"line_number":813,"context_line":"                        LOG.error("},{"line_number":814,"context_line":"                            _LE(\"Failed to delete ports with device_id: %s\"),"}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_145cb5ec","line":811,"updated":"2017-02-09 16:57:11.000000000","message":"Don\u0027t you have to set reraise\u003dFalse here?","commit_id":"1e4eb65b358c9717dc18d0b41c52aa28b080e33b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f468bb9250cf0c7b12e7567f965e28e9b8d7a8a0","unresolved":false,"context_lines":[{"line_number":808,"context_line":"                        timedout_port_ids \u003d [port[\u0027id\u0027] for port in port_list]"},{"line_number":809,"context_line":"                        if timedout_port_ids:"},{"line_number":810,"context_line":"                            self._delete_ports("},{"line_number":811,"context_line":"                                neutron, instance, timedout_port_ids)"},{"line_number":812,"context_line":"                    except Exception:"},{"line_number":813,"context_line":"                        LOG.error("},{"line_number":814,"context_line":"                            _LE(\"Failed to delete ports with device_id: %s\"),"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa31d9ce_98652ce3","line":811,"in_reply_to":"1a430d35_145cb5ec","updated":"2017-02-10 10:04:40.000000000","message":"No, no, we still wait to raise with the original as everything is broken here, so we need to bail out early.","commit_id":"1e4eb65b358c9717dc18d0b41c52aa28b080e33b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"48503561b34aea8b4c92691301aafa6d2eba6ce3","unresolved":false,"context_lines":[{"line_number":813,"context_line":"                        LOG.error("},{"line_number":814,"context_line":"                            _LE(\"Failed to delete ports with device_id: %s\"),"},{"line_number":815,"context_line":"                            instance.uuid, instance\u003dinstance)"},{"line_number":816,"context_line":"                        raise"},{"line_number":817,"context_line":""},{"line_number":818,"context_line":"        return requests_and_created_ports"},{"line_number":819,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1a430d35_1413d50e","line":816,"range":{"start_line":816,"start_character":24,"end_line":816,"end_character":29},"updated":"2017-02-09 16:57:11.000000000","message":"and this is unnecessary (again, due to save_and_reraise)","commit_id":"1e4eb65b358c9717dc18d0b41c52aa28b080e33b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f468bb9250cf0c7b12e7567f965e28e9b8d7a8a0","unresolved":false,"context_lines":[{"line_number":813,"context_line":"                        LOG.error("},{"line_number":814,"context_line":"                            _LE(\"Failed to delete ports with device_id: %s\"),"},{"line_number":815,"context_line":"                            instance.uuid, instance\u003dinstance)"},{"line_number":816,"context_line":"                        raise"},{"line_number":817,"context_line":""},{"line_number":818,"context_line":"        return requests_and_created_ports"},{"line_number":819,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"fa31d9ce_78af98e3","line":816,"range":{"start_line":816,"start_character":24,"end_line":816,"end_character":29},"in_reply_to":"1a430d35_1413d50e","updated":"2017-02-10 10:04:40.000000000","message":"no, I want this here to make sure we log any exception we hit as swallowed, it will correctly re-raise the original though.","commit_id":"1e4eb65b358c9717dc18d0b41c52aa28b080e33b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"73cf203d7b94dc030dace736ebc3515c5e607f01","unresolved":false,"context_lines":[{"line_number":817,"context_line":"                            _LE(\"Failed to delete ports with device_id: %s\"),"},{"line_number":818,"context_line":"                            instance.uuid, instance\u003dinstance)"},{"line_number":819,"context_line":"                        # Let save_and_reraise_exception log we had a problem"},{"line_number":820,"context_line":"                        raise"},{"line_number":821,"context_line":""},{"line_number":822,"context_line":"        return requests_and_created_ports"},{"line_number":823,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"ba2be162_598664ba","line":820,"range":{"start_line":820,"start_character":24,"end_line":820,"end_character":29},"updated":"2017-02-28 18:26:38.000000000","message":"I think this causes the whole shebang to raise _this_ exception, and just log the original exception.  Is that really what you want?  I would have thought the port cleanup should be best-effort - just log the failure - and the original port creation error is more important to bubble up.","commit_id":"2e8c51a5862397957d907ed583509849c5e13d23"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e1a152973358e084a1caa506adff7983b6bbcbbc","unresolved":false,"context_lines":[{"line_number":817,"context_line":"                            _LE(\"Failed to delete ports with device_id: %s\"),"},{"line_number":818,"context_line":"                            instance.uuid, instance\u003dinstance)"},{"line_number":819,"context_line":"                        # Let save_and_reraise_exception log we had a problem"},{"line_number":820,"context_line":"                        raise"},{"line_number":821,"context_line":""},{"line_number":822,"context_line":"        return requests_and_created_ports"},{"line_number":823,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a1ced50_5ae718e4","line":820,"range":{"start_line":820,"start_character":24,"end_line":820,"end_character":29},"in_reply_to":"ba2be162_598664ba","updated":"2017-03-17 17:56:06.000000000","message":"My comment here was trying to say, this gets captured and logged as part of the save_and_reraise. I think this what we want here. Failure to clean up means the admin needs to go fix that. It shouldn\u0027t happen, but it could.","commit_id":"2e8c51a5862397957d907ed583509849c5e13d23"}],"nova/tests/unit/network/test_neutronv2.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"73cf203d7b94dc030dace736ebc3515c5e607f01","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"ba2be162_94e4213c","updated":"2017-02-28 18:26:38.000000000","message":"I may be missing it, but I don\u0027t see a test where the port cleanup fails (raises).  As written, I think you\u0027ll see the exception from the cleanup failure, not the original deletion, which (as noted in other comment) may not be what you want.","commit_id":"2e8c51a5862397957d907ed583509849c5e13d23"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ce1e5ba7e4410df1ec784f1dcb44806b2202227f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"1a1ced50_bffe3969","in_reply_to":"1a1ced50_9a6d1035","updated":"2017-03-17 19:48:37.000000000","message":"Smiley-face, either way, the new test will prove the behavior and make sure it\u0027s the way you want it.  Thanks!","commit_id":"2e8c51a5862397957d907ed583509849c5e13d23"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e1a152973358e084a1caa506adff7983b6bbcbbc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"1a1ced50_9a6d1035","in_reply_to":"ba2be162_94e4213c","updated":"2017-03-17 17:56:06.000000000","message":"You will not, because of save and re-raise context manager, I think.\n\nYou are spot on about the missing test though, that should be added.","commit_id":"2e8c51a5862397957d907ed583509849c5e13d23"}]}