Bug 34243 - Not-restarted UMC raises ImportError when opening modules of uninstalled apps
Not-restarted UMC raises ImportError when opening modules of uninstalled apps
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC (Generic)
UCS 3.2
Other Linux
: P5 normal (vote)
: UCS 3.2-2-errata
Assigned To: Florian Best
Dirk Wiesenthal
:
: 33202 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-03-04 13:15 CET by Florian Best
Modified: 2014-12-05 11:31 CET (History)
6 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): External feedback
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2014-03-04 13:15:46 CET
We received the following traceback:
Traceback:
Failed to import module agorum: No module named agorum
Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.6/univention/management/console/protocol/modserver.py",
line 93, in _load_module
    self.__module = __import__(file_, [], [], modname)
ImportError: No module named agorum

I could reproduce this by
1. uninstalling agorum-app
2. not restarting the UMC-server
3. opening the still visible agorum module

I think we can set this as WONTFIX because the user is warned and prompted to restart the UMC-Server after successful app removal.

Or is here something to do in the view of usability?
Comment 1 Florian Best univentionstaff 2014-03-04 13:47:05 CET
We received also another traceback which was caused by ignoring the UMC-Server-Restart-Hint.

Traceback:
The init function of the module has failed: LDAP_ConnectionError: Opening LDAP connection
failed: {'desc': "Can't contact LDAP server"}
Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.6/univention/management/console/protocol/modserver.py",
line 228, in handle
    self.__handler.init()
  File
"/usr/lib/pymodules/python2.6/univention/management/console/modules/udm/__init__.py", line
85, in init
    self.settings = UDM_Settings()
  File
"/usr/lib/pymodules/python2.6/univention/management/console/modules/udm/udm_ldap.py", line
842, in __init__
    self.read()
  File
"/usr/lib/pymodules/python2.6/univention/management/console/modules/udm/udm_ldap.py", line
845, in read
    self._read_directories()
  File
"/usr/lib/pymodules/python2.6/univention/management/console/modules/udm/udm_ldap.py", line
150, in wrapper_func
    raise LDAP_ConnectionError( 'Opening LDAP connection failed: %s' % str( e ) )
LDAP_ConnectionError: Opening LDAP connection failed: {'desc': "Can't contact LDAP
server"}

Remark:
kurzt vorher das Root-Zertifikat aktualisiert (E-Mail, Bundesland, Ort)

We should think about revising the concept.
Some possibilities:
* force restart of UMC with browser reload
* pop up the message every minute and warn about possibly failures

The most stable possibility would be:
* store the credentials in the frontend
* restart the UMC server in background
* relogin
* set language
* rerender the module-overview
→ this destroys server state (the state is most often not needed within the modules).
Comment 2 Florian Best univentionstaff 2014-03-04 15:04:39 CET
(In reply to Florian Best from comment #1)
> We received also another traceback which was caused by ignoring the
> UMC-Server-Restart-Hint.
→ sorry, I meant: which could be caused by
This was the way I reproduced it.
Comment 3 Alexander Kläser univentionstaff 2014-03-05 16:23:14 CET
(In reply to Florian Best from comment #0)
> ...
> I think we can set this as WONTFIX because the user is warned and prompted
> to restart the UMC-Server after successful app removal.
> 
> Or is here something to do in the view of usability?

Currently, we offer a restart after any operation that _could_ require a restart. It would be better to detect when a restart really is _necessary_ (e.g. via a UCR-Variable that is set) and then enforce this restart (in some way).
Comment 4 Alexander Kläser univentionstaff 2014-03-05 16:23:57 CET
See also Bug 28081.
Comment 5 Alexander Kläser univentionstaff 2014-03-05 16:37:40 CET
(In reply to Alexander Kläser from comment #3)
> Currently, we offer a restart after any operation that _could_ require a
> restart. It would be better to detect when a restart really is _necessary_
> (e.g. via a UCR-Variable that is set) and then enforce this restart (in some
> way).

Setting a UCR variable has the advantage that consequent errors/tracebacks could be commented with a message like "Please restart the UMC server and see if the problem persists".
Comment 6 Dirk Wiesenthal univentionstaff 2014-03-17 10:28:36 CET
For the agorum problem: See Bug#33202
Comment 7 Alexander Kläser univentionstaff 2014-03-19 08:38:34 CET
A more feasible approach would be: catch the import error and send in the JSON error answer a flag that indicates that a UMC restart is recommended. Along with the error message UMC can then offer to the user to restart the UMC server.
Comment 8 Florian Best univentionstaff 2014-03-19 08:51:35 CET
(In reply to Alexander Kläser from comment #7)
> A more feasible approach would be: catch the import error and send in the
> JSON error answer a flag that indicates that a UMC restart is recommended.
> Along with the error message UMC can then offer to the user to restart the
> UMC server.
Another idea: A UMC-Server restart is not necessary, a reload is enough. This can be triggered on ImportError.?!
Comment 9 Florian Best univentionstaff 2014-05-30 14:20:33 CEST
(In reply to Florian Best from comment #1)
> We should think about revising the concept.
> Some possibilities:
> * force restart of UMC with browser reload
This is not a so good idea because maybe in the future we don't need to restart UMC anymore. It also requires user interaction.

> * pop up the message every minute and warn about possibly failures
This would probably disturbing for the user. We already have such dialog which e.g. pops up for every new opened module (instead of a time delay).

> The most stable possibility would be:
> * store the credentials in the frontend
> * restart the UMC server in background
> * relogin
> * set language
> * rerender the module-overview
> → this destroys server state (the state is most often not needed within the
> modules).
This change is currently not worth it because it's too complex.

(In reply to Alexander Kläser from comment #7)
> A more feasible approach would be: catch the import error and send in the
> JSON error answer a flag that indicates that a UMC restart is recommended.
> Along with the error message UMC can then offer to the user to restart the
> UMC server.
A traceback / error message would still show up in that case. The user is already prompted to restart the UMC, and will be again on any operation like opening a module (because frontend-js-hash changed).

(In reply to Florian Best from comment #8)
> Another idea: A UMC-Server restart is not necessary, a reload is enough.
> This can be triggered on ImportError.?!
Triggering the reload on ImportError is too late, so the error message that the module doesn't exists would not show up.

A UMC-Server restart isn't very often required afaik. We have the following cases:
* A change in the list of modules (relogin (new session) required)
* A new license was imported (→ currently the UDM moduleprocess have to die)
* A new SSL certificate was created (→ Apache needs a restart)
* new/changed extended attributes (don't know why, UDM module process have to die)
* change of the own password (new password is not passed to the modules)
* change of UMC-policies or UMC ACL's (?)
any more?

IMHO we should fix those points instead of workarounding by making a UMC-Server restart necessary.
The fix for this bug addresses the first point:

A function umc.app.reloadModules() has been implemented which reloads the module metainformation from the server. These information are reloaded before the modules/list command is executed.
(which is literally Bug #33202).

univention-management-console 6.0.24-4.777.201405301355
univention-management-console-frontend 3.0.152-27.826.201405301357

This function is used after installing/uninstalling/… a App in the AppCenter.

univention-management-console-module-appcenter 3.0.51-15.249.201405301403

2014-05-26-univention-management-console-frontend.yaml
2014-05-30-univention-management-console-module-appcenter.yaml
2014-05-30-univention-management-console.yaml
Comment 10 Dirk Wiesenthal univentionstaff 2014-06-05 16:02:32 CEST
I am able to crash the UMC server by logging in with FF and Chrome, uninstalling pkgdb in FF and opening it in Chrome

05.06.14 16:08:47.754  MAIN        ( ERROR   ) : Traceback (most recent call last):
  File "/usr/sbin/univention-management-console-server", line 209, in <module>
    umc_daemon.do_action()
  File "/usr/lib/pymodules/python2.6/daemon/runner.py", line 186, in do_action
    func(self)
  File "/usr/sbin/univention-management-console-server", line 142, in _restart
    self._start()
  File "/usr/lib/pymodules/python2.6/daemon/runner.py", line 131, in _start
    self.app.run()
  File "/usr/sbin/univention-management-console-server", line 192, in run
    notifier.loop()
  File "/usr/lib/pymodules/python2.6/notifier/nf_generic.py", line 284, in loop
    step()
  File "/usr/lib/pymodules/python2.6/notifier/nf_generic.py", line 271, in step
    not __sockets[ cond ][ fd ]( sock_obj ):
  File "/usr/lib/pymodules/python2.6/univention/management/console/protocol/server.py", line 165, in _receive
    self._handle( state, msg ) 
  File "/usr/lib/pymodules/python2.6/univention/management/console/protocol/server.py", line 282, in _handle
    state.processor.request( msg ) 
  File "/usr/lib/pymodules/python2.6/univention/management/console/protocol/session.py", line 276, in request
    self.handle_request_command( msg ) 
  File "/usr/lib/pymodules/python2.6/univention/management/console/protocol/session.py", line 667, in handle_request_command
    mod_proc = ModuleProcess( module_name, debug = MODULE_DEBUG_LEVEL, locale = self.i18n.locale )
  File "/usr/lib/pymodules/python2.6/univention/management/console/protocol/session.py", line 138, in __init__
    modxmllist = moduleManager[ module ]
KeyError: 'pkgdb'
Comment 11 Dirk Wiesenthal univentionstaff 2014-06-05 16:14:14 CEST
I am not so sure how I like the patch. You do not really catch the error but hide the overview page.

This may improve the usability, but there are too many issues left that should all be addressed.

 * Overview page is removed and then added again. Strange, if you are scrolling the overview at this moment. Can you operate on the store instead?
 * New Installed Apps do not show up in "Favorites"
 * There is a HEAD request before each module opening. This prevents using UMC after installing an application, although all modules are up to date now
 * Do we really need to restart the UMC now that the modules are all up to date? Maybe we can stop requesting a restart after App installation/uninstallation?
 * We need the reloadModules also after "normal" package management (also done in App Center) and possibly Updater (but this may be dangerous during release update)

If we can get rid of the UMC restart in most cases, we still need to track a forced restart during package_manager.no_umc_restart(). See Bug#28081
Comment 12 Florian Best univentionstaff 2014-06-11 14:20:38 CEST
(In reply to Dirk Wiesenthal from comment #10)
> I am able to crash the UMC server by logging in with FF and Chrome,
> uninstalling pkgdb in FF and opening it in Chrome
> 
> 05.06.14 16:08:47.754  MAIN        ( ERROR   ) : Traceback (most recent call
> last):
> "/usr/lib/pymodules/python2.6/univention/management/console/protocol/session.
> py", line 138, in __init__
>     modxmllist = moduleManager[ module ]
> KeyError: 'pkgdb'

This error isn't new, it exists since the beginning of UMC.
The error was that 
1. the XML definition of a module has been removed (e.g. uninstallation)
2. moduleManager.load() (e.g. someone new logged in | "invoke-rc.d univention-management-console-server reload")
3. the ACL's/permitted commands weren't reloaded, so access to the module is still allowed.
→ a active session opened the not-anymore-existing-module caused the UMC server crash

It has been fixed in SVN 50989.
Comment 13 Florian Best univentionstaff 2014-06-11 15:05:28 CEST
sorry, not RESOLVED, the other comments are left ;)
Comment 14 Florian Best univentionstaff 2014-06-11 17:15:44 CEST
(In reply to Dirk Wiesenthal from comment #11)
> I am not so sure how I like the patch. You do not really catch the error but
> hide the overview page.
yep, that was untidy. Changed ;)

>  * Overview page is removed and then added again. Strange, if you are
> scrolling the overview at this moment. Can you operate on the store instead?
Done

>  * New Installed Apps do not show up in "Favorites"
Fixed

>  * There is a HEAD request before each module opening. This prevents using
> UMC after installing an application, although all modules are up to date now
The umc_frontend_new_hash and invoke umc-server reload is not necessary anymore due to the changes. They have been removed from dh-umc-module-install. When packages with UMC modules are rebuild the dialogue doesn't occur anymore then.
umc_frontend_new_hash is of course triggered on release updates in the UMC postinst.
Comment 15 Florian Best univentionstaff 2014-06-12 11:27:23 CEST
>  * Do we really need to restart the UMC now that the modules are all up to
> date? Maybe we can stop requesting a restart after App
> installation/uninstallation?
A restart is only required in the cases listed above (Comment 9) plus also we cannot be sure that the appcenter may update the UMC server. So the restart dialog keeps existing.

>  * We need the reloadModules also after "normal" package management (also
> done in App Center) and possibly Updater (but this may be dangerous during
> release update)
Yes, the reloading of modules is now done in lib.server.askRestart() if the user cancels the dialog.

> If we can get rid of the UMC restart in most cases, we still need to track a
> forced restart during package_manager.no_umc_restart(). See Bug#28081
→ keeps being Bug#28081, the restart dialog still exists.

Package: univention-management-console-module-lib
Version: 3.0.7-2.23.201406121114

Package: univention-management-console-frontend
Version: 3.0.152-29.828.201406121117

Package: univention-management-console-module-appcenter
Version: 3.0.51-19.253.201406121123
Comment 16 Dirk Wiesenthal univentionstaff 2014-06-12 16:11:33 CEST
No Apps installed => No "Installed Apps" category.

Install App, cancel dialog: "All" and "Favorites" are empty, "Installed Apps" is not present.
Comment 17 Florian Best univentionstaff 2014-06-13 09:32:59 CEST
(In reply to Dirk Wiesenthal from comment #16)
> No Apps installed => No "Installed Apps" category.
> 
> Install App, cancel dialog: "All" and "Favorites" are empty, "Installed
> Apps" is not present.
oh yes, the grid didn't have the new categories which caused this.
Comment 18 Alexander Kläser univentionstaff 2014-06-13 15:30:40 CEST
(In reply to Florian Best from comment #14)
> ...
> >  * There is a HEAD request before each module opening. This prevents using
> > UMC after installing an application, although all modules are up to date now
> The umc_frontend_new_hash and invoke umc-server reload is not necessary
> anymore due to the changes. They have been removed from
> dh-umc-module-install. When packages with UMC modules are rebuild the
> dialogue doesn't occur anymore then.
> umc_frontend_new_hash is of course triggered on release updates in the UMC
> postinst.

removing umc_frontend_new_has from the postinst will mean that images (e.g., icons) that are updated with a new module version will probably not be updated in the browser itself due to caching problems? Not sure, but I fear that might be a problem. Could you please check?
Comment 19 Florian Best univentionstaff 2014-06-13 17:31:56 CEST
(In reply to Alexander Kläser from comment #18)
> removing umc_frontend_new_has from the postinst will mean that images (e.g.,
> icons) that are updated with a new module version will probably not be
> updated in the browser itself due to caching problems? Not sure, but I fear
> that might be a problem. Could you please check?
yes, you are right. In IE8-11 the icons are cached.
I reverted the removal of umc_frontend_new_hash.

Also it has to be checked what happens in large environments, if the modules/list request takes too long due to the reload of ACL's.
Comment 20 Alexander Kläser univentionstaff 2014-06-13 17:53:57 CEST
(In reply to Florian Best from comment #19)
> yes, you are right. In IE8-11 the icons are cached.
> I reverted the removal of umc_frontend_new_hash.

The same caching problem might apply to .js files. Instead, the handling of the reload hint (when the HEAD request failed) could be adapted (e.g., only prompt it once per session).
Comment 21 Florian Best univentionstaff 2014-07-03 15:35:33 CEST
The reload of ACL's now only happens to existing sessions, not after a login because there isn't a session open so the ACL's are fresh.

The 'invoke-rc.d umc reload' has been reintegrated → It's probably not required anymore but at least it doesn't matter if it is in there.

The reload-UMC hint is now a singleton.

Package: univention-management-console-frontend
Version: 3.0.152-36.835.201407031523
Package: univention-management-console
Version: 6.0.24-15.790.201407031531
Comment 22 Alexander Kläser univentionstaff 2014-07-08 17:46:30 CEST
Error introduced by revision 51504:

Index: umc/tools.js
===================================================================
--- umc/tools.js        (Revision 51503)
+++ umc/tools.js        (Revision 51504)
@@ -207,6 +207,8 @@
                                                }
                                        }]
                                });
+                       } else {
+                               return; // show this dialog only once per session
                        }
                        if (!this._reloadDialog.open) {
                                // check if UMC needs a browser reload and prompt the user to reload

This patch has as consequence that no hint is displayed after calling umc_frontend_new_hash on the server side.
Comment 23 Florian Best univentionstaff 2014-07-09 09:15:45 CEST
(In reply to Alexander Kläser from comment #22)
> Error introduced by revision 51504:
> 
> This patch has as consequence that no hint is displayed after calling
> umc_frontend_new_hash on the server side.
yes, fixed in univention-management-console-frontend 3.0.152-38.837.20140709084
Comment 24 Dirk Wiesenthal univentionstaff 2014-07-09 17:32:03 CEST
Okay, looks fine. But the actual problem was not resolved, was it? This is still possible in a different session.

Failed to import module pkgdb: No module named pkgdb
Traceback (most recent call last):
  File "/usr/lib/pymodules/python2.6/univention/management/console/protocol/modserver.py", line 94, in _load_module
    self.__module = __import__(file_, [], [], modname)
ImportError: No module named pkgdb
Comment 25 Florian Best univentionstaff 2014-07-10 11:31:46 CEST
(In reply to Dirk Wiesenthal from comment #24)
> Okay, looks fine. But the actual problem was not resolved, was it? This is
> still possible in a different session.
yes, in a different session the error is still possible. It is very unlikely to occur. Nevertheless i am able to catch the import error, too and check if the module doesn't exists to represent a human readable error message.

The trade off here is that the error message can not be translated.
Comment 26 Florian Best univentionstaff 2014-07-10 12:01:12 CEST
Implemented your wish! The error message is currently:
> The requested module "modulid" does not exists.
> This could be caused by a current removal of the module.
> Please relogin to the Univention Management Console to see if the error persists.

Happy with that string?
Comment 27 Florian Best univentionstaff 2014-07-10 12:06:40 CEST
Package: univention-management-console
Version: 6.0.24-17.792.201407101206
Comment 28 Florian Best univentionstaff 2014-07-10 14:28:38 CEST
Well I fixed also the translation of strings before the SET locale requests comes in by changing NullTranslation → Translation and using the locale (not language) from what is given to the process with argument -l because the translation domain is a multiton.
Some missing (unused before) translations have also been added.
Comment 29 Dirk Wiesenthal univentionstaff 2014-07-10 15:13:03 CEST
The last sentence is not translated:

Further information can be found in the logfile /var/log/univention/management-console-module-pkgdb.

The filename is also lacking a ".log".
Comment 30 Dirk Wiesenthal univentionstaff 2014-07-10 15:39:32 CEST
Question: Is it possible that the locale is also set when doing modules/list through lib.askRestart()?

Currently, a newly installed app adds a module in "Installed apps" with the english locale until logged out and logged in again.
Comment 31 Florian Best univentionstaff 2014-07-10 17:18:04 CEST
(In reply to Dirk Wiesenthal from comment #30)
> Question: Is it possible that the locale is also set when doing modules/list
> through lib.askRestart()?
> 
> Currently, a newly installed app adds a module in "Installed apps" with the
> english locale until logged out and logged in again.
oh yes, the translation .mo file was modified while installing an app. The gettext instance is now reloaded.


(In reply to Dirk Wiesenthal from comment #29)
> The last sentence is not translated:
> 
> Further information can be found in the logfile
> /var/log/univention/management-console-module-pkgdb.
> 
> The filename is also lacking a ".log".
fixed.
Comment 32 Dirk Wiesenthal univentionstaff 2014-07-11 15:06:22 CEST
Looks fine.

Big change, but improves UMC's abilities. Minor flaws, but overall modules/list {reload: true} und modules/list {reload: false} can be used one day to not restart UMC completely (except UMC-updates, of course).

Error message okay.

YAML Ok
Comment 33 Florian Best univentionstaff 2014-07-14 10:05:11 CEST
*** Bug 33202 has been marked as a duplicate of this bug. ***
Comment 34 Moritz Muehlenhoff univentionstaff 2014-07-14 10:49:25 CEST
http://errata.univention.de/ucs/3.2/145.html
Comment 35 Moritz Muehlenhoff univentionstaff 2014-07-14 10:50:32 CEST
http://errata.univention.de/ucs/3.2/149.html
Comment 36 Florian Best univentionstaff 2014-07-14 11:00:19 CEST
reset status to VERIFIED until the package univention-management-console is also released as erratum.
Comment 37 Janek Walkenhorst univentionstaff 2014-08-07 17:42:03 CEST
http://errata.univention.de/ucs/3.2/159.html