From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: development@lists.ipfire.org Subject: Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files Date: Thu, 10 Jun 2021 12:24:47 +0100 Message-ID: In-Reply-To: <3d6bdff33c719abfa9695c71b92ecc98650feb01.camel@disroot.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2879672716214916785==" List-Id: --===============2879672716214916785== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, > On 24 May 2021, at 21:23, Robin Roevens wrote: >=20 > Hi Michael >=20 > Michael Tremer schreef op di 18-05-2021 om 16:09 [+0100]: >>=20 >>=20 >>> On 14 May 2021, at 21:07, Robin Roevens >>> wrote: >>>=20 >>> Hi >>>=20 >>> Michael Tremer schreef op vr 14-05-2021 om 13:24 [+0100]: >>>> Hello Jonatan, >>>>=20 >>>> 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. >>>>=20 >>>> It is a weird corner case, but how do we want to handle this? >>>>=20 >>>> -Michael >>>=20 >>> 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? >>>=20 >>> The INSTALL_INITSCRIPTS macro could then be renamed to >>> INSTALL_SERVICE_INITSCRIPTS. >>>=20 >>> 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). >>>=20 >>> So this would then result in: >>> --- >>> define INSTALL_INITSCRIPT >>> install -m 754 -v $(DIR_SRC)/src/initscripts/packages/$(1)=20 >>> /etc/rc.d/init.d/$(1) >>> endef >>>=20 >>> define INSTALL_SERVICE_INITSCRIPTS >>> for service in $(SERVICES); do \ >>> $(call INSTALL_INITSCRIPT,$$service) || exit 1; \ >>> done >>> endef >>> --- >>>=20 >>> 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 >>>=20 >>> 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 >>> --- >>>=20 >>> Maybe that info could also come in handy sooner or later ? >>=20 >> This sounds rather complicated to me. >>=20 >> What are our objectives here? Does Zabbix want/need to start/stop >> services? And if so, why? >=20 > 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).=20 I understood that, but my question remains. Why does Zabbix need to start/sto= p individual services on the firewall? I just want to understand the use-case so I can help to build a better soluti= on. > 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. >=20 > 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. >=20 > 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) >=20 > 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. >=20 > 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). Noted. > If it would make things more clear, I could work my last mail out in an > actual patch ? To remind me again, what does that entail? -Michael > Regards > Robin >=20 >>=20 >>> 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. >>>=20 >>> 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.. >>>=20 >>> 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. >>>=20 >>> Robin >>>=20 >>>>=20 >>>>> On 12 May 2021, at 19:49, Jonatan Schlag < >>>>> jonatan.schlag(a)ipfire.org> >>>>> wrote: >>>>>=20 >>>>> Hi, >>>>>=20 >>>>>> Am 23.04.2021 um 18:16 schrieb Robin Roevens < >>>>>> robin.roevens(a)disroot.org>: >>>>>>=20 >>>>>> =EF=BB=BF* 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. >>>>>>=20 >>>>>> Signed-off-by: Robin Roevens >>>>>> --- >>>>>> lfs/Config | 8 ++++++-- >>>>>> lfs/libvirt | 6 ++++-- >>>>>> lfs/zabbix_agentd | 5 ++++- >>>>>> src/pakfire/meta | 2 ++ >>>>>> 4 files changed, 16 insertions(+), 5 deletions(-) >>>>>>=20 >>>>>> 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=3D%s >>>>>> /install/packages/$(PROG)- >>>>>> $(VER)-$(PAK_VER).ipfire)/g" \ >>>>>> + -e "s/INITSCRIPTS/$(INITSCRIPTS)/g" \ >>>>>> < /usr/src/src/pakfire/meta > /install/packages/meta- >>>>>> $(PROG) >>>>>> endef >>>>>>=20 >>>>>> -define INSTALL_INITSCRIPT >>>>>> - install -m 754 -v >>>>>> $(DIR_SRC)/src/initscripts/packages/$(1)=20 >>>>>> /etc/rc.d/init.d/$(1) >>>>>> +define INSTALL_INITSCRIPTS >>>>>> + for initscript in $(INITSCRIPTS); do \ >>>>>> + install -m 754 -v >>>>>> $(DIR_SRC)/src/initscripts/packages/$$initscript=20 >>>>>> /etc/rc.d/init.d/$$initscript; \ >>>>>> + done >>>>>> endef >>>>>>=20 >>>>>> 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 >>>>>>=20 >>>>>> VER =3D 6.5.0 >>>>>> +SUMMARY =3D Server side daemon and supporting files for >>>>>> libvirt >>>>>>=20 >>>>>> THISAPP =3D libvirt-$(VER) >>>>>> DL_FILE =3D $(THISAPP).tar.xz >>>>>> @@ -37,6 +38,8 @@ PAK_VER =3D 25 >>>>>>=20 >>>>>> DEPS =3D ebtables libpciaccess libtirpc libyajl ncat qemu >>>>>>=20 >>>>>> +INITSCRIPTS=3D libvirtd virtlogd >>>>>> + >>>>>> ############################################################# >>>>>> ###### >>>>>> ############ >>>>>> # Top-level Rules >>>>>> ############################################################# >>>>>> ###### >>>>>> ############ >>>>>> @@ -121,8 +124,7 @@ $(TARGET) : $(patsubst >>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>> cd $(DIR_APP)/build_libvirt && make install >>>>>>=20 >>>>>> #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.=20 >>>>>=20 >>>>> Jonatan >>>>>>=20 >>>>>> # 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 >>>>>>=20 >>>>>> VER =3D 4.2.6 >>>>>> +SUMMARY =3D Zabbix Agent >>>>>>=20 >>>>>> THISAPP =3D zabbix-$(VER) >>>>>> DL_FILE =3D $(THISAPP).tar.gz >>>>>> @@ -35,6 +36,8 @@ PROG =3D zabbix_agentd >>>>>> PAK_VER =3D 4 >>>>>> DEPS =3D >>>>>>=20 >>>>>> +INITSCRIPTS=3D zabbix_agentd >>>>>> + >>>>>> ############################################################# >>>>>> ###### >>>>>> ############ >>>>>> # Top-level Rules >>>>>> ############################################################# >>>>>> ###### >>>>>> ############ >>>>>> @@ -106,7 +109,7 @@ $(TARGET) : $(patsubst >>>>>> %,$(DIR_DL)/%,$(objects)) >>>>>> chown zabbix.zabbix /var/run/zabbix >>>>>>=20 >>>>>> # Install initscripts >>>>>> - $(call INSTALL_INITSCRIPT,zabbix_agentd) >>>>>> + @$(INSTALL_INITSCRIPTS) >>>>>>=20 >>>>>> # 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 >>>>>> --=20 >>>>>> 2.31.1 >>>>>>=20 >>>>>>=20 >>>>>> --=20 >>>>>> Dit bericht is gescanned op virussen en andere gevaarlijke >>>>>> inhoud door MailScanner en lijkt schoon te zijn. >>>>=20 >>>>=20 >>>=20 >>>=20 >>> --=20 >>> Dit bericht is gescanned op virussen en andere gevaarlijke >>> inhoud door MailScanner en lijkt schoon te zijn. >>=20 >>=20 >=20 >=20 > --=20 > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. >=20 --===============2879672716214916785==--