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