On 06/19/2014 02:31 PM, Alexander Bokovoy wrote: > On Wed, 18 Jun 2014, Nathaniel McCallum wrote: >> On Wed, 2014-06-04 at 18:47 +0300, Alexander Bokovoy wrote: >>> On Thu, 01 May 2014, Nathaniel McCallum wrote: >>> >On Tue, 2014-03-11 at 11:09 -0400, Simo Sorce wrote: >>> >> On Tue, 2014-03-11 at 16:05 +0200, Alexander Bokovoy wrote: >>> >> > On Tue, 11 Mar 2014, Jan Pazdziora wrote: >>> >> > >On Mon, Feb 24, 2014 at 02:26:27PM -0500, Nathaniel McCallum wrote: >>> >> > >> Before this patch, ipa-kdb would load global configuration on >>> >> > >> startup >>> >> > >> and never update it. This means that if global configuration is >>> changed, >>> >> > >> the KDC never receives the new configuration until it is restarted. >>> >> > >> >>> >> > >> This patch enables caching of the global configuration with a >>> timeout of >>> >> > >> 60 seconds. >>> >> > >> >>> >> > >> https://fedorahosted.org/freeipa/ticket/4153 >>> >> > > >>> >> > >> >From 7daeae56671d7b3049b0341aad66c96877431bbe Mon Sep 17 00:00:00 >>> >> > >> >2001 >>> >> > >> From: Nathaniel McCallum <npmccal...@redhat.com> >>> >> > >> Date: Mon, 24 Feb 2014 14:19:13 -0500 >>> >> > >> Subject: [PATCH] Periodically refresh global ipa-kdb configuration >>> >> > >> >>> >> > >> Before this patch, ipa-kdb would load global configuration on >>> startup and >>> >> > >> never update it. This means that if global configuration is changed, >>> the >>> >> > >> KDC never receives the new configuration until it is restarted. >>> >> > >> >>> >> > >> This patch enables caching of the global configuration with a >>> timeout of >>> >> > >> 60 seconds. >>> >> > >> >>> >> > >> https://fedorahosted.org/freeipa/ticket/4153 >>> >> > > >>> >> > >I have only read the code and it looks sane, so depending on how much >>> >> > >you consider my word about code-reading worth, ack. >>> >> > > >>> >> > >However, my gut feeling is that my preferred way of handling the issue >>> >> > >(without knowing much about the background of the ticket) would >>> >> > >probably be a HUP signal handler or something similar, rather than >>> >> > >polling for current values with the value timeout. This patch >>> >> > >introduces small nondeterminism to the behaviour when the usage of the >>> >> > >new values cannot really be enfoced by the admin (without the daemon >>> >> > >restart). >>> >> > Thing is, we need the update to happen when other, non-root process >>> >> > makes the changes -- in our case when IPA server running under httpd >>> >> > privileges performs series of MS-RPC calls towards smbd which lead to >>> >> > creating certain LDAP objects. httpd couldn't send SIGHUP to a process >>> >> > not owned by httpd's effective user (non-root). >>> >> >>> >> Yes but this is not really the way to go. >>> >> >>> >> The proper fix is to use syncrepl/persistent search to get a >>> >> notification of when we need to reload the configuration. >>> >> >>> >> On the patch itself I have a NACK due to this syntax used in various >>> >> places: function()->attribute >>> >> >>> >> don't. do. that. ever. >>> >> >>> >> assign whatever come from the function to a local variable and *check* >>> >> it is not null, *then* use it. >>> > >>> >Attached patch fixes the NACK issue, but does not address the question >>> >of the larger approach. Do we need to take a different approach? If so, >>> >what is it? >>> >>> I've tested the patch but was unable to spot any activity to refresh the >>> configuration after I've updated it with >>> ipa config-mod --ipaconfigstring=KDC:Disable\ Lockout >>> >>> At least, dirsrv logs didn't show me any attempt to re-read IPA config >>> past the change. >> >> I spent extensive time testing this today. I am unable to reproduce a >> failure to reload data from LDAP. If I make the change using "ipa >> config-mod", wait a minute and run a kinit the KDC always gets the new >> values. So it works for me. >> >> However, I did uncover a subtle bug with regard to ipaconfigstring which >> would cause it to never to be unset once set. Perhaps you were running >> into this? > Yes, that seems a likely cause. At least, I cannot reproduce it again. > >> >> The new attached version fixes this bug. Everything appears to work... > Thanks, ACK.
Pushed to master. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
