[Freeipa-devel] [PATCH] 88 ipa-adtrust-install: restart httpd to pick up new plugins

[Sumit Bose]

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
-------------- next part --------------
From b195ceb3f3478185682c91adb67265b76af842e5 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 26 Oct 2012 13:12:17 +0200
Subject: [PATCH] Restart httpd if ipa-server-trust-ad is installed or updated

If ipa-server-trust-ad is installed after the ipa server is installed
and configured, httpd needs a restart for additional python modules to
be loaded into httpd on IPA initialization.

Fixes https://fedorahosted.org/freeipa/ticket/3185
---
 freeipa.spec.in | 14 ++++++++++++++
 1 Datei ge?ndert, 14 Zeilen hinzugef?gt(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index af76118fd0294fa4d8934b747c254b891ae7f2cb..5845770576fe1408b244e8404eb9b16225deb68d 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -238,6 +238,7 @@ Requires: libsss_idmap
 # IPA AD trusts cannot be used at the same time with the locator plugin
 # since Winbindd will be configured in a different mode
 Requires(post): %{_sbindir}/update-alternatives
+Requires(post): python
 Requires(postun): %{_sbindir}/update-alternatives
 Requires(preun): %{_sbindir}/update-alternatives
 
@@ -592,6 +593,16 @@ fi
 %post server-trust-ad
 %{_sbindir}/update-alternatives --install %{_libdir}/krb5/plugins/libkrb5/winbind_krb5_locator.so \
 				winbind_krb5_locator.so /dev/null 90
+python -c "import sys; from ipaserver.install import installutils; sys.exit(0 if installutils.is_ipa_configured() else 1);" > /dev/null 2>&1
+if [  $? -eq 0 ]; then
+%if 0%{?fedora} >= 16
+# Use systemd scheme
+    /bin/systemctl try-restart httpd.service >/dev/null 2>&1 || :
+%else
+# Use SystemV scheme only before F16
+    /sbin/service httpd condrestart >/dev/null 2>&1 || :
+%endif
+fi
 
 %preun server-trust-ad
 if [ $1 -eq 0 ]; then
@@ -816,6 +827,9 @@ fi
 %ghost %attr(0644,root,apache) %config(noreplace) %{_sysconfdir}/ipa/ca.crt
 
 %changelog
+* Fri Oct 26 2012 Sumit Bose <sbose at redhat.com> - 3.0.99-2
+- Restart httpd in post install of server-trust-ad
+
 * Wed Oct 24 2012 Martin Kosek <mkosek at redhat.com> - 3.0.99-1
 - Add strict Requires for 389-ds-base and policycoreutils to avoid user
   removing them during package lifetime
-- 
1.7.11.4