| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Remove Nova API calls from zone_migration strategy https://review.opendev.org/c/openstack/watcher/+/988284 | 09:27 |
|---|---|---|
| amoralej_ | watcher meeting is starting in 20 minutes, remember to add your topics to https://etherpad.opendev.org/p/openstack-watcher-irc-meeting | 11:37 |
| amoralej__ | #startmeeting Watcher 28 May 2026 | 12:00 |
| opendevmeet | Meeting started Thu May 28 12:00:48 2026 UTC and is due to finish in 60 minutes. The chair is amoralej__. Information about MeetBot at http://wiki.debian.org/MeetBot. | 12:00 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 12:00 |
| opendevmeet | The meeting name has been set to 'watcher_28_may_2026' | 12:00 |
| amoralej__ | meeting time! | 12:00 |
| *** amoralej__ is now known as amoralej | 12:00 | |
| amoralej | last minute to add your topics to the agenda and we will start | 12:01 |
| sean-k-mooney | o/ | 12:01 |
| chandankumar | o/ | 12:02 |
| *** jgilaber_ is now known as jgilaber | 12:02 | |
| jgilaber | o/ | 12:02 |
| morenod | o/ | 12:02 |
| amoralej | #topic 988964: Extract BaseHelper for SDK connection setup | https://review.opendev.org/c/openstack/watcher/+/988964 | 12:03 |
| amoralej | that's from jgilaber, go ahead please | 12:03 |
| jgilaber | thank amoralej | 12:03 |
| jgilaber | I wanted to ask for reviews for this patch | 12:03 |
| jgilaber | this was a suggestion from dviroel | 12:04 |
| jgilaber | to have a base class for the different classes to reuse the code for dealing with the openstacksdk | 12:04 |
| jgilaber | I would like to have some feedback even if it's not merged immediately to proceed with the work on the placement helper on top of it | 12:05 |
| amoralej | yeah, i started checking but i got sidetracked and missed it | 12:05 |
| * dviroel o/ | 12:05 | |
| sean-k-mooney | without lookign the idea sounds reasonbale to me | 12:06 |
| sean-k-mooney | nova has some utility methods for handelting the sdk connection | 12:07 |
| sean-k-mooney | which serve a similr fucniton | 12:07 |
| amoralej | yeah, me too, maybe the name BaseHelper may be more specific, BaseClientsHelper or something like that, but sounds reasonable | 12:07 |
| sean-k-mooney | i dont really have an issue with that being in a class as methods instead | 12:07 |
| dviroel | ack, i will take a look on that, not sure why i didn't voted before.. | 12:07 |
| sean-k-mooney | ya we could impve the naming | 12:07 |
| sean-k-mooney | one thing to note however is that | 12:08 |
| sean-k-mooney | with the sdk | 12:08 |
| sean-k-mooney | we can use a signel sdk conenction to talk to many services | 12:08 |
| sean-k-mooney | we do not need per service connections | 12:09 |
| sean-k-mooney | althogh we may want to | 12:09 |
| sean-k-mooney | i.e. if difefnt service are in deifferen regions in the future | 12:09 |
| sean-k-mooney | so i think its fine to start like this | 12:09 |
| sean-k-mooney | adn we cna condier if we want to share or not suare a connection later | 12:09 |
| sean-k-mooney | the current code assuems each serivce has its own client so we do not need to change that in this refactor | 12:10 |
| amoralej | yes, i think this is more flexible to handle different configurations for different services | 12:10 |
| sean-k-mooney | jgilaber: my main feedback is you have no test change with this | 12:10 |
| jgilaber | yes, I followed the existing pattern since I did not have a strong use case for sharing the conection | 12:10 |
| sean-k-mooney | so you shoudl at least have some new unit tests for the base helper | 12:10 |
| jgilaber | sean-k-mooney, that's fair, I'll tests after the meeting | 12:11 |
| sean-k-mooney | cool so +0.5 until that is adressed but i dotn really see any blockers | 12:11 |
| amoralej | i guess we can move to next topic? | 12:12 |
| sean-k-mooney | +1 | 12:12 |
| jgilaber | yes, that is all from me, thanks | 12:13 |
| amoralej | #topic 969840: Add spec for Audit Pipeline feature | 12:13 |
| amoralej | #link https://review.opendev.org/c/openstack/watcher-specs/+/969840 | 12:13 |
| dviroel | o/ | 12:13 |
| amoralej | all yours dviroel | 12:13 |
| dviroel | I sent a update to this spec yesterday | 12:14 |
| dviroel | there a a couple of changes | 12:14 |
| dviroel | sean-k-mooney proposed some changes and some additions | 12:14 |
| dviroel | most of them were reflected there | 12:14 |
| dviroel | here is some of the most important changes | 12:15 |
| dviroel | 1) reduce the scope to ONESHOT audit pipeline only, CONTINUOUS and EVENT types can be added in future releases | 12:15 |
| dviroel | the main reason is that it would reduce code and testing effort, making more feasible to implement and for reviers to review it | 12:15 |
| dviroel | it is already a major effort to get this working, and this will help a lot | 12:16 |
| dviroel | focus on the main implementation, and leave other additions to later | 12:16 |
| dviroel | makes sense? ^ | 12:16 |
| sean-k-mooney | i defintly see value in the other types but i think terating them as a strech goal and pulling them in after the intial verison can help hence the suggetion | 12:17 |
| amoralej | +1 sounds reasonable if you find it useful from implementation pov | 12:17 |
| sean-k-mooney | thre are 2 implication to it | 12:17 |
| sean-k-mooney | 1 we will need a new microverion to enabel the other tiems later | 12:17 |
| dviroel | yep | 12:17 |
| sean-k-mooney | 2 we will need a second db migration for the extra fields | 12:18 |
| dviroel | correct | 12:18 |
| sean-k-mooney | i think both are ok | 12:18 |
| sean-k-mooney | and we coudl do both this cycle if we hapen to get the rest of the work done | 12:18 |
| dviroel | agree | 12:18 |
| sean-k-mooney | but if not it make it cleaner to do it next chcle as we have the seam from day one | 12:18 |
| amoralej | that'd require a new spec or is just to reflect in the implementation plan? | 12:18 |
| sean-k-mooney | if it was next cycle a new spec | 12:19 |
| dviroel | a new api microversion should require a new spec then I think | 12:19 |
| sean-k-mooney | in the current spec we can leave the extra work as a stech goal | 12:19 |
| amoralej | ok | 12:19 |
| dviroel | ok, if you folks found any issue with the plan, please later comment in the spec | 12:19 |
| sean-k-mooney | i.e. docuemnt how it will work just not commit to deliving it in the inital veriosn | 12:20 |
| dviroel | ack | 12:20 |
| sean-k-mooney | dviroel: did you want ot suermise any of the other chagne or is that the main one you wanted ot highlight | 12:20 |
| dviroel | yes | 12:20 |
| sean-k-mooney | im hoping to get back to that today but have not got to the updated version yet | 12:20 |
| dviroel | i will be moving to the next change | 12:20 |
| dviroel | 2) pipeline stage has its own object, thinking on future parameters additions, it would be better to define a stage object right now | 12:20 |
| dviroel | i plan to cover the mains ones | 12:21 |
| dviroel | it was a suggestion that makes sense | 12:21 |
| dviroel | it is an extra effort added, but should help us in the future to extend audit pipeline features | 12:21 |
| dviroel | before that, the idea was to provide a list of audit_template uuid onlyu | 12:22 |
| dviroel | it should be better to track the audit template original configuration too | 12:22 |
| amoralej | so we'd i.e. track status on each individual stage | 12:22 |
| amoralej | right? | 12:22 |
| dviroel | since audit_template can be modified at any time | 12:23 |
| sean-k-mooney | amoralej: we coudl but not nessiarly | 12:23 |
| dviroel | we dont need to track the status | 12:23 |
| dviroel | at least not for now | 12:23 |
| sean-k-mooney | the idea is that in the api each satage has the name/desciption of the stage and the template uuid | 12:23 |
| amoralej | ack, so as part of the audit template definition | 12:24 |
| sean-k-mooney | in the db we woudl snapshot the paremter and other data that could mutate | 12:24 |
| sean-k-mooney | after the pipelien is defiend | 12:24 |
| dviroel | this ^ | 12:24 |
| sean-k-mooney | in the stage object so that if we are doing a continouts audit | 12:24 |
| sean-k-mooney | its behvior wont change | 12:24 |
| sean-k-mooney | that and it mean if we ever want to ahve a pre stage input_parmaters directly we can add it trivally | 12:24 |
| sean-k-mooney | or if we ant to be able to skip satges | 12:25 |
| sean-k-mooney | we have a place (the sage object) to hold that per stage info in teh future | 12:25 |
| dviroel | right | 12:25 |
| sean-k-mooney | instead of just a list of uuids | 12:25 |
| dviroel | ack | 12:25 |
| dviroel | it is better to manage and track, it also adds more implementation effort. But llooks like it is the right thing to do | 12:26 |
| dviroel | maybe in th future we could also track individual stage status | 12:26 |
| dviroel | bbut not planned for now | 12:26 |
| dviroel | any other concern with that ^ | 12:26 |
| sean-k-mooney | ya i think the intention is clear but we dont need to codify all posibel future usage, just note taht this change enabels that evoltion as needed | 12:27 |
| dviroel | +1 | 12:27 |
| amoralej | yes, looks like safe and make it more flexible for future | 12:27 |
| dviroel | so moving to next change.. | 12:28 |
| dviroel | 3) max_pipeline_stages config option was removed for now, we will work initially with a hardcoded value | 12:28 |
| dviroel | the original idea was to propose a config option for that | 12:28 |
| dviroel | sean-k-mooney mentioned that we should avoid configuration that affects api behavior | 12:28 |
| dviroel | i think that if we go with a hardcoded of 10 for now, we should be fine | 12:29 |
| amoralej | so, there will be a max stages value, but not configurable | 12:29 |
| dviroel | yes | 12:29 |
| amoralej | ok | 12:29 |
| dviroel | min=2, max=10 is set in the spec now | 12:29 |
| sean-k-mooney | yes, so long term i think we can use unifed limits | 12:30 |
| sean-k-mooney | to allow configureing this via an api | 12:30 |
| sean-k-mooney | but for now i would liek to hardcode | 12:30 |
| amoralej | wfm | 12:30 |
| dviroel | ack, I don't remember if I put that in future works but, it can be a possibility yeah | 12:30 |
| dviroel | talking about Future Works | 12:31 |
| dviroel | 4) Some missing sections/details were added: RPC, notification and Future Work | 12:31 |
| dviroel | Future work should cover the scope that was removed fro this spec | 12:32 |
| dviroel | like CONTINUOUS /EVENT types, new pipeline modes, etc | 12:32 |
| amoralej | so, current PS of the specs already has all the intended changes? | 12:33 |
| dviroel | Yes | 12:33 |
| dviroel | It should at least :) | 12:33 |
| amoralej | ok, i'll add it to may list of patches to be reviewed :) | 12:33 |
| dviroel | I was reviewing and add/removing things, and the spec is long already | 12:34 |
| dviroel | I may be missing something, but it should be all there | 12:34 |
| dviroel | the next change | 12:34 |
| dviroel | 5) In the RESP API section, moving out from PATCH and adopt PUT instead | 12:35 |
| dviroel | for new resource, like audit pipeline, it should not have PATCH | 12:35 |
| dviroel | audit_template already supports updates via PATCh | 12:35 |
| dviroel | the idea is to include PUT in audit_template and the new "default_parameter" would be only updated via PUT | 12:36 |
| dviroel | with all other allowed parameters from PATCH | 12:36 |
| sean-k-mooney | and improatnly change form partial updates(PAcTH in jsonpath form) to full update (PUT in the same json shape as post) | 12:37 |
| amoralej | why not use this to move all the parameters to PUT? | 12:37 |
| sean-k-mooney | concpetually you woudl do an update by doign a GET, mutating the json adn doing a PUT to update it | 12:38 |
| dviroel | amoralej: what do you mean by ALL? | 12:38 |
| sean-k-mooney | amoralej: we could | 12:38 |
| sean-k-mooney | dviroel: i belive they mean replace all useage of PATCH | 12:38 |
| amoralej | my point is, i see weird having patch and put on the same object for different parameters | 12:38 |
| amoralej | yes | 12:38 |
| sean-k-mooney | i also sugested we coudl do that as a seperate microversionb before or after this feature | 12:38 |
| sean-k-mooney | amoralej: on so the current propsal is we wont | 12:39 |
| dviroel | yeah, I think that we should do that, but that's another effort | 12:39 |
| sean-k-mooney | amoralej: in the spec the new microversion that add PUT remvoes PATCH | 12:39 |
| dviroel | the current proposal does not cover other resources yeah | 12:39 |
| sean-k-mooney | so if you want PATCH you use the old microverison because we alwasy have to supprot that anyway | 12:39 |
| amoralej | so, then, I'd keep the PATCH for all the parameters in the object, and then moving all to put | 12:40 |
| dviroel | for audit_template? | 12:40 |
| amoralej | but tbh, if others don't see much problem in mixing the calls, i will not block | 12:40 |
| amoralej | yes, for audit_template | 12:40 |
| sean-k-mooney | im mostly ok with that. i really dont like the use of patch but that is a reasonabel way to do it | 12:40 |
| dviroel | we can continue with PATCH only for audit_template | 12:41 |
| dviroel | and just set PUT for audit pipeline for now | 12:41 |
| dviroel | I am also fine with that too | 12:41 |
| amoralej | yep, that's my suggestion | 12:41 |
| sean-k-mooney | jgilaber: any perfernce ^ sicne you have been quite on the topic | 12:41 |
| dviroel | less changes out of the scope of Audit Pipeline feature | 12:42 |
| jgilaber | sorry it's been a while since I looked at the spec and I miss some context | 12:42 |
| dviroel | amoralej: tbh i had that proposal ready to sent yesterday, but I sent the one with PUT for audit_template too :) | 12:42 |
| jgilaber | but on this particular issue I think I would keep the changes as small as possible | 12:43 |
| amoralej | sorry for being picky :) | 12:43 |
| sean-k-mooney | it will take me a while to get to it but if you like i can draft a spec for the future adoption fo PT | 12:43 |
| sean-k-mooney | *of PUT | 12:43 |
| sean-k-mooney | and we can dicuss that seperatly then | 12:43 |
| dviroel | agree | 12:43 |
| dviroel | but are we OK with using PUT already for Audit Pipeline then? | 12:43 |
| sean-k-mooney | sure. | 12:44 |
| amoralej | yes | 12:44 |
| jgilaber | +1 | 12:44 |
| dviroel | nice | 12:44 |
| dviroel | i will update the spec to remove PUT change from audit_template | 12:45 |
| dviroel | and keep PATCH | 12:45 |
| dviroel | apart from those changes | 12:45 |
| dviroel | there were other small changes | 12:45 |
| * sean-k-mooney may have left 45 comments on the prior revision... | 12:46 | |
| dviroel | data model section was updated to include the readl db type | 12:46 |
| dviroel | now it s more clear the change required in BaseStrategy (a setters addition only) | 12:47 |
| dviroel | and how the Metric Cache will be introduced in the Datasources | 12:47 |
| dviroel | these details were missing and was confusing | 12:47 |
| dviroel | OK, i think that this cover most of the changes in the spec | 12:47 |
| dviroel | we already have almost 1k lines in this spec | 12:48 |
| dviroel | i think that's my personal record in spec I think | 12:48 |
| jgilaber | not an easy one dviroel | 12:48 |
| amoralej | yeah, thanks for all the effort you put on this spec | 12:48 |
| dviroel | thank you all for taking the time to review it | 12:49 |
| dviroel | if there is no more questions, we can move to the next topics | 12:49 |
| amoralej | i added one | 12:49 |
| amoralej | #topic Question about https://review.opendev.org/c/openstack/watcher/+/988284 | 12:49 |
| amoralej | in that review dviroel and myself discussed about adding the host field to the Instance element in the model | 12:50 |
| amoralej | while we can get the host for a instance getting the neighbor in the model graph | 12:50 |
| dviroel | oh sorry, i forgot to get back to this one | 12:51 |
| amoralej | i thought in adding it as a field for convenience | 12:51 |
| amoralej | as i find it cheap to maintain and query | 12:51 |
| sean-k-mooney | so there are 2 related fields | 12:51 |
| amoralej | but it's strictly not required | 12:51 |
| sean-k-mooney | host and hypervior host | 12:51 |
| sean-k-mooney | if we are adding oen we shoudl proahly have both | 12:52 |
| sean-k-mooney | as they are not nessiarly the same | 12:52 |
| dviroel | my concern was to have the same information in 2 ways, any chance of they not match? | 12:52 |
| dviroel | host ^ | 12:52 |
| amoralej | it needs to map to the ComputeNode | 12:52 |
| sean-k-mooney | host is useed to corralate a compute node to a compute service and ti the value used to compunicate with other service | 12:53 |
| amoralej | there is only a ComputeNode in the model, | 12:53 |
| sean-k-mooney | hypveriosr_hsotname is used ot corallate the instance to the hviorpsoer api and as teh resocue provider name for a compute ndoe in placement | 12:53 |
| amoralej | imo, if we need to have info about hypervisor and host, that should be in the ComputeNode, not in the instance | 12:53 |
| amoralej | host is to correlate Instance to ComputeNode | 12:53 |
| amoralej | so should be a single value imo | 12:54 |
| sean-k-mooney | amoralej: striclty speakign its not | 12:54 |
| amoralej | otherwise, it means we are missing something in the model | 12:54 |
| amoralej | an Hypervisor | 12:54 |
| sean-k-mooney | strictly speakign it to corralte the instnace to the comptue service and there is a third node value for instance to compute node | 12:54 |
| sean-k-mooney | amoralej: the disticion really only maters for ironic, vmware and non libvirt virt drivers | 12:55 |
| sean-k-mooney | for libvir tthe 2 vlaues are almost alwasy the same | 12:55 |
| sean-k-mooney | amoralej: oh we are missing somting in the model | 12:55 |
| sean-k-mooney | watcher is assumign the two are interchagneable whwen strcily speaking they are not | 12:56 |
| * dviroel always get confused with both | 12:56 | |
| amoralej | that was my point, if we really need to keep both separated, then we need something else other than having both in the Instance, imo | 12:56 |
| sean-k-mooney | a singal compute service manage many ironic nodes | 12:56 |
| sean-k-mooney | so in the case of ironic we will have many compute nodes(and ironic instnaces) with the saem host value but differnt node/hypervior_hostname values | 12:57 |
| sean-k-mooney | amoralej: the instnace has both in novas api | 12:57 |
| sean-k-mooney | amoralej: lets pick this up in the change | 12:58 |
| jgilaber | looking quickly at the patch, I think storing the host on the Instance object would make it easier to consume it | 12:58 |
| amoralej | so covering the ironic case will require more elements | 12:58 |
| jgilaber | since it's used for example in the ComputeHostSortFilter which does not have access to the model | 12:58 |
| jgilaber | only to Instance objects | 12:58 |
| amoralej | other option would be to add a method to the model parent_host(instance) | 12:59 |
| amoralej | or similar | 12:59 |
| sean-k-mooney | in the nova api instnace have both OS-EXT-SRV-ATTR:host and OS-EXT-SRV-ATTR:hypervisor_hostname | 12:59 |
| sean-k-mooney | so i think we shoudl have both on our instance object too (without the prefix) | 13:00 |
| amoralej | so, i'm fine with adding both | 13:00 |
| sean-k-mooney | https://docs.openstack.org/api-ref/compute/#id30 | 13:00 |
| jgilaber | I'm looking at the nova helper and looks like we're missing the hypervisor_hostname there as well | 13:01 |
| amoralej | dviroel, ^ wdth? we were more on the other option and avoiding adding host value at all | 13:01 |
| dviroel | we can, we might need to also make sure that the model is aligned with that | 13:02 |
| amoralej | that's a good point jgilaber thanks for checking | 13:02 |
| dviroel | the instance is mapped to the right ComputeNode too | 13:02 |
| amoralej | in my patch i added the logic to update host on each map/unmap action | 13:03 |
| amoralej | to maintain them consistent | 13:03 |
| amoralej | that was my plan | 13:03 |
| amoralej | we are out of time, sorry | 13:03 |
| amoralej | let's keep discussing on the review | 13:04 |
| dviroel | ack | 13:04 |
| sean-k-mooney | the thing is a comptue node does not have a hsot value | 13:04 |
| sean-k-mooney | that why im makign the disctintion | 13:04 |
| sean-k-mooney | a comptue node as exposed in placement and the hyvperiors api only has a hypervior_hostname | 13:04 |
| amoralej | yep, i appreciate the feedback | 13:04 |
| sean-k-mooney | a compute service has host vlaue | 13:04 |
| dviroel | I think that we should fix this once for all :) | 13:05 |
| sean-k-mooney | an instance is mapped to both the contoling service(the nova-compute agent) and the compute node (the hyperiovr) | 13:05 |
| dviroel | so it is good to discuss a bit more yes | 13:05 |
| amoralej | I will double check and compare with the API and what we are getting from the client | 13:05 |
| dviroel | lets circle back again | 13:06 |
| amoralej | any volunteer to chair the next meeting ? | 13:06 |
| * dviroel will be out | 13:06 | |
| jgilaber | I can do it | 13:07 |
| dviroel | jgilaber++ | 13:07 |
| amoralej | thanks jgilaber | 13:07 |
| dviroel | one last thing | 13:07 |
| amoralej | sure | 13:07 |
| dviroel | I added to reviers request | 13:07 |
| dviroel | i jsut paste them here | 13:07 |
| dviroel | 989396: Fix notification updates lost during cluster data model synchronization | https://review.opendev.org/c/openstack/watcher/+/989396 | 13:08 |
| dviroel | 987540: Replace global lockutils locks with per-instance RLock | https://review.opendev.org/c/openstack/watcher/+/987540 | 13:08 |
| * dviroel sorry lost the etherpad tab | 13:08 | |
| dviroel | please review these when have some time | 13:08 |
| dviroel | thanks! | 13:08 |
| amoralej | thanks all for joining | 13:08 |
| dviroel | we can discuss later if needed | 13:08 |
| dviroel | thanks amoralej++ | 13:08 |
| amoralej | #endmeeting | 13:09 |
| opendevmeet | Meeting ended Thu May 28 13:09:07 2026 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 13:09 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/watcher_28_may_2026/2026/watcher_28_may_2026.2026-05-28-12.00.html | 13:09 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/watcher_28_may_2026/2026/watcher_28_may_2026.2026-05-28-12.00.txt | 13:09 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/watcher_28_may_2026/2026/watcher_28_may_2026.2026-05-28-12.00.log.html | 13:09 |
| winiciusallan[m] | sorry folks I couldn't attend the meeting but this is a kindly ping if someone could take a look at the preemptible instances spec :) | 13:12 |
| winiciusallan[m] | thx in advance https://review.opendev.org/c/openstack/watcher-specs/+/987171 | 13:12 |
| opendevreview | Joan Gilabert proposed openstack/watcher master: Extract BaseHelper for SDK connection setup https://review.opendev.org/c/openstack/watcher/+/988964 | 14:14 |
| opendevreview | Takashi Kajinami proposed openstack/watcher master: doc: Drop reference to "keystone" CLI https://review.opendev.org/c/openstack/watcher/+/990520 | 15:30 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!