)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2308ce2799f95f1f29e7f33868b32f92fbfaea25","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"detach instance volumes when VM creation fails"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"If the boot-volume creation fails, the data volume is left in state \"in-use\", attached to the server which is now in \"error\" state\". The user can\u0027t detach the volume because of the server\u0027s error state. They can delete the server, which then leaves the volume apparently attached to a server that no longer exists."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Change-Id: I8b1c05317734e14ea73dc868941351bb31210bf0"},{"line_number":12,"context_line":"Closes-bug: #1633249"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"bf8cb3f7_38c1a181","line":9,"updated":"2017-12-23 14:40:27.000000000","message":"Please wrap this line at 79 characters.","commit_id":"f127639cf0d41eed2855c2be479ca86b92ed859b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f859a5497470fcc9fb4bbcc8419d03587465943f","unresolved":false,"context_lines":[{"line_number":10,"context_line":"If the boot-volume creation fails, the data volume is left in state \"in-use\","},{"line_number":11,"context_line":"attached to the server which is now in \"error\" state\"."},{"line_number":12,"context_line":"The user can\u0027t detach the volume because of the server\u0027s error state."},{"line_number":13,"context_line":"They can delete the server,"},{"line_number":14,"context_line":"which then leaves the volume apparently attached to a server that no longer exists."},{"line_number":15,"context_line":"The only way out of this is to ask an administrator to reset the state of"},{"line_number":16,"context_line":"the data volume (this option is not available to regular users by default policy)."},{"line_number":17,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"5f93b717_5e00c3ab","line":14,"range":{"start_line":13,"start_character":0,"end_line":14,"end_character":83},"updated":"2018-01-30 14:52:43.000000000","message":"This is the real bug, and is being addressed in:\n\nhttps://review.openstack.org/#/c/340614/","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"08c244a21f4af3281eea9f94425baf1074950cf1","unresolved":false,"context_lines":[{"line_number":21,"context_line":"so you can delete the instance without care about attached volumes."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I8b1c05317734e14ea73dc868941351bb31210bf0"},{"line_number":24,"context_line":"Closes-bug: #1633249 (reverted from commit e184d62f61a8adc78e776526625bdf5c1d0439c5)"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"5f93b717_9e23ab56","line":24,"range":{"start_line":24,"start_character":21,"end_line":24,"end_character":84},"updated":"2018-01-30 14:52:58.000000000","message":"What is this?","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a3b202348751508c31c3a192b9ff545286eb0a8","unresolved":false,"context_lines":[{"line_number":24,"context_line":"so you can delete the instance without care about attached volumes."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Mel\u0027s patach resolves any other issues will happen while instance is running."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: I8b1c05317734e14ea73dc868941351bb31210bf0"},{"line_number":29,"context_line":"Closes-bug: #1633249"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"3fa0c359_dc4462ba","line":27,"updated":"2018-02-05 10:35:11.000000000","message":"nit: wrap lines at 72 chars.","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"d2343828491b0602af8136c915cfa27229a9272b","unresolved":false,"context_lines":[{"line_number":24,"context_line":"so you can delete the instance without care about attached volumes."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Mel\u0027s patach resolves any other issues will happen while instance is running."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"Change-Id: I8b1c05317734e14ea73dc868941351bb31210bf0"},{"line_number":29,"context_line":"Closes-bug: #1633249"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"3fa0c359_adbcc2bb","line":27,"in_reply_to":"3fa0c359_dc4462ba","updated":"2018-02-05 10:50:05.000000000","message":"Ok\nThanks!","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"345ba0157039b2b1af50a363c95c4ad4e6c1f5dd","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"bug description:"},{"line_number":10,"context_line":"If the boot-volume creation fails, the data volume is left in state"},{"line_number":11,"context_line":"\"in-use\", ttached to the server which is now in \"error\" state\"."},{"line_number":12,"context_line":"The user can\u0027t detach the volume because of the server\u0027s error state."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"They can delete the server,which then leaves the volume apparently"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"3fa0c359_065fc2e5","line":11,"range":{"start_line":11,"start_character":10,"end_line":11,"end_character":17},"updated":"2018-02-15 16:03:01.000000000","message":"attached","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"345ba0157039b2b1af50a363c95c4ad4e6c1f5dd","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"bug description:"},{"line_number":10,"context_line":"If the boot-volume creation fails, the data volume is left in state"},{"line_number":11,"context_line":"\"in-use\", ttached to the server which is now in \"error\" state\"."},{"line_number":12,"context_line":"The user can\u0027t detach the volume because of the server\u0027s error state."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"They can delete the server,which then leaves the volume apparently"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"3fa0c359_8672f27b","line":11,"range":{"start_line":11,"start_character":61,"end_line":11,"end_character":62},"updated":"2018-02-15 16:03:01.000000000","message":"don\u0027t need this double quote","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"345ba0157039b2b1af50a363c95c4ad4e6c1f5dd","unresolved":false,"context_lines":[{"line_number":11,"context_line":"\"in-use\", ttached to the server which is now in \"error\" state\"."},{"line_number":12,"context_line":"The user can\u0027t detach the volume because of the server\u0027s error state."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"They can delete the server,which then leaves the volume apparently"},{"line_number":15,"context_line":"attached to a server that no longer exists."},{"line_number":16,"context_line":"\"Mel\u0027s patch resolves this failure"},{"line_number":17,"context_line":" https://review.openstack.org/#/c/340614/ \""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"3fa0c359_26099edc","line":14,"range":{"start_line":14,"start_character":26,"end_line":14,"end_character":27},"updated":"2018-02-15 16:03:01.000000000","message":", which","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"345ba0157039b2b1af50a363c95c4ad4e6c1f5dd","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"They can delete the server,which then leaves the volume apparently"},{"line_number":15,"context_line":"attached to a server that no longer exists."},{"line_number":16,"context_line":"\"Mel\u0027s patch resolves this failure"},{"line_number":17,"context_line":" https://review.openstack.org/#/c/340614/ \""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The only way out of this is to ask an administrator to reset the state of"},{"line_number":20,"context_line":"the data volume (this option is not available to regular users by"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"3fa0c359_e612d609","line":17,"range":{"start_line":16,"start_character":0,"end_line":17,"end_character":43},"updated":"2018-02-15 16:03:01.000000000","message":"Don\u0027t really need the quotes here.","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"345ba0157039b2b1af50a363c95c4ad4e6c1f5dd","unresolved":false,"context_lines":[{"line_number":16,"context_line":"\"Mel\u0027s patch resolves this failure"},{"line_number":17,"context_line":" https://review.openstack.org/#/c/340614/ \""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The only way out of this is to ask an administrator to reset the state of"},{"line_number":20,"context_line":"the data volume (this option is not available to regular users by"},{"line_number":21,"context_line":"default policy)."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"how the fix resolves the issue:"},{"line_number":24,"context_line":"When the creation fails, compute manager detach the created volumes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"3fa0c359_062d8245","line":21,"range":{"start_line":19,"start_character":0,"end_line":21,"end_character":16},"updated":"2018-02-15 16:03:01.000000000","message":"Or force detach the volume:\n\nhttps://developer.openstack.org/api-ref/block-storage/v3/#force-detach-a-volume\n\nBut that\u0027s also admin-only by default.","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"}],"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2308ce2799f95f1f29e7f33868b32f92fbfaea25","unresolved":false,"context_lines":[{"line_number":1938,"context_line":"                    requested_networks)"},{"line_number":1939,"context_line":"            # Detach volumes when VM creation failed"},{"line_number":1940,"context_line":"            detach_block_devices(context, block_device_mapping)"},{"line_number":1941,"context_line":"            self._cleanup_volumes(context, instance.uuid,"},{"line_number":1942,"context_line":"                    block_device_mapping, raise_exc\u003dFalse)"},{"line_number":1943,"context_line":"            compute_utils.add_instance_fault_from_exc(context, instance,"},{"line_number":1944,"context_line":"                    e, sys.exc_info())"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf8cb3f7_d8bf55f9","line":1941,"range":{"start_line":1941,"start_character":17,"end_line":1941,"end_character":33},"updated":"2017-12-23 14:40:27.000000000","message":"Why not just handle this in _cleanup_volumes?","commit_id":"f127639cf0d41eed2855c2be479ca86b92ed859b"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"98135335bcf356c177531c5aeeeff459479deaab","unresolved":false,"context_lines":[{"line_number":1938,"context_line":"                    requested_networks)"},{"line_number":1939,"context_line":"            # Detach volumes when VM creation failed"},{"line_number":1940,"context_line":"            detach_block_devices(context, block_device_mapping)"},{"line_number":1941,"context_line":"            self._cleanup_volumes(context, instance.uuid,"},{"line_number":1942,"context_line":"                    block_device_mapping, raise_exc\u003dFalse)"},{"line_number":1943,"context_line":"            compute_utils.add_instance_fault_from_exc(context, instance,"},{"line_number":1944,"context_line":"                    e, sys.exc_info())"}],"source_content_type":"text/x-python","patch_set":2,"id":"bf8cb3f7_f8eed9b2","line":1941,"range":{"start_line":1941,"start_character":17,"end_line":1941,"end_character":33},"in_reply_to":"bf8cb3f7_d8bf55f9","updated":"2017-12-24 06:32:44.000000000","message":"At First I handle that in _cleanup_volumes, but when I see that the _cleanup_volumes used in other places, I prefer to do that as now. please let me know what do you think.","commit_id":"f127639cf0d41eed2855c2be479ca86b92ed859b"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"b1cb083715787d357a2f04cb2837b58194095b98","unresolved":false,"context_lines":[{"line_number":1819,"context_line":"            requested_networks, security_groups, block_device_mapping,"},{"line_number":1820,"context_line":"            node\u003dNone, limits\u003dNone):"},{"line_number":1821,"context_line":""},{"line_number":1822,"context_line":"        def detach_block_devices(context, bdms):"},{"line_number":1823,"context_line":"            for bdm in bdms:"},{"line_number":1824,"context_line":"                if bdm.is_volume and bdm.volume_id:"},{"line_number":1825,"context_line":"                    self._detach_volume(context, bdm, instance,"}],"source_content_type":"text/x-python","patch_set":4,"id":"9f91af0f_8770a574","line":1822,"range":{"start_line":1822,"start_character":0,"end_line":1822,"end_character":48},"updated":"2018-01-02 14:19:34.000000000","message":"Why can\u0027t this be done as part of \u0027_cleanup_volumes\u0027?","commit_id":"fcba6d103c239842e825fe805ca144f69e9f0b1f"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"986d4341103310c3e501343960d573bd19bceb7b","unresolved":false,"context_lines":[{"line_number":2437,"context_line":"                context, instance_uuid,"},{"line_number":2438,"context_line":"                expected_attrs\u003d[])"},{"line_number":2439,"context_line":"        except exception.InstanceNotFound:"},{"line_number":2440,"context_line":"            LOG.info(\"Instance is not found during confirmation\","},{"line_number":2441,"context_line":"                     instance\u003dinstance)"},{"line_number":2442,"context_line":""},{"line_number":2443,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f96bb07_dd516b64","line":2440,"range":{"start_line":2440,"start_character":51,"end_line":2440,"end_character":63},"updated":"2018-01-15 08:12:30.000000000","message":"confirmation?","commit_id":"48cc577e607db2ecee99f7356dcb9dea8e920074"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"521a23f7cab71a10686e83ec9f4e3974ba307f61","unresolved":false,"context_lines":[{"line_number":2437,"context_line":"                context, instance_uuid,"},{"line_number":2438,"context_line":"                expected_attrs\u003d[])"},{"line_number":2439,"context_line":"        except exception.InstanceNotFound:"},{"line_number":2440,"context_line":"            LOG.info(\"Instance is not found during confirmation\","},{"line_number":2441,"context_line":"                     instance\u003dinstance)"},{"line_number":2442,"context_line":""},{"line_number":2443,"context_line":"        for bdm in bdms:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f96bb07_63d67452","line":2440,"range":{"start_line":2440,"start_character":51,"end_line":2440,"end_character":63},"in_reply_to":"7f96bb07_dd516b64","updated":"2018-01-15 09:06:30.000000000","message":"copy and paste typo :$","commit_id":"48cc577e607db2ecee99f7356dcb9dea8e920074"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"986d4341103310c3e501343960d573bd19bceb7b","unresolved":false,"context_lines":[{"line_number":2443,"context_line":"        for bdm in bdms:"},{"line_number":2444,"context_line":"            LOG.debug(\"detaching bdm %s\", bdm,"},{"line_number":2445,"context_line":"                      instance_uuid\u003dinstance_uuid)"},{"line_number":2446,"context_line":"            if bdm.is_volume and instance and bdm.volume_id:"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7f96bb07_bd89ffca","line":2446,"range":{"start_line":2446,"start_character":30,"end_line":2446,"end_character":41},"updated":"2018-01-15 08:12:30.000000000","message":"why check instance ?","commit_id":"48cc577e607db2ecee99f7356dcb9dea8e920074"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"521a23f7cab71a10686e83ec9f4e3974ba307f61","unresolved":false,"context_lines":[{"line_number":2443,"context_line":"        for bdm in bdms:"},{"line_number":2444,"context_line":"            LOG.debug(\"detaching bdm %s\", bdm,"},{"line_number":2445,"context_line":"                      instance_uuid\u003dinstance_uuid)"},{"line_number":2446,"context_line":"            if bdm.is_volume and instance and bdm.volume_id:"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7f96bb07_836e2093","line":2446,"range":{"start_line":2446,"start_character":30,"end_line":2446,"end_character":41},"in_reply_to":"7f96bb07_bd89ffca","updated":"2018-01-15 09:06:30.000000000","message":"_detach_volume take instance as parameter\n-\u003e :param instance: the Instance object to detach the volume from","commit_id":"48cc577e607db2ecee99f7356dcb9dea8e920074"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"986d4341103310c3e501343960d573bd19bceb7b","unresolved":false,"context_lines":[{"line_number":2445,"context_line":"                      instance_uuid\u003dinstance_uuid)"},{"line_number":2446,"context_line":"            if bdm.is_volume and instance and bdm.volume_id:"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""},{"line_number":2450,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2451,"context_line":"                      instance_uuid\u003dinstance_uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f96bb07_7d5a974f","line":2448,"range":{"start_line":2448,"start_character":36,"end_line":2448,"end_character":47},"updated":"2018-01-15 08:12:30.000000000","message":"seems we can check whether bdm want to be delete here to avoid another API call?","commit_id":"48cc577e607db2ecee99f7356dcb9dea8e920074"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"521a23f7cab71a10686e83ec9f4e3974ba307f61","unresolved":false,"context_lines":[{"line_number":2445,"context_line":"                      instance_uuid\u003dinstance_uuid)"},{"line_number":2446,"context_line":"            if bdm.is_volume and instance and bdm.volume_id:"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""},{"line_number":2450,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2451,"context_line":"                      instance_uuid\u003dinstance_uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f96bb07_6332f4a9","line":2448,"range":{"start_line":2448,"start_character":36,"end_line":2448,"end_character":47},"in_reply_to":"7f96bb07_7d5a974f","updated":"2018-01-15 09:06:30.000000000","message":"what do you mean?","commit_id":"48cc577e607db2ecee99f7356dcb9dea8e920074"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c3b8927c4c5cfa98356a7c0ecc853d99a398fdb","unresolved":false,"context_lines":[{"line_number":2389,"context_line":"                    self.volume_api.terminate_connection(context,"},{"line_number":2390,"context_line":"                                                         bdm.volume_id,"},{"line_number":2391,"context_line":"                                                         connector)"},{"line_number":2392,"context_line":"                    self.volume_api.detach(context, bdm.volume_id,"},{"line_number":2393,"context_line":"                                           instance.uuid)"},{"line_number":2394,"context_line":""},{"line_number":2395,"context_line":"            except exception.VolumeAttachmentNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f93b717_29811690","line":2392,"updated":"2018-01-29 20:51:16.000000000","message":"When deleting a server, we attempt to detach the volume here. So if this is failing for some reason, your change isn\u0027t going to fix it. We\u0027d need more details about the actual error and how your fix resolves the issue in the commit message.","commit_id":"934032adddb8c4681672345e56f3b529dd04fdba"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"5147c18a53f268097be5955f8483cca7ab3030ee","unresolved":false,"context_lines":[{"line_number":1924,"context_line":"            LOG.exception(e.format_message(), instance\u003dinstance)"},{"line_number":1925,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":1926,"context_line":"                    requested_networks)"},{"line_number":1927,"context_line":"            self._cleanup_volumes(context, instance.uuid,"},{"line_number":1928,"context_line":"                    block_device_mapping, raise_exc\u003dFalse)"},{"line_number":1929,"context_line":"            compute_utils.add_instance_fault_from_exc(context, instance,"},{"line_number":1930,"context_line":"                    e, sys.exc_info())"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_7b71110c","line":1927,"range":{"start_line":1927,"start_character":17,"end_line":1927,"end_character":33},"updated":"2018-01-30 14:36:41.000000000","message":"as bug scenario, it hit this section of code, which never go to shutdown_instance","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ecaf3d46e02ff64c9ee601dda6905d026c4c76ef","unresolved":false,"context_lines":[{"line_number":1924,"context_line":"            LOG.exception(e.format_message(), instance\u003dinstance)"},{"line_number":1925,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":1926,"context_line":"                    requested_networks)"},{"line_number":1927,"context_line":"            self._cleanup_volumes(context, instance.uuid,"},{"line_number":1928,"context_line":"                    block_device_mapping, raise_exc\u003dFalse)"},{"line_number":1929,"context_line":"            compute_utils.add_instance_fault_from_exc(context, instance,"},{"line_number":1930,"context_line":"                    e, sys.exc_info())"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_7e5edf12","line":1927,"range":{"start_line":1927,"start_character":17,"end_line":1927,"end_character":33},"in_reply_to":"5f93b717_7b71110c","updated":"2018-01-30 14:50:12.000000000","message":"OK so let\u0027s say we created a volume and attached it to the instance, so the volume is in-use status, and when we go to delete it here, it fails because the volume is in-use (needs to be available (detached) to delete it).","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ecaf3d46e02ff64c9ee601dda6905d026c4c76ef","unresolved":false,"context_lines":[{"line_number":1928,"context_line":"                    block_device_mapping, raise_exc\u003dFalse)"},{"line_number":1929,"context_line":"            compute_utils.add_instance_fault_from_exc(context, instance,"},{"line_number":1930,"context_line":"                    e, sys.exc_info())"},{"line_number":1931,"context_line":"            self._nil_out_instance_obj_host_and_node(instance)"},{"line_number":1932,"context_line":"            self._set_instance_obj_error_state(context, instance,"},{"line_number":1933,"context_line":"                                               clean_task_state\u003dTrue)"},{"line_number":1934,"context_line":"            return build_results.FAILED"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_3ef057fe","line":1931,"range":{"start_line":1931,"start_character":17,"end_line":1931,"end_character":52},"updated":"2018-01-30 14:50:12.000000000","message":"This sets the instance.host value to None. So when the instance is deleted, we wouldn\u0027t have a compute host to RPC cast to to call _shutdown_instance. This means the instance must be deleted from the API, and the BDMs get cleaned up there, which is this fix from melwitt:\n\nhttps://review.openstack.org/#/c/340614/","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0ef4940abb903ce5198d75c6f33d75e778cf281d","unresolved":false,"context_lines":[{"line_number":2389,"context_line":"                    self.volume_api.terminate_connection(context,"},{"line_number":2390,"context_line":"                                                         bdm.volume_id,"},{"line_number":2391,"context_line":"                                                         connector)"},{"line_number":2392,"context_line":"                    self.volume_api.detach(context, bdm.volume_id,"},{"line_number":2393,"context_line":"                                           instance.uuid)"},{"line_number":2394,"context_line":""},{"line_number":2395,"context_line":"            except exception.VolumeAttachmentNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_db6dc5cc","line":2392,"updated":"2018-01-30 14:13:48.000000000","message":"As I said in the last patch set, you haven\u0027t explained why this isn\u0027t working.","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"0408b439af8918b2bf204bf86b395c02efe67df5","unresolved":false,"context_lines":[{"line_number":2389,"context_line":"                    self.volume_api.terminate_connection(context,"},{"line_number":2390,"context_line":"                                                         bdm.volume_id,"},{"line_number":2391,"context_line":"                                                         connector)"},{"line_number":2392,"context_line":"                    self.volume_api.detach(context, bdm.volume_id,"},{"line_number":2393,"context_line":"                                           instance.uuid)"},{"line_number":2394,"context_line":""},{"line_number":2395,"context_line":"            except exception.VolumeAttachmentNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_3bc3895f","line":2392,"in_reply_to":"5f93b717_db6dc5cc","updated":"2018-01-30 14:27:51.000000000","message":"sorry I didn\u0027t notice you ask me to explain this, \n\n_do_build_and_run_instance doesn\u0027t hit _shutdown_instance at all.","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ecaf3d46e02ff64c9ee601dda6905d026c4c76ef","unresolved":false,"context_lines":[{"line_number":2434,"context_line":"        exc_info \u003d None"},{"line_number":2435,"context_line":""},{"line_number":2436,"context_line":"        try:"},{"line_number":2437,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2438,"context_line":"                context, instance_uuid,"},{"line_number":2439,"context_line":"                expected_attrs\u003d[])"},{"line_number":2440,"context_line":"        except exception.InstanceNotFound:"},{"line_number":2441,"context_line":"            LOG.info(\"Instance is not found\","},{"line_number":2442,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_7b02710d","line":2439,"range":{"start_line":2437,"start_character":12,"end_line":2439,"end_character":34},"updated":"2018-01-30 14:50:12.000000000","message":"You\u0027re pulling the instance from the database even though you don\u0027t use this at all.","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"1cf5bb44e7df2954da68fa1cbc3321cbb5297169","unresolved":false,"context_lines":[{"line_number":2434,"context_line":"        exc_info \u003d None"},{"line_number":2435,"context_line":""},{"line_number":2436,"context_line":"        try:"},{"line_number":2437,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2438,"context_line":"                context, instance_uuid,"},{"line_number":2439,"context_line":"                expected_attrs\u003d[])"},{"line_number":2440,"context_line":"        except exception.InstanceNotFound:"},{"line_number":2441,"context_line":"            LOG.info(\"Instance is not found\","},{"line_number":2442,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_9ee88be2","line":2439,"range":{"start_line":2437,"start_character":12,"end_line":2439,"end_character":34},"in_reply_to":"5f93b717_7b02710d","updated":"2018-01-30 15:00:10.000000000","message":"this -\u003e self._detach_volume(context, bdm, instance,...\nuse the instance, so I have to move the code to be inside this condition  if bdm.is_volume and bdm.volume_id ?","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c8454678df2644cc8de839991f5eb24649ca26d2","unresolved":false,"context_lines":[{"line_number":2434,"context_line":"        exc_info \u003d None"},{"line_number":2435,"context_line":""},{"line_number":2436,"context_line":"        try:"},{"line_number":2437,"context_line":"            instance \u003d objects.Instance.get_by_uuid("},{"line_number":2438,"context_line":"                context, instance_uuid,"},{"line_number":2439,"context_line":"                expected_attrs\u003d[])"},{"line_number":2440,"context_line":"        except exception.InstanceNotFound:"},{"line_number":2441,"context_line":"            LOG.info(\"Instance is not found\","},{"line_number":2442,"context_line":"                     instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_de0b737a","line":2439,"range":{"start_line":2437,"start_character":12,"end_line":2439,"end_character":34},"in_reply_to":"5f93b717_9ee88be2","updated":"2018-01-30 15:03:47.000000000","message":"Rather than lookup the instance from the database again, I\u0027d just pass it into the _cleanup_volumes method.","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ecaf3d46e02ff64c9ee601dda6905d026c4c76ef","unresolved":false,"context_lines":[{"line_number":2443,"context_line":"        for bdm in bdms:"},{"line_number":2444,"context_line":"            if bdm.is_volume and bdm.volume_id:"},{"line_number":2445,"context_line":"                volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2446,"context_line":"                if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_5e1a036f","line":2446,"range":{"start_line":2446,"start_character":16,"end_line":2446,"end_character":61},"updated":"2018-01-30 14:50:12.000000000","message":"This status check is potentially a race, and it\u0027s better just to try and detach the volume and handle any failures that happen as a result and move beyond them.","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"1cf5bb44e7df2954da68fa1cbc3321cbb5297169","unresolved":false,"context_lines":[{"line_number":2443,"context_line":"        for bdm in bdms:"},{"line_number":2444,"context_line":"            if bdm.is_volume and bdm.volume_id:"},{"line_number":2445,"context_line":"                volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2446,"context_line":"                if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_5eee83ed","line":2446,"range":{"start_line":2446,"start_character":16,"end_line":2446,"end_character":61},"in_reply_to":"5f93b717_5e1a036f","updated":"2018-01-30 15:00:10.000000000","message":"Thanks ! I will do that","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ecaf3d46e02ff64c9ee601dda6905d026c4c76ef","unresolved":false,"context_lines":[{"line_number":2445,"context_line":"                volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2446,"context_line":"                if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""},{"line_number":2450,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2451,"context_line":"                      instance_uuid\u003dinstance_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_fea74f0a","line":2448,"range":{"start_line":2448,"start_character":36,"end_line":2448,"end_character":53},"updated":"2018-01-30 14:50:12.000000000","message":"If the bdm isn\u0027t destroyed here, but we delete the volume, I guess the only place it would get removed is when the instance itself is deleted.","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c8454678df2644cc8de839991f5eb24649ca26d2","unresolved":false,"context_lines":[{"line_number":2445,"context_line":"                volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2446,"context_line":"                if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""},{"line_number":2450,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2451,"context_line":"                      instance_uuid\u003dinstance_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_be6b071c","line":2448,"range":{"start_line":2448,"start_character":36,"end_line":2448,"end_character":53},"in_reply_to":"5f93b717_5e9c4369","updated":"2018-01-30 15:03:47.000000000","message":"When the instance is in ERROR state and doesn\u0027t have a host, and you delete the instance, the API does a \"local\" delete which does attempt to detach the volume and delete it if bdm.terminate_on_deletion is True, that happens in the API code here:\n\nhttps://github.com/openstack/nova/blob/91f7a999988b3f857d738d39984117e6c514cbec/nova/compute/api.py#L1987\n\nAnd that\u0027s what Mel\u0027s patch is attempting to fix.","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"1cf5bb44e7df2954da68fa1cbc3321cbb5297169","unresolved":false,"context_lines":[{"line_number":2445,"context_line":"                volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2446,"context_line":"                if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2447,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2448,"context_line":"                                    destroy_bdm\u003dFalse)"},{"line_number":2449,"context_line":""},{"line_number":2450,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2451,"context_line":"                      instance_uuid\u003dinstance_uuid)"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_5e9c4369","line":2448,"range":{"start_line":2448,"start_character":36,"end_line":2448,"end_character":53},"in_reply_to":"5f93b717_fea74f0a","updated":"2018-01-30 15:00:10.000000000","message":"I don\u0027t think that when you delete the instance that will delete the volumes. so what do you think if we remove them here ?","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ecaf3d46e02ff64c9ee601dda6905d026c4c76ef","unresolved":false,"context_lines":[{"line_number":2454,"context_line":"                    self.volume_api.delete(context, bdm.volume_id)"},{"line_number":2455,"context_line":"                except Exception as exc:"},{"line_number":2456,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2457,"context_line":"                    LOG.warning(\u0027Failed to delete volume: %(volume_id)s \u0027"},{"line_number":2458,"context_line":"                                \u0027due to %(exc)s\u0027,"},{"line_number":2459,"context_line":"                                {\u0027volume_id\u0027: bdm.volume_id, \u0027exc\u0027: exc})"},{"line_number":2460,"context_line":"        if exc_info is not None and raise_exc:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_9b265dce","line":2457,"updated":"2018-01-30 14:50:12.000000000","message":"So without your change, this fails because the volume is in-use.","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"1cf5bb44e7df2954da68fa1cbc3321cbb5297169","unresolved":false,"context_lines":[{"line_number":2454,"context_line":"                    self.volume_api.delete(context, bdm.volume_id)"},{"line_number":2455,"context_line":"                except Exception as exc:"},{"line_number":2456,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2457,"context_line":"                    LOG.warning(\u0027Failed to delete volume: %(volume_id)s \u0027"},{"line_number":2458,"context_line":"                                \u0027due to %(exc)s\u0027,"},{"line_number":2459,"context_line":"                                {\u0027volume_id\u0027: bdm.volume_id, \u0027exc\u0027: exc})"},{"line_number":2460,"context_line":"        if exc_info is not None and raise_exc:"}],"source_content_type":"text/x-python","patch_set":10,"id":"5f93b717_7e99ff76","line":2457,"in_reply_to":"5f93b717_9b265dce","updated":"2018-01-30 15:00:10.000000000","message":"right!","commit_id":"759a76af746b0d51669a71033e0fecb672fc8d24"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"f38dfb1c198895cb384ae2e5ba5dfc30ba234b5a","unresolved":false,"context_lines":[{"line_number":2436,"context_line":"        for bdm in bdms:"},{"line_number":2437,"context_line":"            if bdm.is_volume and bdm.volume_id and instance:"},{"line_number":2438,"context_line":"                try:"},{"line_number":2439,"context_line":"                    volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2440,"context_line":"                    if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2441,"context_line":"                        self._detach_volume(context, bdm, instance,"},{"line_number":2442,"context_line":"                                                destroy_bdm\u003dFalse)"},{"line_number":2443,"context_line":"                except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":11,"id":"5f93b717_bf2f653d","line":2440,"range":{"start_line":2439,"start_character":20,"end_line":2440,"end_character":65},"updated":"2018-01-31 13:10:32.000000000","message":"I can\u0027t find other way to prevent error from this unit test -\u003e nova.tests.functional.notification_sample_tests.test_instance.TestInstanceNotificationSample.test_create_delete_server","commit_id":"ec44e95cc74798280a7dfa73de01748396630438"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":2439,"context_line":"        exc_info \u003d None"},{"line_number":2440,"context_line":""},{"line_number":2441,"context_line":"        for bdm in bdms:"},{"line_number":2442,"context_line":"            if bdm.is_volume and bdm.volume_id and instance:"},{"line_number":2443,"context_line":"                try:"},{"line_number":2444,"context_line":"                    volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2445,"context_line":"                    if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_08c16fe5","line":2442,"range":{"start_line":2442,"start_character":46,"end_line":2442,"end_character":59},"updated":"2018-02-02 02:05:54.000000000","message":"I don\u0027t think you need this check, you\u0027ve updated all the call sites and we can assume that instance is an instance object.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":2439,"context_line":"        exc_info \u003d None"},{"line_number":2440,"context_line":""},{"line_number":2441,"context_line":"        for bdm in bdms:"},{"line_number":2442,"context_line":"            if bdm.is_volume and bdm.volume_id and instance:"},{"line_number":2443,"context_line":"                try:"},{"line_number":2444,"context_line":"                    volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2445,"context_line":"                    if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_f28badbc","line":2442,"range":{"start_line":2442,"start_character":46,"end_line":2442,"end_character":59},"in_reply_to":"5f93b717_08c16fe5","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":2441,"context_line":"        for bdm in bdms:"},{"line_number":2442,"context_line":"            if bdm.is_volume and bdm.volume_id and instance:"},{"line_number":2443,"context_line":"                try:"},{"line_number":2444,"context_line":"                    volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2445,"context_line":"                    if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2446,"context_line":"                        self._detach_volume(context, bdm, instance,"},{"line_number":2447,"context_line":"                                                destroy_bdm\u003dFalse)"},{"line_number":2448,"context_line":"                except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_68b91313","line":2445,"range":{"start_line":2444,"start_character":20,"end_line":2445,"end_character":65},"updated":"2018-02-02 02:05:54.000000000","message":"Instead of querying the volume from cinder and checking its status, just try to detach the volume. See comment on PS10.\n\nSo you can remove this.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":2441,"context_line":"        for bdm in bdms:"},{"line_number":2442,"context_line":"            if bdm.is_volume and bdm.volume_id and instance:"},{"line_number":2443,"context_line":"                try:"},{"line_number":2444,"context_line":"                    volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2445,"context_line":"                    if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2446,"context_line":"                        self._detach_volume(context, bdm, instance,"},{"line_number":2447,"context_line":"                                                destroy_bdm\u003dFalse)"},{"line_number":2448,"context_line":"                except Exception as exc:"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_b5c18f56","line":2445,"range":{"start_line":2444,"start_character":20,"end_line":2445,"end_character":65},"in_reply_to":"5f93b717_68b91313","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":2444,"context_line":"                    volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2445,"context_line":"                    if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2446,"context_line":"                        self._detach_volume(context, bdm, instance,"},{"line_number":2447,"context_line":"                                                destroy_bdm\u003dFalse)"},{"line_number":2448,"context_line":"                except Exception as exc:"},{"line_number":2449,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2450,"context_line":"                    LOG.warning(\u0027Failed to detach volume: %(volume_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_28f6ab64","line":2447,"range":{"start_line":2447,"start_character":48,"end_line":2447,"end_character":66},"updated":"2018-02-02 02:05:54.000000000","message":"FYI: Usual style is to indent this the same as the line above it, like this:\n\n self._detach_volume(context, bdm, instance,\n                     destroy_bdm\u003dFalse)\n\nI\u0027m also wondering, shouldn\u0027t we pass destroy_bdm\u003dbdm.delete_on_termination instead?","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":2444,"context_line":"                    volume \u003d self.volume_api.get(context, bdm.volume_id)"},{"line_number":2445,"context_line":"                    if volume.get(\u0027attach_status\u0027) !\u003d \"detached\":"},{"line_number":2446,"context_line":"                        self._detach_volume(context, bdm, instance,"},{"line_number":2447,"context_line":"                                                destroy_bdm\u003dFalse)"},{"line_number":2448,"context_line":"                except Exception as exc:"},{"line_number":2449,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2450,"context_line":"                    LOG.warning(\u0027Failed to detach volume: %(volume_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_75b787f6","line":2447,"range":{"start_line":2447,"start_character":48,"end_line":2447,"end_character":66},"in_reply_to":"5f93b717_28f6ab64","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":2507,"context_line":"        #             and to only ignore the failure if the instance"},{"line_number":2508,"context_line":"        #             is already in ERROR."},{"line_number":2509,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2510,"context_line":"                raise_exc\u003dFalse)"},{"line_number":2511,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2512,"context_line":"        # state without expecting task state to be DELETING"},{"line_number":2513,"context_line":"        instance.vm_state \u003d vm_states.DELETED"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_28c46bf4","line":2510,"updated":"2018-02-02 02:05:54.000000000","message":"This might fit on the previous line now.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"671c564b23d8f10bec0888fb6be00fde0d7747ee","unresolved":false,"context_lines":[{"line_number":2443,"context_line":"                try:"},{"line_number":2444,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2445,"context_line":"                                        destroy_bdm\u003dbdm.delete_on_termination)"},{"line_number":2446,"context_line":"                except Exception as exc:"},{"line_number":2447,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2448,"context_line":"                    LOG.warning(\u0027Failed to detach volume: %(volume_id)s \u0027"},{"line_number":2449,"context_line":"                                \u0027due to %(exc)s\u0027,"},{"line_number":2450,"context_line":"                                {\u0027volume_id\u0027: bdm.volume_id, \u0027exc\u0027: exc})"},{"line_number":2451,"context_line":""},{"line_number":2452,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2453,"context_line":"                      instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa0c359_42706473","line":2450,"range":{"start_line":2446,"start_character":16,"end_line":2450,"end_character":73},"updated":"2018-02-05 14:58:14.000000000","message":"So we hit this when _shutdown_instance is called first and has already detached the volume, correct? Then yes I think we need to add a detach kwarg to this method, I don\u0027t really care what the default is. In the scenarios where we call _shutdown_instance first, we need to pass detach\u003dFalse.","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"406387cf4afee99207c6cb7f74bea6edfc995197","unresolved":false,"context_lines":[{"line_number":2443,"context_line":"                try:"},{"line_number":2444,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2445,"context_line":"                                        destroy_bdm\u003dbdm.delete_on_termination)"},{"line_number":2446,"context_line":"                except Exception as exc:"},{"line_number":2447,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2448,"context_line":"                    LOG.warning(\u0027Failed to detach volume: %(volume_id)s \u0027"},{"line_number":2449,"context_line":"                                \u0027due to %(exc)s\u0027,"},{"line_number":2450,"context_line":"                                {\u0027volume_id\u0027: bdm.volume_id, \u0027exc\u0027: exc})"},{"line_number":2451,"context_line":""},{"line_number":2452,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2453,"context_line":"                      instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa0c359_84a4310a","line":2450,"range":{"start_line":2446,"start_character":16,"end_line":2450,"end_character":73},"in_reply_to":"3fa0c359_42706473","updated":"2018-02-05 19:35:52.000000000","message":"Done","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b2d9dd2d3968a68d568c5b5e88d1defd61793860","unresolved":false,"context_lines":[{"line_number":2443,"context_line":"                try:"},{"line_number":2444,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2445,"context_line":"                                        destroy_bdm\u003dbdm.delete_on_termination)"},{"line_number":2446,"context_line":"                except Exception as exc:"},{"line_number":2447,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2448,"context_line":"                    LOG.warning(\u0027Failed to detach volume: %(volume_id)s \u0027"},{"line_number":2449,"context_line":"                                \u0027due to %(exc)s\u0027,"},{"line_number":2450,"context_line":"                                {\u0027volume_id\u0027: bdm.volume_id, \u0027exc\u0027: exc})"},{"line_number":2451,"context_line":""},{"line_number":2452,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2453,"context_line":"                      instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa0c359_e21ad015","line":2450,"range":{"start_line":2446,"start_character":16,"end_line":2450,"end_character":73},"in_reply_to":"3fa0c359_42706473","updated":"2018-02-05 15:02:10.000000000","message":"Yes, shutdown calls driver.detach first (no through cleanup_volumes) then delete hits is cleanup_volumes and fails here.\nI\u0027m cool with the extra kwargs.","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"406387cf4afee99207c6cb7f74bea6edfc995197","unresolved":false,"context_lines":[{"line_number":2443,"context_line":"                try:"},{"line_number":2444,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2445,"context_line":"                                        destroy_bdm\u003dbdm.delete_on_termination)"},{"line_number":2446,"context_line":"                except Exception as exc:"},{"line_number":2447,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2448,"context_line":"                    LOG.warning(\u0027Failed to detach volume: %(volume_id)s \u0027"},{"line_number":2449,"context_line":"                                \u0027due to %(exc)s\u0027,"},{"line_number":2450,"context_line":"                                {\u0027volume_id\u0027: bdm.volume_id, \u0027exc\u0027: exc})"},{"line_number":2451,"context_line":""},{"line_number":2452,"context_line":"            LOG.debug(\"terminating bdm %s\", bdm,"},{"line_number":2453,"context_line":"                      instance_uuid\u003dinstance.uuid)"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa0c359_c49a39c4","line":2450,"range":{"start_line":2446,"start_character":16,"end_line":2450,"end_character":73},"in_reply_to":"3fa0c359_e21ad015","updated":"2018-02-05 19:35:52.000000000","message":"Done","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3b816dd2e653c7b15cf6235f51bd770de4f9824a","unresolved":false,"context_lines":[{"line_number":2505,"context_line":"        #             future to set an instance fault the first time"},{"line_number":2506,"context_line":"        #             and to only ignore the failure if the instance"},{"line_number":2507,"context_line":"        #             is already in ERROR."},{"line_number":2508,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2509,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2510,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2511,"context_line":"        # state without expecting task state to be DELETING"}],"source_content_type":"text/x-python","patch_set":17,"id":"3fa0c359_8c94488d","line":2508,"updated":"2018-02-06 10:11:48.000000000","message":"nit: I would add a comment to note that the volumes already detached during the above _shutdown_instance() call and this is why detach is not requested from _cleanup_volumes() in this case","commit_id":"80de3c2e6fb3aaa5caf68b597c3483f42c06bbba"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"e04a2f34adc1605147cee27abe27ec2b62f9323f","unresolved":false,"context_lines":[{"line_number":2505,"context_line":"        #             future to set an instance fault the first time"},{"line_number":2506,"context_line":"        #             and to only ignore the failure if the instance"},{"line_number":2507,"context_line":"        #             is already in ERROR."},{"line_number":2508,"context_line":"        self._cleanup_volumes(context, instance, bdms,"},{"line_number":2509,"context_line":"                raise_exc\u003dFalse, detach\u003dFalse)"},{"line_number":2510,"context_line":"        # if a delete task succeeded, always update vm state and task"},{"line_number":2511,"context_line":"        # state without expecting task state to be DELETING"}],"source_content_type":"text/x-python","patch_set":17,"id":"3fa0c359_6c7d3c1b","line":2508,"in_reply_to":"3fa0c359_8c94488d","updated":"2018-02-06 10:28:10.000000000","message":"Done","commit_id":"80de3c2e6fb3aaa5caf68b597c3483f42c06bbba"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"345ba0157039b2b1af50a363c95c4ad4e6c1f5dd","unresolved":false,"context_lines":[{"line_number":1916,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":1917,"context_line":"                    requested_networks)"},{"line_number":1918,"context_line":"            return build_results.FAILED"},{"line_number":1919,"context_line":"        except exception.BuildAbortException as e:"},{"line_number":1920,"context_line":"            LOG.exception(e.format_message(), instance\u003dinstance)"},{"line_number":1921,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":1922,"context_line":"                    requested_networks)"},{"line_number":1923,"context_line":"            self._cleanup_volumes(context, instance,"},{"line_number":1924,"context_line":"                    block_device_mapping, raise_exc\u003dFalse)"},{"line_number":1925,"context_line":"            compute_utils.add_instance_fault_from_exc(context, instance,"},{"line_number":1926,"context_line":"                    e, sys.exc_info())"},{"line_number":1927,"context_line":"            self._nil_out_instance_obj_host_and_node(instance)"},{"line_number":1928,"context_line":"            self._set_instance_obj_error_state(context, instance,"},{"line_number":1929,"context_line":"                                               clean_task_state\u003dTrue)"},{"line_number":1930,"context_line":"            return build_results.FAILED"},{"line_number":1931,"context_line":"        except Exception as e:"},{"line_number":1932,"context_line":"            # Should not reach here."},{"line_number":1933,"context_line":"            LOG.exception(\u0027Unexpected build failure, not rescheduling build.\u0027,"},{"line_number":1934,"context_line":"                          instance\u003dinstance)"},{"line_number":1935,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":1936,"context_line":"                    requested_networks)"},{"line_number":1937,"context_line":"            self._cleanup_volumes(context, instance,"},{"line_number":1938,"context_line":"                    block_device_mapping, raise_exc\u003dFalse)"},{"line_number":1939,"context_line":"            compute_utils.add_instance_fault_from_exc(context, instance,"},{"line_number":1940,"context_line":"                    e, sys.exc_info())"},{"line_number":1941,"context_line":"            self._nil_out_instance_obj_host_and_node(instance)"},{"line_number":1942,"context_line":"            self._set_instance_obj_error_state(context, instance,"},{"line_number":1943,"context_line":"                                               clean_task_state\u003dTrue)"},{"line_number":1944,"context_line":"            return build_results.FAILED"},{"line_number":1945,"context_line":""},{"line_number":1946,"context_line":"    def deallocate_sriov_ports_on_reschedule(self, instance):"},{"line_number":1947,"context_line":"        \"\"\"Determine if networks are needed to be deallocated before reschedule"}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa0c359_26d75e26","line":1944,"range":{"start_line":1919,"start_character":8,"end_line":1944,"end_character":39},"updated":"2018-02-15 16:03:01.000000000","message":"We can combine these two exception blocks, but that\u0027s something to do in a follow up since we want to backport this fix.","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"345ba0157039b2b1af50a363c95c4ad4e6c1f5dd","unresolved":false,"context_lines":[{"line_number":2445,"context_line":"                    self._detach_volume(context, bdm, instance,"},{"line_number":2446,"context_line":"                                        destroy_bdm\u003ddestroy)"},{"line_number":2447,"context_line":"                except Exception as exc:"},{"line_number":2448,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2449,"context_line":"                    LOG.warning(\u0027Failed to detach volume: %(volume_id)s \u0027"},{"line_number":2450,"context_line":"                                \u0027due to %(exc)s\u0027,"},{"line_number":2451,"context_line":"                                {\u0027volume_id\u0027: bdm.volume_id, \u0027exc\u0027: exc})"}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa0c359_09bb55e5","line":2448,"updated":"2018-02-15 16:03:01.000000000","message":"At first this threw me off because I was thinking we don\u0027t use it here, but it\u0027s used below to indicate if we\u0027ve failed to cleanup any volumes.\n\nHowever, as far as I can tell, you don\u0027t have any test that hits this code block and asserts the exception would be re-raised out of _cleanup_volumes.","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"345ba0157039b2b1af50a363c95c4ad4e6c1f5dd","unresolved":false,"context_lines":[{"line_number":5346,"context_line":"        :param destroy_bdm: if True, the corresponding BDM entry will be marked"},{"line_number":5347,"context_line":"                            as deleted. Disabling this is useful for operations"},{"line_number":5348,"context_line":"                            like rebuild, when we don\u0027t want to destroy BDM"},{"line_number":5349,"context_line":"        :param attachment_id: The volume attachment_id for the given instance"},{"line_number":5350,"context_line":"                              and volume."},{"line_number":5351,"context_line":"        \"\"\""},{"line_number":5352,"context_line":"        volume_id \u003d bdm.volume_id"},{"line_number":5353,"context_line":"        compute_utils.notify_about_volume_attach_detach("}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa0c359_e99ce95e","line":5350,"range":{"start_line":5349,"start_character":8,"end_line":5350,"end_character":41},"updated":"2018-02-15 16:03:01.000000000","message":"Hmm, we\u0027re not passing this in, and I\u0027m not sure if we need to, but the bdm does have an attachment_id field on it, so we should probably attempt to pass that through, or if attachment_id here is None, we set:\n\nif not attachment_id:\n    attachment_id \u003d bdm.attachment_id\n\nAlthough maybe that\u0027s handled in driver_bdm.detach?","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"57091f2f7eccf957b477f67993ec2c05d85570ca","unresolved":false,"context_lines":[{"line_number":5346,"context_line":"        :param destroy_bdm: if True, the corresponding BDM entry will be marked"},{"line_number":5347,"context_line":"                            as deleted. Disabling this is useful for operations"},{"line_number":5348,"context_line":"                            like rebuild, when we don\u0027t want to destroy BDM"},{"line_number":5349,"context_line":"        :param attachment_id: The volume attachment_id for the given instance"},{"line_number":5350,"context_line":"                              and volume."},{"line_number":5351,"context_line":"        \"\"\""},{"line_number":5352,"context_line":"        volume_id \u003d bdm.volume_id"},{"line_number":5353,"context_line":"        compute_utils.notify_about_volume_attach_detach("}],"source_content_type":"text/x-python","patch_set":18,"id":"3fa0c359_cf26cd69","line":5350,"range":{"start_line":5349,"start_character":8,"end_line":5350,"end_character":41},"in_reply_to":"3fa0c359_e99ce95e","updated":"2018-02-15 17:05:22.000000000","message":"Looking at this the attachment_id passed into driver_bdm.detach() is not used anyway.\n\nhttps://github.com/openstack/nova/blob/031e66436b1749f04e2fec6b388303ea6e1488f7/nova/virt/block_device.py#L348","commit_id":"13f23fffc7c1eedd89f8700a9ab984c5bdd7d6bc"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"afb8d1e68e9800f854271151b9876573bddf1a4e","unresolved":false,"context_lines":[{"line_number":1914,"context_line":"            msg \u003d \u0027Instance disappeared during build.\u0027"},{"line_number":1915,"context_line":"            LOG.debug(msg, instance\u003dinstance)"},{"line_number":1916,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":1917,"context_line":"                    requested_networks)"},{"line_number":1918,"context_line":"            return build_results.FAILED"},{"line_number":1919,"context_line":"        except exception.BuildAbortException as e:"},{"line_number":1920,"context_line":"            LOG.exception(e.format_message(), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa0c359_0a2fc0f6","line":1917,"updated":"2018-02-15 19:59:50.000000000","message":"Why don\u0027t we need to cleanup volumes here?","commit_id":"61f6751a1807d3c3ee76d0351d17a82c6e1a915a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9676823bb6439c132652a5d4cc9d035be76f1681","unresolved":false,"context_lines":[{"line_number":1914,"context_line":"            msg \u003d \u0027Instance disappeared during build.\u0027"},{"line_number":1915,"context_line":"            LOG.debug(msg, instance\u003dinstance)"},{"line_number":1916,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":1917,"context_line":"                    requested_networks)"},{"line_number":1918,"context_line":"            return build_results.FAILED"},{"line_number":1919,"context_line":"        except exception.BuildAbortException as e:"},{"line_number":1920,"context_line":"            LOG.exception(e.format_message(), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa0c359_e8db6a4c","line":1917,"in_reply_to":"3fa0c359_0a2fc0f6","updated":"2018-02-16 17:07:48.000000000","message":"Don\u0027t know. Presumably because if you already deleted the instance, that was done for you? But then why do we cleanup networks?","commit_id":"61f6751a1807d3c3ee76d0351d17a82c6e1a915a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bd0fd7702f1991597fd13cdafc9f4eb81903f69f","unresolved":false,"context_lines":[{"line_number":1914,"context_line":"            msg \u003d \u0027Instance disappeared during build.\u0027"},{"line_number":1915,"context_line":"            LOG.debug(msg, instance\u003dinstance)"},{"line_number":1916,"context_line":"            self._cleanup_allocated_networks(context, instance,"},{"line_number":1917,"context_line":"                    requested_networks)"},{"line_number":1918,"context_line":"            return build_results.FAILED"},{"line_number":1919,"context_line":"        except exception.BuildAbortException as e:"},{"line_number":1920,"context_line":"            LOG.exception(e.format_message(), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa0c359_7e8dd079","line":1917,"in_reply_to":"3fa0c359_e8db6a4c","updated":"2018-02-16 18:06:16.000000000","message":"Yeah. I would think it\u0027s a race. If the volumes have already been attached when the delete path hits the volume cleanup, things should be fine. If the delete path passes volume cleanup before the volumes have been attached, then they would be stuck in attached state after this code block runs.\n\nThat said, I would expect it\u0027s pretty unlikely that someone would hit that case (though the fact that someone added network cleanup here implies it got run into before).","commit_id":"61f6751a1807d3c3ee76d0351d17a82c6e1a915a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2bd8e1c0a02e0cfdc8cefe7cbcd6d5905b3dff01","unresolved":false,"context_lines":[{"line_number":2456,"context_line":"                try:"},{"line_number":2457,"context_line":"                    LOG.debug(\"Deleting volume: %s\", bdm.volume_id,"},{"line_number":2458,"context_line":"                              instance_uuid\u003dinstance.uuid)"},{"line_number":2459,"context_line":"                    self.volume_api.delete(context, bdm.volume_id)"},{"line_number":2460,"context_line":"                except Exception as exc:"},{"line_number":2461,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2462,"context_line":"                    LOG.warning(\u0027Failed to delete volume: %(volume_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa0c359_3a50cd32","line":2459,"updated":"2018-02-15 17:58:52.000000000","message":"Technically this does change the behavior of the failed build a bit. Before this change, we\u0027d never be able to delete a volume before rescheduling because it could be attached, but now we will, even though we haven\u0027t deleted the instance.\n\nIf we do this and actually reschedule, the volume is gone and we\u0027ll fail to attach it to the instance on the next host, because we deleted it when cleaning up on the first host.\n\nThis code has been around a long time, but doesn\u0027t really make sense in the reschedule case, only the case that we\u0027re deleting the instance too.","commit_id":"61f6751a1807d3c3ee76d0351d17a82c6e1a915a"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"5b4c08a3e7a3793385fb83e98495f42995e6ef96","unresolved":false,"context_lines":[{"line_number":2456,"context_line":"                try:"},{"line_number":2457,"context_line":"                    LOG.debug(\"Deleting volume: %s\", bdm.volume_id,"},{"line_number":2458,"context_line":"                              instance_uuid\u003dinstance.uuid)"},{"line_number":2459,"context_line":"                    self.volume_api.delete(context, bdm.volume_id)"},{"line_number":2460,"context_line":"                except Exception as exc:"},{"line_number":2461,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2462,"context_line":"                    LOG.warning(\u0027Failed to delete volume: %(volume_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa0c359_f55a7628","line":2459,"in_reply_to":"3fa0c359_3a50cd32","updated":"2018-02-15 18:03:09.000000000","message":"I think this is a separate bug that needs to be solved.","commit_id":"61f6751a1807d3c3ee76d0351d17a82c6e1a915a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"afb8d1e68e9800f854271151b9876573bddf1a4e","unresolved":false,"context_lines":[{"line_number":2456,"context_line":"                try:"},{"line_number":2457,"context_line":"                    LOG.debug(\"Deleting volume: %s\", bdm.volume_id,"},{"line_number":2458,"context_line":"                              instance_uuid\u003dinstance.uuid)"},{"line_number":2459,"context_line":"                    self.volume_api.delete(context, bdm.volume_id)"},{"line_number":2460,"context_line":"                except Exception as exc:"},{"line_number":2461,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2462,"context_line":"                    LOG.warning(\u0027Failed to delete volume: %(volume_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa0c359_6a6be41e","line":2459,"in_reply_to":"3fa0c359_3a50cd32","updated":"2018-02-15 19:59:50.000000000","message":"What do you mean here? We\u0027re not calling _cleanup_volumes in the case where we reschedule (L1855, the \u0027else\u0027 branch). Am I missing something?","commit_id":"61f6751a1807d3c3ee76d0351d17a82c6e1a915a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9676823bb6439c132652a5d4cc9d035be76f1681","unresolved":false,"context_lines":[{"line_number":2456,"context_line":"                try:"},{"line_number":2457,"context_line":"                    LOG.debug(\"Deleting volume: %s\", bdm.volume_id,"},{"line_number":2458,"context_line":"                              instance_uuid\u003dinstance.uuid)"},{"line_number":2459,"context_line":"                    self.volume_api.delete(context, bdm.volume_id)"},{"line_number":2460,"context_line":"                except Exception as exc:"},{"line_number":2461,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2462,"context_line":"                    LOG.warning(\u0027Failed to delete volume: %(volume_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa0c359_88ec26f4","line":2459,"in_reply_to":"3fa0c359_6a6be41e","updated":"2018-02-16 17:07:48.000000000","message":"You\u0027re right, I was only looking to see it was in the RescheduledException block, not that it was within the \u0027not retry\u0027 conditional within that. Whew.","commit_id":"61f6751a1807d3c3ee76d0351d17a82c6e1a915a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa930c5c9073026b275df55383fe51074cbaac83","unresolved":false,"context_lines":[{"line_number":2456,"context_line":"                try:"},{"line_number":2457,"context_line":"                    LOG.debug(\"Deleting volume: %s\", bdm.volume_id,"},{"line_number":2458,"context_line":"                              instance_uuid\u003dinstance.uuid)"},{"line_number":2459,"context_line":"                    self.volume_api.delete(context, bdm.volume_id)"},{"line_number":2460,"context_line":"                except Exception as exc:"},{"line_number":2461,"context_line":"                    exc_info \u003d sys.exc_info()"},{"line_number":2462,"context_line":"                    LOG.warning(\u0027Failed to delete volume: %(volume_id)s \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"3fa0c359_f5f6f6f9","line":2459,"in_reply_to":"3fa0c359_f55a7628","updated":"2018-02-15 18:08:16.000000000","message":"The other thing this fails to consider is if nova created the volume or not. The way to tell is if bdm.source_type \u003d\u003d \u0027volume\u0027 or not. If the source_type\u003dvolume, then it\u0027s a pre-existing volume that nova didn\u0027t create, and we *should* delete it before rescheduling. If nova didn\u0027t create it, and we\u0027re going to reschedule, we likely shouldn\u0027t delete it since it will need to be used to attach on the next host.","commit_id":"61f6751a1807d3c3ee76d0351d17a82c6e1a915a"}],"nova/tests/functional/notification_sample_tests/test_instance.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"4a3b202348751508c31c3a192b9ff545286eb0a8","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            # Ensure that instance is in active state after an action"},{"line_number":305,"context_line":"            self._wait_for_state_change(self.admin_api, server, \u0027ACTIVE\u0027)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":308,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":309,"context_line":"    def test_create_delete_server(self, mock_detach):"},{"line_number":310,"context_line":"        server \u003d self._boot_a_server("},{"line_number":311,"context_line":"            extra_params\u003d{\u0027networks\u0027: [{\u0027port\u0027: self.neutron.port_1[\u0027id\u0027]}],"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa0c359_729d3558","line":308,"range":{"start_line":307,"start_character":0,"end_line":308,"end_character":33},"updated":"2018-02-05 10:35:11.000000000","message":"Please don\u0027t mock in the functional test (except if it is no other way to simulate some scenarios, like internal faults). If this test fails due to the new detach behavior in case of build error then the cinder fake needs to be updated to support this detach.","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6b676700c9f68a878e481fcc8ee3e97da4583c5e","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            # Ensure that instance is in active state after an action"},{"line_number":305,"context_line":"            self._wait_for_state_change(self.admin_api, server, \u0027ACTIVE\u0027)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":308,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":309,"context_line":"    def test_create_delete_server(self, mock_detach):"},{"line_number":310,"context_line":"        server \u003d self._boot_a_server("},{"line_number":311,"context_line":"            extra_params\u003d{\u0027networks\u0027: [{\u0027port\u0027: self.neutron.port_1[\u0027id\u0027]}],"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa0c359_ee53a1e1","line":308,"range":{"start_line":307,"start_character":0,"end_line":308,"end_character":33},"in_reply_to":"3fa0c359_2d9f5251","updated":"2018-02-05 12:11:48.000000000","message":"So what I saw as a duplicated detach in tempest is the same that happens here. \nUnit now we did not receive an explicit volume_detach notification as the _shutdown_instance() only sends instance.shutdown and calls driver.detach directly. \n\nNow a second detach is tried when _delete_instance() calls _cleanup_volumes() which calls _detach_volume() and _detach_volume() does emit volume_detach.start notification then it fails to detach the volume (as it already detached during shutdown) and therefore does not emit volume_detach.end. \n\nI noted that Mat suggested to remove the extra state check from _cleanup_volume(). However this way we get an extra WARNING trace in the log [1] and an extra volume_detach.start notification after _every_ normal instance delete that had a volume attached. I think this is undesirable.\n\n[1] http://logs.openstack.org/85/528385/15/check/neutron-tempest-multinode-full/f377ba5/logs/subnode-2/screen-n-cpu.txt.gz#_Feb_04_08_29_17_209782","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"d2343828491b0602af8136c915cfa27229a9272b","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            # Ensure that instance is in active state after an action"},{"line_number":305,"context_line":"            self._wait_for_state_change(self.admin_api, server, \u0027ACTIVE\u0027)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":308,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":309,"context_line":"    def test_create_delete_server(self, mock_detach):"},{"line_number":310,"context_line":"        server \u003d self._boot_a_server("},{"line_number":311,"context_line":"            extra_params\u003d{\u0027networks\u0027: [{\u0027port\u0027: self.neutron.port_1[\u0027id\u0027]}],"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa0c359_2d9f5251","line":308,"range":{"start_line":307,"start_character":0,"end_line":308,"end_character":33},"in_reply_to":"3fa0c359_729d3558","updated":"2018-02-05 10:50:05.000000000","message":"I mock this test since I face a problem with _cleanup_volumes. \n\nwhen self.api.delete_server() executed, it will detach the attached volumes from cinder \"_shutdown_instance\", then it call _cleanup_volumes()  so it try to detach volumes which is \"already detached from cinder\"\n\nI was add a condition to test if the volumes is detached here https://review.openstack.org/#/c/528385/13/nova/compute/manager.py \n\nbut as you see Mel and Matt ask me to remove this condition. \n\nThanks !","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"0dd7caac9d00f1e6228de51fb2f4f6a09ca381dd","unresolved":false,"context_lines":[{"line_number":304,"context_line":"            # Ensure that instance is in active state after an action"},{"line_number":305,"context_line":"            self._wait_for_state_change(self.admin_api, server, \u0027ACTIVE\u0027)"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":308,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":309,"context_line":"    def test_create_delete_server(self, mock_detach):"},{"line_number":310,"context_line":"        server \u003d self._boot_a_server("},{"line_number":311,"context_line":"            extra_params\u003d{\u0027networks\u0027: [{\u0027port\u0027: self.neutron.port_1[\u0027id\u0027]}],"}],"source_content_type":"text/x-python","patch_set":15,"id":"3fa0c359_aedcb918","line":308,"range":{"start_line":307,"start_character":0,"end_line":308,"end_character":33},"in_reply_to":"3fa0c359_ee53a1e1","updated":"2018-02-05 12:26:26.000000000","message":"Thank you for reply, \nwhat do you guess for me to do in this case ? \nI was declare the function which detach volumes outside of _cleanup_volumes() but they asked me to insert it inside the _cleanup_volumes() . \n\nwhat if we pass a parameter for _cleanup_volumes and set it by default to True, so in delete_instance we pass False to prevent the detaching again ?","commit_id":"4ec762b18b1a891f8b7a7c11e93203eb3ad06af3"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":995,"context_line":""},{"line_number":996,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":997,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":998,"context_line":"    def test_attach_detach_vol_to_shelved_server(self, detach):"},{"line_number":999,"context_line":"        self.flags(shelved_offload_time\u003d-1)"},{"line_number":1000,"context_line":"        found_server \u003d self._shelve_server()"},{"line_number":1001,"context_line":"        self.assertEqual(\u0027SHELVED\u0027, found_server[\u0027status\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_f9cdf7b6","line":998,"updated":"2018-02-02 02:05:54.000000000","message":"I don\u0027t think you need to mock _detach volume in this test. I don\u0027t think you need to make any changes to this file for your patch.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":995,"context_line":""},{"line_number":996,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":997,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":998,"context_line":"    def test_attach_detach_vol_to_shelved_server(self, detach):"},{"line_number":999,"context_line":"        self.flags(shelved_offload_time\u003d-1)"},{"line_number":1000,"context_line":"        found_server \u003d self._shelve_server()"},{"line_number":1001,"context_line":"        self.assertEqual(\u0027SHELVED\u0027, found_server[\u0027status\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_35cd7f84","line":998,"in_reply_to":"5f93b717_f9cdf7b6","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":2097,"context_line":""},{"line_number":2098,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":2099,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":2100,"context_line":"    def test_run_terminate_with_vol_attached(self, detach):"},{"line_number":2101,"context_line":"        \"\"\"Make sure it is possible to  run and terminate instance with volume"},{"line_number":2102,"context_line":"        attached"},{"line_number":2103,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_99763370","line":2100,"updated":"2018-02-02 02:05:54.000000000","message":"You don\u0027t need to mock out _detach_volume in these tests.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":2097,"context_line":""},{"line_number":2098,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":2099,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":2100,"context_line":"    def test_run_terminate_with_vol_attached(self, detach):"},{"line_number":2101,"context_line":"        \"\"\"Make sure it is possible to  run and terminate instance with volume"},{"line_number":2102,"context_line":"        attached"},{"line_number":2103,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_f5c2b752","line":2100,"in_reply_to":"5f93b717_99763370","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":279,"context_line":"    @mock.patch(\u0027nova.compute.utils.notify_about_instance_action\u0027)"},{"line_number":280,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":281,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":282,"context_line":"    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)"},{"line_number":283,"context_line":"    def test_delete_instance_without_info_cache(self, mock_get_by_uuid,"},{"line_number":284,"context_line":"                                                detach, mock_notify):"},{"line_number":285,"context_line":"        instance \u003d fake_instance.fake_instance_obj("}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_487f77aa","line":282,"range":{"start_line":282,"start_character":4,"end_line":282,"end_character":55},"updated":"2018-02-02 02:05:54.000000000","message":"You don\u0027t need any of the mock.patch for Instance.get_by_uuid anymore since you\u0027re not looking up the instance object anymore.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":279,"context_line":"    @mock.patch(\u0027nova.compute.utils.notify_about_instance_action\u0027)"},{"line_number":280,"context_line":"    @mock.patch(\u0027nova.compute.manager.ComputeManager.\u0027"},{"line_number":281,"context_line":"                \u0027_detach_volume\u0027)"},{"line_number":282,"context_line":"    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)"},{"line_number":283,"context_line":"    def test_delete_instance_without_info_cache(self, mock_get_by_uuid,"},{"line_number":284,"context_line":"                                                detach, mock_notify):"},{"line_number":285,"context_line":"        instance \u003d fake_instance.fake_instance_obj("}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_b5a66fc0","line":282,"range":{"start_line":282,"start_character":4,"end_line":282,"end_character":55},"in_reply_to":"5f93b717_487f77aa","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":3762,"context_line":"        self.assertRaises(test.TestingException, do_test)"},{"line_number":3763,"context_line":"        set_error.assert_called_once_with(self.context, instance)"},{"line_number":3764,"context_line":""},{"line_number":3765,"context_line":"    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)"},{"line_number":3766,"context_line":"    def test_cleanup_volumes(self, mock_get_by_uuid):"},{"line_number":3767,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":3768,"context_line":"        bdm_do_not_delete_dict \u003d fake_block_device.FakeDbBlockDeviceDict("}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_b2d44a49","line":3765,"updated":"2018-02-02 02:05:54.000000000","message":"As mentioned earlier, you don\u0027t need to mock Instance.get_by_uuid anymore.\n\nHowever, you do need to mock _detach_volume.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":3762,"context_line":"        self.assertRaises(test.TestingException, do_test)"},{"line_number":3763,"context_line":"        set_error.assert_called_once_with(self.context, instance)"},{"line_number":3764,"context_line":""},{"line_number":3765,"context_line":"    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)"},{"line_number":3766,"context_line":"    def test_cleanup_volumes(self, mock_get_by_uuid):"},{"line_number":3767,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.context)"},{"line_number":3768,"context_line":"        bdm_do_not_delete_dict \u003d fake_block_device.FakeDbBlockDeviceDict("}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_75ac679e","line":3765,"in_reply_to":"5f93b717_b2d44a49","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":3777,"context_line":""},{"line_number":3778,"context_line":"        with mock.patch.object(self.compute.volume_api,"},{"line_number":3779,"context_line":"                \u0027delete\u0027) as volume_delete:"},{"line_number":3780,"context_line":"            self.compute._cleanup_volumes(self.context, instance, bdms)"},{"line_number":3781,"context_line":"            volume_delete.assert_called_once_with(self.context,"},{"line_number":3782,"context_line":"                    bdms[1].volume_id)"},{"line_number":3783,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_92426e0e","line":3780,"updated":"2018-02-02 02:05:54.000000000","message":"Then after this, you will assert that _detach_volume was called with expected arguments, for example:\n\n call1 \u003d mock.call(self.context, bdms[0], instance, destroy_bdm\u003dFalse)\n call2 \u003d mock.call(self.context, bdms[1], instance, destroy_bdms\u003dTrue)\n self.assertEqual(2, mock_detach.call_count)\n mock_detach.assert_has_calls([call1, call2])","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":3777,"context_line":""},{"line_number":3778,"context_line":"        with mock.patch.object(self.compute.volume_api,"},{"line_number":3779,"context_line":"                \u0027delete\u0027) as volume_delete:"},{"line_number":3780,"context_line":"            self.compute._cleanup_volumes(self.context, instance, bdms)"},{"line_number":3781,"context_line":"            volume_delete.assert_called_once_with(self.context,"},{"line_number":3782,"context_line":"                    bdms[1].volume_id)"},{"line_number":3783,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_1597234a","line":3780,"in_reply_to":"5f93b717_92426e0e","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":3800,"context_line":"            self.compute._cleanup_volumes(self.context, instance, bdms,"},{"line_number":3801,"context_line":"                    raise_exc\u003dFalse)"},{"line_number":3802,"context_line":"            calls \u003d [mock.call(self.context, bdm.volume_id) for bdm in bdms]"},{"line_number":3803,"context_line":"            self.assertEqual(calls, volume_delete.call_args_list)"},{"line_number":3804,"context_line":""},{"line_number":3805,"context_line":"    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)"},{"line_number":3806,"context_line":"    def test_cleanup_volumes_exception_raise(self, mock_get_by_uuid):"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_32e91ae5","line":3803,"updated":"2018-02-02 02:05:54.000000000","message":"For this test, you could add a new code block very similar to this one, except you will mock _detach_volume instead of volume_api delete.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":3800,"context_line":"            self.compute._cleanup_volumes(self.context, instance, bdms,"},{"line_number":3801,"context_line":"                    raise_exc\u003dFalse)"},{"line_number":3802,"context_line":"            calls \u003d [mock.call(self.context, bdm.volume_id) for bdm in bdms]"},{"line_number":3803,"context_line":"            self.assertEqual(calls, volume_delete.call_args_list)"},{"line_number":3804,"context_line":""},{"line_number":3805,"context_line":"    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)"},{"line_number":3806,"context_line":"    def test_cleanup_volumes_exception_raise(self, mock_get_by_uuid):"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_35925f58","line":3803,"in_reply_to":"5f93b717_32e91ae5","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0a974855d65c55825d1a66125e9ccf6ce897f5c5","unresolved":false,"context_lines":[{"line_number":3822,"context_line":"                    self.compute._cleanup_volumes, self.context, instance,"},{"line_number":3823,"context_line":"                    bdms)"},{"line_number":3824,"context_line":"            calls \u003d [mock.call(self.context, bdm.volume_id) for bdm in bdms]"},{"line_number":3825,"context_line":"            self.assertEqual(calls, volume_delete.call_args_list)"},{"line_number":3826,"context_line":""},{"line_number":3827,"context_line":"    def test_stop_instance_task_state_none_power_state_shutdown(self):"},{"line_number":3828,"context_line":"        # Tests that stop_instance doesn\u0027t puke when the instance power_state"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_f2ee52eb","line":3825,"updated":"2018-02-02 02:05:54.000000000","message":"Same here, you could add a new code block very similar to this one, except mock _detach_volume instead of volume_api delete.","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"},{"author":{"_account_id":27277,"name":"Ameed Ashour","email":"ameed.ashour.ext@nokia.com","username":"ameeda"},"change_message_id":"02b81d56ffed011ab858ce6162f81a5303441e26","unresolved":false,"context_lines":[{"line_number":3822,"context_line":"                    self.compute._cleanup_volumes, self.context, instance,"},{"line_number":3823,"context_line":"                    bdms)"},{"line_number":3824,"context_line":"            calls \u003d [mock.call(self.context, bdm.volume_id) for bdm in bdms]"},{"line_number":3825,"context_line":"            self.assertEqual(calls, volume_delete.call_args_list)"},{"line_number":3826,"context_line":""},{"line_number":3827,"context_line":"    def test_stop_instance_task_state_none_power_state_shutdown(self):"},{"line_number":3828,"context_line":"        # Tests that stop_instance doesn\u0027t puke when the instance power_state"}],"source_content_type":"text/x-python","patch_set":13,"id":"5f93b717_f5971744","line":3825,"in_reply_to":"5f93b717_f2ee52eb","updated":"2018-02-02 08:36:08.000000000","message":"Done","commit_id":"e35fc677b5740f61ac43fd6a74ba0afe03f1674f"}]}