[Kimchi-devel] [PATCH v2] [WoK 00/10] '/config/plugins' API implementation
Daniel Henrique Barboza
danielhb at linux.vnet.ibm.com
Thu Jan 26 12:10:40 UTC 2017
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?
I think it's ok. I'll get it done in v3.
>
> 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