)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"e075c69758f2a2ae362a3210e7d84263eb82c2f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d068bec9_4f36219e","updated":"2026-07-28 14:03:28.000000000","message":"this looks correct to me","commit_id":"361b8409d53c0b276fc297ebfbf3e7e3d755f3e1"}],"watcher/decision_engine/strategy/strategies/vm_workload_consolidation.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"2ad1ab0eefbf906ff1f5e2fec7c87427d7995f6e","unresolved":false,"context_lines":[{"line_number":566,"context_line":"                return True"},{"line_number":567,"context_line":"        return False"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    def is_node_saturated(self, node, cc):"},{"line_number":570,"context_line":"        \"\"\"Check if a node cannot accept any more instances."},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"        A node is saturated when its free allocation is exhausted or"}],"source_content_type":"text/x-python","patch_set":2,"id":"d285fe88_5eb949f4","line":569,"updated":"2026-06-24 13:24:06.000000000","message":"The rationale comment for the 128 MB memory buffer was dropped during the is_node_saturated() refactoring. The original inline code explained why 128 MB is subtracted (remaining memory may be less than the smallest flavor RAM), but the extracted method has no such explanation.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Future maintainers will not understand why the magic number 128 is used for RAM but 0 for CPU and disk. This knowledge is load-bearing: changing the buffer without understanding its purpose could cause the strategy to either miss saturated nodes or falsely mark nodes as saturated.\n\n**Suggestion**:\nAdd a comment in is_node_saturated() explaining the 128 MB buffer: \u0027For memory, remaining capacity may be less than the minimum flavor RAM allocation, so subtract a 128 MB buffer when detecting saturation.\u0027 This preserves the rationale that was in the original consolidation_phase code.","commit_id":"947ac9ed2d0d2ad864c4113b9f5424d395767d24"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"2ad1ab0eefbf906ff1f5e2fec7c87427d7995f6e","unresolved":false,"context_lines":[{"line_number":566,"context_line":"                return True"},{"line_number":567,"context_line":"        return False"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    def is_node_saturated(self, node, cc):"},{"line_number":570,"context_line":"        \"\"\"Check if a node cannot accept any more instances."},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"        A node is saturated when its free allocation is exhausted or"}],"source_content_type":"text/x-python","patch_set":2,"id":"b432c198_0b2a162f","line":569,"updated":"2026-06-24 13:24:06.000000000","message":"is_node_saturated() does both allocation and utilization checks. When called from instance_fits()\u0027s else branch, the allocation check duplicates work already done, and get_node_utilization() adds expensive per-instance metric lookups for every non-fitting destination.\n\n**Severity**: WARNING | **Confidence**: 0.7\n\n**Impact**: At scale with many destination nodes, this adds redundant computation per failed instance_fits() call. The get_free_node_allocation() call is cached so is O(1), but get_node_utilization() involves per-instance metric lookups that may not be fully cached for all nodes.\n\n**Suggestion**:\nSince is_node_saturated is only called when instance_fits already returned False, consider passing the allocation-free result from instance_fits to avoid re-querying, or document that the redundancy is intentional because the utilization check provides additional saturation signal beyond allocation.","commit_id":"947ac9ed2d0d2ad864c4113b9f5424d395767d24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"a237f6e4215c53ce151698d2415581f5d183cb83","unresolved":false,"context_lines":[{"line_number":566,"context_line":"                return True"},{"line_number":567,"context_line":"        return False"},{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    def is_node_saturated(self, node, cc):"},{"line_number":570,"context_line":"        \"\"\"Check if a node cannot accept any more instances."},{"line_number":571,"context_line":""},{"line_number":572,"context_line":"        A node is saturated when its free allocation is exhausted or"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a2a5ce7_fcd45ac1","line":569,"in_reply_to":"b432c198_0b2a162f","updated":"2026-06-30 15:06:08.000000000","message":"instance_fits referst to be destination of a specific vm while is_node_saturated means in its actual status. A node which does not fit for an instnce may be a good fit for others, so there is no duplication","commit_id":"947ac9ed2d0d2ad864c4113b9f5424d395767d24"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"2ad1ab0eefbf906ff1f5e2fec7c87427d7995f6e","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        :param cc: dictionary containing resource capacity coefficients"},{"line_number":576,"context_line":"        :return: True if saturated"},{"line_number":577,"context_line":"        \"\"\""},{"line_number":578,"context_line":"        resources_min \u003d {\u0027cpu\u0027: 0, \u0027ram\u0027: 128, \u0027disk\u0027: 0}"},{"line_number":579,"context_line":"        free \u003d self.get_free_node_allocation(node)"},{"line_number":580,"context_line":"        if ("},{"line_number":581,"context_line":"            free[\u0027vcpu\u0027] \u003c\u003d resources_min[\u0027cpu\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"18d62adc_dcc5998f","line":578,"updated":"2026-06-24 13:24:06.000000000","message":"The magic number 128 (MB memory buffer) and the resources_min dict are defined as a local variable inside is_node_saturated(), creating a new dict on every call. This same pattern was previously inline in consolidation_phase.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Extracting resources_min as a class-level constant (e.g., SATURATION_BUFFER) would improve readability, avoid repeated dict construction, and make it easier to tune the buffer value. It would also make the relationship between the allocation check and the utilization check clearer.\n\n**Recommendation**:\nDefine a class-level constant such as SATURATION_MEMORY_BUFFER_MB \u003d 128 or a dict _SATURATION_BUFFERS \u003d {\u0027cpu\u0027: 0, \u0027ram\u0027: 128, \u0027disk\u0027: 0} and reference it in is_node_saturated(). Add a docstring or comment explaining the 128 MB value.","commit_id":"947ac9ed2d0d2ad864c4113b9f5424d395767d24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"a237f6e4215c53ce151698d2415581f5d183cb83","unresolved":false,"context_lines":[{"line_number":575,"context_line":"        :param cc: dictionary containing resource capacity coefficients"},{"line_number":576,"context_line":"        :return: True if saturated"},{"line_number":577,"context_line":"        \"\"\""},{"line_number":578,"context_line":"        resources_min \u003d {\u0027cpu\u0027: 0, \u0027ram\u0027: 128, \u0027disk\u0027: 0}"},{"line_number":579,"context_line":"        free \u003d self.get_free_node_allocation(node)"},{"line_number":580,"context_line":"        if ("},{"line_number":581,"context_line":"            free[\u0027vcpu\u0027] \u003c\u003d resources_min[\u0027cpu\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"5c7481c5_f65ea5bd","line":578,"in_reply_to":"18d62adc_dcc5998f","updated":"2026-06-30 15:06:08.000000000","message":"i hope to have explained better in next ps","commit_id":"947ac9ed2d0d2ad864c4113b9f5424d395767d24"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"2ad1ab0eefbf906ff1f5e2fec7c87427d7995f6e","unresolved":false,"context_lines":[{"line_number":774,"context_line":"                            instance.uuid,"},{"line_number":775,"context_line":"                        )"},{"line_number":776,"context_line":"                        continue"},{"line_number":777,"context_line":"                    for destination_node in reversed(sorted_nodes):"},{"line_number":778,"context_line":"                        if destination_node.hostname in saturated_nodes:"},{"line_number":779,"context_line":"                            continue"},{"line_number":780,"context_line":"                        if self.instance_fits(instance, destination_node, cc):"}],"source_content_type":"text/x-python","patch_set":2,"id":"76a86252_59ae4d20","line":777,"updated":"2026-06-24 13:24:06.000000000","message":"In offload_phase, nodes added to saturated_nodes are never removed for the duration of the phase. A node marked saturated based on utilization for one large instance will be skipped for all subsequent smaller instances that may still fit, potentially leaving an overloaded node unrelieved.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: In clusters with mixed instance sizes, the offload phase may fail to find valid destinations for instances from overloaded nodes, leaving them overloaded. This matches the pre-existing consolidation_phase pattern but is newly introduced to offload_phase.\n\n**Suggestion**:\nConsider clearing saturated_nodes when moving to a new source instance, or only adding a node to saturated_nodes when the allocation check (not the utilization check) fails, since allocation is the tighter constraint that this patch adds.","commit_id":"947ac9ed2d0d2ad864c4113b9f5424d395767d24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"a237f6e4215c53ce151698d2415581f5d183cb83","unresolved":false,"context_lines":[{"line_number":774,"context_line":"                            instance.uuid,"},{"line_number":775,"context_line":"                        )"},{"line_number":776,"context_line":"                        continue"},{"line_number":777,"context_line":"                    for destination_node in reversed(sorted_nodes):"},{"line_number":778,"context_line":"                        if destination_node.hostname in saturated_nodes:"},{"line_number":779,"context_line":"                            continue"},{"line_number":780,"context_line":"                        if self.instance_fits(instance, destination_node, cc):"}],"source_content_type":"text/x-python","patch_set":2,"id":"6b6c02de_39811c8e","line":777,"in_reply_to":"76a86252_59ae4d20","updated":"2026-06-30 15:06:08.000000000","message":"\"A node marked saturated based on utilization for one large instance will be skipped for all subsequent smaller instances that may still fit, potentially leaving an overloaded node unrelieved.\"\n\nNot really, a node marked as saturated means in its current model state, without any additional vm, so it would be saturated for any new instance","commit_id":"947ac9ed2d0d2ad864c4113b9f5424d395767d24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"40d1cd495f95ac2d5c2d991b48305a4d389b9604","unresolved":true,"context_lines":[{"line_number":293,"context_line":"                    disk\u003ddest[\u0027disk\u0027] + instance_util[\u0027disk\u0027],"},{"line_number":294,"context_line":"                )"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"            if source_node.hostname in self.node_allocation_cache:"},{"line_number":297,"context_line":"                source \u003d self.node_allocation_cache[source_node.hostname]"},{"line_number":298,"context_line":"                self.node_allocation_cache[source_node.hostname] \u003d ("},{"line_number":299,"context_line":"                    source[0] - instance.vcpus,"}],"source_content_type":"text/x-python","patch_set":4,"id":"e289d4c2_2d599bf3","line":296,"range":{"start_line":296,"start_character":44,"end_line":296,"end_character":65},"updated":"2026-07-20 18:06:41.000000000","message":"I have another idea. Include the allocation cache direct in the model. Today we use its migrate_instance just to update instance mapping. There are methods that already calculate used_resources and free_resources, but don\u0027t cache these values. migrate_instance could update the resource allocation in the cache if it exists. Make sense?","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"559bba85e4c2ae9bd2d64f165f044defaa30344e","unresolved":false,"context_lines":[{"line_number":293,"context_line":"                    disk\u003ddest[\u0027disk\u0027] + instance_util[\u0027disk\u0027],"},{"line_number":294,"context_line":"                )"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"            if source_node.hostname in self.node_allocation_cache:"},{"line_number":297,"context_line":"                source \u003d self.node_allocation_cache[source_node.hostname]"},{"line_number":298,"context_line":"                self.node_allocation_cache[source_node.hostname] \u003d ("},{"line_number":299,"context_line":"                    source[0] - instance.vcpus,"}],"source_content_type":"text/x-python","patch_set":4,"id":"fc9b2ac5_14b1ed56","line":296,"updated":"2026-06-30 15:49:14.000000000","message":"The allocation cache is updated in add_migration() and mirrored in optimize_solution() by directly indexing tuple positions (source[0], source[1], source[2]), whereas the sibling node_utilization_cache uses named dict keys (cpu/ram/disk). The two parallel update blocks are now duplicated across add_migration and optimize_solution with the tuple-index pattern, which is error-prone: the meaning of indices [0]/[1]/[2] is undocumented and must stay in lockstep with get_free_node_allocation\u0027s seeding order (vcpu, memory, disk).\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: A future maintainer reordering the tuple or adding a fourth resource has no compiler/linter guard; an index swap (e.g. swapping memory and disk) would silently produce wrong capacity checks only detectable at runtime. The utilization cache uses dict keys precisely to avoid this class of bug.\n\n**Suggestion**:\nStore the allocation cache as a dict(vcpu\u003d..., memory\u003d..., disk\u003d...) keyed the same way the utilization cache is, instead of a positional tuple. This removes the four hand-indexed tuple updates (in add_migration and optimize_solution) and makes the two caches structurally symmetric, reducing the risk of index drift.","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"e075c69758f2a2ae362a3210e7d84263eb82c2f6","unresolved":false,"context_lines":[{"line_number":293,"context_line":"                    disk\u003ddest[\u0027disk\u0027] + instance_util[\u0027disk\u0027],"},{"line_number":294,"context_line":"                )"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"            if source_node.hostname in self.node_allocation_cache:"},{"line_number":297,"context_line":"                source \u003d self.node_allocation_cache[source_node.hostname]"},{"line_number":298,"context_line":"                self.node_allocation_cache[source_node.hostname] \u003d ("},{"line_number":299,"context_line":"                    source[0] - instance.vcpus,"}],"source_content_type":"text/x-python","patch_set":4,"id":"b6b12ecb_54b35700","line":296,"in_reply_to":"c512ab55_7124d8ae","updated":"2026-07-28 14:03:28.000000000","message":"Done","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"9003aa44fa8053a70bb7fd911bfc82133dcad721","unresolved":true,"context_lines":[{"line_number":293,"context_line":"                    disk\u003ddest[\u0027disk\u0027] + instance_util[\u0027disk\u0027],"},{"line_number":294,"context_line":"                )"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"            if source_node.hostname in self.node_allocation_cache:"},{"line_number":297,"context_line":"                source \u003d self.node_allocation_cache[source_node.hostname]"},{"line_number":298,"context_line":"                self.node_allocation_cache[source_node.hostname] \u003d ("},{"line_number":299,"context_line":"                    source[0] - instance.vcpus,"}],"source_content_type":"text/x-python","patch_set":4,"id":"8d242dec_5f96374d","line":296,"range":{"start_line":296,"start_character":44,"end_line":296,"end_character":65},"in_reply_to":"e289d4c2_2d599bf3","updated":"2026-07-23 12:56:52.000000000","message":"Yet, that would work. So we may use the get_node_free_resources method in the model, adding the caching feature there and adding the logic of maintaining the cache updated in the migrate_instance as you said. That should work. I will test it and update this review. That may also improve performance on other strategies.","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"eb0af4d178d4d887116007b4868476c30db811f9","unresolved":true,"context_lines":[{"line_number":293,"context_line":"                    disk\u003ddest[\u0027disk\u0027] + instance_util[\u0027disk\u0027],"},{"line_number":294,"context_line":"                )"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"            if source_node.hostname in self.node_allocation_cache:"},{"line_number":297,"context_line":"                source \u003d self.node_allocation_cache[source_node.hostname]"},{"line_number":298,"context_line":"                self.node_allocation_cache[source_node.hostname] \u003d ("},{"line_number":299,"context_line":"                    source[0] - instance.vcpus,"}],"source_content_type":"text/x-python","patch_set":4,"id":"c512ab55_7124d8ae","line":296,"in_reply_to":"fc9b2ac5_14b1ed56","updated":"2026-07-15 15:11:05.000000000","message":"for consistency we should also use a dict here like we do for the utilization cache","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"559bba85e4c2ae9bd2d64f165f044defaa30344e","unresolved":false,"context_lines":[{"line_number":506,"context_line":"                disk_used,"},{"line_number":507,"context_line":"            )"},{"line_number":508,"context_line":""},{"line_number":509,"context_line":"        return dict("},{"line_number":510,"context_line":"            vcpu\u003dnode.vcpu_capacity - vcpu_used,"},{"line_number":511,"context_line":"            memory\u003dnode.memory_mb_capacity - memory_used,"},{"line_number":512,"context_line":"            disk\u003dnode.disk_gb_capacity - disk_used,"}],"source_content_type":"text/x-python","patch_set":4,"id":"e037b977_15eb1ed3","line":509,"updated":"2026-06-30 15:49:14.000000000","message":"get_free_node_allocation() seeds the cache from get_node_used_resources() (raw summed instance allocations) but computes free against node.vcpu_capacity / memory_mb_capacity / disk_gb_capacity, which already subtract reserved resources and apply overcommit ratios. When overcommit ratios \u003e 1.0 are configured, capacity can exceed raw capacity, and when reservations are large, free can go negative even with instances present. Negative free is handled correctly by is_node_saturated (\u003c\u003d0), but the model mixes raw-used against ratio-adjusted-capacity.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Documenting (in the docstring) that free allocation is capacity-minus-raw-used and intentionally conservative with respect to reservations/overcommit prevents future confusion about why free can be negative or why it differs from get_node_free_resources().\n\n**Recommendation**:\nAdd a one-line note to the get_free_node_allocation docstring clarifying that \u0027used\u0027 is raw flavor allocation while capacity accounts for reserved resources and overcommit ratios, so \u0027free\u0027 is a conservative lower bound. No code change required.","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"559bba85e4c2ae9bd2d64f165f044defaa30344e","unresolved":false,"context_lines":[{"line_number":582,"context_line":"        # saturation threshold is created with 128MB value"},{"line_number":583,"context_line":"        # for memory which is a usual minimum ram size of"},{"line_number":584,"context_line":"        # real world flavors"},{"line_number":585,"context_line":"        saturation_limit \u003d {\u0027cpu\u0027: 0, \u0027ram\u0027: 128, \u0027disk\u0027: 0}"},{"line_number":586,"context_line":"        free \u003d self.get_free_node_allocation(node)"},{"line_number":587,"context_line":"        if ("},{"line_number":588,"context_line":"            free[\u0027vcpu\u0027] \u003c\u003d saturation_limit[\u0027cpu\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"2f29b687_0f1377b7","line":585,"updated":"2026-06-30 15:49:14.000000000","message":"is_node_saturated() applies saturation_limit \u003d {\u0027cpu\u0027: 0, \u0027ram\u0027: 128, \u0027disk\u0027: 0} to both the allocation check and the utilization check. The cpu and disk limits of 0 mean a node with a fractional amount of free vcpu (e.g. 0.5) or free disk is not considered saturated by the allocation branch, relying solely on the utilization branch to catch near-full nodes. The comment only explains the 128MB memory rationale; the cpu/disk\u003d0 choice is undocumented.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: A brief comment noting that cpu/disk use 0 because vcpus and disk are integer flavor units (unlike sub-1MB memory remainders) makes the asymmetric threshold self-explanatory and guards against someone \u0027normalizing\u0027 all limits to a non-zero value.\n\n**Recommendation**:\nExtend the existing comment block to note that vcpus and disk_gb are whole units in the allocation model, so a 0 threshold is sufficient, whereas memory in MB can leave sub-flavor remainders requiring the 128MB floor.","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"559bba85e4c2ae9bd2d64f165f044defaa30344e","unresolved":false,"context_lines":[{"line_number":724,"context_line":"                            disk\u003dsource[\u0027disk\u0027] + instance_util[\u0027disk\u0027],"},{"line_number":725,"context_line":"                        )"},{"line_number":726,"context_line":"                    # Also, adjust allocations cache for the deleted actions."},{"line_number":727,"context_line":"                    if dst_node.hostname in self.node_allocation_cache:"},{"line_number":728,"context_line":"                        dest \u003d self.node_allocation_cache[dst_node.hostname]"},{"line_number":729,"context_line":"                        self.node_allocation_cache[dst_node.hostname] \u003d ("},{"line_number":730,"context_line":"                            dest[0] - instance.vcpus,"}],"source_content_type":"text/x-python","patch_set":4,"id":"df17cd72_1935bfd0","line":727,"updated":"2026-06-30 15:49:14.000000000","message":"There is no unit test covering the allocation-cache adjustment added to optimize_solution() (lines ~726-740). The test suite adds thorough coverage for instance_fits allocation rejection and is_node_saturated, but the optimize_solution allocation path - where migrations are collapsed and the allocation cache is reversed and then re-applied via add_migration - is exercised only indirectly through full do_execute runs. A regression here (wrong sign on src/dst adjustment) would not be caught by a targeted assertion.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: The optimize_solution allocation adjustment is a hand-rolled reverse of add_migration\u0027s update; if the signs are inverted, the allocation cache silently drifts and instance_fits gives wrong answers for subsequent nodes. Because the consolidated migration re-invokes add_migration, the net should be correct, but without a direct assertion a sign error on the intermediate reversal is invisible.\n\n**Suggestion**:\nAdd a focused test (mirroring the existing node_utilization_cache consistency test) that seeds node_allocation_cache, runs optimize_solution over a multi-hop migration, and asserts the allocation cache returns to the pre-optimization used-resource totals on the affected source and destination nodes.","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"559bba85e4c2ae9bd2d64f165f044defaa30344e","unresolved":false,"context_lines":[{"line_number":782,"context_line":"                        )"},{"line_number":783,"context_line":"                        continue"},{"line_number":784,"context_line":"                    for destination_node in reversed(sorted_nodes):"},{"line_number":785,"context_line":"                        if destination_node.hostname in saturated_nodes:"},{"line_number":786,"context_line":"                            continue"},{"line_number":787,"context_line":"                        if self.instance_fits(instance, destination_node, cc):"},{"line_number":788,"context_line":"                            LOG.info("}],"source_content_type":"text/x-python","patch_set":4,"id":"e5d636ab_bda2f00b","line":785,"updated":"2026-06-30 15:49:14.000000000","message":"The offload_phase saturation short-circuit (continue on destination_node.hostname in saturated_nodes) is added before the instance_fits call, but saturation is only recorded inside the else branch after instance_fits returns False. This is correct, but the offload loop will keep re-evaluating instance_fits against nodes that previously failed for a different instance. Marking a node saturated is keyed on is_node_saturated, which depends on current allocation/utilization, so a node can only ever transition toward saturation during a phase - meaning once it fails instance_fits and is saturated it is correctly skipped thereafter.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: No action strictly required; the logic is sound. Noting the monotonic-saturation invariant in a comment would make the correctness of the short-circuit obvious to reviewers.\n\n**Recommendation**:\nOptional: add a short comment at the saturated_nodes set initialization in offload_phase explaining that saturation is monotonic within a phase (allocations only grow on destination nodes), so the skip set is safe to accumulate across instances.","commit_id":"ea584d20d0972d52c5cb479dbe4e92b8756528aa"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"40ea830ac17e4de78cabaf1980f067c87e0b5495","unresolved":false,"context_lines":[{"line_number":722,"context_line":"                            instance.uuid,"},{"line_number":723,"context_line":"                        )"},{"line_number":724,"context_line":"                        continue"},{"line_number":725,"context_line":"                    for destination_node in reversed(sorted_nodes):"},{"line_number":726,"context_line":"                        if destination_node.hostname in saturated_nodes:"},{"line_number":727,"context_line":"                            continue"},{"line_number":728,"context_line":"                        if self.instance_fits(instance, destination_node, cc):"}],"source_content_type":"text/x-python","patch_set":6,"id":"c3682866_cd9aaa2e","line":725,"updated":"2026-07-29 17:55:21.000000000","message":"In offload_phase\u0027s destination loop, the overloaded source node is also iterated as a destination. instance_fits returns False for it (double-counts utilization), then is_node_saturated returns True (overloaded), permanently adding the source to saturated_nodes. After its own instances are migrat...\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: The strategy may fail to relieve overloaded nodes when a previously-saturated node has since had instances migrated away and now has free capacity, producing sub-optimal migration plans that leave nodes overloaded.\n\n**Priority**: Before merge\n**Why This Matters**: The strategy may fail to relieve overloaded nodes when a previously-saturated node has since had instances migrated away and now has free capacity, producing sub-optimal migration plans that leave nodes overloaded.\n\n**Recommendation**:\nSkip the source node in the destination loop by adding `if destination_node.hostname \u003d\u003d node.hostname: continue` at the top of the inner loop, mirroring consolidation_phase\u0027s asc/dsc guard. Alternatively, invalidate saturated_nodes entries when a node\u0027s load decreases.","commit_id":"bb6a9766f5bcca7f9b6f2b27cb451da713450466"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"8f50a2cd2317b52833bb35d4f35d04af4d6db4aa","unresolved":true,"context_lines":[{"line_number":722,"context_line":"                            instance.uuid,"},{"line_number":723,"context_line":"                        )"},{"line_number":724,"context_line":"                        continue"},{"line_number":725,"context_line":"                    for destination_node in reversed(sorted_nodes):"},{"line_number":726,"context_line":"                        if destination_node.hostname in saturated_nodes:"},{"line_number":727,"context_line":"                            continue"},{"line_number":728,"context_line":"                        if self.instance_fits(instance, destination_node, cc):"}],"source_content_type":"text/x-python","patch_set":6,"id":"d60756e8_7e6b6f22","line":725,"in_reply_to":"c3682866_cd9aaa2e","updated":"2026-07-30 07:15:33.000000000","message":"That\u0027s technically true, although the method stops migrating VMs after the first migration that leaves the node below the \"overload\" level, without any minimum margin. Therefore, it seems appropriate not to use it again as a destination in the offload phase (although it could be useful in some cases). Note that in the consolidation phase, the list of saturated nodes is regenerated from scratch, so those nodes could be used as destinations.\n\nBy the way, with the current implementation of `is_overloaded`, I doubt the offload phase will be useful in the vast majority of cases, although that\u0027s a separate issue outside the scope of this patch.","commit_id":"bb6a9766f5bcca7f9b6f2b27cb451da713450466"}],"watcher/tests/unit/decision_engine/strategy/strategies/test_vm_workload_consolidation.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"2ad1ab0eefbf906ff1f5e2fec7c87427d7995f6e","unresolved":false,"context_lines":[{"line_number":572,"context_line":""},{"line_number":573,"context_line":"        # Baseline: Node_1 (40 vcpus, 64 mem, 250 disk) hosts"},{"line_number":574,"context_line":"        # INSTANCE_1 (10 vcpus, 2 mem, 20 disk) runs in Node_1"},{"line_number":575,"context_line":"        # INSTANCE_0 (10 vcpus, 2 mem, 20 disk) runs in Node_o and should fit."},{"line_number":576,"context_line":"        self.assertTrue(self.strategy.instance_fits(instance_0, node_1, cc))"},{"line_number":577,"context_line":""},{"line_number":578,"context_line":"        # --- vcpu: reject when exhausted ---"}],"source_content_type":"text/x-python","patch_set":2,"id":"4be47822_14c86aa0","line":575,"updated":"2026-06-24 13:24:06.000000000","message":"Test comment contains a typo: \u0027Node_o\u0027 should be \u0027Node_0\u0027 in the test_instance_fits_allocation_check docstring.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Correcting the typo improves test readability and avoids confusion for future test maintainers who may reference the comment to understand the test scenario.\n\n**Recommendation**:\nChange \u0027runs in Node_o\u0027 to \u0027runs in Node_0\u0027 on line 575.","commit_id":"947ac9ed2d0d2ad864c4113b9f5424d395767d24"}]}