Hi Michael

I know/see a lot of activity here, so I assume you are always quite
busy and may have missed my last reaction?

Should I proceed with my proposition to add
Summary: SUMMARY
Services: SERVICES
to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro

Should I also add 
InitScripts: INITSCRIPTS
for non-service initscripts.. just for completion of metadata ?
And in that case a more generic INSTALL_INITSCRIPTS macro accepting a
list of initscripts to install? (see mail history below for explanation
of my intentions)

Or should we find other methods to have this info ready for
services.cgi and/or monitoring agents ?

Regards
Robin

Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]:
> Hi Michael
> 
> Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]:
> > 
> > 
> > > On 14 May 2021, at 21:07, Robin Roevens <robin.roevens(a)disroot.org>
> > > wrote:
> > > 
> > > Hi
> > > 
> > > Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]:
> > > > Hello Jonatan,
> > > > 
> > > > Yes I get your point. Just because a package has more than one
> > > > initscript does not mean they both should be listed in
> > > > services.cgi.
> > > > 
> > > > It is a weird corner case, but how do we want to handle this?
> > > > 
> > > > -Michael
> > > 
> > > Maybe a slight renaming of my proposed new variable INITSCRIPTS to
> > > SERVICES in the LFS files would make it a bit more clear that this
> > > concerns only service-initscripts?
> > > 
> > > The INSTALL_INITSCRIPTS macro could then be renamed to
> > > INSTALL_SERVICE_INITSCRIPTS.
> > > 
> > > If we then also keep the original INSTALL_INITSCRIPT macro, that
> > > macro
> > > can  then be used to install non-service initscripts if required.
> > > Or
> > > even to install all required initscripts in the occasion that you
> > > want
> > > to skip some service-initscripts defined in SERVICES because they
> > > are
> > > included in the source, like the situation Jonatan pointed out (in
> > > this
> > > case the SERVICES variable is filled with the sole purpose to serve
> > > as
> > > metadata).
> > > 
> > > So this would then result in:
> > > ---
> > > define INSTALL_INITSCRIPT
> > >   install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1) 
> > > /etc/rc.d/init.d/$(1)
> > > endef
> > > 
> > > define INSTALL_SERVICE_INITSCRIPTS
> > >   for service in $(SERVICES); do \
> > >      $(call INSTALL_INITSCRIPT,$$service) || exit 1; \
> > >   done
> > > endef
> > > ---
> > > 
> > > Optionally we could also add again a variable INITSCRIPTS with the
> > > purpose to list non-service initscripts. so
> > > SERVICES - for service-initscripts
> > > INITSCRIPTS - for non-service-initscripts
> > > 
> > > and in the meta-* files:
> > > ---
> > > Name: NAME
> > > Summary: SUMMARY
> > > ProgVersion: VER
> > > Release: RELEASE
> > > Size: SIZE
> > > Dependencies: DEPS
> > > File: NAME-VER-RELEASE.ipfire
> > > Services: SERVICES
> > > InitScripts: INITSCRIPTS
> > > ---
> > > 
> > > Maybe that info could also come in handy sooner or later ?
> > 
> > This sounds rather complicated to me.
> > 
> > What are our objectives here? Does Zabbix want/need to start/stop
> > services? And if so, why?
> 
> I was not directly thinking of Zabbix specifically here. The objective
> is to have a central 'database' of metadata for the pak's that can come
> in handy for current and possibly future functionality of IPFire. And a
> single way of retrieving this information (both by using "pakfire info"
> or directly accessing the pakfire library, in my proposal). 
> 
> Knowing which pak's provide which services can be used in service.cgi
> (which currently does give users the possibility to start/stop services
> indeed) and can be used for monitoring agents like Zabbix, but also
> Nagios or possibly others to know which services to monitor.
> 
> This instead of my earlier proposal of re-using parts of the
> services.cgi code in a separate script for zabbix_agentd, which you
> pointed out that should be done cleaner using some central way of
> storing/retrieving that information.
> 
> Zabbix does have functionality to implement starting/stopping those
> services using a context menu in the Zabbix web GUI; and a user can
> easily implement that on it's own (as this should be done mainly on
> server-side anyway) if he wants to and has the information of which
> services should run. This could be done by zabbix server logging into
> the IPFire machine using ssh and using addonctrl or by the agent, which
> would then need permission to addonctrl start/stop. But I think that is
> something for the user to decide on his own as you pointed out
> previously, giving zabbix agent root permission on addonctrl start/stop
> is a possible extra security risk. So I don't think we should provide
> that out of the box. (Maybe it could be a configurable 'setting' in the
> IPFire webgui some time in the future, but not something to focus on
> currently)
> 
> But with that said, I don't really understand your remark here as I
> only propose a way for the LFS files to populate the "Services" meta-
> data and I only threw in "Initscripts" for non-service initscripts,
> just for completeness of the meta-data.
> And to prevent duplication of the list of initscripts, I added a macro
> to install those in one go instead of again 'defining' this list by
> needing to call INSTALL_INITSCRIPT individually for each initscript
> while we now have a list of them in a variable. And due to the remark
> of Jonatan I now also kept the original INSTALL_INITSCRIPT for the pak
> maintainer to use in corner cases.
> 
> But this has nothing to do with actually stopping/starting services
> other that providing information about which services are installed
> (and maybe could be stopped/started if wanted).
> 
> If it would make things more clear, I could work my last mail out in an
> actual patch ?
> 
> Regards
> Robin
> 
> > 
> > > In this case the INSTALL_SERVICE_INITSCRIPTS could again be renamed
> > > back to INSTALL_INITSCRIPTS, now accepting a parameter which is a
> > > list
> > > of initscripts to install, so one could call:
> > > ---
> > > $(call INSTALL_INITSCRIPTS,$(SERVICES))
> > > $(call INSTALL_INITSCRIPTS,$(INITSCRIPTS))
> > > ---
> > > to install all initscripts.
> > > 
> > > Or even provide again an additional macro INSTALL_ALL_INITSCRIPTS
> > > which
> > > would then install both SERVICES and INITSCRIPTS (or even replace
> > > the
> > > above suggested INSTALL_SERVICE_INITSCRIPTS, so resulting in a
> > > macro to
> > > install all initscripts (SERVICES and INITSCRIPTS) and a macro to
> > > install a single initscript in case some of the ones listed in
> > > SERVICES
> > > and/or INITSCRIPTS need to be skipped..
> > > 
> > > But I think the INSTALL_INITSCRIPTS which takes a list of services
> > > to
> > > install as parameter is probably the most generic one, giving you
> > > the
> > > possibility to install all SERVICES in one go, and then
> > > individually
> > > install only those INITSCRIPTS not included in the source if
> > > INITSCRIPTS contains such initscript(s) or vice versa.
> > > 
> > > Robin
> > > 
> > > > 
> > > > > On 12 May 2021, at 19:49, Jonatan Schlag <
> > > > > jonatan.schlag(a)ipfire.org>
> > > > > wrote:
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > > Am 23.04.2021 um 18:16 schrieb Robin Roevens <
> > > > > > robin.roevens(a)disroot.org>:
> > > > > > 
> > > > > > * Introduce SUMMARY and INITSCRIPTS macro's in LFS-
> > > > > > makefiles.
> > > > > > * Add a Summary and InitScripts field to the meta-* addon
> > > > > > files
> > > > > > containing the value's of those macro's.
> > > > > > * Replace the INSTALL_INITSCRIPT makefile macro by a new
> > > > > > INSTALL_INITSCRIPTS macro/method that will install all
> > > > > > initscripts
> > > > > > defined in the new INITSCRIPTS macro.
> > > > > > * Adapt libvirt and zabbix_agentd pak as examples of how to
> > > > > > use
> > > > > > this.
> > > > > > 
> > > > > > Signed-off-by: Robin Roevens <robin.roevens(a)disroot.org>
> > > > > > ---
> > > > > > lfs/Config        | 8 ++++++--
> > > > > > lfs/libvirt       | 6 ++++--
> > > > > > lfs/zabbix_agentd | 5 ++++-
> > > > > > src/pakfire/meta  | 2 ++
> > > > > > 4 files changed, 16 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/lfs/Config b/lfs/Config
> > > > > > index eadbbc408..61d6f0c82 100644
> > > > > > --- a/lfs/Config
> > > > > > +++ b/lfs/Config
> > > > > > @@ -299,15 +299,19 @@ define PAK
> > > > > >   # Create meta file
> > > > > >   sed \
> > > > > >       -e "s/NAME/$(PROG)/g" \
> > > > > > +        -e "s/SUMMARY/$(SUMMARY)/g" \
> > > > > >       -e "s/VER/$(VER)/g" \
> > > > > >       -e "s/RELEASE/$(PAK_VER)/g" \
> > > > > >       -e "s/DEPS/$(DEPS)/g" \
> > > > > >       -e "s/SIZE/$$(stat --format=%s
> > > > > > /install/packages/$(PROG)-
> > > > > > $(VER)-$(PAK_VER).ipfire)/g" \
> > > > > > +        -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \
> > > > > >     < /usr/src/src/pakfire/meta > /install/packages/meta-
> > > > > > $(PROG)
> > > > > > endef
> > > > > > 
> > > > > > -define INSTALL_INITSCRIPT
> > > > > > -    install -m 754 -v
> > > > > > $(DIR_SRC)/src/initscripts/packages/$(1) 
> > > > > > /etc/rc.d/init.d/$(1)
> > > > > > +define INSTALL_INITSCRIPTS
> > > > > > +    for initscript in $(INITSCRIPTS); do \
> > > > > > +        install -m 754 -v
> > > > > > $(DIR_SRC)/src/initscripts/packages/$$initscript 
> > > > > > /etc/rc.d/init.d/$$initscript; \
> > > > > > +    done
> > > > > > endef
> > > > > > 
> > > > > > ifeq "$(BUILD_ARCH)" "$(filter $(BUILD_ARCH),aarch64
> > > > > > riscv64)"
> > > > > > diff --git a/lfs/libvirt b/lfs/libvirt
> > > > > > index 28a95d317..be5d3db3a 100644
> > > > > > --- a/lfs/libvirt
> > > > > > +++ b/lfs/libvirt
> > > > > > @@ -25,6 +25,7 @@
> > > > > > include Config
> > > > > > 
> > > > > > VER        = 6.5.0
> > > > > > +SUMMARY       = Server side daemon and supporting files for
> > > > > > libvirt
> > > > > > 
> > > > > > THISAPP    = libvirt-$(VER)
> > > > > > DL_FILE    = $(THISAPP).tar.xz
> > > > > > @@ -37,6 +38,8 @@ PAK_VER    = 25
> > > > > > 
> > > > > > DEPS       = ebtables libpciaccess libtirpc libyajl ncat qemu
> > > > > > 
> > > > > > +INITSCRIPTS= libvirtd virtlogd
> > > > > > +
> > > > > > #############################################################
> > > > > > ######
> > > > > > ############
> > > > > > # Top-level Rules
> > > > > > #############################################################
> > > > > > ######
> > > > > > ############
> > > > > > @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst
> > > > > > %,$(DIR_DL)/%,$(objects))
> > > > > >   cd $(DIR_APP)/build_libvirt && make install
> > > > > > 
> > > > > >   #install initscripts
> > > > > > -    $(call INSTALL_INITSCRIPT,libvirtd)
> > > > > > -    $(call INSTALL_INITSCRIPT,virtlogd)
> > > > > > +    @$(INSTALL_INITSCRIPTS)
> > > > > >   mv /usr/libexec/libvirt-guests.sh /etc/rc.d/init.d/libvirt-
> > > > > > guests
> > > > > And here my approach maybe breaks :-). How could we handle
> > > > > this? It
> > > > > is not a daemon, more something like a startup/shutdown scripts
> > > > > for
> > > > > machines... (And should not appear in the service.cgi). So
> > > > > parsing
> > > > > the root file may yield wrong results and your way would be the
> > > > > better one. 
> > > > > 
> > > > > Jonatan
> > > > > > 
> > > > > >   # Backup
> > > > > > diff --git a/lfs/zabbix_agentd b/lfs/zabbix_agentd
> > > > > > index c69643a54..a72fe024b 100644
> > > > > > --- a/lfs/zabbix_agentd
> > > > > > +++ b/lfs/zabbix_agentd
> > > > > > @@ -25,6 +25,7 @@
> > > > > > include Config
> > > > > > 
> > > > > > VER        = 4.2.6
> > > > > > +SUMMARY    = Zabbix Agent
> > > > > > 
> > > > > > THISAPP    = zabbix-$(VER)
> > > > > > DL_FILE    = $(THISAPP).tar.gz
> > > > > > @@ -35,6 +36,8 @@ PROG       = zabbix_agentd
> > > > > > PAK_VER    = 4
> > > > > > DEPS       =
> > > > > > 
> > > > > > +INITSCRIPTS= zabbix_agentd
> > > > > > +
> > > > > > #############################################################
> > > > > > ######
> > > > > > ############
> > > > > > # Top-level Rules
> > > > > > #############################################################
> > > > > > ######
> > > > > > ############
> > > > > > @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst
> > > > > > %,$(DIR_DL)/%,$(objects))
> > > > > >   chown zabbix.zabbix /var/run/zabbix
> > > > > > 
> > > > > >   # Install initscripts
> > > > > > -    $(call INSTALL_INITSCRIPT,zabbix_agentd)
> > > > > > +    @$(INSTALL_INITSCRIPTS)
> > > > > > 
> > > > > >   # Install sudoers include file
> > > > > >   install -v -m 644 $(DIR_SRC)/config/zabbix_agentd/sudoers \
> > > > > > diff --git a/src/pakfire/meta b/src/pakfire/meta
> > > > > > index d97b2a0fa..849b9cd6c 100644
> > > > > > --- a/src/pakfire/meta
> > > > > > +++ b/src/pakfire/meta
> > > > > > @@ -1,6 +1,8 @@
> > > > > > Name: NAME
> > > > > > +Summary: SUMMARY
> > > > > > ProgVersion: VER
> > > > > > Release: RELEASE
> > > > > > Size: SIZE
> > > > > > Dependencies: DEPS
> > > > > > File: NAME-VER-RELEASE.ipfire
> > > > > > +InitScripts: INITSCRIPTS
> > > > > > -- 
> > > > > > 2.31.1
> > > > > > 
> > > > > > 
> > > > > > -- 
> > > > > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > > > > inhoud door MailScanner en lijkt schoon te zijn.
> > > > 
> > > > 
> > > 
> > > 
> > > -- 
> > > Dit bericht is gescanned op virussen en andere gevaarlijke
> > > inhoud door MailScanner en lijkt schoon te zijn.
> > 
> > 
> 
> 


-- 
Dit bericht is gescanned op virussen en andere gevaarlijke
inhoud door MailScanner en lijkt schoon te zijn.