)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d59b1f865cdd1dbeb324d9c0a77448f95d81032c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Lee Yarwood \u003clyarwood@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-02-12 10:18:54 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"compute: Reject migration requests when source is down"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Attempts to migrate an instance from a host where the compute service is"},{"line_number":10,"context_line":"down or unresponsive cannot obviously be completed until the service is"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9fdfeff1_fa58ab8a","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":54},"updated":"2019-02-12 17:32:26.000000000","message":"note: this is useing the service state field not status field\nso it only take effect if the service is down due to missing a heartbeat and will still allow migration if the status\nhas been set to disabled. as such it will still allow admins to make host as disabled so new instance cant land there and then migrate off existing instnaces. \n\nit also ingores the force_down flag so if the service is marked as down by an external party we will still allow the migration off the host.","commit_id":"cc15d0b327f16c6f9af64c43a5834c4a19de98e8"}],"nova/compute/api.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"10ac17163792957ef098c5fdadb6e51956a3e9fd","unresolved":false,"context_lines":[{"line_number":3496,"context_line":"        host_name is always None in the resize case."},{"line_number":3497,"context_line":"        host_name can be set in the cold migration case only."},{"line_number":3498,"context_line":"        \"\"\""},{"line_number":3499,"context_line":"        # NOTE(lyarwood): As most virt backends can\u0027t resize or migrate if the"},{"line_number":3500,"context_line":"        # source compute service is actually down we need to check and reject"},{"line_number":3501,"context_line":"        # such requests early and in a meaningful way to the user."},{"line_number":3502,"context_line":"        if not self._is_compute_service_up(context, instance.host,"},{"line_number":3503,"context_line":"                                           ignore_forced_down\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_365a4c50","line":3500,"range":{"start_line":3499,"start_character":26,"end_line":3500,"end_character":49},"updated":"2019-02-19 16:17:08.000000000","message":"Nit: *No* virt backend can do this, because if the compute host is down we\u0027ll fail before executing any virt driver code.","commit_id":"328338c438cb4384485c482bce7baec437183df7"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"60979885e3b6db3e08a4c238a338342a5f6e22b0","unresolved":false,"context_lines":[{"line_number":3496,"context_line":"        host_name is always None in the resize case."},{"line_number":3497,"context_line":"        host_name can be set in the cold migration case only."},{"line_number":3498,"context_line":"        \"\"\""},{"line_number":3499,"context_line":"        # NOTE(lyarwood): As most virt backends can\u0027t resize or migrate if the"},{"line_number":3500,"context_line":"        # source compute service is actually down we need to check and reject"},{"line_number":3501,"context_line":"        # such requests early and in a meaningful way to the user."},{"line_number":3502,"context_line":"        if not self._is_compute_service_up(context, instance.host,"},{"line_number":3503,"context_line":"                                           ignore_forced_down\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_7ef0a6fe","line":3500,"range":{"start_line":3499,"start_character":26,"end_line":3500,"end_character":49},"in_reply_to":"9fdfeff1_365a4c50","updated":"2019-02-19 16:32:47.000000000","message":"I wasn\u0027t sure if the VMWare or Hyper-V backends had multiple n-cpu services for the same cluster so went with most. Happy to reword if they would also fail.","commit_id":"328338c438cb4384485c482bce7baec437183df7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"90bf7d6a76368820d387aea31b00f0aa0c9d0f11","unresolved":false,"context_lines":[{"line_number":3496,"context_line":"        host_name is always None in the resize case."},{"line_number":3497,"context_line":"        host_name can be set in the cold migration case only."},{"line_number":3498,"context_line":"        \"\"\""},{"line_number":3499,"context_line":"        # NOTE(lyarwood): As most virt backends can\u0027t resize or migrate if the"},{"line_number":3500,"context_line":"        # source compute service is actually down we need to check and reject"},{"line_number":3501,"context_line":"        # such requests early and in a meaningful way to the user."},{"line_number":3502,"context_line":"        if not self._is_compute_service_up(context, instance.host,"},{"line_number":3503,"context_line":"                                           ignore_forced_down\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_d9a3ecb0","line":3500,"range":{"start_line":3499,"start_character":26,"end_line":3500,"end_character":49},"in_reply_to":"9fdfeff1_7ef0a6fe","updated":"2019-02-19 17:16:36.000000000","message":"No. It used to be the other way round, though: 1 compute could handle multiple clusters. I think we ditched that, although I wouldn\u0027t swear to it. If the compute\u0027s down, though, we don\u0027t get as far as the driver code.","commit_id":"328338c438cb4384485c482bce7baec437183df7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"10ac17163792957ef098c5fdadb6e51956a3e9fd","unresolved":false,"context_lines":[{"line_number":3500,"context_line":"        # source compute service is actually down we need to check and reject"},{"line_number":3501,"context_line":"        # such requests early and in a meaningful way to the user."},{"line_number":3502,"context_line":"        if not self._is_compute_service_up(context, instance.host,"},{"line_number":3503,"context_line":"                                           ignore_forced_down\u003dTrue):"},{"line_number":3504,"context_line":"            LOG.info(\"Unable to resize instance as host %s is currently down\","},{"line_number":3505,"context_line":"                     instance.host, instance\u003dinstance)"},{"line_number":3506,"context_line":"            raise exception.SourceComputeHostDown()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_c3b53fd2","line":3503,"range":{"start_line":3503,"start_character":43,"end_line":3503,"end_character":66},"updated":"2019-02-19 16:17:08.000000000","message":"Why are we ignoring forced_down here? If we\u0027re going to ignore forced_down I think we need:\n\n1. A comment explaining why, given that we don\u0027t ignore it anywhere else afaict.\n2. A functional test of some sort to ensure that there isn\u0027t already anything else which would cause a failure if the source compute is down.\n\nHowever, reading the original spec I don\u0027t think we should ignore it:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/liberty/implemented/mark-host-down.html\n\nIIUC, forced_down was added so that an HA system doesn\u0027t have to wait for heartbeat expiry after it fenced the host. IOW, forced_down means it\u0027s really down, but heartbeat may not have noticed yet.","commit_id":"328338c438cb4384485c482bce7baec437183df7"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"90bf7d6a76368820d387aea31b00f0aa0c9d0f11","unresolved":false,"context_lines":[{"line_number":3500,"context_line":"        # source compute service is actually down we need to check and reject"},{"line_number":3501,"context_line":"        # such requests early and in a meaningful way to the user."},{"line_number":3502,"context_line":"        if not self._is_compute_service_up(context, instance.host,"},{"line_number":3503,"context_line":"                                           ignore_forced_down\u003dTrue):"},{"line_number":3504,"context_line":"            LOG.info(\"Unable to resize instance as host %s is currently down\","},{"line_number":3505,"context_line":"                     instance.host, instance\u003dinstance)"},{"line_number":3506,"context_line":"            raise exception.SourceComputeHostDown()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_99d324d6","line":3503,"range":{"start_line":3503,"start_character":43,"end_line":3503,"end_character":66},"in_reply_to":"9fdfeff1_7efd066d","updated":"2019-02-19 17:16:36.000000000","message":"\u003e \u003e Why are we ignoring forced_down here? If we\u0027re going to ignore\n \u003e \u003e forced_down I think we need:\n \u003e \u003e\n \u003e \u003e 1. A comment explaining why, given that we don\u0027t ignore it\n \u003e anywhere\n \u003e \u003e else afaict.\n \u003e \n \u003e ACK\n \u003e \n \u003e \u003e 2. A functional test of some sort to ensure that there isn\u0027t\n \u003e \u003e already anything else which would cause a failure if the source\n \u003e \u003e compute is down.\n \u003e \n \u003e I\u0027m not entirely sure what you\u0027re asking for here. There\u0027s a\n \u003e plethora of functional tests that would already exercise this with\n \u003e the default of False. Do you specifically want one with it set to\n \u003e True?\n\nI don\u0027t think this is a thing, because I think we should just remove it (see below), but assuming we don\u0027t remove it: this check assumes that resize currently works when the compute host is forced_down, but does it? If there\u0027s anything in the call chain we haven\u0027t thought of, or which anybody adds in the future, which checks service status before calling resize then this would be invalid. I think you\u0027d need a functional test for something like that.\n\n...or just delete it, which I think is the correct option anyway.\n\n \u003e \n \u003e \u003e However, reading the original spec I don\u0027t think we should ignore\n \u003e \u003e it:\n \u003e \u003e\n \u003e \u003e https://specs.openstack.org/openstack/nova-specs/specs/liberty/implemented/mark-host-down.html\n \u003e \u003e\n \u003e \u003e IIUC, forced_down was added so that an HA system doesn\u0027t have to\n \u003e \u003e wait for heartbeat expiry after it fenced the host. IOW,\n \u003e \u003e forced_down means it\u0027s really down, but heartbeat may not have\n \u003e \u003e noticed yet.\n \u003e \n \u003e My understanding was that the initial use case was just a shortcut\n \u003e to allow for quick evacuations, putting the onus on the operator to\n \u003e fence the host. I can\u0027t recall where this came up but I\u0027m sure I\n \u003e was told that since then operators have also started live migrating\n \u003e from forced_down hosts where n-cpu remains up and that\u0027s the use\n \u003e case I wanted to allow by ignoring the flag here.\n \u003e \n \u003e Happy to drop this if it\u0027s controversial.\n\nI\u0027d be interested to see that discussion, but based on the original spec that would be an incorrect use of forced_down. We don\u0027t permit evacuate from a compute host which isn\u0027t down because the VMs might still be running. If you call forced_down you\u0027re asserting that they\u0027re definitely not running, so how can you live migrate?\n\nI think it\u0027s simpler all round not to ignore it.","commit_id":"328338c438cb4384485c482bce7baec437183df7"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"60979885e3b6db3e08a4c238a338342a5f6e22b0","unresolved":false,"context_lines":[{"line_number":3500,"context_line":"        # source compute service is actually down we need to check and reject"},{"line_number":3501,"context_line":"        # such requests early and in a meaningful way to the user."},{"line_number":3502,"context_line":"        if not self._is_compute_service_up(context, instance.host,"},{"line_number":3503,"context_line":"                                           ignore_forced_down\u003dTrue):"},{"line_number":3504,"context_line":"            LOG.info(\"Unable to resize instance as host %s is currently down\","},{"line_number":3505,"context_line":"                     instance.host, instance\u003dinstance)"},{"line_number":3506,"context_line":"            raise exception.SourceComputeHostDown()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_7efd066d","line":3503,"range":{"start_line":3503,"start_character":43,"end_line":3503,"end_character":66},"in_reply_to":"9fdfeff1_c3b53fd2","updated":"2019-02-19 16:32:47.000000000","message":"\u003e Why are we ignoring forced_down here? If we\u0027re going to ignore\n \u003e forced_down I think we need:\n \u003e \n \u003e 1. A comment explaining why, given that we don\u0027t ignore it anywhere\n \u003e else afaict.\n\nACK\n\n \u003e 2. A functional test of some sort to ensure that there isn\u0027t\n \u003e already anything else which would cause a failure if the source\n \u003e compute is down.\n\nI\u0027m not entirely sure what you\u0027re asking for here. There\u0027s a plethora of functional tests that would already exercise this with the default of False. Do you specifically want one with it set to True? \n\n \u003e However, reading the original spec I don\u0027t think we should ignore\n \u003e it:\n \u003e \n \u003e https://specs.openstack.org/openstack/nova-specs/specs/liberty/implemented/mark-host-down.html\n \u003e \n \u003e IIUC, forced_down was added so that an HA system doesn\u0027t have to\n \u003e wait for heartbeat expiry after it fenced the host. IOW,\n \u003e forced_down means it\u0027s really down, but heartbeat may not have\n \u003e noticed yet.\n\nMy understanding was that the initial use case was just a shortcut to allow for quick evacuations, putting the onus on the operator to fence the host. I can\u0027t recall where this came up but I\u0027m sure I was told that since then operators have also started live migrating from forced_down hosts where n-cpu remains up and that\u0027s the use case I wanted to allow by ignoring the flag here.\n\nHappy to drop this if it\u0027s controversial.","commit_id":"328338c438cb4384485c482bce7baec437183df7"}],"nova/exception.py":[{"author":{"_account_id":15888,"name":"Zhenyu Zheng","email":"zheng.zhenyu@outlook.com","username":"Kevin_Zheng"},"change_message_id":"5d630d98f49c4bdf014d22ad8af157b8ecaf2e12","unresolved":false,"context_lines":[{"line_number":1072,"context_line":""},{"line_number":1073,"context_line":""},{"line_number":1074,"context_line":"class SourceComputeHostDown(Invalid):"},{"line_number":1075,"context_line":"    msg_fmt \u003d _(\"Source compute host down.\")"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":""},{"line_number":1078,"context_line":"class InvalidReservationExpiration(Invalid):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_d170b70b","line":1075,"range":{"start_line":1075,"start_character":16,"end_line":1075,"end_character":43},"updated":"2019-02-19 01:53:39.000000000","message":"This might still be too much underlayer information for normal users, how about something like \"Source compute host currently unable to perform the requested action\" and we add LOG in compute/api.py for admins to check","commit_id":"fe9ce421e38d6a323bbf05673ca5c560ebf1c3d1"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"db311ae2c376099ec19702e4e22c480ddb03eaf3","unresolved":false,"context_lines":[{"line_number":1072,"context_line":""},{"line_number":1073,"context_line":""},{"line_number":1074,"context_line":"class SourceComputeHostDown(Invalid):"},{"line_number":1075,"context_line":"    msg_fmt \u003d _(\"Source compute host down.\")"},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":""},{"line_number":1078,"context_line":"class InvalidReservationExpiration(Invalid):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_a7fcd00c","line":1075,"range":{"start_line":1075,"start_character":16,"end_line":1075,"end_character":43},"in_reply_to":"9fdfeff1_d170b70b","updated":"2019-02-19 09:28:44.000000000","message":"Yup that\u0027s a fair suggestion, I\u0027ll respin now and add the INFO log.","commit_id":"fe9ce421e38d6a323bbf05673ca5c560ebf1c3d1"}],"nova/tests/unit/api/openstack/compute/test_migrate_server.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"8467ed8ac3047644ec9b83929fbadd97aaedfed5","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        self._test_migrate_exception(exc_info, webob.exc.HTTPForbidden)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    def test_migrate_src_compute_disabled(self):"},{"line_number":144,"context_line":"        exec_info \u003d exception.SourceComputeHostDown(host\u003d\u0027fake\u0027)"},{"line_number":145,"context_line":"        self._test_migrate_exception(exec_info, webob.exc.HTTPConflict)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    def _test_migrate_live_succeeded(self, param):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_9e11ca07","line":144,"range":{"start_line":144,"start_character":8,"end_line":144,"end_character":17},"updated":"2019-02-19 16:58:37.000000000","message":"nit: It\u0027s exc_info everywhere else.","commit_id":"328338c438cb4384485c482bce7baec437183df7"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"8467ed8ac3047644ec9b83929fbadd97aaedfed5","unresolved":false,"context_lines":[{"line_number":2110,"context_line":""},{"line_number":2111,"context_line":"            if self.cell_type \u003d\u003d \u0027api\u0027 and request_spec:"},{"line_number":2112,"context_line":"                mock_migration.assert_called_once_with(context\u003dself.context)"},{"line_number":2113,"context_line":"                mock_elevated.assert_has_calls([mock.call(), mock.call()])"},{"line_number":2114,"context_line":"                mig.create.assert_called_once_with()"},{"line_number":2115,"context_line":"            else:"},{"line_number":2116,"context_line":"                mock_migration.assert_not_called()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_1e295a9a","line":2113,"updated":"2019-02-19 16:58:37.000000000","message":"I know you\u0027re just following the existing anti-pattern, but I think we\u0027re better off putting this test out of its misery rather than adding to it: it just creates noise in changes like this one without asserting anything we care about. You could delete it and replace it with a useful test.\n\nFor example, a much better test would be to ensure that whatever is supposed to use the elevated context actually does. For e.g.:\n\nmock_get_service.assert_called_once_with(mock_elevated.return_value, \u0027fake_host (or whatever)\u0027)\n\n... which incidentally you don\u0027t appear to be doing anywhere. If you really don\u0027t want to bother with that detail everywhere you could mock _is_compute_service_up instead and add a unit test for it for the detail. Either would be fine, imho.","commit_id":"328338c438cb4384485c482bce7baec437183df7"}],"nova/tests/unit/servicegroup/test_api.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"8467ed8ac3047644ec9b83929fbadd97aaedfed5","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        driver.is_up.return_value \u003d True"},{"line_number":65,"context_line":"        result \u003d self.servicegroup_api.service_is_up(member,"},{"line_number":66,"context_line":"                                        ignore_forced_down\u003dTrue)"},{"line_number":67,"context_line":"        self.assertIs(result, True)"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def test_get_updated_time(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_f918301f","line":66,"updated":"2019-02-19 16:58:37.000000000","message":"Nit: this is probably a separate test.\n\nNot a nit: Don\u0027t think we should ignore this anyway, in which case you don\u0027t need this.","commit_id":"328338c438cb4384485c482bce7baec437183df7"}],"releasenotes/notes/bug-1807434-f33d563e2f59b03b.yaml":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"10ac17163792957ef098c5fdadb6e51956a3e9fd","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    instance being accepted even when the source compute service was down or"},{"line_number":6,"context_line":"    unresponsive. These requests would not be acted on and could later result"},{"line_number":7,"context_line":"    in instances entering an ERROR state if the host ever returned. To avoid"},{"line_number":8,"context_line":"    this the API now returns a 409 ``HTTPConflict`` to the caller to indicate"},{"line_number":9,"context_line":"    that there is an issue with the underlying state of the environment leaving"},{"line_number":10,"context_line":"    the instance in its original state. This potentially allows users to retry"},{"line_number":11,"context_line":"    the resize or migration at a later time or alternatively use evacuation to"}],"source_content_type":"text/x-yaml","patch_set":9,"id":"9fdfeff1_43cdcf84","line":8,"range":{"start_line":8,"start_character":31,"end_line":8,"end_character":51},"updated":"2019-02-19 16:17:08.000000000","message":"Just restating my earlier point that it seems weird to me to consider the nova compute service itself as a resource from the pov of the rest api. This feels like a 503 to me, as the service is temporarily unavailable and there\u0027s nothing the user can do about it.\n\nHowever, I concede that you had that discussion already and this is how it turned out :)","commit_id":"328338c438cb4384485c482bce7baec437183df7"}]}