)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"4478a89a36ef638ea266294be37d63303e3424b7","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Nova allows rebuild of instance when vm_state is ERROR. [1]"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"The vm_state is restored to ACTIVE only after a successful build."},{"line_number":12,"context_line":"This means rebuilding a baremetal instance using the Ironic driver"},{"line_number":13,"context_line":"is impossible because wait_for_active fails if vm_state\u003dERROR is found."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ff82abbf_ed4d123b","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":65},"updated":"2017-11-29 14:11:16.000000000","message":"So if the rebuild fails the vm_state will continue to be ERROR, right ?\n\nWould a combination of task_state + vm_state indicate that a rebuild has failed ?\n\nFor example:\n\n def wait_for_active():\n     if (vm_state \u003d\u003d vm_states.ERROR and task_state !\u003d\n         task_states.REBUILDING):\n         raise InstanceDeployFailure(...)","commit_id":"08a73c42af5fb35fa930db10e5c635314c4bbee6"}],"nova/tests/unit/virt/ironic/test_driver.py":[{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"84334ac6d4a5124c44004f4b2378982fc0547480","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        self._wait_for_active_abort({\u0027vm_state\u0027: vm_states.DELETED})"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def test__wait_for_active_abort_error(self):"},{"line_number":234,"context_line":"        self._wait_for_active_abort({\u0027vm_state\u0027: vm_states.ERROR})"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    @mock.patch.object(ironic_driver.IronicDriver,"},{"line_number":237,"context_line":"                       \u0027_validate_instance_and_node\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf659307_68744f36","side":"PARENT","line":234,"updated":"2018-03-26 07:47:13.000000000","message":"I prefer keeping this test for a case where task_state is not REBUILD_SPAWNING.","commit_id":"05c6b54eec650ed641256a39cc85ca1efe70f9ea"},{"author":{"_account_id":7156,"name":"Mathieu Gagné","email":"mgagne@calavera.ca","username":"mgagne"},"change_message_id":"d6746873342fc59f1fc96dcd740413a9b1380bf0","unresolved":false,"context_lines":[{"line_number":231,"context_line":"        self._wait_for_active_abort({\u0027vm_state\u0027: vm_states.DELETED})"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def test__wait_for_active_abort_error(self):"},{"line_number":234,"context_line":"        self._wait_for_active_abort({\u0027vm_state\u0027: vm_states.ERROR})"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    @mock.patch.object(ironic_driver.IronicDriver,"},{"line_number":237,"context_line":"                       \u0027_validate_instance_and_node\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bf659307_c46e312e","side":"PARENT","line":234,"in_reply_to":"bf659307_68744f36","updated":"2018-03-29 15:49:14.000000000","message":"Done","commit_id":"05c6b54eec650ed641256a39cc85ca1efe70f9ea"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"097f8cb3ffcb3a468eb09e097180268d304fc397","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def test__wait_for_active_abort_error(self):"},{"line_number":254,"context_line":"        self._wait_for_active_abort({\u0027task_state\u0027: task_states.DELETING,"},{"line_number":255,"context_line":"                                     \u0027vm_state\u0027: vm_states.ERROR})"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    @mock.patch.object(ironic_driver.IronicDriver,"},{"line_number":258,"context_line":"                       \u0027_validate_instance_and_node\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_18c4fe3f","line":255,"updated":"2018-08-18 02:01:47.000000000","message":"Previously, this test wasn\u0027t covering the case where task_state \u003d DELETING but now it\u0027s not covering the case where task_state !\u003d DELETING and vm_state \u003d ERROR.\n\nActually, test__wait_for_active_abort_deleting on L247 is already covering what you have here, making this duplicate test coverage. So, I think here, you need to change to task_state \u003d None or something else other than DELETING. This will be to cover the case where the spawn fails and the instance is put into ERROR state somehow, and _wait_for_active is supposed to notice and raise InstanceDeployFailure.","commit_id":"3f4304717f53f68ae6b8af044d5e8d74e996780b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2763613a5453d9553c4d6c42f58aada470aebd41","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    def test__wait_for_active_abort_error(self):"},{"line_number":254,"context_line":"        self._wait_for_active_abort({\u0027task_state\u0027: task_states.DELETING,"},{"line_number":255,"context_line":"                                     \u0027vm_state\u0027: vm_states.ERROR})"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    @mock.patch.object(ironic_driver.IronicDriver,"},{"line_number":258,"context_line":"                       \u0027_validate_instance_and_node\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_2a465bc2","line":255,"in_reply_to":"3f79a3b5_18c4fe3f","updated":"2019-08-08 14:49:12.000000000","message":"I think using task_state\u003dSPAWNING here would work since that\u0027s a non-rebuild case and mimics an initial create failure.\n\nhttps://github.com/openstack/nova/blob/c92cb75675003d7ae3cbb50959a395862b3cfe18/nova/compute/manager.py#L2242","commit_id":"3f4304717f53f68ae6b8af044d5e8d74e996780b"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3d348f33ab4f0901d7e5eaa35a22f9426de31588","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":474,"context_line":"        instance.refresh()"},{"line_number":475,"context_line":"        if (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":476,"context_line":"            instance.vm_state in (vm_states.ERROR, vm_states.DELETED)):"},{"line_number":477,"context_line":"            raise exception.InstanceDeployFailure("},{"line_number":478,"context_line":"                _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":479,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_2d677aeb","side":"PARENT","line":476,"range":{"start_line":476,"start_character":44,"end_line":476,"end_character":49},"updated":"2017-11-29 13:55:48.000000000","message":"Reading the description of bug 1455000, it sounds like ERROR was added intentionally in case the spawn fails on the ironic side and you want to delete the instance. So it seems like you\u0027d be re-introducing that problem if you remove this.\n\nIf you want to distinguish between when it\u0027s OK to wait for ERROR or not, maybe you should check the task_state and if we\u0027re rebuilding, then don\u0027t check for ERROR?","commit_id":"05c6b54eec650ed641256a39cc85ca1efe70f9ea"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"b18c160a18b2d0ed8da6a8c579f33c715c8a6a49","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":474,"context_line":"        instance.refresh()"},{"line_number":475,"context_line":"        if (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":476,"context_line":"            instance.vm_state in (vm_states.ERROR, vm_states.DELETED)):"},{"line_number":477,"context_line":"            raise exception.InstanceDeployFailure("},{"line_number":478,"context_line":"                _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":479,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_4d82fe01","side":"PARENT","line":476,"range":{"start_line":476,"start_character":44,"end_line":476,"end_character":49},"in_reply_to":"ff82abbf_2d677aeb","updated":"2017-11-29 14:15:28.000000000","message":"Sorry I missed this comment here before I added mine.\n\nIf it works, I think that checking both task_state and vm_state would be a good to solve this problem. Otherwise I don\u0027t see how the driver will figure out if the rebuilding failed (it will probably time out eventually).","commit_id":"05c6b54eec650ed641256a39cc85ca1efe70f9ea"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"bd84fe9640a7bd647ea00fe4237af063bb1df68a","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":474,"context_line":"        instance.refresh()"},{"line_number":475,"context_line":"        if (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":476,"context_line":"            instance.vm_state in (vm_states.ERROR, vm_states.DELETED)):"},{"line_number":477,"context_line":"            raise exception.InstanceDeployFailure("},{"line_number":478,"context_line":"                _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":479,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_7029d934","side":"PARENT","line":476,"range":{"start_line":476,"start_character":44,"end_line":476,"end_character":49},"in_reply_to":"ff82abbf_4d82fe01","updated":"2017-11-29 14:42:55.000000000","message":"I agree the task state should be checked as well for rebuilds as suggested.","commit_id":"05c6b54eec650ed641256a39cc85ca1efe70f9ea"},{"author":{"_account_id":7156,"name":"Mathieu Gagné","email":"mgagne@calavera.ca","username":"mgagne"},"change_message_id":"01f2aba63f81ea89b2ef03df34b1e6438ca59087","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":474,"context_line":"        instance.refresh()"},{"line_number":475,"context_line":"        if (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":476,"context_line":"            instance.vm_state in (vm_states.ERROR, vm_states.DELETED)):"},{"line_number":477,"context_line":"            raise exception.InstanceDeployFailure("},{"line_number":478,"context_line":"                _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":479,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ff82abbf_72155948","side":"PARENT","line":476,"range":{"start_line":476,"start_character":44,"end_line":476,"end_character":49},"in_reply_to":"ff82abbf_7029d934","updated":"2017-11-29 18:39:58.000000000","message":"Can you point me to where vm_state is set to ERROR when deployment fails in Ironic? So far, I only see node.provision_state\u003dironic_states.DEPLOYFAIL\n\nAnd I can\u0027t find the equivalent in libvirt driver which works fine.","commit_id":"05c6b54eec650ed641256a39cc85ca1efe70f9ea"},{"author":{"_account_id":23757,"name":"Mateusz Kowalski","email":"mko@redhat.com","username":"makowals"},"change_message_id":"e5d216d66c8664d4798f79bf9abc7626c698a8e7","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":474,"context_line":"        instance.refresh()"},{"line_number":475,"context_line":"        if (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":476,"context_line":"            instance.vm_state in (vm_states.ERROR, vm_states.DELETED)):"},{"line_number":477,"context_line":"            raise exception.InstanceDeployFailure("},{"line_number":478,"context_line":"                _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":479,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"df87a7cf_c9575702","side":"PARENT","line":476,"range":{"start_line":476,"start_character":44,"end_line":476,"end_character":49},"in_reply_to":"ff82abbf_72155948","updated":"2017-12-06 15:03:00.000000000","message":"How about the following?\n\n```\nif (instance.task_state !\u003d task_states.REBUILD_SPAWNING and\n  (instance.task_state \u003d\u003d task_states.DELETING or\n  instance.vm_state in (vm_states.ERROR, vm_states.DELETED)))\n```","commit_id":"05c6b54eec650ed641256a39cc85ca1efe70f9ea"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"abc26f383453c2dd238b99c0946465d46e81d691","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":474,"context_line":"        instance.refresh()"},{"line_number":475,"context_line":"        if (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":476,"context_line":"                instance.vm_state \u003d\u003d vm_states.DELETED):"},{"line_number":477,"context_line":"            raise exception.InstanceDeployFailure("},{"line_number":478,"context_line":"                _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":479,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_978ce97f","line":476,"updated":"2018-03-29 15:58:30.000000000","message":"In PS4 and in the other review comments people were talking about checking for task_states.REBUILD_SPAWNING in addition this. Was there any discussion outside this review about why not to do that? Just wondering why it was decided on and then dropped.","commit_id":"68c3387e04e9b54aaa1ad5128a0b4d5f20360c08"},{"author":{"_account_id":7156,"name":"Mathieu Gagné","email":"mgagne@calavera.ca","username":"mgagne"},"change_message_id":"6bfaaac2a232c387a5c03ac16f46dc4bcc7d630d","unresolved":false,"context_lines":[{"line_number":473,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":474,"context_line":"        instance.refresh()"},{"line_number":475,"context_line":"        if (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":476,"context_line":"                instance.vm_state \u003d\u003d vm_states.DELETED):"},{"line_number":477,"context_line":"            raise exception.InstanceDeployFailure("},{"line_number":478,"context_line":"                _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":479,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bf659307_b0ded36a","line":476,"in_reply_to":"bf659307_978ce97f","updated":"2018-03-29 19:12:38.000000000","message":"I think I had an older version of the change for some reasons. Will restore to previous version.","commit_id":"68c3387e04e9b54aaa1ad5128a0b4d5f20360c08"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"097f8cb3ffcb3a468eb09e097180268d304fc397","unresolved":false,"context_lines":[{"line_number":447,"context_line":"    def _wait_for_active(self, instance):"},{"line_number":448,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":449,"context_line":"        instance.refresh()"},{"line_number":450,"context_line":"        if (instance.task_state !\u003d task_states.REBUILD_SPAWNING and"},{"line_number":451,"context_line":"            (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":452,"context_line":"             instance.vm_state in (vm_states.ERROR, vm_states.DELETED))):"},{"line_number":453,"context_line":"                raise exception.InstanceDeployFailure("}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_381d3abd","line":450,"updated":"2018-08-18 02:01:47.000000000","message":"This is a unfortunate in that we\u0027d have to keep this task_state list in sync with the guards in compute/api if they change, but if the deploy process can put an instance into ERROR state, I see why _wait_for_active needs to abort when it sees ERROR.","commit_id":"3f4304717f53f68ae6b8af044d5e8d74e996780b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2763613a5453d9553c4d6c42f58aada470aebd41","unresolved":false,"context_lines":[{"line_number":447,"context_line":"    def _wait_for_active(self, instance):"},{"line_number":448,"context_line":"        \"\"\"Wait for the node to be marked as ACTIVE in Ironic.\"\"\""},{"line_number":449,"context_line":"        instance.refresh()"},{"line_number":450,"context_line":"        if (instance.task_state !\u003d task_states.REBUILD_SPAWNING and"},{"line_number":451,"context_line":"            (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":452,"context_line":"             instance.vm_state in (vm_states.ERROR, vm_states.DELETED))):"},{"line_number":453,"context_line":"                raise exception.InstanceDeployFailure("}],"source_content_type":"text/x-python","patch_set":7,"id":"7faddb67_6af7738e","line":450,"in_reply_to":"3f79a3b5_381d3abd","updated":"2019-08-08 14:49:12.000000000","message":"I think it\u0027s less about the compute API and the fact that during rebuild in the compute, before calling driver.spawn we set the task_state to REBUILD_SPAWNING:\n\nhttps://github.com/openstack/nova/blob/c92cb75675003d7ae3cbb50959a395862b3cfe18/nova/compute/manager.py#L3056\n\nSo this is just here to handle the case that we\u0027re rebuilding from ERROR. Yes it\u0027s tightly coupled to the task state machine for a rebuild operation, which like you said is unfortunate, but it\u0027s also a relatively simple fix.\n\nA comment here would be helpful.","commit_id":"3f4304717f53f68ae6b8af044d5e8d74e996780b"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"097f8cb3ffcb3a468eb09e097180268d304fc397","unresolved":false,"context_lines":[{"line_number":449,"context_line":"        instance.refresh()"},{"line_number":450,"context_line":"        if (instance.task_state !\u003d task_states.REBUILD_SPAWNING and"},{"line_number":451,"context_line":"            (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":452,"context_line":"             instance.vm_state in (vm_states.ERROR, vm_states.DELETED))):"},{"line_number":453,"context_line":"                raise exception.InstanceDeployFailure("},{"line_number":454,"context_line":"                    _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":455,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3f79a3b5_980c8e95","line":452,"range":{"start_line":452,"start_character":35,"end_line":452,"end_character":50},"updated":"2018-08-18 02:01:47.000000000","message":"Note to self: this is here to make _wait_for_active raise InstanceDeployFailure if the instance has fallen into ERROR state during the deploy process.","commit_id":"3f4304717f53f68ae6b8af044d5e8d74e996780b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3124dab34e115d2c0063de919fa3b311e256c3c3","unresolved":false,"context_lines":[{"line_number":501,"context_line":"        if (instance.task_state !\u003d task_states.REBUILD_SPAWNING and"},{"line_number":502,"context_line":"            (instance.task_state \u003d\u003d task_states.DELETING or"},{"line_number":503,"context_line":"             instance.vm_state in (vm_states.ERROR, vm_states.DELETED))):"},{"line_number":504,"context_line":"                raise exception.InstanceDeployFailure("},{"line_number":505,"context_line":"                    _(\"Instance %s provisioning was aborted\") % instance.uuid)"},{"line_number":506,"context_line":""},{"line_number":507,"context_line":"        node \u003d self._validate_instance_and_node(instance)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7faddb67_aa6fa0fa","line":504,"updated":"2019-08-09 02:42:04.000000000","message":"New pep8 rules fail here for indentation.","commit_id":"93d64aa975cbf6709edc9c51b26fa870666fb4f2"}]}