[Kimchi-devel] [PATCH v2] [WoK 00/10] '/config/plugins' API implementation
Aline Manera
alinefm at linux.vnet.ibm.com
Thu Jan 26 12:14:15 UTC 2017
On 01/26/2017 10:10 AM, Daniel Henrique Barboza wrote:
>
>
> On 01/26/2017 10:04 AM, Aline Manera wrote:
>>
>> Hi Daniel,
>>
>> I have just tested and
>>
>> del cherrypy.tree.apps[<plugin-uri>]
>>
>> Successfully disable a plugin.
>>
>> And
>>
>> cherrypy.tree.mount(<plugin-app>, <plugin-uri>, <plugin-config>)
>>
>> (Check server.py for more details about the parameters to
>> cherrypy.tree.mount)
>>
>> Enables a plugin.
>
> Interesting.
>
>>
>> So, it would be great if you could add that to your V3 patch as well.
>> Then, when we have the async notification events using websockets
>> working we can remove the need to reboot the web server.
>>
>> What do you think?
>
> You're proposing that /config/plugins/*name*/enable|disable would not
> just
> edit the config file but also do the on the fly enable/disablement of
> the plug-ins,
> even if for now the UI will need to reboot the web server after doing so
> because of the pending UI issues?
>
Yeap!
Then when we have support on async notification using websockets, we
just need to update the /config/reload API to send a request to all the
open browser sessions to reload its page (to reflect the plugin changes)
instead of restarting cherrypy
> I think it's ok. I'll get it done in v3.
Thanks!
>
>
>
>>
>> On 01/26/2017 09:41 AM, Aline Manera wrote:
>>>
>>>
>>> On 01/25/2017 07:17 PM, Daniel Henrique Barboza wrote:
>>>>
>>>>
>>>> On 01/25/2017 05:39 PM, Aline Manera wrote:
>>>>>
>>>>>
>>>>> On 01/25/2017 05:08 PM, Daniel Henrique Barboza wrote:
>>>>>>
>>>>>>
>>>>>> On 01/25/2017 03:37 PM, Aline Manera wrote:
>>>>>>> Hi Daniel,
>>>>>>>
>>>>>>> In overall, the patch set looks good but that will not be enough
>>>>>>> to close the feature request.
>>>>>>>
>>>>>>> You still need to add support to plugin dependencies. Kimchi and
>>>>>>> Ginger depends on Ginger Base and when enable/disabling one, the
>>>>>>> other should be enable/disable as well.
>>>>>>> We have already talked about a solution to do that and IMO it is
>>>>>>> simple to be implemented. Any special reason to do not do that
>>>>>>> right now?
>>>>>>
>>>>>> I am not sure if it's simple to be implemented but I'll look into
>>>>>> it and, if feasible,
>>>>>> add it in v3.
>>>>>>
>>>>>>>
>>>>>>> Also, you should investigate how to enable/disable a plugin on
>>>>>>> cherrypy configuration on the fly, without the need to restart
>>>>>>> Wok server.
>>>>>>> Cherrypy handles the API configuration in the form of a dict,
>>>>>>> maybe only removing the plugin entry is enough. Have you checked
>>>>>>> it? Any other solution/suggestion?
>>>>>>
>>>>>> I haven't found a way for cherrypy to reload a configuration on
>>>>>> the fly
>>>>>> without issuing server reload. As far as I've investigated it is
>>>>>> not possible to
>>>>>> retrieve the loaded modules after issuing cherrypy.tree.mount().
>>>>>>
>>>>>
>>>>> The right place to look at is: cherrypy.tree.apps. It will be a
>>>>> dict of plugin URI and plugin Root instance.
>>>>>
>>>>> Something like:
>>>>>
>>>>> {/plugins/kimchi: <KimchiRoot instance>, /plugins/ginger:
>>>>> <GingerRoot instance>, ...}
>>>>>
>>>>> I have not tested it before, but shouldn't we edit this to remove
>>>>> the entry we want and add when needed?
>>>>
>>>> Not sure, I can give a try. But all this discussion raises a
>>>> question. Back in the RFC, we agreed on a
>>>> solution that consists on editing the plug-ins config file and then
>>>> reloads WoK. We're now discussing a
>>>> way to make cherrypy enable/disable them on the fly. Is this now a
>>>> part of this same work?
>>>> My initial understanding was that the /config/reload API +
>>>> /config/plugins API closes the feature
>>>> request.
>>>>
>>>> There's also UI challenges with this 'on the fly' reload. The whole
>>>> idea of doing 'on the fly' is
>>>> to avoid a full UI reload + relogin when enabling/disabling
>>>> plug-in. Say userA is creating a new
>>>> VM snapshot in Kimchi. adminB disables gingerbase. Let's ignore the
>>>> fact that it can take 3 seconds
>>>> for the UI to know about it (the Notification interval). In this
>>>> case, Kimchi was also disabled, so the
>>>> current operation userA was about to do is now invalid. A full UI
>>>> reload is now required to
>>>> remove both plug-ins from the UI. This is the trivial scenario.
>>>>
>>>> The "bad" scenario is, instead of disabling Gingerbase, adminB
>>>> disables Ginger. Should we disrupt
>>>> userA work with a full refresh? Kimchi wasn't affected by it.
>>>> Should we wait until he does
>>>> a refresh window to remove Ginger?
>>>>
>>>> The worst scenario: userA is using Gingers390x, Gingerbase is
>>>> disabled. Unless Gingers390x
>>>> UI is aware of its full dependency tree, it won't be aware that
>>>> disabling Gingerbase cascades
>>>> into disabling Ginger, so a full refresh is required because
>>>> Gingers390x was disabled too.
>>>>
>>>> Note that all these scenarios can be solved by forcing a UI refresh
>>>> every time but, in this case,
>>>> the only difference between the 'on the fly' reload versus a reboot
>>>> reload is that on the second
>>>> you'll need to login again.
>>>>
>>>>
>>>> To sum up, I am not against investigating the cherrypy on the fly
>>>> reload at this moment, but
>>>> I advise against pushing it together with this work, without UI
>>>> changes. The 'reboot reload'
>>>> we proposed in the RFC does not share these problems because when
>>>> the browser loses
>>>> connectivity it goes back to the login screen.
>>>>
>>>
>>> Yeap! Maybe I was not clear in my first comment. What you need to do
>>> to get this patch accepted upstream is only the plugin dependencies
>>> stuff.
>>>
>>> IMO we should have an **investigation** on what is needed to get the
>>> plugin enabling/disabling on the fly to know the next steps.
>>> For example, the UI development needs to wait that decision to avoid
>>> waste of time and code.
>>>
>>> All the scenarios you pointed above are still valid for plugin
>>> enabling/disabling through configuration file.
>>> And the need to restart the Wok server is more intrusive as it may
>>> cause issues in the ongoing tasks the plugin is performing. For
>>> example, some leftovers may be on system due that and cause issues
>>> on future actions.
>>> IMO it is better to an user get a message "API is no longer
>>> available" then 'break' the system in some way due a web server
>>> restart when some tasks was ongoing.
>>> Think about clone/create/migrate virtual machines, create debug
>>> reports, create/restore backups - which a mess it may cause in the
>>> system on a server restart.
>>> The plugin management on the fly will not interrupt any on going
>>> tasks - they only will not be shown on UI anymore but the background
>>> task will continue to work.
>>> And only future requests will be blocked - avoiding leftover on system.
>>>
>>> I personally think the on the fly configuration makes better the
>>> user experience and avoid issues.
>>>
>>> Also to have the plugin enabling/disabling on the fly working
>>> properly we will need to have the websockets communication to inform
>>> all open browser sessions to reload the UI.
>>> So, as I said, we should only have in mind what will be the next
>>> steps on that area.
>>>
>>>>
>>>> Daniel
>>>>
>>>>
>>>>>
>>>>>> Any help is appreciated.
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Regards,
>>>>>>> Aline Manera
>>>>>>>
>>>>>>> On 01/20/2017 03:32 PM, dhbarboza82 at gmail.com wrote:
>>>>>>>> From: Daniel Henrique Barboza <danielhb at linux.vnet.ibm.com>
>>>>>>>>
>>>>>>>> v2:
>>>>>>>> - added User Log capabilities on /config/plugins/enable|disable
>>>>>>>> actions
>>>>>>>> - added 'enable=' as a valid entry in the parsing of the conf file
>>>>>>>>
>>>>>>>> This patch set implements the '/config/plugins' API. The idea
>>>>>>>> of this API is to replace the current '/plugins' API while
>>>>>>>> adding a new attribute called 'enabled' in its return value,
>>>>>>>> representing the current state of the plug-in. It will also
>>>>>>>> return all the plug-ins, regardless of whether it is loaded
>>>>>>>> or not.
>>>>>>>>
>>>>>>>> New actions /config/plugin/*name*/enable and
>>>>>>>> /config/plugin/*name*/disable were implemented to enable/disable
>>>>>>>> the plug-in *name* by editing the plug-in .conf file.
>>>>>>>>
>>>>>>>>
>>>>>>>> This is the API in action:
>>>>>>>>
>>>>>>>> [danielhb at arthas wok_all_plugins]$ curl -k -u danielhb -H
>>>>>>>> "Content-Type: application/json" -H "Accept: application/json"
>>>>>>>> -X GET 'https://localhost:8001/config/plugins'Enter host
>>>>>>>> password for user 'danielhb':
>>>>>>>> [
>>>>>>>> {
>>>>>>>> "enabled":true,
>>>>>>>> "name":"gingerbase"
>>>>>>>> },
>>>>>>>> {
>>>>>>>> "enabled":true,
>>>>>>>> "name":"kimchi"
>>>>>>>> },
>>>>>>>> {
>>>>>>>> "enabled":true,
>>>>>>>> "name":"ginger"
>>>>>>>> }
>>>>>>>> ][danielhb at arthas wok_all_plugins]$
>>>>>>>>
>>>>>>>>
>>>>>>>> [danielhb at arthas wok_all_plugins]$ curl -k -u danielhb -H
>>>>>>>> "Content-Type: application/json" -H "Accept: application/json"
>>>>>>>> -X POST 'https://localhost:8001/config/plugins/ginger/disable'
>>>>>>>> -d'{}'
>>>>>>>> Enter host password for user 'danielhb':
>>>>>>>> {
>>>>>>>> "enabled":false,
>>>>>>>> "name":"ginger"
>>>>>>>> }[danielhb at arthas wok_all_plugins]$
>>>>>>>>
>>>>>>>>
>>>>>>>> [danielhb at arthas wok_all_plugins]$ curl -k -u danielhb -H
>>>>>>>> "Content-Type: applicationjson" -H "Accept: application/json"
>>>>>>>> -X POST 'https://localhost:8001/config/plugins/ginger/enable'
>>>>>>>> -d'{}'
>>>>>>>> Enter host password for user 'danielhb':
>>>>>>>> {
>>>>>>>> "enabled":true,
>>>>>>>> "name":"ginger"
>>>>>>>> }[danielhb at arthas wok_all_plugins]$
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> MIME-Version: 1.0
>>>>>>>> Content-Type: text/plain; charset=UTF-8
>>>>>>>> Content-Transfer-Encoding: 8bit
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Daniel Henrique Barboza (10):
>>>>>>>> /config/plugins API: docs changes
>>>>>>>> /config/plugins API: utils changes
>>>>>>>> /config/plugins: changes in config model
>>>>>>>> /config/plugins API: model changes
>>>>>>>> /config/plugins API: test changes
>>>>>>>> Removing old /plugins API
>>>>>>>> /config/plugins: changing existing UI calls
>>>>>>>> /config/plugins: exclude 'sample' plug-in when not in test_mode
>>>>>>>> Removing configobj, use file operations to edit conf files
>>>>>>>> /config/plugins API: user log and parser improvements
>>>>>>>>
>>>>>>>> docs/API/config.md | 31 ++++++++++++++++++++
>>>>>>>> docs/API/plugins.md | 13 ---------
>>>>>>>> src/wok/control/config.py | 33 +++++++++++++++++++--
>>>>>>>> src/wok/control/plugins.py | 29 -------------------
>>>>>>>> src/wok/i18n.py | 4 +++
>>>>>>>> src/wok/model/plugins.py | 32 +++++++++++++++------
>>>>>>>> src/wok/server.py | 7 ++++-
>>>>>>>> src/wok/utils.py | 65
>>>>>>>> ++++++++++++++++++++++++++++++++++++-----
>>>>>>>> tests/test_api.py | 47 +++++++++++++++++++++++++++++-
>>>>>>>> tests/test_utils.py | 72
>>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++-
>>>>>>>> ui/js/src/wok.api.js | 4 +--
>>>>>>>> ui/js/src/wok.logos.js | 11 ++++---
>>>>>>>> ui/js/src/wok.main.js | 12 +++++---
>>>>>>>> 13 files changed, 288 insertions(+), 72 deletions(-)
>>>>>>>> delete mode 100644 docs/API/plugins.md
>>>>>>>> delete mode 100644 src/wok/control/plugins.py
>>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Kimchi-devel mailing list
>>>>>>> Kimchi-devel at ovirt.org
>>>>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Kimchi-devel mailing list
>>>>>> Kimchi-devel at ovirt.org
>>>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>>>
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> Kimchi-devel mailing list
>>> Kimchi-devel at ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>
>>
>
More information about the Kimchi-devel
mailing list