| opendevreview | JaromÃr Wysoglad proposed openstack/watcher master: [DNM] Add Aetos datasource https://review.opendev.org/c/openstack/watcher/+/955608 | 07:16 |
|---|---|---|
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Add status_message fields to audit, action and actionplan https://review.opendev.org/c/openstack/watcher/+/954745 | 08:26 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Skip actions automatically based on pre_condition results https://review.opendev.org/c/openstack/watcher/+/954746 | 08:26 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Implement action skipping functionality via PATCH API https://review.opendev.org/c/openstack/watcher/+/955753 | 08:26 |
| opendevreview | David proposed openstack/watcher-tempest-plugin master: Add custom flavor and dynamic threshold to workload_balance tests https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/953853 | 09:15 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher-tempest-plugin master: Add api test for skip action https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/955775 | 11:31 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher-tempest-plugin master: Add api test for skip action https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/955775 | 11:37 |
| dviroel | hi all, watcher meeting will start in 10m, please add your topics at meeting's etherpad https://etherpad.opendev.org/p/openstack-watcher-irc-meeting#L48 | 11:50 |
| dviroel | #startmeeting watcher | 12:00 |
| opendevmeet | Meeting started Thu Jul 24 12:00:12 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 all, who's around today? | 12:00 |
| amoralej_ | o/ | 12:00 |
| dviroel | courtesy ping: sean-k-mooney chandankumar morenod rlandy | 12:01 |
| dviroel | let's start with today's meeting agenda | 12:02 |
| rlandy | o/ | 12:02 |
| dviroel | #link https://etherpad.opendev.org/p/openstack-watcher-irc-meeting#L26 (Meeting agenda) | 12:02 |
| dviroel | feel free to add your own topics to the agenda | 12:03 |
| dviroel | first one | 12:03 |
| dviroel | #topic Eventlet Removal | 12:03 |
| chandankumar | o/ | 12:03 |
| dviroel | as usual, this topic is to cover the progress on eventlet removal patches | 12:04 |
| dviroel | this week, there isn't too much updates | 12:04 |
| dviroel | #link https://etherpad.opendev.org/p/watcher-eventlet-removal (watcher evenlet removal etherpad) | 12:04 |
| dviroel | at the beginning of the etherpad there is a list of changes open for review | 12:05 |
| dviroel | one patch recently merged | 12:05 |
| dviroel | the one that merge dec-engine services | 12:05 |
| dviroel | #link https://review.opendev.org/c/openstack/watcher/+/952499 | 12:05 |
| dviroel | the others are open for review | 12:05 |
| sean-k-mooney | are you plannign to respine https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/954264 | 12:06 |
| dviroel | sean-k-mooney: yeah, i was thinking yes, but didn't get into it yet | 12:06 |
| dviroel | or maybe provide a follow up to improve it | 12:06 |
| sean-k-mooney | ack i had a look at it and you seaid you would fix one of the commend in the next patchset 9 days ago | 12:07 |
| sean-k-mooney | so i was partly waiting for you to do that | 12:07 |
| *** amoralej_ is now known as amoralej | 12:07 | |
| dviroel | ack, I can work on it in until the end of the week | 12:08 |
| dviroel | i was busy with other patches, which i should also propose soon | 12:08 |
| sean-k-mooney | ack no worreis | 12:09 |
| dviroel | ack, we can wait for a new ps on the plugin | 12:09 |
| sean-k-mooney | i aslo kind of agree with alfredo about chaning how the test works | 12:09 |
| dviroel | yeah me too | 12:09 |
| sean-k-mooney | i think it might be useful to have a dummy sleep stagey that we could use instead | 12:10 |
| sean-k-mooney | i..e one that will generate sleep actions | 12:10 |
| amoralej | chandankumar is working in another tempest test for continuous | 12:10 |
| sean-k-mooney | noop is also ok | 12:10 |
| sean-k-mooney | there are pros and cons to both aproches | 12:10 |
| amoralej | there is a dummy strategy which creates a testplan with two nop and one sleep | 12:11 |
| sean-k-mooney | yes so i think the main continue audit test shoudl use that or somethign eimilar | 12:11 |
| amoralej | i proposed to use it in https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/955472 | 12:11 |
| sean-k-mooney | not the zone migration stragy | 12:11 |
| sean-k-mooney | and i dont think we shoudl have a continues version of each stragey | 12:11 |
| amoralej | but it's still good to have one as dviroel one to test stuff related to model, which has shown to be problematic | 12:12 |
| dviroel | yeah, unless there is a reason, we can use the dummy instead | 12:12 |
| amoralej | nop or sleep actions will not validate access to model | 12:12 |
| sean-k-mooney | yes and no | 12:12 |
| sean-k-mooney | test of the continue audit fucntionalty shoudl be isolated form testing the logic of the stragies | 12:12 |
| sean-k-mooney | they should be tested independtly | 12:13 |
| sean-k-mooney | model updates are independent of the functioning of the continous audit | 12:13 |
| amoralej | yes, i think that's good, that's why i proposed to use dummy for the a full continuous cycle audit | 12:13 |
| sean-k-mooney | +1 | 12:13 |
| sean-k-mooney | just as an aside. | 12:14 |
| amoralej | but as dviroel found, continuous audits are sensitive to issues related to access to model | 12:14 |
| sean-k-mooney | if you are adding coment for multipel steps in a test | 12:14 |
| dviroel | agree, the scenario that I am testing is a corner case with threading mode | 12:14 |
| sean-k-mooney | that is generaly an indication that you are not propelry breaking up the test into the seprate parts or should at least condier pulling the part out into helper funcitons | 12:14 |
| amoralej | so there is space for both tests | 12:15 |
| chandankumar | I was also working on testing continous audit with cron format here https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/955472 got feedback from amoralej to use nop strategy . just wanted to highlight here to avoid duplicy | 12:15 |
| sean-k-mooney | the corner case shoudl be tested in the unit/functional suite not in the integration tests in general | 12:15 |
| dviroel | i can check how it would be possible to do that | 12:16 |
| chandankumar | ah amoralej already spoke about the same. | 12:16 |
| sean-k-mooney | chandankumar: yep we can loop back to the topic of cron format supprot | 12:16 |
| amoralej | it's interesting to test that kind of synchronization issues with unit tests .. | 12:17 |
| sean-k-mooney | unit and functional test are best positoin to test that | 12:18 |
| sean-k-mooney | tempest is very bad at testing race condtions | 12:18 |
| sean-k-mooney | we generally avoid that if possible | 12:18 |
| sean-k-mooney | but i dont know the detail of the issue dviroel found with threading as there isnt a bug report for it that expains it properly. | 12:20 |
| dviroel | it is not really a race issue, but an initialization problem, of the continuous audit service, which only happens in threading mode | 12:20 |
| sean-k-mooney | well you talks baout runnign the threads in diffent processes | 12:21 |
| sean-k-mooney | which woudl not change with or witherh trehadign | 12:21 |
| sean-k-mooney | and 2 thread can share a single data stucrue in memory | 12:21 |
| sean-k-mooney | so if they are indeepently creatign there own in memory model that is a bug even with eventlet | 12:22 |
| sean-k-mooney | a tempest test is not the correct way to prevent regressing that type of but a unit and preferably functional test is | 12:23 |
| sean-k-mooney | but perhaps we shoudl move on | 12:23 |
| sean-k-mooney | we can dicsuss that outside the meeting | 12:24 |
| dviroel | ack, i can check how I can reproduce the issue with funcional tests perhaps | 12:24 |
| dviroel | yes, we can move on | 12:25 |
| dviroel | thanks for the discussions on that topic | 12:25 |
| dviroel | that's what I have for the evenlet | 12:25 |
| sean-k-mooney | before we move on we shoudl try and create a bug report for this | 12:25 |
| sean-k-mooney | and likely we want to fix this indepenetly of the eventlet removal work | 12:26 |
| sean-k-mooney | or at least in a way we woudl be comfortabel backporting | 12:26 |
| sean-k-mooney | as it stand we would need to split https://review.opendev.org/c/openstack/watcher/+/952257/9 | 12:26 |
| sean-k-mooney | if we were to backport | 12:26 |
| dviroel | ack, I can file a Bug and try to reproduce it in a evenlet env | 12:27 |
| sean-k-mooney | you should be able to use id() | 12:27 |
| sean-k-mooney | or `is` | 12:27 |
| sean-k-mooney | to assert the refence to the data model is the same in the backgroud schduler as it is in the main data model i think | 12:27 |
| dviroel | ack | 12:28 |
| dviroel | will give a try on that | 12:28 |
| dviroel | tks sean-k-mooney | 12:28 |
| dviroel | moving to the next topic then | 12:28 |
| dviroel | #topic Skip actions feature update https://blueprints.launchpad.net/watcher/+spec/add-skip-actions | 12:29 |
| amoralej | i added that one | 12:29 |
| amoralej | i've sent some patches and I'd like to share the progress and how I did the split of the feature | 12:30 |
| dviroel | so you have an spec update too | 12:30 |
| amoralej | #link https://review.opendev.org/c/openstack/watcher-specs/+/954718 | 12:30 |
| amoralej | yes, that's just adding the same field status_message to the audit for convenience | 12:30 |
| amoralej | as it can be useful at some point, i think it'd be good to be covered by the same microversion | 12:30 |
| dviroel | ack, as discussed previously here in the channel I think | 12:31 |
| amoralej | exacgtly | 12:31 |
| amoralej | #link https://review.opendev.org/c/openstack/watcher/+/954745 | 12:31 |
| sean-k-mooney | yes it was | 12:31 |
| sean-k-mooney | i left a minor comment on the spec update but i htink it good in general | 12:31 |
| amoralej | that's the first implementation one, it just adds the status_messages to the database, object, notification and adds api microverson | 12:32 |
| sean-k-mooney | first feedback on that is your doing a lot in one patch | 12:32 |
| amoralej | i will fix that sean-k-mooney , thanks | 12:32 |
| sean-k-mooney | noramly you woudl do the object change in one patch db in anothe rand the api change gos a the end of the serise wehn the entire feature is functional | 12:33 |
| dviroel | +1 | 12:33 |
| amoralej | so, that one should be three patches? 1st db, 2nd, object, 3rd api change? | 12:34 |
| amoralej | note that's just status_message, nothing related to the SKIPPED yet | 12:35 |
| sean-k-mooney | yes can you split this in 3 , db schema changes in first patch, object and notificaiton changes in secodn and api and apiref changes in the final pathc whihc shoudl go after the other 2 you have already proposed | 12:35 |
| amoralej | so, i could join both api changes (status_message and patch action) in the same patch and api microviersion ? | 12:36 |
| sean-k-mooney | yes you can do that and proably should | 12:36 |
| sean-k-mooney | do you have anything that is setting the status message to a value in the first patch | 12:36 |
| amoralej | nop, but in the second | 12:36 |
| amoralej | when doing automatic SKIPPED | 12:36 |
| sean-k-mooney | right so we do not make the api changes before the code that uses the new fields | 12:37 |
| amoralej | i'm setting status_message | 12:37 |
| sean-k-mooney | the api change alwasy comes at the end after the feature is fully working. | 12:37 |
| amoralej | ok, let's move on to the next patches, and at the end we can agree to the entire sequence of patches | 12:37 |
| amoralej | #link https://review.opendev.org/c/openstack/watcher/+/954746/ | 12:38 |
| amoralej | that's the automatic skipped based on pre_condition | 12:38 |
| dviroel | ack, so this one is using the fields from previous patch | 12:39 |
| amoralej | yes | 12:39 |
| amoralej | in this one, i'm extending the nop action to ease testing different situations https://review.opendev.org/c/openstack/watcher/+/954746/4/watcher/applier/actions/nop.py | 12:40 |
| amoralej | i hope i don't need an spec to modify the nop action :) | 12:40 |
| sean-k-mooney | well technialy you do | 12:40 |
| sean-k-mooney | beacuse your modifyint eh api schema for the parmaters | 12:41 |
| amoralej | yes, but it's just testing action .... | 12:41 |
| sean-k-mooney | i think case i think we are likely ok with this as part of the larger work | 12:41 |
| sean-k-mooney | there is no just | 12:41 |
| sean-k-mooney | it follow the same rules as the rest | 12:41 |
| sean-k-mooney | but you r right that its not going to break real uscases | 12:41 |
| sean-k-mooney | i think this is ok but can you split it into its own commit | 12:42 |
| sean-k-mooney | we might want to backprot that for other testing usecases in the future | 12:42 |
| amoralej | the problem is that, for that i need the new exception i'm creating | 12:42 |
| sean-k-mooney | so having it as its own commit will make that simpeler | 12:42 |
| amoralej | so we have chicken-and-egg | 12:42 |
| sean-k-mooney | well you dont you can extend it twice | 12:43 |
| sean-k-mooney | once to add the fail cases | 12:43 |
| amoralej | ok, i can create one commit for the nop action for "everything" but the skip part | 12:43 |
| sean-k-mooney | and then again for the skip part | 12:43 |
| amoralej | ok | 12:43 |
| sean-k-mooney | exactly | 12:43 |
| sean-k-mooney | that can go really early in the series for what its worth | 12:44 |
| sean-k-mooney | although the order is up to you | 12:44 |
| amoralej | yep, actually, would not depend on anything else | 12:44 |
| sean-k-mooney | right that was my thinking as well | 12:44 |
| amoralej | #link https://review.opendev.org/c/openstack/watcher/+/955753/ | 12:44 |
| amoralej | that's the new PATCH /actions/<id> part | 12:44 |
| amoralej | includes its own microversion | 12:45 |
| sean-k-mooney | ya again i would split that in 2, one for the non api part first and hten combine the othe rapi change form before into the second patch and do all the api changes at once | 12:45 |
| amoralej | i think there is no non-api part | 12:46 |
| amoralej | lemme double check | 12:46 |
| sean-k-mooney | you hav changes to the applier | 12:46 |
| sean-k-mooney | but your right its mostly api and docs | 12:46 |
| dviroel | only applier piece yes | 12:47 |
| amoralej | it's very closely related to the api change | 12:47 |
| amoralej | actually, i'd may do that when adding the SKIPPED state | 12:47 |
| amoralej | it'd be more correct probably | 12:47 |
| sean-k-mooney | that might be better its not actully related to the api changes it related to the object changes | 12:47 |
| amoralej | yes, i will do that, it makes sense | 12:48 |
| sean-k-mooney | so this 3 patch serise will likely be 5-7 after the bits are split and re arranged | 12:48 |
| amoralej | somethink to remark, i'm adding https://review.opendev.org/c/openstack/watcher/+/955753/1/watcher/api/controllers/v1/types.py | 12:48 |
| sean-k-mooney | and the nop change can be merged indepentely of the rest | 12:48 |
| amoralej | a new way to validate patch calls based on allowed_attributes only | 12:48 |
| amoralej | in addition to internal_attributes | 12:49 |
| amoralej | i think that may be useful for others | 12:49 |
| amoralej | it's a more restrictive way | 12:49 |
| sean-k-mooney | if you think it might be it should be its on patch | 12:49 |
| sean-k-mooney | but alos we shoudl liekly discuss that seperatly | 12:49 |
| sean-k-mooney | thanks for highligting it however | 12:49 |
| amoralej | implementing here adds a use case to discuss it, which is useful i think | 12:50 |
| amoralej | but i can do depends on | 12:50 |
| sean-k-mooney | i want to dicsson oru api implation a tthe ptg | 12:50 |
| sean-k-mooney | * cant type right now | 12:50 |
| sean-k-mooney | im generallly nnot super happy with how we do api validation and the technologies/libs watcher is cureently useing | 12:51 |
| sean-k-mooney | but i think any large refactorign of that shoudl be indepenent of your series | 12:51 |
| amoralej | so, should i avoid doing that? | 12:52 |
| amoralej | tbh, i felt it was an improvement but there may be a better way | 12:52 |
| sean-k-mooney | was it discussed in the spec as part of the feature | 12:52 |
| amoralej | nop, but that's just an internal implementation detail, do not affect users | 12:53 |
| sean-k-mooney | we can discuss it and perhaps still do it this cycle but its not part of the skipped feature | 12:53 |
| sean-k-mooney | it changes api validation and there for could | 12:53 |
| sean-k-mooney | so its boarderline | 12:53 |
| amoralej | note i'm not applying to any other call | 12:53 |
| amoralej | default behavior is keeping the existing behavior | 12:53 |
| sean-k-mooney | and it skips if its empty so its backward compatible | 12:53 |
| amoralej | exactly | 12:54 |
| sean-k-mooney | but again it should be in a diffent commit | 12:54 |
| amoralej | ok, i will | 12:54 |
| sean-k-mooney | if we ever need that fuctionalty to fix a bug we woudl want to be able to cleanly backport it | 12:54 |
| sean-k-mooney | instead of extracting it form this change | 12:54 |
| dviroel | amoralej: any other patch that you want to highlight? | 12:55 |
| amoralej | 1. nop action change 2. change in validation 3. db change 4. object + notification 5. adds SKIPPED state 6. api change with microversion | 12:55 |
| amoralej | #link https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/955775 | 12:55 |
| amoralej | last one, that's the tempest test | 12:56 |
| amoralej | ah, i need to do the skip for previous versioins | 12:56 |
| amoralej | so, that's it | 12:57 |
| dviroel | altight, thanks for all the proposed changes, I will take some time to review then after you submit again | 12:58 |
| amoralej | thanks for your feedback, it's very helpful | 12:58 |
| dviroel | we have only 2 min left, so i will just skip the bug triage, but there are 2 new bugs there, if you want to take a look | 12:59 |
| dviroel | chandankumar: i believe that you might want to discuss more about yours? | 12:59 |
| dviroel | we are out of time, if needed, we can continue discussing afterwards in the channel | 12:59 |
| dviroel | the other one is a doc bug only, with a patch proposed, we can chat about next week | 13:00 |
| chandankumar | dviroel: we can revisit next time | 13:00 |
| dviroel | #topic Volunteers to chair next meeting | 13:00 |
| chandankumar | dviroel: I will chair for next meeting | 13:00 |
| dviroel | someone wants to chair? | 13:00 |
| dviroel | thanks chandankumar | 13:00 |
| dviroel | let's wrap up for today | 13:01 |
| dviroel | we will meet again next week | 13:01 |
| dviroel | thank you all for participating | 13:01 |
| dviroel | #endmeeting | 13:01 |
| opendevmeet | Meeting ended Thu Jul 24 13:01:19 2025 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 13:01 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-07-24-12.00.html | 13:01 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-07-24-12.00.txt | 13:01 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-07-24-12.00.log.html | 13:01 |
| rlandy | dviroel++ thanks | 13:01 |
| chandankumar | dviroel: ++ thank you for running it | 13:01 |
| amoralej | thanks dviroel++ | 13:01 |
| chandankumar | sean-k-mooney: I will discuss croniter review in next meeting. till then I will continue the discussion on review itself. | 13:01 |
| sean-k-mooney | ack | 13:02 |
| chandankumar | I created this bug https://bugs.launchpad.net/watcher/+bug/2118404 from croniter review to Drop python-dateutil dependency from watcher | 13:02 |
| chandankumar | I will propose the patch for the same soon, seems to be an easy one | 13:02 |
| sean-k-mooney | the main point i wanted to clarify is we need ot review what was approved in the spec (shocker they did not defein what the ment by crontab) so we need to review the test and docs and release note to see if watcher ever supproted the extened syntax or only the 5 colume basic syntax | 13:03 |
| sean-k-mooney | currently watcher is using 4 diffent timezone libs directly or indrectly which is insange | 13:03 |
| sean-k-mooney | i did not check the db schema but we shoudl be storign them as unix timestables or isoformated strings | 13:04 |
| sean-k-mooney | so i think we can jsut use the built in datetime instead or oslo's time utils | 13:04 |
| sean-k-mooney | btu i have not looked too closely | 13:04 |
| sean-k-mooney | ok ya we are suing the sqlalchemy DateTime filed for datatime columns | 13:05 |
| sean-k-mooney | https://docs.sqlalchemy.org/en/20/core/type_basics.html#sqlalchemy.types.DateTime | 13:07 |
| sean-k-mooney | which per there docs are direclty compatible with python datatime module | 13:07 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher-specs master: Add status_message field to the Audits https://review.opendev.org/c/openstack/watcher-specs/+/954718 | 13:11 |
| chandankumar | sean-k-mooney: thank you for the pointer for releasenotes and db schema , let me check that also and add more datapoints there. | 13:40 |
| dviroel | hey sean-k-mooney, one question wrt https://github.com/openstack/watcher-specs/blob/master/specs/2025.2/approved/extend-compute-model-attributes.rst#rest-api-impact | 15:29 |
| dviroel | so you think that make sense to also include here the flavor extra_specs? | 15:29 |
| dviroel | this info is also available in server list --long and also in newer versions of nova notification | 15:29 |
| sean-k-mooney | yes i think including the extra specs is valueable | 15:42 |
| opendevreview | chandan kumar proposed openstack/watcher master: [WIP] replace dateutils usage with datetime module https://review.opendev.org/c/openstack/watcher/+/955809 | 16:45 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Add parameters to force failures in nop action https://review.opendev.org/c/openstack/watcher/+/955813 | 17:19 |
| opendevreview | Douglas Viroel proposed openstack/watcher master: Add new tests to validate GET /infra-optim/v1/data_model https://review.opendev.org/c/openstack/watcher/+/955820 | 18:42 |
| opendevreview | Douglas Viroel proposed openstack/watcher master: Fix api-ref doc for GET /infra-optim/v1/data_model https://review.opendev.org/c/openstack/watcher/+/955711 | 20:01 |
| opendevreview | Douglas Viroel proposed openstack/watcher master: Add new tests to validate GET /infra-optim/v1/data_model https://review.opendev.org/c/openstack/watcher/+/955820 | 20:01 |
| opendevreview | Douglas Viroel proposed openstack/watcher master: WIP - Extend compute model attributes https://review.opendev.org/c/openstack/watcher/+/955827 | 20:01 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!