From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp10.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id 4McBBmKmxWMASwAAbAwnHQ (envelope-from ) for ; Mon, 16 Jan 2023 20:32:50 +0100 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp10.migadu.com with LMTPS id 2Cs9BWKmxWPGaQEAG6o9tA (envelope-from ) for ; Mon, 16 Jan 2023 20:32:50 +0100 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 7A5772EEF4 for ; Mon, 16 Jan 2023 20:32:49 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pHVDN-0005rL-1k; Mon, 16 Jan 2023 14:32:17 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pHVDG-0005qe-AW for guix-patches@gnu.org; Mon, 16 Jan 2023 14:32:10 -0500 Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pHVD8-0001Fi-3r for guix-patches@gnu.org; Mon, 16 Jan 2023 14:32:06 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pHVD7-0007dD-Vi for guix-patches@gnu.org; Mon, 16 Jan 2023 14:32:01 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#60788] [PATCH] services: Add vnstat-service-type. Resent-From: Bruno Victal Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 16 Jan 2023 19:32:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 60788 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Maxim Cournoyer Cc: Maxim Cournoyer , 60788@debbugs.gnu.org Received: via spool by 60788-submit@debbugs.gnu.org id=B60788.167389748129288 (code B ref 60788); Mon, 16 Jan 2023 19:32:01 +0000 Received: (at 60788) by debbugs.gnu.org; 16 Jan 2023 19:31:21 +0000 Received: from localhost ([127.0.0.1]:34906 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pHVCS-0007cJ-Oh for submit@debbugs.gnu.org; Mon, 16 Jan 2023 14:31:21 -0500 Received: from smtpmciv4.myservices.hosting ([185.26.107.240]:58092) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pHVCR-0007cC-Hu for 60788@debbugs.gnu.org; Mon, 16 Jan 2023 14:31:20 -0500 Received: from mail1.netim.hosting (unknown [185.26.106.172]) by smtpmciv4.myservices.hosting (Postfix) with ESMTP id 4AC592077D; Mon, 16 Jan 2023 20:31:16 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by mail1.netim.hosting (Postfix) with ESMTP id ECC3980096; Mon, 16 Jan 2023 20:31:15 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at mail1.netim.hosting Received: from mail1.netim.hosting ([127.0.0.1]) by localhost (mail1-1.netim.hosting [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id sUB8Spowd5qJ; Mon, 16 Jan 2023 20:31:15 +0100 (CET) Received: from [192.168.1.239] (unknown [10.192.1.83]) (Authenticated sender: lumen@makinata.eu) by mail1.netim.hosting (Postfix) with ESMTPSA id 6229480093; Mon, 16 Jan 2023 20:31:14 +0100 (CET) Message-ID: <7d22fea8-112d-1c01-6b6a-6cfe4e0fa9e3@makinata.eu> Date: Mon, 16 Jan 2023 19:31:10 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Content-Language: en-US References: <95b646eb6b23dec213cba43b6e4e7ddc4a601d0f.1673640404.git.mirai@makinata.eu> <87o7qy48zm.fsf@gmail.com> From: Bruno Victal In-Reply-To: <87o7qy48zm.fsf@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: guix-patches-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN ARC-Seal: i=1; s=key1; d=yhetil.org; t=1673897569; a=rsa-sha256; cv=none; b=eGKcKkAYc3akGOGxNAzcnM4oyxYE/51fz9vt9sEIBYlXT+lMMG7XSxsn+CI0Gxv9vTIOoQ LrZk3YYD6mn7dQxdQHcpWu9sJefbKS+3c1XtfZ02Q9uaKhxrJJteZc8z4J4Cf6r+B4IVF7 Lh7QJMBsGZmgY+XNHM89219u7VhAKX/fK38Ur3otIdSFmAvWVb6xUBV5cXzFwxCnCAQKuK 1JRnI4TuaGVV2MNXZTV3dMWOTIx39KIriwuZp+9EMh4/BAqNo65O/2qHfOwOk5xu66V9n1 S+3T0b/+6YVSWpzsbMRIPGSHpK5PIJ+iRNos1b7hC+GS+TlFMjW14vmAi5wb+Q== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1673897569; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=7lOy3QgDFD0BCj6zhkVXggYPWIA2yrCnGNF8rheBBr4=; b=e7DKUwIvlw4WNaQDdUDfDPOcul3igB9tRa2/a2OF8RFs2VulNH6uFzJwwMn4TO29w8sN6O gTva7mMP+3d4iPmOjcexsQZ56d0a0QRS6VoV2d1L1NjIV/WlbbOV4dvmlLXwUNHSEzACQf i1cMfLsx08tGlXhS8JCHJN4HUg96alncZ0e/6Bnhgo3UhkV6J2AvNueP/lfw+ZswZZgHI9 5kKm5Kp0REDSS/GxPwtJHVw3aNeRp3ujVs2aCzc303r65BfQQNL/biHMqo+3I3ok+kBrpk OQFTV/AlqvZCIqacB4YzX6o6dxpMmYZt6FQ3mpqLe/7IJeXYanywKVtCh7ZXwg== X-Migadu-Spam-Score: -1.63 X-Spam-Score: -1.63 X-Migadu-Queue-Id: 7A5772EEF4 X-Migadu-Scanner: scn1.migadu.com Authentication-Results: aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=none X-TUID: oBlZ7lHboUj8 On 2023-01-16 18:42, Maxim Cournoyer wrote: >> Bruno Victal writes: >> +;;; >> +;;; vnstat daemon >> +;;; >> + >> +(define* (camelfy-field-name field-name #:key (dromedary? #f)) >> + (match (string-split (symbol->string field-name) #\-) >> + ((head tail ...) >> + (string-join (cons (if dromedary? head (string-upcase head 0 1)) >> + (map string-capitalize tail)) "")))) > > I'd name this pascal-case-field-name, and drop the apparently unused > dromerady? argument (fun, though :-)). If we need it, we could > introduce a 2nd camel-case-field-name. I was anticipating that this snippet would get reused (i.e. copy-pasted) by other services so I wanted to generalize it before it starts spawning its dromedary cousin. If this isn't preferred, I can drop it and just leave it as pascal-case-field-name. (maybe these kinds of procedures could go into a new "configuration utils" module with useful auxiliary functions?) > >> +(define (dock-field-name field-name) >> + "Drop rightmost '?' character" >> + (let ((str (symbol->string field-name))) >> + (if (string-suffix? "?" str) >> + (string->symbol (string-drop-right str 1)) >> + field-name))) > > I'm not sure about the meaning of 'dock' in this procedure name. > Perhaps 'strip-trailing-?-character' would be better? I couldn't think of any other word that would essentially describe whats amounts to "dropping" the tail of a list/string. (here, 'dock': "the practice of cutting off or trimming the tail of an animal") >> +;; Documentation strings from vnstat.conf manpage adapted to texinfo. >> +;; vnstat checkout: v2.10, commit b3408af1c609aa6265d296cab7bfe59a61d7cf70 >> +(define-configuration vnstat-configuration >> + (package >> + (file-like vnstat) >> + "The vnstat package." >> + empty-serializer) >> + >> + (database-dir >> + (string "/var/lib/vnstat") >> + "\ >> +Specifies the directory where the database is to be stored. >> +A full path must be given and a leading '/' isn't required.") >> + >> + (5-minute-hours >> + (maybe-integer 48) >> + "\ >> +Data retention duration for the 5 minute resolution entries. The configuration >> +defines for how many past hours entries will be stored. Set to @code{-1} for >> +unlimited entries or to @code{0} to disable the data collection of this >> +resolution.") >> + >> + (64bit-interface-counters >> + (maybe-integer -2) >> + "\ >> +Select interface counter handling. Set to @code{1} for defining that all interfaces >> +use 64-bit counters on the kernel side and @code{0} for defining 32-bit counter. Set >> +to @code{-1} for using the old style logic used in earlier versions where counter >> +values within 32-bits are assumed to be 32-bit and anything larger is assumed to >> +be a 64-bit counter. This may produce false results if a 64-bit counter is >> +reset within the 32-bits. Set to @code{-2} for using automatic detection based on >> +available kernel datastructures.") > > Please reflow these paragraphs so they fit under the 80 characters width > limit. Perhaps drop the \ escape, as it doesn't provide much and looks > a bit worst, in my opinion. Each sentence should be separated by two > spaces (that's a Texinfo convention). This comment applies to all > similar documentation texts. They're formatted this way as I find it easier to compare against upstream troff sources. If they're reflowed or if the \ escape is dropped then the diffs get larger to compare against (most of the changes here are easier to compare using word-diffs) >> + (always-add-new-interfaces? >> + (maybe-boolean #t) >> + "\ >> +Enable or disable automatic creation of new database entries for interfaces not >> +currently in the database even if the database file already exists when the >> +daemon is started. New database entries will also get created for new interfaces >> +seen while the daemon is running. Pseudo interfaces lo, lo0 and sit0 are always >> +excluded from getting added.") > > The lo, lo0 and sit0 could use a @samp{} or decorator. Noted. > > [...] > >> +(define (vnstat-serialize-configuration config) >> + (mixed-text-file >> + "vnstat.conf" >> + (serialize-configuration config vnstat-configuration-fields))) >> + >> +(define (vnstat-shepherd-service config) >> + (let ((config-file (vnstat-serialize-configuration config))) >> + (match-record config (package pid-file) >> + (shepherd-service >> + (documentation "Run vnstatd.") >> + (requirement `(networking)) >> + (provision '(vnstatd)) >> + (start #~(make-forkexec-constructor >> + (list #$(file-append package "/sbin/vnstatd") >> + "--daemon" >> + "--config" #$config-file) >> + #:pid-file #$pid-file)) >> + (stop #~(make-kill-destructor)) >> + (actions >> + (list (shepherd-configuration-action config-file) > > I don't understand what (shepherd-configuration-action config-file) > corresponds to? It shows the path to the config file with `herd configuration vnstatd'. It was introduced with 'ebc7de6a1efb702fd0364128cbde19697966c4f4'. >> + (shepherd-action >> + (name 'reload) >> + (documentation "Reload vnstatd.") >> + (procedure >> + #~(lambda (pid) >> + (if pid >> + (begin >> + (kill pid SIGHUP) >> + (format #t >> + "Issued SIGHUP to vnstatd (PID ~a)." >> + pid)) >> + (format #t "vnstatd is not running."))))))))))) > >> +(define (vnstat-account-service config) >> + (match-record config (daemon-group daemon-user) >> + (if (every maybe-value-set? (list daemon-group daemon-user)) >> + (list >> + (user-group >> + (name daemon-group) >> + (system? #t)) >> + (user-account >> + (name daemon-user) >> + (group daemon-group) >> + (system? #t) >> + (home-directory "/var/empty") >> + (shell (file-append shadow "/sbin/nologin")))) >> + '()))) >> + >> +(define vnstat-service-type >> + (service-type >> + (name 'vnstat) >> + (description "vnStat network-traffic monitor service.") >> + (extensions >> + (list (service-extension shepherd-root-service-type >> + (compose list vnstat-shepherd-service)) >> + (service-extension account-service-type >> + vnstat-account-service))) >> + (default-value (vnstat-configuration)))) > > The rest LGTM (a system test would be nice, but since the Shepherd > service definition is straightforward, it could come later, when the > need arise). I've thought a bit about the system test though the way vnstat works is that it needs to collect some data and it also takes a while for the daemon to flag it as ready (~5 minutes). This would be a somewhat slow and non-trivial test to write. WDYT? Cheers, Bruno