)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7818bef3e6a60e07ae8313adb4e7f9413de4375a","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If an instance fails to build, which is possible for a variety of"},{"line_number":10,"context_line":"reasons, we may end up in a situation where we have remnants of a"},{"line_number":11,"context_line":"plugged VIF (typically files) left of the host. This is because we"},{"line_number":12,"context_line":"cleanup from the neutron perspective but don\u0027t attempt to unplug the"},{"line_number":13,"context_line":"VIF, a call which may have many side-effects depending on the VIF"},{"line_number":14,"context_line":"driver. Resolve this by always attempting to unplug VIFs as part of the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fb8cfa7_02f980c6","line":11,"range":{"start_line":11,"start_character":35,"end_line":11,"end_character":37},"updated":"2019-06-26 13:31:47.000000000","message":"nit: on","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c3144f952dc597e77a9b6d76f478b6ed0ab7dbe9","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If an instance fails to build, which is possible for a variety of"},{"line_number":10,"context_line":"reasons, we may end up in a situation where we have remnants of a"},{"line_number":11,"context_line":"plugged VIF (typically files) left of the host. This is because we"},{"line_number":12,"context_line":"cleanup from the neutron perspective but don\u0027t attempt to unplug the"},{"line_number":13,"context_line":"VIF, a call which may have many side-effects depending on the VIF"},{"line_number":14,"context_line":"driver. Resolve this by always attempting to unplug VIFs as part of the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7faddb67_8912d261","line":11,"range":{"start_line":11,"start_character":35,"end_line":11,"end_character":37},"in_reply_to":"9fb8cfa7_02f980c6","updated":"2019-08-12 16:36:17.000000000","message":"Done","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"de64785a825229582b7498519785c898d6dee164","unresolved":false,"context_lines":[{"line_number":13,"context_line":"VIF, a call which may have many side-effects depending on the VIF"},{"line_number":14,"context_line":"driver. Resolve this by always attempting to unplug VIFs as part of the"},{"line_number":15,"context_line":"network cleanup."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"A now invalid note is also removed and a unit test corrected."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ibdbde4ed460a99b0cbe0d6b76e0e5b3c0650f9d9"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fb8cfa7_28f6e3d8","line":16,"updated":"2019-06-26 13:57:43.000000000","message":"In response to Sean\u0027s question about what happens with those drivers that don\u0027t implement the unplug_vifs method, I was thinking, \"well the vifs would just be cleaned up when the guest is destroyed by the user\" but I think you\u0027re saying - or we can deduce - that because we deallocate the network on failure and clean out the cache, when destroying the instance the network info cache would be empty so we wouldn\u0027t unplug on driver.destroy in that case and that\u0027s how the vifs would still get orphaned. Or am I making this up?","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4dc85dd98723383935601471abf3d25f541f7138","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ibdbde4ed460a99b0cbe0d6b76e0e5b3c0650f9d9"},{"line_number":20,"context_line":"Signed-off-by: Stephen Finucane \u003csfinucan@redhat.com\u003e"},{"line_number":21,"context_line":"Closes-Bug: 1831771"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fb8cfa7_28258309","line":21,"range":{"start_line":21,"start_character":0,"end_line":21,"end_character":19},"updated":"2019-06-26 14:24:15.000000000","message":"stephen can you also add\nRelated-Bug: 1830081\n\nwe will need to backport that fix as well to close the downstream bug.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8f25dc8f87de4047aeb06aee0549611fd5e21ffb","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ibdbde4ed460a99b0cbe0d6b76e0e5b3c0650f9d9"},{"line_number":20,"context_line":"Signed-off-by: Stephen Finucane \u003csfinucan@redhat.com\u003e"},{"line_number":21,"context_line":"Closes-Bug: 1831771"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9fb8cfa7_c82aa78f","line":21,"range":{"start_line":21,"start_character":0,"end_line":21,"end_character":19},"in_reply_to":"9fb8cfa7_28258309","updated":"2019-06-26 14:28:58.000000000","message":"Note that fix has a regression:\n\nhttps://review.opendev.org/#/c/667294/\n\nWhich you could probably also hit here.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c3144f952dc597e77a9b6d76f478b6ed0ab7dbe9","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ibdbde4ed460a99b0cbe0d6b76e0e5b3c0650f9d9"},{"line_number":20,"context_line":"Signed-off-by: Stephen Finucane \u003csfinucan@redhat.com\u003e"},{"line_number":21,"context_line":"Closes-Bug: 1831771"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7faddb67_a9154e4b","line":21,"range":{"start_line":21,"start_character":0,"end_line":21,"end_character":19},"in_reply_to":"9fb8cfa7_c82aa78f","updated":"2019-08-12 16:36:17.000000000","message":"Done","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"}],"nova/compute/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d72681b9aab5910e79576727a397223f8b584325","unresolved":false,"context_lines":[{"line_number":2037,"context_line":"                    block_device_mapping, request_spec\u003drequest_spec,"},{"line_number":2038,"context_line":"                    host_lists\u003d[host_list])"},{"line_number":2039,"context_line":"            return build_results.RESCHEDULED"},{"line_number":2040,"context_line":"        except (exception.InstanceNotFound,"},{"line_number":2041,"context_line":"                exception.UnexpectedDeletingTaskStateError):"},{"line_number":2042,"context_line":"            msg \u003d \u0027Instance disappeared during build.\u0027"},{"line_number":2043,"context_line":"            LOG.debug(msg, instance\u003dinstance)"},{"line_number":2044,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":2045,"context_line":"                    requested_networks)"},{"line_number":2046,"context_line":"            return build_results.FAILED"},{"line_number":2047,"context_line":"        except Exception as e:"},{"line_number":2048,"context_line":"            if isinstance(e, exception.BuildAbortException):"},{"line_number":2049,"context_line":"                LOG.error(e.format_message(), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_a121e8fd","line":2046,"range":{"start_line":2040,"start_character":7,"end_line":2046,"end_character":39},"updated":"2019-06-05 22:04:14.000000000","message":"the specific bug downstream was caused when \nand instance that was being created as part fo a heat stack\nwas delete after it had spawned but before it reached teh active state which resulted in an UnexpectedDeletingTaskStateError exception being raised resulting in except block being executed.\n\nthis calls _cleanup_allocated_networks which is modifed below to clean up the networking we had setup on the host.\nso i think this will solve the original bug.\n\nhttps://bugzilla.redhat.com/show_bug.cgi?id\u003d1668159#c10\ni originally suggested calling self.driver.cleanup\nhere instead becasue i was not sure if we also needed to clean up volume connection as we do on line 1981 above\nor on line 2056 below.\n\nso what im wondering is are we fixing the leak of configured network interfaces here but leaking volumes instead?","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d72681b9aab5910e79576727a397223f8b584325","unresolved":false,"context_lines":[{"line_number":2506,"context_line":"                            instance_uuid\u003dinstance.uuid,"},{"line_number":2507,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2508,"context_line":""},{"line_number":2509,"context_line":"    def _cleanup_allocated_networks(self, context, instance,"},{"line_number":2510,"context_line":"            requested_networks):"},{"line_number":2511,"context_line":""},{"line_number":2512,"context_line":"        def _unplug_vifs(instance, requested_networks):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_a1ab281b","line":2509,"range":{"start_line":2509,"start_character":9,"end_line":2509,"end_character":35},"updated":"2019-06-05 22:04:14.000000000","message":"hehe stephen can you add a doc string for this function :)","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b0fabb8f8e0711876987f36b8e6d683c9e7864b6","unresolved":false,"context_lines":[{"line_number":2506,"context_line":"                            instance_uuid\u003dinstance.uuid,"},{"line_number":2507,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2508,"context_line":""},{"line_number":2509,"context_line":"    def _cleanup_allocated_networks(self, context, instance,"},{"line_number":2510,"context_line":"            requested_networks):"},{"line_number":2511,"context_line":""},{"line_number":2512,"context_line":"        def _unplug_vifs(instance, requested_networks):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_2e4cf39c","line":2509,"range":{"start_line":2509,"start_character":9,"end_line":2509,"end_character":35},"in_reply_to":"9fb8cfa7_a1ab281b","updated":"2019-06-21 11:37:15.000000000","message":"Done","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8295985d18ce8cfc5354375956eaf0c0e8b3ff0f","unresolved":false,"context_lines":[{"line_number":2512,"context_line":"        def _unplug_vifs(instance, requested_networks):"},{"line_number":2513,"context_line":"            # If we were told not to allocate networks let\u0027s save ourselves the"},{"line_number":2514,"context_line":"            # trouble of calling the network API."},{"line_number":2515,"context_line":"            if requested_networks and requested_networks.no_allocate:"},{"line_number":2516,"context_line":"                LOG.debug(\"Skipping unplugging VIFs for instance since \""},{"line_number":2517,"context_line":"                          \"networking was not requested.\", instance\u003dinstance)"},{"line_number":2518,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_6279b32d","line":2515,"range":{"start_line":2515,"start_character":15,"end_line":2515,"end_character":33},"updated":"2019-06-05 20:04:38.000000000","message":"Should this be\n\n if not requested networks or requested_networks.no_allocate:\n\n?\n\nAs written we\u0027ll continue to unplug_vifs with that False-ish value.\n\n[Later] I looked, and fwiw all the drivers (eventually) no-op if requested_networks is empty. So you\u0027d be saving some call stacks if you skipped it from here.\n\n[Even later] I see this is just a copy/paste from _deallocate_network - which is also skipped under the same conditions, so I wonder if there\u0027s a DRYing opportunity here.","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b0fabb8f8e0711876987f36b8e6d683c9e7864b6","unresolved":false,"context_lines":[{"line_number":2512,"context_line":"        def _unplug_vifs(instance, requested_networks):"},{"line_number":2513,"context_line":"            # If we were told not to allocate networks let\u0027s save ourselves the"},{"line_number":2514,"context_line":"            # trouble of calling the network API."},{"line_number":2515,"context_line":"            if requested_networks and requested_networks.no_allocate:"},{"line_number":2516,"context_line":"                LOG.debug(\"Skipping unplugging VIFs for instance since \""},{"line_number":2517,"context_line":"                          \"networking was not requested.\", instance\u003dinstance)"},{"line_number":2518,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_8e7b9fbc","line":2515,"range":{"start_line":2515,"start_character":15,"end_line":2515,"end_character":33},"in_reply_to":"9fb8cfa7_41ed8c89","updated":"2019-06-21 11:37:15.000000000","message":"I got this completely mixed up. This expects a NetworkInfo object, not a RequestedNetworks ovo. I\u0027ve rewritten this accordingly.","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d72681b9aab5910e79576727a397223f8b584325","unresolved":false,"context_lines":[{"line_number":2512,"context_line":"        def _unplug_vifs(instance, requested_networks):"},{"line_number":2513,"context_line":"            # If we were told not to allocate networks let\u0027s save ourselves the"},{"line_number":2514,"context_line":"            # trouble of calling the network API."},{"line_number":2515,"context_line":"            if requested_networks and requested_networks.no_allocate:"},{"line_number":2516,"context_line":"                LOG.debug(\"Skipping unplugging VIFs for instance since \""},{"line_number":2517,"context_line":"                          \"networking was not requested.\", instance\u003dinstance)"},{"line_number":2518,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_41ed8c89","line":2515,"range":{"start_line":2515,"start_character":15,"end_line":2515,"end_character":33},"in_reply_to":"9fb8cfa7_6279b32d","updated":"2019-06-05 22:04:14.000000000","message":"ya an early out here is proably a good idea so changing to \n\n\"if not requested networks or requested_networks.no_allocate:\"\n\nsounds good to me.","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d72681b9aab5910e79576727a397223f8b584325","unresolved":false,"context_lines":[{"line_number":2514,"context_line":"            # trouble of calling the network API."},{"line_number":2515,"context_line":"            if requested_networks and requested_networks.no_allocate:"},{"line_number":2516,"context_line":"                LOG.debug(\"Skipping unplugging VIFs for instance since \""},{"line_number":2517,"context_line":"                          \"networking was not requested.\", instance\u003dinstance)"},{"line_number":2518,"context_line":"                return"},{"line_number":2519,"context_line":""},{"line_number":2520,"context_line":"            LOG.debug(\u0027Unplugging VIFs for instance\u0027, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_c18dfcd3","line":2517,"range":{"start_line":2517,"start_character":59,"end_line":2517,"end_character":76},"updated":"2019-06-05 22:04:14.000000000","message":"so with out a doc sting its hard to tell but i assume this is the full instance object based on how it is used so printing this will result in printing the base64 encored userdata to the log which can be large. so can we pass the instance.uuid there instead and below to avoid spamming the logs.","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b0fabb8f8e0711876987f36b8e6d683c9e7864b6","unresolved":false,"context_lines":[{"line_number":2514,"context_line":"            # trouble of calling the network API."},{"line_number":2515,"context_line":"            if requested_networks and requested_networks.no_allocate:"},{"line_number":2516,"context_line":"                LOG.debug(\"Skipping unplugging VIFs for instance since \""},{"line_number":2517,"context_line":"                          \"networking was not requested.\", instance\u003dinstance)"},{"line_number":2518,"context_line":"                return"},{"line_number":2519,"context_line":""},{"line_number":2520,"context_line":"            LOG.debug(\u0027Unplugging VIFs for instance\u0027, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_ce7197e0","line":2517,"range":{"start_line":2517,"start_character":59,"end_line":2517,"end_character":76},"in_reply_to":"9fb8cfa7_c18dfcd3","updated":"2019-06-21 11:37:15.000000000","message":"Nah, I can\u0027t find the place we do it but we override the default log format to include the instance UUID in the log if this parameter is included. Look down through the file and you\u0027ll see far more","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31e4c71289cc172af8f69817a5475535caf9e2cb","unresolved":false,"context_lines":[{"line_number":2475,"context_line":"                            instance_uuid\u003dinstance.uuid,"},{"line_number":2476,"context_line":"                            reason\u003dsix.text_type(exc))"},{"line_number":2477,"context_line":""},{"line_number":2478,"context_line":"    def _cleanup_allocated_networks(self, context, instance,"},{"line_number":2479,"context_line":"            requested_networks):"},{"line_number":2480,"context_line":"        \"\"\"Cleanup networks allocated for instance."},{"line_number":2481,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_da9dcd57","line":2478,"updated":"2019-06-21 15:20:36.000000000","message":"Guess what this is also in conductor:\n\nhttps://github.com/openstack/nova/blob/1ca69e839fcf97aaf2e85b2c069f2167d1dec7f1/nova/conductor/manager.py#L375\n\nBut you won\u0027t have access to the virt driver in that case so you\u0027re not going to get a chance to unplug VIFs there.","commit_id":"08a1e7d5408b9dbca692cccbb3e3ee5b7d653fe3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31e4c71289cc172af8f69817a5475535caf9e2cb","unresolved":false,"context_lines":[{"line_number":2481,"context_line":""},{"line_number":2482,"context_line":"        :param context: nova request context"},{"line_number":2483,"context_line":"        :param instance: nova.objects.instance.Instance object"},{"line_number":2484,"context_line":"        :param requested_networks: the networks on which the instance has ports"},{"line_number":2485,"context_line":"        \"\"\""},{"line_number":2486,"context_line":"        try:"},{"line_number":2487,"context_line":"            self._deallocate_network(context, instance, requested_networks)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_1a2b6530","line":2484,"range":{"start_line":2484,"start_character":35,"end_line":2484,"end_character":79},"updated":"2019-06-21 15:20:36.000000000","message":"This is confusing and doesn\u0027t tell me if this is a list of network ids or what. It looks like it\u0027s actually an objects.NetworkRequestList so just say that.","commit_id":"08a1e7d5408b9dbca692cccbb3e3ee5b7d653fe3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3c1b790c9a9bebb56ade626681ff3bb0d8f3ebfc","unresolved":false,"context_lines":[{"line_number":2481,"context_line":""},{"line_number":2482,"context_line":"        :param context: nova request context"},{"line_number":2483,"context_line":"        :param instance: nova.objects.instance.Instance object"},{"line_number":2484,"context_line":"        :param requested_networks: the networks on which the instance has ports"},{"line_number":2485,"context_line":"        \"\"\""},{"line_number":2486,"context_line":"        try:"},{"line_number":2487,"context_line":"            self._deallocate_network(context, instance, requested_networks)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_601d1472","line":2484,"range":{"start_line":2484,"start_character":35,"end_line":2484,"end_character":79},"in_reply_to":"9fb8cfa7_1a2b6530","updated":"2019-06-21 17:41:15.000000000","message":"Done","commit_id":"08a1e7d5408b9dbca692cccbb3e3ee5b7d653fe3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"812f9c1f7a4092920f22366ad6294c49eebb75e1","unresolved":false,"context_lines":[{"line_number":2497,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2498,"context_line":"        if not network_info:"},{"line_number":2499,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2500,"context_line":"                context, instance)"},{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_d164d8e7","line":2500,"updated":"2019-06-21 11:39:40.000000000","message":"This is copied from \u0027_shutdown_instance\u0027","commit_id":"08a1e7d5408b9dbca692cccbb3e3ee5b7d653fe3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31e4c71289cc172af8f69817a5475535caf9e2cb","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except exception.NovaException:"},{"line_number":2505,"context_line":"            # NOTE(stephenfin): It\u0027s possible that the instance never got"},{"line_number":2506,"context_line":"            # as far as plugging VIFs, in which case we would see an"},{"line_number":2507,"context_line":"            # exception which can be ignored"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_7af061ca","line":2504,"updated":"2019-06-21 15:20:36.000000000","message":"The driver interface also raises NotImplementedError - wouldn\u0027t you want to handle and not blow up the server delete if that\u0027s raised? It looks like neither the vmware or zvm drivers implement that method.","commit_id":"08a1e7d5408b9dbca692cccbb3e3ee5b7d653fe3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3c1b790c9a9bebb56ade626681ff3bb0d8f3ebfc","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except exception.NovaException:"},{"line_number":2505,"context_line":"            # NOTE(stephenfin): It\u0027s possible that the instance never got"},{"line_number":2506,"context_line":"            # as far as plugging VIFs, in which case we would see an"},{"line_number":2507,"context_line":"            # exception which can be ignored"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_401a507d","line":2504,"in_reply_to":"9fb8cfa7_7af061ca","updated":"2019-06-21 17:41:15.000000000","message":"Done","commit_id":"08a1e7d5408b9dbca692cccbb3e3ee5b7d653fe3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"31e4c71289cc172af8f69817a5475535caf9e2cb","unresolved":false,"context_lines":[{"line_number":2505,"context_line":"            # NOTE(stephenfin): It\u0027s possible that the instance never got"},{"line_number":2506,"context_line":"            # as far as plugging VIFs, in which case we would see an"},{"line_number":2507,"context_line":"            # exception which can be ignored"},{"line_number":2508,"context_line":"            LOG.debug(\u0027No plugged VIFs to unplug\u0027, instance\u003dinstance)"},{"line_number":2509,"context_line":"        else:"},{"line_number":2510,"context_line":"            LOG.debug(\u0027Unplugged VIFs for instance\u0027, instance\u003dinstance)"},{"line_number":2511,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_7a77013c","line":2508,"updated":"2019-06-21 15:20:36.000000000","message":"How do you know this is the case? What if there were VIFs but they were unbound or binding_failed? Wouldn\u0027t it be better to just say something along the lines of\n\nLOG.debug(\"Cleaning up VIFs failed for instance. Error: %s\", six.text_type(ex), instance\u003dinstance)","commit_id":"08a1e7d5408b9dbca692cccbb3e3ee5b7d653fe3"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3c1b790c9a9bebb56ade626681ff3bb0d8f3ebfc","unresolved":false,"context_lines":[{"line_number":2505,"context_line":"            # NOTE(stephenfin): It\u0027s possible that the instance never got"},{"line_number":2506,"context_line":"            # as far as plugging VIFs, in which case we would see an"},{"line_number":2507,"context_line":"            # exception which can be ignored"},{"line_number":2508,"context_line":"            LOG.debug(\u0027No plugged VIFs to unplug\u0027, instance\u003dinstance)"},{"line_number":2509,"context_line":"        else:"},{"line_number":2510,"context_line":"            LOG.debug(\u0027Unplugged VIFs for instance\u0027, instance\u003dinstance)"},{"line_number":2511,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_a029cc9b","line":2508,"in_reply_to":"9fb8cfa7_7a77013c","updated":"2019-06-21 17:41:15.000000000","message":"Done","commit_id":"08a1e7d5408b9dbca692cccbb3e3ee5b7d653fe3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fbd8c244e144ae97da51e8aefaffcb185345abe6","unresolved":false,"context_lines":[{"line_number":2484,"context_line":"        :param requested_networks: nova.objects.NetworkRequestList"},{"line_number":2485,"context_line":"        \"\"\""},{"line_number":2486,"context_line":"        try:"},{"line_number":2487,"context_line":"            self._deallocate_network(context, instance, requested_networks)"},{"line_number":2488,"context_line":"        except Exception:"},{"line_number":2489,"context_line":"            LOG.exception(\u0027Failed to deallocate networks\u0027, instance\u003dinstance)"},{"line_number":2490,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_687ffb65","line":2487,"range":{"start_line":2487,"start_character":12,"end_line":2487,"end_character":36},"updated":"2019-06-26 13:51:33.000000000","message":"as matt pointed out below this need to be moved after we try to clean up the ports\n\n_deallocate_network calls deallocate_for_instance\nhttps://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L1773\n\nand deallocate_for_instance clears the network info cache \nhttps://opendev.org/openstack/nova/src/branch/master/nova/network/neutronv2/api.py#L1599-L1604","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"de64785a825229582b7498519785c898d6dee164","unresolved":false,"context_lines":[{"line_number":2484,"context_line":"        :param requested_networks: nova.objects.NetworkRequestList"},{"line_number":2485,"context_line":"        \"\"\""},{"line_number":2486,"context_line":"        try:"},{"line_number":2487,"context_line":"            self._deallocate_network(context, instance, requested_networks)"},{"line_number":2488,"context_line":"        except Exception:"},{"line_number":2489,"context_line":"            LOG.exception(\u0027Failed to deallocate networks\u0027, instance\u003dinstance)"},{"line_number":2490,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_a8d7334b","line":2487,"range":{"start_line":2487,"start_character":12,"end_line":2487,"end_character":36},"in_reply_to":"9fb8cfa7_687ffb65","updated":"2019-06-26 13:57:43.000000000","message":"Good catch - has someone tried to reproduce the failure from the bug and then manually test that this change fixes the problem? The instance.info_cache.network_info might already be loaded so it might not be a problem, but it would be if it\u0027s not already loaded (or is stale).","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c3144f952dc597e77a9b6d76f478b6ed0ab7dbe9","unresolved":false,"context_lines":[{"line_number":2484,"context_line":"        :param requested_networks: nova.objects.NetworkRequestList"},{"line_number":2485,"context_line":"        \"\"\""},{"line_number":2486,"context_line":"        try:"},{"line_number":2487,"context_line":"            self._deallocate_network(context, instance, requested_networks)"},{"line_number":2488,"context_line":"        except Exception:"},{"line_number":2489,"context_line":"            LOG.exception(\u0027Failed to deallocate networks\u0027, instance\u003dinstance)"},{"line_number":2490,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_2416634b","line":2487,"range":{"start_line":2487,"start_character":12,"end_line":2487,"end_character":36},"in_reply_to":"9fb8cfa7_a8d7334b","updated":"2019-08-12 16:36:17.000000000","message":"Done. Also these links are stale now so that should read\n\n\u003e \u0027_deallocate_network\u0027 calls \u0027deallocate_for_instance\u0027\n\u003e\n\u003e https://github.com/openstack/nova/blob/790f62888/nova/compute/manager.py#L1817-L1818\n\u003e\n\u003e and \u0027deallocate_for_instance\u0027 clears the network info cache \n\u003e\n\u003e https://github.com/openstack/nova/blob/790f62888/nova/network/neutronv2/api.py#L1596-L1601","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cb34ce87cddd6bafebc6274f9796ebcee3418b97","unresolved":false,"context_lines":[{"line_number":2491,"context_line":""},{"line_number":2492,"context_line":"        LOG.debug(\u0027Unplugging VIFs for instance\u0027, instance\u003dinstance)"},{"line_number":2493,"context_line":""},{"line_number":2494,"context_line":"        network_info \u003d instance.get_network_info()"},{"line_number":2495,"context_line":""},{"line_number":2496,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2497,"context_line":"        # unplugging the interface, refresh network_info if it is empty."}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_2774d6ce","line":2494,"range":{"start_line":2494,"start_character":9,"end_line":2494,"end_character":49},"updated":"2019-06-26 12:26:59.000000000","message":"ok so this pulls it form the cache if present and retruns and empty NetworkInfo object if not","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cb34ce87cddd6bafebc6274f9796ebcee3418b97","unresolved":false,"context_lines":[{"line_number":2495,"context_line":""},{"line_number":2496,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2497,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2498,"context_line":"        if not network_info:"},{"line_number":2499,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2500,"context_line":"                context, instance)"},{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_070e723a","line":2500,"range":{"start_line":2498,"start_character":8,"end_line":2500,"end_character":34},"updated":"2019-06-26 12:26:59.000000000","message":"then if it is empty we bypass the cache and rebuild the NetworkInfo from neutron directly.\n\ni think this will rearely if ever be taken\nthat said we are trying to prevent leaking interfaces with this change so we should be extra careful and make sure we remove all of them so this makes sense to me.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42fa286d54efd9fb05276123f762cc0d2721cfa4","unresolved":false,"context_lines":[{"line_number":2495,"context_line":""},{"line_number":2496,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2497,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2498,"context_line":"        if not network_info:"},{"line_number":2499,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2500,"context_line":"                context, instance)"},{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_2df7d58d","line":2500,"range":{"start_line":2498,"start_character":8,"end_line":2500,"end_character":34},"in_reply_to":"9fb8cfa7_070e723a","updated":"2019-06-26 13:31:12.000000000","message":"This doesn\u0027t rebuild from neutron unless you pass the force_refresh kwarg, otherwise it just rebuilds the cache from the cache in case you got the instance from the db to delete it before the cache was populated (see amorin\u0027s recent change related to this same logic).","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7818bef3e6a60e07ae8313adb4e7f9413de4375a","unresolved":false,"context_lines":[{"line_number":2495,"context_line":""},{"line_number":2496,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2497,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2498,"context_line":"        if not network_info:"},{"line_number":2499,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2500,"context_line":"                context, instance)"},{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_6d014d6a","line":2500,"range":{"start_line":2498,"start_character":8,"end_line":2500,"end_character":34},"in_reply_to":"9fb8cfa7_070e723a","updated":"2019-06-26 13:31:47.000000000","message":"Will this still return anything, given the prior call to _deallocate_network on line 2487?","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":11583,"name":"Arnaud Morin","email":"arnaud.morin@gmail.com","username":"arnaudmorin"},"change_message_id":"3a4551cc87ffa73c94aaae3e671add0815a0b22d","unresolved":false,"context_lines":[{"line_number":2495,"context_line":""},{"line_number":2496,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2497,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2498,"context_line":"        if not network_info:"},{"line_number":2499,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2500,"context_line":"                context, instance)"},{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_6815bb48","line":2500,"range":{"start_line":2498,"start_character":8,"end_line":2500,"end_character":34},"in_reply_to":"9fb8cfa7_2d259509","updated":"2019-06-26 14:30:06.000000000","message":"I am not sure that this is applicable to your case, but if the instance status is DELETED, it will raise an InstanceInfoCacheNotFound exception here.\nI did the same in https://review.opendev.org/#/c/667294/","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"94f05dc2932e5951053e4cb387348209db58022e","unresolved":false,"context_lines":[{"line_number":2495,"context_line":""},{"line_number":2496,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2497,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2498,"context_line":"        if not network_info:"},{"line_number":2499,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2500,"context_line":"                context, instance)"},{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_2d259509","line":2500,"range":{"start_line":2498,"start_character":8,"end_line":2500,"end_character":34},"in_reply_to":"9fb8cfa7_2df7d58d","updated":"2019-06-26 13:32:29.000000000","message":"I99601773406c61f93002e2f7cbb248cf73cef0ab","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"39ad62248a5d8a052f2d690c45c6a193c02b95dc","unresolved":false,"context_lines":[{"line_number":2495,"context_line":""},{"line_number":2496,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2497,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2498,"context_line":"        if not network_info:"},{"line_number":2499,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2500,"context_line":"                context, instance)"},{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_9cd630f2","line":2500,"range":{"start_line":2498,"start_character":8,"end_line":2500,"end_character":34},"in_reply_to":"9fb8cfa7_6815bb48","updated":"2019-06-26 14:57:12.000000000","message":"the task state shoudl be deleting but not deleted when we call this but i agree with matt\u0027s  top lvel commnet we proably want to pull this out in to a helper function as i was thinking the same.\n\ni was debating if it made sense to jsut wrap this logic into instace.get_network_info \n\nhttps://github.com/openstack/nova/blob/707deb158996d540111c23afd8c916ea1c18906a/nova/objects/instance.py#L1161\n\nso instead of \n\n    def get_network_info(self):\n        if self.info_cache is None:\n            return network_model.NetworkInfo.hydrate([])\n        return self.info_cache.network_info\n\n\nwe woudl do \n\n    def get_network_info(self):\n        if self.info_cache is None:\n            return network_model.NetworkInfo.hydrate([])\n        info \u003d self.info_cache.network_info\n        if info is None and self.vm_state !\u003d vm_states.DELETE:\n           info \u003d  self.network_api.get_instance_nw_info(\n                context, instance)\n        return info\n\nmy issue with that is we dont have an instance of the network api in the instance class and im not sure we want to call neutron the neutronv2 api class directly.\n\nin any case i thinks doing ithis right once and reusing is a good idea just not sure where to put it.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cb34ce87cddd6bafebc6274f9796ebcee3418b97","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_620abc20","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"updated":"2019-06-26 12:26:59.000000000","message":"what driver does not support network?\nnot all driver need to use os-vif but they all should have that method.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6b764a2de5571e04eccd8b12f3bbb3039e6d0400","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"3fa7e38b_15854c5a","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"in_reply_to":"7faddb67_ebdfb335","updated":"2019-10-10 11:28:24.000000000","message":"The DELETING task_state is set by the api caller before hitting the compute host. It is explicitly not guarded in the caller, so this can be set while the compute host is still booting the instance.\n\nThe semaphore on the compute host only prevents the delete from actually starting, but it\u0027s already set in the db before that.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c3144f952dc597e77a9b6d76f478b6ed0ab7dbe9","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_ebdfb335","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"in_reply_to":"9fb8cfa7_08b2ff94","updated":"2019-08-12 16:36:17.000000000","message":"\u003e ya the specific downstream customer issue was as follow.\n \u003e \n \u003e customer created multiple vms as part of a heat stack with the auto\n \u003e rollback on failure feature.\n \u003e \n \u003e a resouce faild to create so heat started rolling back and issue a\n \u003e delete for the server.\n \u003e \n \u003e for one of the server that was in build state it had actully\n \u003e spawned on the compute node but the delete was recived in the api\n \u003e after teh event form libvirt to say the vm was running and before\n \u003e the libvirt driver had updated the task state form building to\n \u003e active.\n \u003e \n \u003e when the driver attemepted to update the task state to active it\n \u003e raised an UnexpectedDeletingTaskStateError which caused us to take\n \u003e a code path that calls _cleanup_allocated_networks but dose not\n \u003e call destory or unplug_vifs on the driver directly so we would leak\n \u003e the interface in that case which this trys to fix.\n\nI\u0027m trying to figure out how to reproduce this locally with no luck. We have semaphores on instance creation and deletion so I\u0027ve no idea how \u0027UnexpectedDeletingTaskStateError\u0027 could ever be raised outside of straight up messing with the DB or using some admin command/API to mess with task_state. Open to ideas here","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2d39646a934f50c76e64038a7b67aff23cd14c78","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_48d9d753","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"in_reply_to":"9fb8cfa7_2848232b","updated":"2019-06-26 13:59:55.000000000","message":"\u003e For those drivers that don\u0027t implement unplug_vifs the bug would\n \u003e still exist where vifs are left plugged on the host (I guess).\n\nI also left a comment/question in the commit message related to this point. If we are still orphaning vifs on the host in this case for those other drivers (looks like zvm and vmware - though I\u0027m not sure how those would work here since they aren\u0027t really running a hypervisor on the local host where the compute service is running) - maybe we should be logging something in the NotImplementedError case?","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b5aab07c66c40c27021eb9a677f17865ae0c87ed","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_c89aa76d","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"in_reply_to":"9fb8cfa7_48d9d753","updated":"2019-06-26 14:03:29.000000000","message":"Note that in at least the case of the libvirt driver, if something fails during the spawn (well, at least if networking setup fails after we\u0027ve plugged vifs) we try to unplug them as part of a rollback:\n\nhttps://github.com/openstack/nova/blob/707deb158996d540111c23afd8c916ea1c18906a/nova/virt/libvirt/driver.py#L5738\n\nBut that doesn\u0027t mean we couldn\u0027t still get into this flow to deallocate networking *after* the driver.spawn is successful but then something else fails in the compute manager build flow.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"de64785a825229582b7498519785c898d6dee164","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_2848232b","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"in_reply_to":"9fb8cfa7_4d580989","updated":"2019-06-26 13:57:43.000000000","message":"For those drivers that don\u0027t implement unplug_vifs the bug would still exist where vifs are left plugged on the host (I guess).","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42fa286d54efd9fb05276123f762cc0d2721cfa4","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_ad30a557","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"in_reply_to":"9fb8cfa7_620abc20","updated":"2019-06-26 13:31:12.000000000","message":"Several drivers don\u0027t implement the unplug_vifs method directly on the driver - it\u0027s easy to see for yourself.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fbd8c244e144ae97da51e8aefaffcb185345abe6","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_4d580989","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"in_reply_to":"9fb8cfa7_ad30a557","updated":"2019-06-26 13:51:33.000000000","message":"ok i gues its done in the manager for those drivers?\n\ni was just expecting this to either have an implementation in the base driver that just looped over network_info and called unplug on each vif or for all driver to implement it.\n\n\nwhen there is a sane default behaviour im not sure why we choose to raise NotImplementedError given it would be calling unplug which the drivers could already supply there own version of.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4dc85dd98723383935601471abf3d25f541f7138","unresolved":false,"context_lines":[{"line_number":2501,"context_line":""},{"line_number":2502,"context_line":"        try:"},{"line_number":2503,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2504,"context_line":"        except NotImplementedError:"},{"line_number":2505,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2506,"context_line":"            pass"},{"line_number":2507,"context_line":"        except exception.NovaException as exc:"},{"line_number":2508,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2509,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_08b2ff94","line":2506,"range":{"start_line":2504,"start_character":8,"end_line":2506,"end_character":16},"in_reply_to":"9fb8cfa7_c89aa76d","updated":"2019-06-26 14:24:15.000000000","message":"ya the specific downstream customer issue was as follow.\n\ncustomer created multiple vms as part of a heat stack with the auto rollback on failure feature.\n\na resouce faild to create so heat started rolling back and issue a delete for the server.\n\nfor one of the server that was in build state it had actully spawned on the compute node but the delete was recived in the api after teh event form libvirt to say the vm was running and before the libvirt driver had updated the task state form building to active.\n\nwhen the driver attemepted to update the task state to active it raised an UnexpectedDeletingTaskStateError which caused us to take a code path that calls _cleanup_allocated_networks but dose not call destory or unplug_vifs on the driver directly so we would leak the interface in that case which this trys to fix.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0289638dc904a280207b70e30b8b937b1aee4059","unresolved":false,"context_lines":[{"line_number":2651,"context_line":"        try:"},{"line_number":2652,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2653,"context_line":"        except NotImplementedError:"},{"line_number":2654,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2655,"context_line":"            pass"},{"line_number":2656,"context_line":"        except exception.NovaException as exc:"},{"line_number":2657,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2658,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_5724a50d","line":2655,"range":{"start_line":2654,"start_character":1,"end_line":2655,"end_character":16},"updated":"2020-02-25 08:42:07.000000000","message":"Is this potentially means that some virt driver does not support direct unplug and therefore we might leak some  plugged vifs (as per commit message \"files\") on the hypervisor? If so then I would leave a debug log here with this info.","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ce2eba7f3b060ce34029f963735ca99fe42524c4","unresolved":false,"context_lines":[{"line_number":2651,"context_line":"        try:"},{"line_number":2652,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2653,"context_line":"        except NotImplementedError:"},{"line_number":2654,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2655,"context_line":"            pass"},{"line_number":2656,"context_line":"        except exception.NovaException as exc:"},{"line_number":2657,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2658,"context_line":"            # VIFs, in which case we would see an exception which can be"}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_a3191b89","line":2655,"range":{"start_line":2654,"start_character":1,"end_line":2655,"end_character":16},"in_reply_to":"1fa4df85_5724a50d","updated":"2020-02-25 11:23:42.000000000","message":"Good point. Done","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0289638dc904a280207b70e30b8b937b1aee4059","unresolved":false,"context_lines":[{"line_number":2657,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2658,"context_line":"            # VIFs, in which case we would see an exception which can be"},{"line_number":2659,"context_line":"            # ignored"},{"line_number":2660,"context_line":"            LOG.debug(\u0027Cleaning up VIFs failed for instance. Error: %s\u0027,"},{"line_number":2661,"context_line":"                      six.text_type(exc), instance\u003dinstance)"},{"line_number":2662,"context_line":"        else:"},{"line_number":2663,"context_line":"            LOG.debug(\u0027Unplugged VIFs for instance\u0027, instance\u003dinstance)"},{"line_number":2664,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_f747d1bb","line":2661,"range":{"start_line":2660,"start_character":1,"end_line":2661,"end_character":60},"updated":"2020-02-25 08:42:07.000000000","message":"Logging an Error on debug level is confusing. I would somehow mention in the log message that this error can be ignored as the vifs wasn\u0027t even plugged in the first place. Or is this information clear from the NovaException message we concatenate to the log?","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ce2eba7f3b060ce34029f963735ca99fe42524c4","unresolved":false,"context_lines":[{"line_number":2657,"context_line":"            # It\u0027s possible that the instance never got as far as plugging"},{"line_number":2658,"context_line":"            # VIFs, in which case we would see an exception which can be"},{"line_number":2659,"context_line":"            # ignored"},{"line_number":2660,"context_line":"            LOG.debug(\u0027Cleaning up VIFs failed for instance. Error: %s\u0027,"},{"line_number":2661,"context_line":"                      six.text_type(exc), instance\u003dinstance)"},{"line_number":2662,"context_line":"        else:"},{"line_number":2663,"context_line":"            LOG.debug(\u0027Unplugged VIFs for instance\u0027, instance\u003dinstance)"},{"line_number":2664,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_63132368","line":2661,"range":{"start_line":2660,"start_character":1,"end_line":2661,"end_character":60},"in_reply_to":"1fa4df85_f747d1bb","updated":"2020-02-25 11:23:42.000000000","message":"No, you\u0027re right - a warning would be much more useful since we\u0027re already in an error flow","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"09ea7efd01fd80e2d2a35d54a78a2304c501bca3","unresolved":false,"context_lines":[{"line_number":2645,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2646,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2647,"context_line":"        if not network_info:"},{"line_number":2648,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2649,"context_line":"                context, instance)"},{"line_number":2650,"context_line":""},{"line_number":2651,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_5b192777","line":2648,"range":{"start_line":2648,"start_character":0,"end_line":2648,"end_character":65},"updated":"2020-02-25 15:15:38.000000000","message":"I assume since we\u0027re in a cleanup phase, this new code is supposed to be best-effort. I.e. any exceptions should be (logged but) ignored. The logic below looks like it\u0027s doing a good job of that -- but this part can still raise exceptions, at least \"couldn\u0027t talk to neutron\" if not others. Shouldn\u0027t we trap and ignore those?","commit_id":"5f1291de1bf547e8d999b1aed3ad414f36310a26"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f1dd787e2f8ebe320c7990db812fb1b7743fdd4d","unresolved":false,"context_lines":[{"line_number":2645,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2646,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2647,"context_line":"        if not network_info:"},{"line_number":2648,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2649,"context_line":"                context, instance)"},{"line_number":2650,"context_line":""},{"line_number":2651,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_9b9f9f4f","line":2648,"range":{"start_line":2648,"start_character":0,"end_line":2648,"end_character":65},"in_reply_to":"1fa4df85_5b192777","updated":"2020-02-25 15:30:44.000000000","message":"Good point. Will add.","commit_id":"5f1291de1bf547e8d999b1aed3ad414f36310a26"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8e9c41f42c60290ef0c67257a58cd15d362ab21f","unresolved":false,"context_lines":[{"line_number":2645,"context_line":"        # NOTE(stephenfin) to avoid nova destroying the instance without"},{"line_number":2646,"context_line":"        # unplugging the interface, refresh network_info if it is empty."},{"line_number":2647,"context_line":"        if not network_info:"},{"line_number":2648,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2649,"context_line":"                context, instance)"},{"line_number":2650,"context_line":""},{"line_number":2651,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":15,"id":"1fa4df85_1be2cfd6","line":2648,"range":{"start_line":2648,"start_character":0,"end_line":2648,"end_character":65},"in_reply_to":"1fa4df85_9b9f9f4f","updated":"2020-02-25 15:32:05.000000000","message":"Cool","commit_id":"5f1291de1bf547e8d999b1aed3ad414f36310a26"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cbb26ef1c1143212816fa445660148a03a4be1b5","unresolved":false,"context_lines":[{"line_number":2649,"context_line":"                network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2650,"context_line":"                    context, instance,"},{"line_number":2651,"context_line":"                )"},{"line_number":2652,"context_line":"            except Exception:"},{"line_number":2653,"context_line":"                LOG.warning("},{"line_number":2654,"context_line":"                    \u0027Failed to update network info cache when cleaning up \u0027"},{"line_number":2655,"context_line":"                    \u0027allocated networks. Stale VIFs may be left on this host.\u0027"}],"source_content_type":"text/x-python","patch_set":16,"id":"1fa4df85_6119d677","line":2652,"range":{"start_line":2652,"start_character":19,"end_line":2652,"end_character":28},"updated":"2020-02-25 16:32:54.000000000","message":"I hate to be that guy, but this code path should be tested...","commit_id":"83fbec245c51a63eb47685713f950a6bc8b3fc19"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a195ccbd76314ddf88a20fe0c140829432a0ec10","unresolved":false,"context_lines":[{"line_number":2649,"context_line":"                network_info \u003d self.network_api.get_instance_nw_info("},{"line_number":2650,"context_line":"                    context, instance,"},{"line_number":2651,"context_line":"                )"},{"line_number":2652,"context_line":"            except Exception:"},{"line_number":2653,"context_line":"                LOG.warning("},{"line_number":2654,"context_line":"                    \u0027Failed to update network info cache when cleaning up \u0027"},{"line_number":2655,"context_line":"                    \u0027allocated networks. Stale VIFs may be left on this host.\u0027"}],"source_content_type":"text/x-python","patch_set":16,"id":"1fa4df85_62da2518","line":2652,"range":{"start_line":2652,"start_character":19,"end_line":2652,"end_character":28},"in_reply_to":"1fa4df85_6119d677","updated":"2020-02-26 14:35:13.000000000","message":"Done","commit_id":"83fbec245c51a63eb47685713f950a6bc8b3fc19"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"cbb26ef1c1143212816fa445660148a03a4be1b5","unresolved":false,"context_lines":[{"line_number":2652,"context_line":"            except Exception:"},{"line_number":2653,"context_line":"                LOG.warning("},{"line_number":2654,"context_line":"                    \u0027Failed to update network info cache when cleaning up \u0027"},{"line_number":2655,"context_line":"                    \u0027allocated networks. Stale VIFs may be left on this host.\u0027"},{"line_number":2656,"context_line":"                )"},{"line_number":2657,"context_line":"                return"},{"line_number":2658,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"1fa4df85_a10fce2c","line":2655,"updated":"2020-02-25 16:32:54.000000000","message":"It would be nice if the exception itself were logged.","commit_id":"83fbec245c51a63eb47685713f950a6bc8b3fc19"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a195ccbd76314ddf88a20fe0c140829432a0ec10","unresolved":false,"context_lines":[{"line_number":2652,"context_line":"            except Exception:"},{"line_number":2653,"context_line":"                LOG.warning("},{"line_number":2654,"context_line":"                    \u0027Failed to update network info cache when cleaning up \u0027"},{"line_number":2655,"context_line":"                    \u0027allocated networks. Stale VIFs may be left on this host.\u0027"},{"line_number":2656,"context_line":"                )"},{"line_number":2657,"context_line":"                return"},{"line_number":2658,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"1fa4df85_02d1f154","line":2655,"in_reply_to":"1fa4df85_a10fce2c","updated":"2020-02-26 14:35:13.000000000","message":"Done","commit_id":"83fbec245c51a63eb47685713f950a6bc8b3fc19"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"835915fe73820499ae6d30b1872303084cb79ef8","unresolved":false,"context_lines":[{"line_number":2657,"context_line":"                return"},{"line_number":2658,"context_line":""},{"line_number":2659,"context_line":"        try:"},{"line_number":2660,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2661,"context_line":"        except NotImplementedError:"},{"line_number":2662,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2663,"context_line":"            LOG.debug("}],"source_content_type":"text/x-python","patch_set":16,"id":"1fa4df85_4d7e3fe5","line":2660,"range":{"start_line":2660,"start_character":12,"end_line":2660,"end_character":59},"updated":"2020-02-26 10:41:58.000000000","message":"I\u0027m thinking the case, when we try to reschedule the instance, and we won\u0027t call \u0027_cleanup_allocated_networks\u0027. So at that case, we may need to unplug the vif","commit_id":"83fbec245c51a63eb47685713f950a6bc8b3fc19"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"a195ccbd76314ddf88a20fe0c140829432a0ec10","unresolved":false,"context_lines":[{"line_number":2657,"context_line":"                return"},{"line_number":2658,"context_line":""},{"line_number":2659,"context_line":"        try:"},{"line_number":2660,"context_line":"            self.driver.unplug_vifs(instance, network_info)"},{"line_number":2661,"context_line":"        except NotImplementedError:"},{"line_number":2662,"context_line":"            # This is an optional method so ignore things if it doesn\u0027t exist"},{"line_number":2663,"context_line":"            LOG.debug("}],"source_content_type":"text/x-python","patch_set":16,"id":"1fa4df85_22798dfb","line":2660,"range":{"start_line":2660,"start_character":12,"end_line":2660,"end_character":59},"in_reply_to":"1fa4df85_4d7e3fe5","updated":"2020-02-26 14:35:13.000000000","message":"Hmm, that\u0027s another code path. Man, I need to go audit all these things :( I think this fix is an improvement on what we have though and should fixes the immediate issue described in the bug report","commit_id":"83fbec245c51a63eb47685713f950a6bc8b3fc19"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"3024a0eb10aa2e91e9137d8916ee2e6d801cca03","unresolved":false,"context_lines":[{"line_number":2646,"context_line":"            except Exception as exc:"},{"line_number":2647,"context_line":"                LOG.warning("},{"line_number":2648,"context_line":"                    \u0027Failed to update network info cache when cleaning up \u0027"},{"line_number":2649,"context_line":"                    \u0027allocated networks. Stale VIFs may be left on this host.\u0027"},{"line_number":2650,"context_line":"                    \u0027Error: %s\u0027, six.text_type(exc)"},{"line_number":2651,"context_line":"                )"},{"line_number":2652,"context_line":"                return"},{"line_number":2653,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"1fa4df85_0e4b532b","line":2650,"range":{"start_line":2649,"start_character":77,"end_line":2650,"end_character":21},"updated":"2020-02-26 23:04:10.000000000","message":"nit: missing space","commit_id":"b3e14931d6aac6ee5776ce1e6974c75a5a6b1823"}],"nova/tests/functional/test_compute_mgr.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cb34ce87cddd6bafebc6274f9796ebcee3418b97","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        # The NeutronFixture is needed to stub out \u0027get_instance_nw_info\u0027 used"},{"line_number":37,"context_line":"        # as part of cleanup"},{"line_number":38,"context_line":"        def get_instance_nw_info(self, *args, **kwargs):"},{"line_number":39,"context_line":"            return"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"        self.stub_out(\u0027nova.network.neutronv2.api.API.get_instance_nw_info\u0027,"},{"line_number":42,"context_line":"                      get_instance_nw_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fb8cfa7_c7c13a31","line":39,"range":{"start_line":39,"start_character":11,"end_line":39,"end_character":18},"updated":"2019-06-26 12:26:59.000000000","message":"i would prefer if this actully returned an empty nova.network.models.NetworkInfo object instead of none\n\nthe real function never returns none\nfor nova networks we intiallise the result here\n\nhttps://opendev.org/openstack/nova/src/branch/master/nova/network/manager.py#L537\n\nthen we build up a list of vifs, loop over the vifs and add them to the network info object before returning it\nhttps://opendev.org/openstack/nova/src/branch/master/nova/network/manager.py#L592-L597\n\nfor neutron we also always creates an empty NetworkInfo object and populate it https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L1745-L1763\n\nso it will always be an empty NetworkInfo object or a populated one but never None.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"c3144f952dc597e77a9b6d76f478b6ed0ab7dbe9","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        # The NeutronFixture is needed to stub out \u0027get_instance_nw_info\u0027 used"},{"line_number":37,"context_line":"        # as part of cleanup"},{"line_number":38,"context_line":"        def get_instance_nw_info(self, *args, **kwargs):"},{"line_number":39,"context_line":"            return"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"        self.stub_out(\u0027nova.network.neutronv2.api.API.get_instance_nw_info\u0027,"},{"line_number":42,"context_line":"                      get_instance_nw_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7faddb67_a40a531d","line":39,"range":{"start_line":39,"start_character":11,"end_line":39,"end_character":18},"in_reply_to":"9fb8cfa7_c7c13a31","updated":"2019-08-12 16:36:17.000000000","message":"Good point. Done.","commit_id":"1ac4e5c0fbcfce6698e5f17e827ed5057504edce"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9670ed1b23db405a2b1c388a53fd4a821af12dd4","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        # The NeutronFixture is needed to stub out \u0027get_instance_nw_info\u0027 used"},{"line_number":38,"context_line":"        # as part of cleanup"},{"line_number":39,"context_line":"        def get_instance_nw_info(self, *args, **kwargs):"},{"line_number":40,"context_line":"            return network_model.NetworkInfo()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        self.stub_out(\u0027nova.network.neutronv2.api.API.get_instance_nw_info\u0027,"},{"line_number":43,"context_line":"                      get_instance_nw_info)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def test_instance_fault_message_no_traceback_with_retry(self):"},{"line_number":46,"context_line":"        \"\"\"This test simulates a spawn failure on the last retry attempt."}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_61da511e","line":43,"range":{"start_line":39,"start_character":0,"end_line":43,"end_character":43},"updated":"2019-10-17 22:09:47.000000000","message":"Prefer to spell this\n\n self.useFixture(fixtures.MockPatch(\n    \u0027nova.network.neutronv2.api.API.get_instance_nw_info\u0027,\n    return_value\u003dnetwork_model.NetworkInfo()))\n\nOr if it\u0027s important that NetworkInfo() not be called until get_instance_nw_info is invoked, use\n\n    side_effect\u003dnetwork_model.NetworkInfo","commit_id":"8c576c9fb1e1f82043aa7949f830a1b903c26b0a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"bd7cb68edbf346eee8974cc4f2598db857ae6d6f","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        # The NeutronFixture is needed to stub out \u0027get_instance_nw_info\u0027 used"},{"line_number":38,"context_line":"        # as part of cleanup"},{"line_number":39,"context_line":"        def get_instance_nw_info(self, *args, **kwargs):"},{"line_number":40,"context_line":"            return network_model.NetworkInfo()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        self.stub_out(\u0027nova.network.neutronv2.api.API.get_instance_nw_info\u0027,"},{"line_number":43,"context_line":"                      get_instance_nw_info)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def test_instance_fault_message_no_traceback_with_retry(self):"},{"line_number":46,"context_line":"        \"\"\"This test simulates a spawn failure on the last retry attempt."}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_5c0b737c","line":43,"range":{"start_line":39,"start_character":0,"end_line":43,"end_character":43},"in_reply_to":"3fa7e38b_5911455d","updated":"2019-10-18 10:43:32.000000000","message":"Done","commit_id":"8c576c9fb1e1f82043aa7949f830a1b903c26b0a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"3103ff29715f10e86de334cac25d6b22af2a9bda","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        # The NeutronFixture is needed to stub out \u0027get_instance_nw_info\u0027 used"},{"line_number":38,"context_line":"        # as part of cleanup"},{"line_number":39,"context_line":"        def get_instance_nw_info(self, *args, **kwargs):"},{"line_number":40,"context_line":"            return network_model.NetworkInfo()"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        self.stub_out(\u0027nova.network.neutronv2.api.API.get_instance_nw_info\u0027,"},{"line_number":43,"context_line":"                      get_instance_nw_info)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def test_instance_fault_message_no_traceback_with_retry(self):"},{"line_number":46,"context_line":"        \"\"\"This test simulates a spawn failure on the last retry attempt."}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_5911455d","line":43,"range":{"start_line":39,"start_character":0,"end_line":43,"end_character":43},"in_reply_to":"3fa7e38b_61da511e","updated":"2019-10-18 10:05:12.000000000","message":"Agree. I\u0027ll update this at some point during an inevitable rebase due to the patches below.","commit_id":"8c576c9fb1e1f82043aa7949f830a1b903c26b0a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"c8b02ddaa38df7d025099eba9a9fcb07c702ae1f","unresolved":false,"context_lines":[{"line_number":10,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"import fixtures"},{"line_number":14,"context_line":"import mock"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"from nova import context"}],"source_content_type":"text/x-python","patch_set":9,"id":"3fa7e38b_e515e2e6","line":13,"updated":"2019-10-18 13:57:37.000000000","message":"Well this is fun. This works in py3 because it imports \u0027fixtures\u0027, which has MockPatch. It fails in py2 because it imports \u0027nova.tests.functional.fixtures\u0027, which doesn\u0027t.","commit_id":"fc1ece0b71c95ff9ddfe657b04671f312da24b2a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0289638dc904a280207b70e30b8b937b1aee4059","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        fake_server_actions.stub_out_action_events(self)"},{"line_number":38,"context_line":"        fake_network.set_stub_network_methods(self)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        # the NeutronFixture is needed to stub out \u0027get_instance_nw_info\u0027 used"},{"line_number":41,"context_line":"        # as part of cleanup"},{"line_number":42,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":43,"context_line":"            \u0027nova.network.neutron.API.get_instance_nw_info\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_b7c63916","line":40,"range":{"start_line":40,"start_character":14,"end_line":40,"end_character":28},"updated":"2020-02-25 08:42:07.000000000","message":"But the NeutronFixture is not used here. Or do I miss something?","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ce2eba7f3b060ce34029f963735ca99fe42524c4","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        fake_server_actions.stub_out_action_events(self)"},{"line_number":38,"context_line":"        fake_network.set_stub_network_methods(self)"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        # the NeutronFixture is needed to stub out \u0027get_instance_nw_info\u0027 used"},{"line_number":41,"context_line":"        # as part of cleanup"},{"line_number":42,"context_line":"        self.useFixture(fixtures.MockPatch("},{"line_number":43,"context_line":"            \u0027nova.network.neutron.API.get_instance_nw_info\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_0304af16","line":40,"range":{"start_line":40,"start_character":14,"end_line":40,"end_character":28},"in_reply_to":"1fa4df85_b7c63916","updated":"2020-02-25 11:23:42.000000000","message":"Yeah, I\u0027m not sure what I was at here. It is necessary though","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9670ed1b23db405a2b1c388a53fd4a821af12dd4","unresolved":false,"context_lines":[{"line_number":7354,"context_line":"            self.instance, mock_nwinfo.return_value)"},{"line_number":7355,"context_line":"        mock_save.assert_called_once_with()"},{"line_number":7356,"context_line":"        self.assertEqual("},{"line_number":7357,"context_line":"            \u0027False\u0027, self.instance.system_metadata[\u0027network_allocated\u0027])"},{"line_number":7358,"context_line":""},{"line_number":7359,"context_line":"    def test_deallocate_network_none_requested(self):"},{"line_number":7360,"context_line":"        # Tests that we don\u0027t deallocate networks if \u0027none\u0027 were"}],"source_content_type":"text/x-python","patch_set":7,"id":"3fa7e38b_a1d7e9ee","line":7357,"range":{"start_line":7357,"start_character":12,"end_line":7357,"end_character":19},"updated":"2019-10-17 22:09:47.000000000","message":"weird","commit_id":"8c576c9fb1e1f82043aa7949f830a1b903c26b0a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0289638dc904a280207b70e30b8b937b1aee4059","unresolved":false,"context_lines":[{"line_number":7353,"context_line":"        mock_unplug.assert_called_once_with("},{"line_number":7354,"context_line":"            self.instance, mock_nwinfo.return_value)"},{"line_number":7355,"context_line":"        mock_save.assert_called_once_with()"},{"line_number":7356,"context_line":"        self.assertEqual("},{"line_number":7357,"context_line":"            \u0027False\u0027, self.instance.system_metadata[\u0027network_allocated\u0027])"},{"line_number":7358,"context_line":""},{"line_number":7359,"context_line":"    def test_deallocate_network_none_requested(self):"}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_d7cbb51b","line":7356,"range":{"start_line":7356,"start_character":8,"end_line":7356,"end_character":24},"updated":"2020-02-25 08:42:07.000000000","message":"nit: self.assertFalse(","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"ce2eba7f3b060ce34029f963735ca99fe42524c4","unresolved":false,"context_lines":[{"line_number":7353,"context_line":"        mock_unplug.assert_called_once_with("},{"line_number":7354,"context_line":"            self.instance, mock_nwinfo.return_value)"},{"line_number":7355,"context_line":"        mock_save.assert_called_once_with()"},{"line_number":7356,"context_line":"        self.assertEqual("},{"line_number":7357,"context_line":"            \u0027False\u0027, self.instance.system_metadata[\u0027network_allocated\u0027])"},{"line_number":7358,"context_line":""},{"line_number":7359,"context_line":"    def test_deallocate_network_none_requested(self):"}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_e3063321","line":7356,"range":{"start_line":7356,"start_character":8,"end_line":7356,"end_character":24},"in_reply_to":"1fa4df85_d7cbb51b","updated":"2020-02-25 11:23:42.000000000","message":"Can\u0027t - False is a string, not a bool","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"efceb1bcf3c8ce6f7703beba3af66b37cf8bdb06","unresolved":false,"context_lines":[{"line_number":7353,"context_line":"        mock_unplug.assert_called_once_with("},{"line_number":7354,"context_line":"            self.instance, mock_nwinfo.return_value)"},{"line_number":7355,"context_line":"        mock_save.assert_called_once_with()"},{"line_number":7356,"context_line":"        self.assertEqual("},{"line_number":7357,"context_line":"            \u0027False\u0027, self.instance.system_metadata[\u0027network_allocated\u0027])"},{"line_number":7358,"context_line":""},{"line_number":7359,"context_line":"    def test_deallocate_network_none_requested(self):"}],"source_content_type":"text/x-python","patch_set":14,"id":"1fa4df85_67da4eea","line":7356,"range":{"start_line":7356,"start_character":8,"end_line":7356,"end_character":24},"in_reply_to":"1fa4df85_e3063321","updated":"2020-02-25 12:00:37.000000000","message":"True. My bad.","commit_id":"751a0f0ceb49fac9797d16abbfdc4a013870ef7c"}],"nova/tests/unit/conductor/test_conductor.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d72681b9aab5910e79576727a397223f8b584325","unresolved":false,"context_lines":[{"line_number":3140,"context_line":"                with mock.patch.object(fake_inst, \u0027save\u0027) as mock_save:"},{"line_number":3141,"context_line":"                    self.conductor._cleanup_allocated_networks("},{"line_number":3142,"context_line":"                        self.context, fake_inst, req_net)"},{"line_number":3143,"context_line":"        deallocate.assert_called_once_with("},{"line_number":3144,"context_line":"            self.context, fake_inst, requested_networks\u003dreq_net)"},{"line_number":3145,"context_line":"        self.assertEqual(\u0027False\u0027,"},{"line_number":3146,"context_line":"                         fake_inst.system_metadata[\u0027network_allocated\u0027],"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_644d3e71","side":"PARENT","line":3143,"range":{"start_line":3143,"start_character":7,"end_line":3143,"end_character":18},"updated":"2019-06-05 22:04:14.000000000","message":"deallocate is only in scope here because python is dumb and leaks varable outside the scope of both with and for loops.\n\nat first the deallocate mock is declared inside the for loop as the alais for the context manager created by the first with block.\n\nso normally we would expect both the context manager created by the nested with blocks to be destroyed when we leave teh loop body but in python that is not what actlly happens.\n\nin the old version here deallocate would be bound to the second context manter that was create so this was previould not testing what peopel though as it was recreate on teh second iteration fo the loop so it could nver be called twice.","commit_id":"f2b58bb20bf5a7b77023d2a2d689631991a8a5e8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"8295985d18ce8cfc5354375956eaf0c0e8b3ff0f","unresolved":false,"context_lines":[{"line_number":3146,"context_line":"            self.assertEqual(\u0027False\u0027,"},{"line_number":3147,"context_line":"                             fake_inst.system_metadata[\u0027network_allocated\u0027],"},{"line_number":3148,"context_line":"                             fake_inst.system_metadata)"},{"line_number":3149,"context_line":"            mock_save.assert_called_once_with()"},{"line_number":3150,"context_line":""},{"line_number":3151,"context_line":"    @mock.patch.object(objects.ComputeNode, \u0027get_by_host_and_nodename\u0027,"},{"line_number":3152,"context_line":"                       side_effect\u003dexc.ComputeHostNotFound(\u0027source-host\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_426d8f30","line":3149,"updated":"2019-06-05 20:04:38.000000000","message":"Why did you indent this?\n\nI guess it\u0027s now slightly more accurate because it\u0027s validating that the calls are made on the first (replete) set of networks and then not made the second time with None.","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b0fabb8f8e0711876987f36b8e6d683c9e7864b6","unresolved":false,"context_lines":[{"line_number":3146,"context_line":"            self.assertEqual(\u0027False\u0027,"},{"line_number":3147,"context_line":"                             fake_inst.system_metadata[\u0027network_allocated\u0027],"},{"line_number":3148,"context_line":"                             fake_inst.system_metadata)"},{"line_number":3149,"context_line":"            mock_save.assert_called_once_with()"},{"line_number":3150,"context_line":""},{"line_number":3151,"context_line":"    @mock.patch.object(objects.ComputeNode, \u0027get_by_host_and_nodename\u0027,"},{"line_number":3152,"context_line":"                       side_effect\u003dexc.ComputeHostNotFound(\u0027source-host\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_ae5f833f","line":3149,"in_reply_to":"9fb8cfa7_426d8f30","updated":"2019-06-21 11:37:15.000000000","message":"\u003e Why did you indent this?\n \u003e \n \u003e I guess it\u0027s now slightly more accurate because it\u0027s validating\n \u003e that the calls are made on the first (replete) set of networks and\n \u003e then not made the second time with None.\n\nYeah, as it stood, we were only testing the results of the first call and not both calls, and I assume the latter was the original intent so this was a bug.\n\n*However*, this is unrelated so I\u0027ve dropped the hunk","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d72681b9aab5910e79576727a397223f8b584325","unresolved":false,"context_lines":[{"line_number":3146,"context_line":"            self.assertEqual(\u0027False\u0027,"},{"line_number":3147,"context_line":"                             fake_inst.system_metadata[\u0027network_allocated\u0027],"},{"line_number":3148,"context_line":"                             fake_inst.system_metadata)"},{"line_number":3149,"context_line":"            mock_save.assert_called_once_with()"},{"line_number":3150,"context_line":""},{"line_number":3151,"context_line":"    @mock.patch.object(objects.ComputeNode, \u0027get_by_host_and_nodename\u0027,"},{"line_number":3152,"context_line":"                       side_effect\u003dexc.ComputeHostNotFound(\u0027source-host\u0027))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_a44a5652","line":3149,"in_reply_to":"9fb8cfa7_426d8f30","updated":"2019-06-05 22:04:14.000000000","message":"this helps with the unintuitive scoping but you need to pull the with context manager above the for other wise they will be recreated one each iteration of the for and this still  test what you want it to test.","commit_id":"fd5d55ede24a9cb5f6fa0eed1e8bd4aaa4d11011"}]}