| opendevreview | Winicius Allan Bezerra da Silva proposed openstack/watcher-specs master: Add Preemptible Instances spec for 2027.1 cycle https://review.opendev.org/c/openstack/watcher-specs/+/1014252 | 03:24 |
|---|---|---|
| amoralej | watcher weekly meeting is starting in 18 minutes, remember to add your topics to https://etherpad.opendev.org/p/openstack-watcher-irc-meeting | 12:42 |
| opendevreview | Merged openstack/watcher master: reno: Update master for unmaintained/2025.1 https://review.opendev.org/c/openstack/watcher/+/1013202 | 12:47 |
| amoralej | meeting time | 13:01 |
| amoralej | #startmeeting watcher | 13:01 |
| opendevmeet | Meeting started Thu Nov 5 13:01:35 2026 UTC and is due to finish in 60 minutes. The chair is amoralej. Information about MeetBot at http://wiki.debian.org/MeetBot. | 13:01 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 13:01 |
| opendevmeet | The meeting name has been set to 'watcher' | 13:01 |
| amoralej | who is arount today? | 13:01 |
| dviroel | o/ | 13:01 |
| jgilaber | o/ | 13:02 |
| morenod | o/ | 13:02 |
| amoralej | courtesy ping: rlandy sean-k-mooney chandankumar | 13:03 |
| amoralej | let's start with the first topic in the agenda | 13:03 |
| amoralej | #topic 2025.1 already moved to unmaintained | 13:04 |
| amoralej | jgilaber added this one, is there any pending change we need to commit wrt the topic? | 13:04 |
| jgilaber | no | 13:04 |
| jgilaber | I just wanted to do a quick announcement | 13:05 |
| dviroel | did we abandoned all 2025.1 backports? | 13:05 |
| amoralej | #info https://review.opendev.org/c/openstack/releases/+/1010956 is merged | 13:05 |
| amoralej | i think we did | 13:05 |
| jgilaber | yes, well elod did | 13:05 |
| dviroel | ah ok | 13:05 |
| jgilaber | we also dropped the 2025.1 job from the tempest plugin check and gate | 13:05 |
| amoralej | #info https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/1013016 | 13:05 |
| amoralej | yep, that :) | 13:05 |
| amoralej | let's move to next topic, we have a packed agenda today | 13:06 |
| amoralej | #topic Preemptible Instances spec refreshed. Reviews are appreciated | 13:06 |
| jgilaber | nothing else from my side, just fyi and thanks for the reviews | 13:06 |
| amoralej | thanks jgilaber for taking care | 13:06 |
| amoralej | #link https://review.opendev.org/c/openstack/watcher-specs/+/1014252 | 13:06 |
| amoralej | winiciusallan is not here but left this request for reviews | 13:07 |
| amoralej | he has sent the review for 2027.1 | 13:07 |
| jgilaber | I haven't looked at the spec yet, but looks like there is some issue with section headers | 13:07 |
| amoralej | yes it's failing syntax check | 13:08 |
| dviroel | I think that when reviewing, it is easier if we compare with the one from previous cycle, to understand what changed | 13:08 |
| jgilaber | I've added it to my review queue | 13:08 |
| jgilaber | +1 | 13:08 |
| dviroel | but yeah, I think that we can comment in the spec itself | 13:09 |
| amoralej | also he left a comment in the agenda "The instance's CDM has `disk` as the sum of root_disk, ephemeral, and swap. Would we want to consider only one of these?" | 13:09 |
| amoralej | we can comment on that in the review | 13:09 |
| jgilaber | amoralej, that is the change you did for boot from volume right? | 13:09 |
| amoralej | yes | 13:10 |
| amoralej | i'd say it should use the disk field as is | 13:10 |
| amoralej | but need to check | 13:10 |
| amoralej | in the spec history he commented on the change, we have some info there | 13:10 |
| amoralej | hi winiciusallan[m], just on time :) | 13:11 |
| winiciusallan[m] | sorry folks, my IRC client was tricking me | 13:11 |
| dviroel | yeah, i think that CDM should hide these details, and strategy would not need to worry | 13:11 |
| winiciusallan[m] | I'm now on o/ | 13:11 |
| dviroel | it is good to have a common behavior across strategies | 13:12 |
| amoralej | we were commenting on your topic | 13:12 |
| amoralej | wrt disk, i think we agree that it should use the `disk` field as found in the model, right? | 13:12 |
| dviroel | we can comment in the spec, but yeah | 13:13 |
| jgilaber | I need more details, but at first glance I'd agree, yes | 13:13 |
| winiciusallan[m] | good, that was my main concern atm | 13:13 |
| amoralej | winiciusallan[m], anything to remark in the spec other than the change to limit the sorting options to age, vcpu, ram and disk? | 13:13 |
| winiciusallan[m] | nope. diffing the specs, these were the main changes, nothing else has changed from the feature perspective | 13:14 |
| dviroel | ++ | 13:15 |
| dviroel | thanks for proposing it | 13:15 |
| amoralej | yep, thanks for the update | 13:15 |
| amoralej | let's move to the next topic | 13:15 |
| winiciusallan[m] | thanks! | 13:15 |
| amoralej | #topic Zone Migration - Drain AZ | 13:15 |
| amoralej | dviroel, stage is yours | 13:15 |
| dviroel | ah | 13:16 |
| dviroel | so yeah, the spec has some updates based on comments from reviewers | 13:16 |
| dviroel | while i was working on a PoC a hit an issue | 13:16 |
| dviroel | which I would like to get some feedback | 13:16 |
| dviroel | can be here on in the spec itseld | 13:17 |
| dviroel | the main issue is with the pinned_az attribute from a server/instance | 13:17 |
| dviroel | this is part of the nova request spec object | 13:17 |
| dviroel | which can now (since 2.104) be updated via server update | 13:17 |
| dviroel | so users can pin or unpin a server to an AZ | 13:18 |
| dviroel | the issue is that we don't reveice any notification about it | 13:18 |
| dviroel | since it only udpates the Req Spec object in the database | 13:18 |
| dviroel | so no instance.update is generated | 13:18 |
| dviroel | the issue is: we may have a pinned_az in the CDM that is not up to date | 13:19 |
| dviroel | and strategy may rely on that to take some decisions | 13:19 |
| dviroel | it could be only updated via periodic collector run | 13:19 |
| dviroel | I made a sugestion in the spec, for zone_migration | 13:19 |
| dviroel | amoralej: already commented there, so he is aware | 13:20 |
| amoralej | yep | 13:20 |
| dviroel | the idea there is to defer the SKIP of a pinned_az to the Action itself | 13:20 |
| dviroel | since during the action execution, it retrieve details of the server via nova API | 13:20 |
| dviroel | so user would still choose if they want to repin an instance | 13:21 |
| dviroel | if they decide to not reping, and the pinned_az is conflicting with the new destination (node or AZ) | 13:21 |
| dviroel | it would skip the action there | 13:21 |
| dviroel | this is different from previous proposal that would skip the pinned instance in the strategy | 13:22 |
| jgilaber | that seems like a good option to me | 13:22 |
| dviroel | so these instances would not be considered as a migration | 13:22 |
| dviroel | both have cons in my opinion | 13:22 |
| jgilaber | proposing migrations is not expensive | 13:22 |
| dviroel | ideally we would have that info always up to date in the CDM | 13:22 |
| dviroel | but is very unlikely that Res Spec updates will generate notification in nova | 13:23 |
| amoralej | could nova send a notification or there is any architectural or policy constraint for them to send it? | 13:23 |
| dviroel | sean-k-mooney: or melwitt may have a better view of that and may comment later.. | 13:23 |
| dviroel | amoralej: nova send instance.update notification for instances attributes updates. | 13:24 |
| amoralej | it's true that sending actions is not expensive (depending on how many) | 13:24 |
| dviroel | and pinned_az, as scheduler_hints are Req Spec object attributes | 13:24 |
| dviroel | instance.update doesn't even send Req Spec info in their notifications | 13:25 |
| amoralej | but, it'd be good that the strategy creates the AP as accurate as posible, and skip would be reserved for more "abnormal" cases imo | 13:25 |
| dviroel | yeah | 13:26 |
| amoralej | although i agree on having a skip condition at the just in case, i.e. an originally unpinned vm is pinned since the last collection | 13:26 |
| dviroel | i am not happy with any of the 2 options atm | 13:26 |
| amoralej | yep, it's choosing which bad is worst :) | 13:26 |
| jgilaber | atm I'm leaning towards the second option, but I need to read again the spec | 13:27 |
| dviroel | we could update those fields on any instance.update too, like a state change, but still doesn 't solve | 13:27 |
| jgilaber | I have not looked at it since PTG bascially | 13:27 |
| dviroel | yeah, the idea was to explain the issue and possibilities, and we can get feedbacks in the spec itself | 13:28 |
| amoralej | if you compare to previous PS the difference is more clear | 13:28 |
| dviroel | +1 | 13:28 |
| dviroel | note that, change in the Action can aslo benefit other strategies | 13:29 |
| amoralej | so the only actual problem with doing it in the strategy is, a pinned instance is unpinned after last collection and user runs with skip_pinned_instances=true | 13:29 |
| amoralej | right? | 13:29 |
| dviroel | yeah, I think so | 13:29 |
| amoralej | assuming we implement skip condition in the action anyway | 13:30 |
| dviroel | yes | 13:30 |
| amoralej | ok, so i guess we can continue discussing in the review | 13:31 |
| dviroel | mainly because we don't need to check if the pinned_az is == src_az | 13:31 |
| dviroel | we only need to check if is pinned | 13:31 |
| dviroel | or not | 13:31 |
| dviroel | we would filter src_az -> nodes -> instances | 13:31 |
| dviroel | so if they are pinned and user don't want to repin, skipo | 13:31 |
| dviroel | but if user already unpinned those... | 13:32 |
| amoralej | yep, in that case an unpinned vm wouldn't be migrated | 13:32 |
| dviroel | as sean mentioned, there are some cases that all instances can be pinned to default az | 13:32 |
| amoralej | that's an interesting one ... | 13:33 |
| amoralej | the "fake pinned" to nova az | 13:33 |
| amoralej | sean-k-mooney was in favor of unpinning them unconditionally iiuc | 13:34 |
| dviroel | if the user manually unpinned all, it would be safe to user skip_pinned_instances=false too, since in the Action, we will not pin if the instance is not pinned | 13:34 |
| dviroel | amoralej yeah, that's another issue that i am not convinced that watcher should handle | 13:35 |
| dviroel | it is not up to watcher fix this i think | 13:35 |
| amoralej | i'm not sure, tbh | 13:36 |
| amoralej | but apparently, from nova point of view, instances should not be pinned to nova az | 13:36 |
| dviroel | so then it is up to nova deny | 13:37 |
| amoralej | that's also true :) | 13:37 |
| dviroel | but if they are already pinned, there is a new api to unpin | 13:37 |
| dviroel | so users can fix that | 13:37 |
| amoralej | i think we can keep the discussion in the review | 13:37 |
| dviroel | ack | 13:37 |
| dviroel | lets move to next topic | 13:37 |
| amoralej | can we move to next topic? | 13:37 |
| amoralej | ok | 13:37 |
| dviroel | yeah | 13:38 |
| amoralej | #topic Reviews | 13:38 |
| amoralej | #topic Fix babel configs and add translations tox target | https://review.opendev.org/c/openstack/watcher-dashboard/+/998324 | 13:38 |
| amoralej | that's the one related to translation in the dashboard | 13:38 |
| dviroel | the one that we mention in previous meeting | 13:39 |
| amoralej | yep | 13:39 |
| amoralej | so i guess it's just call for reviews | 13:39 |
| dviroel | yeah | 13:39 |
| amoralej | also https://review.opendev.org/c/openstack/watcher-dashboard/+/1012875 | 13:39 |
| dviroel | ah, i was talking about this one ^ | 13:40 |
| dviroel | the fix for https://launchpad.net/bugs/2167806 | 13:40 |
| amoralej | the first one is adding support for translation in babel and tox | 13:40 |
| amoralej | second is fixing bug | 13:40 |
| amoralej | i'm adding to the list of pending to review | 13:41 |
| dviroel | ++ | 13:42 |
| amoralej | chandan added a screenshot of the reproducer and fixed | 13:42 |
| amoralej | next review in the list is mine | 13:43 |
| amoralej | #topic https://review.opendev.org/c/openstack/watcher/+/1013297 Fix for https://launchpad.net/bugs/2170351 | 13:43 |
| amoralej | that's fix for one of the bugs we discussed recently | 13:43 |
| amoralej | the one for workload_stabilization | 13:44 |
| dviroel | oh nice | 13:44 |
| jgilaber | looks reasonable | 13:45 |
| amoralej | feel free to review and provide feedback there, no need to discuss it now | 13:46 |
| amoralej | we can move to bugs section | 13:46 |
| dviroel | ack, xs change, easy to review | 13:46 |
| amoralej | yep :) | 13:46 |
| amoralej | #topic bugs | 13:46 |
| amoralej | #topic https://bugs.launchpad.net/watcher/+bug/2172453 - host_maintenance strategy fails when maintenance_node is excluded by audit scope | 13:46 |
| amoralej | i think we started discussing this with no conclusion | 13:47 |
| jgilaber | yes iirc we were not sure if we consider the described behaviour a bug | 13:47 |
| amoralej | so, the question is, what should be the resulting state of a host_maintenance strategy where maintenance_node has vms but doesn't migrate any | 13:48 |
| dviroel | we may need to define a common behavior across strategies | 13:48 |
| jgilaber | i.e if an audit can't formulate a plan to optimize it's goad, failed it a valid state | 13:48 |
| dviroel | same issue could happen with zone_migration, etc.. | 13:48 |
| amoralej | yes | 13:49 |
| dviroel | if the input parameter specific a resource that does not exists, i think that it should fail | 13:49 |
| sean-k-mooney | dviroel: i was orgianlly in favor of skipping pinned instnace, but durign the ptg i was convice that for backward compatibtliy with the existng bevhior that unpinng them woudl be correct | 13:49 |
| jgilaber | in cases like this failing is valid imo because it's because of user input | 13:49 |
| sean-k-mooney | or rather not skipping them | 13:49 |
| sean-k-mooney | and sorry for being late o/ | 13:49 |
| amoralej | np | 13:50 |
| morenod | but the input is correct, you can select the same host, and the audit template accepts it | 13:50 |
| morenod | FAILED result may cause the user to review what is wrong | 13:50 |
| jgilaber | right, but with the given input (parameter+scope) you can't achieve the goal | 13:51 |
| amoralej | so, for me, for the particular case o host_maintenance where it only manages a host, excluding it in the scope sounds as incorrect | 13:51 |
| sean-k-mooney | well i woudl condier the input invoald | 13:51 |
| sean-k-mooney | bu tnot at teh schema level | 13:51 |
| morenod | it can be a wrong usage of the parameters, but not incorrect | 13:51 |
| sean-k-mooney | its logicly invalid at the stragey level | 13:51 |
| sean-k-mooney | as you explxitly requested oen of the 2 node specifed eslwehre in the request to be exlcuded form the model | 13:52 |
| amoralej | something we probably should start doing is using the status_message of audits | 13:52 |
| morenod | imho, the correct result is an empty action plan when the parameters selected points to that, and FAILED result if there is a real failure creating the action plan | 13:52 |
| sean-k-mooney | so this shoudl fail and either be rejected syncornsly in the api or more likely async in the desion engion when the stragey is evaulate in the audit | 13:53 |
| sean-k-mooney | with a reason set in the status message | 13:53 |
| sean-k-mooney | i think xpected result: the audit succeeds with an empty action plan. is incorrect | 13:53 |
| jgilaber | +1, I think the failed result is correct, the problem might be whether it's actually visible to the user | 13:53 |
| dviroel | needs to be async, to cover the continuous audit case | 13:53 |
| sean-k-mooney | it should be failed and the status mesage filed shoudl say why IMO | 13:54 |
| sean-k-mooney | the user has explcity specified two contractory requirements | 13:54 |
| amoralej | +1 | 13:54 |
| sean-k-mooney | {"scope": [{"compute": [{"exclude": [{"compute_nodes": [{"name": "compute-1"}]}]}]}]} -> comptue-1 must be exlcuded form the data model used in this audit | 13:54 |
| amoralej | the problem in this particular case is not that watcher can not optimize it, it's that the parameters are contradictory | 13:55 |
| sean-k-mooney | {"maintenance_node": "compute-1", "backup_node": "compute-2"} -> compute-1 is requird for the stragey to be implemented | 13:55 |
| amoralej | for me it's different that a consolidation that can not consolidate because of the load, i.e | 13:55 |
| sean-k-mooney | it is and we can convay that in the status_message | 13:56 |
| amoralej | but i agree that a status_message would be needed | 13:56 |
| sean-k-mooney | or reason what ever that filed is caled | 13:56 |
| sean-k-mooney | ya its status_message | 13:56 |
| amoralej | btw, i'll be sending a patch to support updating status_message in audits when updating the state (i want it for the upgrades change too) | 13:56 |
| sean-k-mooney | i tought we coudl aslready do that internally | 13:57 |
| sean-k-mooney | status_message is intenally not setable via teh rest api | 13:57 |
| amoralej | well, for audits, we don't have the option in the update_audit_state method | 13:57 |
| amoralej | we never used it anywhere yet | 13:58 |
| sean-k-mooney | ah ok is it two diffent calls today | 13:58 |
| amoralej | in audits, we do in actionplans and actions | 13:58 |
| sean-k-mooney | ack | 13:58 |
| sean-k-mooney | that makes sense to me to add | 13:58 |
| amoralej | so, i don't know if we have an agreement on the bug | 13:58 |
| sean-k-mooney | although the reall way to do this is not via a new method | 13:58 |
| sean-k-mooney | we shoudl be doing this by takign the audit ovo seting the filed value and calling save | 13:58 |
| amoralej | i'm updating the existing one | 13:58 |
| sean-k-mooney | we shoudl not generlaly ahve helper method for data filed updtes unless we are enforece soem pre/post contaions in a central way | 13:59 |
| amoralej | let's discuss that separatedly :) | 13:59 |
| amoralej | back to the bug | 13:59 |
| sean-k-mooney | ack os as writen i woudl mark the bugs as invalid based on expected outcome | 14:00 |
| sean-k-mooney | but we could mark it low | 14:00 |
| sean-k-mooney | and recored that we shoudl update the status message | 14:00 |
| sean-k-mooney | to say why it failed | 14:00 |
| amoralej | would that be acceptable for the rest? | 14:00 |
| jgilaber | yes, sounds good to me | 14:00 |
| dviroel | /me was distracted | 14:01 |
| dviroel | so conflicting input parameters and scope should invalidate the audit, is that it? | 14:01 |
| amoralej | yes | 14:02 |
| amoralej | with status_message | 14:02 |
| dviroel | yeah, but it is still up to the strategy to check that? | 14:02 |
| dviroel | not in the api? | 14:02 |
| amoralej | yes, there is logic depending on each strategy | 14:02 |
| dviroel | ack | 14:02 |
| sean-k-mooney | desison engine | 14:02 |
| dviroel | makes sense | 14:02 |
| sean-k-mooney | wthere its ceneralised or in the stragey is an implemation detial | 14:02 |
| sean-k-mooney | everually it woudl be nice to have a commen helper for this | 14:03 |
| amoralej | morenod, ^ is that good enough? | 14:03 |
| sean-k-mooney | but atht woudl requrie us to have some normaliseation of the input parmaters | 14:03 |
| sean-k-mooney | which is out of scope fo the bug | 14:03 |
| morenod | ok | 14:04 |
| amoralej | I'll update the bug | 14:04 |
| amoralej | ok, we are out of time | 14:04 |
| amoralej | although there is no more time, just to mention morenod has found memleak https://bugs.launchpad.net/watcher/+bug/2173078 | 14:05 |
| amoralej | we'll discuss next week | 14:05 |
| amoralej | so, i think we can close the meeting | 14:05 |
| amoralej | any last minute topic to discuss? | 14:05 |
| amoralej | Thanks all for joining | 14:06 |
| amoralej | #endmeeting | 14:06 |
| opendevmeet | Meeting ended Thu Nov 5 14:06:34 2026 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 14:06 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/watcher/2026/watcher.2026-11-05-13.01.html | 14:06 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/watcher/2026/watcher.2026-11-05-13.01.txt | 14:06 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/watcher/2026/watcher.2026-11-05-13.01.log.html | 14:06 |
| dviroel | thanks amoralej++ | 14:06 |
| *** gmaan is now known as gmaan_afk | 14:20 | |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Add support to update status_message in update_audit_state https://review.opendev.org/c/openstack/watcher/+/1014342 | 15:03 |
| amoralej | ^ the change i mentioned in the call. Note we already use a method update_audit_state to update the audit state. I'm just adding the option to set the status_message too | 15:04 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Fix duplicate workload stabilization migrations https://review.opendev.org/c/openstack/watcher/+/1013297 | 15:24 |
| *** gmaan_afk is now known as gmaan | 16:12 | |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!