)]}'
{"nova/compute/resource_tracker.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"efecc7b254aa958ed0985186ddda9ee6b75cac38","unresolved":false,"context_lines":[{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        for instance in instances:"},{"line_number":781,"context_line":"            if (instance.vm_state !\u003d vm_states.DELETED"},{"line_number":782,"context_line":"                  and not instance.cleaned):"},{"line_number":783,"context_line":"                self._update_usage_from_instance(context, instance)"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    def _find_orphaned_instances(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_df199ed8","line":782,"updated":"2015-08-19 23:46:52.000000000","message":"This makes no sense to me. !\u003dDELETED is stronger than .cleaned, right? You can\u0027t be cleaned\u003dTrue if you\u0027re not DELETED, so why do we need this extra bit of logic?","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b3dc102de8502424c4b537c54bb41b6629de3893","unresolved":false,"context_lines":[{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        for instance in instances:"},{"line_number":781,"context_line":"            if (instance.vm_state !\u003d vm_states.DELETED"},{"line_number":782,"context_line":"                  and not instance.cleaned):"},{"line_number":783,"context_line":"                self._update_usage_from_instance(context, instance)"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    def _find_orphaned_instances(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_d5809dc2","line":782,"in_reply_to":"fa1b9901_75a3d100","updated":"2015-08-20 13:23:55.000000000","message":"But error instances are supposed to report resource usage, AFAIK. They should not be excluded until they are deleted...","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":15967,"name":"vikas choudhary","email":"choudharyvikas16@gmail.com","username":"vikaschoudhary16"},"change_message_id":"98d93798cb9897b8ba199ba7f39bae70852dad21","unresolved":false,"context_lines":[{"line_number":779,"context_line":""},{"line_number":780,"context_line":"        for instance in instances:"},{"line_number":781,"context_line":"            if (instance.vm_state !\u003d vm_states.DELETED"},{"line_number":782,"context_line":"                  and not instance.cleaned):"},{"line_number":783,"context_line":"                self._update_usage_from_instance(context, instance)"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"    def _find_orphaned_instances(self):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_75a3d100","line":782,"in_reply_to":"fa1b9901_df199ed8","updated":"2015-08-20 00:37:59.000000000","message":"This issue is coming in scenario when vm_state is not \"DELETED\" (rather it is ERROR ) , but the resources have got cleaned from virtualization driver(ironic in this bug).So just checking \"DELETED\"  alone was not able to skip instances which ended up in \"ERROR\" state but got resources cleaned from virtualization driver and thus leading to incorrect hypervisor stats.","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"efecc7b254aa958ed0985186ddda9ee6b75cac38","unresolved":false,"context_lines":[{"line_number":602,"context_line":"            node \u003d _validate_instance_and_node(self.ironicclient, instance)"},{"line_number":603,"context_line":"        except exception.InstanceNotFound:"},{"line_number":604,"context_line":"            instance.cleaned \u003d True"},{"line_number":605,"context_line":"            instance.save()"},{"line_number":606,"context_line":"            return hardware.InstanceInfo("},{"line_number":607,"context_line":"                state\u003dmap_power_state(ironic_states.NOSTATE))"},{"line_number":608,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_5ff82e7b","line":605,"updated":"2015-08-19 23:46:52.000000000","message":"This is different behavior from the libvirt driver which is the only other driver that uses this cleaned flag. Why not allow compute manager to retry the delete and set instance.cleaned in destroy() to mirror the existing behavior?","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":15967,"name":"vikas choudhary","email":"choudharyvikas16@gmail.com","username":"vikaschoudhary16"},"change_message_id":"123603e62d701d4a831890bf3399fe710026909b","unresolved":false,"context_lines":[{"line_number":602,"context_line":"            node \u003d _validate_instance_and_node(self.ironicclient, instance)"},{"line_number":603,"context_line":"        except exception.InstanceNotFound:"},{"line_number":604,"context_line":"            instance.cleaned \u003d True"},{"line_number":605,"context_line":"            instance.save()"},{"line_number":606,"context_line":"            return hardware.InstanceInfo("},{"line_number":607,"context_line":"                state\u003dmap_power_state(ironic_states.NOSTATE))"},{"line_number":608,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_12060452","line":605,"in_reply_to":"fa1b9901_0e86e9da","updated":"2015-08-20 15:53:17.000000000","message":"Sorry Dan, i could not get what you refering to as \"original destroy()\".Here is only one destroy which is being called by \u0027spawn\u0027 and it seems all sequential.where is race condition? which part of code can get executed here simultaneously and lead to unpredictable situation?","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"96240af2929cf33b13af37f6b7da674920568c7e","unresolved":false,"context_lines":[{"line_number":602,"context_line":"            node \u003d _validate_instance_and_node(self.ironicclient, instance)"},{"line_number":603,"context_line":"        except exception.InstanceNotFound:"},{"line_number":604,"context_line":"            instance.cleaned \u003d True"},{"line_number":605,"context_line":"            instance.save()"},{"line_number":606,"context_line":"            return hardware.InstanceInfo("},{"line_number":607,"context_line":"                state\u003dmap_power_state(ironic_states.NOSTATE))"},{"line_number":608,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_55f22607","line":605,"in_reply_to":"fa1b9901_12060452","updated":"2015-08-20 15:58:34.000000000","message":"If you hit this exception path, it\u0027s because the instance has already been destroyed. That is why you would get an InstanceNotFound exception. That means that some other destroy operation came in while the spawn was happening (either by the user or because spawn called destroy itself).","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":15967,"name":"vikas choudhary","email":"choudharyvikas16@gmail.com","username":"vikaschoudhary16"},"change_message_id":"2fe7a2906e580af73fe8d89ff6b81a370dedb940","unresolved":false,"context_lines":[{"line_number":602,"context_line":"            node \u003d _validate_instance_and_node(self.ironicclient, instance)"},{"line_number":603,"context_line":"        except exception.InstanceNotFound:"},{"line_number":604,"context_line":"            instance.cleaned \u003d True"},{"line_number":605,"context_line":"            instance.save()"},{"line_number":606,"context_line":"            return hardware.InstanceInfo("},{"line_number":607,"context_line":"                state\u003dmap_power_state(ironic_states.NOSTATE))"},{"line_number":608,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_81287c34","line":605,"in_reply_to":"fa1b9901_1b17ff48","updated":"2015-08-20 17:03:10.000000000","message":"Also in spawn, exception is not InstanceNotFound rather its callback timout.","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":15967,"name":"vikas choudhary","email":"choudharyvikas16@gmail.com","username":"vikaschoudhary16"},"change_message_id":"a6512ef385541169a98cb1911f018e8d5d619baa","unresolved":false,"context_lines":[{"line_number":602,"context_line":"            node \u003d _validate_instance_and_node(self.ironicclient, instance)"},{"line_number":603,"context_line":"        except exception.InstanceNotFound:"},{"line_number":604,"context_line":"            instance.cleaned \u003d True"},{"line_number":605,"context_line":"            instance.save()"},{"line_number":606,"context_line":"            return hardware.InstanceInfo("},{"line_number":607,"context_line":"                state\u003dmap_power_state(ironic_states.NOSTATE))"},{"line_number":608,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_1b17ff48","line":605,"in_reply_to":"fa1b9901_55f22607","updated":"2015-08-20 16:25:51.000000000","message":"Dan ,here we are not calling destroy.This is not spawn..this is get_info.It seems some confusion is there :)","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":15967,"name":"vikas choudhary","email":"choudharyvikas16@gmail.com","username":"vikaschoudhary16"},"change_message_id":"98d93798cb9897b8ba199ba7f39bae70852dad21","unresolved":false,"context_lines":[{"line_number":602,"context_line":"            node \u003d _validate_instance_and_node(self.ironicclient, instance)"},{"line_number":603,"context_line":"        except exception.InstanceNotFound:"},{"line_number":604,"context_line":"            instance.cleaned \u003d True"},{"line_number":605,"context_line":"            instance.save()"},{"line_number":606,"context_line":"            return hardware.InstanceInfo("},{"line_number":607,"context_line":"                state\u003dmap_power_state(ironic_states.NOSTATE))"},{"line_number":608,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_62c47590","line":605,"in_reply_to":"fa1b9901_5ff82e7b","updated":"2015-08-20 00:37:59.000000000","message":"Thanks for review Dan :)\nActually in this case, compute manager is not trying to delete the instance rather its trying to deploy the instance.Its the ironic driver which on callback-timeout \u0027cleans\u0027 the\nresources. compute manager on deploy failure will set vm_state to \"ERROR\" (not deleted).So resources are getting \u0027cleaned\u0027 but compute manager is not calling destroy(and also not setting state to DELETED).","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b3dc102de8502424c4b537c54bb41b6629de3893","unresolved":false,"context_lines":[{"line_number":602,"context_line":"            node \u003d _validate_instance_and_node(self.ironicclient, instance)"},{"line_number":603,"context_line":"        except exception.InstanceNotFound:"},{"line_number":604,"context_line":"            instance.cleaned \u003d True"},{"line_number":605,"context_line":"            instance.save()"},{"line_number":606,"context_line":"            return hardware.InstanceInfo("},{"line_number":607,"context_line":"                state\u003dmap_power_state(ironic_states.NOSTATE))"},{"line_number":608,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_0e86e9da","line":605,"in_reply_to":"fa1b9901_62c47590","updated":"2015-08-20 13:23:55.000000000","message":"Right, and it shouldn\u0027t need to either. This happens because there is a race between spawn() and destroy(). So, sure, the spawn() shouldn\u0027t need to call destroy like this, because the original destroy() that it raced with should have deleted it.","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"efecc7b254aa958ed0985186ddda9ee6b75cac38","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                              \u0027node\u0027: node_uuid})"},{"line_number":798,"context_line":"                self.destroy(context, instance, network_info)"},{"line_number":799,"context_line":"                instance.cleaned \u003d True"},{"line_number":800,"context_line":"                instance.save()"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"    def _unprovision(self, ironicclient, instance, node):"},{"line_number":803,"context_line":"        \"\"\"This method is called from destroy() to unprovision"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_fff02293","line":800,"updated":"2015-08-19 23:46:52.000000000","message":"If you put this clean operation in destroy, then we will be more like the existing implementation and avoid another roundtrip to the conductor/DB right?","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b3dc102de8502424c4b537c54bb41b6629de3893","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                              \u0027node\u0027: node_uuid})"},{"line_number":798,"context_line":"                self.destroy(context, instance, network_info)"},{"line_number":799,"context_line":"                instance.cleaned \u003d True"},{"line_number":800,"context_line":"                instance.save()"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"    def _unprovision(self, ironicclient, instance, node):"},{"line_number":803,"context_line":"        \"\"\"This method is called from destroy() to unprovision"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_6efa0545","line":800,"in_reply_to":"fa1b9901_22a67d53","updated":"2015-08-20 13:23:55.000000000","message":"If the instance is in ERROR state, then the expectation is that the user must delete it. That\u0027s what the state is for -- to communicate to the user that an error has happened and that they must acknowledge it by deleting it.","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"780bee65e6980f123f3b3c552338ce2a4592a85f","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                              \u0027node\u0027: node_uuid})"},{"line_number":798,"context_line":"                self.destroy(context, instance, network_info)"},{"line_number":799,"context_line":"                instance.cleaned \u003d True"},{"line_number":800,"context_line":"                instance.save()"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"    def _unprovision(self, ironicclient, instance, node):"},{"line_number":803,"context_line":"        \"\"\"This method is called from destroy() to unprovision"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_5e518c0d","line":800,"in_reply_to":"fa1b9901_6efa0545","updated":"2015-08-22 15:03:43.000000000","message":"Does it not make more sense to only set instance.cleaned when destroy is called?  I see the logic above, I guess this question is more of a \"Why is it in two places?\" question.","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"},{"author":{"_account_id":15967,"name":"vikas choudhary","email":"choudharyvikas16@gmail.com","username":"vikaschoudhary16"},"change_message_id":"98d93798cb9897b8ba199ba7f39bae70852dad21","unresolved":false,"context_lines":[{"line_number":797,"context_line":"                              \u0027node\u0027: node_uuid})"},{"line_number":798,"context_line":"                self.destroy(context, instance, network_info)"},{"line_number":799,"context_line":"                instance.cleaned \u003d True"},{"line_number":800,"context_line":"                instance.save()"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"    def _unprovision(self, ironicclient, instance, node):"},{"line_number":803,"context_line":"        \"\"\"This method is called from destroy() to unprovision"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa1b9901_22a67d53","line":800,"in_reply_to":"fa1b9901_fff02293","updated":"2015-08-20 00:37:59.000000000","message":"you are right Dan.Actually clean operation is being done here when instance spawn is failing. To make it in sync with existing libvirt implementation \u0027destroy\u0027 call can be commented out from here.But the problem is ironic node will not be available for instance deploy until this \u0027ERRORED\u0027 instance is not deleted. It might be a deliberate decision to clean ironic_node/hypervisor from this point.Please suggest.","commit_id":"b6e2258f465377e705ec61f19b71bc84f1f41175"}]}