)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":25562,"name":"Chen","email":"dstbtgagt@foxmail.com","username":"chenn2"},"change_message_id":"ab37facddfef56a1c7813f9d1e22e561c959a592","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This change now includes pre-migrating migration records when looking"},{"line_number":16,"context_line":"for active evacuations and provides these to"},{"line_number":17,"context_line":"_destroy_evacuated_instances. Additionally these records are also used"},{"line_number":18,"context_line":"within init_host to skip running init_instance against any instance"},{"line_number":19,"context_line":"associated with an active evacuation."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Closes-bug: #1764883"},{"line_number":22,"context_line":"Change-Id: I379678dfdb2609f12a572d4f99c8e9da4deab803"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_1984660a","line":19,"range":{"start_line":17,"start_character":30,"end_line":19,"end_character":37},"updated":"2018-06-12 17:18:46.000000000","message":"Just wondering in what way this is related to the problem described in the first paragraph","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d7f66887454633fef48a73a0a20a3c3d213d9e5b","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"This change now includes pre-migrating migration records when looking"},{"line_number":16,"context_line":"for active evacuations and provides these to"},{"line_number":17,"context_line":"_destroy_evacuated_instances. Additionally these records are also used"},{"line_number":18,"context_line":"within init_host to skip running init_instance against any instance"},{"line_number":19,"context_line":"associated with an active evacuation."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Closes-bug: #1764883"},{"line_number":22,"context_line":"Change-Id: I379678dfdb2609f12a572d4f99c8e9da4deab803"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f7c97a3_85a9e57c","line":19,"range":{"start_line":17,"start_character":30,"end_line":19,"end_character":37},"in_reply_to":"5f7c97a3_1984660a","updated":"2018-06-12 20:18:59.000000000","message":"AFAICT running init_instance against an evacuating instance would set the task_state to None and race the rebuild on the destination.\n\nI initially didn\u0027t want to call it out in any great detail here as running init_instance against deleted/evacuating instances just seemed wrong anyway but happy to add more details here in a respin.","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"}],"nova/compute/manager.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"e410c37b91ff54c20e8d1bf9486f6b5b015ba6a1","unresolved":false,"context_lines":[{"line_number":623,"context_line":"        \"\"\"Destroys evacuated instances."},{"line_number":624,"context_line":""},{"line_number":625,"context_line":"        While nova-compute was down, the instances running on it could be"},{"line_number":626,"context_line":"        evacuated to another host. This method looks for evacuation migration"},{"line_number":627,"context_line":"        records where this is the source host and which were either started"},{"line_number":628,"context_line":"        (accepted) or complete (done). From those migration records, local"},{"line_number":629,"context_line":"        instances reported by the hypervisor are compared to the instances"},{"line_number":630,"context_line":"        for the migration records and those local guests are destroyed, along"},{"line_number":631,"context_line":"        with instance allocation records in Placement for this node."}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_0d9749ee","line":628,"range":{"start_line":626,"start_character":36,"end_line":628,"end_character":37},"updated":"2018-05-30 12:51:42.000000000","message":"this might need update as well","commit_id":"2cee381224cc0e204be29488645e4ae4522f2ff3"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"128171784bf6c4bd6588e30e0f3507747c359489","unresolved":false,"context_lines":[{"line_number":623,"context_line":"        \"\"\"Destroys evacuated instances."},{"line_number":624,"context_line":""},{"line_number":625,"context_line":"        While nova-compute was down, the instances running on it could be"},{"line_number":626,"context_line":"        evacuated to another host. This method looks for evacuation migration"},{"line_number":627,"context_line":"        records where this is the source host and which were either started"},{"line_number":628,"context_line":"        (accepted) or complete (done). From those migration records, local"},{"line_number":629,"context_line":"        instances reported by the hypervisor are compared to the instances"},{"line_number":630,"context_line":"        for the migration records and those local guests are destroyed, along"},{"line_number":631,"context_line":"        with instance allocation records in Placement for this node."}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_708a5664","line":628,"range":{"start_line":626,"start_character":36,"end_line":628,"end_character":37},"in_reply_to":"5f7c97a3_0d9749ee","updated":"2018-05-30 13:17:15.000000000","message":"ACK, nice catch, thanks!","commit_id":"2cee381224cc0e204be29488645e4ae4522f2ff3"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"e410c37b91ff54c20e8d1bf9486f6b5b015ba6a1","unresolved":false,"context_lines":[{"line_number":794,"context_line":"            instance.task_state in [task_states.REBUILDING,"},{"line_number":795,"context_line":"                                    task_states.REBUILD_BLOCK_DEVICE_MAPPING,"},{"line_number":796,"context_line":"                                    task_states.REBUILD_SPAWNING]):"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"            # NOTE(jichenjc) compute stopped before instance was fully"},{"line_number":799,"context_line":"            # spawned so set to ERROR state. This is consistent to BUILD"},{"line_number":800,"context_line":"            LOG.debug(\"Instance failed to rebuild correctly, \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_8d57d958","line":797,"updated":"2018-05-30 12:51:42.000000000","message":"unrelated change","commit_id":"2cee381224cc0e204be29488645e4ae4522f2ff3"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"128171784bf6c4bd6588e30e0f3507747c359489","unresolved":false,"context_lines":[{"line_number":794,"context_line":"            instance.task_state in [task_states.REBUILDING,"},{"line_number":795,"context_line":"                                    task_states.REBUILD_BLOCK_DEVICE_MAPPING,"},{"line_number":796,"context_line":"                                    task_states.REBUILD_SPAWNING]):"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"            # NOTE(jichenjc) compute stopped before instance was fully"},{"line_number":799,"context_line":"            # spawned so set to ERROR state. This is consistent to BUILD"},{"line_number":800,"context_line":"            LOG.debug(\"Instance failed to rebuild correctly, \""}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_508f1a74","line":797,"in_reply_to":"5f7c97a3_8d57d958","updated":"2018-05-30 13:17:15.000000000","message":"Fixed.","commit_id":"2cee381224cc0e204be29488645e4ae4522f2ff3"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"e410c37b91ff54c20e8d1bf9486f6b5b015ba6a1","unresolved":false,"context_lines":[{"line_number":1144,"context_line":"            self._destroy_evacuated_instances(context, evacuations)"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"            # Initialise instances on the host that are not evacuating"},{"line_number":1147,"context_line":"            for inst in (i for i in instances if i[\u0027uuid\u0027] not in evacuations):"},{"line_number":1148,"context_line":"                self._init_instance(context, inst)"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"        finally:"},{"line_number":1151,"context_line":"            if CONF.defer_iptables_apply:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_cd9cf10f","line":1148,"range":{"start_line":1147,"start_character":12,"end_line":1148,"end_character":50},"updated":"2018-05-30 12:51:42.000000000","message":"do we need a ut for this?","commit_id":"2cee381224cc0e204be29488645e4ae4522f2ff3"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"128171784bf6c4bd6588e30e0f3507747c359489","unresolved":false,"context_lines":[{"line_number":1144,"context_line":"            self._destroy_evacuated_instances(context, evacuations)"},{"line_number":1145,"context_line":""},{"line_number":1146,"context_line":"            # Initialise instances on the host that are not evacuating"},{"line_number":1147,"context_line":"            for inst in (i for i in instances if i[\u0027uuid\u0027] not in evacuations):"},{"line_number":1148,"context_line":"                self._init_instance(context, inst)"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":"        finally:"},{"line_number":1151,"context_line":"            if CONF.defer_iptables_apply:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f7c97a3_b0946e8c","line":1148,"range":{"start_line":1147,"start_character":12,"end_line":1148,"end_character":50},"in_reply_to":"5f7c97a3_cd9cf10f","updated":"2018-05-30 13:17:15.000000000","message":"Yeah, it would help if I included my changes covering this in the commit wouldn\u0027t it :)","commit_id":"2cee381224cc0e204be29488645e4ae4522f2ff3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d21543531e7c1ce98c99e338c7c0b6e2e8d83a60","unresolved":false,"context_lines":[{"line_number":624,"context_line":"        While nova-compute was down, the instances running on it could be"},{"line_number":625,"context_line":"        evacuated to another host. This method takes a list of evacuation"},{"line_number":626,"context_line":"        migration records where this is the source host and which were either"},{"line_number":627,"context_line":"        started (accepted), in-progress (pre-migrating) or complete (done)."},{"line_number":628,"context_line":"        From those migration records, local instances reported by the"},{"line_number":629,"context_line":"        hypervisor are compared to the instances for the migration records and"},{"line_number":630,"context_line":"        those local guests are destroyed, along with instance allocation"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_6081a4fb","line":627,"range":{"start_line":627,"start_character":59,"end_line":627,"end_character":74},"updated":"2018-06-18 18:39:11.000000000","message":"\"done\" is the state when the destination has completed the adoption, and \"complete\" is when the source has cleaned up. So, I think I would avoid this wording :)","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3f8e82689078b3f54059406b1ae2c5e90de0373a","unresolved":false,"context_lines":[{"line_number":624,"context_line":"        While nova-compute was down, the instances running on it could be"},{"line_number":625,"context_line":"        evacuated to another host. This method takes a list of evacuation"},{"line_number":626,"context_line":"        migration records where this is the source host and which were either"},{"line_number":627,"context_line":"        started (accepted), in-progress (pre-migrating) or complete (done)."},{"line_number":628,"context_line":"        From those migration records, local instances reported by the"},{"line_number":629,"context_line":"        hypervisor are compared to the instances for the migration records and"},{"line_number":630,"context_line":"        those local guests are destroyed, along with instance allocation"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_518d5c33","line":627,"range":{"start_line":627,"start_character":59,"end_line":627,"end_character":74},"in_reply_to":"5f7c97a3_6081a4fb","updated":"2018-06-19 14:29:55.000000000","message":"Done","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d21543531e7c1ce98c99e338c7c0b6e2e8d83a60","unresolved":false,"context_lines":[{"line_number":1130,"context_line":"                # source compute can come back online shortly after the RT"},{"line_number":1131,"context_line":"                # claims on the destination that in-turn moves the migration to"},{"line_number":1132,"context_line":"                # pre-migrating."},{"line_number":1133,"context_line":"                \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":1134,"context_line":"                \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":1135,"context_line":"            }"},{"line_number":1136,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_40d58846","line":1133,"range":{"start_line":1133,"start_character":39,"end_line":1133,"end_character":54},"updated":"2018-06-18 18:39:11.000000000","message":"Note to others, because this took me some digging to find:\n\nThis never gets set in any rebuild paths, but does deep in resource tracker because of _move_claim, which will set this state on a migration record related to a move if it finds one.\n\nI was going to argue that we never set this state during an evacuation, but it gets set implicitly in the bowels of the thing.\n\nIf the source is allowed to come up before the evacuation has completed, I think we\u0027ve got other issues. For one thing, we\u0027ll fail to find the migration in the evacuation code in conductor, which means we won\u0027t track where the instance went, and we won\u0027t mark it as error if it actually failed to start on the destination, or if we don\u0027t schedule it. Instead, these will sit at \"complete\".\n\nHowever, having them go from complete to \"error\" would also be kinda weird. Actually, I think that RT will create a new migration for the instance, which will also be weird.\n\nSo, not that we shouldn\u0027t include this, but I think allowing the source to startup before things are finished will still get us into a bad state.","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dcf438d4af4c038f4e9968444adece0e9735823d","unresolved":false,"context_lines":[{"line_number":1130,"context_line":"                # source compute can come back online shortly after the RT"},{"line_number":1131,"context_line":"                # claims on the destination that in-turn moves the migration to"},{"line_number":1132,"context_line":"                # pre-migrating."},{"line_number":1133,"context_line":"                \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":1134,"context_line":"                \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":1135,"context_line":"            }"},{"line_number":1136,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_aa2e9789","line":1133,"range":{"start_line":1133,"start_character":39,"end_line":1133,"end_character":54},"in_reply_to":"5f7c97a3_0ac9d458","updated":"2018-06-19 20:36:34.000000000","message":"Yeah, it\u0027ll be hard to hit all of them reliably without instrumenting some breakpoints in the runtime code I think.","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c566b409756df61722e054c6415c3922c12d2bfb","unresolved":false,"context_lines":[{"line_number":1130,"context_line":"                # source compute can come back online shortly after the RT"},{"line_number":1131,"context_line":"                # claims on the destination that in-turn moves the migration to"},{"line_number":1132,"context_line":"                # pre-migrating."},{"line_number":1133,"context_line":"                \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":1134,"context_line":"                \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":1135,"context_line":"            }"},{"line_number":1136,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_1c2bf9ea","line":1133,"range":{"start_line":1133,"start_character":39,"end_line":1133,"end_character":54},"in_reply_to":"5f7c97a3_40d58846","updated":"2018-06-25 13:10:48.000000000","message":"\u003e For one thing, we\u0027ll fail to find the migration in the evacuation code in conductor\n\nWhy? Is that when we\u0027re in rebuild_instance in conductor doing the evac and the source comes up at the same time?\n\nAt that point, the migration record is created in the API and it\u0027s status would be \u0027accepted\u0027 which we already check here on startup:\n\nhttps://github.com/openstack/nova/blob/9113dc0c067d7b48a0ec0c8308d4c5f36dbd24a0/nova/compute/api.py#L4451\n\nSo we should find the migration here in conductor:\n\nhttps://github.com/openstack/nova/blob/9113dc0c067d7b48a0ec0c8308d4c5f36dbd24a0/nova/conductor/manager.py#L891\n\nIf we\u0027re worried about the migration record lookup in conductor, we could pass it from the API: https://review.openstack.org/#/c/500176/ - but I don\u0027t think I\u0027m following the concern or the comment.","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3f8e82689078b3f54059406b1ae2c5e90de0373a","unresolved":false,"context_lines":[{"line_number":1130,"context_line":"                # source compute can come back online shortly after the RT"},{"line_number":1131,"context_line":"                # claims on the destination that in-turn moves the migration to"},{"line_number":1132,"context_line":"                # pre-migrating."},{"line_number":1133,"context_line":"                \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":1134,"context_line":"                \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":1135,"context_line":"            }"},{"line_number":1136,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_0ac9d458","line":1133,"range":{"start_line":1133,"start_character":39,"end_line":1133,"end_character":54},"in_reply_to":"5f7c97a3_40d58846","updated":"2018-06-19 14:29:55.000000000","message":"Yeah I can\u0027t seem to trigger any additional weird behaviour in my func tests here but I agree there\u0027s likely more dragons lurking when the source does return early.","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d21543531e7c1ce98c99e338c7c0b6e2e8d83a60","unresolved":false,"context_lines":[{"line_number":1137,"context_line":"            with utils.temporary_mutation(context, read_deleted\u003d\u0027yes\u0027):"},{"line_number":1138,"context_line":"                evacuations \u003d objects.MigrationList.get_by_filters(context,"},{"line_number":1139,"context_line":"                                                                   filters)"},{"line_number":1140,"context_line":"            evacuations \u003d {mig.instance_uuid: mig for mig in evacuations}"},{"line_number":1141,"context_line":""},{"line_number":1142,"context_line":"            # checking that instance was not already evacuated to other host"},{"line_number":1143,"context_line":"            self._destroy_evacuated_instances(context, evacuations)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_203e4cb9","line":1140,"updated":"2018-06-18 18:39:11.000000000","message":"I\u0027d rather not put all of this evac-specific stuff in init_host(). Why not just make _destroy_evacuated_instances() return a list of things it thinks qualify, and then use that to skip init_instance below?\n\nMoving the query logic down here just so you have the list is a bit overkill I think. Plus, the _destroy_evacuated_instances() logic could later determine that something isn\u0027t worth destroying even though it was technically an evac or something, so returning the list of things to continue with seems like good separation.","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3f8e82689078b3f54059406b1ae2c5e90de0373a","unresolved":false,"context_lines":[{"line_number":1137,"context_line":"            with utils.temporary_mutation(context, read_deleted\u003d\u0027yes\u0027):"},{"line_number":1138,"context_line":"                evacuations \u003d objects.MigrationList.get_by_filters(context,"},{"line_number":1139,"context_line":"                                                                   filters)"},{"line_number":1140,"context_line":"            evacuations \u003d {mig.instance_uuid: mig for mig in evacuations}"},{"line_number":1141,"context_line":""},{"line_number":1142,"context_line":"            # checking that instance was not already evacuated to other host"},{"line_number":1143,"context_line":"            self._destroy_evacuated_instances(context, evacuations)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_4ae4ace0","line":1140,"in_reply_to":"5f7c97a3_203e4cb9","updated":"2018-06-19 14:29:55.000000000","message":"Done.","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"0a5ff9573ebb5ee6f896f9af93857572e72b5362","unresolved":false,"context_lines":[{"line_number":622,"context_line":"        \"\"\"Destroys evacuated instances."},{"line_number":623,"context_line":""},{"line_number":624,"context_line":"        While nova-compute was down, the instances running on it could be"},{"line_number":625,"context_line":"        evacuated to another host. This method takes a list of evacuation"},{"line_number":626,"context_line":"        migration records where this is the source host and which were either"},{"line_number":627,"context_line":"        started (accepted), in-progress (pre-migrating) or migrated (done)."},{"line_number":628,"context_line":"        From those migration records, local instances reported by the"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_87c93b52","line":625,"range":{"start_line":625,"start_character":47,"end_line":625,"end_character":62},"updated":"2018-06-20 19:03:24.000000000","message":"nit","commit_id":"e9a98b0178006944f8228eae6964c7a1094e27e8"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"0a5ff9573ebb5ee6f896f9af93857572e72b5362","unresolved":false,"context_lines":[{"line_number":643,"context_line":"            \u0027status\u0027: [\u0027accepted\u0027, \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":644,"context_line":"            \u0027migration_type\u0027: \u0027evacuation\u0027,"},{"line_number":645,"context_line":"        }"},{"line_number":646,"context_line":""},{"line_number":647,"context_line":"        with utils.temporary_mutation(context, read_deleted\u003d\u0027yes\u0027):"},{"line_number":648,"context_line":"            evacuations \u003d objects.MigrationList.get_by_filters(context,"},{"line_number":649,"context_line":"                                                               filters)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5f7c97a3_67ce5f5c","line":646,"updated":"2018-06-20 19:03:24.000000000","message":"nit","commit_id":"e9a98b0178006944f8228eae6964c7a1094e27e8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3f9f13f24e399ef4eb323c18e2d0c62f54fdd60","unresolved":false,"context_lines":[{"line_number":636,"context_line":"            # included in case the source node comes back up while instances"},{"line_number":637,"context_line":"            # are being evacuated to another host. We don\u0027t want the same"},{"line_number":638,"context_line":"            # instance being reported from multiple hosts."},{"line_number":639,"context_line":"            # NOTE(lyarwood): pre-migrating is also included here as the"},{"line_number":640,"context_line":"            # source compute can come back online shortly after the RT"},{"line_number":641,"context_line":"            # claims on the destination that in-turn moves the migration to"},{"line_number":642,"context_line":"            # pre-migrating."}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_2e0d293f","line":639,"updated":"2018-07-20 19:23:13.000000000","message":"What happens if the rebuild on the destination host fails? We could have destroyed the guest from the source here, evac on the dest fails, then what? Will a rebuild of the server (now that the source compute is back up) fix it? I think the answer is yes but needed to ask. We\u0027ll only update the instance.host on the destination during evacuate if we successfully rebuilt the instance on that host:\n\nhttps://github.com/openstack/nova/blob/d9e04c4ff0b1a9c3383f1848dc846e93030d83cb/nova/compute/manager.py#L2977","commit_id":"6e9a6ef56e74aaf9d518b6ee294c33758fce7289"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aecdd9826b0c02a9bd885a161bf1327d55adf591","unresolved":false,"context_lines":[{"line_number":636,"context_line":"            # included in case the source node comes back up while instances"},{"line_number":637,"context_line":"            # are being evacuated to another host. We don\u0027t want the same"},{"line_number":638,"context_line":"            # instance being reported from multiple hosts."},{"line_number":639,"context_line":"            # NOTE(lyarwood): pre-migrating is also included here as the"},{"line_number":640,"context_line":"            # source compute can come back online shortly after the RT"},{"line_number":641,"context_line":"            # claims on the destination that in-turn moves the migration to"},{"line_number":642,"context_line":"            # pre-migrating."}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_39ec9526","line":639,"in_reply_to":"5f7c97a3_2e0d293f","updated":"2018-07-20 19:46:42.000000000","message":"Done","commit_id":"6e9a6ef56e74aaf9d518b6ee294c33758fce7289"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3f9f13f24e399ef4eb323c18e2d0c62f54fdd60","unresolved":false,"context_lines":[{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"            # Initialise instances on the host that are not evacuating"},{"line_number":1145,"context_line":"            for instance in instances:"},{"line_number":1146,"context_line":"                if not evacuated_instances:"},{"line_number":1147,"context_line":"                    self._init_instance(context, instance)"},{"line_number":1148,"context_line":"                elif instance.uuid not in evacuated_instances:"},{"line_number":1149,"context_line":"                    self._init_instance(context, instance)"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_99ffe9fe","line":1148,"range":{"start_line":1146,"start_character":16,"end_line":1148,"end_character":62},"updated":"2018-07-20 19:23:13.000000000","message":"This could be a single OR yeah?\n\nif not evacuated_instances or instance.uuid in evacuated_instances:\n    self._init_instance(context, instance)","commit_id":"6e9a6ef56e74aaf9d518b6ee294c33758fce7289"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aecdd9826b0c02a9bd885a161bf1327d55adf591","unresolved":false,"context_lines":[{"line_number":1143,"context_line":""},{"line_number":1144,"context_line":"            # Initialise instances on the host that are not evacuating"},{"line_number":1145,"context_line":"            for instance in instances:"},{"line_number":1146,"context_line":"                if not evacuated_instances:"},{"line_number":1147,"context_line":"                    self._init_instance(context, instance)"},{"line_number":1148,"context_line":"                elif instance.uuid not in evacuated_instances:"},{"line_number":1149,"context_line":"                    self._init_instance(context, instance)"},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"        finally:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_59ef111b","line":1148,"range":{"start_line":1146,"start_character":16,"end_line":1148,"end_character":62},"in_reply_to":"5f7c97a3_99ffe9fe","updated":"2018-07-20 19:46:42.000000000","message":"Done","commit_id":"6e9a6ef56e74aaf9d518b6ee294c33758fce7289"}],"nova/tests/functional/regressions/test_bug_1764883.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d21543531e7c1ce98c99e338c7c0b6e2e8d83a60","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        self._wait_for_migration_status(server, [\u0027pre-migrating\u0027])"},{"line_number":110,"context_line":"        self.computes.get(source_compute).start()"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        # Wait for the instance to go into an ERROR state, update our copy and"},{"line_number":113,"context_line":"        # ensure the task_state is None and it has remained on the source"},{"line_number":114,"context_line":"        self._wait_for_state_change(self.api, server, \u0027ACTIVE\u0027)"},{"line_number":115,"context_line":"        server \u003d self.api.get_server(server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_c0dd1814","line":112,"range":{"start_line":112,"start_character":46,"end_line":112,"end_character":51},"updated":"2018-06-18 18:39:11.000000000","message":"Update this?","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3f8e82689078b3f54059406b1ae2c5e90de0373a","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        self._wait_for_migration_status(server, [\u0027pre-migrating\u0027])"},{"line_number":110,"context_line":"        self.computes.get(source_compute).start()"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        # Wait for the instance to go into an ERROR state, update our copy and"},{"line_number":113,"context_line":"        # ensure the task_state is None and it has remained on the source"},{"line_number":114,"context_line":"        self._wait_for_state_change(self.api, server, \u0027ACTIVE\u0027)"},{"line_number":115,"context_line":"        server \u003d self.api.get_server(server[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_ead258fd","line":112,"range":{"start_line":112,"start_character":46,"end_line":112,"end_character":51},"in_reply_to":"5f7c97a3_c0dd1814","updated":"2018-06-19 14:29:55.000000000","message":"Done","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d21543531e7c1ce98c99e338c7c0b6e2e8d83a60","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        self._wait_for_state_change(self.api, server, \u0027ACTIVE\u0027)"},{"line_number":115,"context_line":"        server \u003d self.api.get_server(server[\u0027id\u0027])"},{"line_number":116,"context_line":"        self.assertIsNone(server[\u0027OS-EXT-STS:task_state\u0027])"},{"line_number":117,"context_line":"        self.assertNotEqual(source_compute, server[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_00d31003","line":117,"updated":"2018-06-18 18:39:11.000000000","message":"Maybe you should assert the state(s) of the migration record and check to see if we got a new one?","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"3f8e82689078b3f54059406b1ae2c5e90de0373a","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        self._wait_for_state_change(self.api, server, \u0027ACTIVE\u0027)"},{"line_number":115,"context_line":"        server \u003d self.api.get_server(server[\u0027id\u0027])"},{"line_number":116,"context_line":"        self.assertIsNone(server[\u0027OS-EXT-STS:task_state\u0027])"},{"line_number":117,"context_line":"        self.assertNotEqual(source_compute, server[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_caf0bc94","line":117,"in_reply_to":"5f7c97a3_00d31003","updated":"2018-06-19 14:29:55.000000000","message":"Done, I only see one migration record FWIW.","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dcf438d4af4c038f4e9968444adece0e9735823d","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        self._wait_for_state_change(self.api, server, \u0027ACTIVE\u0027)"},{"line_number":115,"context_line":"        server \u003d self.api.get_server(server[\u0027id\u0027])"},{"line_number":116,"context_line":"        self.assertIsNone(server[\u0027OS-EXT-STS:task_state\u0027])"},{"line_number":117,"context_line":"        self.assertNotEqual(source_compute, server[\u0027OS-EXT-SRV-ATTR:host\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f7c97a3_6a4e3f6f","line":117,"in_reply_to":"5f7c97a3_caf0bc94","updated":"2018-06-19 20:36:34.000000000","message":"Yeah, you\u0027d have to hit a specific ordering to see the dupe (start the source and finish the migration before the destination found it).","commit_id":"87de0c79497b821f91dd3b954836a2c150df78dc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3f9f13f24e399ef4eb323c18e2d0c62f54fdd60","unresolved":false,"context_lines":[{"line_number":26,"context_line":"    \"\"\"Assert the behaviour of evacuating instances when the src returns early."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    This test asserts that evacuating instances end up in an ACTIVE state on"},{"line_number":29,"context_line":"    the destination even when that host comes back online during an evacuation"},{"line_number":30,"context_line":"    while the migration record is in a pre-migrating state."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_d910014f","line":29,"range":{"start_line":29,"start_character":30,"end_line":29,"end_character":39},"updated":"2018-07-20 19:23:13.000000000","message":"This is wrong, the dest host doesn\u0027t come back online, the source host does in this scenario.","commit_id":"6e9a6ef56e74aaf9d518b6ee294c33758fce7289"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aecdd9826b0c02a9bd885a161bf1327d55adf591","unresolved":false,"context_lines":[{"line_number":26,"context_line":"    \"\"\"Assert the behaviour of evacuating instances when the src returns early."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    This test asserts that evacuating instances end up in an ACTIVE state on"},{"line_number":29,"context_line":"    the destination even when that host comes back online during an evacuation"},{"line_number":30,"context_line":"    while the migration record is in a pre-migrating state."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_b9df4565","line":29,"range":{"start_line":29,"start_character":30,"end_line":29,"end_character":39},"in_reply_to":"5f7c97a3_d910014f","updated":"2018-07-20 19:46:42.000000000","message":"Done","commit_id":"6e9a6ef56e74aaf9d518b6ee294c33758fce7289"}],"nova/tests/unit/compute/test_compute.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c3f9f13f24e399ef4eb323c18e2d0c62f54fdd60","unresolved":false,"context_lines":[{"line_number":7500,"context_line":""},{"line_number":7501,"context_line":"        mock_get_filter.assert_called_once_with(fake_context,"},{"line_number":7502,"context_line":"                                         {\u0027source_compute\u0027: self.compute.host,"},{"line_number":7503,"context_line":"                                          \u0027status\u0027: [\u0027accepted\u0027,"},{"line_number":7504,"context_line":"                                            \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":7505,"context_line":"                                          \u0027migration_type\u0027: \u0027evacuation\u0027})"},{"line_number":7506,"context_line":"        mock_get_inst.assert_called_once_with(fake_context)"},{"line_number":7507,"context_line":"        mock_get_nw.assert_called_once_with(fake_context, evacuated_instance)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_591e315e","line":7504,"range":{"start_line":7503,"start_character":52,"end_line":7504,"end_character":69},"updated":"2018-07-20 19:23:13.000000000","message":"nit: let\u0027s fix up this alignment","commit_id":"6e9a6ef56e74aaf9d518b6ee294c33758fce7289"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aecdd9826b0c02a9bd885a161bf1327d55adf591","unresolved":false,"context_lines":[{"line_number":7500,"context_line":""},{"line_number":7501,"context_line":"        mock_get_filter.assert_called_once_with(fake_context,"},{"line_number":7502,"context_line":"                                         {\u0027source_compute\u0027: self.compute.host,"},{"line_number":7503,"context_line":"                                          \u0027status\u0027: [\u0027accepted\u0027,"},{"line_number":7504,"context_line":"                                            \u0027pre-migrating\u0027, \u0027done\u0027],"},{"line_number":7505,"context_line":"                                          \u0027migration_type\u0027: \u0027evacuation\u0027})"},{"line_number":7506,"context_line":"        mock_get_inst.assert_called_once_with(fake_context)"},{"line_number":7507,"context_line":"        mock_get_nw.assert_called_once_with(fake_context, evacuated_instance)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5f7c97a3_d9da4174","line":7504,"range":{"start_line":7503,"start_character":52,"end_line":7504,"end_character":69},"in_reply_to":"5f7c97a3_591e315e","updated":"2018-07-20 19:46:42.000000000","message":"Done","commit_id":"6e9a6ef56e74aaf9d518b6ee294c33758fce7289"}]}
