Bug 196530

Created attachment 131474 [details]
patch to allow for starting/stopping/status of multiple daemons

If you think this general approach is acceptable, let me know and I'll see if I
can roll a specfile patch as well to do the needed changes there.

I think this definitely does have potential, yes.

There are some issues with the current implementation though, eg. files in the
config dir need to be whitelisted: load only *.conf files in order to skip eg.
*.rpmsave and friends, *~ etc, and probably keep stricter track of the
ez-ipupdate processes started by the init script in order to not blow away other
possibly independently started ones.

Debian appears to be doing similar things; see ez-ipupdate-3.0.11b8/debian/init
after a rpmbuild -qp (or make prep if you're working with FE CVS).  I think
their implementation covers this stuff pretty well, including starting only
processes that have been marked as to-be-daemonized in the respective config files.

Could you have a look at the Debian stuff and revise the patch based on it? 
Also, ideas how to cleanly upgrade existing configs to the new setup are welcome.

By the way, I'm not actively using ez-ipupdate myself, because I need something
that works behind a firewall at the moment (ddclient).  So in case you have an
Extras account set up, maintainership is up for grabs...

All the changes you mention seem reasonable. I had actually considered a few of
them before, but I didn't really need them, and didn't see the point unless you
were interested in incorporating a new init script. I'll have a look at the
Debian one and see about incorporating those changes.

Also, I just recently got my extras acct set up! So I'd be willing to take over
maintainership of the package as well. What needs to be done besides changing
ownership in the owners file?

Changing ownership is just a matter of changing owners.list and reassigning open
Bugzilla bugs.  This is the only open bug report against ez-ipupdate, so I'll
take care of it right now, and you can go ahead and make the owners.list change.
 Thanks!

BTW, if the transition from the old init mechanism can't be made all smooth and
transparent, I'd recommend shipping the new stuff for FC6+ only.

Thanks -- I went ahead and changed the owners list...

I'm thinking for the transition what I will probably do is use %pre or %post
scripts to move ez-ipupdate.conf to /etc/ez-ipupdate.d (or whatever I settle on)
and rename it to something like "default.conf". I'll need to work out the
details and test it to make sure it goes smoothly, but I think something like
that should work.

Created attachment 131815 [details]
proposed init script 2

Here's a second proposed init script that I think addresses all of the concerns
that Ville had before. Ville, any comments on this?

If this looks good, I'll see what I can to to add some pre/post magic to make
transitioning to the new scheme seamless for the typical user.

Looks mostly good, but some comments based on just skimming the script, not
actually tested:

- the pidfile: and config: lines in the top commentary are now stale, and based
on the fact that there has been a typo (by yours truly) in the pidfile: line for
ages without anyone noticing, they should probably be just removed.

- if I understand correctly, the pidfiles are now named *.conf which is unusual
and could be somewhat confusing, how about using "$(basename foo.conf .conf).pid"?

- reload() needs similar adjustments as stop()

- the status action needs *.conf whitelisting (instead of *)

- I would add LSB compliant action aliases as appropriate (try-restart =>
condrestart, force-reload => reload), see /usr/share/fedora/template.init if you
have fedora-rpmdevtools installed and
http://refspecs.freestandards.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html

Created attachment 131822 [details]
proposed init script 3

Good call on all counts. Here's an updated script that I think incorporates all
of your suggestions. I've briefly tested it and all the modes seem to work.

Thoughts?

Looks even better (still not tested but I trust you do test it).  Still a few
comments, some of them being on the nitpicky side:

- the pidfile: and config: lines in the top commentary were used by some old
init script tool (linuxconf? I don't remember), AFAIK the plural pidfiles: and
configs: mean nothing to it.  I think I'd still remove them, but of course they
can be useful for humans as they are now.

- the for ... loops in start(), stop() and reload() should probably have a
safeguard that tests that the current file is readable like the status action
does.  Otherwise things may break if the dirs exist but do not contain any files.

- the start(), stop() and reload() return values, or actually what $RETVAL is
after they're run, doesn't seem to be correct; $RETVAL remains to be set to
whatever was returned by the last operation on a particular daemon in a loop, so
if the last one succeeds and there were failures before it, the failures are not
reflected in the overall return value of the init script.  Fixing it could be a
bit tedious, but in case you feel like it...

Created attachment 131837 [details]
proposed init script 4

All those suggestions seem reasonable so I went ahead and incorporated them.
I'd rather get this right the first time than have to revisit it again later.
:-)

Please have a look when you get the chance, and let me know what you think.

Created attachment 131838 [details]
proposed script 5

Slightly revised script -- add .pid to end of pidfiles. Started looking over
the specfile updates that will need to be made with this too, but I'm not
through with that as of yet.

Apart from stop() and reload() testing for readability of a wrong file (should
be $pidfile, not $ez_configfile), looks good to me.

Created attachment 131850 [details]
proposed script 6

Doh! Good catch. This should fix that and also the $ez_name in those sections.

Created attachment 131851 [details]
specfile 1

Here's the current version of the specfile that I'm working with. This seems to
do a smooth upgrade to the new scheme on the box where I've tested it. Can you
also give this a once over and let me know what you think?

There is one question I have. The version string here is pretty complex and I'm
not sure how I should change the version number here. What is the versioning
scheme that you were using for these packages?

The init script looks ok to me now.

Looking at the specfile, there seems to be a problem with %post: one should try
hard to make it return a zero error code; otherwise on upgrades the transaction
aborts prematurely and the old one is not removed, and causes scary errors on
initial install too (untested).  For example, the last line returns 1 on new
installs.  I would add the whole shebang after chkconfig --add inside a "if [ $1
-gt 1 ] ; then ... fi" and remove the [ $1 -ge 1] check from the last line.  The
migration stuff is for upgrades anyway.

I see you removed some explicit %defattrs from the %files section; note that now
some of the example-*.conf doc files are installed as executable which needs
fixing some other way if you don't like the %defattr(644,root,root,755).

The version number scheme is the standard one from the package naming guidelines
for pre-release packages (I think that's what "b8" means).  See
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-e104844825856d7c45f2f0241586985c0495966b
for more info and rationale.  You seem to have nailed it exactly right in your
revised specfile ;)

Ok, I've gone ahead and incorporated your most recent set of suggestions and
have committed the changes to devel CVS. If you think the most recent versions
of the files look OK, then I'll go ahead and do a plague build of it.

The test in %post should be -gt 1, -ge 1 is always true.  (Another mistake of
mine originally, it seems.)

Also, if I was writing the specfile today, I'd not remove the user and group at
all.  But both removing them and leaving them have their upsides and downsides,
and IIRC there's no hard rule either way, so it's your call.

Ahh good catch. I had actually noticed the argument discrepancy before and meant
to go back and fix it.

I also tend to agree with you about not removing the user and group. I prefer to
leave them in place so that if there are stray files laying around they don't
end up being owned by a different user later.

I think this should just about do it. I'll go ahead and kick off a plague build
with all of these changes. Many thanks for the guidance :-).

If there are no complaints after a week or two with the devel package, then I'll
probably go ahead and roll these changes into FC5's tree as well.

Right, that's why I consider removing created users/groups bad too.  By the way,
now that they're no longer removed, the Requires(postun) on userdel/groupdel can
go too.

The plan sounds good, thanks for your patience :)

Long overdue update. The new init script is in place in fc6 and seems to be
fine. I've also removed the "requires(postun)" in the devel tree and in fc6, but
am not planning a new build for it since it really shouldn't affect the package.

I decided not to change this in fc5 after all, but any newer FC releases should
get it.