From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 18 Feb 2020 15:44:13 +0300 (MSK) From: Ivan Zakharyaschev To: Aleksei Nikiforov In-Reply-To: Message-ID: References: User-Agent: Alpine 2.20 (LFD 67 2015-01-07) MIME-Version: 1.0 Content-Type: multipart/mixed; BOUNDARY="1807885841-1946299884-1582024042=:6363" Content-ID: Cc: devel@lists.altlinux.org, ldv@altlinux.org Subject: Re: [devel] [APT PATCH] test/integration/framework/test-apt-method-http*: use nginx as a forking daemon X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Feb 2020 12:44:13 -0000 Archived-At: List-Archive: List-Post: This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --1807885841-1946299884-1582024042=:6363 Content-Type: text/plain; CHARSET=UTF-8 Content-Transfer-Encoding: 8BIT Content-ID: Hello! On Tue, 18 Feb 2020, Aleksei Nikiforov wrote: > 18.02.2020 06:39, Ivan Zakharyaschev пишет: > > This is needed not to send requests to nginx before it is ready to > > listen: when the main process forks and exits, nginx is considered to > > be ready. > > > > Without this change, I observed a failure in the http test, but not > > the https one, probably, because the delay in the https test was > > larger due to an extra package being built: > > > > Run Testcase (22/29) test-apt-method-http > > Building package: simple-package > > Test for successful execution of apt-get update … > > Err http://localhost x86_64 release > > Could not connect to localhost:8080 (127.0.0.1). - connect (111 > > Connection refused) > > > > However: > > > > Run Testcase (23/29) test-apt-method-https > > Building package: simple-package > > Building package: conflicting-package-one > > Test for successful execution of apt-get update … PASS > > Test that package(s) are not installed with rpm -q simple-package … PASS > > Test for successful execution of apt-get install simple-package … PASS > > Test that package(s) are installed with rpm -q simple-package … PASS > > Pinning invalid key in apt > > Test for failure in execution of apt-get update … PASS > > Test that package(s) are not installed with rpm -q conflicting-package-one … > > PASS > > Test for failure in execution of apt-get install conflicting-package-one … > > PASS > > Test that package(s) are not installed with rpm -q conflicting-package-one … > > PASS > > --- > > test/integration/framework | 2 -- > > test/integration/test-apt-method-http | 5 +++-- > > test/integration/test-apt-method-https | 5 +++-- > > test/integration/test-apt-method-https-invalid-cert-hostname | 5 +++-- > > 4 files changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/test/integration/framework b/test/integration/framework > > index 5f0f58eda..eccfb174e 100644 > > --- a/test/integration/framework > > +++ b/test/integration/framework > > @@ -560,7 +560,6 @@ nginxsetuphttp() { > > cat >> $TMPWORKINGDIRECTORY/nginx/nginx.conf << ENDCONFIG > > worker_processes 1; > > error_log $TMPWORKINGDIRECTORY/nginx/error.log; > > -daemon off; > > pid $TMPWORKINGDIRECTORY/nginx/nginx.pid; > > > > events { > > @@ -606,7 +605,6 @@ nginxsetuphttps() { > > cat > > $TMPWORKINGDIRECTORY/nginx/nginx.conf << ENDCONFIG > > worker_processes 1; > > error_log $TMPWORKINGDIRECTORY/nginx/error.log; > > -daemon off; > > pid $TMPWORKINGDIRECTORY/nginx/nginx.pid; > > > > events { > > diff --git a/test/integration/test-apt-method-http > > b/test/integration/test-apt-method-http > > index 0872c99d4..d463a0d14 100755 > > --- a/test/integration/test-apt-method-http > > +++ b/test/integration/test-apt-method-http > > @@ -17,8 +17,9 @@ rpm http://localhost:8080/ $(getarchitecture) apt-tests > > rpm http://localhost:8080/ noarch apt-tests > > END > > > > -/usr/sbin/nginx -c $TMPWORKINGDIRECTORY/nginx/nginx.conf -p > > $TMPWORKINGDIRECTORY &>> $TMPWORKINGDIRECTORY/nginx/process-stderr.log & > > -NGINXPID=$! > > +/usr/sbin/nginx -c $TMPWORKINGDIRECTORY/nginx/nginx.conf -p > > $TMPWORKINGDIRECTORY &>> $TMPWORKINGDIRECTORY/nginx/process-stderr.log > > +NGINXPID="$(cat $TMPWORKINGDIRECTORY/nginx/nginx.pid)" > > +[ -n "$NGINXPID" ] > > > > addtrap 'prefix' "kill -SIGTERM $NGINXPID; [ \"$EXIT_CODE\" = '0' ] || cat > > $TMPWORKINGDIRECTORY/nginx/process-stderr.log;" > Does nginx always finish starting before actual test runs with these changes? > Or does it sometimes still fail to start in time but produces different error? In my ca. 10 runs, this never happened. (In my 3-4 runs of the previous revision, it always happened.) In my solution, I relied on the description of the behavior of a "traditional UNIX forking daemon" given in man systemd.service: If set to forking it is expected that the process configured with ExecStart= will call fork() as part of its start-up. The parent process is expected to exit when start-up is complete and all communication channels set up. The child continues to run as the main daemon process. This is the behavior of traditional UNIX daemons. If this setting is used, it is recommended to also use the PIDFile= option, so that systemd can identify the main process of the daemon. systemd will proceed starting follow-up units as soon as the parent process exits. I believe that nginx conforms to this specification without further explorations. However, I did practical experiments as stated above. > And I'd prefer to have a verbose error message like 'Nginx failed start in > time' or something else instead of default one. Let's look what the error messages look like now and before my changes. To model it, I did: chmod a-x /usr/sbin/nginx In my revision: Run Testcase (22/29) test-apt-method-http Building package: simple-package E: Looks like the testcases ended prematurely with exitcode: 126 test-apt-method-http ... FAIL Quit short and clear IMHO. In the previous revision, the reason for failure is not caught as early and not reported clearly. The messages are long: Run Testcase (22/29) test-apt-method-http Building package: simple-package Test for successful execution of apt-get update … Err http://localhost x86_64 release Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Err http://localhost noarch release Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Err http://localhost x86_64/apt-tests pkglist Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Err http://localhost x86_64/apt-tests release Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Err http://localhost noarch/apt-tests pkglist Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Err http://localhost noarch/apt-tests release Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Failed to fetch http://localhost:8080/x86_64/base/release Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Failed to fetch http://localhost:8080/noarch/base/release Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Failed to fetch http://localhost:8080/x86_64/base/pkglist.apt-tests Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Failed to fetch http://localhost:8080/x86_64/base/release.apt-tests Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Failed to fetch http://localhost:8080/noarch/base/pkglist.apt-tests Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Failed to fetch http://localhost:8080/noarch/base/release.apt-tests Could not connect to localhost:8080 (127.0.0.1). - connect (111 Connection refused) Reading Package Lists... Building Dependency Tree... W: Release files for some repositories could not be retrieved or authenticated. Such repositories are being ignored. W: You may want to run apt-get update to correct these problems E: Some index files failed to download, they have been ignored, or old ones used instead. FAIL Test that package(s) are not installed with rpm -q simple-package … PASS Test for successful execution of apt-get install simple-package … Reading Package Lists... Building Dependency Tree... E: Couldn't find package simple-package FAIL Test that package(s) are installed with rpm -q simple-package … FAIL ./test-apt-method-http: line 1: kill: (30573) - No such process test-apt-method-http ... FAIL So, I think, after my changes the clearness of the failure messages improved. I'd suggest you can make an additional patch if you'd like to improve the error messages somehow. -- Best regards, Ivan --1807885841-1946299884-1582024042=:6363--