Add s2i-core version #121

omron93 · 2017-05-03T12:44:34Z

Discussed in #115

image name	size	content	used by images
s2i-core	~183MB	rhel7 base + s2i settings	webstack, dbs
s2i-base	~423MB	s2i-core + libraries	perl python, php, ruby

(npm not added yet)

Which rpm packages should be installed in s2i-core and which in s2i-base?

I've choosed findutils, gettext, scl-utils, tar, yum-utils for core version. (installed in by database images or used in s2i scripts)

@bparees @hhorak @jsvgoncalves @torsava @GrahamDumpleton @pkubatrh Please take a look. Which rpms to install in s2i-core?

bparees · 2017-05-03T18:49:01Z

base/Dockerfile

+RUN rpmkeys --import file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-7 && \
+  INSTALL_PKGS="autoconf \
+  automake \
+  bsdtar \


i think you need bsdtar in core (needed for s2i).

Thanks, added.

torsava · 2017-05-11T14:40:20Z

base/test/run

@@ -6,7 +6,7 @@
 # IMAGE_NAME specifies a name of the candidate image used for testing.
 # The image has to be available before this script is executed.
 #
-IMAGE_NAME=${IMAGE_NAME-openshift/base-centos7-candidate}
+IMAGE_NAME=${IMAGE_NAME-centos/s2i-base-centos7-candidate}


Just out of curiosity, what's the reason behind this change?

This image is no longer built to openshift/ namespace. It is built as centos/s2i-base-centos7. This changes was forgotten to be done in #114

Ah, thanks!

torsava · 2017-05-11T15:03:00Z

base/Dockerfile

+# This is the list of basic dependencies that all language Docker image can
+# consume.
+# Also setup the 'openshift' user that is used for the build execution and for the
+# application runtime execution.


The user is not being set in the base so the comment should be removed. Also in tho .rhel7 version

Thanks, updated.

torsava · 2017-05-11T15:10:35Z

base/Dockerfile

+# Also setup the 'openshift' user that is used for the build execution and for the
+# application runtime execution.
+# TODO: Use better UID and GID values
+RUN rpmkeys --import file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-7 && \


I believe this can be omitted since it's already in core.

Thanks, updated.

torsava · 2017-05-11T15:17:12Z

base/Dockerfile

+  which \
+  zlib-devel" && \
+  mkdir -p ${HOME}/.pki/nssdb && \
+  chown -R 1001:0 ${HOME}/.pki && \


I've tested it and it seems these two lines above can also be safely removed from the base.

Thanks, updated.

torsava · 2017-05-11T15:20:33Z

base/Dockerfile.rhel7

+# application runtime execution.
+# TODO: Use better UID and GID values
+RUN yum repolist > /dev/null && \
+  yum-config-manager --enable rhel-7-server-optional-rpms && \


These two lines above should be able to be omitted as well.

Thanks, updated.

torsava · 2017-05-11T15:32:17Z

I've tried building s2i-python-container on top of the new base image and everything went well, all tests are passing. But let's see how dbs do when build against the core.

Just a note: I would have appreciated splitting the change into more commits so the diffs are easier to navigate.

I've made a few comments, however, overall the PR looks good to me. 👍

omron93 · 2017-05-12T08:58:12Z

@torsava Thanks for the review.

I've tried building s2i-python-container on top of the new base image and everything went well, all tests are passing. But let's see how dbs do when build against the core.

Github CI for container images is testing changes also for all images which uses this image as a base. So if CI pass tests for current s2i based images are passing too.

I'll try to test PRs for database images manually.

Just a note: I would have appreciated splitting the change into more commits so the diffs are easier to navigate.

Sorry. I've split it into more commits. I hope it is easier to review now.

pkubatrh · 2017-05-15T07:05:41Z

core/README.md

+`docker exec ... /bin/bash` command, the collection will be automatically enabled.
+
+*Note*:
+The language interpreter executables in the collection (e.g., `ruby`)


As this is s2i-core this should not mention language interpreter executables as none of the language images will be built directly upon s2i-core afaik.

Maybe write it in a general manner, something along the lines "Executables provided by the image layered upon s2i-core might not be present in a directory named in the PATH ..."

Thanks. I've made it more generic.

torsava · 2017-05-15T08:23:06Z

@omron93 Thanks for the commit split, it's much more readable now. And the changes look good as well!

omron93 · 2017-05-15T15:14:57Z

Pull requests with adding s2i support for database images (sclorg/mongodb-container#239 and sclorg/mariadb-container#32) are working with centos/s2i-core-centos7 as the base image.

pkubatrh · 2017-05-16T12:57:40Z

LGTM, however we need to wait with merging this until after we manage to sync up in the rhel space.

torsava · 2017-05-24T14:24:31Z

@omron93 Regarding the movement from the contrib directory structure to the root directory structure as mandated by sclorg/container-common-scripts#8: Should I make a new PR or will you incorporate it here into yours?

omron93 · 2017-05-25T09:59:48Z

Should I make a new PR or will you incorporate it here into yours?

@torsava I can add the change into this PR.

omron93 · 2017-05-25T15:35:13Z

Rebased + contrib moved to /root/opt/app-root.

hhorak · 2017-05-26T07:17:01Z

Sorry, I broke your PR again by merging #118. Anyway, there are two another issues, that make the tests fail:

Failure on this line: https://github.com/omron93/s2i-base-container/blob/core-version/core/Dockerfile#L54

Step 8 : COPY bin/ /usr/bin/
lstat bin/: no such file or directory

RHEL testing failing during squashing, maybe Updated hack/build.sh to use the newer docker-squash tool #110 might help (just re-started the test there to see whether docker-squash works)

omron93 · 2017-05-26T09:48:47Z

Rebased.

Failure on this line: https://github.com/omron93/s2i-base-container/blob/core-version/core/Dockerfile#L54
Step 8 : COPY bin/ /usr/bin/
lstat bin/: no such file or directory

Thanks. It is fixed now.

RHEL testing failing during squashing, maybe #110 might help (just re-started the test there to see whether docker-squash works)

This PR adds using of https://github.com/sclorg/container-common-scripts . So squashing should be updated there. (it needs to be fixed too... hopefully I'll do it next week)

hhorak · 2017-06-28T21:29:15Z

@omron93 It looks like it is a good time for merging, so once you have some time to make the final rebase, please do and merge. Review is done I guess.

Fix missing indentation

omron93 · 2017-06-29T11:45:56Z

Tests are passing. So merging.

omron93 mentioned this pull request May 3, 2017

[Discuss] Common s2i-base-npm layer for builder images (opinions welcome) #115

Closed

bparees reviewed May 3, 2017

View reviewed changes

torsava reviewed May 11, 2017

View reviewed changes

omron93 force-pushed the core-version branch from b808b2f to 279f53a Compare May 12, 2017 08:37

pkubatrh reviewed May 15, 2017

View reviewed changes

omron93 mentioned this pull request May 16, 2017

Avoid errors when the process does not have enough permissions #120

Closed

omron93 force-pushed the core-version branch from dcce98d to 774f621 Compare May 25, 2017 15:34

omron93 force-pushed the core-version branch from 774f621 to 88aec96 Compare May 25, 2017 15:40

omron93 force-pushed the core-version branch 2 times, most recently from c539c79 to cb30e6d Compare May 26, 2017 08:19

torsava mentioned this pull request Jun 14, 2017

Move help.1 file into a root/ subdirectory sclorg/container-common-scripts#8

Merged

Marek Skalický added 4 commits June 29, 2017 10:28

Create s2i-core image version

c34ceb1

Add s2i-base image version

e1e3ea8

Use share build scripts

c0ced00

Update README

803061d

Marek Skalický added 7 commits June 29, 2017 10:28

Move bsdtar to s2i-core

89eb5a2

Remove rendundant things from base Dockerfiles.

ba86609

Fix comments and RHEL labels in core Dockerfiles

269ab87

Update readme to work with shared build scripts

87743d9

Remove old hack/build.sh

5c147d0

Update READMEs to be generic, not only for RHSCL language images

282692f

Move contrib to root/opt/app-root and update common

9e09ce6

Fix missing indentation

omron93 force-pushed the core-version branch 2 times, most recently from 71b5eee to 9e09ce6 Compare June 29, 2017 10:32

omron93 merged commit 798c7a6 into sclorg:master Jun 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add s2i-core version #121

Add s2i-core version #121

omron93 commented May 3, 2017

bparees May 3, 2017

omron93 May 4, 2017

torsava May 11, 2017

omron93 May 12, 2017

torsava May 12, 2017

torsava May 11, 2017

omron93 May 12, 2017

torsava May 11, 2017 •

edited

Loading

omron93 May 12, 2017

torsava May 11, 2017

omron93 May 12, 2017

torsava May 11, 2017

omron93 May 12, 2017

torsava commented May 11, 2017

omron93 commented May 12, 2017

pkubatrh May 15, 2017

omron93 May 15, 2017

torsava commented May 15, 2017

omron93 commented May 15, 2017 •

edited

Loading

pkubatrh commented May 16, 2017

torsava commented May 24, 2017

omron93 commented May 25, 2017

omron93 commented May 25, 2017

hhorak commented May 26, 2017

omron93 commented May 26, 2017

hhorak commented Jun 28, 2017

omron93 commented Jun 29, 2017

Add s2i-core version #121

Add s2i-core version #121

Conversation

omron93 commented May 3, 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

Choose a reason for hiding this comment

torsava May 11, 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

torsava commented May 11, 2017

omron93 commented May 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

torsava commented May 15, 2017

omron93 commented May 15, 2017 • edited Loading

pkubatrh commented May 16, 2017

torsava commented May 24, 2017

omron93 commented May 25, 2017

omron93 commented May 25, 2017

hhorak commented May 26, 2017

omron93 commented May 26, 2017

hhorak commented Jun 28, 2017

omron93 commented Jun 29, 2017

torsava May 11, 2017 •

edited

Loading

omron93 commented May 15, 2017 •

edited

Loading