)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"efb189f6d37976d560a80cfd5bfb1d2c7a66a7e2","unresolved":false,"context_lines":[{"line_number":9,"context_line":"Instance information such as an instance_uuid set to an ironic node by"},{"line_number":10,"context_line":"_add_driver_fields() is not cleared when spawning is aborted by an"},{"line_number":11,"context_line":"exception raised before ironic starts deployment. Then, ironic node"},{"line_number":12,"context_line":"stays AVAILABLE state with instance_uuid set. This information is not"},{"line_number":13,"context_line":"cleared even if the instance is deleted. The ironic node cannot be"},{"line_number":14,"context_line":"removed nor deployed again becuase instance_uuid remains."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch adds a method to remove the information and calls it"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"bacf61ea_459b8fc9","line":13,"range":{"start_line":12,"start_character":46,"end_line":13,"end_character":40},"updated":"2016-08-04 10:10:55.000000000","message":"This seems like the way we should fix this bug, to be consistent with other drivers.","commit_id":"25b04950be0cffd87cdc9ef0261b719e2e27a51f"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"e1f56e37c50cc8f9564c3ca692341d56909b4c8e","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        try:"},{"line_number":379,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch)"},{"line_number":380,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":381,"context_line":"            msg \u003d (_LW(\"Failed to remove deploy parameters from node %(node)s \""},{"line_number":382,"context_line":"                       \"when unprovisioning the instance %(instance)s\")"},{"line_number":383,"context_line":"                   % {\u0027node\u0027: node.uuid, \u0027instance\u0027: instance.uuid})"},{"line_number":384,"context_line":"            LOG.warning(msg)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def _cleanup_deploy(self, node, instance, network_info):"},{"line_number":387,"context_line":"        self._unplug_vifs(node, instance, network_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_af282fe3","line":384,"range":{"start_line":381,"start_character":12,"end_line":384,"end_character":28},"updated":"2016-08-01 06:22:28.000000000","message":"String interpolation should be delayed to be handled by the logging code, rather than being done at the point of the logging call.\nSo it should be:\n\n            LOG.warning(_LW(\"Failed to remove deploy parameters \"\n                            \"from node %(node)s when unprovisioning \"\n                            \"the instance %(instance)s\",\n                        {\u0027node\u0027: node.uuid, \u0027instance\u0027: instance.uuid})","commit_id":"53dd9adf92fea7497b983d585a0eddffeae817fe"},{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"4e555b9c061932f566ce00ed6ad29cd3adef861e","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        try:"},{"line_number":379,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch)"},{"line_number":380,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":381,"context_line":"            msg \u003d (_LW(\"Failed to remove deploy parameters from node %(node)s \""},{"line_number":382,"context_line":"                       \"when unprovisioning the instance %(instance)s\")"},{"line_number":383,"context_line":"                   % {\u0027node\u0027: node.uuid, \u0027instance\u0027: instance.uuid})"},{"line_number":384,"context_line":"            LOG.warning(msg)"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def _cleanup_deploy(self, node, instance, network_info):"},{"line_number":387,"context_line":"        self._unplug_vifs(node, instance, network_info)"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_276c51cd","line":384,"range":{"start_line":381,"start_character":12,"end_line":384,"end_character":28},"in_reply_to":"bacf61ea_af282fe3","updated":"2016-08-01 13:22:38.000000000","message":"Done","commit_id":"53dd9adf92fea7497b983d585a0eddffeae817fe"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"412f8005db8f53056739253059c132d1dc26c44e","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        try:"},{"line_number":379,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch)"},{"line_number":380,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":381,"context_line":"            LOG.warning(_LW(\"Failed to remove deploy parameters from node \""},{"line_number":382,"context_line":"                            \"%(node)s when unprovisioning the instance \""},{"line_number":383,"context_line":"                            \"%(instance)s\"),"},{"line_number":384,"context_line":"                        {\u0027node\u0027: node.uuid, \u0027instance\u0027: instance.uuid})"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def _cleanup_deploy(self, node, instance, network_info):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_c47d21b3","line":383,"range":{"start_line":381,"start_character":12,"end_line":383,"end_character":43},"updated":"2016-08-02 18:37:14.000000000","message":"It is better to contain the message of BadRequest which comes from Ironic to know actual reason without checking the log of Ironic side. That would be useful for operators.","commit_id":"d4bfcda64940ff40f3f6b5dc3618f0bb9b37b460"},{"author":{"_account_id":13689,"name":"Hironori Shiina","email":"Hironori.Shiina@fujitsu.com","username":"shiina"},"change_message_id":"53a8ad0f37842166b60fc69c7b1a4fcd405ae3cc","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        try:"},{"line_number":379,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch)"},{"line_number":380,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":381,"context_line":"            LOG.warning(_LW(\"Failed to remove deploy parameters from node \""},{"line_number":382,"context_line":"                            \"%(node)s when unprovisioning the instance \""},{"line_number":383,"context_line":"                            \"%(instance)s\"),"},{"line_number":384,"context_line":"                        {\u0027node\u0027: node.uuid, \u0027instance\u0027: instance.uuid})"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def _cleanup_deploy(self, node, instance, network_info):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_f078b8fa","line":383,"range":{"start_line":381,"start_character":12,"end_line":383,"end_character":43},"in_reply_to":"bacf61ea_570e01e6","updated":"2016-08-03 12:28:42.000000000","message":"Done","commit_id":"d4bfcda64940ff40f3f6b5dc3618f0bb9b37b460"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"909647fb8784ef1622f255077d1e7b3e70232304","unresolved":false,"context_lines":[{"line_number":378,"context_line":"        try:"},{"line_number":379,"context_line":"            self.ironicclient.call(\u0027node.update\u0027, node.uuid, patch)"},{"line_number":380,"context_line":"        except ironic.exc.BadRequest:"},{"line_number":381,"context_line":"            LOG.warning(_LW(\"Failed to remove deploy parameters from node \""},{"line_number":382,"context_line":"                            \"%(node)s when unprovisioning the instance \""},{"line_number":383,"context_line":"                            \"%(instance)s\"),"},{"line_number":384,"context_line":"                        {\u0027node\u0027: node.uuid, \u0027instance\u0027: instance.uuid})"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"    def _cleanup_deploy(self, node, instance, network_info):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_570e01e6","line":383,"range":{"start_line":381,"start_character":12,"end_line":383,"end_character":43},"in_reply_to":"bacf61ea_c47d21b3","updated":"2016-08-02 18:45:58.000000000","message":"++","commit_id":"d4bfcda64940ff40f3f6b5dc3618f0bb9b37b460"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"06e40efd29f12060e327abb472cd69c8bc38bd1b","unresolved":false,"context_lines":[{"line_number":712,"context_line":""},{"line_number":713,"context_line":"        def _cleanup(node, instance, network_info):"},{"line_number":714,"context_line":"            self._cleanup_deploy(node, instance, network_info)"},{"line_number":715,"context_line":"            self._remove_driver_fields(node, instance)"},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"        # NOTE(Shrews): The default ephemeral device needs to be set for"},{"line_number":718,"context_line":"        # services (like cloud-init) that depend on it being returned by the"}],"source_content_type":"text/x-python","patch_set":6,"id":"bacf61ea_886e3ec7","line":715,"updated":"2016-08-04 10:33:04.000000000","message":"Why can\u0027t we move _remove_driver_fields into _cleanup_deploy? I think there is something about the ironic API I am missing here.","commit_id":"25b04950be0cffd87cdc9ef0261b719e2e27a51f"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"47208517e94563861748e6b8afaba16557953754","unresolved":false,"context_lines":[{"line_number":880,"context_line":"        if node.provision_state in _UNPROVISION_STATES:"},{"line_number":881,"context_line":"            self._unprovision(instance, node)"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"        self._cleanup_deploy(node, instance, network_info)"},{"line_number":884,"context_line":"        LOG.info(_LI(\u0027Successfully unprovisioned Ironic node %s\u0027),"},{"line_number":885,"context_line":"                 node.uuid, instance\u003dinstance)"},{"line_number":886,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bacf61ea_e51ba3c8","line":883,"updated":"2016-08-04 10:29:26.000000000","message":"Why don\u0027t we have _remove_driver_fields(..) called here? Is that usually done by _unprovision somehow?\n\nWould adding that not fix this bug? Or does _validate_instance_and_node fail when we are in the ERROR state?\n\nIt feels like the bug is when _unprovision is not called, we don\u0027t fully clean everything up?","commit_id":"25b04950be0cffd87cdc9ef0261b719e2e27a51f"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"209f3e7a91d5f88c898e5716c9ef5a899aa0454c","unresolved":false,"context_lines":[{"line_number":880,"context_line":"        if node.provision_state in _UNPROVISION_STATES:"},{"line_number":881,"context_line":"            self._unprovision(instance, node)"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"        self._cleanup_deploy(node, instance, network_info)"},{"line_number":884,"context_line":"        LOG.info(_LI(\u0027Successfully unprovisioned Ironic node %s\u0027),"},{"line_number":885,"context_line":"                 node.uuid, instance\u003dinstance)"},{"line_number":886,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bacf61ea_685dca78","line":883,"in_reply_to":"bacf61ea_08e88e0f","updated":"2016-08-04 10:43:51.000000000","message":"Ah, that makes sense. Although, that probably means we should really be waiting here to make sure the unprovision completes correctly, but I guess that may take an awfully long time in some cases?\n\nI guess we need an else clause to the _UNPROVISION_STATES, if we don\u0027t call unprovision, we probably need to do that removal of the uuid, after checking we are not in some other pending state.\n\nNote that delete can be called at any time in Nova. You should correctly perform a delete, even if you are half way through spawning the instance.","commit_id":"25b04950be0cffd87cdc9ef0261b719e2e27a51f"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"a7846806cfce001fed47da6ec2cdeb5caa90294e","unresolved":false,"context_lines":[{"line_number":880,"context_line":"        if node.provision_state in _UNPROVISION_STATES:"},{"line_number":881,"context_line":"            self._unprovision(instance, node)"},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"        self._cleanup_deploy(node, instance, network_info)"},{"line_number":884,"context_line":"        LOG.info(_LI(\u0027Successfully unprovisioned Ironic node %s\u0027),"},{"line_number":885,"context_line":"                 node.uuid, instance\u003dinstance)"},{"line_number":886,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bacf61ea_08e88e0f","line":883,"in_reply_to":"bacf61ea_e51ba3c8","updated":"2016-08-04 10:35:00.000000000","message":"IIRC usually Ironic does clean up itself. Actually in this stage a node will be in the \"cleaning\" state and _remove_driver_fields will always fail (updates are not allowed in transient states).","commit_id":"25b04950be0cffd87cdc9ef0261b719e2e27a51f"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"3f6c3209c9f52c61f1751664acb7c87500899aee","unresolved":false,"context_lines":[{"line_number":372,"context_line":"            LOG.error(msg)"},{"line_number":373,"context_line":"            raise exception.InstanceDeployFailure(msg)"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def _remove_driver_fields(self, node, instance):"},{"line_number":376,"context_line":"        patch \u003d [{\u0027path\u0027: \u0027/instance_info\u0027, \u0027op\u0027: \u0027remove\u0027},"},{"line_number":377,"context_line":"                 {\u0027path\u0027: \u0027/instance_uuid\u0027, \u0027op\u0027: \u0027remove\u0027}]"},{"line_number":378,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ac06dbe_8b1bb1c9","line":375,"updated":"2016-08-23 09:03:18.000000000","message":"Nit: _remove_instance_info_from_node()? might make this code easier to read?","commit_id":"0e24e9e2ec254364ffe029226b9ae5956002df54"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"57228842c1866151237d6b07904b5d5d26395c8b","unresolved":false,"context_lines":[{"line_number":372,"context_line":"            LOG.error(msg)"},{"line_number":373,"context_line":"            raise exception.InstanceDeployFailure(msg)"},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"    def _remove_driver_fields(self, node, instance):"},{"line_number":376,"context_line":"        patch \u003d [{\u0027path\u0027: \u0027/instance_info\u0027, \u0027op\u0027: \u0027remove\u0027},"},{"line_number":377,"context_line":"                 {\u0027path\u0027: \u0027/instance_uuid\u0027, \u0027op\u0027: \u0027remove\u0027}]"},{"line_number":378,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ac06dbe_dd3a66e6","line":375,"in_reply_to":"1ac06dbe_8b1bb1c9","updated":"2016-08-23 17:08:19.000000000","message":"I prefer _remove_instance_info_from_node(), but this is an internal method we can rename it later.","commit_id":"0e24e9e2ec254364ffe029226b9ae5956002df54"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"86aa40d4476619d1dea262e39589ee55ac579326","unresolved":false,"context_lines":[{"line_number":876,"context_line":"        if node.provision_state in _UNPROVISION_STATES:"},{"line_number":877,"context_line":"            self._unprovision(instance, node)"},{"line_number":878,"context_line":"        else:"},{"line_number":879,"context_line":"            self._remove_driver_fields(node, instance)"},{"line_number":880,"context_line":""},{"line_number":881,"context_line":"        self._cleanup_deploy(node, instance, network_info)"},{"line_number":882,"context_line":"        LOG.info(_LI(\u0027Successfully unprovisioned Ironic node %s\u0027),"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ac06dbe_8bfe11ac","line":879,"updated":"2016-08-23 09:01:35.000000000","message":"Nit: it seems worth adding a noting this is needed when we hit errors while building the instance, such that we can\u0027t call unprovision to drop this instance\u0027s claim on this ironic node.","commit_id":"0e24e9e2ec254364ffe029226b9ae5956002df54"}]}