| opendevreview | David proposed openstack/watcher-tempest-plugin master: Add metrics for RAM to gnocchi and prometheus injected data https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/949773 | 09:17 |
|---|---|---|
| opendevreview | chandan kumar proposed openstack/watcher-tempest-plugin master: Use get_host_for_server and get_host_other_than to get src/dst node https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/949557 | 09:57 |
| opendevreview | chandan kumar proposed openstack/watcher-tempest-plugin master: Use get_host_for_server and get_host_other_than to get src/dst node https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/949557 | 10:23 |
| opendevreview | David proposed openstack/watcher-tempest-plugin master: Add metrics for RAM to gnocchi and prometheus injected data https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/949773 | 10:27 |
| opendevreview | David proposed openstack/watcher-tempest-plugin master: Add tests for workload_balance with injected data https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/949722 | 10:27 |
| opendevreview | David proposed openstack/watcher-tempest-plugin master: Add tests for workload balance with real data https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/950335 | 11:53 |
| chandankumar | #startmeeting watcher | 12:01 |
| opendevmeet | Meeting started Thu May 22 12:01:03 2025 UTC and is due to finish in 60 minutes. The chair is chandankumar. Information about MeetBot at http://wiki.debian.org/MeetBot. | 12:01 |
| opendevmeet | Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. | 12:01 |
| opendevmeet | The meeting name has been set to 'watcher' | 12:01 |
| chandankumar | o/ | 12:01 |
| chandankumar | who all are around here for watcher meeting? | 12:01 |
| dviroel | o/ | 12:01 |
| chandankumar | courtesy ping: dviroel amoralej jgilaber_ | 12:01 |
| *** jgilaber_ is now known as jgilaber | 12:01 | |
| jgilaber | o/ | 12:02 |
| morenod | o/ | 12:02 |
| rlandy | o/ | 12:02 |
| chandankumar | let me start with the first topic | 12:03 |
| amoralej | o/ | 12:03 |
| chandankumar | #topic Eventlet removal | 12:03 |
| sean-k-mooney | o/ | 12:03 |
| mtembo | o/ | 12:03 |
| chandankumar | dviroel: sent a call for MAAS maintainers | 12:04 |
| dviroel | right | 12:04 |
| chandankumar | #link https://lists.openstack.org/archives/list/openstack-discuss@lists.openstack.org/thread/SCKFJ4X6I7OSENXEYNCSJBKV55EP43MC/ | 12:04 |
| dviroel | as an action from last meeting, I sent this email to the ML | 12:04 |
| dviroel | to call for maintainers for MAAS support | 12:04 |
| rlandy | what if no response is received? | 12:04 |
| chandankumar | thank you dviroel ! | 12:05 |
| dviroel | this is one the things that will be affected by eventlet removal and one of the features that have no testing and docs | 12:05 |
| dviroel | i mentioned that we also plan to mark features as experimental, if they lack support/testings/doc | 12:05 |
| sean-k-mooney | rlandy: we will mark it expermintal this cycle | 12:06 |
| sean-k-mooney | next cycle we will likely deprecate | 12:06 |
| sean-k-mooney | and remove in a future reelase | 12:06 |
| dviroel | right | 12:06 |
| dviroel | once we propose a patch for that, the plan is to send an email again, to announce it | 12:07 |
| sean-k-mooney | the ironic integration is in a similar state but i think we have more of a chace of adressing the gaps | 12:07 |
| rlandy | ok | 12:07 |
| sean-k-mooney | because if noting else we can ask the ironic tema for help | 12:08 |
| chandankumar | we can wait for 2 week for response? then propose the patch | 12:08 |
| sean-k-mooney | we shoudl mark that as experimental this cycle too but follow up with them when we have time | 12:08 |
| sean-k-mooney | no we can propsoe the patch to make it experimental now | 12:08 |
| sean-k-mooney | and even merge it | 12:08 |
| sean-k-mooney | to remove that label i would liek to see ci in place and docs | 12:08 |
| sean-k-mooney | so they can remove the experimental marker when they adress the gaps if someone shows up | 12:09 |
| dviroel | right, but we should do the same for all that lack testing and docs too | 12:09 |
| sean-k-mooney | yes | 12:09 |
| chandankumar | sounds like a plan! | 12:09 |
| sean-k-mooney | thats why i raised ironic | 12:09 |
| dviroel | that's why it deserves another mail to ML when we do that | 12:10 |
| sean-k-mooney | yep | 12:10 |
| dviroel | ok | 12:10 |
| sean-k-mooney | we shoudl annouch each one we want to mark as experimetnal and or deprecated for visablity | 12:10 |
| chandankumar | let me add a action item for marking maas and ironic as a experimental and communicate the same on email | 12:10 |
| chandankumar | Anyone one wants to take this action item? | 12:10 |
| dviroel | I can take it | 12:11 |
| chandankumar | #action dviroel to propose patch for marking MAAS and Ironic support experimental in watcher and communicate the same to the Mailing list | 12:11 |
| chandankumar | thank you dviroel ! | 12:12 |
| dviroel | np | 12:12 |
| chandankumar | there are two open reviews related to eventlet removal | 12:12 |
| dviroel | yep, small ones | 12:12 |
| chandankumar | 1. https://review.opendev.org/c/openstack/watcher/+/949641: Move eventlet command scripts to a different dir | 12:12 |
| dviroel | but a start | 12:12 |
| chandankumar | 2. https://review.opendev.org/c/openstack/watcher/+/949658: Remove deprecated executor in message handling servers | 12:12 |
| chandankumar | please review it in your free cycles, | 12:13 |
| chandankumar | thank you dviroel once again for bringing it up! | 12:13 |
| dviroel | I also started working on changes related to the BackgroundSchedulerService and Service class, but I will bring more details in the next meetings | 12:13 |
| chandankumar | great! | 12:14 |
| chandankumar | dviroel: annything else to add on this topic? | 12:14 |
| dviroel | yep, that's all that I | 12:14 |
| chandankumar | thank you! | 12:14 |
| chandankumar | moving to next topic | 12:14 |
| dviroel | tks | 12:15 |
| chandankumar | #topic Parameters for volume migration in zone migration strategy | 12:15 |
| chandankumar | jgilaber: has proposed this topic and summarized the current state of input parameters for volume migration in the zone migration strategy in this etherpad https://etherpad.opendev.org/p/zone_migration_volume_migration_parameters | 12:15 |
| chandankumar | jgilaber: want to introduce it? | 12:15 |
| jgilaber | yes thanks chandankumar | 12:15 |
| jgilaber | I have a patch for making the dst_pool parameter mandatory in zone migration | 12:16 |
| jgilaber | but as amoralej pointed out, the UX is not great | 12:16 |
| chandankumar | #link https://etherpad.opendev.org/p/zone_migration_volume_migration_parameters | 12:16 |
| jgilaber | there are scenarios where that parameter would not be used at all | 12:16 |
| jgilaber | so I wrote in that etherpad the current volume migration scenarios that the strategy supports | 12:17 |
| jgilaber | so we can decide on the parameters that we want to keep, and which we want to mark as required | 12:17 |
| jgilaber | feel free to add comments to the etherpad | 12:18 |
| chandankumar | #link https://review.opendev.org/c/openstack/watcher/+/950149: Handle missing dst_pool parameter in zone_migration | 12:18 |
| amoralej | My suggestion would be to rethink which use cases we want to support and which parameter we need to each one based on that | 12:18 |
| amoralej | and we may involve some cinder expert to make sure the implementation is correct, as there are some implementation details which are unclear | 12:19 |
| amoralej | by use cases i mean: migrate to a new pool, retype in the same pool, or migrate via retype, and also which are the affected volumes, those in the src_pool AND src_type ? | 12:20 |
| sean-k-mooney | well thsoe 3 are a oneof choice | 12:21 |
| sean-k-mooney | i think its valid to supprot all of the abvoe | 12:21 |
| amoralej | i agree, i think those are valid ones | 12:21 |
| dviroel | right, but the way that parameters are set to mandatory, don't look correct | 12:22 |
| amoralej | but then we need to define interaction between dst_type and dst_pool | 12:22 |
| sean-k-mooney | dviroel: well it may be | 12:22 |
| dviroel | if you want to retype, you need to set a dst_pool, which is not used by retype | 12:22 |
| sean-k-mooney | im nto sure hwo its expsed in the cinder api | 12:22 |
| amoralej | not really | 12:22 |
| amoralej | only dst_type | 12:22 |
| sean-k-mooney | but in nova retyp and codle migrate are implemtned underneate as teh same code | 12:23 |
| amoralej | actually, retype does not specify destination pool, only type | 12:23 |
| sean-k-mooney | cold migrate is just a resize to the same flavor | 12:23 |
| amoralej | but if the dst_type is not assigned to the src_pool it will do also the migration under the hood | 12:23 |
| sean-k-mooney | amoralej: right which is likely why this was optional in the first palce | 12:23 |
| amoralej | yes, that's my guess too | 12:24 |
| sean-k-mooney | so what we need to do is map out each of the possibel api actions we could call | 12:24 |
| sean-k-mooney | then map out the vaild import parmater that satify the api consstratits | 12:24 |
| sean-k-mooney | and then see who to evolve the scema to match | 12:24 |
| sean-k-mooney | for example if you are not retyping then we may requrie teh pools but make them optional or forbdi them if you are | 12:25 |
| amoralej | and also, we may want to revise management attached and dettached in the strategy. According to doc we may do migrate or retype for attached unless volumes are multi-attached | 12:25 |
| sean-k-mooney | we will have to decied how much to encode in the schema and how much to do in the validate fucntion in the api | 12:25 |
| amoralej | ^ this is where it'd be good to get cinder team input | 12:25 |
| sean-k-mooney | right multi atatch volume cant be mvoed while they have mroe the one attachment | 12:26 |
| sean-k-mooney | so we shoudl filter those out | 12:26 |
| dviroel | right, one important thing to check is the multi-attach too | 12:26 |
| sean-k-mooney | honestly fixing the cidner part almost feesl like ti should have a spec | 12:27 |
| sean-k-mooney | on one hand it might be small and easy | 12:27 |
| amoralej | according to the doc, that seems the main limitation to retype/migrate attached volumes. If we filter out those, we probably can unify how we manage attached/dettached | 12:27 |
| sean-k-mooney | btu its oen of those things where you reallly need to write it down to reason about it | 12:27 |
| amoralej | and document clearly | 12:27 |
| sean-k-mooney | shoudl we start a etherpad to try and help with discussion | 12:27 |
| jgilaber | yes, I think a spec might be needed here, I doubt it will be a simple change | 12:28 |
| amoralej | we can use the one that @jgilaber created | 12:28 |
| sean-k-mooney | one other thing to consdier | 12:28 |
| sean-k-mooney | if your using ceph | 12:28 |
| sean-k-mooney | there are no pools | 12:28 |
| amoralej | I'd put a section describing current status and one with the desired behavior | 12:28 |
| sean-k-mooney | but you shoudl be able to migrate between ceph clusters | 12:28 |
| sean-k-mooney | current and desireed state is defintly somethign we shoudl capture | 12:29 |
| amoralej | iiuc migrate call can also migrete from cluster to cluster ? | 12:30 |
| jgilaber | there is a cluster option in the api call https://docs.openstack.org/api-ref/block-storage/v3/index.html?expanded=detach-volume-from-server-detail#migrate-a-volume | 12:30 |
| dviroel | tbh, this doesn't need to be pools, can be backends, so pool might not be the correct name there too | 12:30 |
| jgilaber | I guess that's meant to be used with ceph? | 12:30 |
| sean-k-mooney | its not clear | 12:31 |
| sean-k-mooney | cinder shoudl have some other admin/user facign docs for this | 12:32 |
| amoralej | iiuc in migrate call, when using host, host can be backend or backend#pool | 12:32 |
| sean-k-mooney | we can review that but we shoudl also dicuss this mroe with the cinder team | 12:32 |
| amoralej | +1 | 12:32 |
| dviroel | ++ | 12:32 |
| chandankumar | +1 | 12:32 |
| jgilaber | sounds good, short term, what are your thoughts on https://review.opendev.org/c/openstack/watcher/+/950149? should I abandon it, or change the behaviour so that if dst_pool is not passed migration is skipped | 12:33 |
| chandankumar | Let me add a action item here to add section describing current state and desired behavior on the etherpad https://etherpad.opendev.org/p/zone_migration_volume_migration_parameters and continue the discussion there | 12:33 |
| jgilaber | to avoid the error described in the associated bug | 12:33 |
| sean-k-mooney | i think instead of tightenign the schema | 12:34 |
| sean-k-mooney | we will end up makeign ti a little looser adn do a more contextural check in python | 12:35 |
| amoralej | I don't like making it mandatory tbh | 12:35 |
| sean-k-mooney | wll that what i ment by looser | 12:35 |
| dviroel | and improve the documentation :) | 12:35 |
| sean-k-mooney | we will likely remove the mandaory requirement form other filed as weell and docuemtn the combinations that work adn also add a check outside the scema in the python code to enfoce teh workign combidnations | 12:36 |
| sean-k-mooney | i think encoding the logic in jsonschema is going to be overly complex | 12:36 |
| amoralej | there will be no way, i'd say | 12:36 |
| sean-k-mooney | we have 5+ paralle axis | 12:37 |
| jgilaber | I don't see a simple way, no | 12:37 |
| amoralej | note that the behavior also depends on how types/pools/backends are configures | 12:37 |
| sean-k-mooney | there are 3 diffent storage operations + vm operations and orderign considertaons | 12:37 |
| amoralej | so implementing a use case may involve also configure a type in a certain way | 12:37 |
| sean-k-mooney | yep | 12:38 |
| sean-k-mooney | since we acknowlage this is a gap i think we shoudl move on for now | 12:38 |
| jgilaber | +1 | 12:38 |
| amoralej | short term, something that may fix is to add a check for dst_pool before calling _volume_migrate | 12:39 |
| amoralej | if it's not defined, just ignore that volume | 12:39 |
| amoralej | we may save some errors, but still the implementation needs to be revised in deep | 12:39 |
| amoralej | but yeah, we can move on | 12:40 |
| sean-k-mooney | we coudl but that geenally would not be correct api behavior | 12:40 |
| sean-k-mooney | if the inputs do not meet the preqistis for the rested orpetaon we shoudl return a 400 | 12:40 |
| sean-k-mooney | and not attempt to do the operation | 12:41 |
| chandankumar | I think we have got some direction on the above review, we can bring this one in the next meeting? | 12:41 |
| sean-k-mooney | sure if there is progress on it | 12:41 |
| sean-k-mooney | shall we move on to https://etherpad.opendev.org/p/openstack-watcher-irc-meeting#L52 | 12:42 |
| chandankumar | yes let me add one action item | 12:42 |
| chandankumar | #action jgilaber to add section describing current state and desired behavior on the etherpad https://etherpad.opendev.org/p/zone_migration_volume_migration_parameters and continue the discussion. | 12:43 |
| chandankumar | thank you everyone for the great discussion | 12:43 |
| chandankumar | moving to next one | 12:43 |
| chandankumar | #topic Reviews | 12:43 |
| chandankumar | #link 1. Check logs in some cinder and nova helper tests https://review.opendev.org/c/openstack/watcher/+/949187 | 12:44 |
| jgilaber | this patch adds tests to reproduce the bug https://bugs.launchpad.net/watcher/+bug/2110149 | 12:44 |
| jgilaber | some logging calls were misformated, but the unit tests were not logging them | 12:45 |
| chandankumar | #link 2. Fix incorrect logging format https://review.opendev.org/c/openstack/watcher/+/822559 | 12:45 |
| jgilaber | because alembic would reconfigure the logging and disable it | 12:45 |
| jgilaber | the second link fixes the logging call | 12:45 |
| jgilaber | the patch has been opened for a while, I've taken it over | 12:45 |
| sean-k-mooney | i think the only thing i was waiting for on the second patch was for you to update that once you got the logging woring in the previous one | 12:46 |
| sean-k-mooney | so ill re review that | 12:46 |
| sean-k-mooney | i think directionally the patches are fine | 12:46 |
| jgilaber | yes, thanks sean-k-mooney | 12:46 |
| chandankumar | #link 3. Add spec for skip actions from action plan executions https://review.opendev.org/c/openstack/watcher-specs/+/949528 | 12:46 |
| sean-k-mooney | its form this really shade guy by the name of amoralej :) i dont know if we shoudl review | 12:48 |
| chandankumar | everybody please take a look at the open reviews and have feedback! | 12:48 |
| sean-k-mooney | am that also on my queue | 12:48 |
| amoralej | yeah, so there aer a couple of questions open in that spec | 12:48 |
| amoralej | i'd like to get the feedback before sending a new version of it | 12:48 |
| sean-k-mooney | ack do you want to raise any of them now | 12:49 |
| amoralej | one of them is where to allow the transition | 12:49 |
| amoralej | i was thinking of allowing actions from PENDING->SKIPPED | 12:50 |
| sean-k-mooney | that is what i avocated for on irc when we last spoke about it | 12:50 |
| amoralej | but @dviroel made a good point that we should check that not only the action is pending but the parent action plan | 12:50 |
| amoralej | it's slightly different | 12:51 |
| amoralej | but i think it covers better the intent | 12:51 |
| sean-k-mooney | yes that because of how they are stored in teh db | 12:51 |
| amoralej | second one is about your proposal of implementin api call ' /v1/action_plans/{plan_id}/actions/{action id or index}' | 12:52 |
| sean-k-mooney | without going into the hsitory too much, correct me if im wrong but for some reason tehy deceid to create the actions in the db before the plan is is accpated or rejected | 12:52 |
| amoralej | instead of my proposal on patching /v1/actions/{action_id} | 12:52 |
| amoralej | exactly | 12:52 |
| amoralej | the actions are created as soon as the action plan is created | 12:52 |
| sean-k-mooney | right which is a way to do it | 12:53 |
| sean-k-mooney | its not how i woudl do it if we wrote it today | 12:53 |
| sean-k-mooney | but that is why we need to also check the action plan state | 12:53 |
| amoralej | and since then actions are a 1st level api element | 12:53 |
| amoralej | not a nested one into action_plans | 12:53 |
| sean-k-mooney | tehy are but i am not conficed they shoudl have a writeable api at all | 12:54 |
| sean-k-mooney | to me actions are intenal state | 12:54 |
| sean-k-mooney | which si why i was expect to modify the actiov via the action plan | 12:54 |
| sean-k-mooney | i understand why you wanted to modify it with /v1/actions/{action_id} | 12:55 |
| amoralej | wdym by internal state ? | 12:55 |
| amoralej | you mean from db store point of view or from api point of view? | 12:56 |
| sean-k-mooney | i mean that action are generate by the decsion engine | 12:56 |
| sean-k-mooney | use accppat action plans or reject them today | 12:56 |
| sean-k-mooney | they do not have granulartiy beyond that | 12:56 |
| amoralej | right | 12:56 |
| sean-k-mooney | so ot me teh actions are expsoe mroe for debuging or tracablity | 12:56 |
| sean-k-mooney | not as a first class thing that user shoudl direcly manipulate | 12:57 |
| chandankumar | time check: 3mins left , we have one more topic left | 12:57 |
| amoralej | ok, i think i get your point | 12:57 |
| sean-k-mooney | now we coudl open them up to the user | 12:57 |
| sean-k-mooney | bu tsicne they cant be sued on tehre own it feels a bit wer to do that | 12:57 |
| amoralej | i see what do you mean now | 12:58 |
| sean-k-mooney | we can do what you suggest btu to me that is a larger chnage then i intially tought | 12:59 |
| amoralej | but in practical terms, i think it will be harder to implement your way | 12:59 |
| amoralej | with small win, imo | 12:59 |
| sean-k-mooney | so the other thing im reflecting on is im not sure we shoudl be supproting patch on any of the apis | 12:59 |
| amoralej | implementing a call to /v1/action_plans/{plan_id}/actions/{action id or index} would require to also implement new api attribute actions under the action_plan | 12:59 |
| sean-k-mooney | yes | 13:00 |
| sean-k-mooney | and we will need a new one for actions also | 13:00 |
| sean-k-mooney | for the patch method if we decied to supprot htat | 13:00 |
| amoralej | well, we will need to add the patch method | 13:00 |
| amoralej | right | 13:00 |
| sean-k-mooney | so im more inclide to modeel this after nova instance actions insead | 13:01 |
| amoralej | but wouldn't be that much easier that adding showing the actions under action_plan + patch/post | 13:01 |
| amoralej | ? | 13:01 |
| sean-k-mooney | btu that a longer dicssion then our -1 mins allow | 13:01 |
| amoralej | yeah | 13:01 |
| sean-k-mooney | amoralej: eiaser yes | 13:01 |
| chandankumar | sean-k-mooney: amoralej I think we can take up this discussion on the spec itself? | 13:01 |
| sean-k-mooney | correct im not sure | 13:01 |
| sean-k-mooney | yep | 13:01 |
| chandankumar | or we can do a spec review meeting in future to review the spec. | 13:02 |
| amoralej | +1 | 13:02 |
| chandankumar | Do we want to skip the bug triage for today? | 13:02 |
| sean-k-mooney | we can carry on the converstaion in gerrit and we can chat on irc outside the meeting as weel | 13:02 |
| sean-k-mooney | i think so | 13:03 |
| chandankumar | I will paste the bug links here, feel free to take a look | 13:03 |
| amoralej | we are 3 minutes overtime, so i guess so | 13:03 |
| chandankumar | #link https://bugs.launchpad.net/watcher/+bug/2110895 - Drop post/patch/delete undocumented rest api call from watcher action | 13:03 |
| chandankumar | #link https://bugs.launchpad.net/watcher/+bug/2110897 - Add more docs around openstack optimize audit command in doc | 13:03 |
| chandankumar | #link https://bugs.launchpad.net/watcher/+bug/2107467 (workload_stabilization strategy does not show standard_deviation if it's below the audit thre | 13:03 |
| chandankumar | #link https://bugs.launchpad.net/watcher/+bug/2110991 [doc] Plugin docs still refers to Voluptuous schemas | 13:04 |
| chandankumar | going over last topic | 13:04 |
| chandankumar | #topic volunteer to chair next meeting | 13:04 |
| chandankumar | anyone up for that? | 13:04 |
| rlandy | I'll do it | 13:04 |
| chandankumar | #action rlandy to chair for next meeting | 13:04 |
| chandankumar | rlandy: thank you! | 13:05 |
| chandankumar | time to wrap up! | 13:05 |
| dviroel | ++ | 13:05 |
| chandankumar | #endmeeting | 13:05 |
| opendevmeet | Meeting ended Thu May 22 13:05:11 2025 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) | 13:05 |
| opendevmeet | Minutes: https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-05-22-12.01.html | 13:05 |
| opendevmeet | Minutes (text): https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-05-22-12.01.txt | 13:05 |
| opendevmeet | Log: https://meetings.opendev.org/meetings/watcher/2025/watcher.2025-05-22-12.01.log.html | 13:05 |
| dviroel | tks chandankumar++ | 13:05 |
| amoralej | Thanks chandankumar++ | 13:05 |
| chandankumar | sorry for the long meeting. | 13:05 |
| mtembo | Thanks Chandankumar++ | 13:05 |
| jgilaber | thanks chandankumar you can blame me for the meeting duration :) | 13:05 |
| amoralej | sean-k-mooney, wrt using patch calls, another thing i considered for my proposal was consistency with the existing implementation, i understand we may improve thing, but maybe it should be a different initiative to maintain consistency in the api | 13:06 |
| amoralej | no blame at all when discussions are useful :) | 13:07 |
| chandankumar | :-) | 13:08 |
| sean-k-mooney | amoralej: ya so when i started looking at the tech debit i was wondering if we shoud lremvoe all the current patch urls | 13:18 |
| sean-k-mooney | it will be a new api microversion either way | 13:18 |
| sean-k-mooney | the discussion we are having is really about the scope | 13:19 |
| amoralej | yes i guess that should be a separate spec | 13:19 |
| sean-k-mooney | the mroe i looke at the watchre api the less sound it looks to me | 13:19 |
| amoralej | the main problem of patch is the requirement to do proper validation? | 13:19 |
| sean-k-mooney | not just that. historically it was not doabel in a broser alsthough you can do that now | 13:20 |
| sean-k-mooney | you jsut cant do it without doing a ajax request | 13:20 |
| amoralej | got it | 13:21 |
| sean-k-mooney | meaning to use it in the hroizon plugin we ned to do a post to a new api provided by the plugin which will on the servier side do a patch | 13:21 |
| sean-k-mooney | that woudl be requried anyway for secuirty reasons | 13:21 |
| amoralej | for me is a matter of priorities and order | 13:21 |
| sean-k-mooney | but my inclination is if we allow patch | 13:22 |
| sean-k-mooney | we woudl only allow it for 1 field | 13:22 |
| amoralej | sure | 13:22 |
| sean-k-mooney | and we woudl only allwo a subset of tansations | 13:22 |
| amoralej | i think i wrote that in the spec, otherwise it's my fault | 13:22 |
| sean-k-mooney | i.e. once its skiped you cant unskip | 13:22 |
| sean-k-mooney | amoralej: you may hae i have not looked at the spec in enough detail | 13:23 |
| amoralej | my proposal would be to do this as a patch, and then consider to move all the patch to post consistently | 13:23 |
| amoralej | and a spec for it | 13:23 |
| amoralej | i have no idea how to manage upgrade | 13:24 |
| amoralej | we'd need to keep supporting both with microversions, i guess, for some time | 13:24 |
| sean-k-mooney | amoralej: we dont tend to remove apis at least not for a few years | 13:27 |
| sean-k-mooney | the patch urls woudl work with the older microversion | 13:27 |
| sean-k-mooney | it sjust that in the new microversoin we woudl ahve somethign to replace them | 13:27 |
| sean-k-mooney | to me having peopel directly update teh state of an action while restful | 13:28 |
| sean-k-mooney | is leaking too much internal state to the end user | 13:28 |
| sean-k-mooney | i also thing put woudl be better then patch | 13:30 |
| sean-k-mooney | i really dont think that allowign an end use to pass data that we then execute is correct | 13:30 |
| amoralej | wdym with "pass data that we then execute" ? | 13:32 |
| amoralej | user would be only sending the state | 13:32 |
| sean-k-mooney | the patch interface recieve a "jason patch" defintion | 13:33 |
| sean-k-mooney | which si a docuemtn edxpre hwo to ttransfrom the resocue using add/remove/copy/replece and other operations | 13:33 |
| sean-k-mooney | so its data that we execute without our process | 13:34 |
| amoralej | ah | 13:34 |
| amoralej | got it | 13:34 |
| sean-k-mooney | if there is a bug in the execution engine it coudl be a securety issue | 13:34 |
| amoralej | you mean arbitrary data in the json that we have to transform | 13:34 |
| amoralej | got it | 13:34 |
| amoralej | that's right | 13:34 |
| sean-k-mooney | im kind of thinkign about sql injection and other fun things | 13:35 |
| sean-k-mooney | if we were to do a put | 13:35 |
| sean-k-mooney | we woudl replace teh entire object with the desired state | 13:35 |
| sean-k-mooney | rather then expresing the set of operations we want to apply to it to achive the desired state | 13:36 |
| sean-k-mooney | so the imperitive nature of it makes me uneasy | 13:36 |
| amoralej | i mean, in this case, i'd say the logic to process the json is pretty simple, but it's correct that it's more exposed to issues that exposing actions | 13:36 |
| sean-k-mooney | well we need to make sure we do full input validaton | 13:38 |
| sean-k-mooney | if we only allow the state to be updated | 13:38 |
| sean-k-mooney | and only allow it for a strict enmu vlaue for specific transtions | 13:38 |
| amoralej | yep | 13:38 |
| sean-k-mooney | then its workable | 13:38 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Set actionplan state to FAILED if any action has failed https://review.opendev.org/c/openstack/watcher/+/949225 | 14:37 |
| opendevreview | Joan Gilabert proposed openstack/watcher master: Add unit test zone migration with_attached_volume https://review.opendev.org/c/openstack/watcher/+/950664 | 15:09 |
| opendevreview | Joan Gilabert proposed openstack/watcher master: Support zone migration strategy without compute_nodes https://review.opendev.org/c/openstack/watcher/+/950665 | 15:09 |
| opendevreview | David proposed openstack/watcher-tempest-plugin master: Add tests for workload_balance with injected data https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/949722 | 15:56 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Set actionplan state to FAILED if any action has failed https://review.opendev.org/c/openstack/watcher/+/949225 | 17:06 |
| opendevreview | Alfredo Moralejo proposed openstack/watcher master: Set actionplan state to FAILED if any action has failed https://review.opendev.org/c/openstack/watcher/+/949225 | 17:20 |
Generated by irclog2html.py 4.0.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!