)]}'
{"nova/compute/api.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"6724e327bf6c1c0585f329b84ec7b9ee45815fa6","unresolved":false,"context_lines":[{"line_number":3033,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3034,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3035,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3036,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3037,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3038,"context_line":""},{"line_number":3039,"context_line":"        # If this is a resize down, a revert might go over quota."}],"source_content_type":"text/x-python","patch_set":10,"id":"7f515b1d_f07c3084","line":3036,"range":{"start_line":3036,"start_character":28,"end_line":3036,"end_character":48},"updated":"2017-09-27 06:48:38.000000000","message":"please update https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/servers.py#L745 to handle this exception ,or we will have 500 error...","commit_id":"2c9f2ee30d5a2801ef096ee22adcb2e09a82ed0a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0373ae1d0d58ebe559a701ad61c4ef09e28c453a","unresolved":false,"context_lines":[{"line_number":3033,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3034,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3035,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3036,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3037,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3038,"context_line":""},{"line_number":3039,"context_line":"        # If this is a resize down, a revert might go over quota."}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_35acf79d","line":3036,"range":{"start_line":3036,"start_character":28,"end_line":3036,"end_character":48},"in_reply_to":"3f4b6375_77f3e19f","updated":"2019-06-26 16:55:22.000000000","message":"\u003e I guess that makes sense. If a developer calls _revert_resize with\n \u003e the wrong migration type in the future, we\u0027ll get a trace and a 500\n \u003e instead of proceeding to call nova-compute with the wrong stuff.\n\nThis was a super old comment in my drafts, please ignore.","commit_id":"2c9f2ee30d5a2801ef096ee22adcb2e09a82ed0a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"52599964a2f7a83802bbd140e50a4d08bad1d550","unresolved":false,"context_lines":[{"line_number":3033,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3034,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3035,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3036,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3037,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3038,"context_line":""},{"line_number":3039,"context_line":"        # If this is a resize down, a revert might go over quota."}],"source_content_type":"text/x-python","patch_set":10,"id":"3f4b6375_77f3e19f","line":3036,"range":{"start_line":3036,"start_character":28,"end_line":3036,"end_character":48},"in_reply_to":"7f515b1d_b9fa1572","updated":"2019-06-26 16:52:59.000000000","message":"I guess that makes sense. If a developer calls _revert_resize with the wrong migration type in the future, we\u0027ll get a trace and a 500 instead of proceeding to call nova-compute with the wrong stuff.","commit_id":"2c9f2ee30d5a2801ef096ee22adcb2e09a82ed0a"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"700b32078ac107f3b9934458ef4e9e9fc7ed5118","unresolved":false,"context_lines":[{"line_number":3033,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3034,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3035,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3036,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3037,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3038,"context_line":""},{"line_number":3039,"context_line":"        # If this is a resize down, a revert might go over quota."}],"source_content_type":"text/x-python","patch_set":10,"id":"7f515b1d_b9fa1572","line":3036,"range":{"start_line":3036,"start_character":28,"end_line":3036,"end_character":48},"in_reply_to":"7f515b1d_f07c3084","updated":"2017-10-09 09:32:02.000000000","message":"A 500 error would be correct here, as this is essentially an assertion. A user should not be able to cause this to happen.","commit_id":"2c9f2ee30d5a2801ef096ee22adcb2e09a82ed0a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"11912846d35f4e522ec91a6ca357edbd889d1eeb","unresolved":false,"context_lines":[{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3074,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3075,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3076,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3077,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3078,"context_line":""},{"line_number":3079,"context_line":"        # If this is a resize down, a revert might go over quota."},{"line_number":3080,"context_line":"        self._check_quota_for_upsize(context, instance, instance.flavor,"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_336e2e2f","line":3077,"range":{"start_line":3075,"start_character":0,"end_line":3077,"end_character":67},"updated":"2018-01-31 15:31:31.000000000","message":"this seems kind of odd... was this just to provide an easy way to inject an error for testing or was this an actual error that was happening somehow?","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"22b88dfdce4a974bf94e473f1b8236404ebf7032","unresolved":false,"context_lines":[{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3074,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3075,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3076,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3077,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3078,"context_line":""},{"line_number":3079,"context_line":"        # If this is a resize down, a revert might go over quota."},{"line_number":3080,"context_line":"        self._check_quota_for_upsize(context, instance, instance.flavor,"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_b9645d23","line":3077,"range":{"start_line":3075,"start_character":0,"end_line":3077,"end_character":67},"in_reply_to":"5f93b717_336e2e2f","updated":"2018-01-31 16:50:33.000000000","message":"Uh, yeah, and also seemingly not related to the thing this is trying to change.\n\nIf this is really something we need, it\u0027d be good to have it separated with its own justification.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b612389f0667f341195e7f4ef768713fcc7d321c","unresolved":false,"context_lines":[{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3074,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3075,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3076,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3077,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3078,"context_line":""},{"line_number":3079,"context_line":"        # If this is a resize down, a revert might go over quota."},{"line_number":3080,"context_line":"        self._check_quota_for_upsize(context, instance, instance.flavor,"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_de8eea77","line":3077,"range":{"start_line":3075,"start_character":0,"end_line":3077,"end_character":67},"in_reply_to":"5f93b717_88ad889c","updated":"2018-01-31 18:38:18.000000000","message":"So make it a separate patch and build the justification there.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6729acaf72c733fda8b83aa4651139b7bcff0359","unresolved":false,"context_lines":[{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3074,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3075,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3076,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3077,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3078,"context_line":""},{"line_number":3079,"context_line":"        # If this is a resize down, a revert might go over quota."},{"line_number":3080,"context_line":"        self._check_quota_for_upsize(context, instance, instance.flavor,"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_88ad889c","line":3077,"range":{"start_line":3075,"start_character":0,"end_line":3077,"end_character":67},"in_reply_to":"5f93b717_b9645d23","updated":"2018-01-31 18:28:32.000000000","message":"There\u0027s probably a really good reason for this requiring a bunch of context I\u0027ve long since forgotten. Uncharacteristically, I haven\u0027t added a comment :/\n\nI suspect, though, it\u0027s because when called via revert_resize_on_error we can\u0027t check for vm_state\u003dRESIZED.\n\nNot unrelated: there\u0027s no order-by in migration_get_by_instance_and_status().\n\nIt\u0027s obviously not a problem to take this out of this patch. In general, though, it\u0027s probably a good assertion to make in this code path, even if I can\u0027t remember exactly why I added it.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6a317474992891f29aa4ddd5bda25035131c476a","unresolved":false,"context_lines":[{"line_number":3072,"context_line":""},{"line_number":3073,"context_line":"    def _revert_resize(self, context, instance, migration,"},{"line_number":3074,"context_line":"                       check_task_state\u003dTrue):"},{"line_number":3075,"context_line":"        if migration.migration_type !\u003d \u0027resize\u0027:"},{"line_number":3076,"context_line":"            raise exception.InvalidMigrationType(type\u003dmigration.migration_type,"},{"line_number":3077,"context_line":"                                                 expected\u003d\u0027resize\u0027)"},{"line_number":3078,"context_line":""},{"line_number":3079,"context_line":"        # If this is a resize down, a revert might go over quota."},{"line_number":3080,"context_line":"        self._check_quota_for_upsize(context, instance, instance.flavor,"}],"source_content_type":"text/x-python","patch_set":12,"id":"3fa7e38b_79338ff7","line":3077,"range":{"start_line":3075,"start_character":0,"end_line":3077,"end_character":67},"in_reply_to":"5f93b717_de8eea77","updated":"2019-11-13 22:45:33.000000000","message":"The idea was probably something along the lines of only allow reverting a resize on an instance in ERROR status if it\u0027s going through a resize. There are other ways to do that though, e.g. checking the old_vm_state system metadata, old/new flavor or migration_context stashed on the instance which aren\u0027t cleaned up when finish_resize fails.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"22b88dfdce4a974bf94e473f1b8236404ebf7032","unresolved":false,"context_lines":[{"line_number":3086,"context_line":"            expected_task_state \u003d None"},{"line_number":3087,"context_line":""},{"line_number":3088,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":3089,"context_line":"        instance.save(expected_task_state\u003dexpected_task_state)"},{"line_number":3090,"context_line":""},{"line_number":3091,"context_line":"        migration.status \u003d \u0027reverting\u0027"},{"line_number":3092,"context_line":"        migration.save()"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_390fed37","line":3089,"updated":"2018-01-31 16:50:33.000000000","message":"We should be able to provide a list of possible task states the thing could be in that we consider legit for auto-revert right? Just ignoring task state for the thing that is supposed to provide locking across multiple concurrent actions doesn\u0027t seem like a good plan to me.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6729acaf72c733fda8b83aa4651139b7bcff0359","unresolved":false,"context_lines":[{"line_number":3086,"context_line":"            expected_task_state \u003d None"},{"line_number":3087,"context_line":""},{"line_number":3088,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":3089,"context_line":"        instance.save(expected_task_state\u003dexpected_task_state)"},{"line_number":3090,"context_line":""},{"line_number":3091,"context_line":"        migration.status \u003d \u0027reverting\u0027"},{"line_number":3092,"context_line":"        migration.save()"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_68e4bce0","line":3089,"in_reply_to":"5f93b717_390fed37","updated":"2018-01-31 18:28:32.000000000","message":"We could, but I suspect it would be large enough to be meaningless, a maintenance burden, and an ongoing source of errors when it isn\u0027t maintained. Think: a new state is added, but somebody forgets to add it to this list, and the unit test for this list. Everything will work fine until it fails in production in the new state, at which point it will fail unnecessarily.\n\nHowever, remember we only call revert_resize_on_error from a resize operation, so we\u0027re already holding the task state \u0027lock\u0027 here. I could make this clearer with a comment.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b612389f0667f341195e7f4ef768713fcc7d321c","unresolved":false,"context_lines":[{"line_number":3086,"context_line":"            expected_task_state \u003d None"},{"line_number":3087,"context_line":""},{"line_number":3088,"context_line":"        instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":3089,"context_line":"        instance.save(expected_task_state\u003dexpected_task_state)"},{"line_number":3090,"context_line":""},{"line_number":3091,"context_line":"        migration.status \u003d \u0027reverting\u0027"},{"line_number":3092,"context_line":"        migration.save()"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_fee08628","line":3089,"in_reply_to":"5f93b717_68e4bce0","updated":"2018-01-31 18:38:18.000000000","message":"Shouldn\u0027t that mean the list of states is really small then?","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"}],"nova/compute/manager.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"50f948fef4c8ca08c65ab4b034fef290f1ca2a74","unresolved":false,"context_lines":[{"line_number":4013,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4014,"context_line":"                # NOTE(mdbooth): We set instance.task_state and"},{"line_number":4015,"context_line":"                # migration.status here to the same values they are set to in"},{"line_number":4016,"context_line":"                # API.revert_resize()."},{"line_number":4017,"context_line":"                instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":4018,"context_line":"                instance.save()"},{"line_number":4019,"context_line":"                migration.status \u003d \u0027reverting\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f287b81_324c6cc6","line":4016,"updated":"2017-08-23 07:59:59.000000000","message":"api revert_resize side has a over quota check ,is it possible a race condition might cause over quota issue if the original resize is a resize down action?","commit_id":"a3362a66610012af9ac2e7e5cac25c4d5ab25ef1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d07f21c640685154e3dd3f4a9e2a7d9b2e284fde","unresolved":false,"context_lines":[{"line_number":4013,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4014,"context_line":"                # NOTE(mdbooth): We set instance.task_state and"},{"line_number":4015,"context_line":"                # migration.status here to the same values they are set to in"},{"line_number":4016,"context_line":"                # API.revert_resize()."},{"line_number":4017,"context_line":"                instance.task_state \u003d task_states.RESIZE_REVERTING"},{"line_number":4018,"context_line":"                instance.save()"},{"line_number":4019,"context_line":"                migration.status \u003d \u0027reverting\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"7f287b81_b8eefd85","line":4016,"in_reply_to":"7f287b81_324c6cc6","updated":"2017-08-23 09:23:10.000000000","message":"Hmm. The first version of this patch actually called API.revert_resize(), but I ditched that because it was getting convoluted to:\n\n1) Not do the old quota stuff (now removed)\n2) Not do a cast back to ourselves.\n\nI might reexamine that.","commit_id":"a3362a66610012af9ac2e7e5cac25c4d5ab25ef1"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"50f948fef4c8ca08c65ab4b034fef290f1ca2a74","unresolved":false,"context_lines":[{"line_number":4019,"context_line":"                migration.status \u003d \u0027reverting\u0027"},{"line_number":4020,"context_line":"                migration.save()"},{"line_number":4021,"context_line":""},{"line_number":4022,"context_line":"                self.revert_resize(context, instance, migration, [])"},{"line_number":4023,"context_line":""},{"line_number":4024,"context_line":"        self._update_scheduler_instance_info(context, instance)"},{"line_number":4025,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":8,"id":"7f287b81_52e498ac","line":4022,"range":{"start_line":4022,"start_character":15,"end_line":4022,"end_character":68},"updated":"2017-08-23 07:59:59.000000000","message":"I think this is a new action we provided which means a reno is good to provide those info to user","commit_id":"a3362a66610012af9ac2e7e5cac25c4d5ab25ef1"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"d07f21c640685154e3dd3f4a9e2a7d9b2e284fde","unresolved":false,"context_lines":[{"line_number":4019,"context_line":"                migration.status \u003d \u0027reverting\u0027"},{"line_number":4020,"context_line":"                migration.save()"},{"line_number":4021,"context_line":""},{"line_number":4022,"context_line":"                self.revert_resize(context, instance, migration, [])"},{"line_number":4023,"context_line":""},{"line_number":4024,"context_line":"        self._update_scheduler_instance_info(context, instance)"},{"line_number":4025,"context_line":"        self._notify_about_instance_usage("}],"source_content_type":"text/x-python","patch_set":8,"id":"7f287b81_d898a96b","line":4022,"range":{"start_line":4022,"start_character":15,"end_line":4022,"end_character":68},"in_reply_to":"7f287b81_52e498ac","updated":"2017-08-23 09:23:10.000000000","message":"Yep, good point.","commit_id":"a3362a66610012af9ac2e7e5cac25c4d5ab25ef1"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"22b88dfdce4a974bf94e473f1b8236404ebf7032","unresolved":false,"context_lines":[{"line_number":4444,"context_line":"            network_info \u003d do_finish_resize(self, context, instance, bdms)"},{"line_number":4445,"context_line":"        except Exception:"},{"line_number":4446,"context_line":"            LOG.exception(\u0027Migration failed on the destination host. \u0027"},{"line_number":4447,"context_line":"                          \u0027Starting automatic revert.\u0027, instance\u003dinstance)"},{"line_number":4448,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4449,"context_line":"                self.compute_api.revert_resize_on_error(context, instance,"},{"line_number":4450,"context_line":"                                                        migration)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_d42ed090","line":4447,"updated":"2018-01-31 16:50:33.000000000","message":"..or the instance was running fine and we choked on something mundane.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6729acaf72c733fda8b83aa4651139b7bcff0359","unresolved":false,"context_lines":[{"line_number":4444,"context_line":"            network_info \u003d do_finish_resize(self, context, instance, bdms)"},{"line_number":4445,"context_line":"        except Exception:"},{"line_number":4446,"context_line":"            LOG.exception(\u0027Migration failed on the destination host. \u0027"},{"line_number":4447,"context_line":"                          \u0027Starting automatic revert.\u0027, instance\u003dinstance)"},{"line_number":4448,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":4449,"context_line":"                self.compute_api.revert_resize_on_error(context, instance,"},{"line_number":4450,"context_line":"                                                        migration)"}],"source_content_type":"text/x-python","patch_set":12,"id":"5f93b717_e8044c1a","line":4447,"in_reply_to":"5f93b717_d42ed090","updated":"2018-01-31 18:28:32.000000000","message":"There was a patch series leading up to this change which has long since merged. The purpose of those patches was to allow the mundane stuff to be moved out of this block. Notice how the @wrap_exception and @wrap_instance_fault are also now limited in scope.\n\nWith the exception of updating the migration and instance records in the db, any failure of do_finish_resize means we\u0027re not running correctly.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"}],"releasenotes/notes/resize-auto-revert-6e1648828aba16b2.yaml":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"22b88dfdce4a974bf94e473f1b8236404ebf7032","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - If a resize or cold migration operation fails after an"},{"line_number":4,"context_line":"    instance has moved to the destination compute, Nova will"},{"line_number":5,"context_line":"    now automatically start a revert."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"5f93b717_349934c3","line":5,"updated":"2018-01-31 16:50:33.000000000","message":"This is an API behavior change right?\n\nConsider the cloud has resize_confirm_window\u003d1 and my script\u0027s logic is that I wait for the instance to enter ACTIVE or ERROR state. On ACTIVE, I assume things worked, and on ERROR I wake up a human. Now, I\u0027ll never (reliably) get ERROR state in my polling and all the humans will remain asleep. I have to change my script to poll for whether or not the flavor looks different now instead of an indication that the thing failed.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b612389f0667f341195e7f4ef768713fcc7d321c","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - If a resize or cold migration operation fails after an"},{"line_number":4,"context_line":"    instance has moved to the destination compute, Nova will"},{"line_number":5,"context_line":"    now automatically start a revert."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"5f93b717_7e5c76ec","line":5,"in_reply_to":"5f93b717_0b02d20b","updated":"2018-01-31 18:38:18.000000000","message":"My argument would be to not auto-revert, independent of the behavior change decision. If we\u0027re going to do it, it seems like it\u0027s reasonably surprising to people.\n\nI agree that a 500 would be fixed without a microversion, but not in a way that makes some other big change, but in just putting the instance into ERROR state or something like that.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"6729acaf72c733fda8b83aa4651139b7bcff0359","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - If a resize or cold migration operation fails after an"},{"line_number":4,"context_line":"    instance has moved to the destination compute, Nova will"},{"line_number":5,"context_line":"    now automatically start a revert."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"5f93b717_0b02d20b","line":5,"in_reply_to":"5f93b717_349934c3","updated":"2018-01-31 18:28:32.000000000","message":"It\u0027s a behaviour change, sure, which is why I called it out in a release note. Whether it\u0027s a change in the defined API is arguable, though. In general, things which result in errors other than parameter or state validation type errors are a grey area for the API, and we do change that from time to time without adding microversions. If this error was synchronous it would be a 5XX error, and in general we fix those.\n\nI agree it\u0027s at least arguable, though. If we thought that the error behaviour was sufficiently well defined in the API to be maintained, and that in practice somebody might be relying on it, we\u0027d need to add a microversion to resize. We\u0027d also have to bump some RPC calls as it would have to propagate api-\u003econductor-\u003ecompute. It would also make this impossible to backport.\n\nIf the consensus is that this really is an API change I\u0027ll add the above, but personally I\u0027m not convinced it\u0027s worth it.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"52599964a2f7a83802bbd140e50a4d08bad1d550","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - If a resize or cold migration operation fails after an"},{"line_number":4,"context_line":"    instance has moved to the destination compute, Nova will"},{"line_number":5,"context_line":"    now automatically start a revert."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"9fb8cfa7_35de373e","line":5,"in_reply_to":"5f93b717_7e5c76ec","updated":"2019-06-26 16:52:59.000000000","message":"I looked at this old review today because it came up during an internal call, and on the surface, it reminded me of different review we recently merged around cold migrate to self failing [before it starts] and setting the server state to ACTIVE/STOPPED instead of ERROR:\n\nhttps://review.opendev.org/633227\n\nAfter reading the comments on this old review, I wondered if the recently merged change was in the same vein. We discussed it in IRC:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-06-26.log.html#t2019-06-26T16:17:02\n\nand the conclusion was that the recently merged change does not break automated script\u0027s ability to carry out a cold migrate wrt server state observation. Whereas this old review would have, because it would have auto-reverted a server that already went through the RESIZING state and so a transition like: ACTIVE -\u003e RESIZING -\u003e ACTIVE would no longer be distinguishable as success/failure to migrate/resize.","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6a317474992891f29aa4ddd5bda25035131c476a","unresolved":false,"context_lines":[{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - If a resize or cold migration operation fails after an"},{"line_number":4,"context_line":"    instance has moved to the destination compute, Nova will"},{"line_number":5,"context_line":"    now automatically start a revert."}],"source_content_type":"text/x-yaml","patch_set":12,"id":"3fa7e38b_f96d3fcc","line":5,"in_reply_to":"9fb8cfa7_35de373e","updated":"2019-11-13 22:45:33.000000000","message":"In discussing trying to recover from a failure in finish_resize today with melwitt [1] I suggested one option, which is part of this patch but I wouldn\u0027t do the auto-rollback thing, is allow revert_resize in the API to work on instances in ERROR status but only if we can detect in the API that the instance was undergoing a resize when it failed, i.e. the old_vm_state/old_flavor/new_flavor/migration_context etc are on the instance still. It wouldn\u0027t be automatic. The admin/user would have to do it manually. It might not work, I don\u0027t trust that the revert flow is totally idempotent depending on how finish_resize fails (setting up networks/volumes back to the source could be janky, you\u0027d have to test it). Anyway, it\u0027s an idea. It would likely be a microversion as well since it\u0027s a change in API behavior (though maybe that\u0027s debatable if it\u0027s a bug fix of sorts).\n\n[1] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-11-13.log.html#t2019-11-13T21:49:12","commit_id":"d1ee02700c10ddefdf17b0ef9138c20cf5730efd"}]}