Fix broken CI testing, including from conf.d
#92
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The return status of a loop is that of the last command executed in the loop.
That means previously, the build would not fail during tests unless the last
test script failed. Any/all of the other tests could have failed and the build
would still pass.
This commit does two things:
1) explicitly execute the test file with `... sh $file ...`
2) exit (non-zero) the tests loop on a failing test
…tate As implemented, the `HEALTHCHECK` will check whether the actual `inspircd` process is running in the container by check for open port 6667. This check will run every 1 second (interval), allowing 3 seconds for the actual `CMD` to return (timeout). Failures in the first 60 seconds after startup (start-period) will not count towards an `unhealthy` state. After the start-period, 3 successive failed checks (retries) will result in an `unhealthy` state. A successful check at any time after the start of the container will result in a `healthy` state until 3 successive failed checks. There was once a similar `HEALTHCHECK` but it was unfortunately lost to the work in b00ac85.
The `inspircd/inspircd-extras` repo has moved to `inspircd/inspircd-contrib`.
The custom configuration check in `conf/config.sh` checks for the presence of a `conf.d` directory, but is not very specific as to where that directory is to be expected. Docker users are expected to mount the `conf.d` directory under `$INSPIRCD_ROOT` set in the entrypoint script (`/inspircd`), but because the entrypoint script is running at `/`, the subsequent `exec ...` starts (and stays in) that directory. Thus, `conf.d` is never found. The commit passes the `$INSPIRCD_ROOT` to the `exec`'d `inspircd` allowing the `conf/config.sh` script to (use and) look for `conf.d` in the expected location. Additionally, the working dir is changed to `$INSPIRCD_ROOT` before the `exec ...` call, so that any `<include executable="...`s will be correct path `/inspircd/conf`, as opposed to `/conf` (which is the base config template dir).
The current version of InspIRCd in this image (v3.1.0, as of this commit) only outputs the desired `LINK` notification in debug mode.
With the new multi-stage build system, the linter complains that there are `FROM` commands found at places other than the first line. A `.dockerfilelintrc` file was specific to turn off this rule.
4 tasks
HumorBaby
force-pushed
the
fix-refactor-tests
branch
3 times, most recently
from
4360a0b to
eda6180
Compare
SISheogorath
reviewed
Aug 7, 2019
tests/secrets.sh
Outdated
| @@ -52,7 +52,7 @@ SECRETKEY=$(docker secret create test-secrets-key /tmp/test-secrets-key.pem) | |||
| DOCKERSERVICE=$(docker service create -q -d -p "${CLIENT_PORT}:6667" -p "${TLS_CLIENT_PORT}:6697" --secret source=test-secrets-key,target=inspircd.key --secret source=test-secrets-cert,target=inspircd.crt inspircd:testing) | |||
| sleep 35 | |||
| # Make sure TLS is working | |||
| TLSCHECK=$(echo quit | timeout 10 openssl s_client -ign_eof -connect "localhost:${TLS_CLIENT_PORT}" 2>/dev/null | grep -c "OU=Secret Server Admins") | |||
| TLSCHECK=$(echo quit | timeout 10 openssl s_client -ign_eof -connect "$(cat /etc/hostname):${TLS_CLIENT_PORT}" 2>/dev/null | grep -c "OU\s*=\s*Secret Server Admins") | |||
There was a problem hiding this comment.
Careful, a hostname has to be configured correctly in order to resolve to an IP address. That's not always the case. I would keep that on localhost instead.
There was a problem hiding this comment.
You are certainly correct. I am just having issues with this test on my local machine where connecting openssl through localhost hangs after connect, but works fine if the hostname or non-loopback address is used:
$ openssl s_client -ign_eof -connect localhost:31142 -security_debug
CONNECTED(00000003)
# hangs hereI figured it was normal docker swarm behavior, but I guess not, since the test works fine for travis. Anyway, I fixed up the requested change. The build will still fail though, because of the release of v3.2.0, but https://travis-ci.org/inspircd/inspircd-docker/builds/568864531?utm_source=github_status&utm_medium=notification showed that the build should pass otherwise (when I disable 'fail on newer release available' ).
When docker secrets are used, no dhparams are generated/present (unless the user mounts some). This prevents `inspircd` from starting up. Additionally, some newer versions of openssl now include spaces around the `=` in `OU=...`, so the `grep` pattern was updated.
The previously implementation of the `find ...` pipeline would have led to and error if find was not able to find any `*.conf` files in the `conf.d` directory (because of how globbing happens). Now, an empty string will be piped to the while loop, which will _do nothing_ quietly.
A few tests make temp dirs with `mkdir` without `-p`, which will throw errors if the directory already exists.
HumorBaby
force-pushed
the
fix-refactor-tests
branch
from
a6dabb2 to
a457e8b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR started when I noticed that files in a
conf.ddirectory mounted according to the docs were not being used. In the processes of tracking down the solution, I also noticed that CI tests were not actually doing anything, and in fact, most of them were in a failing state. Both issues are resolved by this PR.*
tests: fix failing `customConfig` testincidentally addresses the issue in #90Checklist
1
tests: fix...commits are required changes for tests to be able to successfully pass.tests: refactor...can be dropped or split off into a new PR if desired, as they are not essential to fixing the main issue of failing tests.2 Technically, the
releasetest fails becausev3.2.0has been released. However, this PR has been successfully tested with both versions.3 No changes to the docs needed.