| opendevreview | chandan kumar proposed openstack/watcher master: Drops forbidden patch/delete/post action apis https://review.opendev.org/c/openstack/watcher/+/954021 | 07:56 |
|---|---|---|
| opendevreview | chandan kumar proposed openstack/watcher master: Drops forbidden patch/delete/post action apis https://review.opendev.org/c/openstack/watcher/+/954021 | 10:33 |
| dviroel | #startmeeting watcher | 12:00 |
| 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 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 12:00 |
| opendevmeet | The meeting name has been set to 'watcher' | 12:00 |
| dviroel | hi o/ | 12:00 |
| dviroel | who's around today? | 12:01 |
| jgilaber | o/ | 12:01 |
| morenod | o/ | 12:01 |
| dviroel | courtesy ping: amoralej sean-k-mooney chandankumar | 12:01 |
| amoralej | o/ | 12:02 |
| sean-k-mooney | o/ | 12:03 |
| dviroel | alright, let's start with today's meeting agenda | 12:03 |
| dviroel | #link https://etherpad.opendev.org/p/openstack-watcher-irc-meeting#L44 (Meeting agenda) | 12:03 |
| dviroel | feel free to add your own topics to the agenda | 12:03 |
| dviroel | there are topics to place your changes that requires attention from reviewers | 12:03 |
| dviroel | we also have a topic for bugs, if you want to discuss any | 12:03 |
| dviroel | #topic Announcements | 12:03 |
| dviroel | we reached flamingo's second milestone today \o/ | 12:04 |
| dviroel | #link https://releases.openstack.org/flamingo/schedule.html (2025.2 Flamingo Release Schedule) | 12:04 |
| dviroel | and with that | 12:04 |
| dviroel | all features that requires a spec and are going to be implemented in Flamingo (2025.2) | 12:05 |
| dviroel | should have a spec merged today | 12:05 |
| dviroel | from our open specs in gerrit | 12:05 |
| dviroel | #link https://review.opendev.org/q/project:openstack/watcher-specs+is:+open | 12:05 |
| dviroel | now there is only still open | 12:05 |
| dviroel | #link https://review.opendev.org/c/openstack/watcher-specs/+/947282 (Adds spec for extend compute model attributes) | 12:05 |
| dviroel | which has some approvals already | 12:06 |
| dviroel | so, if nobody disagrees with it | 12:06 |
| rlandy | o/ (apologies late to the meeting) | 12:06 |
| chandankumar | o/ | 12:06 |
| dviroel | I/we can workflow it at the end of the day today | 12:06 |
| dviroel | you can also see the already approved specs for this release here | 12:07 |
| dviroel | #link https://specs.openstack.org/openstack/watcher-specs/specs/2025.2/index.html | 12:07 |
| dviroel | any other comment on this topic? | 12:08 |
| dviroel | this was the only announcement from my side :) - anyone has other announcement to make? | 12:09 |
| dviroel | ok :) next topic | 12:10 |
| dviroel | #topic Eventlet Removal Updates | 12:11 |
| dviroel | not too much bring this week i guess | 12:11 |
| dviroel | last week we already discussed about the issue with decision-engine and its 2 services started from the same service manager | 12:11 |
| dviroel | #link https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-06-26-12.00.log.html#l-46 | 12:12 |
| dviroel | logs from last meeting ^ | 12:12 |
| dviroel | one possible fix to this issue, which was recently updated is | 12:12 |
| dviroel | #link https://review.opendev.org/c/openstack/watcher/+/952499 | 12:12 |
| 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 |
| dviroel | #link https://review.opendev.org/c/openstack/watcher/+/952257 | 12:13 |
| 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:14 |
| dviroel | this one is the one that I am working right now, refactoring it and going to update soon | 12:15 |
| dviroel | this is all that I have | 12:15 |
| dviroel | any question on this topic? | 12:16 |
| dviroel | ok, next topic is | 12:17 |
| dviroel | #topic Handling storage model automatically | 12:17 |
| jgilaber | I added that one | 12:18 |
| jgilaber | I've been looking into how to detect if cinder is enabled to enable the storage model | 12:18 |
| jgilaber | but I'm not sure what is the best way to handle the existing config option | 12:18 |
| jgilaber | #link I've been looking into how to detect if cinder is enabled to enable the storage model | 12:18 |
| jgilaber | I'm not sure if we keep using it, how to reconcile user input and auto detection | 12:19 |
| jgilaber | so I though | 12:19 |
| jgilaber | Option 1: Set its default value to None and use it if the user sets it, otherwise detect automatically | 12:19 |
| jgilaber | Option 2: Deprecated and always detect automatically | 12:19 |
| jgilaber | I think option 1 is better because the docs mention that watcher supports custom collector plugins | 12:20 |
| dviroel | jgilaber: can you share the link again? | 12:20 |
| 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 |
| jgilaber | sorry about the link | 12:20 |
| jgilaber | #link https://docs.openstack.org/watcher/latest/configuration/watcher.html#collector.collector_plugins | 12:20 |
| dviroel | yeah, I agree that we should keep the option that allows users to disable individual collectors | 12:22 |
| dviroel | which in this case is the opposite, they select which one they want to enable | 12:22 |
| amoralej | so, autodetection would be only applied if the plugin is enabled | 12:22 |
| jgilaber | amoralej: IIUC that is the current behavior, only use the collector if the plugin is enabled in the config | 12:24 |
| amoralej | so if 'storage' is in collection_plugins list -> if no cinder available -> left warning message and skip collection | 12:24 |
| amoralej | yes, actually, current behavior is pretty good, just there is some corner case to cover, iiuc | 12:24 |
| jgilaber | ah ok I see what you mean now | 12:24 |
| 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:25 |
| amoralej | imo the only thing that is needed is a last case check inside the collector itself | 12:26 |
| 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 |
| jgilaber | we could do both, enable it by default, but check if cinder is present as amoralej suggested | 12:27 |
| 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:29 |
| 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:30 |
| jgilaber | +1 I'll cover that in a follow-up | 12:31 |
| 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 |
| amoralej | https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/collector/cinder.py#L150-L155 | 12:31 |
| dviroel | right, it would avoid collecting info while it is not needed | 12:33 |
| 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:34 |
| amoralej | +1 | 12:35 |
| dviroel | after deleting this single audit, it will continue to collecting? don't know | 12:35 |
| amoralej | it does, once a collection has happened and storage_model is not empty, it will continue collecting according to the period parameter | 12:36 |
| amoralej | if storage is in collector_plugins | 12:37 |
| dviroel | something that we should also take a look | 12:38 |
| dviroel | jgilaber: anything else to cover in this topic? | 12:39 |
| jgilaber | nope, I think that is ok for now, thanks! | 12:40 |
| dviroel | jgilaber: ty! | 12:40 |
| dviroel | #topic Call for reviews | 12:40 |
| dviroel | there is one in the list | 12:40 |
| dviroel | #link https://review.opendev.org/c/openstack/watcher/+/954021 | 12:40 |
| dviroel | from chandankumar | 12:41 |
| chandankumar | dviroel: thanks! It removes unused forbidden apis from action.py | 12:42 |
| dviroel | there is more info the related bug: | 12:43 |
| dviroel | #link https://launchpad.net/bugs/2110895 | 12:43 |
| chandankumar | I have started working on tackling bugs related to tech debts | 12:43 |
| chandankumar | it is one of them. | 12:43 |
| chandankumar | Please have a look when free, thank you! | 12:43 |
| amoralej | i wonder why those methods were created to just raise operationnotpermited ... | 12:43 |
| chandankumar | I got this old commit https://github.com/openstack/watcher/commit/22c9c4df877b5fabcdf74bb80e61d24d06700dcc | 12:44 |
| chandankumar | pointing to https://bugs.launchpad.net/watcher/+bug/1533281 | 12:45 |
| amoralej | ah, that explains i guet | 12:45 |
| amoralej | guess | 12:45 |
| chandankumar | The reason behind removing these actions is because an action plan is atomic. | 12:45 |
| chandankumar | The only way for us to be able to delete actions should be when we delete the | 12:45 |
| chandankumar | action plan these actions relate to. | 12:45 |
| chandankumar | it is from long text pasted in the bug | 12:46 |
| 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 |
| sean-k-mooney | ill try and read back again later but if my input is needed please ping directly | 12:46 |
| 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 |
| sean-k-mooney | i have not looke at the impe for the api fix | 12:47 |
| chandankumar | the existing code mention about FIXME: blueprint edit-action-plan-flow but I was not able to find out the blueprint | 12:48 |
| sean-k-mooney | but skimming it quicklyu it looks light a straige delete | 12:48 |
| sean-k-mooney | the exiting code was related to that yes | 12:48 |
| amoralej | yep | 12:48 |
| sean-k-mooney | they started the imple but never finsihed it | 12:48 |
| sean-k-mooney | the orgiall paths came form the template they used to crate teh files | 12:48 |
| chandankumar | I deleted the code as I was not finding its usage any where | 12:49 |
| sean-k-mooney | ya that the correct approch but i think you have only looked at actions | 12:49 |
| sean-k-mooney | if i recall there are other places where we need to clean up | 12:49 |
| sean-k-mooney | i would like one patch to do all the removal personally | 12:49 |
| sean-k-mooney | i could be miss rememberign but i though it affect other enpoitnss too | 12:50 |
| * dviroel was reading the LP | 12:51 | |
| sean-k-mooney | oh not all uses of OperationNotPermitted are for this pattern | 12:52 |
| chandankumar | I did an audit of all the rest api and captured it somewhere in etherpad | 12:52 |
| chandankumar | let me find that one | 12:52 |
| 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:52 |
| chandankumar | https://etherpad.opendev.org/p/watcher-apiref-audit | 12:53 |
| dviroel | i also need some time to dig more, and place my comments in the review | 12:53 |
| dviroel | ack, thanks for the link chandankumar | 12:53 |
| chandankumar | sure | 12:54 |
| chandankumar | thank you sean-k-mooney dviroel amoralej ! | 12:54 |
| sean-k-mooney | so they have this parttern as well | 12:54 |
| sean-k-mooney | if self.from_audits: | 12:54 |
| sean-k-mooney | raise exception.OperationNotPermitted | 12:54 |
| sean-k-mooney | so they put protection to prevent self calls to the api | 12:55 |
| sean-k-mooney | btu since watcher shoudl never call its own api | 12:55 |
| sean-k-mooney | im not sure why those are there | 12:55 |
| sean-k-mooney | thats kind of sepreate however so we can maybe look into that seperately ad possibel other tech debt | 12:55 |
| sean-k-mooney | https://github.com/openstack/watcher/blob/93366df2641b64b7bc345ed91bfbef7ae17de25a/watcher/api/controllers/v1/audit.py#L676-L677 | 12:56 |
| chandankumar | raise exception.OperationNotPermitted is called in almost all the apis | 12:56 |
| sean-k-mooney | right but openstack service are not ment to ever all there own api | 12:57 |
| sean-k-mooney | so im not sure if those check are dead code | 12:57 |
| sean-k-mooney | or if there is other technial debth that we need to look into | 12:57 |
| chandankumar | I will dig into that exceptions and update the same patch based on that | 12:57 |
| 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:57 |
| sean-k-mooney | ack for now lets keep ti small just update it if you see other endpoitn that uncontionaly raise | 12:58 |
| dviroel | +1 | 12:58 |
| chandankumar | in that case, I will open a seperate bug for this exception to see what it breaks | 12:58 |
| chandankumar | and why it is there in the first place | 12:58 |
| sean-k-mooney | perfect | 12:59 |
| sean-k-mooney | i just skimmed the other entry point adn i dont see any other routes that uncondtionally raise | 12:59 |
| * dviroel time check | 12:59 | |
| 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 |
| dviroel | +1 | 12:59 |
| chandankumar | sounds good! | 12:59 |
| dviroel | thanks chandankumar | 13:00 |
| chandankumar | thanks sean-k-mooney dviroel amoralej ! | 13:00 |
| amoralej | thanks chandankumar for taking care of that one | 13:00 |
| dviroel | going to skip bug triage for now, there is only one bug there that we can cover next week | 13:00 |
| dviroel | #topic Volunteers to chair next meeting | 13:00 |
| dviroel | i am available next week again | 13:00 |
| dviroel | but if someone wants to chair, pls let me know | 13:01 |
| dviroel | or put yout name in the etherpad | 13:01 |
| amoralej | i can take it | 13:01 |
| dviroel | let's wrap up for today | 13:01 |
| dviroel | thanks amoralej! | 13:01 |
| dviroel | all yours :) | 13:01 |
| chandankumar | thanks amoralej dviroel ! | 13:01 |
| dviroel | ok, we will meet again next week | 13:01 |
| dviroel | thank you all for participating | 13:01 |
| jgilaber | thanks! | 13:01 |
| dviroel | #endmeeting | 13:02 |
| opendevmeet | Meeting ended Thu Jul 3 13:02:04 2025 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 13:02 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-07-03-12.00.html | 13:02 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-07-03-12.00.txt | 13:02 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-07-03-12.00.log.html | 13:02 |
| opendevreview | chandan kumar proposed openstack/watcher master: Drops forbidden patch/delete/post action apis https://review.opendev.org/c/openstack/watcher/+/954021 | 13:34 |
| opendevreview | chandan kumar proposed openstack/watcher master: Drops forbidden patch/delete/post action apis https://review.opendev.org/c/openstack/watcher/+/954021 | 15:05 |
| opendevreview | Ho Minh Quang Ngo proposed openstack/watcher master: Add options to disable migration in host maintenance https://review.opendev.org/c/openstack/watcher/+/952538 | 15:17 |
| opendevreview | Merged openstack/watcher master: add missing bindeps for docs https://review.opendev.org/c/openstack/watcher/+/950329 | 16:03 |
| opendevreview | Merged openstack/watcher master: Add warning message for experimental integrations https://review.opendev.org/c/openstack/watcher/+/952017 | 17:27 |
| opendevreview | Merged openstack/watcher master: Drop unused fake class https://review.opendev.org/c/openstack/watcher/+/953701 | 17:28 |
| opendevreview | Merged openstack/watcher master: resolve fixme comments in RequestContext https://review.opendev.org/c/openstack/watcher/+/950357 | 17:28 |
| opendevreview | Merged openstack/watcher-specs master: Adds spec for extend compute model attributes https://review.opendev.org/c/openstack/watcher-specs/+/947282 | 18:59 |
| opendevreview | Merged openstack/watcher master: Update workload balance doc per review comments https://review.opendev.org/c/openstack/watcher/+/953679 | 19:57 |
| opendevreview | Douglas Viroel proposed openstack/watcher master: Merge decision engine services into a single one https://review.opendev.org/c/openstack/watcher/+/952499 | 20:29 |
| opendevreview | Douglas Viroel proposed openstack/watcher master: WIP - Extend decision engine to support threading mode https://review.opendev.org/c/openstack/watcher/+/952257 | 20:29 |
| opendevreview | Ronelle Landy proposed openstack/watcher master: WIP: Update Overload standard deviation doc https://review.opendev.org/c/openstack/watcher/+/954067 | 21:04 |
| *** haleyb is now known as haleyb|out | 21:52 | |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!