)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"51eefb83cfad2c6309a2983cf1f56af5f007556c","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"                             build_results.RESCHEDULED)"},{"line_number":1731,"context_line":"                    if result in fails:"},{"line_number":1732,"context_line":"                        # Cleanup the allocations records for the instance"},{"line_number":1733,"context_line":"                        # building failed or rescheduled."},{"line_number":1734,"context_line":"                        rt \u003d self._get_resource_tracker()"},{"line_number":1735,"context_line":"                        rt.reportclient.delete_allocation_for_instance("},{"line_number":1736,"context_line":"                            instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_e1e9451a","line":1733,"range":{"start_line":1733,"start_character":26,"end_line":1733,"end_character":41},"updated":"2017-08-24 13:22:49.000000000","message":"I\u0027m not sure about this one - the instance is taking up resources on this node until it\u0027s moved (rescheduled) or deleted, and we\u0027ll remove the allocations on delete now:\n\nhttps://review.openstack.org/#/c/496942/\n\nSo in that case I don\u0027t think we should remove the allocations here.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1a02b6227f2b3a4fb04182ea4eb0f7fba1aa65f1","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"                             build_results.RESCHEDULED)"},{"line_number":1731,"context_line":"                    if result in fails:"},{"line_number":1732,"context_line":"                        # Cleanup the allocations records for the instance"},{"line_number":1733,"context_line":"                        # building failed or rescheduled."},{"line_number":1734,"context_line":"                        rt \u003d self._get_resource_tracker()"},{"line_number":1735,"context_line":"                        rt.reportclient.delete_allocation_for_instance("},{"line_number":1736,"context_line":"                            instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_bc3a4695","line":1733,"range":{"start_line":1733,"start_character":26,"end_line":1733,"end_character":41},"in_reply_to":"7f287b81_01e0d195","updated":"2017-08-24 13:47:46.000000000","message":"Hmm, so if the instance.host is set to None, then that means when the user goes to delete it, the API will do a \u0027local delete\u0027 since there is no host associated to cast to for the cleanup, so we\u0027d never get to _complete_deletion which removes the allocations for a deleted instance.\n\nAnd because of bug 1679750 we know we don\u0027t remove allocations in the API during a \u0027local delete\u0027. With Ocata computes we\u0027d just rely on the periodic task removing the allocations for the removed instance, but we can\u0027t rely on that now.\n\nSo yeah I guess we either have to do this here, or in the API during local delete when instance.host is None...","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"407bc235baa8c47dbe1ea33544b465ed0d1bc11f","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"                             build_results.RESCHEDULED)"},{"line_number":1731,"context_line":"                    if result in fails:"},{"line_number":1732,"context_line":"                        # Cleanup the allocations records for the instance"},{"line_number":1733,"context_line":"                        # building failed or rescheduled."},{"line_number":1734,"context_line":"                        rt \u003d self._get_resource_tracker()"},{"line_number":1735,"context_line":"                        rt.reportclient.delete_allocation_for_instance("},{"line_number":1736,"context_line":"                            instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_7712b70b","line":1733,"range":{"start_line":1733,"start_character":26,"end_line":1733,"end_character":41},"in_reply_to":"7f287b81_5c224254","updated":"2017-08-24 14:03:29.000000000","message":"Yea, at least we should explictly remove the allocations at here. If you are really afraid there is too generically, we can put this into \u0027_do_build_and_run_instance\u0027 function","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"274dbdf4c76a3d49499fa52c2bad3ec3d482bda7","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"                             build_results.RESCHEDULED)"},{"line_number":1731,"context_line":"                    if result in fails:"},{"line_number":1732,"context_line":"                        # Cleanup the allocations records for the instance"},{"line_number":1733,"context_line":"                        # building failed or rescheduled."},{"line_number":1734,"context_line":"                        rt \u003d self._get_resource_tracker()"},{"line_number":1735,"context_line":"                        rt.reportclient.delete_allocation_for_instance("},{"line_number":1736,"context_line":"                            instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_5c224254","line":1733,"range":{"start_line":1733,"start_character":26,"end_line":1733,"end_character":41},"in_reply_to":"7f287b81_bc3a4695","updated":"2017-08-24 13:54:59.000000000","message":"Actually the update_available_resource periodic task will cleanup the allocations for the instance if it\u0027s deleted from the API:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1172\n\nThat code gets all the allocations for the node and then if one of the allocations has an instance that\u0027s not found (deleted), it removes the allocation.\n\nSo we could rely on the periodic task to remove the allocations when the build fails and instance.host is set to None, but maybe it\u0027s better to be explicit here when we know there is a failure? I just worry a bit about relying on how build_results.FAILED is used and doing this generically and getting it wrong later, or regressing this, but worst case is the periodic task cleans it up.\n\nAlternatively if the periodic task runs and instance.host is None but the instance isn\u0027t deleted yet, we\u0027d hit this:\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1174\n\nAnd just ignore it.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8d8b10d20eb0429331b92b2aff1c18a91f1d5c32","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"                             build_results.RESCHEDULED)"},{"line_number":1731,"context_line":"                    if result in fails:"},{"line_number":1732,"context_line":"                        # Cleanup the allocations records for the instance"},{"line_number":1733,"context_line":"                        # building failed or rescheduled."},{"line_number":1734,"context_line":"                        rt \u003d self._get_resource_tracker()"},{"line_number":1735,"context_line":"                        rt.reportclient.delete_allocation_for_instance("},{"line_number":1736,"context_line":"                            instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_e12b85e8","line":1733,"in_reply_to":"7f287b81_e1e9451a","updated":"2017-08-24 13:29:48.000000000","message":"Yeah, we shouldn\u0027t wipe out allocations until all remnants of the vm (even if non-operational) is gone.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"3987c90cc6e291946a0dbcc56a54d3a87c34db15","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"                             build_results.RESCHEDULED)"},{"line_number":1731,"context_line":"                    if result in fails:"},{"line_number":1732,"context_line":"                        # Cleanup the allocations records for the instance"},{"line_number":1733,"context_line":"                        # building failed or rescheduled."},{"line_number":1734,"context_line":"                        rt \u003d self._get_resource_tracker()"},{"line_number":1735,"context_line":"                        rt.reportclient.delete_allocation_for_instance("},{"line_number":1736,"context_line":"                            instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_01e0d195","line":1733,"range":{"start_line":1733,"start_character":26,"end_line":1733,"end_character":41},"in_reply_to":"7f287b81_e1e9451a","updated":"2017-08-24 13:37:23.000000000","message":"we return build_results.FAILED  at https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1890 and https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1876\n\nI guess those are the cases we want to remove the allcoations.\n\nThose two cases are nil the host of instance, that means release the resource.\n\nOnly one case is needn\u0027t, it is https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1867\n\nThat is the deleted instance. So I guess this delete action just failed.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"51eefb83cfad2c6309a2983cf1f56af5f007556c","unresolved":false,"context_lines":[{"line_number":1851,"context_line":"                self.network_api.cleanup_instance_network_on_host("},{"line_number":1852,"context_line":"                    context, instance, self.host)"},{"line_number":1853,"context_line":""},{"line_number":1854,"context_line":"            self._nil_out_instance_obj_host_and_node(instance)"},{"line_number":1855,"context_line":"            instance.task_state \u003d task_states.SCHEDULING"},{"line_number":1856,"context_line":"            instance.save()"},{"line_number":1857,"context_line":""},{"line_number":1858,"context_line":"            self.compute_task_api.build_instances(context, [instance],"},{"line_number":1859,"context_line":"                    image, filter_properties, admin_password,"},{"line_number":1860,"context_line":"                    injected_files, requested_networks, security_groups,"},{"line_number":1861,"context_line":"                    block_device_mapping)"},{"line_number":1862,"context_line":"            return build_results.RESCHEDULED"},{"line_number":1863,"context_line":"        except (exception.InstanceNotFound,"},{"line_number":1864,"context_line":"                exception.UnexpectedDeletingTaskStateError):"},{"line_number":1865,"context_line":"            msg \u003d \u0027Instance disappeared during build.\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_a1cc5dbc","line":1862,"range":{"start_line":1854,"start_character":0,"end_line":1862,"end_character":44},"updated":"2017-08-24 13:22:49.000000000","message":"I think somewhere in here is where we want to remove the allocations for the instance on this node, just like how we\u0027re doing network/port cleanup before we cast to conductor to pick another host.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"51eefb83cfad2c6309a2983cf1f56af5f007556c","unresolved":false,"context_lines":[{"line_number":3793,"context_line":"                    context, instance, \"resize.prep.end\","},{"line_number":3794,"context_line":"                    extra_usage_info\u003dextra_usage_info)"},{"line_number":3795,"context_line":""},{"line_number":3796,"context_line":"    def _reschedule_resize_or_reraise(self, context, image, instance, exc_info,"},{"line_number":3797,"context_line":"            instance_type, request_spec, filter_properties):"},{"line_number":3798,"context_line":"        \"\"\"Try to re-schedule the resize or re-raise the original error to"},{"line_number":3799,"context_line":"        error out the instance."}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_c1d64928","line":3796,"updated":"2017-08-24 13:22:49.000000000","message":"This is another case where we\u0027re rescheduling and moving the instance off this host and we should cleanup source allocations. Do you want to handle that in this fix or another one? In the resize case, we don\u0027t want to remove all allocations for the instance because during a resize the scheduler \u0027doubled\u0027 the allocations for the instance so it has allocations on the source and destination nodes. We would only want to remove the allocations for the destination node using SchedulerReportClient.remove_provider_from_instance_allocation. This should work fine from this code since prep_resize runs on the destination node.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"3987c90cc6e291946a0dbcc56a54d3a87c34db15","unresolved":false,"context_lines":[{"line_number":3793,"context_line":"                    context, instance, \"resize.prep.end\","},{"line_number":3794,"context_line":"                    extra_usage_info\u003dextra_usage_info)"},{"line_number":3795,"context_line":""},{"line_number":3796,"context_line":"    def _reschedule_resize_or_reraise(self, context, image, instance, exc_info,"},{"line_number":3797,"context_line":"            instance_type, request_spec, filter_properties):"},{"line_number":3798,"context_line":"        \"\"\"Try to re-schedule the resize or re-raise the original error to"},{"line_number":3799,"context_line":"        error out the instance."}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_6160b502","line":3796,"in_reply_to":"7f287b81_4179392d","updated":"2017-08-24 13:37:23.000000000","message":"I think the resize case is more complex than this one, so I prefer to another fix. And I see the bug didn\u0027t mark as pike rc related, so I didn\u0027t fix it yet.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"322679d40902153a5d03dd6a0a36f4c7bdc3bcaa","unresolved":false,"context_lines":[{"line_number":3793,"context_line":"                    context, instance, \"resize.prep.end\","},{"line_number":3794,"context_line":"                    extra_usage_info\u003dextra_usage_info)"},{"line_number":3795,"context_line":""},{"line_number":3796,"context_line":"    def _reschedule_resize_or_reraise(self, context, image, instance, exc_info,"},{"line_number":3797,"context_line":"            instance_type, request_spec, filter_properties):"},{"line_number":3798,"context_line":"        \"\"\"Try to re-schedule the resize or re-raise the original error to"},{"line_number":3799,"context_line":"        error out the instance."}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_4179392d","line":3796,"in_reply_to":"7f287b81_c1d64928","updated":"2017-08-24 13:24:36.000000000","message":"Correction: \"and we should cleanup *destination* allocations\"","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"69aa3bd0866bf64269adbca17f7e234a98957a80","unresolved":false,"context_lines":[{"line_number":1723,"context_line":"                try:"},{"line_number":1724,"context_line":"                    result \u003d self._do_build_and_run_instance(*args, **kwargs)"},{"line_number":1725,"context_line":"                except Exception:"},{"line_number":1726,"context_line":"                    result \u003d build_results.FAILED"},{"line_number":1727,"context_line":"                    raise"},{"line_number":1728,"context_line":"                finally:"},{"line_number":1729,"context_line":"                    fails \u003d (build_results.FAILED,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_576338b2","line":1726,"range":{"start_line":1726,"start_character":20,"end_line":1726,"end_character":49},"updated":"2017-08-24 15:17:59.000000000","message":"In this case, we don\u0027t necessarily know that instance.host is None or what happened.","commit_id":"f4c39bc21a995c8911f915e75e438c2870332b8e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c575a15d9ebb0fa283ae33c971f8fae46296a383","unresolved":false,"context_lines":[{"line_number":1723,"context_line":"                try:"},{"line_number":1724,"context_line":"                    result \u003d self._do_build_and_run_instance(*args, **kwargs)"},{"line_number":1725,"context_line":"                except Exception:"},{"line_number":1726,"context_line":"                    result \u003d build_results.FAILED"},{"line_number":1727,"context_line":"                    raise"},{"line_number":1728,"context_line":"                finally:"},{"line_number":1729,"context_line":"                    fails \u003d (build_results.FAILED,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_9772c048","line":1726,"range":{"start_line":1726,"start_character":20,"end_line":1726,"end_character":49},"in_reply_to":"7f287b81_576338b2","updated":"2017-08-24 15:22:11.000000000","message":"If instance.host !\u003d None, then the user could attempt to delete it, but it would mean removing allocations for this host/node when the instance.host is still pointed at it, so the resource tracker is going to consider resources still claimed.","commit_id":"f4c39bc21a995c8911f915e75e438c2870332b8e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0a655ac3898424ec77a43dbde6faedc5757165e1","unresolved":false,"context_lines":[{"line_number":1723,"context_line":"                try:"},{"line_number":1724,"context_line":"                    result \u003d self._do_build_and_run_instance(*args, **kwargs)"},{"line_number":1725,"context_line":"                except Exception:"},{"line_number":1726,"context_line":"                    result \u003d build_results.FAILED"},{"line_number":1727,"context_line":"                    raise"},{"line_number":1728,"context_line":"                finally:"},{"line_number":1729,"context_line":"                    fails \u003d (build_results.FAILED,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_ba6da399","line":1726,"range":{"start_line":1726,"start_character":20,"end_line":1726,"end_character":49},"in_reply_to":"7f287b81_9772c048","updated":"2017-08-24 15:57:03.000000000","message":"This exception block shouldn\u0027t actually happen unless something went wrong when _decode_files is called in _do_build_and_run_instance, otherwise L1882 is going to handle any otherwise unhandled Exceptions and return build_results.FAILED and instance.host would be None.\n\nI checked the API and we don\u0027t attempt to decode injected files there so if there is a problem with decoding those it\u0027s going to fail in the compute. Plus I don\u0027t think we\u0027d set the instance in ERROR state in that case - that just seems like a separate bug, and we should validate the injected files can be decoded in the API and raise a 400 to the user there if they can\u0027t be, but maybe it wasn\u0027t done in the API for performance reasons? I might be worrying about this too much.","commit_id":"f4c39bc21a995c8911f915e75e438c2870332b8e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f533f9ab82105fab90872939360ba434adb141e8","unresolved":false,"context_lines":[{"line_number":1723,"context_line":"                try:"},{"line_number":1724,"context_line":"                    result \u003d self._do_build_and_run_instance(*args, **kwargs)"},{"line_number":1725,"context_line":"                except Exception:"},{"line_number":1726,"context_line":"                    result \u003d build_results.FAILED"},{"line_number":1727,"context_line":"                    raise"},{"line_number":1728,"context_line":"                finally:"},{"line_number":1729,"context_line":"                    fails \u003d (build_results.FAILED,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_dad50fb9","line":1726,"range":{"start_line":1726,"start_character":20,"end_line":1726,"end_character":49},"in_reply_to":"7f287b81_ba6da399","updated":"2017-08-24 16:01:25.000000000","message":"If _decode_files fails, we never spawned a guest and we\u0027re not actually consuming resources, so I think it\u0027s actually OK to remove the allocations in this case then.","commit_id":"f4c39bc21a995c8911f915e75e438c2870332b8e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5cadeeb8ced431d3d543947d1037043fe6ff0e99","unresolved":false,"context_lines":[{"line_number":1729,"context_line":"                    fails \u003d (build_results.FAILED,"},{"line_number":1730,"context_line":"                             build_results.RESCHEDULED)"},{"line_number":1731,"context_line":"                    if result in fails:"},{"line_number":1732,"context_line":"                        # Cleanup the allocations records for the instance"},{"line_number":1733,"context_line":"                        # building failed or rescheduled."},{"line_number":1734,"context_line":"                        rt \u003d self._get_resource_tracker()"},{"line_number":1735,"context_line":"                        rt.reportclient.delete_allocation_for_instance("},{"line_number":1736,"context_line":"                            instance.uuid)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_f42462c8","line":1733,"range":{"start_line":1732,"start_character":24,"end_line":1733,"end_character":57},"updated":"2017-08-24 15:01:10.000000000","message":"Based on the discussion in https://review.openstack.org/#/c/496995/5/nova/compute/manager.py@1733 I think we should at least have some more details in this comment, like the fact that when build_results.FAILED happens, the instance is either not found (already deleted) or it\u0027s instance.host field is set to None, which means deleting it later will happen in the API and not reach the compute service to remove the allocations. I totally missed the part about instance.host \u003d None in my earlier pass on this, so given we\u0027re doing something here based on things that happen in other parts of this flow, and it\u0027s tightly-coupled that way, we should be clear in the comments to avoid confusion.","commit_id":"f4c39bc21a995c8911f915e75e438c2870332b8e"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0b39eb07ace4db767a771e03f62919e218bf31d6","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        self.assertEqual(old_flavor[\u0027disk\u0027] + new_flavor[\u0027disk\u0027],"},{"line_number":1190,"context_line":"                         allocation[\u0027DISK_GB\u0027])"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    def assertEmptyUsages(self, allocation):"},{"line_number":1193,"context_line":"        self.assertEqual(0, allocation[\u0027VCPU\u0027])"},{"line_number":1194,"context_line":"        self.assertEqual(0, allocation[\u0027MEMORY_MB\u0027])"},{"line_number":1195,"context_line":"        self.assertEqual(0, allocation[\u0027DISK_GB\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_a1e7bde4","line":1192,"range":{"start_line":1192,"start_character":8,"end_line":1192,"end_character":25},"updated":"2017-08-24 13:32:07.000000000","message":"Other tests are doing this by calling assertFlavorMatchesAllocation with something like:\n\nself.assertFlavorMatchesAllocation(\n   {\u0027vcpus\u0027: 0, \u0027ram\u0027: 0, \u0027disk\u0027: 0}, allocation)\n\nI don\u0027t care much either way since I think ^ is kind of confusing. assertEmptyUsages being explicit is probably better.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"86517afaa4070ab65d8744b850ca34f5f1654144","unresolved":false,"context_lines":[{"line_number":1189,"context_line":"        self.assertEqual(old_flavor[\u0027disk\u0027] + new_flavor[\u0027disk\u0027],"},{"line_number":1190,"context_line":"                         allocation[\u0027DISK_GB\u0027])"},{"line_number":1191,"context_line":""},{"line_number":1192,"context_line":"    def assertEmptyUsages(self, allocation):"},{"line_number":1193,"context_line":"        self.assertEqual(0, allocation[\u0027VCPU\u0027])"},{"line_number":1194,"context_line":"        self.assertEqual(0, allocation[\u0027MEMORY_MB\u0027])"},{"line_number":1195,"context_line":"        self.assertEqual(0, allocation[\u0027DISK_GB\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_1c36fae7","line":1192,"range":{"start_line":1192,"start_character":8,"end_line":1192,"end_character":25},"in_reply_to":"7f287b81_a1e7bde4","updated":"2017-08-24 13:42:33.000000000","message":"Done","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0b39eb07ace4db767a771e03f62919e218bf31d6","unresolved":false,"context_lines":[{"line_number":1883,"context_line":"            server, source_rp_uuid, dest_rp_uuid)"},{"line_number":1884,"context_line":""},{"line_number":1885,"context_line":"    def test_rescheduling_when_booting_instance(self):"},{"line_number":1886,"context_line":"        global failed_hostname"},{"line_number":1887,"context_line":"        failed_hostname \u003d None"},{"line_number":1888,"context_line":"        old_build_resources \u003d (compute_manager.ComputeManager."},{"line_number":1889,"context_line":"            _build_resources)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_41f5b982","line":1886,"range":{"start_line":1886,"start_character":8,"end_line":1886,"end_character":30},"updated":"2017-08-24 13:32:07.000000000","message":"This seems weird, do we need to rely on a global? Could we set a variable on self? For example, we have another functional test that tests reschedules between hosts:\n\nhttps://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_1671648.py#L90\n\nThe rest of this test isn\u0027t too different from that otherwise.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"86517afaa4070ab65d8744b850ca34f5f1654144","unresolved":false,"context_lines":[{"line_number":1883,"context_line":"            server, source_rp_uuid, dest_rp_uuid)"},{"line_number":1884,"context_line":""},{"line_number":1885,"context_line":"    def test_rescheduling_when_booting_instance(self):"},{"line_number":1886,"context_line":"        global failed_hostname"},{"line_number":1887,"context_line":"        failed_hostname \u003d None"},{"line_number":1888,"context_line":"        old_build_resources \u003d (compute_manager.ComputeManager."},{"line_number":1889,"context_line":"            _build_resources)"}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_9c578a03","line":1886,"range":{"start_line":1886,"start_character":8,"end_line":1886,"end_character":30},"in_reply_to":"7f287b81_41f5b982","updated":"2017-08-24 13:42:33.000000000","message":"I can give a try on the self.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0b39eb07ace4db767a771e03f62919e218bf31d6","unresolved":false,"context_lines":[{"line_number":1949,"context_line":"                networks\u003d[])"},{"line_number":1950,"context_line":""},{"line_number":1951,"context_line":"        created_server \u003d self.api.post_server({\u0027server\u0027: server_req})"},{"line_number":1952,"context_line":"        self._wait_for_state_change(self.admin_api, created_server, \u0027ERROR\u0027)"},{"line_number":1953,"context_line":""},{"line_number":1954,"context_line":"        LOG.info(\u0027failed on %s\u0027, failed_hostname)"},{"line_number":1955,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_41fff933","line":1952,"updated":"2017-08-24 13:32:07.000000000","message":"Again, instances in ERROR state that are on a compute node actually taking up resources should still have allocations claimed against that node provider so that the scheduler doesn\u0027t send other build requests there until the instance is deleted or moved.","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"86517afaa4070ab65d8744b850ca34f5f1654144","unresolved":false,"context_lines":[{"line_number":1949,"context_line":"                networks\u003d[])"},{"line_number":1950,"context_line":""},{"line_number":1951,"context_line":"        created_server \u003d self.api.post_server({\u0027server\u0027: server_req})"},{"line_number":1952,"context_line":"        self._wait_for_state_change(self.admin_api, created_server, \u0027ERROR\u0027)"},{"line_number":1953,"context_line":""},{"line_number":1954,"context_line":"        LOG.info(\u0027failed on %s\u0027, failed_hostname)"},{"line_number":1955,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7f287b81_5cafe2e2","line":1952,"in_reply_to":"7f287b81_41fff933","updated":"2017-08-24 13:42:33.000000000","message":"we nil the host and node of instance, that means we will release the resource https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1876\n\nhttps://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1890","commit_id":"f8e2c234704016aa0401c32ec11878b8bb812c41"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5cadeeb8ced431d3d543947d1037043fe6ff0e99","unresolved":false,"context_lines":[{"line_number":1126,"context_line":"                            }}"},{"line_number":1127,"context_line":""},{"line_number":1128,"context_line":"        self.flavor3 \u003d self.api.post_flavor(flavor_body)"},{"line_number":1129,"context_line":"        self.failed_hostname \u003d None"},{"line_number":1130,"context_line":""},{"line_number":1131,"context_line":"    def _other_hostname(self, host):"},{"line_number":1132,"context_line":"        other_host \u003d {\u0027host1\u0027: \u0027host2\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"7f287b81_f476a2d8","line":1129,"updated":"2017-08-24 15:01:10.000000000","message":"FWIW you could just set this in the test that uses it so it\u0027s not confusing for others to see this for unrelated tests.","commit_id":"f4c39bc21a995c8911f915e75e438c2870332b8e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7a4fee8115f3976305661d60dd8aa31ad3c0dde6","unresolved":false,"context_lines":[{"line_number":1877,"context_line":"        self._delete_and_check_allocations("},{"line_number":1878,"context_line":"            server, source_rp_uuid, dest_rp_uuid)"},{"line_number":1879,"context_line":""},{"line_number":1880,"context_line":"    def test_rescheduling_when_booting_instance(self):"},{"line_number":1881,"context_line":"        self.failed_hostname \u003d None"},{"line_number":1882,"context_line":"        old_build_resources \u003d (compute_manager.ComputeManager."},{"line_number":1883,"context_line":"            _build_resources)"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f287b81_aeb8c129","line":1880,"updated":"2017-08-24 17:32:34.000000000","message":"This isn\u0027t a move operation, so it\u0027s weird to have it in the ServerMovingTests class. Obviously it has a lot of the stuff we need, but I think we\u0027re at the point of needing some refactorage here.","commit_id":"c19ebcbd58bc673e336bc5fa3b815c7b0a20d628"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7a4fee8115f3976305661d60dd8aa31ad3c0dde6","unresolved":false,"context_lines":[{"line_number":1891,"context_line":"            return old_build_resources(sl, *args, **kwargs)"},{"line_number":1892,"context_line":""},{"line_number":1893,"context_line":"        self.stub_out(\u0027nova.compute.manager.ComputeManager._build_resources\u0027,"},{"line_number":1894,"context_line":"                      fake_build_resources)"},{"line_number":1895,"context_line":""},{"line_number":1896,"context_line":"        server_req \u003d self._build_minimal_create_server_request("},{"line_number":1897,"context_line":"                self.api, \u0027some-server\u0027, flavor_id\u003dself.flavor1[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f287b81_8ebaa507","line":1894,"updated":"2017-08-24 17:32:34.000000000","message":"IMHO, this is not a very functional test pattern, mocking out something in the middle of the compute manager. I think the better way would be to just define a subclass of the Fake driver and have it fail its first spawn.","commit_id":"c19ebcbd58bc673e336bc5fa3b815c7b0a20d628"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8fc8d8f7c5ffa06dd18de91fc48953701eb495e5","unresolved":false,"context_lines":[{"line_number":1891,"context_line":"            return old_build_resources(sl, *args, **kwargs)"},{"line_number":1892,"context_line":""},{"line_number":1893,"context_line":"        self.stub_out(\u0027nova.compute.manager.ComputeManager._build_resources\u0027,"},{"line_number":1894,"context_line":"                      fake_build_resources)"},{"line_number":1895,"context_line":""},{"line_number":1896,"context_line":"        server_req \u003d self._build_minimal_create_server_request("},{"line_number":1897,"context_line":"                self.api, \u0027some-server\u0027, flavor_id\u003dself.flavor1[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f287b81_7c57e5cb","line":1894,"in_reply_to":"7f287b81_8ebaa507","updated":"2017-08-28 20:53:13.000000000","message":"https://review.openstack.org/#/c/498596/","commit_id":"c19ebcbd58bc673e336bc5fa3b815c7b0a20d628"}]}