Bug 46433 - Provide wrapper for docker logs with univention-app
Provide wrapper for docker logs with univention-app
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: App Center
UCS 4.2
Other Linux
: P5 enhancement (vote)
: UCS 4.3-1-errata
Assigned To: Richard Ulmer
Dirk Wiesenthal
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-02-27 11:40 CET by Valentin Heidelberger
Modified: 2018-08-29 12:49 CEST (History)
4 users (show)

See Also:
What kind of report is it?: Feature Request
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):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Valentin Heidelberger univentionstaff 2018-02-27 11:40:51 CET
univention-app provides several wrappers for docker commands. One that is currently not supported but used relatively often is the docker logs command.
"univention-app logs" would be a nice enhancement.
Comment 1 Valentin Heidelberger univentionstaff 2018-02-27 12:03:07 CET
Opened a pull request on GitHub to add this feature: https://github.com/univention/univention-corporate-server/pull/3
Comment 2 Dirk Wiesenthal univentionstaff 2018-02-27 12:21:46 CET
Hmmm... where do I comment? Here or at the PR?

Anyway, thanks for the patch.

One would need to add a code path for Non-Docker Apps, though. At least say that it is not supported.

Maybe this makes sense for Non-Docker Apps, too? Like the samba.log for samba4? Kopano also writes logs. But this would have to be mentioned in the ini file...
Comment 3 Valentin Heidelberger univentionstaff 2018-02-27 13:19:14 CET
(In reply to Dirk Wiesenthal from comment #2)
> Hmmm... where do I comment? Here or at the PR?
> 
> Anyway, thanks for the patch.
> 
> One would need to add a code path for Non-Docker Apps, though. At least say
> that it is not supported.
> 
> Maybe this makes sense for Non-Docker Apps, too? Like the samba.log for
> samba4? Kopano also writes logs. But this would have to be mentioned in the
> ini file...

Maybe journalctl is sufficient for this. I'd need to check that for each non-docker app though.
Samba for instance could be checked with 'journalctl -u samba.service'
journalctl also supports the different parameters.
Comment 4 Valentin Heidelberger univentionstaff 2018-02-27 13:32:08 CET
(In reply to Dirk Wiesenthal from comment #2)
> Hmmm... where do I comment? Here or at the PR?
> 
> Anyway, thanks for the patch.
> 
> One would need to add a code path for Non-Docker Apps, though. At least say
> that it is not supported.
> 
> Maybe this makes sense for Non-Docker Apps, too? Like the samba.log for
> samba4? Kopano also writes logs. But this would have to be mentioned in the
> ini file...

Is there a list of non-docker apps?

journalctl won't work for every app though. The Let's Encrypt app for example just uses a normal log file and has no service unit.
Comment 5 Erik Damrose univentionstaff 2018-02-27 13:34:09 CET
I think there is no simple solution for non docker apps. E.g. Kopano services write to several log files
Comment 6 Dirk Wiesenthal univentionstaff 2018-02-27 13:45:19 CET
Yep, thats what I meant. Either you extend the ini file for Non-Docker Apps 

LogFiles=/var/log/univention/letsencrypt.log,...
LogJournal=samba.service

Or you just ignore them. This can be done by:
if not app.docker:
  self.fatal('...')
  return False

(or something)
Comment 7 Valentin Heidelberger univentionstaff 2018-02-27 13:58:51 CET
(In reply to Dirk Wiesenthal from comment #6)
> Yep, thats what I meant. Either you extend the ini file for Non-Docker Apps 
> 
> LogFiles=/var/log/univention/letsencrypt.log,...
> LogJournal=samba.service
> 
> Or you just ignore them. This can be done by:
> if not app.docker:
>   self.fatal('...')
>   return False
> 
> (or something)

Added a check that raises a ShellAppNotRunning exception: https://github.com/univention/univention-corporate-server/pull/3

This looks as follows:
univention-app logs letsencrypt                                            
Cannot run command: letsencrypt=1.1.2-3 is not running in a container
Comment 8 Richard Ulmer univentionstaff 2018-08-27 12:33:49 CEST
The changes in the pull request work as expected, so I applied them. I additionally changed the indentation to tabs and removed an extraneous blank line.

git commits (sorted from most recent to oldest):
21f02fbd32 Bug #46433: Update yaml file
730b82d3e9 Bug #46433: Merge branch 'rulmer/46433' into 4.3-1
37328c118c Bug #46433: Add changelog entry
5e5fd490bc Bug #46433: Remove second blank line between methods
e0a9b7501c Bug #46433: Only use tabs for indentation
04c3717856 Bug #46433: Add code path for non-docker apps
43cc9bb506 Bug #46433: Provide wrapper for docker logs with univention-app
Comment 9 Dirk Wiesenthal univentionstaff 2018-08-29 03:07:01 CEST
As discussed, please remove the following arguments as they are too closely tied to docker logs.

Also make sure the App is actually installed, otherwise you get
Traceback (most recent call last):
  File "/usr/bin/univention-app", line 91, in <module>
    main()
  File "/usr/bin/univention-app", line 78, in main
    ret = args.func(args)
  File "/usr/lib/pymodules/python2.7/univention/appcenter/actions/__init__.py", line 226, in call_with_namespace
    result = self.main(namespace)
  File "/usr/lib/pymodules/python2.7/univention/appcenter/actions/logs.py", line 71, in main
    return subprocess.call(docker_logs + [docker.container])
  File "/usr/lib/python2.7/subprocess.py", line 168, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/usr/lib/python2.7/subprocess.py", line 390, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1024, in _execute_child
    raise child_exception
TypeError: execv() arg 2 must contain only strings

This happens if
  docker = self._get_docker(args.app)
  docker.container is None
Comment 10 Richard Ulmer univentionstaff 2018-08-29 10:06:15 CEST
I have stripped away the 'docker logs' related parameters and changed the log messages.

git commits (sorted from most recent to oldest):
c5806ec6ac Bug #46433: Update yaml file
39e2c6ef94 Bug #46433: Add changelog entry
0760c40ca6 Bug #46433: Simplify the 'univention-app logs' command
Comment 11 Dirk Wiesenthal univentionstaff 2018-08-29 10:36:54 CEST
Ok, works
Comment 12 Arvid Requate univentionstaff 2018-08-29 12:49:40 CEST
<http://errata.software-univention.de/ucs/4.3/221.html>