[Freeipa-devel] [PATCH 59] Modify spec file to add ipa_memcached service Install &, control ipa_memcached instance

[Simo Sorce]

On Fri, 2012-02-03 at 20:23 -0500, John Dennis wrote:
> On 02/03/2012 07:58 PM, Simo Sorce wrote:
> > On Fri, 2012-02-03 at 19:12 -0500, John Dennis wrote:
> >> On 01/25/2012 09:12 AM, Rob Crittenden wrote:
> >>> John Dennis wrote:
> >>>> This patch supersedes the previous patch, it corrects two issue Rob
> >>>> raised in a later patch review
> >>>>
> >>>> The fixed issues are:
> >>>>
> >>>> * spec file did not install ipa_memcached SysV initscript on SvsV systems.
> >>>>
> >>>> * Typo in the name of the pid file variable in the ipa_memcached initscript
> >>>>
> >>>
> >>> NACK, two small things.
> >>>
> >>> ipa-server-install man page needs to be updated with new option.
> >>
> >> I'm going to delete the -M option, things won't run without ipa_memcache_d
> >>
> >>> ipa_memcached is not chkconfig'd on/off when it is installed/uninstalled.
> >>
> >> I'm confused by this, this is handled by the SimpleServiceInstance
> >> class, it's not chkconfig'd by the spec file, nor should it be.
> >>
> >> During install you get this:
> >>
> >> Configuring ipa_memcached
> >>     [1/2]: starting ipa_memcached
> >>     [2/2]: configuring ipa_memcached to start on boot
> >> done configuring ipa_memcached.
> >>
> >> Which in detail is doing this:
> >>
> >> DEBUG Configuring ipa_memcached
> >> DEBUG   [1/2]: starting ipa_memcached
> >> DEBUG args=/bin/systemctl is-active ipa_memcached.service
> >> DEBUG stdout=unknown
> >> DEBUG stderr=
> >> DEBUG Saving StateFile to '/var/lib/ipa/sysrestore/sysrestore.state'
> >> DEBUG args=/bin/systemctl restart ipa_memcached.service
> >> DEBUG stdout=
> >> DEBUG stderr=
> >> DEBUG   duration: 0 seconds
> >> DEBUG   [2/2]: configuring ipa_memcached to start on boot
> >> DEBUG args=/bin/systemctl enable ipa_memcached.service
> >> DEBUG stdout=
> >> DEBUG stderr=ln -s '/lib/systemd/system/ipa_memcached.service'
> >> '/etc/systemd/system/multi-user.target.wants/ipa_memcached.service'
> >> DEBUG args=/bin/systemctl is-enabled ipa_memcached.service
> >> DEBUG stdout=enabled
> >> DEBUG stderr=
> >> DEBUG Saving StateFile to '/var/lib/ipa/sysrestore/sysrestore.state'
> >> DEBUG args=/bin/systemctl disable ipa_memcached.service
> >> DEBUG stdout=
> >> DEBUG stderr=rm
> >> '/etc/systemd/system/multi-user.target.wants/ipa_memcached.service'
> >> DEBUG   duration: 0 seconds
> >> DEBUG done configuring ipa_memcached.
> >>
> >> As you can see from above on f16 it's not using chkconfig but systemd,
> >> but it should use chkconfig on other OS's.
> >>
> >> So if it's not being chkconfig'd on it's a bug in host installation code.
> >
> > In IPA no service should be started by chkconfig or systemd itself, we
> > handle all daemons via ipactl and the list we set in LDAP. please do not
> > diverge.
> >
> > You get a NACK on the approach, please be consistent with the rest of
> > IPA controlled services.
> 
> This is exactly how the kadmin (e.g. KPASSWD) and ipa_webgui services 
> are managed. Also ipa_memcached is controlled by ipactl in exactly the 
> same way the other services are. So how is it inconsistent?
> 
> Does that mean kadmin(KPASSWD) is also broken?

No it means I should not review patches this late :-(

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York