)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"fa4d038e97cec7592c431409654f1e9066438576","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dan Smith \u003cdansmith@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-11-17 13:09:58 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"WIP: Alternate fix for validating image on rebuild"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: I1a46ef1503be2febcd20f4594f44344d05525446"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"1f485f77_90b49081","line":7,"range":{"start_line":7,"start_character":5,"end_line":7,"end_character":14},"updated":"2017-11-17 21:39:03.000000000","message":"Oh, and it\u0027s not really alternate, is it? You\u0027re building on top of the previous fix.","commit_id":"57b30e0c783def785e2c539d3d56afc72f8c1bcb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Alternate fix for validating image on rebuild"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This aims to fix the issue described in bug 1664931 where a rebuild"},{"line_number":10,"context_line":"fails to validate the existing host with the scheduler when a new"},{"line_number":11,"context_line":"image is provided. The previous attempt to do this could cause rebuilds"},{"line_number":12,"context_line":"to fail unnecessarily because we ran _all_ of the filters during a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f485f77_73c91e57","line":9,"updated":"2017-11-17 22:33:51.000000000","message":"I think we\u0027d want some kind of release note that explains this fixes the same bug but requires that the SameHostFilter be enabled, which it\u0027s not by default, and that out-of-tree filters might be affected by the solution.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9754b3288025824adfc444a6e9677733eafa0866","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This aims to classify filters as either policy-only or not, and only apply"},{"line_number":17,"context_line":"the former during a rebuild scheduler check. We do this by using the"},{"line_number":18,"context_line":"same_host scheduler hint, set to our own uuid. This should (a) filter out"},{"line_number":19,"context_line":"all hosts other than the one we\u0027re running on and (b) be detectable by"},{"line_number":20,"context_line":"the filtering infrastructure as an internally-generated scheduling request"},{"line_number":21,"context_line":"in order to trigger the policy-only filtering behavior."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ff82abbf_21518fd9","line":18,"range":{"start_line":18,"start_character":0,"end_line":18,"end_character":9},"updated":"2017-11-21 01:57:24.000000000","message":"not anymore","commit_id":"71896898ae33290a536e992fa1db7fab1a0600ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33888d8a27c624a152c27594a2ca5a934b9b5687","unresolved":false,"context_lines":[{"line_number":13,"context_line":"rebuild, which could cause usage/resource filters to prevent an otherwise"},{"line_number":14,"context_line":"valid rebuild from succeeding."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This aims to classify filters as either policy-only or not, and only apply"},{"line_number":17,"context_line":"the former during a rebuild scheduler check. We do this by using an internal"},{"line_number":18,"context_line":"scheduler hint, indicating a policy check. This should (a) filter out"},{"line_number":19,"context_line":"all hosts other than the one we\u0027re running on and (b) be detectable by"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"ff82abbf_76c98376","line":16,"updated":"2017-11-21 20:35:51.000000000","message":"The \"policy-only\" wording in here could probably be updated.","commit_id":"5f094f93d4e99fc002869e792d6991c21dfd82ff"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1274e4aea795c764818f55eb1241b59260d2667f","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dan Smith \u003cdansmith@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-11-21 12:41:02 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Alternate fix for validating image on rebuild"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This aims to fix the issue described in bug 1664931 where a rebuild"},{"line_number":10,"context_line":"fails to validate the existing host with the scheduler when a new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"ff82abbf_4965c0b4","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":9},"updated":"2017-11-21 21:01:56.000000000","message":"It\u0027s not really alternate, we\u0027re building on top of the previous fix.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"65f220bf14553e56276296a3a3cd30e66e62ffcf","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dan Smith \u003cdansmith@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-11-21 12:41:02 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Alternate fix for validating image on rebuild"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This aims to fix the issue described in bug 1664931 where a rebuild"},{"line_number":10,"context_line":"fails to validate the existing host with the scheduler when a new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"ff82abbf_490b808d","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":9},"in_reply_to":"ff82abbf_29d39c3a","updated":"2017-11-21 21:11:46.000000000","message":"It won\u0027t be a clean revert if we merge this first... Compare the original fix [1] and what we\u0027re doing here [2]\n\n[1] https://review.openstack.org/#/c/519672/1/nova/compute/api.py\n[2] https://review.openstack.org/#/c/521186/7/nova/compute/api.py","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"71e4f4ef557c8e4e7f2b12b49e001d43b0d8e1ca","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dan Smith \u003cdansmith@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-11-21 12:41:02 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Alternate fix for validating image on rebuild"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This aims to fix the issue described in bug 1664931 where a rebuild"},{"line_number":10,"context_line":"fails to validate the existing host with the scheduler when a new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"ff82abbf_6948c4cb","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":9},"in_reply_to":"ff82abbf_490b808d","updated":"2017-11-21 21:14:27.000000000","message":"Right, I said we _considered_ reverting it entirely. But this effectively un-does the critical part of the original patch, which was to request a specific destination, bypassing all the filters.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"9610a4ab214eeb5896b653da3eafee06210d98b4","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dan Smith \u003cdansmith@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-11-21 12:41:02 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Alternate fix for validating image on rebuild"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This aims to fix the issue described in bug 1664931 where a rebuild"},{"line_number":10,"context_line":"fails to validate the existing host with the scheduler when a new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"ff82abbf_29d39c3a","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":9},"in_reply_to":"ff82abbf_4965c0b4","updated":"2017-11-21 21:04:09.000000000","message":"It\u0027s replacing the previous fix entirely. We considered reverting it.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"24bebff8026ffefe44d91b138b2f27271f10be53","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Dan Smith \u003cdansmith@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-11-27 07:24:43 -0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refined fix for validating image on rebuild"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This aims to fix the issue described in bug 1664931 where a rebuild"},{"line_number":10,"context_line":"fails to validate the existing host with the scheduler when a new"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ff82abbf_1a80e6be","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":7},"updated":"2017-11-27 20:32:27.000000000","message":"Cheers! ;)","commit_id":"cdc045f84a722c2fccdd9df3645b95f64f491ebc"}],"nova/compute/api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33ec2a73378e5dac977f2c6432e9af8621d7f669","unresolved":false,"context_lines":[{"line_number":2958,"context_line":"                host \u003d None     # This tells conductor to call the scheduler."},{"line_number":2959,"context_line":"                if \u0027scheduler_hints\u0027 not in request_spec:"},{"line_number":2960,"context_line":"                    request_spec[\u0027scheduler_hints\u0027] \u003d {}"},{"line_number":2961,"context_line":"                request_spec.scheduler_hints[\u0027same_host\u0027] \u003d [instance.uuid]"},{"line_number":2962,"context_line":"        except exception.RequestSpecNotFound:"},{"line_number":2963,"context_line":"            # Some old instances can still have no RequestSpec object attached"},{"line_number":2964,"context_line":"            # to them, we need to support the old way"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_2d4227ac","line":2961,"updated":"2017-11-17 20:39:31.000000000","message":"We\u0027ll have to be careful that this request_spec doesn\u0027t get saved after this point - should we call request_spec.obj_reset_changes([\u0027scheduler_hints\u0027]) here?","commit_id":"298e57e0bf90c155c9f3684086e88795a1d86578"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1d6dc6c5595e588fceb7e9c04120bb9614c8316d","unresolved":false,"context_lines":[{"line_number":2958,"context_line":"                host \u003d None     # This tells conductor to call the scheduler."},{"line_number":2959,"context_line":"                if \u0027scheduler_hints\u0027 not in request_spec:"},{"line_number":2960,"context_line":"                    request_spec[\u0027scheduler_hints\u0027] \u003d {}"},{"line_number":2961,"context_line":"                request_spec.scheduler_hints[\u0027same_host\u0027] \u003d [instance.uuid]"},{"line_number":2962,"context_line":"        except exception.RequestSpecNotFound:"},{"line_number":2963,"context_line":"            # Some old instances can still have no RequestSpec object attached"},{"line_number":2964,"context_line":"            # to them, we need to support the old way"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_4d91e3f0","line":2961,"in_reply_to":"1f485f77_2d4227ac","updated":"2017-11-17 20:43:06.000000000","message":"or obj_clone() it, sure.","commit_id":"298e57e0bf90c155c9f3684086e88795a1d86578"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b7626c04fa771b72f7ddabf4da0c6327f08e566b","unresolved":false,"context_lines":[{"line_number":2961,"context_line":"                    # Nuke the id on this so we can\u0027t accidentally save"},{"line_number":2962,"context_line":"                    # this hint hack later"},{"line_number":2963,"context_line":"                    del request_spec.id"},{"line_number":2964,"context_line":"                request_spec.scheduler_hints[\u0027same_host\u0027] \u003d [instance.uuid]"},{"line_number":2965,"context_line":"        except exception.RequestSpecNotFound:"},{"line_number":2966,"context_line":"            # Some old instances can still have no RequestSpec object attached"},{"line_number":2967,"context_line":"            # to them, we need to support the old way"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f485f77_706a341a","line":2964,"updated":"2017-11-17 21:35:22.000000000","message":"Maybe put a comment here that this essentially \"sets\" _request_is_policy in all filters. I had trouble getting the logic right, but I\u0027m probably just slow.","commit_id":"57b30e0c783def785e2c539d3d56afc72f8c1bcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5ad4e39f1845a7419f183724f4a7f6455e06619","unresolved":false,"context_lines":[{"line_number":2961,"context_line":"                    # Nuke the id on this so we can\u0027t accidentally save"},{"line_number":2962,"context_line":"                    # this hint hack later"},{"line_number":2963,"context_line":"                    del request_spec.id"},{"line_number":2964,"context_line":"                request_spec.scheduler_hints[\u0027same_host\u0027] \u003d [instance.uuid]"},{"line_number":2965,"context_line":"        except exception.RequestSpecNotFound:"},{"line_number":2966,"context_line":"            # Some old instances can still have no RequestSpec object attached"},{"line_number":2967,"context_line":"            # to them, we need to support the old way"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f485f77_d0b6888a","line":2964,"in_reply_to":"1f485f77_706a341a","updated":"2017-11-17 21:40:06.000000000","message":"Ack.","commit_id":"57b30e0c783def785e2c539d3d56afc72f8c1bcb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e01caebead361aa797ca42b6bac2d4c2db931d5d","unresolved":false,"context_lines":[{"line_number":2966,"context_line":"                # requires that the scheduler returns only the same"},{"line_number":2967,"context_line":"                # host that we are currently on and only checks policy"},{"line_number":2968,"context_line":"                # filters."},{"line_number":2969,"context_line":"                request_spec.scheduler_hints[\u0027_nova_check_type\u0027] \u003d [\u0027policy\u0027]"},{"line_number":2970,"context_line":"                request_spec.force_hosts \u003d [instance.host]"},{"line_number":2971,"context_line":"                request_spec.force_nodes \u003d [instance.node]"},{"line_number":2972,"context_line":"                host \u003d None"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_4184ba5e","line":2969,"updated":"2017-11-21 18:56:21.000000000","message":"Per discussion in IRC, we could make this more straight-forward, at least for the time being, by just calling this _is_rebuild, make it a boolean, and then rename the filter attribute to RUN_FOR_REBUILD. Then we don\u0027t have to get bogged down in \"what does policy mean?\" for the various filters, and we can just scope them to the rebuild use case where the flavor doesn\u0027t change but the image can, so we need to run through any filters that care about a new image, but can skip everything else.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33888d8a27c624a152c27594a2ca5a934b9b5687","unresolved":false,"context_lines":[{"line_number":2964,"context_line":"                # NOTE(danms): Passing host\u003dNone tells conductor to"},{"line_number":2965,"context_line":"                # call the scheduler. The _nova_check_type hint"},{"line_number":2966,"context_line":"                # requires that the scheduler returns only the same"},{"line_number":2967,"context_line":"                # host that we are currently on and only checks policy"},{"line_number":2968,"context_line":"                # filters."},{"line_number":2969,"context_line":"                request_spec.scheduler_hints[\u0027_nova_check_type\u0027] \u003d [\u0027rebuild\u0027]"},{"line_number":2970,"context_line":"                request_spec.force_hosts \u003d [instance.host]"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff82abbf_36a1bbaa","line":2967,"range":{"start_line":2967,"start_character":64,"end_line":2967,"end_character":70},"updated":"2017-11-21 20:35:51.000000000","message":"questionable wording, maybe re-word to something like, \"and only checks filters which would impact rebuild.\"","commit_id":"5f094f93d4e99fc002869e792d6991c21dfd82ff"}],"nova/scheduler/filters/__init__.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33ec2a73378e5dac977f2c6432e9af8621d7f669","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            return False"},{"line_number":42,"context_line":"        if \u0027scheduler_hints\u0027 not in spec_obj:"},{"line_number":43,"context_line":"            return False"},{"line_number":44,"context_line":"        same_host \u003d spec_obj.scheduler_hints.get(\u0027same_host\u0027)"},{"line_number":45,"context_line":"        # This is a policy-only check if the hint same_host is set"},{"line_number":46,"context_line":"        # to just [self] for the instance."},{"line_number":47,"context_line":"        return same_host \u003d\u003d [spec_obj.instance_uuid]"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_2ded87b6","line":44,"updated":"2017-11-17 20:39:31.000000000","message":"We\u0027ll have to be careful here, if the instance was created with same_host originally and we\u0027re doing a move operation. However, you can\u0027t create an instance with a same_host hint to itself, so the below check should never be True in that case. And the only way to migrate to the same host is via resize, which doesn\u0027t take scheduler hints. Might be good to document those caveats here so we don\u0027t forget them.","commit_id":"298e57e0bf90c155c9f3684086e88795a1d86578"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1d6dc6c5595e588fceb7e9c04120bb9614c8316d","unresolved":false,"context_lines":[{"line_number":41,"context_line":"            return False"},{"line_number":42,"context_line":"        if \u0027scheduler_hints\u0027 not in spec_obj:"},{"line_number":43,"context_line":"            return False"},{"line_number":44,"context_line":"        same_host \u003d spec_obj.scheduler_hints.get(\u0027same_host\u0027)"},{"line_number":45,"context_line":"        # This is a policy-only check if the hint same_host is set"},{"line_number":46,"context_line":"        # to just [self] for the instance."},{"line_number":47,"context_line":"        return same_host \u003d\u003d [spec_obj.instance_uuid]"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_2da58747","line":44,"in_reply_to":"1f485f77_2ded87b6","updated":"2017-11-17 20:43:06.000000000","message":"Ack.","commit_id":"298e57e0bf90c155c9f3684086e88795a1d86578"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"b7626c04fa771b72f7ddabf4da0c6327f08e566b","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            return self.host_passes(obj, filter_properties)"},{"line_number":60,"context_line":"        else:"},{"line_number":61,"context_line":"            # If we don\u0027t filter, default to passing the host."},{"line_number":62,"context_line":"            return True"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def host_passes(self, host_state, filter_properties):"},{"line_number":65,"context_line":"        \"\"\"Return True if the HostState passes the filter, otherwise False."}],"source_content_type":"text/x-python","patch_set":2,"id":"1f485f77_70011438","line":62,"updated":"2017-11-17 21:35:22.000000000","message":"To my (admittedly tired) brain, inverting the if makes it clearer:\n\n  if request_is_policy and not self.POLICY_FILTER:\n      return True # pass the host, we\u0027re irrelevant because\n                  # request is policy and we\u0027re not\n  else:\n      self.host_passes()","commit_id":"57b30e0c783def785e2c539d3d56afc72f8c1bcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f5ad4e39f1845a7419f183724f4a7f6455e06619","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            return self.host_passes(obj, filter_properties)"},{"line_number":60,"context_line":"        else:"},{"line_number":61,"context_line":"            # If we don\u0027t filter, default to passing the host."},{"line_number":62,"context_line":"            return True"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def host_passes(self, host_state, filter_properties):"},{"line_number":65,"context_line":"        \"\"\"Return True if the HostState passes the filter, otherwise False."}],"source_content_type":"text/x-python","patch_set":2,"id":"1f485f77_f0c3c4eb","line":62,"in_reply_to":"1f485f77_70011438","updated":"2017-11-17 21:40:06.000000000","message":"I was doing it the convoluted way just to save some cycles if POLICY_FILTER\u003dTrue, as slim installs with placement would likely only have a few policy-only filters enabled. If it makes more sense to others to have it your way I\u0027ll happily change it.","commit_id":"57b30e0c783def785e2c539d3d56afc72f8c1bcb"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1bcd2f48bbd6fd33dc284503732ba23a51ebc961","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            return self.host_passes(obj, filter_properties)"},{"line_number":60,"context_line":"        else:"},{"line_number":61,"context_line":"            # If we don\u0027t filter, default to passing the host."},{"line_number":62,"context_line":"            return True"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def host_passes(self, host_state, filter_properties):"},{"line_number":65,"context_line":"        \"\"\"Return True if the HostState passes the filter, otherwise False."}],"source_content_type":"text/x-python","patch_set":2,"id":"1f485f77_b3ac369f","line":62,"in_reply_to":"1f485f77_f0362403","updated":"2017-11-17 22:06:30.000000000","message":"Heh, duh, will change.","commit_id":"57b30e0c783def785e2c539d3d56afc72f8c1bcb"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"2e880bc0b495d89582efb8679b45c5aeec09301f","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            return self.host_passes(obj, filter_properties)"},{"line_number":60,"context_line":"        else:"},{"line_number":61,"context_line":"            # If we don\u0027t filter, default to passing the host."},{"line_number":62,"context_line":"            return True"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def host_passes(self, host_state, filter_properties):"},{"line_number":65,"context_line":"        \"\"\"Return True if the HostState passes the filter, otherwise False."}],"source_content_type":"text/x-python","patch_set":2,"id":"1f485f77_f0362403","line":62,"in_reply_to":"1f485f77_f0c3c4eb","updated":"2017-11-17 21:43:23.000000000","message":"A compromise would be\n\n  if not self.POLICY_FILTER and request_is_policy","commit_id":"57b30e0c783def785e2c539d3d56afc72f8c1bcb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"26645566365b0200644097ced9a695ea98d108d3","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from nova import filters"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def _request_is_policy(spec_obj):"},{"line_number":23,"context_line":"    \"\"\"Returns True if request applies to a policy check only."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    :param spec_obj: An objects.RequestSpec to examine (or None)."}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_ccb70cf5","line":22,"updated":"2017-11-18 18:28:29.000000000","message":"I was thinking about the issue where we\u0027re double allocating for Placement on a rebuild, and I think if we moved this to a utility method (nova.scheduler.utils?) we could then re-use this in the FilterScheduler to avoid the claim if this method returns True. That seems like a pretty simple fix for that bug as well. But we might want to fix that in a separate patch? I\u0027m not really sure because if we use this one and backport it, we\u0027ll likely want those patches together because we\u0027re trying to fix (1) rebuild with new image goes through the scheduler (maintain that fix), (2) don\u0027t double allocate on rebuild and (3) don\u0027t fail the rebuild because non-policy filters fail (this patch). Maybe it\u0027s OK if they are separate - it would make backports cleaner probably.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from nova import filters"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def _request_is_policy(spec_obj):"},{"line_number":23,"context_line":"    \"\"\"Returns True if request applies to a policy check only."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    :param spec_obj: An objects.RequestSpec to examine (or None)."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_f37cefb6","line":22,"in_reply_to":"1f485f77_ccb70cf5","updated":"2017-11-20 14:49:13.000000000","message":"Yeah I dunno. I had kinda assumed they should be separate, although for old branches that have never seen the previous fix, this fix becomes the breaking one until the subsequent patch to fix the doubling. Although I guess that\u0027s only ocata (right?) so that\u0027s probably a good reason to keep them separate actually.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5bf5d79cee820903c64df83b7bf0bbf55092e769","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from nova import filters"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def _request_is_policy(spec_obj):"},{"line_number":23,"context_line":"    \"\"\"Returns True if request applies to a policy check only."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    :param spec_obj: An objects.RequestSpec to examine (or None)."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_f0494260","line":22,"in_reply_to":"ff82abbf_2ec8ac53","updated":"2017-11-20 15:19:46.000000000","message":"Yep, I have this in scheduler.utils for the next rev.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6fea1e802ca60b6c53bdacec9180e7579aca17bc","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from nova import filters"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"def _request_is_policy(spec_obj):"},{"line_number":23,"context_line":"    \"\"\"Returns True if request applies to a policy check only."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    :param spec_obj: An objects.RequestSpec to examine (or None)."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_2ec8ac53","line":22,"in_reply_to":"ff82abbf_f37cefb6","updated":"2017-11-20 15:14:38.000000000","message":"The doubling starts in Pike, so yeah they should be separate. But if we moved this to a utility then the fix for the doubling which uses this could be simpler, or we just refactor this into a utility in the patch that fixes the doubling.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"878d93ea5dd4176427a5e303856255f7a9b39fa2","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    # be created with same_host\u003d[self] because they could not know their uuid"},{"line_number":33,"context_line":"    # at boot time. Thus, the check below will still be safe because only"},{"line_number":34,"context_line":"    # we could be calling the scheduler with such a hint."},{"line_number":35,"context_line":"    same_host \u003d spec_obj.scheduler_hints.get(\u0027same_host\u0027)"},{"line_number":36,"context_line":"    # This is a policy-only check if the hint same_host is set"},{"line_number":37,"context_line":"    # to just [self] for the instance."},{"line_number":38,"context_line":"    return same_host \u003d\u003d [spec_obj.instance_uuid]"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_36d002f8","line":35,"range":{"start_line":35,"start_character":45,"end_line":35,"end_character":56},"updated":"2017-11-20 16:53:58.000000000","message":"there could be a side-effect where an instance is booted by using the same_host hint (\"I want my new instance to be neighbor of that other instance UUID\") so we would keep that hint when we persist the RequestSpec but then for any other scheduling operation, we would replay it.\nTechnically, for a move, it\u0027s not a problem since we don\u0027t care of all the other hosts if we enable SameHostFilter but for a resize, it could be a problem if we enable resizes to the same host where we would still need to check for the capacity given a new flavor.\n\nTBH, I\u0027d be in favor of a straight new hint, even if that sucks because it would be not API-visible (and there could be risks of a custom filter defining that hint name for a totally different purpose).","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6eae89ce5bf209ba5a268b6291dfe01f082d72c9","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    # be created with same_host\u003d[self] because they could not know their uuid"},{"line_number":33,"context_line":"    # at boot time. Thus, the check below will still be safe because only"},{"line_number":34,"context_line":"    # we could be calling the scheduler with such a hint."},{"line_number":35,"context_line":"    same_host \u003d spec_obj.scheduler_hints.get(\u0027same_host\u0027)"},{"line_number":36,"context_line":"    # This is a policy-only check if the hint same_host is set"},{"line_number":37,"context_line":"    # to just [self] for the instance."},{"line_number":38,"context_line":"    return same_host \u003d\u003d [spec_obj.instance_uuid]"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_766c4a08","line":35,"range":{"start_line":35,"start_character":45,"end_line":35,"end_character":56},"in_reply_to":"ff82abbf_36d002f8","updated":"2017-11-20 17:06:55.000000000","message":"Right, which is why I do this after the save in compute/api.\n\nAfter realizing that our behavior without SameHost enabled will be broken, I think a new hint for this purpose is probably a good plan.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"878d93ea5dd4176427a5e303856255f7a9b39fa2","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    # other parameters. We care about running policy filters (i.e."},{"line_number":49,"context_line":"    # ImagePropertiesFilter) but not things that check usage on the"},{"line_number":50,"context_line":"    # existing compute node, etc."},{"line_number":51,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _filter_one(self, obj, filter_properties):"},{"line_number":54,"context_line":"        \"\"\"Return True if the object passes the filter, otherwise False.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_561ee60c","line":51,"range":{"start_line":51,"start_character":4,"end_line":51,"end_character":17},"updated":"2017-11-20 16:53:58.000000000","message":"I don\u0027t want to overthink on naming that attribute, but I think that \"policy\" carries a special meaning in Nova which is not exactly what it relates here.\n\nHere, I\u0027d rather consider naming it as \"capability_filter\" but I leave you NACK my idea.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6eae89ce5bf209ba5a268b6291dfe01f082d72c9","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    # other parameters. We care about running policy filters (i.e."},{"line_number":49,"context_line":"    # ImagePropertiesFilter) but not things that check usage on the"},{"line_number":50,"context_line":"    # existing compute node, etc."},{"line_number":51,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _filter_one(self, obj, filter_properties):"},{"line_number":54,"context_line":"        \"\"\"Return True if the object passes the filter, otherwise False.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_3676c2da","line":51,"range":{"start_line":51,"start_character":4,"end_line":51,"end_character":17},"in_reply_to":"ff82abbf_561ee60c","updated":"2017-11-20 17:06:55.000000000","message":"I don\u0027t like the name capability fort his either. Since we can change it later easily, let\u0027s not stall over the naming.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # existing compute node, etc."},{"line_number":51,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _filter_one(self, obj, filter_properties):"},{"line_number":54,"context_line":"        \"\"\"Return True if the object passes the filter, otherwise False.\"\"\""},{"line_number":55,"context_line":"        if not self.POLICY_FILTER and _request_is_policy(filter_properties):"},{"line_number":56,"context_line":"            # If we don\u0027t filter, default to passing the host."}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_73f7de8c","line":53,"range":{"start_line":53,"start_character":31,"end_line":53,"end_character":48},"updated":"2017-11-17 22:33:51.000000000","message":"OK so apparently this is a RequestSpec now, we just never renamed this variable. We can clean that up separately later.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # existing compute node, etc."},{"line_number":51,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _filter_one(self, obj, filter_properties):"},{"line_number":54,"context_line":"        \"\"\"Return True if the object passes the filter, otherwise False.\"\"\""},{"line_number":55,"context_line":"        if not self.POLICY_FILTER and _request_is_policy(filter_properties):"},{"line_number":56,"context_line":"            # If we don\u0027t filter, default to passing the host."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_934b2386","line":53,"range":{"start_line":53,"start_character":31,"end_line":53,"end_character":48},"in_reply_to":"1f485f77_73f7de8c","updated":"2017-11-20 14:49:13.000000000","message":"Yeah, took me a bit to figure that out :(","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"878d93ea5dd4176427a5e303856255f7a9b39fa2","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    # existing compute node, etc."},{"line_number":51,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def _filter_one(self, obj, filter_properties):"},{"line_number":54,"context_line":"        \"\"\"Return True if the object passes the filter, otherwise False.\"\"\""},{"line_number":55,"context_line":"        if not self.POLICY_FILTER and _request_is_policy(filter_properties):"},{"line_number":56,"context_line":"            # If we don\u0027t filter, default to passing the host."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_16fffeaa","line":53,"range":{"start_line":53,"start_character":31,"end_line":53,"end_character":48},"in_reply_to":"ff82abbf_934b2386","updated":"2017-11-20 16:53:58.000000000","message":"I don\u0027t recall why changing that wasn\u0027t straightforward, but I somehow struggled doing that in the main change that was modifying all the filter interfaces.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9754b3288025824adfc444a6e9677733eafa0866","unresolved":false,"context_lines":[{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class BaseHostFilter(filters.BaseFilter):"},{"line_number":23,"context_line":"    \"\"\"Base class for host filters.\"\"\""},{"line_number":24,"context_line":"    def _filter_one(self, obj, filter_properties):"},{"line_number":25,"context_line":"        \"\"\"Return True if the object passes the filter, otherwise False.\"\"\""},{"line_number":26,"context_line":"        return self.host_passes(obj, filter_properties)"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_611eb70d","side":"PARENT","line":24,"range":{"start_line":24,"start_character":31,"end_line":24,"end_character":48},"updated":"2017-11-21 01:57:24.000000000","message":"Just a heads up for backports to anything older than mitaka, this will not be a RequestSpec since I14f18c4507498b1d24a9318fafc97193861ca0b6.","commit_id":"57728836f25e9201ddec1e7790b552cfa840d572"}],"nova/scheduler/filters/affinity_filter.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    # The hosts the instances are running on doesn\u0027t change within a request"},{"line_number":30,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":35,"context_line":"        affinity_uuids \u003d spec_obj.get_scheduler_hint(\u0027different_host\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_f37e2ee2","line":32,"range":{"start_line":32,"start_character":4,"end_line":32,"end_character":17},"updated":"2017-11-17 22:33:51.000000000","message":"This, and the CIDR and server group (anti)affinity filters below being False is confusing to me. These don\u0027t seem like *resource* filters really, but we are marking them as POLICY_FILTER\u003dFalse so we don\u0027t check them when doing a rebuild. Are we coupling this too much to the rebuild issue?\n\nMaybe it would make more sense to use a RESOURCE_FILTER or COUNTING_FILTER or something like that, so Ram/Disk/Core would all be COUNTING_FILTER\u003dTrue and we\u0027d ignore them for a policy-based request, but everything else defaults to COUNTING_FILTER\u003dFalse (including SameHostFilter) so that those filters get run for a policy-based request.\n\nIt\u0027s probably confusing either way.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    # The hosts the instances are running on doesn\u0027t change within a request"},{"line_number":30,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":35,"context_line":"        affinity_uuids \u003d spec_obj.get_scheduler_hint(\u0027different_host\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_ae035c0d","line":32,"range":{"start_line":32,"start_character":4,"end_line":32,"end_character":17},"in_reply_to":"1f485f77_f37e2ee2","updated":"2017-11-20 14:49:13.000000000","message":"There are definitely some of these filters that were hard to judge. This is clearly not a resource filter, but I guess it seems a bit like a policy sort of deal.\n\nIMHO, We don\u0027t want to run this on rebuild because if someone has force migrated an instance to our host, we don\u0027t want to block rebuilds of that instance until that situation is corrected. Maybe we should have prevented the other from being here in the first place, or warned or whatever, but we don\u0027t want to block rebuild once it\u0027s there I think.\n\nI definitely don\u0027t want this to become the \"RUN_DURING_REBUILD\u003dTrue\" flag, but I also don\u0027t want to introduce a ton of super complicated logic in a backport that generates its own set of new bugs. That\u0027s kindof why I was initially against this approach for a backportable (downstream) fix to this, but I\u0027m not sure I see any other way.\n\nI really wish this hadn\u0027t been classified as a CVE and just considered a bug, because I think the backport for this is far more painful than the actual issue :(","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"878d93ea5dd4176427a5e303856255f7a9b39fa2","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    # This probably would be false normally, but we need it to be set in order"},{"line_number":51,"context_line":"    # to do rebuild host verification"},{"line_number":52,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_16edbeee","line":51,"updated":"2017-11-20 16:53:58.000000000","message":"I\u0027m not convinced by that, could you please explain why we need that filter ?","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6eae89ce5bf209ba5a268b6291dfe01f082d72c9","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    # This probably would be false normally, but we need it to be set in order"},{"line_number":51,"context_line":"    # to do rebuild host verification"},{"line_number":52,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_167dbeb0","line":51,"in_reply_to":"ff82abbf_16edbeee","updated":"2017-11-20 17:06:55.000000000","message":"Because we\u0027re using SameHost for the policy check. if we skip it here, it\u0027ll never filter down to our own host :)","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    \"\"\""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    # NOTE(danms): Rebuild is supposed to fail on this filter if the"},{"line_number":88,"context_line":"    # group policy has been violated. That makes this policy."},{"line_number":89,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_8fe67344","line":88,"updated":"2017-11-21 16:01:36.000000000","message":"This the counter-intuitive one where we have a test for it, right? Would it be possible to have a comment to that effect?","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3cc5cdfbb9479510a3fa52cc1cf80c1c099a079","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    \"\"\""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    # NOTE(danms): Rebuild is supposed to fail on this filter if the"},{"line_number":88,"context_line":"    # group policy has been violated. That makes this policy."},{"line_number":89,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_e0706c75","line":88,"in_reply_to":"ff82abbf_8fe67344","updated":"2017-11-21 17:23:43.000000000","message":"I do have a comment.. What do you want it to say that it isn\u0027t? If you set this to False and run the functional tests you\u0027ll see which one fails. Do you just want the test name called out here or something? It\u0027s really asserting an API-level behavior, so it seems like just calling out the \"supposed to\" behavior should be okay, IMHO.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4762691d1cb272fc3614fbd6df95bb9f6a322b4c","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    \"\"\""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    # NOTE(danms): Rebuild is supposed to fail on this filter if the"},{"line_number":88,"context_line":"    # group policy has been violated. That makes this policy."},{"line_number":89,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_26750c96","line":88,"in_reply_to":"ff82abbf_9be31534","updated":"2017-11-21 18:18:25.000000000","message":"BTW, I did change this to false and ran the functional tests and nothing failed?","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"04441cb2a7cb21bb449f962578273d7bdb3f83df","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    \"\"\""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    # NOTE(danms): Rebuild is supposed to fail on this filter if the"},{"line_number":88,"context_line":"    # group policy has been violated. That makes this policy."},{"line_number":89,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_e6f5e4cc","line":88,"in_reply_to":"ff82abbf_9be31534","updated":"2017-11-21 18:27:42.000000000","message":"We could come up with a new method for the filters, something like host_acceptable, which means \"I want you to decide whether this host, if it had all its resources available, could run this instance.\" I suspect we\u0027ll run into ambiguity with things like cpu and thread policy though. Also, would that be backportable? Probably not.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ad677e3f834ca77c76da163f8430b3f4ef82845","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    \"\"\""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    # NOTE(danms): Rebuild is supposed to fail on this filter if the"},{"line_number":88,"context_line":"    # group policy has been violated. That makes this policy."},{"line_number":89,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_a08fc4ca","line":88,"in_reply_to":"ff82abbf_e0706c75","updated":"2017-11-21 17:52:55.000000000","message":"Fair enough :)","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d4dd0150242c97d3bf0e5052665a6e06e4bfd89","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    \"\"\""},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    # NOTE(danms): Rebuild is supposed to fail on this filter if the"},{"line_number":88,"context_line":"    # group policy has been violated. That makes this policy."},{"line_number":89,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_9be31534","line":88,"in_reply_to":"ff82abbf_e0706c75","updated":"2017-11-21 18:17:46.000000000","message":"I agree that it\u0027s confusing for this to be policy\u003dtrue but _GroupAffinityFilter is not, but I don\u0027t have great suggestions.\n\nWe could just call the variable RUN_FOR_REBUILD to make it super targeted and explicit about which use case this is for and not try to overthink it. It\u0027s definitely gross to tightly couple things this way between an API operation and the scheduler, but I don\u0027t have better ideas right now, especially for something that has to be backported to stable branches.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"}],"nova/scheduler/filters/aggregate_instance_extra_specs.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"878d93ea5dd4176427a5e303856255f7a9b39fa2","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_96af8e93","line":36,"updated":"2017-11-20 16:53:58.000000000","message":"not needed for a pure rebuild where you only change the image, but I can imagine the problem when you wanna resize on the same host.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_4f05cbc1","line":36,"in_reply_to":"ff82abbf_964a2e7e","updated":"2017-11-21 16:01:36.000000000","message":"Right, but previously we didn\u0027t cover that case. We never checked with the scheduler at all during a rebuild, so any admin-made changes between instance creation and rebuild are ignored. Do we want to start enforcing those now, and potentially failing some rebuilds that would previously succeed?","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6eae89ce5bf209ba5a268b6291dfe01f082d72c9","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_964a2e7e","line":36,"in_reply_to":"ff82abbf_96af8e93","updated":"2017-11-20 17:06:55.000000000","message":"Except if the admin has changed aggregate details after you booted...","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33888d8a27c624a152c27594a2ca5a934b9b5687","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    RUN_ON_REBUILD \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff82abbf_169ef755","line":36,"updated":"2017-11-21 20:35:51.000000000","message":"Semi-questionable since the host aggregates could change, the extra specs we\u0027re going to check here are going to be from when the server was originally created, so I\u0027m not sure we\u0027d want to run this for a rebuild and potentially fail the rebuild because the operator changed the aggregates after the initial server create.","commit_id":"5f094f93d4e99fc002869e792d6991c21dfd82ff"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1274e4aea795c764818f55eb1241b59260d2667f","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    RUN_ON_REBUILD \u003d False"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_299b9cc9","line":36,"updated":"2017-11-21 21:01:56.000000000","message":"OK, so we\u0027re saying we\u0027re not going to fail rebuilds because an operator changed something? I\u0027m fine either way, but we need to be consistent. So I\u0027m using this as the baseline.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1e4472d47a0be61e5ed146ac266e8d5d4fdb64e2","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    RUN_ON_REBUILD \u003d False"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_78d90255","line":36,"in_reply_to":"ff82abbf_299b9cc9","updated":"2017-11-27 15:19:16.000000000","message":"I don\u0027t think we can make that sort of consistency statement across the board. As you pointed out, we need to re-run the numa filter, which could fail if the admin has changed something about the topology of the host, even if the new image has the same requirements. Evaluating these based on this \"operator changed something\" criteria isn\u0027t likely to provide the externally-visible consistency you think it will, IMHO.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e97df6e5c5bac31dbb2534682ae46e6953ceb7b","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    RUN_ON_REBUILD \u003d False"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_9f162555","line":36,"in_reply_to":"ff82abbf_5a63ce42","updated":"2017-11-27 20:47:36.000000000","message":"\u003e But since we\u0027re still trying to prevent as much failed rebuilds as possible, we\u0027re not going to start running AggregateInstanceExtraSpecsFilter to catch admin-made aggregate metadata changes.\n\nI\u0027m not sure about the assertion here. I don\u0027t think we can anticipate *everything* that can cause a rebuild to fail, especially considering external factors to the actual rebuild like the admin changing aggregate metadata. I\u0027ve been trying to keep this scoped to things that could fail during a rebuild due to a new image, so any filters that deal with image properties should be run on rebuild. Changing aggregates while you have an instance in those aggregates is asking for trouble, as noted earlier, and Sylvain has a spec related to that, and I don\u0027t think we need to bend over backward to codify what we think could happen in this change.\n\nThat\u0027s also why I suggested we just rename the variable to RUN_ON_REBUILD rather than POLICY_FILTER since the \"policy\" is unclear in all cases.\n\nFWIW this and the AZ filter were True back in PS6:\n\nhttps://review.openstack.org/#/c/521186/6..9/nova/scheduler/filters/aggregate_instance_extra_specs.py","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a539100d317765c5f3d188abc12aebbda456cffd","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    RUN_ON_REBUILD \u003d False"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_849b3b16","line":36,"in_reply_to":"ff82abbf_78d90255","updated":"2017-11-27 17:34:58.000000000","message":"Also, the flavor doesn\u0027t change on a rebuild, so the extra specs are coming from the embedded flavor in the instance (the version of the flavor when the instance was created), so not really something we need to run on rebuild IMO.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"24bebff8026ffefe44d91b138b2f27271f10be53","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # Aggregate data and instance type does not change within a request"},{"line_number":34,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    RUN_ON_REBUILD \u003d False"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":39,"context_line":"        \"\"\"Return a list of hosts that can create instance_type"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_5a63ce42","line":36,"in_reply_to":"ff82abbf_849b3b16","updated":"2017-11-27 20:32:27.000000000","message":"What I had in mind was the admin changing aggregate metadata, but Dan makes a good point. We\u0027re running the NUMA filter because image properties might have changed, but it has the side effect of potentially failing rebuilds on admin-made topology changes.\n\nI had this whole thing in my head about us not *needing* to run the NUMA filter, and the rebuild would just fail at the compute layer if there\u0027s a topology mismatch, but of course it\u0027s better to fail at the scheduler level before we get to the compute, since we\u0027re going to fail regardless. So based on that, yeah, we need to run the NUMA filter. But since we\u0027re still trying to prevent as much failed rebuilds as possible, we\u0027re not going to start running AggregateInstanceExtraSpecsFilter to catch admin-made aggregate metadata changes.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"}],"nova/scheduler/filters/aggregate_multitenancy_isolation.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    # Aggregate data and tenant do not change within a request"},{"line_number":29,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":34,"context_line":"        \"\"\"If a host is in an aggregate that has the metadata key"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_afffb7a8","line":31,"updated":"2017-11-21 16:01:36.000000000","message":"I guess the same applies here as my comments on PS3 of AggregateInstanceExtraSpecsFilter - if an admin makes changes to an aggregate between instance creation and rebuild, do we want to start enforcing that?","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3cc5cdfbb9479510a3fa52cc1cf80c1c099a079","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    # Aggregate data and tenant do not change within a request"},{"line_number":29,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":34,"context_line":"        \"\"\"If a host is in an aggregate that has the metadata key"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_202df442","line":31,"in_reply_to":"ff82abbf_afffb7a8","updated":"2017-11-21 17:23:43.000000000","message":"I dunno, it\u0027s all a judgment call. Since we\u0027re not examining the actual host resources here, it feels like a policy thing. We\u0027d fail on a resize to same host, for example.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1274e4aea795c764818f55eb1241b59260d2667f","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    # Aggregate data and tenant do not change within a request"},{"line_number":29,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    RUN_ON_REBUILD \u003d True"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":34,"context_line":"        \"\"\"If a host is in an aggregate that has the metadata key"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_49d7004d","line":31,"updated":"2017-11-21 21:01:56.000000000","message":"This is not consistent. It could only fail because of admin changes to aggregate metadata - the tenant certainly can\u0027t change with a rebuild.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"24bebff8026ffefe44d91b138b2f27271f10be53","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    # Aggregate data and tenant do not change within a request"},{"line_number":29,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    RUN_ON_REBUILD \u003d True"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":34,"context_line":"        \"\"\"If a host is in an aggregate that has the metadata key"}],"source_content_type":"text/x-python","patch_set":9,"id":"ff82abbf_ff1e51b9","line":31,"updated":"2017-11-27 20:32:27.000000000","message":"So why are we running this one again?","commit_id":"7f6ad3b80919d73ccffba7ef6ce0bfcc84d4a6c3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e97df6e5c5bac31dbb2534682ae46e6953ceb7b","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    # Aggregate data and tenant do not change within a request"},{"line_number":29,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    RUN_ON_REBUILD \u003d True"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":34,"context_line":"        \"\"\"If a host is in an aggregate that has the metadata key"}],"source_content_type":"text/x-python","patch_set":9,"id":"ff82abbf_02b6383f","line":31,"in_reply_to":"ff82abbf_ff1e51b9","updated":"2017-11-27 20:47:36.000000000","message":"You mean why are we running this one but not AggregateInstanceExtraSpecsFilter? idk, good question.\n\nThis was discussed back in PS5:\n\nhttps://review.openstack.org/#/c/521186/5..7/nova/scheduler/filters/aggregate_multitenancy_isolation.py\n\nIf we aren\u0027t going to run AggregateInstanceExtraSpecsFilter or the AZ filter on rebuild then I don\u0027t really see why we\u0027d run this one on rebuild - maybe this is leftover from when the variable was called POLICY_FILTER and there is lingering confusion.","commit_id":"7f6ad3b80919d73ccffba7ef6ce0bfcc84d4a6c3"}],"nova/scheduler/filters/availability_zone_filter.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Availability zones do not change within a request"},{"line_number":36,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":41,"context_line":"        availability_zone \u003d spec_obj.availability_zone"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_2cb095f4","line":38,"updated":"2017-11-21 16:01:36.000000000","message":"The AZ can\u0027t be changed during a rebuild, is this actually a policy filter?","commit_id":"71896898ae33290a536e992fa1db7fab1a0600ff"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Availability zones do not change within a request"},{"line_number":36,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":41,"context_line":"        availability_zone \u003d spec_obj.availability_zone"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_afed77f0","line":38,"updated":"2017-11-21 16:01:36.000000000","message":"AZ can\u0027t change during rebuild, should not be policy.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d4dd0150242c97d3bf0e5052665a6e06e4bfd89","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Availability zones do not change within a request"},{"line_number":36,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":41,"context_line":"        availability_zone \u003d spec_obj.availability_zone"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_bba29999","line":38,"in_reply_to":"ff82abbf_40d35839","updated":"2017-11-21 18:17:46.000000000","message":"Is it possible for an admin to change the AZ of a host by changing the host aggregates? I feel like that\u0027s possible and we don\u0027t check to see that a host has instances before changing it\u0027s aggregates, which prompted a spec from Sylvain:\n\nhttps://review.openstack.org/#/c/446446/","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3cc5cdfbb9479510a3fa52cc1cf80c1c099a079","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Availability zones do not change within a request"},{"line_number":36,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":41,"context_line":"        availability_zone \u003d spec_obj.availability_zone"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_40d35839","line":38,"in_reply_to":"ff82abbf_afed77f0","updated":"2017-11-21 17:23:43.000000000","message":"Ack yeah I think I agree.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33888d8a27c624a152c27594a2ca5a934b9b5687","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Availability zones do not change within a request"},{"line_number":36,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    RUN_ON_REBUILD \u003d True"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":41,"context_line":"        availability_zone \u003d spec_obj.availability_zone"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff82abbf_d6c88f56","line":38,"updated":"2017-11-21 20:35:51.000000000","message":"So if the admin changed the aggregate metadata which changed the AZ after the instance was initially created, the rebuild could now fail on this, is that something we\u0027d want? It doesn\u0027t really have anything to do with what can happen on a rebuild (no move, no new AZ, etc). I\u0027d lean toward this being False.","commit_id":"5f094f93d4e99fc002869e792d6991c21dfd82ff"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1274e4aea795c764818f55eb1241b59260d2667f","unresolved":false,"context_lines":[{"line_number":35,"context_line":"    # Availability zones do not change within a request"},{"line_number":36,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    RUN_ON_REBUILD \u003d False"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":41,"context_line":"        availability_zone \u003d spec_obj.availability_zone"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_c9ab10d4","line":38,"updated":"2017-11-21 21:01:56.000000000","message":"Consistent.","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"}],"nova/scheduler/filters/compute_capabilities_filter.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    # Instance type and host capabilities do not change within a request"},{"line_number":31,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    # This checks that the flavor is valid for a given host, which is"},{"line_number":34,"context_line":"    # policy. An operator could change this after an instance is booted,"},{"line_number":35,"context_line":"    # preventing a rebuild from proceeding with a no-longer-valid-here"},{"line_number":36,"context_line":"    # flavor, but that is probably desirable."}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_539182f0","line":33,"updated":"2017-11-17 22:33:51.000000000","message":"This is helpful. I\u0027m thinking we might want to have a description in each filter for the reasoning behind why it\u0027s whatever value it is, but we could also follow up on that.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    # Instance type and host capabilities do not change within a request"},{"line_number":31,"context_line":"    run_filter_once_per_request \u003d True"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    # This checks that the flavor is valid for a given host, which is"},{"line_number":34,"context_line":"    # policy. An operator could change this after an instance is booted,"},{"line_number":35,"context_line":"    # preventing a rebuild from proceeding with a no-longer-valid-here"},{"line_number":36,"context_line":"    # flavor, but that is probably desirable."}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_ce1b4071","line":33,"in_reply_to":"1f485f77_539182f0","updated":"2017-11-20 14:49:13.000000000","message":"Yeah, and I started off on this thinking I might do that, but.. we have a lot of filters.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # This checks that the flavor is valid for a given host, which is"},{"line_number":34,"context_line":"    # policy. An operator could change this after an instance is booted,"},{"line_number":35,"context_line":"    # preventing a rebuild from proceeding with a no-longer-valid-here"},{"line_number":36,"context_line":"    # flavor, but that is probably desirable."},{"line_number":37,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def _get_capabilities(self, host_state, scope):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_2fbba7e2","line":36,"updated":"2017-11-21 16:01:36.000000000","message":"Same question about starting to enforce admin-made changes.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c7f6b31a18c7f6f47fc13c1cc321b6fcbb8fadb2","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # This checks that the flavor is valid for a given host, which is"},{"line_number":34,"context_line":"    # policy. An operator could change this after an instance is booted,"},{"line_number":35,"context_line":"    # preventing a rebuild from proceeding with a no-longer-valid-here"},{"line_number":36,"context_line":"    # flavor, but that is probably desirable."},{"line_number":37,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def _get_capabilities(self, host_state, scope):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_3bc76915","line":36,"in_reply_to":"ff82abbf_00b6b0fb","updated":"2017-11-21 18:08:00.000000000","message":"So on IRC we said that since flavoUrs stay with the instance, any changes to the top-level flavoUr don\u0027t apply to the instance, so this can be changed to False.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3cc5cdfbb9479510a3fa52cc1cf80c1c099a079","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    # This checks that the flavor is valid for a given host, which is"},{"line_number":34,"context_line":"    # policy. An operator could change this after an instance is booted,"},{"line_number":35,"context_line":"    # preventing a rebuild from proceeding with a no-longer-valid-here"},{"line_number":36,"context_line":"    # flavor, but that is probably desirable."},{"line_number":37,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def _get_capabilities(self, host_state, scope):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_00b6b0fb","line":36,"in_reply_to":"ff82abbf_2fbba7e2","updated":"2017-11-21 17:23:43.000000000","message":"As I said in my comment above, I feel like this should fail. I\u0027ll leave it to consensus about whether or not we actually mark it as policy.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"}],"nova/scheduler/filters/compute_filter.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33ec2a73378e5dac977f2c6432e9af8621d7f669","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":35,"context_line":"        \"\"\"Returns True for only active compute nodes.\"\"\""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"        if self.request_is_policy(spec_obj):"},{"line_number":38,"context_line":"            return True"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"        service \u003d host_state.service"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_6d537f6c","line":37,"updated":"2017-11-17 20:39:31.000000000","message":"It would be nice if we could define an abstractmethod for each filter to indicate if it\u0027s a resource or policy filter, and check that in BaseHostFilter._filter_one() before calling host_passes(). Then we maybe have less whack-a-mole.","commit_id":"298e57e0bf90c155c9f3684086e88795a1d86578"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":25,"context_line":"class ComputeFilter(filters.BaseHostFilter):"},{"line_number":26,"context_line":"    \"\"\"Filter on active Compute nodes.\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def __init__(self):"},{"line_number":31,"context_line":"        self.servicegroup_api \u003d servicegroup.API()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_1398aa19","line":28,"updated":"2017-11-17 22:33:51.000000000","message":"This one is a bit confusing at first too, but I think you can make the case that you can\u0027t \"claim\" resources on a disabled compute, therefore making this a resource type of filter.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":25,"context_line":"class ComputeFilter(filters.BaseHostFilter):"},{"line_number":26,"context_line":"    \"\"\"Filter on active Compute nodes.\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    def __init__(self):"},{"line_number":31,"context_line":"        self.servicegroup_api \u003d servicegroup.API()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_4e21709f","line":28,"in_reply_to":"1f485f77_1398aa19","updated":"2017-11-20 14:49:13.000000000","message":"Right, and certainly if you think about it in the rebuild case, it\u0027s clear we don\u0027t want to run this.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":37,"context_line":"        \"\"\"Returns True for only active compute nodes.\"\"\""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"        service \u003d host_state.service"},{"line_number":40,"context_line":"        if service[\u0027disabled\u0027]:"},{"line_number":41,"context_line":"            LOG.debug(\"%(host_state)s is disabled, reason: %(reason)s\","}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_d3e8d27c","line":38,"updated":"2017-11-17 22:33:51.000000000","message":"shocking","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"}],"nova/scheduler/filters/json_filter.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    selecting hosts."},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _op_compare(self, args, op):"},{"line_number":33,"context_line":"        \"\"\"Returns True if the specified operator can successfully"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_93b49a81","line":30,"updated":"2017-11-17 22:33:51.000000000","message":"This one could be confusing, but for the purpose of rebuild, we aren\u0027t going to have the \u0027query\u0027 hint so this will noop right away anyway.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    selecting hosts."},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _op_compare(self, args, op):"},{"line_number":33,"context_line":"        \"\"\"Returns True if the specified operator can successfully"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_ae511c11","line":30,"in_reply_to":"1f485f77_93b49a81","updated":"2017-11-20 14:49:13.000000000","message":"Well, it compares against anything on the host structure, right? So it could be a little bit of either, but more often used for resource stuff I assumed.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6fea1e802ca60b6c53bdacec9180e7579aca17bc","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    selecting hosts."},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _op_compare(self, args, op):"},{"line_number":33,"context_line":"        \"\"\"Returns True if the specified operator can successfully"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_ae66fc50","line":30,"in_reply_to":"ff82abbf_ae511c11","updated":"2017-11-20 15:14:38.000000000","message":"Yeah I\u0027m not sure, but as noted, ultimately I don\u0027t think it matters because the only thing that cares about the POLICY_FILTER tag is the rebuild scenario, and in that scenario, we won\u0027t have the query hint, so this noops right away.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33888d8a27c624a152c27594a2ca5a934b9b5687","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    selecting hosts."},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    RUN_ON_REBUILD \u003d True"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def _op_compare(self, args, op):"},{"line_number":33,"context_line":"        \"\"\"Returns True if the specified operator can successfully"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff82abbf_d6cdef5d","line":30,"updated":"2017-11-21 20:35:51.000000000","message":"This can be False just to save time - this is a noop since rebuild won\u0027t have the \u0027query\u0027 hint.","commit_id":"5f094f93d4e99fc002869e792d6991c21dfd82ff"}],"nova/scheduler/filters/numa_topology_filter.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":23,"context_line":"class NUMATopologyFilter(filters.BaseHostFilter):"},{"line_number":24,"context_line":"    \"\"\"Filter on requested NUMA topology.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def _satisfies_cpu_policy(self, host_state, extra_specs, image_props):"},{"line_number":29,"context_line":"        \"\"\"Check that the host_state provided satisfies any available"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_8f77533b","line":26,"updated":"2017-11-21 16:01:36.000000000","message":"We\u0027re checking image_props here, shouldn\u0027t it be True?","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ad677e3f834ca77c76da163f8430b3f4ef82845","unresolved":false,"context_lines":[{"line_number":23,"context_line":"class NUMATopologyFilter(filters.BaseHostFilter):"},{"line_number":24,"context_line":"    \"\"\"Filter on requested NUMA topology.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def _satisfies_cpu_policy(self, host_state, extra_specs, image_props):"},{"line_number":29,"context_line":"        \"\"\"Check that the host_state provided satisfies any available"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_9b8eb576","line":26,"in_reply_to":"ff82abbf_608f7c2a","updated":"2017-11-21 17:52:55.000000000","message":"Hrmm, yeah, we only use image_props to check CPU and thread policy, so if we change that in the image, can we land in a situation where the instance\u0027s host can no longer accept the new cpu_policy and/or cpu_thread_policy?","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3cc5cdfbb9479510a3fa52cc1cf80c1c099a079","unresolved":false,"context_lines":[{"line_number":23,"context_line":"class NUMATopologyFilter(filters.BaseHostFilter):"},{"line_number":24,"context_line":"    \"\"\"Filter on requested NUMA topology.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def _satisfies_cpu_policy(self, host_state, extra_specs, image_props):"},{"line_number":29,"context_line":"        \"\"\"Check that the host_state provided satisfies any available"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_608f7c2a","line":26,"in_reply_to":"ff82abbf_8f77533b","updated":"2017-11-21 17:23:43.000000000","message":"Yeah, fair point, and I struggled with this one a bit (which I guess means there needs to be a comment). This is very much a resource-y filter, because it\u0027s making sure we have space and topology on the host. And, it sets new limits which it expects the compute host to honor, but that won\u0027t happen.\n\nSince rebuild is in-place and necessarily gets the same numa configuration the original instance had (right?) I feel like this should be false for rebuild. Maybe we need some more NUMA-focused people to opine here...","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d4dd0150242c97d3bf0e5052665a6e06e4bfd89","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        # NOTE(stephenfin): There can be conflicts between the policy"},{"line_number":35,"context_line":"        # specified by the image and that specified by the instance, but this"},{"line_number":36,"context_line":"        # is not the place to resolve these. We do this during scheduling."},{"line_number":37,"context_line":"        cpu_policy \u003d [extra_specs.get(\u0027hw:cpu_policy\u0027),"},{"line_number":38,"context_line":"                      image_props.get(\u0027hw_cpu_policy\u0027)]"},{"line_number":39,"context_line":"        cpu_thread_policy \u003d [extra_specs.get(\u0027hw:cpu_thread_policy\u0027),"},{"line_number":40,"context_line":"                             image_props.get(\u0027hw_cpu_thread_policy\u0027)]"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        if not host_topology:"},{"line_number":43,"context_line":"            return True"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"        if fields.CPUAllocationPolicy.DEDICATED not in cpu_policy:"},{"line_number":46,"context_line":"            return True"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"        if fields.CPUThreadAllocationPolicy.REQUIRE not in cpu_thread_policy:"},{"line_number":49,"context_line":"            return True"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"        # the presence of siblings in at least one cell indicates"},{"line_number":52,"context_line":"        # hyperthreading (HT)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_fbd021cc","line":49,"range":{"start_line":37,"start_character":8,"end_line":49,"end_character":23},"updated":"2017-11-21 18:17:46.000000000","message":"Right this seems like the tricky part, if the new image for rebuild has a different numa policy then should we validate that image against this host and make the rebuild possibly fail? I think that might be OK considering the original CVE was all about running the scheduler filters on a supplied image to make sure it\u0027s still valid for the host the instance is running on. Maybe if we thinking about it like this: \"resource constraints aside, would this filter pass on the provided image during a server create?\"\n\nThe \u0027provided image\u0027 part gets confusing when we\u0027re talking about the affinity and ComputeFilter, which have nothing to do with images.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"1274e4aea795c764818f55eb1241b59260d2667f","unresolved":false,"context_lines":[{"line_number":23,"context_line":"class NUMATopologyFilter(filters.BaseHostFilter):"},{"line_number":24,"context_line":"    \"\"\"Filter on requested NUMA topology.\"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    RUN_ON_REBUILD \u003d False"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"    def _satisfies_cpu_policy(self, host_state, extra_specs, image_props):"},{"line_number":29,"context_line":"        \"\"\"Check that the host_state provided satisfies any available"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_a997ac90","line":26,"updated":"2017-11-21 21:01:56.000000000","message":"The issue I see is if we go from thread policy prefer to isolate, for example. The latter\u0027s doc [1] says:\n\n  If the host does have an SMT architecture - that is, one\n  or more cores have thread siblings - then each vCPU is\n  placed on a different physical core. No vCPUs from other\n  guests are placed on the same core.\n\nWe might fail at the compute level if there are not enough available host pCPUs? Someone more NUMA-y can weigh in. But if this is a problem, it\u0027s been there all long, since we\u0027ve only just now starting calling the scheduler for rebuilds, so we\u0027re not introducing any new bugs.\n\n[1] https://docs.openstack.org/nova/pike/admin/flavors.html#extra-specs","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ad4852f462729cd7c0277cc86b6813aa367eb1a4","unresolved":false,"context_lines":[{"line_number":34,"context_line":"        # NOTE(stephenfin): There can be conflicts between the policy"},{"line_number":35,"context_line":"        # specified by the image and that specified by the instance, but this"},{"line_number":36,"context_line":"        # is not the place to resolve these. We do this during scheduling."},{"line_number":37,"context_line":"        cpu_policy \u003d [extra_specs.get(\u0027hw:cpu_policy\u0027),"},{"line_number":38,"context_line":"                      image_props.get(\u0027hw_cpu_policy\u0027)]"},{"line_number":39,"context_line":"        cpu_thread_policy \u003d [extra_specs.get(\u0027hw:cpu_thread_policy\u0027),"},{"line_number":40,"context_line":"                             image_props.get(\u0027hw_cpu_thread_policy\u0027)]"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        if not host_topology:"},{"line_number":43,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_099ed855","line":40,"range":{"start_line":37,"start_character":8,"end_line":40,"end_character":69},"updated":"2017-11-21 21:07:41.000000000","message":"Oh yeah...","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ad4852f462729cd7c0277cc86b6813aa367eb1a4","unresolved":false,"context_lines":[{"line_number":74,"context_line":"        ram_ratio \u003d host_state.ram_allocation_ratio"},{"line_number":75,"context_line":"        cpu_ratio \u003d host_state.cpu_allocation_ratio"},{"line_number":76,"context_line":"        extra_specs \u003d spec_obj.flavor.extra_specs"},{"line_number":77,"context_line":"        image_props \u003d spec_obj.image.properties"},{"line_number":78,"context_line":"        requested_topology \u003d spec_obj.numa_topology"},{"line_number":79,"context_line":"        host_topology, _fmt \u003d hardware.host_topology_and_format_from_host("},{"line_number":80,"context_line":"                host_state)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ff82abbf_e998546b","line":77,"range":{"start_line":77,"start_character":8,"end_line":77,"end_character":47},"updated":"2017-11-21 21:07:41.000000000","message":"Oh yeah...","commit_id":"3b0e8e2ae7566c83bd763c7008d03b59a6d8c1dc"}],"nova/scheduler/filters/pci_passthrough_filter.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6fea1e802ca60b6c53bdacec9180e7579aca17bc","unresolved":false,"context_lines":[{"line_number":23,"context_line":"class PciPassthroughFilter(filters.BaseHostFilter):"},{"line_number":24,"context_line":"    \"\"\"Pci Passthrough Filter based on PCI request"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    Filter that schedules instances on a host if the host has devices"},{"line_number":27,"context_line":"    to meet the device requests in the \u0027extra_specs\u0027 for the flavor."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    PCI resource tracker provides updated summary information about the"},{"line_number":30,"context_line":"    PCI devices for each host, like::"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_6ed554b9","line":27,"range":{"start_line":26,"start_character":4,"end_line":27,"end_character":68},"updated":"2017-11-20 15:14:38.000000000","message":"I think the pci requests just come through extra specs. Docs seem to indicate the same.\n\nhttps://docs.openstack.org/nova/latest/admin/pci-passthrough.html?#configure-a-flavor-controller\n\nhttps://docs.openstack.org/nova/latest/user/flavors.html?highlight\u003dpci","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5bf5d79cee820903c64df83b7bf0bbf55092e769","unresolved":false,"context_lines":[{"line_number":23,"context_line":"class PciPassthroughFilter(filters.BaseHostFilter):"},{"line_number":24,"context_line":"    \"\"\"Pci Passthrough Filter based on PCI request"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    Filter that schedules instances on a host if the host has devices"},{"line_number":27,"context_line":"    to meet the device requests in the \u0027extra_specs\u0027 for the flavor."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    PCI resource tracker provides updated summary information about the"},{"line_number":30,"context_line":"    PCI devices for each host, like::"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_b09adac2","line":27,"range":{"start_line":26,"start_character":4,"end_line":27,"end_character":68},"in_reply_to":"ff82abbf_6ed554b9","updated":"2017-11-20 15:19:46.000000000","message":"Ack, I shall remove it.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    # FIXME(danms): Is this right? Can we override something here in the image"},{"line_number":44,"context_line":"    # so that we could get a pci device for free? The bug seems to imply so..."},{"line_number":45,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_134f8a93","line":44,"range":{"start_line":44,"start_character":50,"end_line":44,"end_character":57},"updated":"2017-11-17 22:33:51.000000000","message":"Which bug? As far as I can tell, this filter doesn\u0027t check anything with image metadata and it\u0027s comparing resources on the host based on the pci requests in the request spec, so it seems more like the ram/core/disk filter.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    # FIXME(danms): Is this right? Can we override something here in the image"},{"line_number":44,"context_line":"    # so that we could get a pci device for free? The bug seems to imply so..."},{"line_number":45,"context_line":"    POLICY_FILTER \u003d True"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_ce6260d9","line":44,"range":{"start_line":44,"start_character":50,"end_line":44,"end_character":57},"in_reply_to":"1f485f77_134f8a93","updated":"2017-11-20 14:49:13.000000000","message":"Well, I feel like the original bug we marked as a CVE was asserting some sort of exposure by booting to a host and then rebuilding to one with additional hardware resources. I thought that pci_requests comes from flavor extra specs, which might be merged with image metadata somehow?\n\nAnyway, I didn\u0027t spend time tracking it down, but since the original bug seemed to assert something about hardware, I figured it was worth calling out. If you\u0027re sure it\u0027s not harmful I can flip the flag.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":46,"context_line":"        \"\"\"Return true if the host has the required PCI devices.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_4fb86bc7","line":43,"updated":"2017-11-21 16:01:36.000000000","message":"This is form the flavour, correct? If ComputeCapabilitiesFilter is policy because admin can change flavour, shouldn\u0027t this be policy as well? In any case, they should the consistent, whatever we decide about enforcing admin-made changes.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"c7f6b31a18c7f6f47fc13c1cc321b6fcbb8fadb2","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":46,"context_line":"        \"\"\"Return true if the host has the required PCI devices.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_3bf0c967","line":43,"in_reply_to":"ff82abbf_3b0529c3","updated":"2017-11-21 18:08:00.000000000","message":"Ditto about top-level vs instance flavoUrs.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d4dd0150242c97d3bf0e5052665a6e06e4bfd89","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":46,"context_line":"        \"\"\"Return true if the host has the required PCI devices.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_3b9529e5","line":43,"in_reply_to":"ff82abbf_3b0529c3","updated":"2017-11-21 18:17:46.000000000","message":"From our IRC conversation, the rebuild will use the embedded instance flavor and the original extra specs used to build the instance, so if the operator does change the pci passthrough extra specs on the filter, it shouldn\u0027t really affect the rebuild since it\u0027s not going to use that flavor, it will use the embedded one.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3cc5cdfbb9479510a3fa52cc1cf80c1c099a079","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":46,"context_line":"        \"\"\"Return true if the host has the required PCI devices.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_8077203b","line":43,"in_reply_to":"ff82abbf_4fb86bc7","updated":"2017-11-21 17:23:43.000000000","message":"I had originally marked this as True, because the original bug seemed to imply something about providing access to hardware that didn\u0027t get exposed before. See the discussion on this in the review here.\n\nI\u0027d love a good analysis and opinion from someone more versed in the PCI stuff than I.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ad677e3f834ca77c76da163f8430b3f4ef82845","unresolved":false,"context_lines":[{"line_number":40,"context_line":""},{"line_number":41,"context_line":"    \"\"\""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":46,"context_line":"        \"\"\"Return true if the host has the required PCI devices.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_3b0529c3","line":43,"in_reply_to":"ff82abbf_8077203b","updated":"2017-11-21 17:52:55.000000000","message":"It\u0027s not so much about the user changing anything (which they can\u0027t, I don\u0027t think, but yeah, someone more PCI-y would be useful here), it\u0027s about admin making changes to PCI in flavour extra specs after the instance has been booted, and us deciding whether we want to enforce that on rebuild. I don\u0027t think I have a preference, but we should be consistent.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"}],"nova/scheduler/filters/retry_filter.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":26,"context_line":"    purposes"},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":32,"context_line":"        \"\"\"Skip nodes that have already been attempted.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_53586254","line":29,"updated":"2017-11-17 22:33:51.000000000","message":"This one is kind of confusing. Policy is you can only have x number of retries. We don\u0027t have retries with rebuild, so we shouldn\u0027t care about this filter, but the semantics of what POLICY_FILTER means here is fuzzy.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":26,"context_line":"    purposes"},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":32,"context_line":"        \"\"\"Skip nodes that have already been attempted.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_2eabccd6","line":29,"in_reply_to":"1f485f77_53586254","updated":"2017-11-20 14:49:13.000000000","message":"Well, my reasoning here was that this does not enforce anything about _where_ a host lands.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6fea1e802ca60b6c53bdacec9180e7579aca17bc","unresolved":false,"context_lines":[{"line_number":26,"context_line":"    purposes"},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def host_passes(self, host_state, spec_obj):"},{"line_number":32,"context_line":"        \"\"\"Skip nodes that have already been attempted.\"\"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_ae67dc25","line":29,"in_reply_to":"ff82abbf_2eabccd6","updated":"2017-11-20 15:14:38.000000000","message":"\u003e Well, my reasoning here was that this does not enforce anything\n \u003e about _where_ a host lands.\n\nOK, that\u0027s a good thought, I keep getting tripped up in how this flag should be interpreted and it\u0027s good to have the hints / reasoning behind it per-filter, so maybe for this one add a comment about that.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33888d8a27c624a152c27594a2ca5a934b9b5687","unresolved":false,"context_lines":[{"line_number":26,"context_line":"    purposes"},{"line_number":27,"context_line":"    \"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    # NOTE(danms): This does not affect _where_ an instance lands, so not"},{"line_number":30,"context_line":"    # policy"},{"line_number":31,"context_line":"    RUN_ON_REBUILD \u003d False"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def host_passes(self, host_state, spec_obj):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff82abbf_d6f64f90","line":30,"range":{"start_line":29,"start_character":4,"end_line":30,"end_character":12},"updated":"2017-11-21 20:35:51.000000000","message":"nit: could update this, or just remove it - we don\u0027t reschedule on a rebuild so that\u0027s why we don\u0027t care about this one for rebuild.","commit_id":"5f094f93d4e99fc002869e792d6991c21dfd82ff"}],"nova/scheduler/filters/trusted_filter.py":[{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":230,"context_line":"class TrustedFilter(filters.BaseHostFilter):"},{"line_number":231,"context_line":"    \"\"\"Trusted filter to support Trusted Compute Pools.\"\"\""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def __init__(self):"},{"line_number":236,"context_line":"        self.compute_attestation \u003d ComputeAttestation()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_8fdaf314","line":233,"updated":"2017-11-21 16:01:36.000000000","message":"Same question as PciPassthroughFilter - whatever we decide about enforcing admin-made changes (to flavour for example), we need to be consistent, and this filter does check flavour extra specs.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d4dd0150242c97d3bf0e5052665a6e06e4bfd89","unresolved":false,"context_lines":[{"line_number":230,"context_line":"class TrustedFilter(filters.BaseHostFilter):"},{"line_number":231,"context_line":"    \"\"\"Trusted filter to support Trusted Compute Pools.\"\"\""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def __init__(self):"},{"line_number":236,"context_line":"        self.compute_attestation \u003d ComputeAttestation()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_7babb11c","line":233,"in_reply_to":"ff82abbf_7b02d1b6","updated":"2017-11-21 18:17:46.000000000","message":"Plus, same response as in the pci passthrough filter I think - we use the original embedded instance flavor for rebuild, not the one being requested during a server create (which is when this would normally be used). And in this filter, we get the flavor off the request spec, which persists the original flavor used to create the instance, so during a migration or rebuild, the flavor checked here is the same as when we created the instance, so I don\u0027t think we need to run this on rebuild regardless of the state of the flavor at the API level now. That does make it more of a case for renaming the variable to be RUN_FOR_REBUILD though since it\u0027s so explicit in scope.\n\nIn other words, if the instance is already running on this host, it passed the TrustedFilter already, so we shouldn\u0027t fail a rebuild on this host since it\u0027s already \"trusted\" for this instance.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3cc5cdfbb9479510a3fa52cc1cf80c1c099a079","unresolved":false,"context_lines":[{"line_number":230,"context_line":"class TrustedFilter(filters.BaseHostFilter):"},{"line_number":231,"context_line":"    \"\"\"Trusted filter to support Trusted Compute Pools.\"\"\""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def __init__(self):"},{"line_number":236,"context_line":"        self.compute_attestation \u003d ComputeAttestation()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_a0a9047c","line":233,"in_reply_to":"ff82abbf_8fdaf314","updated":"2017-11-21 17:23:43.000000000","message":"I made it false since this is deprecated and \"has been marked experimental for some time with no tests.\"","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ad677e3f834ca77c76da163f8430b3f4ef82845","unresolved":false,"context_lines":[{"line_number":230,"context_line":"class TrustedFilter(filters.BaseHostFilter):"},{"line_number":231,"context_line":"    \"\"\"Trusted filter to support Trusted Compute Pools.\"\"\""},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    def __init__(self):"},{"line_number":236,"context_line":"        self.compute_attestation \u003d ComputeAttestation()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_7b02d1b6","line":233,"in_reply_to":"ff82abbf_a0a9047c","updated":"2017-11-21 17:52:55.000000000","message":"Yeah, that\u0027s valid. If we\u0027re telling people to not use it, we probably shouldn\u0027t run it on rebuild.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"}],"nova/scheduler/filters/type_filter.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d8aca55974b530ac036ebdd38bfb596166ad6a24","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    (spread) set to 1 (default)."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self):"},{"line_number":36,"context_line":"        super(TypeAffinityFilter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_d339f2f1","line":33,"updated":"2017-11-17 22:33:51.000000000","message":"not sure about this one either; rebuild wouldn\u0027t fail on this either way because the flavor isn\u0027t going to change, and it does seem like policy to me, but what policy is and isn\u0027t is confusing.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    (spread) set to 1 (default)."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self):"},{"line_number":36,"context_line":"        super(TypeAffinityFilter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_2ed98c82","line":33,"in_reply_to":"1f485f77_d339f2f1","updated":"2017-11-20 14:49:13.000000000","message":"Well, my thinking (which I should have documented) was that:\n\n(a) it\u0027s deprecated so probably no need to run it\n\n(b) if the admin has changed the affinity mapping after the instance has booted, we probably shouldn\u0027t block rebuilds since it\u0027s already where it is.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6fea1e802ca60b6c53bdacec9180e7579aca17bc","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    (spread) set to 1 (default)."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self):"},{"line_number":36,"context_line":"        super(TypeAffinityFilter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_ee03a40e","line":33,"in_reply_to":"ff82abbf_2ed98c82","updated":"2017-11-20 15:14:38.000000000","message":"Hmm, (a) won\u0027t apply for backports, and (b) sounds like a conflict with the reasoning behind the RetryFilter, i.e. this filter applies to policy about *where* the instance lands, so if the operator changes the affinity mapping after the fact, it\u0027s a policy change and seems we should make this POLICY_FILTER\u003dTrue, but I can see your point about not making rebuild fail here. If this filter checked anything in an image though, would we set it to True? I think we probably would. So we might be getting mixed up in what this flag means in a pure sense vs what it means for rebuild specifically - without calling it the REBUILD_FILTER flag or whatever.\n\nHonestly on this one I don\u0027t really care either way. We\u0027ll remove it in Queens and I doubt anyone is relying on this filter anyway given how limited it is.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"efc0e885f983edcffe552d569305dd02f5c66af2","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    (spread) set to 1 (default)."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self):"},{"line_number":36,"context_line":"        super(TypeAffinityFilter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_0f002367","line":33,"updated":"2017-11-21 16:01:36.000000000","message":"Ditto about flavour.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4ad677e3f834ca77c76da163f8430b3f4ef82845","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    (spread) set to 1 (default)."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self):"},{"line_number":36,"context_line":"        super(TypeAffinityFilter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_9bff95a9","line":33,"in_reply_to":"ff82abbf_00b230e7","updated":"2017-11-21 17:52:55.000000000","message":"Same agreement :)","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f3cc5cdfbb9479510a3fa52cc1cf80c1c099a079","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    (spread) set to 1 (default)."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self):"},{"line_number":36,"context_line":"        super(TypeAffinityFilter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_00b230e7","line":33,"in_reply_to":"ff82abbf_0f002367","updated":"2017-11-21 17:23:43.000000000","message":"Same argument here about deprecatedness.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d4dd0150242c97d3bf0e5052665a6e06e4bfd89","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    (spread) set to 1 (default)."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    POLICY_FILTER \u003d False"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def __init__(self):"},{"line_number":36,"context_line":"        super(TypeAffinityFilter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_a6327cf0","line":33,"in_reply_to":"ff82abbf_9bff95a9","updated":"2017-11-21 18:17:46.000000000","message":"Same comments from the TrustedFilter - flavor here is from the original create.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d4dd0150242c97d3bf0e5052665a6e06e4bfd89","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        return not other_types_on_host"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"class AggregateTypeAffinityFilter(filters.BaseHostFilter):"},{"line_number":60,"context_line":"    \"\"\"AggregateTypeAffinityFilter limits instance_type by aggregate"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    return True if no instance_type key is set or if the aggregate metadata"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_46285085","line":59,"range":{"start_line":59,"start_character":6,"end_line":59,"end_character":33},"updated":"2017-11-21 18:17:46.000000000","message":"This one is not deprecated and doesn\u0027t extend the other deprecated filter, and is related to aggregates, so should probably be POLICY_FILTER\u003dTrue yeah?\n\nIf it were RUN_FOR_REBUILD I\u0027d say it\u0027s False though since rebuild doesn\u0027t care about the flavor since it doesn\u0027t change.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"33ec2a73378e5dac977f2c6432e9af8621d7f669","unresolved":false,"context_lines":[{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def setUp(self):"},{"line_number":1070,"context_line":"        filters \u003d ["},{"line_number":1071,"context_line":"            \u0027SameHostFilter\u0027,  # Required for the policy bypass"},{"line_number":1072,"context_line":"            \u0027RetryFilter\u0027,     # Default"},{"line_number":1073,"context_line":"            \u0027ComputeFilter\u0027,   # Required to ensure this skips"},{"line_number":1074,"context_line":"            \u0027ImagePropertiesFilter\u0027,  # Required for the policy check"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_adebb775","line":1071,"updated":"2017-11-17 20:39:31.000000000","message":"This is the only non-default filter isn\u0027t it? Would it be better to take the default list of filters and append this one in so we get tighter coverage of at least the default set of filters?","commit_id":"298e57e0bf90c155c9f3684086e88795a1d86578"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"1d6dc6c5595e588fceb7e9c04120bb9614c8316d","unresolved":false,"context_lines":[{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"    def setUp(self):"},{"line_number":1070,"context_line":"        filters \u003d ["},{"line_number":1071,"context_line":"            \u0027SameHostFilter\u0027,  # Required for the policy bypass"},{"line_number":1072,"context_line":"            \u0027RetryFilter\u0027,     # Default"},{"line_number":1073,"context_line":"            \u0027ComputeFilter\u0027,   # Required to ensure this skips"},{"line_number":1074,"context_line":"            \u0027ImagePropertiesFilter\u0027,  # Required for the policy check"}],"source_content_type":"text/x-python","patch_set":1,"id":"1f485f77_ad6e17fd","line":1071,"in_reply_to":"1f485f77_adebb775","updated":"2017-11-17 20:43:06.000000000","message":"Yeah I just didn\u0027t have access to CONF here.","commit_id":"298e57e0bf90c155c9f3684086e88795a1d86578"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"142251aa315f07bd1c10f3a9a1e8e6fd915e38da","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"        filters \u003d CONF.filter_scheduler.enabled_filters"},{"line_number":1073,"context_line":"        # We need SameHostFilter for the hint to work for validating our"},{"line_number":1074,"context_line":"        # existing host"},{"line_number":1075,"context_line":"        filters.append(\u0027SameHostFilter\u0027)"},{"line_number":1076,"context_line":"        self.flags(enabled_filters\u003dfilters, group\u003d\u0027filter_scheduler\u0027)"},{"line_number":1077,"context_line":"        super(ServerRebuildTestCase, self).setUp()"},{"line_number":1078,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_ea5508fb","line":1075,"updated":"2017-11-18 21:11:05.000000000","message":"What happens if we remove this? Thinking about the fact people will have to enable this filter for the fix, what happens if they don\u0027t or miss this part? Do they get NoValidHost or potentially a new host (half evacuate)? If we don\u0027t get a NoValidHost, we might need to check in conductor that the selected host is the same as instance.host and fail if not.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f4c2b34536fee0f26ef29b1683b952b6272a35d8","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"        filters \u003d CONF.filter_scheduler.enabled_filters"},{"line_number":1073,"context_line":"        # We need SameHostFilter for the hint to work for validating our"},{"line_number":1074,"context_line":"        # existing host"},{"line_number":1075,"context_line":"        filters.append(\u0027SameHostFilter\u0027)"},{"line_number":1076,"context_line":"        self.flags(enabled_filters\u003dfilters, group\u003d\u0027filter_scheduler\u0027)"},{"line_number":1077,"context_line":"        super(ServerRebuildTestCase, self).setUp()"},{"line_number":1078,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_4e0e50e6","line":1075,"in_reply_to":"1f485f77_ea5508fb","updated":"2017-11-20 14:49:13.000000000","message":"Yep, I mentioned adding that check on Friday for this reason. With two computes, removing this means we could get either one of them and I think we\u0027ll cast to the wrong compute, probably confusing it about us requesting a rebuild of an instance it doesn\u0027t have and recreate\u003dFalse.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6fea1e802ca60b6c53bdacec9180e7579aca17bc","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"        filters \u003d CONF.filter_scheduler.enabled_filters"},{"line_number":1073,"context_line":"        # We need SameHostFilter for the hint to work for validating our"},{"line_number":1074,"context_line":"        # existing host"},{"line_number":1075,"context_line":"        filters.append(\u0027SameHostFilter\u0027)"},{"line_number":1076,"context_line":"        self.flags(enabled_filters\u003dfilters, group\u003d\u0027filter_scheduler\u0027)"},{"line_number":1077,"context_line":"        super(ServerRebuildTestCase, self).setUp()"},{"line_number":1078,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_9026b6b7","line":1075,"in_reply_to":"ff82abbf_4e0e50e6","updated":"2017-11-20 15:14:38.000000000","message":"OK, maybe we should make this per-test and restart the scheduler service to apply the filters change? Were you going to have a test that doesn\u0027t enable the SameHostFilter, weighs the hosts such that the host that the instance is not running on gets picked, and we fail in the conductor if the selected host !\u003d instance.host in the recreate\u003dFalse case?\n\nCode-wise I think that should be pretty simple in conductor, it\u0027s just sounds more confusing when I say it.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"5bf5d79cee820903c64df83b7bf0bbf55092e769","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"        filters \u003d CONF.filter_scheduler.enabled_filters"},{"line_number":1073,"context_line":"        # We need SameHostFilter for the hint to work for validating our"},{"line_number":1074,"context_line":"        # existing host"},{"line_number":1075,"context_line":"        filters.append(\u0027SameHostFilter\u0027)"},{"line_number":1076,"context_line":"        self.flags(enabled_filters\u003dfilters, group\u003d\u0027filter_scheduler\u0027)"},{"line_number":1077,"context_line":"        super(ServerRebuildTestCase, self).setUp()"},{"line_number":1078,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_f0be22fe","line":1075,"in_reply_to":"ff82abbf_9026b6b7","updated":"2017-11-20 15:19:46.000000000","message":"It\u0027s hard to validate that it\u0027s doing one thing over the other in a functional test unfortunately. But yeah, really I need to validate that it does the right thing (i.e. 200), and that it fails in both situations we\u0027re checking for. Perhaps I should write a fake driver subclass to help validate that the right compute got the request in the situations we expect, but that\u0027ll just make this longer.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"98ec32269cef051c5c2a5b44042516fbae51a5c8","unresolved":false,"context_lines":[{"line_number":1072,"context_line":"        filters \u003d CONF.filter_scheduler.enabled_filters"},{"line_number":1073,"context_line":"        # We need SameHostFilter for the hint to work for validating our"},{"line_number":1074,"context_line":"        # existing host"},{"line_number":1075,"context_line":"        filters.append(\u0027SameHostFilter\u0027)"},{"line_number":1076,"context_line":"        self.flags(enabled_filters\u003dfilters, group\u003d\u0027filter_scheduler\u0027)"},{"line_number":1077,"context_line":"        super(ServerRebuildTestCase, self).setUp()"},{"line_number":1078,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"ff82abbf_d0d75e23","line":1075,"in_reply_to":"ff82abbf_f0be22fe","updated":"2017-11-20 15:24:26.000000000","message":"So actually, I think it\u0027s not so simple.\n\nIf we assert that host\u003d\u003dinstance.host after the reschedule, then we\u0027ll (likely) break all rebuilds immediately if people don\u0027t have SameHost enabled. Because a rebuild will almost definitely choose another host, we\u0027ll freak out and cancel the rebuild. Effectively, we\u0027d force everyone that wants rebuild to work to have SameHost enabled.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"89ff3ae7ff330fafce5ee2b9e70442d8c5f97a42","unresolved":false,"context_lines":[{"line_number":1116,"context_line":""},{"line_number":1117,"context_line":"        # Disable the host we\u0027re on so ComputeFilter would have ruled it out"},{"line_number":1118,"context_line":"        # normally"},{"line_number":1119,"context_line":"        self._disable_compute_for(server)"},{"line_number":1120,"context_line":""},{"line_number":1121,"context_line":"        # Now update the image metadata to be something that won\u0027t work with"},{"line_number":1122,"context_line":"        # the fake compute driver we\u0027re using since the fake driver has an"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f485f77_aab15049","line":1119,"updated":"2017-11-18 21:15:23.000000000","message":"This does sort of make it hard to know if we failed the rebuild with NoValidHost because the ComputeFilter kicked it out or if because the ImagePropertiesFilter kicked it out (as the test originally did). I can\u0027t think of good ways to assert which filter kicked it out though.","commit_id":"c7f82c084e129ccd188253b599192b1b45d3a05b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8137f17e65b63b24ab2ad414f76c2e5acfb6160b","unresolved":false,"context_lines":[{"line_number":1068,"context_line":"    # can use to update image metadata via our compute images proxy API."},{"line_number":1069,"context_line":"    microversion \u003d \u00272.38\u0027"},{"line_number":1070,"context_line":""},{"line_number":1071,"context_line":"    def setUp(self):"},{"line_number":1072,"context_line":"        filters \u003d CONF.filter_scheduler.enabled_filters"},{"line_number":1073,"context_line":"        self.flags(enabled_filters\u003dfilters, group\u003d\u0027filter_scheduler\u0027)"},{"line_number":1074,"context_line":"        super(ServerRebuildTestCase, self).setUp()"},{"line_number":1075,"context_line":""},{"line_number":1076,"context_line":"    def _disable_compute_for(self, server):"},{"line_number":1077,"context_line":"        # Refresh to get its host"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff82abbf_4d4acc27","line":1074,"range":{"start_line":1071,"start_character":4,"end_line":1074,"end_character":50},"updated":"2017-11-20 21:58:54.000000000","message":"We can remove this now.","commit_id":"71896898ae33290a536e992fa1db7fab1a0600ff"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"62bf853efcf2b07bead057f58982d4b37d7317cc","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":49,"context_line":"CONF \u003d cfg.CONF"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class ServersTestBase(integrated_helpers._IntegratedTestBase):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_ecd94d83","line":49,"updated":"2017-11-21 15:17:40.000000000","message":"nit: we don\u0027t need this anymore either","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"37a1c87a79062d3395049865fff3b6854d5ae1ec","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":49,"context_line":"CONF \u003d cfg.CONF"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"class ServersTestBase(integrated_helpers._IntegratedTestBase):"}],"source_content_type":"text/x-python","patch_set":5,"id":"ff82abbf_ec7aad22","line":49,"in_reply_to":"ff82abbf_ecd94d83","updated":"2017-11-21 15:25:33.000000000","message":"Sorry, I would have expected pep8 to catch this. I assume you have the lock for this now anyway so I\u0027ll let you hit it when you do the reno.","commit_id":"0a61ba4c991a64e48dedd22c774469aae61ebcfa"}]}