[SSSD] [PATCHES] Enable sending DEBUG logs to journald

Michal Židek mzidek at redhat.com
Wed Jul 9 13:14:31 UTC 2014


On 07/09/2014 11:43 AM, Jakub Hrozek wrote:
> On Tue, Jul 08, 2014 at 09:53:50AM +0200, Sumit Bose wrote:
>> On Mon, Jul 07, 2014 at 10:20:21PM +0200, Jakub Hrozek wrote:
>>> On Mon, Jul 07, 2014 at 03:57:28PM +0200, Sumit Bose wrote:
>>>> On Mon, Jul 07, 2014 at 03:21:54PM +0200, Jakub Hrozek wrote:
>>>>> On Tue, Jun 03, 2014 at 09:53:13AM +0200, Jakub Hrozek wrote:
>>>>>> On Mon, Feb 24, 2014 at 08:39:25AM -0500, Simo Sorce wrote:
>>>>>>> On Mon, 2014-02-24 at 11:54 +0100, Jakub Hrozek wrote:
>>>>>>>> On Thu, Feb 20, 2014 at 08:47:30AM -0500, Stephen Gallagher wrote:
>>>>>>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>>>>>>> Hash: SHA1
>>>>>>>>>
>>>>>>>>> On 02/19/2014 05:18 PM, Lukas Slebodnik wrote:
>>>>>>>>>> On (19/02/14 22:03), Sumit Bose wrote:
>>>>>>>>>>> On Wed, Feb 19, 2014 at 09:04:48PM +0100, Lukas Slebodnik wrote:
>>>>>>>>>>>> On (19/02/14 12:37), Jakub Hrozek wrote:
>>>>>>>>>>>>> On Wed, Feb 19, 2014 at 12:02:23PM +0100, Jakub Hrozek
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>> I realized I made a mistake in the rebasing above. I
>>>>>>>>>>>>>>> fixed that and then needed to do another manual rebase of
>>>>>>>>>>>>>>> these patches atop it. These patches apply atop the
>>>>>>>>>>>>>>> "[SSSD] DEBUG macro refactoring v6" patches.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> These patches work for me, so ACK.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Pushed to master.
>>>>>>>>>>>>
>>>>>>>>>>>> find_uid-tests failed with these patches on fedora20.
>>>>>>>>>>>> http://kojipkgs.fedoraproject.org//work/tasks/8367/6548367/build.log
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>> Do you have additional patches in this build?
>>>>>>>>>>>
>>>>>>>>>>> /builddir/build/SRPMS/sssd-1.11.90-0.20140219.1816.git22a9323._temp.fc20.src.rpm
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>> The git hash does not look like anything from master.
>>>>>>>>>>>
>>>>>>>>>> Yes, I was testing some patches.
>>>>>>>>>>
>>>>>>>>>>> I started a koji build
>>>>>>>>>>> http://koji.fedoraproject.org/koji/taskinfo?taskID=6549081 with
>>>>>>>>>>> current master (4cde267bec52ae1723a125d19439a5c75b47ebb7) which
>>>>>>>>>>> is working fine.
>>>>>>>>>>>
>>>>>>>>>> I tried one more time with master and koji works fine.
>>>>>>>>>> Unfortunately, I am able to reproduce it locally in mock :-(
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I've seen that happen intermittently as well in mock and koji. I
>>>>>>>>> suspect there may be a bug/race condition in sd_uid_get_sessions(),
>>>>>>>>> but I can't reproduce it consistently (and the DEBUG error doesn't
>>>>>>>>> seem to fire...)
>>>>>>>>>
>>>>>>>>> A quick look at the code suggests that we're probably missing a
>>>>>>>>> 'return EOK' after '*result = false', though.
>>>>>>>>>
>>>>>>>>> I'll run a couple tests and see if that fixes the issue...
>>>>>>>>
>>>>>>>> I found another issue related to journald support..if you compile sssd
>>>>>>>> with journald support and run it on the foreground (-i) then the debug
>>>>>>>> messages are still redirected to journal. That seems strange to me, but
>>>>>>>> also in line with what a comment in the code says:
>>>>>>>>
>>>>>>>>           /* If we are not outputting logs to files, we should be sending them
>>>>>>>>            * to journald.
>>>>>>>>            * NOTE: on modern systems, this is where
>>>>>>>>            * stdout/stderr will end up
>>>>>>>>
>>>>>>>> So I'm not entirely sure about the expected behaviour, but I would
>>>>>>>> prefer if there was still a way to see the debug messages directly on
>>>>>>>> the console.
>>>>>>>>
>>>>>>>> I hacked up a patch that also adds a third state where if sssd is running
>>>>>>>> on the foreground even with journald support, then just fprintf is used.
>>>>>>>> See the attachment.
>>>>>>>
>>>>>>> Wouldn't it be simpler to just set debug_file = stderr ?
>>>>>>
>>>>>> I tried to go this way, but it seems actually harder to me..
>>>>>>
>>>>>> To summarize:
>>>>>> with "sssd -f" debug_file should be a log file
>>>>>> with "sssd" debug_file should be NULL, indicating journald
>>>>>> with "sssd -i", debug_file should be stderr, indicating stderr.
>>>>>>
>>>>>> The problem is, that we don't pass the "-i" flag down to the responders
>>>>>> and back ends when forking them. So the alternative would be to add "-i"
>>>>>> to sssd_be and all responders, pass the flag to the child processes and
>>>>>> only set debug_file to stderr if '-i' is present. If you prefer this way,
>>>>>> I can prepare a patch along these lines.
>>>>>>
>>>>>> But then we also terminate the process which is running with "-i" if its
>>>>>> stdin goes away, I guess it would be safe to keep this logic since stdin
>>>>>> in the child processes is /dev/null already, I just wanted to mention
>>>>>> there is a number of cases to test..
>>>>>
>>>>> Any opinions? I'd like to fix this bug before tagging 1.12.0..
>>>>
>>>> Maybe having an option --debug-to-stdout would be nicer for the child
>>>> processes because it fits better to the existing SSSD_DEBUG_OPTS and
>>>> will have no additional meaning as -i? For the monitor this option
>>>> should be rejected and hidden from the help.
>>>>
>>>> bye,
>>>> Sumit
>>>
>>> Perhaps..
>>>
>>> to clarify, you're suggesting that --debug-to-stdout would be added to
>>> all processes (monitor and child), but only child processes in libexec
>>> would expose it and this option would be used when spawning the child
>>> processes?
>>
>> yes, my idea was to add it to SSSD_DEBUG_OPTS which would make it
>> immediately available to all processes. But having it for the monitor
>> make no sense because '-D --debug-to-stdout' and '-f --debug-to-stdout'
>> are config errors and '-i --debug-to-stdout' is a no-op.
>>
>> bye,
>> Sumit
>
> Patches are attached. I found out the same issue also affected the CLI
> tools and tests, which is fixed with the second patch.
>
> In order to test, you need to run configure --with-initscript=systemd
> --with-systemdunitdir=/usr/lib/systemd/system --with-syslog=journald
>
> sssd -i -d <N>  logs to stderr
> sssd -i -f -d <N> logs to files
> sssd -D -d <N> logs to journald
> sssd -D -f -d <N> logs to files
>
> It's not possible to log to journald when running on the foreground or
> to stderr when running as deamon.
>

I think the behaviour with these patches makes sense and
works as expected. Ack.

Michal



More information about the sssd-devel mailing list