Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-init s2i extensibility #32

Closed
wants to merge 4 commits into from
Closed

Pre-init s2i extensibility #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38a4fe5
Add s2i pre-init extensibility for 1.10
Oct 18, 2017
977997b
Add s2i pre-init extensibility for 1.12
Oct 18, 2017
2b31881
Add s2i pre-init and default server block extensibility for 1.8
Oct 19, 2017
dc2c012
Fix permission issue
Oct 19, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions 1.10/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ LABEL summary="$SUMMARY" \
version="1.10" \
release="1"

ENV NGINX_CONFIGURATION_PATH=/opt/app-root/etc/nginx.d
ENV NGINX_DEFAULT_CONF_PATH=/opt/app-root/etc/nginx.default.d
ENV NGINX_CONFIGURATION_PATH=/opt/app-root/etc/nginx.d \
NGINX_DEFAULT_CONF_PATH=/opt/app-root/etc/nginx.default.d \
NGINX_CONTAINER_SCRIPTS_PATH=/usr/share/container-scripts/nginx \
NGINX_APP_ROOT=/opt/app-root/

RUN yum install -y yum-utils gettext hostname && \
yum install -y centos-release-scl-rh epel-release && \
Expand All @@ -52,6 +54,8 @@ COPY ./root/ /
RUN sed -i -f /opt/app-root/nginxconf.sed /etc/opt/rh/rh-nginx110/nginx/nginx.conf && \
mkdir -p /opt/app-root/etc/nginx.d/ && \
mkdir -p /opt/app-root/etc/nginx.default.d/ && \
mkdir -p /opt/app-root/etc/nginx-pre-init/ && \
mkdir -p /usr/share/container-scripts/nginx/nginx-pre-init/ && \
chmod -R a+rwx /opt/app-root/etc && \
chmod -R a+rwx /var/opt/rh/rh-nginx110 && \
chown -R 1001:0 /opt/app-root && \
Expand Down
8 changes: 6 additions & 2 deletions 1.10/Dockerfile.rhel7
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ LABEL summary="$SUMMARY" \
version="1.10" \
release="1.2"

ENV NGINX_CONFIGURATION_PATH=/opt/app-root/etc/nginx.d
ENV NGINX_DEFAULT_CONF_PATH=/opt/app-root/etc/nginx.default.d
ENV NGINX_CONFIGURATION_PATH=/opt/app-root/etc/nginx.d \
NGINX_DEFAULT_CONF_PATH=/opt/app-root/etc/nginx.default.d \
NGINX_CONTAINER_SCRIPTS_PATH=/usr/share/container-scripts/nginx \
NGINX_APP_ROOT=/opt/app-root/

# We need to call 2 (!) yum commands before being able to enable repositories properly
# This is a workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1479388
Expand All @@ -57,6 +59,8 @@ COPY ./root/ /
RUN sed -i -f /opt/app-root/nginxconf.sed /etc/opt/rh/rh-nginx110/nginx/nginx.conf && \
mkdir -p /opt/app-root/etc/nginx.d/ && \
mkdir -p /opt/app-root/etc/nginx.default.d/ && \
mkdir -p /opt/app-root/etc/nginx-pre-init/ && \
mkdir -p /usr/share/container-scripts/nginx/nginx-pre-init/ && \
chmod -R a+rwx /opt/app-root/etc && \
chmod -R a+rwx /var/opt/rh/rh-nginx110 && \
chown -R 1001:0 /opt/app-root && \
Expand Down
1 change: 1 addition & 0 deletions 1.10/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ S2I build folder structure:
| :-------------------------- | ----------------------------------------- |
| ./nginx-cfg/*.conf | Should contain all nginx configuration we want to include into image |
| ./nginx-default-cfg/*.conf | Contains any nginx config snippets to include in the default server block |
| ./nginx-pre-init/*.sh | Contains shell scripts that are sourced right before nginx is launched |
| ./ | Should contain nginx application source code |

Environment variables and volumes
Expand Down
31 changes: 31 additions & 0 deletions 1.10/root/usr/share/container-scripts/nginx/common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/bin/sh

# get_matched_files finds file for image extending
function get_matched_files() {
local custom_dir default_dir
custom_dir="$1"
default_dir="$2"
files_matched="$3"
find "$default_dir" -maxdepth 1 -type f -name "$files_matched" -printf "%f\n"
[ -d "$custom_dir" ] && find "$custom_dir" -maxdepth 1 -type f -name "$files_matched" -printf "%f\n"
}

# process_extending_files process extending files in $1 and $2 directories
# - source all *.sh files
# (if there are files with same name source only file from $1)
function process_extending_files() {
local custom_dir default_dir
custom_dir=$1
default_dir=$2
while read filename ; do
if [ $filename ]; then
echo "=> sourcing $filename ..."
# Custom file is prefered
if [ -f $custom_dir/$filename ]; then
source $custom_dir/$filename
elif [ -f $default_dir/$filename ]; then
source $default_dir/$filename
fi
fi
done <<<"$(get_matched_files "$custom_dir" "$default_dir" '*.sh' | sort -u)"
}
10 changes: 10 additions & 0 deletions 1.10/s2i/bin/assemble
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ if [ -d ./nginx-cfg ]; then
cp -v ./nginx-cfg/*.conf "${NGINX_CONFIGURATION_PATH}"
rm -rf ./nginx-cfg
fi
chmod -Rf g+rw ${NGINX_CONFIGURATION_PATH}
fi

if [ -d ./nginx-default-cfg ]; then
Expand All @@ -19,6 +20,15 @@ if [ -d ./nginx-default-cfg ]; then
cp -v ./nginx-default-cfg/*.conf "${NGINX_DEFAULT_CONF_PATH}"
rm -rf ./nginx-default-cfg
fi
chmod -Rf g+rw ${NGINX_DEFAULT_CONF_PATH}
fi

if [ -d ./nginx-pre-init ]; then
echo "---> Copying nginx pre-init scripts..."
if [ "$(ls -A ./nginx-pre-init/*)" ]; then
cp -v ./nginx-pre-init/* "${NGINX_APP_ROOT}/etc/nginx-pre-init"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why to move pre-init scripts there?

I'm not against it, maybe I'm missing something.
Otherwise /opt/app-root/src seems to me as better location that /opt/app-root/etc - the etc is misleading...
But generate_container_user and scl_enable are already in /opt/app-root/etc, so no big harm. But I vote for moving these scripts into NGINX_CONTAINER_SCRIPTS_PATH.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes more sense. I've used this location before, don't know why I chose something else here than NGINX_CONTAINER_SCRIPTS_PATH. Will change that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed this in #41, since I'm not allowed to push to this branch.

rm -rf ./nginx-pre-init
fi
fi

# Fix source directory permissions
Expand Down
6 changes: 6 additions & 0 deletions 1.10/s2i/bin/run
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,10 @@ source /opt/app-root/etc/generate_container_user

set -e

source ${NGINX_CONTAINER_SCRIPTS_PATH}/common.sh

if [ "$(ls -A ${NGINX_APP_ROOT}/etc/nginx-pre-init)" ]; then
process_extending_files ${NGINX_APP_ROOT}/etc/nginx-pre-init ${NGINX_CONTAINER_SCRIPTS_PATH}/nginx-pre-init
fi

exec nginx -g "daemon off;"
8 changes: 8 additions & 0 deletions 1.10/test/pre-init-test-app/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Test NGINX passed</title>
</head>
<body>
<h1>NGINX is working</h1>
</body>
</html>
8 changes: 8 additions & 0 deletions 1.10/test/pre-init-test-app/index2.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Test NGINX passed</title>
</head>
<body>
<h1>NGINX2 is working</h1>
</body>
</html>
8 changes: 8 additions & 0 deletions 1.10/test/pre-init-test-app/nginx-cfg/default.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
server {
listen 8080;
server_name localhost2;
root /opt/app-root/src;
index ${INDEX_FILE};
resolver ${DNS_SERVER};

}
19 changes: 19 additions & 0 deletions 1.10/test/pre-init-test-app/nginx-pre-init/init.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
setup_dns_env_var() {
if [ -z "$DNS_SERVER" ]; then
export DNS_SERVER=`cat /etc/resolv.conf | grep "nameserver " | awk '{print $2}' | tr '\n' ' '`
echo "Using system dns server ${DNS_SERVER}"
else
echo "Using user defined dns server: ${DNS_SERVER}"
fi
}

inject_env_vars() {
## Replace only specified environment variables in specified file.
envsubst '${DNS_SERVER},${INDEX_FILE}' < ${NGINX_CONFIGURATION_PATH}/$1 > output.conf
cp output.conf ${NGINX_CONFIGURATION_PATH}/$1
echo "This is $1: " && cat ${NGINX_CONFIGURATION_PATH}/$1
}

export INDEX_FILE=index2.html
setup_dns_env_var
inject_env_vars "default.conf"
89 changes: 71 additions & 18 deletions 1.10/test/run
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ container_ip() {
}

run_s2i_build() {
s2i build ${s2i_args} file://${test_dir}/test-app ${IMAGE_NAME} ${IMAGE_NAME}-testapp
s2i build ${s2i_args} file://${test_dir}/${1} ${IMAGE_NAME} ${IMAGE_NAME}-${1}
}

prepare() {
Expand All @@ -43,7 +43,7 @@ prepare() {
# TODO: S2I build require the application is a valid 'GIT' repository, we
# should remove this restriction in the future when a file:// is used.
info "Build the test application image"
pushd ${test_dir}/test-app >/dev/null
pushd ${test_dir}/${1} >/dev/null
git init
git config user.email "build@localhost" && git config user.name "builder"
git add -A && git commit -m "Sample commit"
Expand All @@ -52,7 +52,7 @@ prepare() {

run_test_application() {
run_args=${CONTAINER_ARGS:-}
docker run --user=100001 ${run_args} --cidfile=${cid_file} ${IMAGE_NAME}-testapp
docker run --user=100001 ${run_args} --cidfile=${cid_file} ${IMAGE_NAME}-${1}
}

cleanup_test_app() {
Expand All @@ -68,17 +68,17 @@ cleanup_test_app() {

cleanup() {
info "Cleaning up the test application image"
if image_exists ${IMAGE_NAME}-testapp; then
docker rmi -f ${IMAGE_NAME}-testapp
if image_exists ${IMAGE_NAME}-${1}; then
docker rmi -f ${IMAGE_NAME}-${1}
fi
rm -rf ${test_dir}/test-app/.git
rm -rf ${test_dir}/${1}/.git
}

check_result() {
local result="$1"
if [[ "$result" != "0" ]]; then
info "TEST FAILED (${result})"
cleanup
cleanup $2
exit $result
fi
}
Expand Down Expand Up @@ -116,6 +116,18 @@ test_scl_usage() {
echo "ERROR[/bin/bash -c "${run_cmd}"] Expected '${expected}', got '${out}'"
return 1
fi
test_command "$run_cmd" "$expected"
return $?
}

test_command() {
local run_cmd="$1"
local expected="$2"
local message="$3"

if [ $message ]; then
info ${3}
fi
out=$(docker exec $(cat ${cid_file}) /bin/bash -c "${run_cmd}" 2>&1)
if ! echo "${out}" | grep -q "${expected}"; then
echo "ERROR[exec /bin/bash -c "${run_cmd}"] Expected '${expected}', got '${out}'"
Expand All @@ -125,7 +137,9 @@ test_scl_usage() {
if ! echo "${out}" | grep -q "${expected}"; then
echo "ERROR[exec /bin/sh -ic "${run_cmd}"] Expected '${expected}', got '${out}'"
return 1
fi
fi

return 0
}

test_for_output()
Expand Down Expand Up @@ -174,44 +188,83 @@ test_connection() {
return 1
}

test_connection_s2i() {
cat $cid_file
info "Testing the HTTP connection (http://$(container_ip):${test_port})"

if test_for_output "http://$(container_ip):${test_port}/" "NGINX is working" &&
test_for_output "http://$(container_ip):${test_port}/" "NGINX2 is working" localhost2 &&
test_for_output "http://$(container_ip):${test_port}/nginx-cfg/default.conf" "404"; then
return 0
fi

return 1
}

test_application() {
# Verify that the HTTP connection can be established to test application container
run_test_application &
run_test_application "test-app" &

# Wait for the container to write it's CID file
wait_for_cid

test_scl_usage "nginx -v" "nginx version: nginx/1.10."
check_result $?
check_result $? "test-app"

test_connection
check_result $?
check_result $? "test-app"
cleanup_test_app
}

test_pre_init_script() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely +1 for passing app name as argument to functions. So be able to use these functions in more cases.

test_pre_init_script and test_application
test_connection_s2i and test_connection
are very similar. Only functions used for s2i app testing adds 1-2 lines of code.

I suggest:
1. remove cleanup_test_app from test_application and have to call it explicitly after testing of each app finished.
(2. rename test_application to test_application_basics)
3. when testing s2i app use same test_application and before calling cleanup_test_app do

test_command "cat /opt/app-root/etc/nginx.d/default.conf | grep 'resolver' | sed -e 's/resolver\(.*\);/\1/' | grep 'DNS_SERVER'" ""
check_result $? "pre-init-test-app"

test_for_output "http://$(container_ip):${test_port}/aliased/index2.html" "NGINX2 is working"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, will implement this. Just a note, it's the other way around, test_connection has one more line to test. But i got your point, and I'll fix my code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't changed this in #41, because I'd rather see using the test.lib.sh from common repo.

# Verify that the HTTP connection can be established to test application container
run_test_application "pre-init-test-app" &

# Wait for the container to write it's CID file
wait_for_cid

test_command "cat /opt/app-root/etc/nginx.d/default.conf | grep 'resolver' | sed -e 's/resolver\(.*\);/\1/' | grep 'DNS_SERVER'" ""
check_result $? "pre-init-test-app"

test_connection_s2i
check_result $? "pre-init-test-app"
cleanup_test_app

}

cid_file=$(mktemp -u --suffix=.cid)

# Since we built the candidate image locally, we don't want S2I attempt to pull
# it from Docker hub
s2i_args="--force-pull=false"

prepare
run_s2i_build
check_result $?
prepare "test-app"
run_s2i_build "test-app"
check_result $? "test-app"

# Verify the 'usage' script is working properly when running the base image with 's2i usage ...'
test_s2i_usage
check_result $?
check_result $? "test-app"

# Verify the 'usage' script is working properly when running the base image with 'docker run ...'
test_docker_run_usage
check_result $?
check_result $? "test-app"

# Test application with default uid
test_application
test_application

# Test application with random uid
CONTAINER_ARGS="-u 12345" test_application
cleanup
cleanup "test-app"

# Test pre-init script functionality
cid_file=$(mktemp -u --suffix=.cid)

prepare "pre-init-test-app"
run_s2i_build "pre-init-test-app"
check_result $? "pre-init-test-app"

test_pre_init_script
cleanup "pre-init-test-app"

info "All tests finished successfully."
8 changes: 6 additions & 2 deletions 1.12/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ LABEL summary="$SUMMARY" \
name="centos/nginx-112-centos7" \
version="1"

ENV NGINX_CONFIGURATION_PATH=/opt/app-root/etc/nginx.d
ENV NGINX_DEFAULT_CONF_PATH=/opt/app-root/etc/nginx.default.d
ENV NGINX_CONFIGURATION_PATH=/opt/app-root/etc/nginx.d \
NGINX_DEFAULT_CONF_PATH=/opt/app-root/etc/nginx.default.d \
NGINX_CONTAINER_SCRIPTS_PATH=/usr/share/container-scripts/nginx \
NGINX_APP_ROOT=/opt/app-root/

RUN yum install -y yum-utils gettext hostname && \
yum install -y centos-release-scl-rh epel-release && \
Expand All @@ -51,6 +53,8 @@ COPY ./root/ /
RUN sed -i -f /opt/app-root/nginxconf.sed /etc/opt/rh/rh-nginx112/nginx/nginx.conf && \
mkdir -p /opt/app-root/etc/nginx.d/ && \
mkdir -p /opt/app-root/etc/nginx.default.d/ && \
mkdir -p /opt/app-root/etc/nginx-pre-init/ && \
mkdir -p /usr/share/container-scripts/nginx/nginx-pre-init/ && \
chmod -R a+rwx /opt/app-root/etc && \
chmod -R a+rwx /var/opt/rh/rh-nginx112 && \
chown -R 1001:0 /opt/app-root && \
Expand Down
8 changes: 6 additions & 2 deletions 1.12/Dockerfile.rhel7
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ LABEL summary="$SUMMARY" \
name="rhscl/nginx-112-rhel7" \
version="1"

ENV NGINX_CONFIGURATION_PATH=/opt/app-root/etc/nginx.d
ENV NGINX_DEFAULT_CONF_PATH=/opt/app-root/etc/nginx.default.d
ENV NGINX_CONFIGURATION_PATH=/opt/app-root/etc/nginx.d \
NGINX_DEFAULT_CONF_PATH=/opt/app-root/etc/nginx.default.d \
NGINX_CONTAINER_SCRIPTS_PATH=/usr/share/container-scripts/nginx \
NGINX_APP_ROOT=/opt/app-root/

# We need to call 2 (!) yum commands before being able to enable repositories properly
# This is a workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1479388
Expand Down Expand Up @@ -57,6 +59,8 @@ COPY ./root/ /
RUN sed -i -f /opt/app-root/nginxconf.sed /etc/opt/rh/rh-nginx112/nginx/nginx.conf && \
mkdir -p /opt/app-root/etc/nginx.d/ && \
mkdir -p /opt/app-root/etc/nginx.default.d/ && \
mkdir -p /opt/app-root/etc/nginx-pre-init/ && \
mkdir -p /usr/share/container-scripts/nginx/nginx-pre-init/ && \
chmod -R a+rwx /opt/app-root/etc && \
chmod -R a+rwx /var/opt/rh/rh-nginx112 && \
chown -R 1001:0 /opt/app-root && \
Expand Down
1 change: 1 addition & 0 deletions 1.12/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ S2I build folder structure:
| :-------------------------- | ----------------------------------------- |
| ./nginx-cfg/*.conf | Should contain all nginx configuration we want to include into image |
| ./nginx-default-cfg/*.conf | Contains any nginx config snippets to include in the default server block |
| ./nginx-pre-init/*.sh | Contains shell scripts that are sourced right before nginx is launched |
| ./ | Should contain nginx application source code |

Environment variables and volumes
Expand Down
Loading