Add source-to-image support #239

omron93 · 2017-04-13T13:23:59Z

First implementation of s2i support to allow extending this image.

I'll add documentation on Thursday.

@hhorak @praiskup @pkubatrh Please take a look and comment.

omron93 · 2017-04-13T13:31:47Z

[test-openshift]

bparees · 2017-04-13T16:17:29Z

where's the assemble script?

pkubatrh

Missing s2i/bin tree (probably just not added to the commit?) makes the tests fail at build stage.

pkubatrh · 2017-04-18T06:44:00Z

3.2/root/usr/share/container-scripts/mongodb/common.sh

+      source $default_dir/$filename
+    fi
+  done
+  shopt -s nullglob


This should be shopt -u nullglob

Thanks, fixed.

pkubatrh · 2017-04-18T06:44:31Z

3.2/Dockerfile

@@ -27,6 +28,9 @@ RUN yum install -y centos-release-scl && \
    rpm -V $INSTALL_PKGS && \
    yum clean all

+# Copy the S2I scripts from the specific language image to $STI_SCRIPTS_PATH.
+COPY s2i/bin/ $STI_SCRIPTS_PATH


The s2i/bin/ tree is missing from git

You are right. Files are added.

hhorak · 2017-04-19T04:46:48Z

3.2/root/usr/share/container-scripts/mongodb/README.md

+$ s2i build ~/image-configuration/ centos/mongodb-32-centos7 my-mongodb-centos7
+```
+
+The directory passed to `s2i build` can contain these directories:


I'd be more specific here: "...should contain one or more of the following directories:"

hhorak · 2017-04-19T04:51:40Z

3.2/root/usr/share/container-scripts/mongodb/init/50-update-users.sh

@@ -0,0 +1,25 @@
+# update_users creates default users (see usage)
+# if users are already created, updates passwords to match
+# environmental variables


s/environmental/environment/

hhorak · 2017-04-19T04:51:50Z

3.2/root/usr/share/container-scripts/mongodb/README.md

+The directory passed to `s2i build` can contain these directories:
+- `mongodb-cfg/`
+  - when running `run-mongod` or `run-mongod-replication` commands contained `mongod.conf` file is used for `mongod` configuration
+    - `envsubst` command is run on this file to still allow customization of the image using environmental variables


s/environmental/environment/

hhorak · 2017-04-19T04:52:07Z

3.2/root/usr/share/container-scripts/mongodb/pre-init/10-check-env-vars.sh

@@ -0,0 +1,23 @@
+# check_env_vars checks environmental variables


s/environmental/environment/

hhorak · 2017-04-19T05:25:04Z

3.2/root/usr/share/container-scripts/mongodb/common.sh

+  default_dir=$2
+
+  shopt -s nullglob
+  for filename in $(echo $custom_dir/*.sh $default_dir/*.sh | xargs -r basename -a | sort | uniq); do


I'd rather use find command instead of echo $dir | xargs... e.g. find $custom_dir $default_dir -maxdepth 1 -type f -name '*.sh' -printf "%f\n" | sort -u should do similar thing I hope. It will also allow removing the shopt -s nullglob lines.

To really support spaces in filename (if some crazy man does it), we might do something like this:
find "$custom_dir" "$default_dir" -maxdepth 1 -type f -name '*.sh' -printf "%f\n" | sort -u | while read filename ; do ...

When running for example process_extending_files ${APP_DATA}/mongodb-pre-init/ ${CONTAINER_SCRIPTS_PATH}/pre-init/ the user provided directory doesn't have to exist.

With shopt -s nullglob echo $custom_dir/*.sh prints nothing in this case. But find "$custom_dir" returns 1 and prints error message.

@hhorak Would be using find with checking that directories exists better?

Difference is that in current implementation

@omron93 I found this during mariadb PoC as well, this is how I solved it:
https://github.com/sclorg/mariadb-container/pull/32/files#diff-fe50a7af34bff8099d244f9c0cd56273R177
(I can't say that I'm very proud of running similar find command twice, but it works)

hhorak · 2017-04-19T05:32:23Z

3.2/s2i/bin/assemble

+
+shopt -s dotglob
+echo "---> Installing application source ..."
+ls -A /tmp/a/* &>/dev/null && mv /tmp/src/* ./


I'm not sure whether I'm missing something or it is a typo -- shouldn't ls -A /tmp/a/ be rather ls -A /tmp/src/?

You are right. It is typo - fixed.

hhorak · 2017-04-19T05:39:44Z

3.2/root/usr/share/container-scripts/mongodb/README.md

+- `mongodb-init/`
+  - contained shell scripts (`*.sh`) are sourced when `mongod` server is started
+    - in this phase `mongod` server don't have enabled authentication when running `run-mongod` command (for `run-mongod-replication` command configured users are already created and authentication is enabled)
+


I think we should also include a simple example and typical use case for every directory. The use cases might be:

changing a config option based on the environment variable (mongodb-cfg)

checking that admin password is longer than 16 bytes (additional restrictions) (mongodb-pre-init)

storing some initial documents (constants) into the database (mongodb-init)

These examples may be used for both, as a documentation and by the testing script.

OK. I'll add example app configuration. And it should be also used in image tests.

hhorak · 2017-04-19T05:45:48Z

3.2/root/usr/share/container-scripts/mongodb/README.md

+- `mongodb-init/`
+  - contained shell scripts (`*.sh`) are sourced when `mongod` server is started
+    - in this phase `mongod` server don't have enabled authentication when running `run-mongod` command (for `run-mongod-replication` command configured users are already created and authentication is enabled)
+


When sourcing the scripts, some variables are defined and sometimes users may need them in their extension scripts. For example REPLICATION to make things differently for replication use case and non-replication scenarios. I think we should be explicit about which variables users may use this way and consider all the other variables (even if available and set) to be an implementation detail only, so users don't depend on them. I'd start with most necessary variables only and add more when needed later.

Added. Please review.

hhorak

I've provided few in-line comments, but generally this looks very nice IMO. We should gather broader feedback now.

hhorak · 2017-04-19T05:57:39Z

3.2/s2i/bin/usage

+set -o nounset
+set -o pipefail
+
+man -l /help.1


The /help.1 is meant to be formatted for atomic tool that shows the help file using groff tool. Using just pure groff would also limit dependencies, since man requires more deps. So, man -l /help.1 may be replaced by groff -man -ETascii /help.1.

groff-base should also be installed in the Dockerfile, so we make sure it is available.

+1 for using groff instead of man. However we should use groff -t -man -ETascii /help.1 to properly format any tables located in the help file.

Thanks for suggestion. Now should be done this way.

hhorak · 2017-04-19T06:30:50Z

3.2/root/usr/share/container-scripts/mongodb/common.sh

+# process_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_files() {


process_files does not describe the function specifically enough, I'd rather use something like process_extending_files as a function name.

It is better. Renamed.

omron93 · 2017-04-24T13:38:27Z

Rebased.

Changes:

added example and using it in tests
fixed process_extending_files to allow env vars modification
- in old implementation while loop was run in subshell so no env vars chages were possible
mongodb user added to root group
updated README and example README

@bparees @pkubatrh @hhorak Please take a look again.

bparees · 2017-04-24T18:39:25Z

3.2/s2i/bin/assemble

+
+shopt -s dotglob
+echo "---> Installing application source ..."
+ls -A /tmp/src/* &>/dev/null && mv /tmp/src/* ./


what's the point of the ls?

To not to exit with mv error in case that no extending files are present.

Would use mv /tmp/src/* ./ 2>/dev/null || true which is a bit more readable.

@bparees Are files in /tmp/src/ always owned by current images user? (does s2i build always set this?)
If not, mv /tmp/src/* ./ 2>/dev/null could hide useful error message. But it is true that || true is more readable.

Anyway, thanks for suggestions.

we always chmod the files to 666 as we layer them into the container, so permissions should not be a problem.

Changed to what Petr suggested.

omron93 · 2017-05-03T08:45:35Z

[test-openshift]

alikhajeh1 · 2017-06-10T08:50:14Z

Any update on this? It's needed for SNI according to #240

hhorak · 2017-07-11T21:31:54Z

I think the SNI is good enough reason to not wait with this any more, let's merge it after rebasing.

omron93 · 2017-07-20T07:33:52Z

Rebased.

I'll apply these changes to 2.6 and 3.0-pg versions too.
@hhorak @bparees Please review.

omron93 · 2017-07-24T14:40:28Z

[test-openshift]

omron93 · 2017-07-24T15:30:41Z

Images from 744cb94 are accessible in

docker.io/mskalick/mongodb-26-centos7:pr239
docker.io/mskalick/mongodb-30-upg-centos7:pr239
docker.io/mskalick/mongodb-32-centos7:pr239

@pkubatrh @torsava Please review this PR.

bparees · 2017-07-24T16:48:50Z

2.6/s2i/bin/run

@@ -0,0 +1 @@
+/bin/run-mongod


should this take/respect args so you can start it w/ replication?

@bparees What do you mean by this?

In majority of s2i based images run script is used as default image CMD - it is not a mongodb case (it has different default CMD). So I've added this link mainly for "compatibility" reasons and expected that it won't be used. Is the assumption wrong, is something using specially /usr/libexec/run ?

Or is this comment related to #246 ?

yes it's related to #246, but I guess it's ok at least for now. The entrypoint for the user-built image will be "container-entrypoint" still and the default command will be this run script, the user can change the command to "/bin/run-mongod-replication" if they want to use replication.

pvalena · 2017-07-24T19:01:10Z

I can confirm via testing that creating ConfigMaps in OpenShift and then mounting mongod.conf and mongodb.pem from ConfigMaps work as expected.

I have built container from this PR (744cb94), pushed it to local OpenShift cluster(registry) and successfully deployed the container with custom configuration, which enabled ssl (similar to #239 (comment)).

Note that pushing a custom container to the registry required a 'magic' workaround for some unknown reason. I will investigate that further later on.

dongboyan77 · 2017-07-25T10:06:23Z

Hi, I test with image docker.io/mskalick/mongodb-30-upg-centos7:pr239 on ocp 3.6 cluster, find 2 issues:

after changing user name and password via env vars, still can login with old user name and password in new pod,
petset-replicas, after removing all db pods, new pod cannot be running again, the expect result is all db pods are running again

`# oc logs -f mongodb-0
=> sourcing 10-check-env-vars.sh ...
=> sourcing 30-set-config-file.sh ...
=> sourcing 35-setup-default-datadir.sh ...
=> sourcing 40-setup-keyfile.sh ...
=> [Tue Jul 25 09:29:37] Waiting for local MongoDB to accept connections ...
note: noprealloc may hurt performance in many applications
2017-07-25T09:29:38.317+0000 I CONTROL [initandlisten] MongoDB starting : pid=26 port=27017 dbpath=/var/lib/mongodb/data 64-bit host=mongodb-0
2017-07-25T09:29:38.317+0000 I CONTROL [initandlisten] db version v3.0.11
2017-07-25T09:29:38.317+0000 I CONTROL [initandlisten] git version: 48f8b49dc30cc2485c6c1f3db31b723258fcbf39
2017-07-25T09:29:38.317+0000 I CONTROL [initandlisten] build info: Linux c1bg.rdu2.centos.org 2.6.32-573.22.1.el6.x86_64 #1 SMP Wed Mar 23 03:35:39 UTC 2016 x86_64 BOOST_LIB_VERSION=1_53
2017-07-25T09:29:38.317+0000 I CONTROL [initandlisten] allocator: tcmalloc
2017-07-25T09:29:38.317+0000 I CONTROL [initandlisten] options: { config: "/etc/mongod.conf", net: { port: 27017 }, replication: { oplogSizeMB: 64, replSet: "rs0" }, security: { keyFile: "/var/lib/mongodb/keyfile" }, storage: { dbPath: "/var/lib/mongodb/data", mmapv1: { preallocDataFiles: false, smallFiles: true } }, systemLog: { quiet: true } }
2017-07-25T09:29:38.322+0000 W - [initandlisten] Detected unclean shutdown - /var/lib/mongodb/data/mongod.lock is not empty.
2017-07-25T09:29:38.332+0000 I STORAGE [initandlisten] **************
old lock file: /var/lib/mongodb/data/mongod.lock. probably means unclean shutdown,
but there are no journal files to recover.
this is likely human error or filesystem corruption.
please make sure that your journal directory is mounted.
found 2 dbs.
see: http://dochub.mongodb.org/core/repair for more information

2017-07-25T09:29:38.334+0000 I STORAGE [initandlisten] exception in initAndListen: 12596 old lock file, terminating
2017-07-25T09:29:38.334+0000 I CONTROL [initandlisten] dbexit: rc: 100
`

omron93 · 2017-07-25T11:21:57Z

@dongboyan77 Thank you for the testing.

after changing user name and password via env vars, still can login with old user name and password in new pod,

Are you changing only password in new pod?

If you are changing both MONGODB_USER and MONGODB_PASSWORD it is intentional that "old" users in database are preserved.

petset-replicas, after removing all db pods, new pod cannot be running again, the expect result is all db pods are running again

Is this working with centos/mongodb-30-upg-centos7 from Docker Hub?

How can I test it myself?

dongboyan77 · 2017-07-26T03:23:15Z

@omron93 ok, I change both MONGODB_USER and MONGODB_PASSWORD , so it's normal.
and test with centos/mongodb-30-upg-centos7 , the same issue.

I use this template https://github.com/sclorg/mongodb-container/blob/master/examples/petset/mongodb-petset-persistent.yaml

possible to do this using s2i)

omron93 · 2017-07-26T12:11:06Z

Removed section about mounting custom configuration file.
[test-openshift]

and test with centos/mongodb-30-upg-centos7 , the same issue.

OK. Also basic scaling was working for me with this template.

Then it is unrelated to this PR, so I've created new issue #249
@dongboyan77 Please take a look.

This is very similar to the MongoDB PoC: sclorg/mongodb-container#239 It adds extending support using [source-to-image](https://github.com/openshift/source-to-image). For example to build customized MariaDB database image `my-mariadb-centos7` with configuration in `~/image-configuration/` run: ``` $ s2i build ~/image-configuration/ centos/mariadb-101-centos7 my-mariadb-centos7 ``` The directory passed to `s2i build` can contain these directories: - `mysql-cfg/` - when starting the container, files from this directory will be used as a configuration for the `mysqld` daemon - `envsubst` command is run on this file to still allow customization of the image using environmental variables - `mysql-pre-init/` - shell scripts (`*.sh`) available in this directory are sourced before `mysqld` daemon is started - `mysql-init/` - shell scripts (`*.sh`) available in this directory are sourced when `mysqld` daemon is started locally - in this phase, use `${mysql_flags}` to connect to the locally running daemon, for example `mysql $mysql_flags < dump.sql` Variables that can be used in the scripts provided to s2i: - `$mysql_flags` -- arguments for the `mysql` tool that will connect to the locally running `mysqld` during initialization - `$MYSQL_RUNNING_AS_MASTER` -- variable defined when the container is run with `run-mysqld-master` command - `$MYSQL_RUNNING_AS_SLAVE` -- variable defined when the container is run with `run-mysqld-slave` command - `$MYSQL_DATADIR_FIRST_INIT` -- variable defined when the container was initialized from the empty data dir During `s2i build` all provided files are copied into `/opt/app-root/src` directory into the resulting image. If some configuration files are present in the destination directory, files with the same name are overwritten. Also only one file with the same name can be used for customization and user provided files are preferred over default files in `/usr/share/container-scripts/mysql/`- so it is possible to overwrite them. Same configuration directory structure can be used to customize the image every time the image is started using `docker run`. The directory has to be mounted into `/opt/app-root/src/` in the image (`-v ./image-configuration/:/opt/app-root/src/`). This overwrites customization built into the image.

* Implement source-to-image support This is very similar to the MongoDB PoC: sclorg/mongodb-container#239 It adds extending support using [source-to-image](https://github.com/openshift/source-to-image). For example to build customized MariaDB database image `my-mariadb-centos7` with configuration in `~/image-configuration/` run: ``` $ s2i build ~/image-configuration/ centos/mariadb-101-centos7 my-mariadb-centos7 ``` The directory passed to `s2i build` can contain these directories: - `mysql-cfg/` - when starting the container, files from this directory will be used as a configuration for the `mysqld` daemon - `envsubst` command is run on this file to still allow customization of the image using environmental variables - `mysql-pre-init/` - shell scripts (`*.sh`) available in this directory are sourced before `mysqld` daemon is started - `mysql-init/` - shell scripts (`*.sh`) available in this directory are sourced when `mysqld` daemon is started locally - in this phase, use `${mysql_flags}` to connect to the locally running daemon, for example `mysql $mysql_flags < dump.sql` Variables that can be used in the scripts provided to s2i: - `$mysql_flags` -- arguments for the `mysql` tool that will connect to the locally running `mysqld` during initialization - `$MYSQL_RUNNING_AS_MASTER` -- variable defined when the container is run with `run-mysqld-master` command - `$MYSQL_RUNNING_AS_SLAVE` -- variable defined when the container is run with `run-mysqld-slave` command - `$MYSQL_DATADIR_FIRST_INIT` -- variable defined when the container was initialized from the empty data dir During `s2i build` all provided files are copied into `/opt/app-root/src` directory into the resulting image. If some configuration files are present in the destination directory, files with the same name are overwritten. Also only one file with the same name can be used for customization and user provided files are preferred over default files in `/usr/share/container-scripts/mysql/`- so it is possible to overwrite them. Same configuration directory structure can be used to customize the image every time the image is started using `docker run`. The directory has to be mounted into `/opt/app-root/src/` in the image (`-v ./image-configuration/:/opt/app-root/src/`). This overwrites customization built into the image. * Implement source-to-image support This is very similar to the MongoDB PoC: sclorg/mongodb-container#239 It adds extending support using [source-to-image](https://github.com/openshift/source-to-image). For example to build customized MariaDB database image `my-mariadb-centos7` with configuration in `~/image-configuration/` run: ``` $ s2i build ~/image-configuration/ centos/mariadb-101-centos7 my-mariadb-centos7 ``` The directory passed to `s2i build` can contain these directories: - `mysql-cfg/` - when starting the container, files from this directory will be used as a configuration for the `mysqld` daemon - `envsubst` command is run on this file to still allow customization of the image using environmental variables - `mysql-pre-init/` - shell scripts (`*.sh`) available in this directory are sourced before `mysqld` daemon is started - `mysql-init/` - shell scripts (`*.sh`) available in this directory are sourced when `mysqld` daemon is started locally - in this phase, use `${mysql_flags}` to connect to the locally running daemon, for example `mysql $mysql_flags < dump.sql` Variables that can be used in the scripts provided to s2i: - `$mysql_flags` -- arguments for the `mysql` tool that will connect to the locally running `mysqld` during initialization - `$MYSQL_RUNNING_AS_MASTER` -- variable defined when the container is run with `run-mysqld-master` command - `$MYSQL_RUNNING_AS_SLAVE` -- variable defined when the container is run with `run-mysqld-slave` command - `$MYSQL_DATADIR_FIRST_INIT` -- variable defined when the container was initialized from the empty data dir During `s2i build` all provided files are copied into `/opt/app-root/src` directory into the resulting image. If some configuration files are present in the destination directory, files with the same name are overwritten. Also only one file with the same name can be used for customization and user provided files are preferred over default files in `/usr/share/container-scripts/mysql/`- so it is possible to overwrite them. Same configuration directory structure can be used to customize the image every time the image is started using `docker run`. The directory has to be mounted into `/opt/app-root/src/` in the image (`-v ./image-configuration/:/opt/app-root/src/`). This overwrites customization built into the image. * Report tests result properly * Add OpenShift tests File test-lib-openshift.sh is a candidate for container-common-scripts. * Fix test for s2i mount * Update common submodule and use test-lib-openshift.sh from that submodule * Add SSL connection testing * Add self-signed-ssl example

pkubatrh requested changes Apr 18, 2017

View reviewed changes

omron93 force-pushed the s2i-support branch from 60543a9 to 6a08d10 Compare April 18, 2017 16:00

hhorak reviewed Apr 19, 2017

View reviewed changes

hhorak mentioned this pull request Apr 19, 2017

Add example for replication with Docker. #224

Merged

hhorak reviewed Apr 19, 2017

View reviewed changes

hhorak mentioned this pull request Apr 19, 2017

WIP: Implement s2i support sclorg/mariadb-container#32

Closed

omron93 force-pushed the s2i-support branch 4 times, most recently from 3b700e0 to 98ab64f Compare April 24, 2017 10:36

bparees reviewed Apr 24, 2017

View reviewed changes

pkubatrh approved these changes Apr 25, 2017

View reviewed changes

omron93 mentioned this pull request Apr 26, 2017

Having an external route for mongo #240

Closed

omron93 mentioned this pull request May 15, 2017

Add s2i-core version sclorg/s2i-base-container#121

Merged

ecwpz91 mentioned this pull request Jun 14, 2017

MongoDB Archive Install #244

Closed

Marek Skalický added 5 commits July 24, 2017 14:36

Add s2i support to mongodb 2.6

c0b63d4

Add s2i support to mongodb 3.0-upg

2777b65

Use COPY instruction instead of ADD in Dockerfiles

a8ee296

Add s2i support to mongodb 3.2 Fedora Dockerfile

ae83331

Use only stable oc client for OpenShift testing

744cb94

bparees reviewed Jul 24, 2017

View reviewed changes

omron93 mentioned this pull request Jul 26, 2017

petset-replicas - unclear shutdown #249

Closed

Remove section in README about mounting configuration file (it is

6206f81

possible to do this using s2i)

hhorak merged commit 8164d53 into sclorg:master Jul 26, 2017

hhorak mentioned this pull request Jul 26, 2017

make the container easily configurable sclorg/postgresql-container#177

Closed

1 task

This was referenced Sep 18, 2017

Restore Redis dump sclorg/redis-container#19

Closed

Support for env var replacement sclorg/nginx-container#19

Closed

This was referenced Feb 1, 2018

Add support for extending image and enabling SSL connections sclorg/s2i-perl-container#133

Open

Add support for extending image sclorg/redis-container#33

Closed

Add support for extending image and enabling SSL connections sclorg/varnish-container#34

Open

		@@ -0,0 +1,23 @@
		# check_env_vars checks environmental variables

		@@ -0,0 +1 @@
		/bin/run-mongod

Add source-to-image support #239

Add source-to-image support #239

Conversation

omron93 commented Apr 13, 2017

omron93 commented Apr 13, 2017

bparees commented Apr 13, 2017

pkubatrh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hhorak Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hhorak Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hhorak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omron93 commented Apr 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omron93 commented May 3, 2017

alikhajeh1 commented Jun 10, 2017 • edited Loading

hhorak commented Jul 11, 2017

omron93 commented Jul 20, 2017

omron93 commented Jul 24, 2017

omron93 commented Jul 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvalena commented Jul 24, 2017

dongboyan77 commented Jul 25, 2017

omron93 commented Jul 25, 2017

dongboyan77 commented Jul 26, 2017

omron93 commented Jul 26, 2017

hhorak Apr 19, 2017 •

edited

Loading

hhorak Apr 19, 2017 •

edited

Loading

alikhajeh1 commented Jun 10, 2017 •

edited

Loading