From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id UFoYM12sxWOINQEAbAwnHQ (envelope-from ) for ; Mon, 16 Jan 2023 20:58:21 +0100 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id 6BwsM12sxWO6VQEA9RJhRA (envelope-from ) for ; Mon, 16 Jan 2023 20:58:21 +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 60F1B3F4AE for ; Mon, 16 Jan 2023 20:58:21 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pHVcL-0005Qi-Pj; Mon, 16 Jan 2023 14:58:05 -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 1pHVcJ-0005QG-Fk for guix-patches@gnu.org; Mon, 16 Jan 2023 14:58:03 -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 1pHVcJ-0006Yl-6I for guix-patches@gnu.org; Mon, 16 Jan 2023 14:58:03 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pHVcI-0008Fl-Ak for guix-patches@gnu.org; Mon, 16 Jan 2023 14:58:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#60788] [PATCH] services: Add vnstat-service-type. Resent-From: Maxim Cournoyer Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Mon, 16 Jan 2023 19:58:02 +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: Bruno Victal Cc: 60788@debbugs.gnu.org Received: via spool by 60788-submit@debbugs.gnu.org id=B60788.167389902331662 (code B ref 60788); Mon, 16 Jan 2023 19:58:02 +0000 Received: (at 60788) by debbugs.gnu.org; 16 Jan 2023 19:57:03 +0000 Received: from localhost ([127.0.0.1]:34916 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pHVbK-0008Eb-MJ for submit@debbugs.gnu.org; Mon, 16 Jan 2023 14:57:03 -0500 Received: from mail-qv1-f50.google.com ([209.85.219.50]:38681) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pHVbG-0008E2-Dy for 60788@debbugs.gnu.org; Mon, 16 Jan 2023 14:57:00 -0500 Received: by mail-qv1-f50.google.com with SMTP id qb7so20206826qvb.5 for <60788@debbugs.gnu.org>; Mon, 16 Jan 2023 11:56:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=76D+9zCiPLi96+MVmkv2aNFVPXmuxrNkDvuXvvJah/E=; b=blLK2Se82EWECyxqse24lfX1/nvKoqQf5z9NHL2/YFPhOjgbgAvY7ZM6bukXS1glwZ IkJsV4ojON8TOhnQiKXomUXJmlNw6T91ATvUSMn/iXi1ZNnTxbXrsZxtYYV6U91FEmCV jNK5D3Wkb0pmVN6KOICex4bwrapi7DiGLXyxUFQ/FTAR+IwVPO/q3L2Czwn6d7NN5cVd L11/dLPt2rd/8IKDMOWRnD2tOsfdZabfWFBVHlo8l/hbI/QE4UtH6sWNkhV8Og8J9Iak Zd4qLEaOLVRk22MgJLxr2Eu8vjt2GpBbgU2ZZDzn9IlK2dvzZmtqPKv+B94TLMlcVDKr 9WIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=76D+9zCiPLi96+MVmkv2aNFVPXmuxrNkDvuXvvJah/E=; b=x2/aLdbvqr/1OWkstj1F106qGVVFH7hQs3LVQGyyxV5CHVDxLqk9QnPOKzeFntVQ28 jiSc0eeQCpK33N1fmY0jBlRYZAk3wIyAc0SMuKsiFz3Dme8VaepqUu6a7tF0vTSNR07y K3J36HrYlAavPKe528wHdUi8b/hP0JvgD8lWbO4T0sF0TpGMbpMtcHZoVL2mx4DgfmkB s3EF/6Pe+Y3TCKYKB1IhYIYtUxi4d3yBDliKSZ+wjSPVZnVevrFL1X58mJfBN/qFzu/A F7xddQl/qeHsRpnp5iO8F4yWrOBZ7/fsHxTaHwQNGukkYyhmA7VtKKkco/NMG+LpdGPM 10hw== X-Gm-Message-State: AFqh2kq1DeKeUrsvWT4I9iI1dMM4f0CU3P+IvHnrOt6xqm5/Cknu2HBN ZJWfsNTQcDUlrqjG2Lu7dOP55kTdGx4I6RPi X-Google-Smtp-Source: AMrXdXvuyDD5EMSYzR7+8zQm1CmqTWvrUdU46NhHF/t0JXnJtON9aLZxnxdUhvYB9V9jOWIVIqCWdg== X-Received: by 2002:a05:6214:3248:b0:532:2c06:d514 with SMTP id mt8-20020a056214324800b005322c06d514mr28839qvb.34.1673899012615; Mon, 16 Jan 2023 11:56:52 -0800 (PST) Received: from hurd (dsl-10-130-209.b2b2c.ca. [72.10.130.209]) by smtp.gmail.com with ESMTPSA id cx15-20020a05620a51cf00b00704c62638f4sm18424362qkb.89.2023.01.16.11.56.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Jan 2023 11:56:52 -0800 (PST) From: Maxim Cournoyer References: <95b646eb6b23dec213cba43b6e4e7ddc4a601d0f.1673640404.git.mirai@makinata.eu> <87o7qy48zm.fsf@gmail.com> <7d22fea8-112d-1c01-6b6a-6cfe4e0fa9e3@makinata.eu> Date: Mon, 16 Jan 2023 14:56:50 -0500 In-Reply-To: <7d22fea8-112d-1c01-6b6a-6cfe4e0fa9e3@makinata.eu> (Bruno Victal's message of "Mon, 16 Jan 2023 19:31:10 +0000") Message-ID: <87fsca45j1.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain 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=1673899101; a=rsa-sha256; cv=none; b=YTo3aKYJtJDnNs6Mi0w55bSTfQ58xp/lSYmdU1El84jszK8CVh0gpmvcIwwyVtgxlo+Qx7 O9tYeSGJaaJblOFORoN/C3s7145IwvQXCRGACaW3VwBsjUJnSRApoms/xB/vpodEZTTzpn YhQNDsUnMzPPqyklVMRRI0PFvMPu3+5kFJrI6npgwsiEt/pvyiFe4g+MCWEZaM5fCh0tpV Qij9sO42ksR/OjsT3ylcqZa40mv2Vb2H1Tt0Nx7DxljJ1jBkUKOaNktrBv0NAI6Zd3PpIT szVi384riPO4GvlYfjgt4neWSKI4as+4ZgXc/XSF/JoW2jfxwMOCgr7uxJRepw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20210112 header.b=blLK2Se8; 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=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1673899101; 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: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: dkim-signature; bh=76D+9zCiPLi96+MVmkv2aNFVPXmuxrNkDvuXvvJah/E=; b=e7q38HJ5eyZNDeYERWR3pctvcNJM31URbwOvwUtCM6b8JQS5ksjnh5Jvmzs4S3UzETaxxk Xcf8QkKOmjw56/08FSK1cY6prfW1VBwyD+thvCxm90vmM1cI43RPQU4bDaMjFerL6pc9Y+ gRhMv2A4mIVt9MfIXip1nv9A90HhfZuMkb8TRqsjUnesR5kc8bCmTL6BC8KmUM07a78PPA Pp7PZ5zUE6xKuJ7/4OLTdQ1woR8p4hNAubXrjxaYKF7mOMZrSlqky+AIdkdOTh5e/0Inqw j5NFV60mL7qcvFYpwDflqazdhLn3He9+Co2SZXtH34+hAYBD+dYUuBeHIKjzKQ== X-Migadu-Spam-Score: -0.53 X-Spam-Score: -0.53 X-Migadu-Queue-Id: 60F1B3F4AE X-Migadu-Scanner: scn1.migadu.com Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20210112 header.b=blLK2Se8; 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=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none) X-TUID: FNs6VoHUu8CS Hi Bruno, Bruno Victal writes: > 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") Ouch. I'm even more in favor of an alternative name now that I know the meaning of 'dock' :-). >>> +;; 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) I think we should take ownership of that text, and update it when it makes sense without necessarily having to keep every details the same with upstream (especially for the double space rule and other conventions); in this view I'd opt to adjust it so the Guix source reads beautifully rather than for diffing against the orginal source :-). If this truly provides value to you (to be able to more easily diff against the troff source), then perhaps how to do so should be detailed in a comment for the next person to understand how this works and why it is this way. >>> + (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'. Thanks! That's news to me :-). >>> + (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. Hm, not nice indeed. Then I guess it's fine without a test. One last thing: in general I think we should try to have our services run in a Linux namespace (unprivileged); have you tried running vnstat in a wrapper produced with least-authority-wrapper (from (guix least-authority)) ? -- Thanks, Maxim