Recently I needed to run two nginx instances with different users. In FreeBSD's Apache port this can be accomplished with multiple profiles, but stock rc.d script for nginx lacks this functionality. I had added it along the lines of Apache rc.d script. Fix: The following patch adds the needed functionality. How-To-Repeat: Look at the rc.d/nginx: it is not possible to run more that one server with different configuration files.
Responsible Changed From-To: freebsd-ports-bugs->osa Over to maintainer (via the GNATS Auto Assign Tool)
On Tue, Jun 24, 2008 at 06:47:05PM +0400, Eygene Ryabinkin wrote: > > >Submitter-Id: current-users > >Originator: Eygene Ryabinkin > >Organization: Code Labs > >Confidential: no > >Synopsis: [patch] add multiple profiles support to www/nginx rc.d script > >Severity: non-critical > >Priority: medium > >Category: ports > >Class: change-request > >Release: FreeBSD 7.0-STABLE i386 > >Environment: > > System: FreeBSD XXX 7.0-STABLE FreeBSD 7.0-STABLE #16: Tue Mar 18 11:49:55 MSK 2008 root@XXX:/usr/obj/usr/src/sys/XXX i386 > > >Description: > > Recently I needed to run two nginx instances with different users. > In FreeBSD's Apache port this can be accomplished with multiple > profiles, but stock rc.d script for nginx lacks this functionality. > > I had added it along the lines of Apache rc.d script. > > >How-To-Repeat: > > Look at the rc.d/nginx: it is not possible to run more that one > server with different configuration files. > > >Fix: > > The following patch adds the needed functionality. [patch skipped] It doesn't work because pidfile configuration available only from configuration file. So, I contacted with Igor Sysoev and he wrote the patch for 0.7.3 for overwrite global variables in configuration file from command line. Could you test my changes? Index: ports/www/nginx-devel/Makefile =================================================================== RCS file: /home/pcvs/ports/www/nginx-devel/Makefile,v retrieving revision 1.149 diff -u -r1.149 Makefile --- ports/www/nginx-devel/Makefile 23 Jun 2008 11:13:33 -0000 1.149 +++ ports/www/nginx-devel/Makefile 26 Jun 2008 15:18:40 -0000 @@ -7,6 +7,7 @@ PORTNAME= nginx PORTVERSION= 0.7.3 +PORTREVISION= 1 CATEGORIES= www MASTER_SITES= http://sysoev.ru/nginx/ MASTER_SITES+= ${MASTER_SITE_LOCAL} @@ -43,8 +44,10 @@ NGINX_TMPDIR?= ${NGINX_VARDIR}/tmp/nginx HTTP_PORT?= 80 -CONFLICTS?= nginx-0.5.* +CONFLICTS?= nginx-0.6.* USE_RC_SUBR= nginx.sh +SUB_LIST+= RC_SUBR_SUFFIX=${RC_SUBR_SUFFIX} WWWOWN=${WWWOWN} + HAS_CONFIGURE= yes CONFIGURE_ARGS+=--prefix=${ETCDIR} \ --with-cc-opt="-I ${LOCALBASE}/include" \ Index: ports/www/nginx-devel/files/nginx.sh.in =================================================================== RCS file: /home/pcvs/ports/www/nginx-devel/files/nginx.sh.in,v retrieving revision 1.3 diff -u -r1.3 nginx.sh.in --- ports/www/nginx-devel/files/nginx.sh.in 18 Jun 2007 07:13:27 -0000 1.3 +++ ports/www/nginx-devel/files/nginx.sh.in 26 Jun 2008 15:18:40 -0000 @@ -2,43 +2,106 @@ # $FreeBSD: ports/www/nginx-devel/files/nginx.sh.in,v 1.3 2007/06/18 07:13:27 osa Exp $ # PROVIDE: nginx -# REQUIRE: DAEMON -# BEFORE: LOGIN +# REQUIRE: LOGIN cleanvar # KEYWORD: shutdown -# Define these nginx_* variables in one of these files: -# /etc/rc.conf -# /etc/rc.conf.local -# /etc/rc.conf.d/nginx # -# DO NOT CHANGE THESE DEFAULT VALUES HERE -# -nginx_enable=${nginx_enable-"NO"} -nginx_flags=${nginx_flags-""} -nginx_pidfile=${nginx_pidfile-"/var/run/nginx.pid"} +# Add the following lines to /etc/rc.conf to enable nginx: +# nginx_enable (bool): Set to "NO" by default. +# Set it to "YES" to enable nginx +# nginx_profiles (str): Set to "" by default. +# Define your profiles here. +# nginxlimits_enable (bool): Set to "NO" by default. +# Set it to yes to run `limits $limits_args` +# just before nginx starts. +# nginx_flags (str): Set to "" by default. +# Extra flags passed to start command. +# nginxlimits_args (str): Default to "-e -U %%WWWOWN%%" +# Arguments of pre-start limits run. . %%RC_SUBR%% name="nginx" rcvar=`set_rcvar` + +start_precmd="nginx_precmd" +restart_precmd="nginx_checkconfig" +reload_precmd="nginx_checkconfig" +reload_cmd="nginx_reload" +configtest_cmd="nginx_checkconfig" command="%%PREFIX%%/sbin/nginx" +_pidprefix="/var/run/nginx" +pidfile="${_pidprefix}.pid" +required_files=%%PREFIX%%/etc/nginx/nginx.conf + +[ -z "$nginx_enable" ] && nginx_enable="NO" +[ -z "$nginx_profiles" ] && nginx_profiles="" +[ -z "$nginx_flags" ] && nginx_flags="" +[ -z "$nginxlimits_enable" ] && nginxlimits_enable="NO" +[ -z "$nginxlimits_args" ] && nginxlimits_args="-e -U %%WWWOWN%%" load_rc_config $name -pidfile="${nginx_pidfile}" - -extra_commands="configtest reload" - -configtest_cmd="configtest_cmd" -configtest_cmd() { - echo "Configuration syntax test for ${name}." - if ${command} ${nginx_flags} -t; then - : +if [ -n "$2" ]; then + profile="$2" + if [ "x${nginx_profiles}" != "x" ]; then + pidfile="${_pidprefix}.${profile}.pid" + eval nginx_configfile="\${nginx_${profile}_configfile:-}" + if [ "x${nginx_configfile}" = "x" ]; then + echo "You must define a configuration file (nginx_${profile}_configfile)" + exit 1 + fi + required_files="${nginx_configfile}" + eval nginx_enable="\${nginx_${profile}_enable:-${nginx_enable}}" + eval nginx_flags="\${nginx_${profile}_flags:-${nginx_flags}}" + eval nginxlimits_enable="\${nginxlimits_${profile}_enable:-${nginxlimits_enable}}" + eval nginxlimits_args="\${nginxlimits_${profile}_args:-${nginxlimits_args}}" + nginx_flags="-c ${nginx_configfile} -d \"pid ${pidfile};\" ${nginx_flags}" else - err 8 "FATAL: bad config for ${name}" + echo "$0: extra argument ignored" + fi +else + if [ "x${nginx_profiles}" != "x" -a "x$1" != "x" ]; then + for profile in ${nginx_profiles}; do + echo "===> nginx profile: ${profile}" + %%PREFIX%%/etc/rc.d/nginx%%RC_SUBR_SUFFIX%% $1 ${profile} + retcode="$?" + if [ "0${retcode}" -ne 0 ]; then + failed="${profile} (${retcode}) ${failed:-}" + else + success="${profile} ${success:-}" + fi + done + exit 0 + fi +fi + +nginx_requirepidfile() +{ + if [ ! "0`check_pidfile ${pidfile} ${command}`" -gt 1 ]; then + echo "${name} not running? (check $pidfile)." + exit 1 fi } -start_cmd="echo \"Starting ${name}.\"; /usr/bin/limits -U www ${command} ${nginx_flags}" +nginx_checkconfig() +{ + echo "Performing sanity check on nginx configuration:" + eval ${command} ${nginx_flags} -t +} + +nginx_precmd() +{ + nginx_checkconfig + + if checkyesno nginxlimits_enable + then + eval `/usr/bin/limits ${nginxlimits_args}` 2>/dev/null + else + return 0 + fi + +} +extra_commands="reload configtest" run_rc_command "$1" --- /dev/null Thu Jun 26 19:11:00 2008 +++ files/patch-d_param1 Wed Jun 25 22:32:50 2008 @@ -0,0 +1,235 @@ +Index: src/core/ngx_conf_file.c +=================================================================== +--- src/core/ngx_conf_file.c (revision 1383) ++++ src/core/ngx_conf_file.c (working copy) +@@ -58,14 +58,52 @@ + + + char * ++ngx_conf_param(ngx_conf_t *cf) ++{ ++ ngx_str_t *param; ++ ngx_buf_t b; ++ ngx_conf_file_t conf_file; ++ ++ param = &cf->cycle->conf_param; ++ ++ if (param->len == 0) { ++ return NGX_CONF_OK; ++ } ++ ++ ngx_memzero(&conf_file, sizeof(ngx_conf_file_t)); ++ ++ ngx_memzero(&b, sizeof(ngx_buf_t)); ++ ++ b.start = param->data; ++ b.pos = param->data; ++ b.last = param->data + param->len; ++ b.end = b.last; ++ b.temporary = 1; ++ ++ conf_file.file.fd = NGX_INVALID_FILE; ++ conf_file.file.name.data = (u_char *) "command line"; ++ conf_file.line = 1; ++ ++ cf->conf_file = &conf_file; ++ cf->conf_file->buffer = &b; ++ ++ return ngx_conf_parse(cf, NULL); ++} ++ ++ ++char * + ngx_conf_parse(ngx_conf_t *cf, ngx_str_t *filename) + { + char *rv; + ngx_fd_t fd; + ngx_int_t rc; + ngx_buf_t *b; +- ngx_uint_t block; + ngx_conf_file_t *prev; ++ enum { ++ parse_file = 0, ++ parse_block, ++ parse_param ++ } type; + + #if (NGX_SUPPRESS_WARN) + fd = NGX_INVALID_FILE; +@@ -120,10 +158,14 @@ + cf->conf_file->file.log = cf->log; + cf->conf_file->line = 1; + +- block = 0; ++ type = parse_file; + ++ } else if (cf->conf_file->file.fd != NGX_INVALID_FILE) { ++ ++ type = parse_block; ++ + } else { +- block = 1; ++ type = parse_param; + } + + +@@ -145,24 +187,38 @@ + } + + if (rc == NGX_CONF_BLOCK_DONE) { +- if (!block) { ++ ++ if (type != parse_block) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "unexpected \"}\""); + goto failed; + } + +- block = 0; ++ goto done; + } + +- if (rc == NGX_CONF_FILE_DONE && block) { +- ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, +- "unexpected end of file, expecting \"}\""); +- goto failed; +- } ++ if (rc == NGX_CONF_FILE_DONE) { + +- if (rc != NGX_OK && rc != NGX_CONF_BLOCK_START) { ++ if (type == parse_block) { ++ ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, ++ "unexpected end of file, expecting \"}\""); ++ goto failed; ++ } ++ + goto done; + } + ++ if (rc == NGX_CONF_BLOCK_START) { ++ ++ if (cf->conf_file->file.fd == NGX_INVALID_FILE) { ++ ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, ++ "block directives are not supported " ++ "in -d option"); ++ goto failed; ++ } ++ } ++ ++ /* rc == NGX_OK || rc == NGX_CONF_BLOCK_START */ ++ + if (cf->handler) { + + /* +@@ -398,10 +454,19 @@ + for ( ;; ) { + + if (b->pos >= b->last) { ++ + if (cf->conf_file->file.offset + >= ngx_file_size(&cf->conf_file->file.info)) + { + if (cf->args->nelts > 0) { ++ ++ if (cf->conf_file->file.fd == NGX_INVALID_FILE) { ++ ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, ++ "unexpected end of parameter, " ++ "expecting \";\""); ++ return NGX_ERROR; ++ } ++ + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "unexpected end of file, " + "expecting \";\" or \"}\""); +Index: src/core/ngx_conf_file.h +=================================================================== +--- src/core/ngx_conf_file.h (revision 1384) ++++ src/core/ngx_conf_file.h (working copy) +@@ -317,6 +317,7 @@ + #define addressof(addr) ((int) &addr) + + ++char *ngx_conf_param(ngx_conf_t *cf); + char *ngx_conf_parse(ngx_conf_t *cf, ngx_str_t *filename); + + +Index: src/core/nginx.c +=================================================================== +--- src/core/nginx.c (revision 1384) ++++ src/core/nginx.c (working copy) +@@ -637,8 +637,7 @@ + case 'c': + if (argv[i + 1] == NULL) { + ngx_log_error(NGX_LOG_EMERG, cycle->log, 0, +- "the option: \"%s\" requires file name", +- argv[i]); ++ "the option \"-c\" requires file name"); + return NGX_ERROR; + } + +@@ -646,9 +645,20 @@ + cycle->conf_file.len = ngx_strlen(cycle->conf_file.data); + break; + ++ case 'd': ++ if (argv[i + 1] == NULL) { ++ ngx_log_error(NGX_LOG_EMERG, cycle->log, 0, ++ "the option \"-d\" requires parameter"); ++ return NGX_ERROR; ++ } ++ ++ cycle->conf_param.data = (u_char *) argv[++i]; ++ cycle->conf_param.len = ngx_strlen(cycle->conf_param.data); ++ break; ++ + default: + ngx_log_error(NGX_LOG_EMERG, cycle->log, 0, +- "invalid option: \"%s\"", argv[i]); ++ "invalid option: \"-%s\"", argv[i]); + return NGX_ERROR; + } + } +Index: src/core/ngx_cycle.c +=================================================================== +--- src/core/ngx_cycle.c (revision 1384) ++++ src/core/ngx_cycle.c (working copy) +@@ -90,6 +90,16 @@ + old_cycle->conf_file.len + 1); + + ++ cycle->conf_param.len = old_cycle->conf_param.len; ++ cycle->conf_param.data = ngx_pnalloc(pool, old_cycle->conf_param.len); ++ if (cycle->conf_param.data == NULL) { ++ ngx_destroy_pool(pool); ++ return NULL; ++ } ++ ngx_memcpy(cycle->conf_param.data, old_cycle->conf_param.data, ++ old_cycle->conf_param.len); ++ ++ + n = old_cycle->pathes.nelts ? old_cycle->pathes.nelts : 10; + + cycle->pathes.elts = ngx_pcalloc(pool, n * sizeof(ngx_path_t *)); +@@ -238,6 +248,11 @@ + log->log_level = NGX_LOG_DEBUG_ALL; + #endif + ++ if (ngx_conf_param(&conf) != NGX_CONF_OK) { ++ ngx_destroy_cycle_pools(&conf); ++ return NULL; ++ } ++ + if (ngx_conf_parse(&conf, &cycle->conf_file) != NGX_CONF_OK) { + ngx_destroy_cycle_pools(&conf); + return NULL; +Index: src/core/ngx_cycle.h +=================================================================== +--- src/core/ngx_cycle.h (revision 1384) ++++ src/core/ngx_cycle.h (working copy) +@@ -60,6 +60,7 @@ + ngx_cycle_t *old_cycle; + + ngx_str_t conf_file; ++ ngx_str_t conf_param; + ngx_str_t root; + ngx_str_t lock_file; + ngx_str_t hostname; -- ozz
Sergey, good day. Thu, Jun 26, 2008 at 03:21:49PM +0000, Sergey A. Osokin wrote: > [patch skipped] > > It doesn't work because pidfile configuration available > only from configuration file. It works, but only when configuration file specifies the proper pid file ;)) > So, I contacted with Igor > Sysoev and he wrote the patch for 0.7.3 for overwrite > global variables in configuration file from command line. It is not very good if rc.d script will override PID file location: can be a bit time-consuming to find out why PID file is not on the location specified in the configuration file. I used to dig through this situation with Apache trying to find out why newsyslog isn't rotating log files. With my patch administrator can specify the location of PID file via nginx_<profile>_pidfile variable. Though this is arguable, what should be the source of the PID file location. > Could you test my changes? Will try them, thanks. -- Eygene Remember that it is hard to read the on-line manual while single-stepping the kernel. -- FreeBSD Developers handbook
On Thu, Jun 26, 2008 at 07:38:48PM +0400, Eygene Ryabinkin wrote: > Sergey, good day. > > Thu, Jun 26, 2008 at 03:21:49PM +0000, Sergey A. Osokin wrote: > > [patch skipped] > > > > It doesn't work because pidfile configuration available > > only from configuration file. > > It works, but only when configuration file specifies the > proper pid file ;)) What did you do if user don't specify pid in configfile? :-) rc should known where can find pidfile location today. Also, without pid you can't stop process with rc. So, you can run, but can't stop -> doesn't work. -- ozz
Thu, Jun 26, 2008 at 04:04:34PM +0000, Sergey A. Osokin wrote: > On Thu, Jun 26, 2008 at 07:38:48PM +0400, Eygene Ryabinkin wrote: > > It works, but only when configuration file specifies the > > proper pid file ;)) > > What did you do if user don't specify pid in configfile? :-) He should specify it if he wants to run multiple instances of nginx. One PID file can not be used by two processes -- some PID will be lost and it is not very good. > rc should known where can find pidfile location > today. Also, without pid you can't stop process with rc. > So, you can run, but can't stop -> doesn't work. I said that it is arguable ;)) Perhaps the best way to proceed is to detect the situation when configuration file and command-line arguments specify different PID files and report it in the rc.d script. Will try to draft the modifications. -- rea
On Thu, Jun 26, 2008 at 08:13:16PM +0400, Eygene Ryabinkin wrote: > Thu, Jun 26, 2008 at 04:04:34PM +0000, Sergey A. Osokin wrote: > > On Thu, Jun 26, 2008 at 07:38:48PM +0400, Eygene Ryabinkin wrote: > > > It works, but only when configuration file specifies the > > > proper pid file ;)) > > > > What did you do if user don't specify pid in configfile? :-) > > He should specify it if he wants to run multiple instances of nginx. > One PID file can not be used by two processes -- some PID will be > lost and it is not very good. > > > rc should known where can find pidfile location > > today. Also, without pid you can't stop process with rc. > > So, you can run, but can't stop -> doesn't work. > > I said that it is arguable ;)) Perhaps the best way to proceed is > to detect the situation when configuration file and command-line > arguments specify different PID files and report it in the rc.d > script. Will try to draft the modifications. So, I committed my changes with update nginx-devel to 0.7.5. Also, I'd like close this PR. Thank you fo report. -- osa@FreeBSD.org
Sergey, good day. Tue, Jul 01, 2008 at 08:24:26AM +0000, Sergey A. Osokin wrote: > On Thu, Jun 26, 2008 at 08:13:16PM +0400, Eygene Ryabinkin wrote: > > I said that it is arguable ;)) Perhaps the best way to proceed is > > to detect the situation when configuration file and command-line > > arguments specify different PID files and report it in the rc.d > > script. Will try to draft the modifications. > > So, I committed my changes with update nginx-devel to 0.7.5. Also, > I'd like close this PR. Thank you fo report. Tested the modifications -- looks good for me, thanks! Nginx detects the case when PID file location was specified twice and refuses to start, so this satisfies my concerns. PR can be closed. Thanks again! -- Eygene Remember that it is hard to read the on-line manual while single-stepping the kernel. -- FreeBSD Developers handbook
State Changed From-To: open->closed Thank you for report!