[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] improve nginx-service
From: |
Ludovic Courtès |
Subject: |
Re: [PATCH] improve nginx-service |
Date: |
Thu, 27 Oct 2016 14:41:18 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Hi!
Julien Lepiller <address@hidden> skribis:
> On Mon, 24 Oct 2016 22:51:27 +0200
> address@hidden (Ludovic Courtès) wrote:
[...]
>> >> I suppose we could make ‘nginx-service-type’ extensible (info
>> >> "(guix) Service Types and Services") so that people can write
>> >> services that define new vhosts?
>> >
>> > You mean something like udev-service-type where you could extend it
>> > with a list of vhosts?
>>
>> Yes, exactly. So for example one could write a service for some
>> high-level Web service that would in turn create an nginx vhost.
>> WDYT?
>
> Sounds great. I tried to work on it, but I'm stuck.
>
> I'm trying to implement a service that extends nginx-service-type with
> a vhost (default-nginx-vhost-service-type), and it creates a root
> directory for a default empty index file. Now I would like to pass the
> absolute path of this directory to the <nginx-vhost-configuration>, but
> it is too early for that. System configuration with this new service
> fails with:
>
> Backtrace:
> In guix/ui.scm:
> 1220: 19 [run-guix-command system "reconfigure" "/etc/config.scm"]
[...]
> ERROR: In procedure string-append:
> ERROR: In procedure string-append: Wrong type (expecting string):
> #<<computed-file> name: "default-nginx-vhost" gexp: #<gexp (begin
> (mkdir #<gexp-output out>) (symlink #<gexp-input #<<plain-file> name:
> "index.html" content: "[...]" references: ()>:out> (string-append
> #<gexp-output out> "/index.html"))) 5959270> options: (#:local-build?
> #t)>
I think the backtrace, ahem, very clearly shows what’s wrong.
(Suddenly I’m scared: what if this backtrace gave you the same feeling
that I have when I get a C++ compilation error?!)
> From 13748b6bfffef19080f4fa3bde2a6ae7d5c8d067 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <address@hidden>
> Date: Wed, 26 Oct 2016 21:33:34 +0200
> Subject: [PATCH] services: Make nginx-service-type extensible
>
> gnu/services/web.scm (nginx-service): Removed.
> gnu/services/web.scm (nginx-service-sytsem): Make extensible.
> gnu/services/web.scm (default-nginx-vhost-service-type): New variable.
What I had in mind was just to add ‘compose’ and ‘extend’ to
‘nginx-service-type’ (this is where we define how extensions are
handled). There’s a bit of extra bookeeping to do, in particular moving
the list of vhosts to <nginx-configuration>, as in this untested patch:
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 59e1e54..a319450 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -27,6 +27,7 @@
#:use-module (gnu packages web)
#:use-module (guix records)
#:use-module (guix gexp)
+ #:use-module (srfi srfi-1)
#:use-module (ice-9 match)
#:export (nginx-configuration
nginx-configuration?
@@ -67,7 +68,9 @@
(nginx nginx-configuration-nginx) ;<package>
(log-directory nginx-configuration-log-directory) ;string
(run-directory nginx-configuration-run-directory) ;string
- (file nginx-configuration-file)) ;string | file-like
+ (file nginx-configuration-file) ;string | file-like
+ (vhosts nginx-configuration-vhosts ;list of
<nginx-vhost-configuration>
+ (default (list (nginx-vhost-configuration)))))
(define (config-domain-strings names)
"Return a string denoting the nginx config representation of NAMES, a list
@@ -148,7 +151,8 @@ of index files."
(define nginx-activation
(match-lambda
- (($ <nginx-configuration> nginx log-directory run-directory config-file)
+ (($ <nginx-configuration> nginx log-directory run-directory
+ config-file vhosts)
#~(begin
(use-modules (guix build utils))
@@ -164,7 +168,10 @@ of index files."
(mkdir-p (string-append #$run-directory "/scgi_temp"))
;; Check configuration file syntax.
(system* (string-append #$nginx "/sbin/nginx")
- "-c" #$config-file "-t")))))
+ "-c" #$(or config-file
+ (default-nginx-config log-directory
+ run-directory vhosts))
+ "-t")))))
(define nginx-shepherd-service
(match-lambda
@@ -192,14 +199,24 @@ of index files."
(service-extension activation-service-type
nginx-activation)
(service-extension account-service-type
- (const %nginx-accounts))))))
+ (const %nginx-accounts))))
+
+ ;; Concatenate the list of vhosts we're given
+ (compose concatenate)
+
+ ;; Append the list of vhosts we were passed to those already
+ ;; in the config.
+ (extend (lambda (config vhosts)
+ (nginx-configuration
+ (inherit config)
+ (vhosts (append (nginx-configuration-vhosts config)
+ vhosts)))))))
(define* (nginx-service #:key (nginx nginx)
(log-directory "/var/log/nginx")
(run-directory "/var/run/nginx")
(vhost-list (list (nginx-vhost-configuration)))
- (config-file
- (default-nginx-config log-directory run-directory
vhost-list)))
+ (config-file #f))
"Return a service that runs NGINX, the nginx web server.
The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
@@ -207,6 +224,7 @@ files in LOG-DIRECTORY, and stores temporary runtime files
in RUN-DIRECTORY."
(service nginx-service-type
(nginx-configuration
(nginx nginx)
+ (vhosts vhost-list)
(log-directory log-directory)
(run-directory run-directory)
(file config-file))))
With that in place, it becomes possible to write another service that
provides additional vhosts, like:
(define vh
(nginx-vhost-configuration …))
(define foo
(service-type (name 'foo)
(extensions (list (service-extension nginx-service-type
(const (list vh)))))))
or simply:
(simple-service 'my-extra-vhost nginx-service-type (list vh))
Does that make sense?
Of course feel free to start from the patch above!
HTH,
Ludo’.