12:00:31 <dviroel> #startmeeting watcher
12:00:31 <opendevmeet> Meeting started Thu Jul  3 12:00:31 2025 UTC and is due to finish in 60 minutes.  The chair is dviroel. Information about MeetBot at http://wiki.debian.org/MeetBot.
12:00:31 <opendevmeet> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
12:00:31 <opendevmeet> The meeting name has been set to 'watcher'
12:00:52 <dviroel> hi o/
12:01:10 <dviroel> who's around today?
12:01:18 <jgilaber> o/
12:01:25 <morenod> o/
12:01:53 <dviroel> courtesy ping: amoralej sean-k-mooney chandankumar
12:02:01 <amoralej> o/
12:03:06 <sean-k-mooney> o/
12:03:06 <dviroel> alright, let's start with today's meeting agenda
12:03:16 <dviroel> #link https://etherpad.opendev.org/p/openstack-watcher-irc-meeting#L44 (Meeting agenda)
12:03:25 <dviroel> feel free to add your own topics to the agenda
12:03:35 <dviroel> there are topics to place your changes that requires attention from reviewers
12:03:44 <dviroel> we also have a topic for bugs, if you want to discuss any
12:03:58 <dviroel> #topic Announcements
12:04:20 <dviroel> we reached flamingo's second milestone today \o/
12:04:27 <dviroel> #link https://releases.openstack.org/flamingo/schedule.html (2025.2 Flamingo Release Schedule)
12:04:39 <dviroel> and with that
12:05:05 <dviroel> all features that requires a spec and are going to be implemented in Flamingo (2025.2)
12:05:12 <dviroel> should have a spec merged today
12:05:23 <dviroel> from our open specs in gerrit
12:05:30 <dviroel> #link https://review.opendev.org/q/project:openstack/watcher-specs+is:+open
12:05:43 <dviroel> now there is only still open
12:05:54 <dviroel> #link https://review.opendev.org/c/openstack/watcher-specs/+/947282 (Adds spec for extend compute model attributes)
12:06:08 <dviroel> which has some approvals already
12:06:25 <dviroel> so, if nobody disagrees with it
12:06:33 <rlandy> o/ (apologies late to the meeting)
12:06:38 <chandankumar> o/
12:06:49 <dviroel> I/we can workflow it at the end of the day today
12:07:57 <dviroel> you can also see the already approved specs for this release here
12:07:59 <dviroel> #link https://specs.openstack.org/openstack/watcher-specs/specs/2025.2/index.html
12:08:29 <dviroel> any other comment on this topic?
12:09:20 <dviroel> this was the only announcement from my side :) - anyone has other announcement to make?
12:10:38 <dviroel> ok :) next topic
12:11:09 <dviroel> #topic Eventlet Removal Updates
12:11:22 <dviroel> not too much bring this week i guess
12:11:48 <dviroel> last week we already discussed about the issue with decision-engine and its 2 services started from the same service manager
12:12:18 <dviroel> #link https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-06-26-12.00.log.html#l-46
12:12:26 <dviroel> logs from last meeting ^
12:12:38 <dviroel> one possible fix to this issue, which was recently updated is
12:12:50 <dviroel> #link https://review.opendev.org/c/openstack/watcher/+/952499
12:13:40 <dviroel> which is open for review, but I don't plan to merge it before having the other eventlet patch ready (and working)
12:13:48 <dviroel> #link https://review.opendev.org/c/openstack/watcher/+/952257
12:14:37 <dviroel> the latter one is really testing the decision engine both with monkey patching enabled and disabled (there is a new job that disabled monkey patching in the dec-eng)
12:15:07 <dviroel> this one is the one that I am working right now, refactoring it and going to update soon
12:15:48 <dviroel> this is all that I have
12:16:11 <dviroel> any question on this topic?
12:17:24 <dviroel> ok, next topic is
12:17:49 <dviroel> #topic Handling storage model automatically
12:18:06 <jgilaber> I added that one
12:18:15 <jgilaber> I've been looking into how to detect if cinder is enabled to enable the storage model
12:18:28 <jgilaber> but I'm not sure what is the best way to handle the existing config option
12:18:36 <jgilaber> #link I've been looking into how to detect if cinder is enabled to enable the storage model
12:19:05 <jgilaber> I'm not sure if we keep using it, how to reconcile user input and auto detection
12:19:17 <jgilaber> so I though
12:19:18 <jgilaber> Option 1: Set its default value to None and use it if the user sets it, otherwise detect automatically
12:19:25 <jgilaber> Option 2: Deprecated and always detect automatically
12:20:14 <jgilaber> I think option 1 is better because the docs mention that watcher supports custom collector plugins
12:20:22 <dviroel> jgilaber: can you share the link again?
12:20:25 <amoralej> i think having a way to totally disabling collecting storage model is fine as some users may want to totally avoid unneeded api access
12:20:37 <jgilaber> sorry about the link
12:20:50 <jgilaber> #link https://docs.openstack.org/watcher/latest/configuration/watcher.html#collector.collector_plugins
12:22:01 <dviroel> yeah, I agree that we should keep the option that allows users to disable individual collectors
12:22:14 <dviroel> which in this case is the opposite, they select which one they want to enable
12:22:46 <amoralej> so, autodetection would be only applied if the plugin is enabled
12:24:22 <jgilaber> amoralej: IIUC that is the current behavior, only use the collector if the plugin is enabled in the config
12:24:24 <amoralej> so if 'storage' is in collection_plugins list -> if no cinder available -> left warning message and skip collection
12:24:45 <amoralej> yes, actually, current behavior is pretty good, just there is some corner case to cover, iiuc
12:24:54 <jgilaber> ah ok I see what you mean now
12:25:41 <amoralej> i.e. if a user create a zone_migration with storage_backend, it tries to collect and if there is no cinder, a bunch of errors appear
12:26:11 <amoralej> imo the only thing that is needed is a last case check inside the collector itself
12:27:05 <dviroel> we could consider enable storage by default, if we could guarantee that there is no issues with that, i.e. watcher is able to handle this cases where there is no cinder/backends
12:27:43 <jgilaber> we could do both, enable it by default, but check if cinder is present as amoralej suggested
12:29:18 <amoralej> so, in the most common case, user enables storage, runs the decision-engine and there is no cinder, it works fine, it will not even try to collect storage until there is a strategy that explicitely requests the storage_model
12:30:12 <amoralej> it'd be a bit strange that a user create a zone_migration with storage_backend parameter if there is no cinder ... but still it'd be good to cover that case
12:31:42 <jgilaber> +1 I'll cover that in a follow-up
12:31:52 <amoralej> in other case, there is a protection in place that skips storage collection unless something actually requires it even if it's enabled
12:31:53 <amoralej> https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/collector/cinder.py#L150-L155
12:33:10 <dviroel> right, it would avoid collecting info while it is not needed
12:34:23 <dviroel> for me, it is more about covering the corner cases, where there is no backend, and user creates a audit that will trigger the collector
12:35:31 <amoralej> +1
12:35:51 <dviroel> after deleting this single audit, it will continue to collecting? don't know
12:36:28 <amoralej> it does, once a collection has happened and storage_model is not empty, it will continue collecting according to the period parameter
12:37:05 <amoralej> if storage is in collector_plugins
12:38:29 <dviroel> something that we should also take a look
12:39:48 <dviroel> jgilaber: anything else to cover in this topic?
12:40:04 <jgilaber> nope, I think that is ok for now, thanks!
12:40:22 <dviroel> jgilaber: ty!
12:40:35 <dviroel> #topic Call for reviews
12:40:42 <dviroel> there is one in the list
12:40:59 <dviroel> #link https://review.opendev.org/c/openstack/watcher/+/954021
12:41:12 <dviroel> from chandankumar
12:42:59 <chandankumar> dviroel: thanks! It removes unused forbidden apis from action.py
12:43:11 <dviroel> there is more info the related bug:
12:43:13 <dviroel> #link https://launchpad.net/bugs/2110895
12:43:23 <chandankumar> I have started working on tackling bugs related to tech debts
12:43:45 <chandankumar> it is one of them.
12:43:53 <chandankumar> Please have a look when free, thank you!
12:43:59 <amoralej> i wonder why those methods were created to just raise operationnotpermited ...
12:44:46 <chandankumar> I got this old commit https://github.com/openstack/watcher/commit/22c9c4df877b5fabcdf74bb80e61d24d06700dcc
12:45:04 <chandankumar> pointing to https://bugs.launchpad.net/watcher/+bug/1533281
12:45:36 <amoralej> ah, that explains i guet
12:45:42 <amoralej> guess
12:45:57 <chandankumar> The reason behind removing these actions is because an action plan is atomic.
12:45:57 <chandankumar> The only way for us to be able to delete actions should be when we delete the
12:45:57 <chandankumar> action plan these actions relate to.
12:46:08 <chandankumar> it is from long text pasted in the bug
12:46:16 <sean-k-mooney> sorry i have been kidn of distracted today so i have been perodicily check the meeting topics but im not following the converstaion fully
12:46:55 <sean-k-mooney> ill try and read back again later but if my input is needed please ping directly
12:47:09 <amoralej> yes, i understand that, but my question is why they implemented that way instead of just removing the methods, maybe for backwards compatibility in the exposed api methods or something
12:47:40 <sean-k-mooney> i have not looke at the impe for the api fix
12:48:10 <chandankumar> the existing code mention about FIXME: blueprint edit-action-plan-flow but I was not able to find out the blueprint
12:48:13 <sean-k-mooney> but skimming it quicklyu it looks light a straige delete
12:48:25 <sean-k-mooney> the exiting code was related to that yes
12:48:27 <amoralej> yep
12:48:34 <sean-k-mooney> they started the imple but never finsihed it
12:48:54 <sean-k-mooney> the orgiall paths came form the template they used to crate teh files
12:49:12 <chandankumar> I deleted the code as I was not finding its usage any where
12:49:31 <sean-k-mooney> ya that the correct approch but i think you have only looked at actions
12:49:44 <sean-k-mooney> if i recall there are other places where we need to clean up
12:49:58 <sean-k-mooney> i would like one patch to do all the removal personally
12:50:52 <sean-k-mooney> i could be miss rememberign but i though it affect other enpoitnss too
12:51:24 * dviroel was reading the LP
12:52:01 <sean-k-mooney> oh not all uses of OperationNotPermitted are for this pattern
12:52:21 <chandankumar> I did an audit of all the rest api and captured it somewhere in etherpad
12:52:24 <chandankumar> let me find that one
12:52:49 <sean-k-mooney> we can double check this offline in the gerrit review but if those are the only routes that shoudl not exist we shoud eb good
12:53:14 <chandankumar> https://etherpad.opendev.org/p/watcher-apiref-audit
12:53:22 <dviroel> i also need some time to dig more, and place my comments in the review
12:53:47 <dviroel> ack, thanks for the link chandankumar
12:54:28 <chandankumar> sure
12:54:37 <chandankumar> thank you sean-k-mooney dviroel amoralej !
12:54:37 <sean-k-mooney> so they have this parttern as well
12:54:45 <sean-k-mooney> if self.from_audits:
12:54:47 <sean-k-mooney> raise exception.OperationNotPermitted
12:55:03 <sean-k-mooney> so they put protection to prevent self calls to the api
12:55:14 <sean-k-mooney> btu since watcher shoudl never call its own api
12:55:24 <sean-k-mooney> im not sure why those are there
12:55:50 <sean-k-mooney> thats kind of sepreate however so we can maybe look into that seperately ad possibel other tech debt
12:56:00 <sean-k-mooney> https://github.com/openstack/watcher/blob/93366df2641b64b7bc345ed91bfbef7ae17de25a/watcher/api/controllers/v1/audit.py#L676-L677
12:56:42 <chandankumar> raise exception.OperationNotPermitted is called in almost all the apis
12:57:02 <sean-k-mooney> right but openstack service are not ment to ever all there own api
12:57:14 <sean-k-mooney> so im not sure if those check are dead code
12:57:26 <sean-k-mooney> or if there is other technial debth that we need to look into
12:57:42 <chandankumar> I will dig into that exceptions and update the same patch based on that
12:57:46 <sean-k-mooney> i.e. if those every block anything it indeciates that watcher is calling it own api endpoitn which si not good
12:58:11 <sean-k-mooney> ack for now lets keep ti small just update it if you see other endpoitn that uncontionaly raise
12:58:33 <dviroel> +1
12:58:34 <chandankumar> in that case, I will open a seperate bug for this exception to see what it breaks
12:58:52 <chandankumar> and why it is there in the first place
12:59:06 <sean-k-mooney> perfect
12:59:24 <sean-k-mooney> i just skimmed the other entry point adn i dont see any other routes that uncondtionally raise
12:59:40 * dviroel time check
12:59:43 <sean-k-mooney> so i think the scope fo the inial patch is good and we can look into the ohter issuse seperately
12:59:53 <dviroel> +1
12:59:53 <chandankumar> sounds good!
13:00:10 <dviroel> thanks chandankumar
13:00:17 <chandankumar> thanks sean-k-mooney dviroel amoralej !
13:00:28 <amoralej> thanks chandankumar for taking care of that one
13:00:41 <dviroel> going to skip bug triage for now,  there is only one bug there that we can cover next week
13:00:49 <dviroel> #topic Volunteers to chair next meeting
13:00:59 <dviroel> i am available next week again
13:01:14 <dviroel> but if someone wants to chair, pls let me know
13:01:25 <dviroel> or put yout name in the etherpad
13:01:33 <amoralej> i can take it
13:01:33 <dviroel> let's wrap up for today
13:01:39 <dviroel> thanks amoralej!
13:01:45 <dviroel> all yours :)
13:01:49 <chandankumar> thanks amoralej dviroel !
13:01:51 <dviroel> ok, we will meet again next week
13:01:57 <dviroel> thank you all for participating
13:01:57 <jgilaber> thanks!
13:02:04 <dviroel> #endmeeting