This pull request adds support for building and testing s2i-{base,core} in C10S #306

phracek · 2024-07-02T08:59:37Z

This pull request adds support for building and testing container in C10s.

The pull request is separated to commits for better review:

Copy Dockerfile.c9s to Dockerfile.c10s
Fixes in Dockerfile.c10s
Update README's
Update build-and-push action

phracek · 2024-07-02T09:00:47Z

[test]

phracek · 2024-07-04T11:26:28Z

All tests passed :)

github-actions · 2024-07-15T11:49:30Z

Pull Request validation

Success

🟢 CI - All checks have passed
🟢 Review - Reviewed by a member
🟢 Approval - Changes were approved

Auto Merge

Success

🟢 Pull Request is not marked as draft and it's not blocked by dont-merge label
🟢 Pull Request meet requirements, title has correct form
🟢 Pull Request meet requirements, mergeable is true
🟢 Pull Request meet requirements, mergeable_state is clean
🟢 Pull Request has correct target branch master
🟢 Pull Request was merged

hhorak

I don't see any issues with c10s, when comparing to c9s. However, there are some differences in the Dockerfiles when comparing Fedora and c10s and if the changes were introduced to make the Fedora Dockerfile working fine, we might need to go through the same troubles again with c10s later.

So, I think either c10s should adopt some changes from Fedora, or Fedora should adopt some changes from c10s, whatever is correct for each particular difference. For the reference, this is the current diff of Fedora and c10s Dockerfiles:

--- Dockerfile.f40	2024-02-16 13:14:17.445015743 +0100
+++ Dockerfile.c10s	2024-07-15 13:38:56.082301348 +0200
@@ -1,13 +1,10 @@
 # This image is the base image for all s2i configurable container images.
-FROM quay.io/fedora/fedora:40
+FROM quay.io/centos/centos:stream10-development
 
 ENV SUMMARY="Base image which allows using of source-to-image."	\
     DESCRIPTION="The s2i-core image provides any images layered on top of it \
 with all the tools needed to use source-to-image functionality while keeping \
-the image size as small as possible." \
-    NAME=s2i-core \
-    VERSION=40 \
-    ARCH=x86_64
+the image size as small as possible."
 
 LABEL summary="$SUMMARY" \
       description="$DESCRIPTION" \
@@ -15,10 +12,9 @@ LABEL summary="$SUMMARY" \
       io.k8s.display-name="s2i core" \
       io.openshift.s2i.scripts-url=image:///usr/libexec/s2i \
       io.s2i.scripts-url=image:///usr/libexec/s2i \
-      com.redhat.component="$NAME" \
-      name="fedora/$NAME" \
-      version="$VERSION" \
-      usage="This image is supposed to be used as a base image for other images that support source-to-image" \
+      com.redhat.component="s2i-core-container" \
+      name="sclorg/s2i-core-c10s" \
+      version="1" \
       maintainer="SoftwareCollections.org <sclorg@redhat.com>"
 
 ENV \
@@ -29,34 +25,36 @@ ENV \
     APP_ROOT=/opt/app-root \
     # The $HOME is not set by default, but some applications needs this variable
     HOME=/opt/app-root/src \
-    PATH=/opt/app-root/src/bin:/opt/app-root/bin:$PATH \
-    PLATFORM="fedora"
+    PATH=/opt/app-root/src/bin:/opt/app-root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin \
+    PLATFORM="el10"
 
 # This is the list of basic dependencies that all language container image can
 # consume.
 # 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 INSTALL_PKGS="bsdtar \
   findutils \
-  gettext \
-  glibc-langpack-en \
   groff-base \
+  glibc-locale-source \
+  glibc-langpack-en \
+  gettext \
   rsync \
+  scl-utils \
   tar \
-  unzip" && \
+  unzip \
+  xz \
+  yum" && \
   mkdir -p ${HOME}/.pki/nssdb && \
   chown -R 1001:0 ${HOME}/.pki && \
-  dnf install -y --setopt=tsflags=nodocs $INSTALL_PKGS && \
+  yum install -y --setopt=tsflags=nodocs $INSTALL_PKGS && \
   rpm -V $INSTALL_PKGS && \
-  dnf clean all -y
+  yum -y clean all --enablerepo='*'
 
 # Copy extra files to the image.
 COPY ./core/root/ /
 
-# Create a platform-python symlink if it does not exist already
-RUN [ -e /usr/libexec/platform-python ] || ln -s /usr/bin/python3 /usr/libexec/platform-python
-
 # Directory with the sources is set as the working directory so all STI scripts
 # can execute relative to this path.
 WORKDIR ${HOME}

phracek · 2024-07-16T12:09:30Z

I don't see any issues with c10s, when comparing to c9s. However, there are some differences in the Dockerfiles when comparing Fedora and c10s and if the changes were introduced to make the Fedora Dockerfile working fine, we might need to go through the same troubles again with c10s later.

So, I think either c10s should adopt some changes from Fedora, or Fedora should adopt some changes from c10s, whatever is correct for each particular difference. For the reference, this is the current diff of Fedora and c10s Dockerfiles:

--- Dockerfile.f40	2024-02-16 13:14:17.445015743 +0100
+++ Dockerfile.c10s	2024-07-15 13:38:56.082301348 +0200
@@ -1,13 +1,10 @@
 # This image is the base image for all s2i configurable container images.
-FROM quay.io/fedora/fedora:40
+FROM quay.io/centos/centos:stream10-development
 
 ENV SUMMARY="Base image which allows using of source-to-image."	\
     DESCRIPTION="The s2i-core image provides any images layered on top of it \
 with all the tools needed to use source-to-image functionality while keeping \
-the image size as small as possible." \
-    NAME=s2i-core \
-    VERSION=40 \
-    ARCH=x86_64
+the image size as small as possible."
 
 LABEL summary="$SUMMARY" \
       description="$DESCRIPTION" \
@@ -15,10 +12,9 @@ LABEL summary="$SUMMARY" \
       io.k8s.display-name="s2i core" \
       io.openshift.s2i.scripts-url=image:///usr/libexec/s2i \
       io.s2i.scripts-url=image:///usr/libexec/s2i \
-      com.redhat.component="$NAME" \
-      name="fedora/$NAME" \
-      version="$VERSION" \
-      usage="This image is supposed to be used as a base image for other images that support source-to-image" \
+      com.redhat.component="s2i-core-container" \
+      name="sclorg/s2i-core-c10s" \
+      version="1" \
       maintainer="SoftwareCollections.org <sclorg@redhat.com>"
 
 ENV \
@@ -29,34 +25,36 @@ ENV \
     APP_ROOT=/opt/app-root \
     # The $HOME is not set by default, but some applications needs this variable
     HOME=/opt/app-root/src \
-    PATH=/opt/app-root/src/bin:/opt/app-root/bin:$PATH \
-    PLATFORM="fedora"
+    PATH=/opt/app-root/src/bin:/opt/app-root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin \
+    PLATFORM="el10"
 
 # This is the list of basic dependencies that all language container image can
 # consume.
 # 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 INSTALL_PKGS="bsdtar \
   findutils \
-  gettext \
-  glibc-langpack-en \
   groff-base \
+  glibc-locale-source \
+  glibc-langpack-en \
+  gettext \
   rsync \
+  scl-utils \
   tar \
-  unzip" && \
+  unzip \
+  xz \
+  yum" && \
   mkdir -p ${HOME}/.pki/nssdb && \
   chown -R 1001:0 ${HOME}/.pki && \
-  dnf install -y --setopt=tsflags=nodocs $INSTALL_PKGS && \
+  yum install -y --setopt=tsflags=nodocs $INSTALL_PKGS && \
   rpm -V $INSTALL_PKGS && \
-  dnf clean all -y
+  yum -y clean all --enablerepo='*'
 
 # Copy extra files to the image.
 COPY ./core/root/ /
 
-# Create a platform-python symlink if it does not exist already
-RUN [ -e /usr/libexec/platform-python ] || ln -s /usr/bin/python3 /usr/libexec/platform-python
-
 # Directory with the sources is set as the working directory so all STI scripts
 # can execute relative to this path.
 WORKDIR ${HOME}

This should be fixed by 8998857

phracek · 2024-07-16T12:32:32Z

$ diff core/Dockerfile.c10s core/Dockerfile.f40
2c2
< FROM quay.io/centos/centos:stream10-development
---
> FROM quay.io/fedora/fedora:40
9c9,10
<     VERSION=10
---
>     VERSION=40 \
>     ARCH=x86_64
17,18c18,19
<       com.redhat.component="$NAME-container" \
<       name="sclorg/$NAME-c$VERSIONs" \
---
>       com.redhat.component="$NAME" \
>       name="fedora/$NAME" \
19a21
>       usage="This image is supposed to be used as a base image for other images that support source-to-image" \
30,31c32,33
<     PATH=/opt/app-root/src/bin:/opt/app-root/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin \
<     PLATFORM="el10"
---
>     PATH=/opt/app-root/src/bin:/opt/app-root/bin:$PATH \
>     PLATFORM="fedora"
38d39
<
44d44
<   glibc-locale-source \
46d45
<   scl-utils \
48,50c47
<   unzip \
<   xz \
<   yum" && \
---
>   unzip" && \
55c52
<   dnf -y clean all --enablerepo='*'
---
>   dnf clean all -y
59a57,59
> # Create a platform-python symlink if it does not exist already
> RUN [ -e /usr/libexec/platform-python ] || ln -s /usr/bin/python3 /usr/libexec/platform-python
>

$ diff core/Dockerfile.c10s core/Dockerfile.c9s
2c2
< FROM quay.io/centos/centos:stream10-development
---
> FROM quay.io/centos/centos:stream9
8,9c8
<     NAME=s2i-core \
<     VERSION=10
---
>     VERSION=9
17,19c16,18
<       com.redhat.component="$NAME-container" \
<       name="sclorg/$NAME-c$VERSIONs" \
<       version="$VERSION" \
---
>       com.redhat.component="s2i-core-container" \
>       name="sclorg/s2i-core-c9s" \
>       version="1" \
31c30
<     PLATFORM="el10"
---
>     PLATFORM="el9"

phracek · 2024-07-16T13:02:28Z

[test]

Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

Update name in LABEL. Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

@hhorak

as suggested by @hhorak. Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

phracek · 2024-07-17T06:39:27Z

rebased against master

[test]

hhorak · 2024-07-17T12:55:12Z

There are still differences that I don't understand why they should be there:
Core:

--- Dockerfile.f40	2024-07-15 13:38:40.012265088 +0200
+++ Dockerfile.c10s	2024-07-17 14:19:30.916921466 +0200
<...snipped...>
 RUN INSTALL_PKGS="bsdtar \
   findutils \
   gettext \
   glibc-langpack-en \
   groff-base \
+  glibc-locale-source \
   rsync \
+  scl-utils \
   tar \
-  unzip" && \
+  unzip \
+  xz \
+  yum" && \

Base:

--- Dockerfile.f40	2024-07-15 13:38:40.012265088 +0200
+++ Dockerfile.c10s	2024-07-17 14:19:30.916921466 +0200
<...snipped...>
-RUN INSTALL_PKGS="autoconf \
+RUN INSTALL_PKGS="nodejs autoconf \
   automake \
   bzip2 \
   gcc-c++ \
@@ -34,15 +35,17 @@ RUN INSTALL_PKGS="autoconf \
   lsof \
   make \
   mariadb-connector-c-devel \
-  nodejs-npm \
   openssl-devel \
   patch \
   procps-ng \
+  nodejs-npm \
+  redhat-rpm-config \
   sqlite-devel \
   unzip \
-  wget2 \
+  wget2-wget \
   which \
   zlib-ng-compat-devel" && \
   dnf install -y --setopt=tsflags=nodocs $INSTALL_PKGS && \
   rpm -V $INSTALL_PKGS && \
-  dnf clean all -y
+  node -v | grep -qe "^v$NODEJS_VER\." && echo "Found VERSION $NODEJS_VER" && \
+  dnf -y clean all --enablerepo='*'

IOW, we install those packages in c10s, while not in F40:

glibc-locale-source, scl-utils, xz, and yum in core
nodejs and redhat-rpm-config in s2i-base

I guess nodejs would be pulled as a dependency of npm, so could be omitted without any real change. If it's the case for other packages as well and we actually need them (while depending on dnf to pull them in in Fedora), we should add them explicitly to Fedora's dockerfile, or remove from c10s otherwise.
At least scl-utils could likely be removed entirely.

phracek · 2024-07-17T15:10:28Z

@hhorak Regarding redhat-rpm-config was added by request from here: sclorg/s2i-python-container#346

glibc-locale-source was added from the beginning of Dockerfile.rhel8 here: ceec37c and I guess this is not needed.

hhorak · 2024-07-18T13:53:45Z

Thanks for the analysis, I found only one mention of xz in this thread: #177 (comment) but it does not give a hint whether it's needed or not.

Since presence of a package might determine need for security fixes with a higher standard, I'd prefer not including packages if not necessary.

yum is a symlink-only package, so I don't expect any security issues there, and it might be very convenient for users to have it, due to backward compatibility.

That means, I'd propose removing glibc-locale-source, scl-utils, and xz and only re-add them when we find the real need.

In case they are missing for some reason, we can add it later on. Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

phracek · 2024-07-19T11:47:55Z

That means, I'd propose removing glibc-locale-source, scl-utils, and xz and only re-add them when we find the real need.

It were removed by commit 5b77796

phracek · 2024-07-19T11:47:59Z

[test]

phracek · 2024-07-22T10:51:05Z

CentOS 10 failures are expected. See https://issues.redhat.com/browse/TFT-2726.

hhorak

I see no further questions about packages installed in Fedora vs. c10s, all looks good to me now.

phracek added the ready for review The pull request is ready to review label Jul 2, 2024

This was referenced Jul 2, 2024

Add support for building, testing C10S images sclorg/container-common-scripts#377

Closed

Add support for testing C10S by GitHub Action #305

Merged

phracek added the easyfix label Jul 3, 2024

phracek requested review from hhorak, pkubatrh and SlouchyButton July 3, 2024 12:50

github-actions bot added the pr/missing-review label Jul 15, 2024

hhorak reviewed Jul 15, 2024

View reviewed changes

hhorak self-assigned this Jul 15, 2024

github-actions bot added the pr/failing-ci label Jul 16, 2024

phracek force-pushed the building_testing_c10s branch from 8998857 to 9e0ab10 Compare July 16, 2024 12:30

github-actions bot removed the pr/failing-ci label Jul 16, 2024

phracek force-pushed the building_testing_c10s branch 2 times, most recently from 9a8747a to 03bb3ee Compare July 16, 2024 13:02

github-actions bot added pr/failing-ci and removed pr/failing-ci labels Jul 16, 2024

phracek added 5 commits July 17, 2024 08:38

Add copy c10s from c9s

571e91d

Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

Fix c10s proper package names.

50c17d4

Update name in LABEL. Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

Update README with support C10S

6245f48

Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

Update build-and-push action and add the latest container-common-scripts

acd32fb

Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

United F40, C10S and C9S a couple bit more

65bc079

as suggested by @hhorak. Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

phracek force-pushed the building_testing_c10s branch from 03bb3ee to 65bc079 Compare July 17, 2024 06:39

github-actions bot added pr/failing-ci and removed pr/failing-ci labels Jul 17, 2024

Let's remove packages glibc-locale-resource, scl-utils and xz.

5b77796

In case they are missing for some reason, we can add it later on. Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>

github-actions bot added the pr/failing-ci label Jul 19, 2024

hhorak approved these changes Jul 22, 2024

View reviewed changes

github-actions bot removed pr/missing-review pr/failing-ci labels Jul 22, 2024

github-actions bot merged commit 9b9fbd5 into master Jul 24, 2024
11 checks passed

github-actions bot deleted the building_testing_c10s branch July 24, 2024 07:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

This pull request adds support for building and testing s2i-{base,core} in C10S #306

This pull request adds support for building and testing s2i-{base,core} in C10S #306

phracek commented Jul 2, 2024 •

edited by github-actions bot

Loading

phracek commented Jul 2, 2024

phracek commented Jul 4, 2024

github-actions bot commented Jul 15, 2024 •

edited

Loading

hhorak left a comment

phracek commented Jul 16, 2024

phracek commented Jul 16, 2024

phracek commented Jul 16, 2024

phracek commented Jul 17, 2024

hhorak commented Jul 17, 2024

phracek commented Jul 17, 2024

hhorak commented Jul 18, 2024

phracek commented Jul 19, 2024

phracek commented Jul 19, 2024

phracek commented Jul 22, 2024

hhorak left a comment

This pull request adds support for building and testing s2i-{base,core} in C10S #306

This pull request adds support for building and testing s2i-{base,core} in C10S #306

Conversation

phracek commented Jul 2, 2024 • edited by github-actions bot Loading

phracek commented Jul 2, 2024

phracek commented Jul 4, 2024

github-actions bot commented Jul 15, 2024 • edited Loading

Pull Request validation

Success

Auto Merge

Success

hhorak left a comment

Choose a reason for hiding this comment

phracek commented Jul 16, 2024

phracek commented Jul 16, 2024

phracek commented Jul 16, 2024

phracek commented Jul 17, 2024

hhorak commented Jul 17, 2024

phracek commented Jul 17, 2024

hhorak commented Jul 18, 2024

phracek commented Jul 19, 2024

phracek commented Jul 19, 2024

phracek commented Jul 22, 2024

hhorak left a comment

Choose a reason for hiding this comment

phracek commented Jul 2, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Jul 15, 2024 •

edited

Loading