Removing guix-devel. On Tue, Jan 30 2024, Carlo Zancanaro wrote:
> + (define (file-contains? file string) > + (string-contains (call-with-input-file file > + get-string-all) > + string)) > + > + (define (connection-error?) > + (file-contains? "/var/log/letsencrypt/letsencrypt.log" > + "Failed to establish a new connection")) > + > + (let ((script-code 0)) > (for-each > (match-lambda > ((name . command) > (begin > (format #t "Acquiring or renewing certificate: ~a~%" > name) Here we could add ‘(force-output)’, because otherwise those logs arrive after the certbot logs, and it's hard to understand anything. > - (set! code (or (apply system* command) code))))) > - '#$commands) code))))))) > + (unless (zero? (status:exit-val (apply system* command))) > + ;; Certbot errors are always exit code 1, but we'd like > + ;; to separate connection errors from other error > types. > + (if (connection-error?) > + ;; If we have a connection error, then bail early > + ;; with exit code 2. We don't expect this to > + ;; resolve within the timespan of this script. Could we have a (log + force-output) here too? (I imagine within a ‘begin’) > + (exit 2) > + ;; If we have any other type of error, then > continue > + ;; but exit with a failing status code in the end. and here? > + (set! script-code 1)))))) And maybe a log also in case the command succeeds. (So that would mean to replace ‘unless’ with ‘if’). > + '#$commands) > + (exit script-code)))))))) > > + (let loop ((attempt 0)) > + (let ((code (status:exit-val > + (system* #$(certbot-command config))))) > + (cond > + ((and (= code 2) ; Exit code 2 means connection > error > + (< attempt 12)) ; 12 * 10 seconds = 2 minutes ^------ This comment is not true because certbot takes time to execute (around 15s on my vm). I don't think there is a need to be that precise. Maybe you can just add in in the let form, as in (let ((code ...) (max-attempts 12)). > + (sleep 10) > + (loop (1+ attempt))) > + ((zero? code) > + ;; Success! > + #t) > + (else > + ;; Failure. > + #f)))))) Also could you update the example in the docs? >From the doc: >> @defvar certbot-service-type >> A service type for the @code{certbot} Let's Encrypt client. Its value >> must be a @code{certbot-configuration} record as in this example: >> >> @lisp >> (define %certbot-deploy-hook >> (program-file "certbot-deploy-hook.scm" >> (with-imported-modules '((gnu services herd)) >> #~(begin >> (use-modules (gnu services herd)) >> (with-shepherd-action 'nginx ('reload) result result))))) ^ This part isn't useful anymore. However, we could add a nginx-service-type and a dhcp-client-service-type so that people have an idea of what the minimal config is, maybe like I did in my first review: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=46961#23. >> (service certbot-service-type >> (certbot-configuration >> (email "foo@@example.net") >> (certificates >> (list >> (certificate-configuration >> (domains '("example.net" "www.example.net")) >> (deploy-hook %certbot-deploy-hook)) >> (certificate-configuration >> (domains '("bar.example.net"))))))) >> @end lisp We are almost there, thanks! Clément