)]}'
{"nova/scheduler/client/report.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e5766704bed6ad1a330c67e5da7f498e843c6bbc","unresolved":false,"context_lines":[{"line_number":902,"context_line":"        \"\"\""},{"line_number":903,"context_line":"        url \u003d \u0027/allocations/%s\u0027 % consumer_uuid"},{"line_number":904,"context_line":"        payload \u003d copy.deepcopy(alloc_request)"},{"line_number":905,"context_line":"        payload[\u0027project_id\u0027] \u003d project_id"},{"line_number":906,"context_line":"        payload[\u0027user_id\u0027] \u003d user_id"},{"line_number":907,"context_line":"        r \u003d self.put(url, payload, version\u003d\u00271.10\u0027)"},{"line_number":908,"context_line":"        if r.status_code !\u003d 204:"},{"line_number":909,"context_line":"            LOG.warning("},{"line_number":910,"context_line":"                \u0027Unable to submit allocation for instance \u0027"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_86a9cc91","line":907,"range":{"start_line":905,"start_character":8,"end_line":907,"end_character":50},"updated":"2017-07-07 20:52:34.000000000","message":"OK project_id and user_id were required for PUT /allocations in 1.8, so we could just use 1.8, but technically if we are here, we got the allocation request candidates, which was 1.10, so requesting with 1.10 is also fine.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"}],"nova/scheduler/host_manager.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"13c0508f9c6b30d35abf91c98cce18a61e2e8ac1","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        return _locked_update(self, compute, service, aggregates, inst_dict)"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def _update_from_compute_node(self, compute):"},{"line_number":187,"context_line":"        self.uuid \u003d compute.uuid"},{"line_number":188,"context_line":"        \"\"\"Update information about a host from a ComputeNode object.\"\"\""},{"line_number":189,"context_line":"        # NOTE(jichenjc): if the compute record is just created but not updated"},{"line_number":190,"context_line":"        # some field such as free_disk_gb can be None"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f1d235d_ea113aae","line":187,"updated":"2017-07-10 17:23:41.000000000","message":"This kinda looks like it was supposed to be part of something else. If not, we need a test for this right? Or am I missing it?","commit_id":"ecd2da72d9fa374d9fc3e00bb763b357a3f2df09"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"689a4a17af3b65ee4ce6bdf05782db3d20a712c0","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        return _locked_update(self, compute, service, aggregates, inst_dict)"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def _update_from_compute_node(self, compute):"},{"line_number":187,"context_line":"        self.uuid \u003d compute.uuid"},{"line_number":188,"context_line":"        \"\"\"Update information about a host from a ComputeNode object.\"\"\""},{"line_number":189,"context_line":"        # NOTE(jichenjc): if the compute record is just created but not updated"},{"line_number":190,"context_line":"        # some field such as free_disk_gb can be None"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f1d235d_e52bab2d","line":187,"in_reply_to":"3f1d235d_ea113aae","updated":"2017-07-10 17:54:19.000000000","message":"I will remove this. It duplicates line 201.","commit_id":"ecd2da72d9fa374d9fc3e00bb763b357a3f2df09"}],"nova/scheduler/manager.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"06ae6524fac83ede33395ec239754a4097c637f3","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                # NOTE(jaypipes): We just pick the first allocation request"},{"line_number":116,"context_line":"                # that includes the compute node UUID as a resource provider"},{"line_number":117,"context_line":"                # UUID. In the future, we can possibly make this \"selection\""},{"line_number":118,"context_line":"                # strategy configurable?"},{"line_number":119,"context_line":"                LOG.debug(\"Finding first allocation request containing \""},{"line_number":120,"context_line":"                          \"resource provider %s\", cn_uuid)"},{"line_number":121,"context_line":"                ar_match \u003d None"}],"source_content_type":"text/x-python","patch_set":16,"id":"3f1d235d_3653746d","line":118,"updated":"2017-07-06 18:25:01.000000000","message":"alloc_reqs is still the same list we got during GET /allocation_candidates, yeah? If so, perhaps the selection strategy is some form of sorting that happens during that request?","commit_id":"bc33b1ee8551ed7189562382018b86bd8a086f77"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"7d3342c853ab874166539f6967dba465d5dd5866","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                    LOG.warning(\"You are using the chance scheduler. Cannot \""},{"line_number":112,"context_line":"                                \"claim resources in the scheduler.\")"},{"line_number":113,"context_line":"                    return False"},{"line_number":114,"context_line":"                # NOTE(jaypipes): We just pick the first allocation request"},{"line_number":115,"context_line":"                # that includes the compute node UUID as a resource provider"},{"line_number":116,"context_line":"                # UUID. In the future, we can possibly make this \"selection\""},{"line_number":117,"context_line":"                # strategy configurable?"},{"line_number":118,"context_line":"                LOG.debug(\"Finding first allocation request containing \""},{"line_number":119,"context_line":"                          \"resource provider %s\", cn_uuid)"},{"line_number":120,"context_line":"                ar_match \u003d None"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f1d235d_ef005aae","line":117,"range":{"start_line":114,"start_character":0,"end_line":117,"end_character":40},"updated":"2017-07-07 18:21:37.000000000","message":"Now that I\u0027m reading this, I think the previous patch in this set is wrong. The entire concern that led to the creation of the allocation_candidate scheme was for complex RPs. If the scheduler driver selected a host based on, say, a particular SR-IOV (public net vs. private), we didn\u0027t have a way for placement to know which SR-IOV to allocate against. So when we pick a host, we need to return the matching allocation_request dict along with that host, since a host with complex sub-resources could have many potential allocations. If you\u0027re simply picking one allocation_request (among many) for that host, you\u0027ve negated the whole reason for doing this.","commit_id":"0edf396a79d54dde8a694c85a3a05588d7d87983"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3a892ba5842a345da87ecf4066597ba46cdb83dd","unresolved":false,"context_lines":[{"line_number":111,"context_line":"                    LOG.warning(\"You are using the chance scheduler. Cannot \""},{"line_number":112,"context_line":"                                \"claim resources in the scheduler.\")"},{"line_number":113,"context_line":"                    return False"},{"line_number":114,"context_line":"                # NOTE(jaypipes): We just pick the first allocation request"},{"line_number":115,"context_line":"                # that includes the compute node UUID as a resource provider"},{"line_number":116,"context_line":"                # UUID. In the future, we can possibly make this \"selection\""},{"line_number":117,"context_line":"                # strategy configurable?"},{"line_number":118,"context_line":"                LOG.debug(\"Finding first allocation request containing \""},{"line_number":119,"context_line":"                          \"resource provider %s\", cn_uuid)"},{"line_number":120,"context_line":"                ar_match \u003d None"}],"source_content_type":"text/x-python","patch_set":18,"id":"3f1d235d_6ffe8a6c","line":117,"range":{"start_line":114,"start_character":0,"end_line":117,"end_character":40},"in_reply_to":"3f1d235d_ef005aae","updated":"2017-07-07 18:31:00.000000000","message":"This is merely matching the existing behaviour of the scheduler. When placement returns information about nested providers, we will need to modify select_destinations() to select one of the nested providers (things like SR-IOV VFs) in addition to selecting the target compute host.","commit_id":"0edf396a79d54dde8a694c85a3a05588d7d87983"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e5766704bed6ad1a330c67e5da7f498e843c6bbc","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def _raise_mismatch_no_valid_hosts():"},{"line_number":55,"context_line":"    LOG.warning(\"We selected a compute host that was not found \""},{"line_number":56,"context_line":"                \"in the list of allocation requests. This really \""},{"line_number":57,"context_line":"                \"should not be possible.\")"},{"line_number":58,"context_line":"    msg \u003d _(\"Mismatch between allocation candidates and scheduler\")"},{"line_number":59,"context_line":"    raise exception.NoValidHost(msg)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_c6e7c446","line":57,"range":{"start_line":56,"start_character":53,"end_line":57,"end_character":40},"updated":"2017-07-07 20:52:34.000000000","message":"Not sure we need to say this, unless we\u0027re going to provide some direction on things the operator should investigate.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e5766704bed6ad1a330c67e5da7f498e843c6bbc","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            dest \u003d dests[idx]"},{"line_number":109,"context_line":"            cn_uuid \u003d dest.uuid"},{"line_number":110,"context_line":"            if cn_uuid is None:"},{"line_number":111,"context_line":"                LOG.warning(\"You are using the chance scheduler. Cannot \""},{"line_number":112,"context_line":"                            \"claim resources in the scheduler.\")"},{"line_number":113,"context_line":"                return False"},{"line_number":114,"context_line":"            # NOTE(jaypipes): We just pick the first allocation request"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_46dbb47c","line":111,"range":{"start_line":111,"start_character":47,"end_line":111,"end_character":63},"updated":"2017-07-07 20:52:34.000000000","message":"Rather than assume this, wouldn\u0027t it be better to check the driver.USES_ALLOCATION_CANDIDATES flag? Actually, we shouldn\u0027t even call this method if driver.USES_ALLOCATION_CANDIDATES is False.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d9b424791645297c00c705ede79344c51e7f468c","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            dest \u003d dests[idx]"},{"line_number":109,"context_line":"            cn_uuid \u003d dest.uuid"},{"line_number":110,"context_line":"            if cn_uuid is None:"},{"line_number":111,"context_line":"                LOG.warning(\"You are using the chance scheduler. Cannot \""},{"line_number":112,"context_line":"                            \"claim resources in the scheduler.\")"},{"line_number":113,"context_line":"                return False"},{"line_number":114,"context_line":"            # NOTE(jaypipes): We just pick the first allocation request"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_de3ac7cb","line":111,"range":{"start_line":111,"start_character":47,"end_line":111,"end_character":63},"in_reply_to":"3f1d235d_46dbb47c","updated":"2017-07-10 14:20:56.000000000","message":"Heh, yes, this was done before that USES_ALLOCATION_CANDIDATES thing was added. I will remove it now because as you say, the code cannot reach here.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e5766704bed6ad1a330c67e5da7f498e843c6bbc","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                        ar_match \u003d ar"},{"line_number":125,"context_line":"                        break"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            if not ar_match:"},{"line_number":128,"context_line":"                _raise_mismatch_no_valid_hosts()"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"            LOG.debug(\"Attempting claim for consumer %s using allocation \""},{"line_number":131,"context_line":"                      \"request: %s\", consumer_uuid, ar_match)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_86b42cf4","line":128,"range":{"start_line":127,"start_character":12,"end_line":128,"end_character":48},"updated":"2017-07-07 20:52:34.000000000","message":"If you raise NoValidHost here, shouldn\u0027t we undo any claims on previously processed destinations?","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9d025252bfa024f7489e1b07b929d9c88df4d14d","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                        ar_match \u003d ar"},{"line_number":125,"context_line":"                        break"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            if not ar_match:"},{"line_number":128,"context_line":"                _raise_mismatch_no_valid_hosts()"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"            LOG.debug(\"Attempting claim for consumer %s using allocation \""},{"line_number":131,"context_line":"                      \"request: %s\", consumer_uuid, ar_match)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_1e139f68","line":128,"range":{"start_line":127,"start_character":12,"end_line":128,"end_character":48},"in_reply_to":"3f1d235d_1e595ffd","updated":"2017-07-10 14:38:36.000000000","message":"My fear is this case, someone does a nova boot with --min-count 100 and we get to request 100 and novalidhost, and don\u0027t cleanup the first 99 allocations we created.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d9b424791645297c00c705ede79344c51e7f468c","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                        ar_match \u003d ar"},{"line_number":125,"context_line":"                        break"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            if not ar_match:"},{"line_number":128,"context_line":"                _raise_mismatch_no_valid_hosts()"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"            LOG.debug(\"Attempting claim for consumer %s using allocation \""},{"line_number":131,"context_line":"                      \"request: %s\", consumer_uuid, ar_match)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_1e595ffd","line":128,"range":{"start_line":127,"start_character":12,"end_line":128,"end_character":48},"in_reply_to":"3f1d235d_86b42cf4","updated":"2017-07-10 14:20:56.000000000","message":"Yeah, so I was unsure how to handle this. I can certainly attempt to undo the claims by deleting any that succeeded if that\u0027s what you think is appropriate... the claims would get removed by the periodic job on the compute node if we let this code be as-is.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e5766704bed6ad1a330c67e5da7f498e843c6bbc","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                      \"request: %s\", consumer_uuid, ar_match)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"            pc \u003d self.placement_client"},{"line_number":134,"context_line":"            claim_success \u003d pc.claim_resources("},{"line_number":135,"context_line":"                    consumer_uuid, ar_match, project_id, user_id)"},{"line_number":136,"context_line":"            all_claims_success \u0026\u003d claim_success"},{"line_number":137,"context_line":"            if claim_success:"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_c65ea4e8","line":134,"range":{"start_line":134,"start_character":28,"end_line":134,"end_character":30},"updated":"2017-07-07 20:52:34.000000000","message":"This could just be self.placement_client.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d9b424791645297c00c705ede79344c51e7f468c","unresolved":false,"context_lines":[{"line_number":131,"context_line":"                      \"request: %s\", consumer_uuid, ar_match)"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"            pc \u003d self.placement_client"},{"line_number":134,"context_line":"            claim_success \u003d pc.claim_resources("},{"line_number":135,"context_line":"                    consumer_uuid, ar_match, project_id, user_id)"},{"line_number":136,"context_line":"            all_claims_success \u0026\u003d claim_success"},{"line_number":137,"context_line":"            if claim_success:"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_5e36b7b6","line":134,"range":{"start_line":134,"start_character":28,"end_line":134,"end_character":30},"in_reply_to":"3f1d235d_c65ea4e8","updated":"2017-07-10 14:20:56.000000000","message":"Done","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e5766704bed6ad1a330c67e5da7f498e843c6bbc","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                            \"allocation request: %s.\","},{"line_number":140,"context_line":"                            consumer_uuid, ar_match)"},{"line_number":141,"context_line":"            else:"},{"line_number":142,"context_line":"                LOG.info(\"Successfully claimed resources for %s using \""},{"line_number":143,"context_line":"                         \"allocation request %s\", consumer_uuid, ar_match)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        return all_claims_success"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_266f9893","line":142,"range":{"start_line":142,"start_character":20,"end_line":142,"end_character":24},"updated":"2017-07-07 20:52:34.000000000","message":"debug? I thought we always tried to be very light on logging in the scheduler b/c of speed.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d9b424791645297c00c705ede79344c51e7f468c","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                            \"allocation request: %s.\","},{"line_number":140,"context_line":"                            consumer_uuid, ar_match)"},{"line_number":141,"context_line":"            else:"},{"line_number":142,"context_line":"                LOG.info(\"Successfully claimed resources for %s using \""},{"line_number":143,"context_line":"                         \"allocation request %s\", consumer_uuid, ar_match)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        return all_claims_success"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_7ebbb30d","line":142,"range":{"start_line":142,"start_character":20,"end_line":142,"end_character":24},"in_reply_to":"3f1d235d_266f9893","updated":"2017-07-10 14:20:56.000000000","message":"Oh? I wasn\u0027t aware of that. I will change it.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e0c678792f918b796284021acfe3cc8f41fef665","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        dests \u003d self.driver.select_destinations(ctxt, spec_obj, instance_uuids,"},{"line_number":182,"context_line":"            p_sums)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        # NOTE(jaypipes): OK, so the scheduler picked one or more hosts. Let\u0027s"},{"line_number":185,"context_line":"        # attempt to claim resources for each instance."},{"line_number":186,"context_line":"        if alloc_reqs and instance_uuids:"},{"line_number":187,"context_line":"            project_id \u003d spec_obj.project_id"},{"line_number":188,"context_line":"            # NOTE(jaypipes): So, the RequestSpec doesn\u0027t store the user_id,"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_a62e08f8","line":185,"range":{"start_line":184,"start_character":8,"end_line":185,"end_character":55},"updated":"2017-07-07 20:54:54.000000000","message":"We can\u0027t do any of this until the minimum compute version is new enough to handle allocations being made in the scheduler, right? Because I thought the plan was for the computes to check if the allocations were already made for them, and if so, they don\u0027t claim - but here we\u0027re making the allocation in the scheduler *and* still going to claim in the compute.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d9b424791645297c00c705ede79344c51e7f468c","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        dests \u003d self.driver.select_destinations(ctxt, spec_obj, instance_uuids,"},{"line_number":182,"context_line":"            p_sums)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        # NOTE(jaypipes): OK, so the scheduler picked one or more hosts. Let\u0027s"},{"line_number":185,"context_line":"        # attempt to claim resources for each instance."},{"line_number":186,"context_line":"        if alloc_reqs and instance_uuids:"},{"line_number":187,"context_line":"            project_id \u003d spec_obj.project_id"},{"line_number":188,"context_line":"            # NOTE(jaypipes): So, the RequestSpec doesn\u0027t store the user_id,"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_fecfc3ad","line":185,"range":{"start_line":184,"start_character":8,"end_line":185,"end_character":55},"in_reply_to":"3f1d235d_a62e08f8","updated":"2017-07-10 14:20:56.000000000","message":"See Dan\u0027s comment. We already check that on the compute node and will not overwrite any allocation into the placement API.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1efc0540e6736056da0b8e27cbd7d8c6b121c204","unresolved":false,"context_lines":[{"line_number":181,"context_line":"        dests \u003d self.driver.select_destinations(ctxt, spec_obj, instance_uuids,"},{"line_number":182,"context_line":"            p_sums)"},{"line_number":183,"context_line":""},{"line_number":184,"context_line":"        # NOTE(jaypipes): OK, so the scheduler picked one or more hosts. Let\u0027s"},{"line_number":185,"context_line":"        # attempt to claim resources for each instance."},{"line_number":186,"context_line":"        if alloc_reqs and instance_uuids:"},{"line_number":187,"context_line":"            project_id \u003d spec_obj.project_id"},{"line_number":188,"context_line":"            # NOTE(jaypipes): So, the RequestSpec doesn\u0027t store the user_id,"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_feea23aa","line":185,"range":{"start_line":184,"start_character":8,"end_line":185,"end_character":55},"in_reply_to":"3f1d235d_fecfc3ad","updated":"2017-07-10 14:28:16.000000000","message":"The spec and my original understanding of when we do the allocations in the scheduler was based on some idea that the compute would check if allocations were already done for it and if so, it wouldn\u0027t do the claim.\n\nWith this, at best we have a redundant claim and no changed allocations in the compute. At worst, something fails in the claim, like with the overhead calculation or PCI devices (or whatever other things we don\u0027t yet account for in the scheduler).\n\nSo I guess that\u0027s fine, but then shouldn\u0027t we also have the retry logic in this change that handles cleaning up the allocations and finding a new host in the compute fails? I think we said the retries would be handled in conductor since that\u0027s what the compute casts to for the retry.\n\nWe also have to sort out passing the alternate hosts to the compute for the retries.","commit_id":"72f225b5c4054320dfe14bf335cc990a715b1eab"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"13c0508f9c6b30d35abf91c98cce18a61e2e8ac1","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                for rr in ar[\u0027allocations\u0027]:"},{"line_number":123,"context_line":"                    if rr[\u0027resource_provider\u0027][\u0027uuid\u0027] \u003d\u003d cn_uuid:"},{"line_number":124,"context_line":"                        ar_match \u003d ar"},{"line_number":125,"context_line":"                        break"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            if not ar_match:"},{"line_number":128,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f1d235d_0a98f667","line":125,"updated":"2017-07-10 17:23:41.000000000","message":"If this is sorted wrong, this will be more expensive than necessary. Can\u0027t we build a mapping by uuid once before we do this loop and just index it here?","commit_id":"ecd2da72d9fa374d9fc3e00bb763b357a3f2df09"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"689a4a17af3b65ee4ce6bdf05782db3d20a712c0","unresolved":false,"context_lines":[{"line_number":122,"context_line":"                for rr in ar[\u0027allocations\u0027]:"},{"line_number":123,"context_line":"                    if rr[\u0027resource_provider\u0027][\u0027uuid\u0027] \u003d\u003d cn_uuid:"},{"line_number":124,"context_line":"                        ar_match \u003d ar"},{"line_number":125,"context_line":"                        break"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            if not ar_match:"},{"line_number":128,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"}],"source_content_type":"text/x-python","patch_set":20,"id":"3f1d235d_856cf79c","line":125,"in_reply_to":"3f1d235d_0a98f667","updated":"2017-07-10 17:54:19.000000000","message":"Done","commit_id":"ecd2da72d9fa374d9fc3e00bb763b357a3f2df09"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"fd18287dd9620993e7106acbcf0d185a22d34375","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            return"},{"line_number":104,"context_line":"        LOG.debug(\"Cleaning up allocations for %s\", instance_uuids)"},{"line_number":105,"context_line":"        for uuid in instance_uuids:"},{"line_number":106,"context_line":"            self.placement_client.delete_allocations_for_instance(uuid)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def _claim_resources(self, alloc_reqs, instance_uuids, dests, project_id,"},{"line_number":109,"context_line":"                         user_id):"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f1d235d_3bbd4268","line":106,"range":{"start_line":106,"start_character":34,"end_line":106,"end_character":65},"updated":"2017-07-10 19:24:24.000000000","message":"This method doesn\u0027t exist, it\u0027s delete_allocation_for_instance - you must be missing a test.","commit_id":"5056a17f26edf76f8a962f97546e172ce304bc2b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"08857ecdbdd5548590da7395dafca8bbf92782d0","unresolved":false,"context_lines":[{"line_number":103,"context_line":"            return"},{"line_number":104,"context_line":"        LOG.debug(\"Cleaning up allocations for %s\", instance_uuids)"},{"line_number":105,"context_line":"        for uuid in instance_uuids:"},{"line_number":106,"context_line":"            self.placement_client.delete_allocations_for_instance(uuid)"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def _claim_resources(self, alloc_reqs, instance_uuids, dests, project_id,"},{"line_number":109,"context_line":"                         user_id):"}],"source_content_type":"text/x-python","patch_set":21,"id":"3f1d235d_5b7c7eb4","line":106,"range":{"start_line":106,"start_character":34,"end_line":106,"end_character":65},"in_reply_to":"3f1d235d_3bbd4268","updated":"2017-07-10 19:26:13.000000000","message":"No, not missing a test. Just an unfortunate similar typo in the unit tests I added.. :( Will fix.","commit_id":"5056a17f26edf76f8a962f97546e172ce304bc2b"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54b16d3e9530e733e6b47fc466fac719549169de","unresolved":false,"context_lines":[{"line_number":121,"context_line":"            LOG.debug(\"Finding first allocation request containing \""},{"line_number":122,"context_line":"                      \"resource provider %s\", cn_uuid)"},{"line_number":123,"context_line":"            if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":124,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":125,"context_line":"                _raise_mismatch_no_valid_hosts()"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            # TODO(jaypipes): We just pick the first allocation request"},{"line_number":128,"context_line":"            # that includes the compute node UUID as a resource provider"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_d6b503cf","line":125,"range":{"start_line":124,"start_character":16,"end_line":125,"end_character":48},"updated":"2017-07-10 19:52:11.000000000","message":"Might be good to log something here? You were logging that as a warning before:\n\nLOG.warning(\"We selected a compute host that was not found \"\n                \"in the list of allocation requests.\n\nI think we could still log that with the CN uuid in the message for context.","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"05c3a3b4dd9f4f947bcd2e6675dfd2fe8e194b9f","unresolved":false,"context_lines":[{"line_number":121,"context_line":"            LOG.debug(\"Finding first allocation request containing \""},{"line_number":122,"context_line":"                      \"resource provider %s\", cn_uuid)"},{"line_number":123,"context_line":"            if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":124,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":125,"context_line":"                _raise_mismatch_no_valid_hosts()"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            # TODO(jaypipes): We just pick the first allocation request"},{"line_number":128,"context_line":"            # that includes the compute node UUID as a resource provider"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_d67063df","line":125,"range":{"start_line":124,"start_character":16,"end_line":125,"end_character":48},"in_reply_to":"3f1d235d_d65f4370","updated":"2017-07-10 19:59:47.000000000","message":"Yeah, the \"this shouldn\u0027t happen\" was weird - if we\u0027re going to say something shouldn\u0027t happen, I\u0027d think we should mention how it could happen and what should be investigated. If we don\u0027t know that, let\u0027s just not say it shouldn\u0027t happen. :)","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"896f4e58b8cd00f58b717ebe9de75864a0c72d26","unresolved":false,"context_lines":[{"line_number":121,"context_line":"            LOG.debug(\"Finding first allocation request containing \""},{"line_number":122,"context_line":"                      \"resource provider %s\", cn_uuid)"},{"line_number":123,"context_line":"            if cn_uuid not in alloc_reqs_by_rp_uuid:"},{"line_number":124,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":125,"context_line":"                _raise_mismatch_no_valid_hosts()"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            # TODO(jaypipes): We just pick the first allocation request"},{"line_number":128,"context_line":"            # that includes the compute node UUID as a resource provider"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_d65f4370","line":125,"range":{"start_line":124,"start_character":16,"end_line":125,"end_character":48},"in_reply_to":"3f1d235d_d6b503cf","updated":"2017-07-10 19:57:04.000000000","message":"ok, I\u0027ll put it back. I guess the part you didn\u0027t like was just the \"This shouldn\u0027t happen\" part?","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"10d60ff70cbaa44221543fde2adfbab614aa4984","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"},{"line_number":125,"context_line":"                _raise_mismatch_no_valid_hosts()"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"            # TODO(jaypipes): We just pick the first allocation request"},{"line_number":128,"context_line":"            # that includes the compute node UUID as a resource provider"},{"line_number":129,"context_line":"            # UUID. In the future, we can possibly make this \"selection\""},{"line_number":130,"context_line":"            # strategy configurable?"},{"line_number":131,"context_line":"            ar_match \u003d alloc_reqs_by_rp_uuid[cn_uuid][0]"},{"line_number":132,"context_line":"            LOG.debug(\"Attempting claim for consumer %s using allocation \""},{"line_number":133,"context_line":"                      \"request: %s\", consumer_uuid, ar_match)"},{"line_number":134,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_b654c755","line":131,"range":{"start_line":127,"start_character":12,"end_line":131,"end_character":56},"updated":"2017-07-10 19:55:48.000000000","message":"This sounds like the host_subset_size config option a bit, but that\u0027s currently only used by the filter scheduler.","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54b16d3e9530e733e6b47fc466fac719549169de","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            claim_success \u003d self.placement_client.claim_resources("},{"line_number":136,"context_line":"                    consumer_uuid, ar_match, project_id, user_id)"},{"line_number":137,"context_line":"            if not claim_success:"},{"line_number":138,"context_line":"                LOG.warning(\"Failed to claim resources for %s using \""},{"line_number":139,"context_line":"                            \"allocation request: %s.\","},{"line_number":140,"context_line":"                            consumer_uuid, ar_match)"},{"line_number":141,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_f6335f61","line":138,"range":{"start_line":138,"start_character":20,"end_line":138,"end_character":27},"updated":"2017-07-10 19:52:11.000000000","message":"Should this be a warning or debug? Seems like this could be common if you\u0027ve got a busy cloud and we are always picking the first allocation request match above (as noted with the TODO), plus we aren\u0027t doing retries in here yet, right? So for example, if I do nova boot --flavor m1.large --min-count 50 and fill up the same compute node after the first let\u0027s say 20 instances, then we\u0027re going to be warning on the other 30 and kicking them out and failing with NoValidHost, right?","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"05c3a3b4dd9f4f947bcd2e6675dfd2fe8e194b9f","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            claim_success \u003d self.placement_client.claim_resources("},{"line_number":136,"context_line":"                    consumer_uuid, ar_match, project_id, user_id)"},{"line_number":137,"context_line":"            if not claim_success:"},{"line_number":138,"context_line":"                LOG.warning(\"Failed to claim resources for %s using \""},{"line_number":139,"context_line":"                            \"allocation request: %s.\","},{"line_number":140,"context_line":"                            consumer_uuid, ar_match)"},{"line_number":141,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_9c01ed07","line":138,"range":{"start_line":138,"start_character":20,"end_line":138,"end_character":27},"in_reply_to":"3f1d235d_96ff8b33","updated":"2017-07-10 19:59:47.000000000","message":"Yeah I don\u0027t mean we\u0027d log the warning for each instance we \"unclaim\", I meant we could see this a lot in a busy cloud - but then we\u0027d also be seeing NoValidHost warnings elsewhere probably which are just as bad - and we\u0027re seeing a lot of those, we\u0027re doing something wrong.","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"d2716580c934546388b06695b5ea67bc78f9dd5f","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            claim_success \u003d self.placement_client.claim_resources("},{"line_number":136,"context_line":"                    consumer_uuid, ar_match, project_id, user_id)"},{"line_number":137,"context_line":"            if not claim_success:"},{"line_number":138,"context_line":"                LOG.warning(\"Failed to claim resources for %s using \""},{"line_number":139,"context_line":"                            \"allocation request: %s.\","},{"line_number":140,"context_line":"                            consumer_uuid, ar_match)"},{"line_number":141,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_fce58139","line":138,"range":{"start_line":138,"start_character":20,"end_line":138,"end_character":27},"in_reply_to":"3f1d235d_9c01ed07","updated":"2017-07-10 20:05:23.000000000","message":"Sure, but the next patch in the series will add retries here, so ...","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"896f4e58b8cd00f58b717ebe9de75864a0c72d26","unresolved":false,"context_lines":[{"line_number":135,"context_line":"            claim_success \u003d self.placement_client.claim_resources("},{"line_number":136,"context_line":"                    consumer_uuid, ar_match, project_id, user_id)"},{"line_number":137,"context_line":"            if not claim_success:"},{"line_number":138,"context_line":"                LOG.warning(\"Failed to claim resources for %s using \""},{"line_number":139,"context_line":"                            \"allocation request: %s.\","},{"line_number":140,"context_line":"                            consumer_uuid, ar_match)"},{"line_number":141,"context_line":"                self._cleanup_allocations(claimed_instance_uuids)"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_96ff8b33","line":138,"range":{"start_line":138,"start_character":20,"end_line":138,"end_character":27},"in_reply_to":"3f1d235d_f6335f61","updated":"2017-07-10 19:57:04.000000000","message":"No, see the return False below? You\u0027d see a single WARNING.","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"}],"nova/tests/unit/scheduler/test_scheduler.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"54b16d3e9530e733e6b47fc466fac719549169de","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        fake_spec.instance_uuid \u003d uuids.instance"},{"line_number":389,"context_line":"        fake_spec.project_id \u003d uuids.project_id"},{"line_number":390,"context_line":"        from_primitives.return_value \u003d fake_spec"},{"line_number":391,"context_line":"        place_res \u003d (None, None)"},{"line_number":392,"context_line":"        mock_get_ac.return_value \u003d place_res"},{"line_number":393,"context_line":"        ctx \u003d mock.Mock(user_id\u003duuids.user_id)"},{"line_number":394,"context_line":"        with mock.patch.object(self.manager.driver, \u0027select_destinations\u0027"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_96230b06","line":391,"range":{"start_line":391,"start_character":8,"end_line":391,"end_character":32},"updated":"2017-07-10 19:52:11.000000000","message":"This seems unrelated. I\u0027m not entirely sure about the history on this test, but I think it means something along the boundary of passing a RequestSpec, rather than old placement client or something, and the old placement client is tested in test_select_destination_old_placement. Maybe I\u0027m just missing something here.","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ef552f1ec73cb8d90ae39e067ae5750323e83e20","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        fake_spec.instance_uuid \u003d uuids.instance"},{"line_number":389,"context_line":"        fake_spec.project_id \u003d uuids.project_id"},{"line_number":390,"context_line":"        from_primitives.return_value \u003d fake_spec"},{"line_number":391,"context_line":"        place_res \u003d (None, None)"},{"line_number":392,"context_line":"        mock_get_ac.return_value \u003d place_res"},{"line_number":393,"context_line":"        ctx \u003d mock.Mock(user_id\u003duuids.user_id)"},{"line_number":394,"context_line":"        with mock.patch.object(self.manager.driver, \u0027select_destinations\u0027"}],"source_content_type":"text/x-python","patch_set":22,"id":"3f1d235d_dc5d2516","line":391,"range":{"start_line":391,"start_character":8,"end_line":391,"end_character":32},"in_reply_to":"3f1d235d_96230b06","updated":"2017-07-10 20:04:35.000000000","message":"This was necessary because \"mock.sentinel.alloc_reqs\" is not None and is not iterable, which caused an issue when attempting to call the new _claim_resources() method. Setting the return from placement API to (None, None) ignores the _claim_resources() code paths, which isn\u0027t what\u0027s being tested in this unit test.","commit_id":"3305d80358f9cb98caf106600a3cae4a8e41d60a"}]}