Re: [Kimchi-devel] [PATCH v2] [WoK 00/10] '/config/plugins' API implementation

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(a)gmail.com wrote: >>>>>>> From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; >>>>>>> >>>>>>> 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@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@arthas wok_all_plugins]$ >>>>>>> >>>>>>> >>>>>>> [danielhb@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@arthas wok_all_plugins]$ >>>>>>> >>>>>>> >>>>>>> [danielhb@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@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(a)ovirt.org >>>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >>>>>> >>>>> >>>>> _______________________________________________ >>>>> Kimchi-devel mailing list >>>>> Kimchi-devel(a)ovirt.org >>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >>>>> >>>> >>> >> >> _______________________________________________ >> Kimchi-devel mailing list >> Kimchi-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >> >