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:27:15 +0100 Message-ID: <5A6FBFC4-9888-4FD0-8972-8C2DE7495042@ipfire.org> In-Reply-To: <15277bb1b0bd75da76715ac4557ce00785861139.camel@disroot.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4671081983903654484==" List-Id: --===============4671081983903654484== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hello, > On 6 Jun 2021, at 19:41, Robin Roevens wrote: >=20 > Hi Michael >=20 > I know/see a lot of activity here, so I assume you are always quite > busy and may have missed my last reaction? Yes, sorry, I missed this, or didn=E2=80=99t know what to say. Feel free to r= emind me if you didn=E2=80=99t hear from me within a couple of days. > Should I proceed with my proposition to add > Summary: SUMMARY > Services: SERVICES > to the pak metadata including an INSTALL_SERVICE_INITSCRIPTS macro I would say yes. Do we have any unanswered questions left? Is Jonatan happy? > Should I also add=20 > InitScripts: INITSCRIPTS > for non-service initscripts.. just for completion of metadata ? Hmm, I would say no. If there is no point to it, then leave it. We can always add it later if we decide that we want it later. > 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) >=20 > Or should we find other methods to have this info ready for > services.cgi and/or monitoring agents ? No, I believe this is a good solution. We can then have a function that queri= es the package database and shows all services correctly. That sounds clean to me and is re-usable and extensible. -Michael >=20 > Regards > Robin >=20 > Robin Roevens schreef op ma 24-05-2021 om 22:23 [+0200]: >> 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 >>=20 >> 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). >>=20 >> If it would make things more clear, I could work my last mail out in an >> actual patch ? >>=20 >> 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 >=20 > --=20 > Dit bericht is gescanned op virussen en andere gevaarlijke > inhoud door MailScanner en lijkt schoon te zijn. >=20 --===============4671081983903654484==--