On 10/30/2012 09:39 PM, Sumit Bose wrote: > On Tue, Oct 30, 2012 at 03:55:04PM +0100, Martin Kosek wrote: >> On 10/30/2012 02:35 PM, Sumit Bose wrote: >>> On Mon, Oct 29, 2012 at 05:11:27PM -0400, Rob Crittenden wrote: >>>> Sumit Bose wrote: >>>>> On Wed, Oct 24, 2012 at 01:07:03PM +0200, Martin Kosek wrote: >>>>>> On 10/24/2012 12:48 PM, Sumit Bose wrote: >>>>>>> On Wed, Oct 24, 2012 at 12:31:57PM +0200, Martin Kosek wrote: >>>>>>>> On 10/24/2012 12:19 PM, Sumit Bose wrote: >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> this patches fixes https://fedorahosted.org/freeipa/ticket/3185 by >>>>>>>>> restarting httpd as one of the last steps of ipa-adtrust-install. >>>>>>>>> >>>>>>>>> bye, >>>>>>>>> Sumit >>>>>>>>> >>>>>>>> >>>>>>>> This patch is targeted to pick up trust plugins (adtrustinstance, >>>>>>>> dcerpc) >>>>>>>> installed during freeipa-server-trust-ad RPM install? I am still not >>>>>>>> sure if we >>>>>>>> should not rather reload httpd server during freeipa-server update >>>>>>>> %post, >>>>>>>> because this way, httpd will be restarted every time that someone runs >>>>>>>> ipa-adtrust-install even though the plugins were already picked up >>>>>>>> long time ago... >>>>>>> >>>>>>> yes, I think you are right. A restart during the package installation >>>>>>> might be better. Also the the case of updates we might want to restart >>>>>>> httpd in the %post section. >>>>>> >>>>>> Exactly. I think simple reload would be enough to force httpd load all >>>>>> new >>>>>> Python bits, we do not need to do a full blown restart, IMO. >>>>>> >>>>>> We will just need to find out if IPA is actually configured so that we >>>>>> do not >>>>>> reload httpd in that case. Checking that >>>>>> /var/lib/ipa/sysrestore/sysrestore.index >>>>>> exists and has at least two lines should be enough for the check. We do >>>>>> it >>>>>> similarly in is_ipa_configured() function. >>>>>> >>>>>> I am thinking that we will need the check+reload for both freeipa-server >>>>>> + >>>>>> freeipa-server-trust-ad, right? Because someone can install >>>>>> freeipa-server at >>>>>> once and then install freeipa-server-trust-ad after that. >>>>> >>>>> The new version of the patch add a conditional restart to the >>>>> freeipa-server-trust-ad package. So far I do not see the reason why it >>>>> must be done for freeipa-server. Maybe freeipa-python? >>>> >>>> Would this be overkill to determine if IPA is already configured >>>> rather than counting values in sysrestore? >>>> >>>> python -c "from ipaserver.install import installutils; print >>>> installutils.is_ipa_configured()" >>>> >>>> We don't have to address it in this patch but at what point do we >>>> want to drop the sysV checks in our dev spec file? >>>> >>> >>> New version attached. I preferred to use exit codes instead of print >>> because it looks readline is doing some magic on the output. >>> >>> bye, >>> Sumit >>> >> >> I think the approach is OK, but I really don't like the "1 - int(boolean >> value)" equation: >> >> +python -c "import sys; from ipaserver.install import installutils; >> sys.exit(1-int(installutils.is_ipa_configured()));" > /dev/null 2>&1 >> >> I think that: >> >> "sys.exit(0 if installutils.is_ipa_configured() else 1)" would be better. >> >> Martin > > sure, looks much more like python than my old C code :-). I also added > 'Requires(post): python' to be on the safe side. > > New version attached. > > bye, > Sumit >
Works fine, ACK. Rebased for ipa-3-0 and pushed to master, ipa-3-0. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
