)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e0db4a39a495201f048d0ee1af2a232eceff7668","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"72ae497f_2ad7433e","updated":"2026-08-17 19:37:26.000000000","message":"I think that we need to adjust some details about the sorting based on what was defined in the spec. Feel free to ping us in the irc if you have any question","commit_id":"4a1f1a6a839ebafd3e40db49309ec960aead5bf0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e02cee0f730c75ce0ebe71fe884780d4b1c272","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"5dcdb7e1_3e0524ce","updated":"2026-08-26 13:57:30.000000000","message":"Some considerations in the implementation, the most critical is actually when creating the action, it seems that curent input_parameters would break the actions schema","commit_id":"231988760117ee3f73284839b002ea25955c7a00"}],"doc/source/strategies/workload_preemption.rst":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e0db4a39a495201f048d0ee1af2a232eceff7668","unresolved":true,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"* Flavors for preemptible instances must have the extra spec"},{"line_number":18,"context_line":"  ``lifecycle:preemptible`` set to ``true``."},{"line_number":19,"context_line":"* The Compute Data Model (CDM) must be available and must contain"},{"line_number":20,"context_line":"  ``flavor_extra_specs`` for instances (requires the extended CDM feature"},{"line_number":21,"context_line":"  to be enabled)."},{"line_number":22,"context_line":"* Optionally, flavors can set ``lifecycle:preempt_policy`` to ``shelve``"},{"line_number":23,"context_line":"  or ``delete`` to override the strategy-level default preemption mode."},{"line_number":24,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"e64ebc6f_7c291495","line":21,"range":{"start_line":19,"start_character":1,"end_line":21,"end_character":17},"updated":"2026-08-17 19:37:26.000000000","message":"we may consider enabled it by default now if we have an usage for them","commit_id":"4a1f1a6a839ebafd3e40db49309ec960aead5bf0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"* Flavors for preemptible instances must have the extra spec"},{"line_number":18,"context_line":"  ``lifecycle:preemptible`` set to ``true``."},{"line_number":19,"context_line":"* The Compute Data Model (CDM) must be available and must contain"},{"line_number":20,"context_line":"  ``flavor_extra_specs`` for instances (requires the extended CDM feature"},{"line_number":21,"context_line":"  to be enabled)."},{"line_number":22,"context_line":"* Optionally, flavors can set ``lifecycle:preempt_policy`` to ``shelve``"},{"line_number":23,"context_line":"  or ``delete`` to override the strategy-level default preemption mode."},{"line_number":24,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"d2cdf915_c0b6383a","line":21,"range":{"start_line":19,"start_character":1,"end_line":21,"end_character":17},"in_reply_to":"e64ebc6f_7c291495","updated":"2026-09-03 01:19:42.000000000","message":"we could but maybe next cyce","commit_id":"4a1f1a6a839ebafd3e40db49309ec960aead5bf0"},{"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":"f7d7e3f9f895b9486d5d134275580511e82ab502","unresolved":false,"context_lines":[{"line_number":146,"context_line":"   - *Conditional*: The instance must be older than ``min_instance_age``"},{"line_number":147,"context_line":"     seconds."},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"3. **Priority sorting and preemption**: Sorts candidates according to"},{"line_number":150,"context_line":"   ``preemption_preference`` using a tuple-projection model (Python\u0027s"},{"line_number":151,"context_line":"   stable sort). Iterates sorted candidates, accumulates freed resources,"},{"line_number":152,"context_line":"   and stops as soon as all configured thresholds are satisfied."}],"source_content_type":"text/x-rst","patch_set":7,"id":"cc3406d4_3c5be585","line":149,"updated":"2026-08-27 15:38:41.000000000","message":"The documented algorithm says the strategy iterates candidates and \u0027stops as soon as all configured thresholds are satisfied\u0027, but it does not mention the all-or-nothing fallback: if all candidates would be exhausted without meeting the thresholds, do_execute logs a warning and returns a solution with no actions at all (no partial preemption).\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: An operator configuring an unreachable threshold (e.g. too few preemptible instances, or regular workloads dominating allocation) will see audits that silently produce no actions, which is surprising and hard to diagnose without reading the source or debug logs.\n\n**Recommendation**:\nAdd a sentence to the Algorithm section (and optionally the release note) stating that if preempting every eligible candidate still cannot satisfy the configured thresholds, the strategy emits no actions and the audit logs a warning, so operators can predict empty solutions for unreachable targets.","commit_id":"1ffbe7a54b8b2a1393a3173f0762efa819bf1b01"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":false,"context_lines":[{"line_number":123,"context_line":"         - Total number of preempted instances"},{"line_number":124,"context_line":"       * - ``vcpu_freed``"},{"line_number":125,"context_line":"         - Total number of vCPUs freed"},{"line_number":126,"context_line":"       * - ``mem_mb_freed``"},{"line_number":127,"context_line":"         - Total amount of RAM freed (in MB)"},{"line_number":128,"context_line":"       * - ``disk_gb_freed``"},{"line_number":129,"context_line":"         - Total amount of disk freed (in GB)"},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"Algorithm"},{"line_number":132,"context_line":"---------"}],"source_content_type":"text/x-rst","patch_set":8,"id":"c7cd59ed_10b3d08e","line":129,"range":{"start_line":126,"start_character":7,"end_line":129,"end_character":45},"updated":"2026-09-03 01:19:42.000000000","message":"it does not hurt to be precises i guess.\nits better to be overly verbose then not include the unit but its aslo in the name 😉","commit_id":"6c3f4f93c2e97eca16fe1ea621e7ff6e3a9dfd15"}],"watcher/decision_engine/strategy/strategies/workload_preemption.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e0db4a39a495201f048d0ee1af2a232eceff7668","unresolved":true,"context_lines":[{"line_number":150,"context_line":"                key_tuple.append(val)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"            elif key.startswith(SORT_KEY_RESOURCE_PREFIX):"},{"line_number":153,"context_line":"                resource_name \u003d key[len(SORT_KEY_RESOURCE_PREFIX) :].upper()"},{"line_number":154,"context_line":"                attr \u003d RESOURCE_ATTR_MAP.get(resource_name)"},{"line_number":155,"context_line":"                if attr:"},{"line_number":156,"context_line":"                    val \u003d getattr(instance, attr, 0)"},{"line_number":157,"context_line":"                else:"},{"line_number":158,"context_line":"                    val \u003d 0"},{"line_number":159,"context_line":"                if reverse:"}],"source_content_type":"text/x-python","patch_set":2,"id":"f1430719_1d4cc22d","line":156,"range":{"start_line":153,"start_character":0,"end_line":156,"end_character":51},"updated":"2026-08-17 19:37:26.000000000","message":"So, based on the approved spec[1] and discussions in gerrit, it was proposed that using \"resource:\" would use placement resource allocation instead of flavors. By translating to vpuc/memory/disk and getting from the model you are still using the values from the flavor that are already stored in the CDM. In order to fix this, you will also need to extend de CDM to include this info from placement (not only these 3, but also others mentioned in the Spec), otherwise we would need to fallback to use \"flavor:vcpus\" instead of \"resource:VCPU\" and also update the spec to be in sync with the implementation (if other reviewers agree with this approach).\n\n[1] https://github.com/openstack/watcher-specs/blob/master/specs/2026.2/approved/preemptible-instances.rst#generalized-priority-sorting","commit_id":"4a1f1a6a839ebafd3e40db49309ec960aead5bf0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e0db4a39a495201f048d0ee1af2a232eceff7668","unresolved":true,"context_lines":[{"line_number":456,"context_line":"            return self.solution"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        # Get all instances and filter for preemptible candidates"},{"line_number":459,"context_line":"        all_instances \u003d self._get_all_instances_list()"},{"line_number":460,"context_line":"        candidates \u003d filter_preemptible_candidates("},{"line_number":461,"context_line":"            all_instances, self.min_instance_age"},{"line_number":462,"context_line":"        )"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"        if not candidates:"},{"line_number":465,"context_line":"            LOG.warning("}],"source_content_type":"text/x-python","patch_set":2,"id":"26ff0993_4c298e8d","line":462,"range":{"start_line":459,"start_character":0,"end_line":462,"end_character":9},"updated":"2026-08-17 19:37:26.000000000","message":"Ok to ge all instances from the model but you need to exclude some instances that are marked as exclude in current audit scope. So you would need to check if it doesn\u0027t have watcher_exclude set before considering it as a candidate. See this bug[1] for more info.\nWe plan to fix this for all strategies in a common method, but for now, the strategy needs to handle it.\n\n[1] https://bugs.launchpad.net/watcher/+bug/2151776","commit_id":"4a1f1a6a839ebafd3e40db49309ec960aead5bf0"},{"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":"31ac8266e70ae498abe44a73a2f5266f81981dde","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    now \u003d timeutils.utcnow()"},{"line_number":64,"context_line":"    candidates \u003d []"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    for instance in instances:"},{"line_number":67,"context_line":"        # Mandatory invariant: must be marked as preemptible"},{"line_number":68,"context_line":"        extra_specs \u003d instance.flavor_extra_specs or {}"},{"line_number":69,"context_line":"        preemptible_value \u003d str(extra_specs.get(PREEMPTIBLE_KEY, \u0027false\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"986daa51_92e90950","line":66,"updated":"2026-08-26 01:54:06.000000000","message":"Watcher\u0027s audit scope exclusion mechanism marks instances excluded by UUID, metadata, or project with instance.watcher_exclude\u003dTrue (watcher/decision_engine/scope/compute.py:39-44) instead of removing them from the cluster data model. Every other instance-acting strategy checks instance.watcher_exclude before acting (workload_balance.py:158, vm_workload_consolidation.py:719/782, basic_consolidation.py:401, outlet_temp_control.py:188, uniform_airflow.py:185, workload_stabilization.py:453). filter_preemptible_candidates() has no such check, so an instance an operator explicitly excluded from the audit template scope can still be preempted (deleted or shelved) because it carries the preemptible flavor extra spec.\n\n**Severity**: HIGH | **Confidence**: 0.85\n\n**Impact**: With default preemption_mode\u003d\u0027delete\u0027, the strategy can irreversibly delete an instance the operator explicitly listed in the audit template\u0027s exclusion section, defeating the documented opt-out mechanism and causing data loss for that workload.\n\n**Priority**: Before merge\n**Recommendation**:\nIn filter_preemptible_candidates, skip instances where instance.watcher_exclude is True (with a debug log, matching the other exclusion reasons), and add a unit test with an excluded preemptible instance asserting no action is emitted for it.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":36630,"name":"Winicius Allan Bezerra da Silva","display_name":"Winicius Allan","email":"winiciusab12@gmail.com","username":"winiciusallan"},"change_message_id":"70b55a54cbfcc5fdf0bf00f6008ec2a064b760f4","unresolved":true,"context_lines":[{"line_number":63,"context_line":"    now \u003d timeutils.utcnow()"},{"line_number":64,"context_line":"    candidates \u003d []"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    for instance in instances:"},{"line_number":67,"context_line":"        # Mandatory invariant: must be marked as preemptible"},{"line_number":68,"context_line":"        extra_specs \u003d instance.flavor_extra_specs or {}"},{"line_number":69,"context_line":"        preemptible_value \u003d str(extra_specs.get(PREEMPTIBLE_KEY, \u0027false\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"f2c77622_c8d36489","line":66,"in_reply_to":"2b95afdb_6b3fa58c","updated":"2026-08-27 03:01:50.000000000","message":"I did the changes, but I still have questions about handling the instance filtering. Is filtering all and then scoping with audit_scope the best practice? What about instances in ERROR and/or BUILDING?","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e02cee0f730c75ce0ebe71fe884780d4b1c272","unresolved":true,"context_lines":[{"line_number":63,"context_line":"    now \u003d timeutils.utcnow()"},{"line_number":64,"context_line":"    candidates \u003d []"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    for instance in instances:"},{"line_number":67,"context_line":"        # Mandatory invariant: must be marked as preemptible"},{"line_number":68,"context_line":"        extra_specs \u003d instance.flavor_extra_specs or {}"},{"line_number":69,"context_line":"        preemptible_value \u003d str(extra_specs.get(PREEMPTIBLE_KEY, \u0027false\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"2b95afdb_6b3fa58c","line":66,"in_reply_to":"986daa51_92e90950","updated":"2026-08-26 13:57:30.000000000","message":"Correct, I also mentioned this below when listing all instances. We need to filter instances that contains watcher_exclude\u003dTrue, example:\nhttps://github.com/openstack/watcher/blob/ec06744bffd4ba858da7f4996cc872723dfbde70/watcher/decision_engine/strategy/strategies/vm_workload_consolidation.py#L718-L724","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":63,"context_line":"    now \u003d timeutils.utcnow()"},{"line_number":64,"context_line":"    candidates \u003d []"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    for instance in instances:"},{"line_number":67,"context_line":"        # Mandatory invariant: must be marked as preemptible"},{"line_number":68,"context_line":"        extra_specs \u003d instance.flavor_extra_specs or {}"},{"line_number":69,"context_line":"        preemptible_value \u003d str(extra_specs.get(PREEMPTIBLE_KEY, \u0027false\u0027))"}],"source_content_type":"text/x-python","patch_set":3,"id":"e7d0d651_5b68ea35","line":66,"in_reply_to":"f2c77622_c8d36489","updated":"2026-09-03 01:19:42.000000000","message":"i think that is a larger question about what watcher operats on thien this specific stragey\n\n\nwatcher may want to be more selective about which isntence it operats on in the futre but if we do that i woudl like to centralise the logic.\n\n\nif a instance is permetivable and its in error and it was selected as a candiate to try and delete or shelve its valid for watcher to attept to do that.\n\nnow in the error case shielve likely woudl fails and we coudl check for active in its precondtion as a follow up but delete woudl work so i think for now we do not need to expclity expclued errored instnaces.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"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":"31ac8266e70ae498abe44a73a2f5266f81981dde","unresolved":false,"context_lines":[{"line_number":124,"context_line":""},{"line_number":125,"context_line":"    def make_sort_key(instance):"},{"line_number":126,"context_line":"        key_tuple \u003d []"},{"line_number":127,"context_line":"        for key, direction in preemption_preference:"},{"line_number":128,"context_line":"            reverse \u003d direction \u003d\u003d \u0027dec\u0027"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"            if key \u003d\u003d SORT_KEY_AGE:"}],"source_content_type":"text/x-python","patch_set":3,"id":"f2a1854d_c6f09eca","line":127,"updated":"2026-08-26 01:54:06.000000000","message":"sort_candidates treats any direction other than \u0027dec\u0027 as ascending (workload_preemption.py:127: reverse \u003d direction \u003d\u003d \u0027dec\u0027), and the get_schema() entry for preemption_preference (lines 247-263) declares items as arrays of strings without constraining direction to \u0027asc\u0027/\u0027dec\u0027. Nova\u0027s own sort directions are \u0027asc\u0027/\u0027desc\u0027, so an operator writing the natural \u0027desc\u0027 value gets a silently accepted parameter that sorts newest-first instead of the intended oldest-first, preempting exactly the wrong instances first.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: A mistyped or Nova-convention direction value silently reorders preemption candidates, preempting the newest instances first and leaving the protected-age oldest instances running, contrary to the operator\u0027s intent; there is no error, warning, or audit-time rejection to surface the mistake.\n\n**Suggestion**:\nConstrain direction in the schema (e.g., an items enum or validation in pre_execute rejecting directions outside \u0027asc\u0027/\u0027dec\u0027), and/or emit a warning in sort_candidates when direction is unrecognized, mirroring the existing unknown-key warning.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e02cee0f730c75ce0ebe71fe884780d4b1c272","unresolved":true,"context_lines":[{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        return {\u0027vcpu\u0027: total_vcpu, \u0027memory\u0027: total_memory, \u0027disk\u0027: total_disk}"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def _get_total_allocated_resources(self):"},{"line_number":302,"context_line":"        \"\"\"Get total allocated resources across all instances."},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        :returns: dict with keys \u0027vcpu\u0027, \u0027memory\u0027, \u0027disk\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"91146496_2de84e99","line":301,"range":{"start_line":301,"start_character":8,"end_line":301,"end_character":38},"updated":"2026-08-26 13:57:30.000000000","message":"we can use the self.compute_model.get_node_used_resources() here, or no?","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":36630,"name":"Winicius Allan Bezerra da Silva","display_name":"Winicius Allan","email":"winiciusab12@gmail.com","username":"winiciusallan"},"change_message_id":"70b55a54cbfcc5fdf0bf00f6008ec2a064b760f4","unresolved":true,"context_lines":[{"line_number":298,"context_line":""},{"line_number":299,"context_line":"        return {\u0027vcpu\u0027: total_vcpu, \u0027memory\u0027: total_memory, \u0027disk\u0027: total_disk}"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def _get_total_allocated_resources(self):"},{"line_number":302,"context_line":"        \"\"\"Get total allocated resources across all instances."},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        :returns: dict with keys \u0027vcpu\u0027, \u0027memory\u0027, \u0027disk\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"dce2e247_1410b2af","line":301,"range":{"start_line":301,"start_character":8,"end_line":301,"end_character":38},"in_reply_to":"91146496_2de84e99","updated":"2026-08-27 03:01:50.000000000","message":"yes, we can! did not know about this method. thanks! before trying to use this, I saw that this unconditionally get all instnaces from the nodes, so I would like to confirm the filtering questions in above comments before proceeding.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e02cee0f730c75ce0ebe71fe884780d4b1c272","unresolved":true,"context_lines":[{"line_number":308,"context_line":"        total_disk \u003d 0"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"        for instance in self.compute_model.get_all_instances().values():"},{"line_number":311,"context_line":"            if instance.state \u003d\u003d element.InstanceState.ACTIVE.value:"},{"line_number":312,"context_line":"                total_vcpu +\u003d instance.vcpus"},{"line_number":313,"context_line":"                total_memory +\u003d instance.memory"},{"line_number":314,"context_line":"                total_disk +\u003d instance.disk"}],"source_content_type":"text/x-python","patch_set":3,"id":"c80c75a4_d5a4a7a8","line":311,"range":{"start_line":311,"start_character":15,"end_line":311,"end_character":66},"updated":"2026-08-26 13:57:30.000000000","message":"why only Active ones?","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":308,"context_line":"        total_disk \u003d 0"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"        for instance in self.compute_model.get_all_instances().values():"},{"line_number":311,"context_line":"            if instance.state \u003d\u003d element.InstanceState.ACTIVE.value:"},{"line_number":312,"context_line":"                total_vcpu +\u003d instance.vcpus"},{"line_number":313,"context_line":"                total_memory +\u003d instance.memory"},{"line_number":314,"context_line":"                total_disk +\u003d instance.disk"}],"source_content_type":"text/x-python","patch_set":3,"id":"b1494af0_1460dd77","line":311,"range":{"start_line":311,"start_character":15,"end_line":311,"end_character":66},"in_reply_to":"acb5517c_3a66d100","updated":"2026-09-03 01:19:42.000000000","message":"i dont think you shoudl be excluign instnace at this level\n\nerrored isntance shoudl be prematble at a minium\n\ninstnace that are in buildign but also exceed the min age are likely stuck and are also a good candiate to premet espiclly fi the policy is delete.\n\nstoped,pasued,errord,suspened isntance all take the same amount of reousce on the host form an allcoation persective so i think the filterign shoudl be mostly left to the auit scope","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":36630,"name":"Winicius Allan Bezerra da Silva","display_name":"Winicius Allan","email":"winiciusab12@gmail.com","username":"winiciusallan"},"change_message_id":"70b55a54cbfcc5fdf0bf00f6008ec2a064b760f4","unresolved":true,"context_lines":[{"line_number":308,"context_line":"        total_disk \u003d 0"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"        for instance in self.compute_model.get_all_instances().values():"},{"line_number":311,"context_line":"            if instance.state \u003d\u003d element.InstanceState.ACTIVE.value:"},{"line_number":312,"context_line":"                total_vcpu +\u003d instance.vcpus"},{"line_number":313,"context_line":"                total_memory +\u003d instance.memory"},{"line_number":314,"context_line":"                total_disk +\u003d instance.disk"}],"source_content_type":"text/x-python","patch_set":3,"id":"acb5517c_3a66d100","line":311,"range":{"start_line":311,"start_character":15,"end_line":311,"end_character":66},"in_reply_to":"c80c75a4_d5a4a7a8","updated":"2026-08-27 03:01:50.000000000","message":"The idea was to filter to avoid catching instances with no allocated resources e.g. in ERROR or still BUILDING. turns out that this excludes some eligible instances, for example in stopped state.\n\nalthough the VM is not running, placement will still allocate resources for this instances so it can run when start again.\n\nmaybe we should exclude some instances but not picking up only the ACTIVE ones or pick ALL instances and only filter them with audit scope?","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e02cee0f730c75ce0ebe71fe884780d4b1c272","unresolved":true,"context_lines":[{"line_number":416,"context_line":"        Evaluates cluster-wide resource thresholds and preempts"},{"line_number":417,"context_line":"        preemptible instances until all configured thresholds are met."},{"line_number":418,"context_line":"        \"\"\""},{"line_number":419,"context_line":"        if not self._has_any_threshold():"},{"line_number":420,"context_line":"            LOG.info("},{"line_number":421,"context_line":"                \"No resource threshold configured. \""},{"line_number":422,"context_line":"                \"Set at least one of cpu_free_percentage, \""},{"line_number":423,"context_line":"                \"ram_free_percentage, or disk_free_percentage.\""},{"line_number":424,"context_line":"            )"},{"line_number":425,"context_line":"            return self.solution"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        total_resources \u003d self._get_total_cluster_resources()"},{"line_number":428,"context_line":"        allocated_resources \u003d self._get_total_allocated_resources()"}],"source_content_type":"text/x-python","patch_set":3,"id":"2ad7cc7d_b5cb188f","line":425,"range":{"start_line":419,"start_character":0,"end_line":425,"end_character":32},"updated":"2026-08-26 13:57:30.000000000","message":"Can\u0027t we define this at strategy spec? using AnyOf?","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":416,"context_line":"        Evaluates cluster-wide resource thresholds and preempts"},{"line_number":417,"context_line":"        preemptible instances until all configured thresholds are met."},{"line_number":418,"context_line":"        \"\"\""},{"line_number":419,"context_line":"        if not self._has_any_threshold():"},{"line_number":420,"context_line":"            LOG.info("},{"line_number":421,"context_line":"                \"No resource threshold configured. \""},{"line_number":422,"context_line":"                \"Set at least one of cpu_free_percentage, \""},{"line_number":423,"context_line":"                \"ram_free_percentage, or disk_free_percentage.\""},{"line_number":424,"context_line":"            )"},{"line_number":425,"context_line":"            return self.solution"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"        total_resources \u003d self._get_total_cluster_resources()"},{"line_number":428,"context_line":"        allocated_resources \u003d self._get_total_allocated_resources()"}],"source_content_type":"text/x-python","patch_set":3,"id":"42e16067_07104284","line":425,"range":{"start_line":419,"start_character":0,"end_line":425,"end_character":32},"in_reply_to":"2ad7cc7d_b5cb188f","updated":"2026-09-03 01:19:42.000000000","message":"right we shoudl not allow you to defien an audit at the api leve unless it valid.\nso if we are requiring treshold those shoudl be validated then or when validating the tempelat ideally rather then when executing the audit iteslf.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"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":"31ac8266e70ae498abe44a73a2f5266f81981dde","unresolved":false,"context_lines":[{"line_number":473,"context_line":""},{"line_number":474,"context_line":"            action_type \u003d self._get_preemption_action(instance)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"            self.solution.add_action("},{"line_number":477,"context_line":"                action_type\u003daction_type,"},{"line_number":478,"context_line":"                resource_id\u003dinstance.uuid,"},{"line_number":479,"context_line":"                input_parameters\u003d{\u0027resource_name\u0027: instance.name},"}],"source_content_type":"text/x-python","patch_set":3,"id":"ab7339a1_8adad679","line":476,"updated":"2026-08-26 01:54:06.000000000","message":"do_execute() adds actions with input_parameters\u003d{\u0027resource_name\u0027: instance.name} in addition to resource_id. The Delete and Shelve action schemas (watcher/applier/actions/delete.py:43-60, shelve.py:43-60) declare only \u0027resource_id\u0027 with \u0027additionalProperties\u0027: False. Solution.add_action injects resource_id into input_parameters (watcher/decision_engine/solution/default.py:52-57), and the planner passes input_parameters unchanged into objects.Action (watcher/decision_engine/planner/weight.py:179-187). When the applier loads each action, ActionFactory.make_action() calls validate_parameters(), which runs jsonschema.validate against the action schema; the extra \u0027resource_name\u0027 key fails validation with additionalProperties: False.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Impact**: Every delete/shelve action generated by the Workload Preemption strategy raises jsonschema.ValidationError in ActionFactory.make_action during applier execution, so the strategy produces action plans that can never be applied; the feature is non-functional end to end.\n\n**Priority**: Before merge\n**Recommendation**:\nDrop the resource_name parameter and call self.solution.add_action(action_type\u003daction_type, resource_id\u003dinstance.uuid), matching host_maintenance\u0027s stop action pattern. Alternatively declare resource_name in the Delete/Shelve schemas. Add a unit test that validates emitted actions against the loaded action schema (e.g., via jsonschema or ActionFactory) so regressions are caught before merge.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":false,"context_lines":[{"line_number":473,"context_line":""},{"line_number":474,"context_line":"            action_type \u003d self._get_preemption_action(instance)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"            self.solution.add_action("},{"line_number":477,"context_line":"                action_type\u003daction_type,"},{"line_number":478,"context_line":"                resource_id\u003dinstance.uuid,"},{"line_number":479,"context_line":"                input_parameters\u003d{\u0027resource_name\u0027: instance.name},"}],"source_content_type":"text/x-python","patch_set":3,"id":"5c144d75_9adae5c8","line":476,"in_reply_to":"61036dc0_f1a5cb06","updated":"2026-09-03 01:19:42.000000000","message":"Acknowledged","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e02cee0f730c75ce0ebe71fe884780d4b1c272","unresolved":true,"context_lines":[{"line_number":473,"context_line":""},{"line_number":474,"context_line":"            action_type \u003d self._get_preemption_action(instance)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"            self.solution.add_action("},{"line_number":477,"context_line":"                action_type\u003daction_type,"},{"line_number":478,"context_line":"                resource_id\u003dinstance.uuid,"},{"line_number":479,"context_line":"                input_parameters\u003d{\u0027resource_name\u0027: instance.name},"}],"source_content_type":"text/x-python","patch_set":3,"id":"b1dad88f_cf8e3ba2","line":476,"in_reply_to":"ab7339a1_8adad679","updated":"2026-08-26 13:57:30.000000000","message":"Correct, you would need to drop resource_name here.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":36630,"name":"Winicius Allan Bezerra da Silva","display_name":"Winicius Allan","email":"winiciusab12@gmail.com","username":"winiciusallan"},"change_message_id":"70b55a54cbfcc5fdf0bf00f6008ec2a064b760f4","unresolved":true,"context_lines":[{"line_number":473,"context_line":""},{"line_number":474,"context_line":"            action_type \u003d self._get_preemption_action(instance)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"            self.solution.add_action("},{"line_number":477,"context_line":"                action_type\u003daction_type,"},{"line_number":478,"context_line":"                resource_id\u003dinstance.uuid,"},{"line_number":479,"context_line":"                input_parameters\u003d{\u0027resource_name\u0027: instance.name},"}],"source_content_type":"text/x-python","patch_set":3,"id":"61036dc0_f1a5cb06","line":476,"in_reply_to":"b1dad88f_cf8e3ba2","updated":"2026-08-27 03:01:50.000000000","message":"Yah, it makes sense. Dropped this parameter.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e02cee0f730c75ce0ebe71fe884780d4b1c272","unresolved":true,"context_lines":[{"line_number":500,"context_line":"                instance.disk,"},{"line_number":501,"context_line":"            )"},{"line_number":502,"context_line":"        else:"},{"line_number":503,"context_line":"            # Loop completed without breaking - check if we met thresholds"},{"line_number":504,"context_line":"            if not self._check_thresholds_met("},{"line_number":505,"context_line":"                total_resources, allocated_resources, freed_resources"},{"line_number":506,"context_line":"            ):"},{"line_number":507,"context_line":"                LOG.warning("},{"line_number":508,"context_line":"                    \"Exhausted all %d preemptible candidates but \""},{"line_number":509,"context_line":"                    \"thresholds are still not met.\","},{"line_number":510,"context_line":"                    len(sorted_candidates),"},{"line_number":511,"context_line":"                )"},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"        return self.solution"},{"line_number":514,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"7f736c4f_c66e021a","line":511,"range":{"start_line":503,"start_character":0,"end_line":511,"end_character":17},"updated":"2026-08-26 13:57:30.000000000","message":"If we don\u0027t met the thresholds, shouldn\u0027t we return an emptu solution then? In that case, it is better to build a list of instances to be preempted/shelved instead of creationg actions and addtion to the solution. If thresholds are met, then we loop and create all actions into the solution. Makes sense?\nIn this case, we should also set all indicators to zero.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":36630,"name":"Winicius Allan Bezerra da Silva","display_name":"Winicius Allan","email":"winiciusab12@gmail.com","username":"winiciusallan"},"change_message_id":"70b55a54cbfcc5fdf0bf00f6008ec2a064b760f4","unresolved":true,"context_lines":[{"line_number":500,"context_line":"                instance.disk,"},{"line_number":501,"context_line":"            )"},{"line_number":502,"context_line":"        else:"},{"line_number":503,"context_line":"            # Loop completed without breaking - check if we met thresholds"},{"line_number":504,"context_line":"            if not self._check_thresholds_met("},{"line_number":505,"context_line":"                total_resources, allocated_resources, freed_resources"},{"line_number":506,"context_line":"            ):"},{"line_number":507,"context_line":"                LOG.warning("},{"line_number":508,"context_line":"                    \"Exhausted all %d preemptible candidates but \""},{"line_number":509,"context_line":"                    \"thresholds are still not met.\","},{"line_number":510,"context_line":"                    len(sorted_candidates),"},{"line_number":511,"context_line":"                )"},{"line_number":512,"context_line":""},{"line_number":513,"context_line":"        return self.solution"},{"line_number":514,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"08fb2be1_3fbc696e","line":511,"range":{"start_line":503,"start_character":0,"end_line":511,"end_character":17},"in_reply_to":"7f736c4f_c66e021a","updated":"2026-08-27 03:01:50.000000000","message":"Yes, this makes sense. I made some changes, PTAL.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"59e02cee0f730c75ce0ebe71fe884780d4b1c272","unresolved":true,"context_lines":[{"line_number":520,"context_line":"            mem_mb_freed\u003dself.mem_mb_freed,"},{"line_number":521,"context_line":"            disk_gb_freed\u003dself.disk_gb_freed,"},{"line_number":522,"context_line":"        )"},{"line_number":523,"context_line":"        LOG.debug(self.compute_model.to_string())"}],"source_content_type":"text/x-python","patch_set":3,"id":"29b93d2b_91443a9f","line":523,"range":{"start_line":523,"start_character":0,"end_line":523,"end_character":49},"updated":"2026-08-26 13:57:30.000000000","message":"hum, I don\u0027t think that is required/useful to print this here.","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"author":{"_account_id":36630,"name":"Winicius Allan Bezerra da Silva","display_name":"Winicius Allan","email":"winiciusab12@gmail.com","username":"winiciusallan"},"change_message_id":"70b55a54cbfcc5fdf0bf00f6008ec2a064b760f4","unresolved":false,"context_lines":[{"line_number":520,"context_line":"            mem_mb_freed\u003dself.mem_mb_freed,"},{"line_number":521,"context_line":"            disk_gb_freed\u003dself.disk_gb_freed,"},{"line_number":522,"context_line":"        )"},{"line_number":523,"context_line":"        LOG.debug(self.compute_model.to_string())"}],"source_content_type":"text/x-python","patch_set":3,"id":"0a2dc890_94b765fc","line":523,"range":{"start_line":523,"start_character":0,"end_line":523,"end_character":49},"in_reply_to":"29b93d2b_91443a9f","updated":"2026-08-27 03:01:50.000000000","message":"agree. removed","commit_id":"231988760117ee3f73284839b002ea25955c7a00"},{"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":"009808ef51113169e0687ef362bee9cba40345f6","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                    created \u003d datetime.datetime.fromisoformat(created)"},{"line_number":134,"context_line":"                if created.tzinfo is None:"},{"line_number":135,"context_line":"                    created \u003d created.replace(tzinfo\u003ddatetime.timezone.utc)"},{"line_number":136,"context_line":"                # Age is the inverse of creation timestamp."},{"line_number":137,"context_line":"                # Older instances have a higher \"age\" value."},{"line_number":138,"context_line":"                # We negate the timestamp so that older instances"},{"line_number":139,"context_line":"                # get a higher sort value, then apply the standard"}],"source_content_type":"text/x-python","patch_set":4,"id":"0f5de14c_017c163e","line":136,"updated":"2026-08-26 21:51:23.000000000","message":"The make_sort_key comment (lines 136-141) states \u0027We negate the timestamp so that older instances get a higher sort value\u0027. After negation older instances (larger created.timestamp()) get a SMALLER key, and the \u0027dec\u0027 branch then flips the value back to +timestamp so the ascending sorted() call puts oldest first. The runtime ordering is correct and matches the documented default [[\u0027age\u0027,\u0027dec\u0027]] (oldest first), but the explanation describes the opposite of what the negation does, and this logic already relies on a double negation that is easy to get wrong. A future maintainer \u0027fixing\u0027 the code to match the comment would invert candidate ordering (newest preempted first) without failing any assertion that documents intent.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: Future edits to the preemption preference logic guided by the incorrect comment can silently invert preemption priority (newest-first instead of oldest-first), which unit tests currently only pin indirectly through example ordering.\n\n**Recommendation**:\nRewrite the comment to state the actual scheme, e.g. \u0027sort() is ascending; for direction \u0027dec\u0027 we use +timestamp so the oldest instances sort first; for \u0027asc\u0027 we use -timestamp so the newest sort first\u0027, and keep the existing oldest-first/newest-first unit tests as the behavioral pin.","commit_id":"19d3332769497a2a02e63e4afa2c47fd77b50474"},{"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":"009808ef51113169e0687ef362bee9cba40345f6","unresolved":false,"context_lines":[{"line_number":473,"context_line":""},{"line_number":474,"context_line":"            action_type \u003d self._get_preemption_action(instance)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"            self.solution.add_action("},{"line_number":477,"context_line":"                action_type\u003daction_type,"},{"line_number":478,"context_line":"                resource_id\u003dinstance.uuid,"},{"line_number":479,"context_line":"                input_parameters\u003d{\u0027resource_name\u0027: instance.name},"}],"source_content_type":"text/x-python","patch_set":4,"id":"7d196ffa_ec8ff217","line":476,"updated":"2026-08-26 21:51:23.000000000","message":"The strategy adds every preemption action with input_parameters\u003d{\u0027resource_name\u0027: instance.name} (workload_preemption.py:476-480). DefaultSolution.add_action merges resource_id into the same dict, so the persisted action input_parameters become {\u0027resource_id\u0027: \u003cuuid\u003e, \u0027resource_name\u0027: \u003cname\u003e}. However the delete and shelve actions (watcher/applier/actions/delete.py and shelve.py, added in the dependency change) declare schemas with only a \u0027resource_id\u0027 property and \u0027additionalProperties\u0027: False. When the applier builds each action, ActionFactory.make_action (watcher/applier/actions/factory.py:40) calls validate_parameters(), which runs jsonschema.validate on exactly this dict, so every delete/shelve action generated by this strategy raises ValidationError (\u0027resource_name\u0027 was unexpected) and the action plan cannot be applied. Other actions that receive resource_name (migration, change_nova_service_state, change_node_power_state) all declare it in their schemas; delete/shelve do not.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Impact**: Every action plan produced by the workload_preemption strategy fails when the applier loads its actions; audits will end in a failed/errored action plan and no instance is ever preempted. The unit tests never load actions through ActionFactory, so this is not caught by the new test suite.\n\n**Priority**: Before merge\n**Recommendation**:\nAdd \u0027resource_name\u0027: {\u0027type\u0027: \u0027string\u0027, \u0027minLength\u0027: 1} to the schemas of the delete and shelve actions (matching migration/change_nova_service_state), or stop passing input_parameters from the strategy. Add one test that runs a generated solution through ActionFactory.make_action (or jsonschema-validates input_parameters against the loaded action schema) to lock the strategy/action contract.","commit_id":"19d3332769497a2a02e63e4afa2c47fd77b50474"},{"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":"7d881a3d51fda27fba6ccc657522605e09d005c4","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    def get_schema(cls):"},{"line_number":215,"context_line":"        return {"},{"line_number":216,"context_line":"            \"properties\": {"},{"line_number":217,"context_line":"                \"cpu_free_percentage\": {"},{"line_number":218,"context_line":"                    \"description\": ("},{"line_number":219,"context_line":"                        \"Target free CPU percentage (allocated). \""},{"line_number":220,"context_line":"                        \"Only evaluated if explicitly set.\""}],"source_content_type":"text/x-python","patch_set":5,"id":"02355b53_25ee14f2","line":217,"updated":"2026-08-27 03:16:42.000000000","message":"get_schema() declares cpu_free_percentage, ram_free_percentage and disk_free_percentage as {\"type\": \"number\", ..., \"default\": None}. Watcher validates strategy parameters with StrictDefaultValidatingDraft4Validator (watcher/common/utils.py:122-158), which fills schema defaults into the parameters dict and then validates it. Filled-in None values then fail the \"type\": \"number\" check (reproduced against the same jsonschema extension logic: \"ValidationError: None is not of type \u0027number\u0027\"). watcher/api/controllers/v1/audit.py:730-743 runs this validator on every audit create with a predefined strategy (parameters default to {}), so any audit that does not explicitly set all three percentage parameters is rejected with \u0027Invalid parameters for strategy\u0027. watcher/decision_engine/strategy/context/default.py:59-66 hits the same failure at execution time when audit.parameters is empty. No other strategy in the tree uses \"default\": None; existing conventions use concrete defaults (e.g. workload_balance threshold 25.0).\n\n**Severity**: HIGH | **Confidence**: 0.92\n\n**Impact**: Operators cannot create (or, with no parameters, execute) an audit using the new strategy except by explicitly supplying all three *_free_percentage values; the documented quick-start and multi-threshold examples fail with \u0027Invalid parameters for strategy\u0027. The new feature is effectively unusable via the API on its documented defaults.\n\n**Priority**: Before merge\n**Recommendation**:\nRemove the \"default\" keys from the three *_free_percentage subschemas (pre_execute already treats a missing key as None), or declare them as \"type\": [\"number\", \"null\"] with \"default\": None so the filled default validates. Add a unit test that runs utils.StrictDefaultValidatingDraft4Validator(get_schema()) on {} and on a partial parameter set like {\u0027cpu_free_percentage\u0027: 20.0} to lock the contract.","commit_id":"8c713b785430238121b232a83978e10ebb614a05"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":41,"context_line":"SORT_KEY_AGE \u003d \u0027age\u0027"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"def filter_preemptible_candidates(instances, min_instance_age):"},{"line_number":45,"context_line":"    \"\"\"Filter instances to only include preemptible candidates."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    This is a reusable filtering function that evaluates every instance"}],"source_content_type":"text/x-python","patch_set":8,"id":"b277de64_6803d487","line":44,"range":{"start_line":44,"start_character":4,"end_line":44,"end_character":33},"updated":"2026-09-03 01:19:42.000000000","message":"i feel like we will ant to factor this out of this in the future\nits fien for now but i think we want to move to a predicate model where we can ask the CDM to give use the set of candiate instnace and pass in a predicate to elimeitnte/iclude instances in teh future.\n\nthat out of scope fo this patch, it just feels like a capablity that shoudl be provided by the CDM rather then each stragy implementing it themselves when needed","commit_id":"6c3f4f93c2e97eca16fe1ea621e7ff6e3a9dfd15"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":81,"context_line":"                \"Instance %s excluded: watcher_exclude is True\", instance.uuid"},{"line_number":82,"context_line":"            )"},{"line_number":83,"context_line":"            continue"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"        # Must be in ACTIVE state to be preempted"},{"line_number":86,"context_line":"        if instance.state !\u003d element.InstanceState.ACTIVE.value:"},{"line_number":87,"context_line":"            LOG.debug("},{"line_number":88,"context_line":"                \"Instance %s excluded: state is %s, not active\","},{"line_number":89,"context_line":"                instance.uuid,"},{"line_number":90,"context_line":"                instance.state,"},{"line_number":91,"context_line":"            )"},{"line_number":92,"context_line":"            continue"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        # Conditional invariant: minimum instance age"}],"source_content_type":"text/x-python","patch_set":8,"id":"2f6d0511_dafc5c1d","line":91,"range":{"start_line":84,"start_character":1,"end_line":91,"end_character":13},"updated":"2026-09-03 01:19:42.000000000","message":"we can do this but that is not a hard requireemtn\n\nyou shoudl be able to prempet stoped, active, paused, suspended or errord instnaces\n\nthis proably shoudl have a todo to remove this limiation\n\ni woudl not expect none Acitvie instnace to be skipped in genral and the design did nto restrict ti to active isntnace only.\n\nso this is ok as a first pass but its not really a constiant we shoudl continue to enforce","commit_id":"6c3f4f93c2e97eca16fe1ea621e7ff6e3a9dfd15"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":284,"context_line":"        }"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _get_total_cluster_resources(self):"},{"line_number":287,"context_line":"        \"\"\"Get total cluster resources across all enabled compute nodes."},{"line_number":288,"context_line":""},{"line_number":289,"context_line":"        :returns: dict with keys \u0027vcpu\u0027, \u0027memory\u0027, \u0027disk\u0027"},{"line_number":290,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"c43f13b7_ed59a29c","line":287,"range":{"start_line":287,"start_character":10,"end_line":287,"end_character":72},"updated":"2026-09-03 01:19:42.000000000","message":"this is actully not entrily accurate, the funciton is getign the resouces form the already scoped model. when we scope an audite to a subset of the cloud we remove the comptue nodes form teh CDM\nso its _get_total_cluster_resources but rather\n\n_get_total_scoped_cluster_resources\n\nbut i kind of feel like we may want to define get_cluser_resouces_totals or  get_cluser_resouces_summary on the cdm and have it return a map of resouce to ammount  posisbel also with usage so {resocue : (total, used)}\n\n\ni thinik we can condier that refactor in a follow up.\n\neffectivly we want to flaten the data models into computign the sumation of each resouce from each node and we can do that genericlly rather then sepcial casing in the future","commit_id":"6c3f4f93c2e97eca16fe1ea621e7ff6e3a9dfd15"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":392,"context_line":"        \"\"\""},{"line_number":393,"context_line":"        return list(self.compute_model.get_all_instances().values())"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"    def pre_execute(self):"},{"line_number":396,"context_line":"        self._pre_execute()"},{"line_number":397,"context_line":"        self.cpu_free_percentage \u003d self.input_parameters.get("},{"line_number":398,"context_line":"            \u0027cpu_free_percentage\u0027, None"},{"line_number":399,"context_line":"        )"},{"line_number":400,"context_line":"        self.ram_free_percentage \u003d self.input_parameters.get("},{"line_number":401,"context_line":"            \u0027ram_free_percentage\u0027, None"},{"line_number":402,"context_line":"        )"},{"line_number":403,"context_line":"        self.disk_free_percentage \u003d self.input_parameters.get("},{"line_number":404,"context_line":"            \u0027disk_free_percentage\u0027, None"},{"line_number":405,"context_line":"        )"},{"line_number":406,"context_line":"        self.min_instance_age \u003d self.input_parameters.get("},{"line_number":407,"context_line":"            \u0027min_instance_age\u0027, DEFAULT_MIN_INSTANCE_AGE"},{"line_number":408,"context_line":"        )"},{"line_number":409,"context_line":"        self.preemption_preference \u003d self.input_parameters.get("},{"line_number":410,"context_line":"            \u0027preemption_preference\u0027, [[\u0027age\u0027, \u0027dec\u0027]]"},{"line_number":411,"context_line":"        )"},{"line_number":412,"context_line":"        self.preemption_mode \u003d self.input_parameters.get("},{"line_number":413,"context_line":"            \u0027preemption_mode\u0027, DEFAULT_PREEMPTION_MODE"},{"line_number":414,"context_line":"        )"},{"line_number":415,"context_line":""},{"line_number":416,"context_line":"        # Convert preemption_preference to list of tuples"},{"line_number":417,"context_line":"        self.preemption_preference \u003d ["},{"line_number":418,"context_line":"            tuple(p) for p in self.preemption_preference"},{"line_number":419,"context_line":"        ]"},{"line_number":420,"context_line":""},{"line_number":421,"context_line":"    def do_execute(self, audit\u003dNone):"},{"line_number":422,"context_line":"        \"\"\"Strategy execution phase."},{"line_number":423,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"a935fcff_363b6ed7","line":420,"range":{"start_line":395,"start_character":2,"end_line":420,"end_character":1},"updated":"2026-09-03 01:19:42.000000000","message":"this is not corect\n\nyou shoudl not be adding datavalues to sell outside fo __init__ or at the latest __post_init__\n\nespically if your going to use those values form _has_any_threshold or toher palces wehre it coudl lead to an atibute error.\n\nall of this shoudl be done when the stragy obejct is constuted","commit_id":"6c3f4f93c2e97eca16fe1ea621e7ff6e3a9dfd15"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"58cc7a77f85791f63f9123c212af19a6091a1eb7","unresolved":true,"context_lines":[{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        # Sort candidates according to preemption_preference"},{"line_number":459,"context_line":"        sorted_candidates \u003d sort_candidates("},{"line_number":460,"context_line":"            candidates, self.preemption_preference"},{"line_number":461,"context_line":"        )"},{"line_number":462,"context_line":""},{"line_number":463,"context_line":"        LOG.debug("}],"source_content_type":"text/x-python","patch_set":8,"id":"09718fa8_141cfa83","line":460,"range":{"start_line":460,"start_character":24,"end_line":460,"end_character":50},"updated":"2026-09-03 01:19:42.000000000","message":"your mixing programign styles here\n\n\non one hand your almost impleitng a strage verison of the build patter when your are building a set of constrati vai pre_execute that you are then fucntioally applying here\n\non the ohter hand you are mutaten the shared state of the object instad of jsut alltin a function here\n\nso this is really not that testabel and you get all the complexity of sideffect adn object oration without the encapsuslation as you are reusing   self.preemption_preference for multiple things eventually converting it into a list of tuples which is what your pasing here if an only if pre_execute is called before execute so this data fields type is now temperally coploeld to the point in the call graph you access it in.\n\n\ndont do that\n\na named variabel shoudl have a singel time for its entire lifetyme if you are saving it on an object\n\nwithin a fucntion names can be reused but its not ok to do that on an obejct and have it change acoress diffent fucntions","commit_id":"6c3f4f93c2e97eca16fe1ea621e7ff6e3a9dfd15"}],"watcher/tests/unit/decision_engine/strategy/strategies/test_workload_preemption.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":"a673a7b1d99149263a97286c20042dc6442c7735","unresolved":false,"context_lines":[{"line_number":842,"context_line":"        self.strategy.do_execute()"},{"line_number":843,"context_line":""},{"line_number":844,"context_line":"        # Only uuid-0002 should be preempted"},{"line_number":845,"context_line":"        if len(self.strategy.solution.actions) \u003e 0:"},{"line_number":846,"context_line":"            for action in self.strategy.solution.actions:"},{"line_number":847,"context_line":"                self.assertNotEqual("},{"line_number":848,"context_line":"                    \"uuid-0001-0000-0000-000000000001\","}],"source_content_type":"text/x-python","patch_set":1,"id":"1a3af978_1ad85c44","line":845,"updated":"2026-08-08 03:17:45.000000000","message":"The test guards its assertions with `if len(self.strategy.solution.actions) \u003e 0:` before iterating. If no actions are taken (e.g., due to a bug in filter_preemptable_candidates), the test body never executes and the test passes without verifying any behavior. The test should assert that exactly o...\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: A regression that prevents any preemption from occurring (e.g., filter_preemptable_candidates excluding all instances) would not be caught by this test. The test cannot distinguish between \u0027young instance correctly excluded\u0027 and \u0027no instances preempted at all.\u0027\n\n**Suggestion**:\nReplace the `if len \u003e 0` guard with `self.assertEqual(1, len(self.strategy.solution.actions))` and assert the action targets uuid-0002 (the old enough instance). This matches the pattern used in other tests in the same file.","commit_id":"ca7aaf96ba531eb6543d38ed30eb28c9505f6035"},{"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":"c04b8c19615c6ff6b3393a8b85c5e81b270f434c","unresolved":false,"context_lines":[{"line_number":868,"context_line":"        self.strategy.do_execute()"},{"line_number":869,"context_line":""},{"line_number":870,"context_line":"        # Only uuid-0002 should be preempted"},{"line_number":871,"context_line":"        if len(self.strategy.solution.actions) \u003e 0:"},{"line_number":872,"context_line":"            for action in self.strategy.solution.actions:"},{"line_number":873,"context_line":"                self.assertNotEqual("},{"line_number":874,"context_line":"                    \"uuid-0001-0000-0000-000000000001\","}],"source_content_type":"text/x-python","patch_set":2,"id":"9e278b27_b385de1f","line":871,"updated":"2026-08-13 17:25:02.000000000","message":"The test wraps its core assertion in an \u0027if len(actions) \u003e 0\u0027 guard, meaning the test passes even when zero preemption actions are produced. The test\u0027s stated purpose is to verify that young instances are protected while old ones are preempted, but the conditional guard means a no-op strategy wou...\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: A regression that causes the strategy to skip all instances (not just young ones) would not be caught by this test. The test should assert that exactly 1 action is produced and that it targets the correct (old enough) instance.\n\n**Suggestion**:\nReplace the conditional guard with an unconditional assertion: assertEqual(1, len(self.strategy.solution.actions)) followed by assertEqual on the action\u0027s resource_id to confirm it preempts uuid-0002 (the old-enough instance), and assertNotEqual or assert that uuid-0001 (the young instance) is never preempted.","commit_id":"4a1f1a6a839ebafd3e40db49309ec960aead5bf0"},{"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":"2fbf51a21c3a819b8814a2afac3aa262249a8df4","unresolved":false,"context_lines":[{"line_number":793,"context_line":"                    \"uuid-0002-0000-0000-000000000002\","},{"line_number":794,"context_line":"                    vcpus\u003d4,"},{"line_number":795,"context_line":"                    created\u003ddatetime.datetime("},{"line_number":796,"context_line":"                        2026, 6, 30, 23, 0, 0, tzinfo\u003ddatetime.timezone.utc"},{"line_number":797,"context_line":"                    ),"},{"line_number":798,"context_line":"                ),"},{"line_number":799,"context_line":"            ],"}],"source_content_type":"text/x-python","patch_set":8,"id":"6a0c0329_91a9e9bc","line":796,"updated":"2026-08-27 18:13:07.000000000","message":"test_min_instance_age_protects_young_instances wraps its only meaningful assertion in `if len(self.strategy.solution.actions) \u003e 0:`. If a regression in filter_preemptible_candidates or do_execute caused the strategy to emit no actions at all (e.g., the age filter incorrectly excluding both the young and the old instance), the test still passes because the loop body never executes. The test therefore does not actually verify that the old-enough instance (uuid-0002) is preempted.\n\n**Severity**: SUGGESTION | **Confidence**: 0.85\n\n**Impact**: A regression that over-filters candidates (returning an empty or wrong candidate set) would not be caught, weakening protection of the min_instance_age invariant that shields recently created instances from preemption.\n\n**Recommendation**:\nReplace the guard with an unconditional assertion, mirroring test_watcher_excluded_instances_not_preempted: assertEqual(1, len(self.strategy.solution.actions)) and assertEqual(\u0027uuid-0002-0000-0000-000000000002\u0027, actions[0][\u0027input_parameters\u0027][\u0027resource_id\u0027]).","commit_id":"6c3f4f93c2e97eca16fe1ea621e7ff6e3a9dfd15"}]}