Re: [PATCH 1/3] buildprocess: Add extra metadata to meta-* files

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 14003 bytes --]

Hello,

> On 6 Jun 2021, at 19:41, Robin Roevens <robin.roevens(a)disroot.org> wrote:
> 
> 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?

Yes, sorry, I missed this, or didn’t know what to say. Feel free to remind me if you didn’t 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 
> 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)
> 
> 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 queries the package database and shows all services correctly.

That sounds clean to me and is re-usable and extensible.

-Michael

> 
> 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.
>