Bug 10433

Summary: syslog-ng rc5.d symlinks missing after install, broken postinst
Product: [Build System, Metadata & Runtime] Other YP Layers Reporter: Haris Okanovic <haris.okanovic>
Component: layersAssignee: Markus Lehtonen <markus.lehtonen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: Medium+ CC: adrian.ratiu, freesilicon, markus.lehtonen, poky.bs.watcher, poky.watcher, ross.burton
Version: unspecified   
Target Milestone: 2.3 M2   
Hardware: x86   
OS: Multiple   
Whiteboard: Need backport to 2.2.1
OS type for building Yocto: --- Type of Regression: ---
Verified: Documentation change: No (bug/feature does not impact docs)
Attachments:
Description Flags
A workaround for update-alternatives and update-rc.d ordering issue in syslog-ng none

Description Haris Okanovic 2016-10-13 23:06:48 UTC
Created attachment 3476 [details]
A workaround for update-alternatives and update-rc.d ordering issue in syslog-ng

syslog-ng's postinst script appears to be broken. It calls `update-alternatives` to alias it's init.d script (synlink syslog --> syslog.syslog-ng) and `update-rc.d` to install start-up symlinks (in rc[2-5].d); but they happen in the wrong order. It first attempts update-rc.d on the aliased init.d filename (syslog) which doesn't exist yet, then it calls update-alternatives to create it. Postinst returns 0, so opkg thinks syslog-ng was successfully installed and configured, but it won't start on boot-up.

The bug appears to have been introduced by commit 03071511, which reversed the order of update-alternatives and update-rc.d (bbclass) inherits, to ironically fix a similar reordering issue in prerm! I've attached one possible (somewhat hacky) solution to both issues -- workaround.patch. See commit message for details.

The correct fix will likely require some changes to update-alternatives and update-rc.d bbclasses in how they generate maintainer scripts. It seems like there should be some ordering guarantees between those two pieces of generated sh code.
Comment 1 Markus Lehtonen 2016-11-11 14:11:33 UTC
Seeing these bugs, I think using update-alternatives to manage init scripts is a bad idea altogether. Whatever ordering we have with the classes we always have some kind of a problem because of missing symlink at some point. I try to fix it by making the syslog packages work like any other daemon package in that regard: don't use update alternatives.
Comment 2 Markus Lehtonen 2016-11-22 12:30:07 UTC
Patches sent for oe-core and meta-oe:
https://patchwork.openembedded.org/series/4013/
https://patchwork.openembedded.org/series/4014/
Comment 3 David Vincent 2016-11-24 08:13:13 UTC
I submitted the following series that rather modify the task ordering between update-alternatives and update-rc.d instead of simply removing it and creating RCONFLICTS:
https://patchwork.openembedded.org/series/4061/

It has been successfully tested with syslog-ng on morty. It does not yet cleanly apply, but I will resubmit a v2 ASAP.
Comment 4 Markus Lehtonen 2016-11-24 10:21:05 UTC
(In reply to comment #3)
> I submitted the following series that rather modify the task ordering
> between update-alternatives and update-rc.d instead of simply removing it
> and creating RCONFLICTS:

I'm still not convinced that init scripts should be managed by update-alternatives. First, I don't see much reasoning why two syslog services should be installed in parallel. Also, no other package or distro use this kind of construction, AFAIK.

If we really want to make these install in parallel I think we should just name the files differently, e.g.
/etc/init.d/busybox-syslog
/etc/init.d/sysklogd
/etc/init.d/rsyslog
...
Comment 5 David Vincent 2016-11-24 11:10:45 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > I submitted the following series that rather modify the task ordering
> > between update-alternatives and update-rc.d instead of simply removing it
> > and creating RCONFLICTS:
> 
> I'm still not convinced that init scripts should be managed by
> update-alternatives. First, I don't see much reasoning why two syslog
> services should be installed in parallel. Also, no other package or distro
> use this kind of construction, AFAIK.

Maybe there should be a virtual/logger, then ?

But the problem could also be raised by cron daemons for example. In fact, I think the main problem lies with busybox which offers alternatives for such packages and these alternatives must be overridden when a new one is selected.

> 
> If we really want to make these install in parallel I think we should just
> name the files differently, e.g.
> /etc/init.d/busybox-syslog
> /etc/init.d/sysklogd
> /etc/init.d/rsyslog
> ...

The problem is when using different names is that update-rc.d will install all links in rc*.d thus creating logger conflicts (I don't know if that's even possible but it's the general idea).
Comment 6 Markus Lehtonen 2016-11-24 11:23:22 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > I submitted the following series that rather modify the task ordering
> > > between update-alternatives and update-rc.d instead of simply removing it
> > > and creating RCONFLICTS:
> > 
> > I'm still not convinced that init scripts should be managed by
> > update-alternatives. First, I don't see much reasoning why two syslog
> > services should be installed in parallel. Also, no other package or distro
> > use this kind of construction, AFAIK.
> 
> Maybe there should be a virtual/logger, then ?

This doesn't work nicely because the virtual/* packages are on source package level. This syslog overlap is on binary (runtime) package level. My patch introduces VIRTUAL-RUNTIME_syslog just for this purpose.


> But the problem could also be raised by cron daemons for example. In fact, I
> think the main problem lies with busybox which offers alternatives for such
> packages and these alternatives must be overridden when a new one is
> selected.

I think these should and are handled by VIRTUAL-RUNTIME_* definitions
Comment 7 Markus Lehtonen 2016-12-08 16:33:00 UTC
My latest patchset is here:
http://git.openembedded.org/openembedded-core-contrib/log/?h=marquiz/fixes-10433

However, it isn't a possible solution because of bugs in opkg and smartpm (for deb backend it works as expected). A solution would be to merge David's patch, but as a temporary workaround. I'd suggest the following:
1. Merge David's patch
2. Close this bug
3. Create a new bug that would
   - be about reverting David's patch and merging my patches
   - block on bug 9675
   - block on bug 10777
Comment 8 David Vincent 2016-12-09 09:08:12 UTC
Before closing this bug, I just want to add that my patch is harmless in the long run. Even if a solution is found for this initscript/alternatives issue (for syslogd and any other daemons conflicting with busybox...), all it does is create comments inside prerm/postinst sections of package scripts and organize the two sections accordingly if update-rc.d and update-alternatives are both inherited in a recipe.

I completely agree that initscripts should not be managed via alternatives but it is also not forbidden to do so. No distribution does this, because I think busybox is not used in them outside initramfs and conflicting packages like you did is perfect at handling this case. But the situation is different in the embedded world where busybox is always running somewhere...

So before reverting it, I think it must be noted that this could be the kind of bug which reappears just because another layer is not so thoughtful about this.
Comment 9 Markus Lehtonen 2016-12-09 09:44:16 UTC
Busybox shouldn't be a problem, even in the present situation, because it is divided into multiple (runtime) sub-packages. Busybox-syslog can be replaced with something else even if the rest of busybox is installed in the system.

The reason I'd like to get your patch reverted in the future, assuming we can achieve a "packaging-level solution", is that I'd like keep the code as simple as possible. Thus, even if it is harmless, I kind see it as a kludge to work around an issue in just one package.
Comment 10 David Vincent 2016-12-13 08:39:06 UTC
OK, I now see your point. BTW, I resubmitted my patch since it did not apply on latest 'master', so there should be no problem merging it now.
Comment 11 Ross Burton 2017-01-19 11:49:49 UTC
Merged in oe-core aa87b1a4dcc14e4dfe719b6c55045c5662bc59c2.
Comment 12 Markus Lehtonen 2017-02-13 13:46:01 UTC
Fix backported and merged into morty:
http://git.openembedded.org/openembedded-core/commit/?h=morty&id=b0c70bef015f1b2a30556a5db5e255592d5bf316