Skip to content

add distgen support#437

Open
ndavidova wants to merge 4 commits into
masterfrom
ndavidov/distgen
Open

add distgen support#437
ndavidova wants to merge 4 commits into
masterfrom
ndavidov/distgen

Conversation

@ndavidova

@ndavidova ndavidova commented May 28, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@ndavidova

Copy link
Copy Markdown
Contributor Author

[test]

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

Pull Request validation

Failed

🔴 Failed or pending statuses:

  • Testing Farm - RHEL9 - Unsubscribed host - 8.0[error]
  • Testing Farm - RHEL9 - 8.4[error]
  • Testing Farm - Fedora - 8.4[error]

🔴 Review - Missing review from a member (1 required)


Triggered by Workflow Run

@github-actions

Copy link
Copy Markdown

Testing Farm results

namecomposearchstatusstarted (UTC)timelogs
Fedora - 8.4Fedora-latestx86_64✅ passed28.05.2026 13:02:5310min 10stest pipeline

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

Testing Farm results

namecomposearchstatusstarted (UTC)timelogs
Fedora - 8.0Fedora-latestx86_64✅ passed01.06.2026 10:09:069min 53stest pipeline
CentOS Stream 9 - 8.4CentOS-Stream-9x86_64✅ passed01.06.2026 10:09:0615min 26stest pipeline
CentOS Stream 10 - 8.4CentOS-Stream-10x86_64✅ passed01.06.2026 10:09:0514min 15stest pipeline
RHEL10 - Unsubscribed host - 8.4RHEL-10.2-Nightlyx86_64✅ passed01.06.2026 10:09:0453min 14stest pipeline
RHEL10 - 8.4RHEL-10.2-Nightlyx86_64✅ passed01.06.2026 10:09:0553min 51stest pipeline
RHEL8 - 8.0RHEL-8.10.0-Nightlyx86_64❌ error01.06.2026 10:09:0541min 27stest pipeline
RHEL9 - Unsubscribed host - 8.0RHEL-9.8.0-Nightlyx86_64✅ passed01.06.2026 10:09:0655min 59stest pipeline
RHEL9 - Unsubscribed host - 8.4RHEL-9.8.0-Nightlyx86_64✅ passed01.06.2026 10:09:0556min 18stest pipeline
RHEL9 - 8.0RHEL-9.8.0-Nightlyx86_64✅ passed01.06.2026 10:09:0456min 32stest pipeline
RHEL9 - 8.4RHEL-9.8.0-Nightlyx86_64✅ passed01.06.2026 10:09:0556min 29stest pipeline
Fedora - 8.4Fedora-latestx86_64✅ passed09.06.2026 13:49:5813min 12stest pipeline
CentOS Stream 9 - 8.0CentOS-Stream-9x86_64✅ passed01.06.2026 10:09:0813min 48stest pipeline

@ndavidova

Copy link
Copy Markdown
Contributor Author

[test]

@ndavidova ndavidova marked this pull request as ready for review May 28, 2026 15:05
@ndavidova ndavidova requested review from phracek and pkhartsk and removed request for pkhartsk May 29, 2026 11:28
@pkhartsk

pkhartsk commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Would it also make sense to include the s2i-common directories in the sources? Or just make them symlinks to the one already present in the project root

@pkhartsk pkhartsk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Found some possible simplifications of the (IMO) very complicated src/Dockerfile

Comment thread src/Dockerfile Outdated
Comment on lines +65 to +69
{% if spec.prod == 'fedora' %}# This image must forever use UID 27 for mysql user so our volumes are safe in the future. This should *never* change.
# Instead of relying on the DB server package, we will do the setup ourselves before any package is installed
{% else %}# This image must forever use UID 27 for mysql user so our volumes are
# safe in the future. This should *never* change, the last test is there
# to make sure of that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems useless to do this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm aware that this would cause the dockerfiles to differ cosmetically, but functionally they remain the same, and this is just pretty much the same comment...

Comment thread src/Dockerfile Outdated
io.k8s.display-name="MySQL ${MYSQL_VERSION}" \
io.openshift.expose-services="3306:mysql" \
io.openshift.tags="database,${NAME},${NAME}${MYSQL_SHORT_VERSION}{% if spec.prod == 'fedora' and spec.version == '8.0' %},rh-${NAME}${MYSQL_SHORT_VERSION}{% elif spec.prod != 'fedora' %},${NAME}-${MYSQL_SHORT_VERSION}{% endif %}" \
com.redhat.component="{% if spec.prod == 'rhel8' or spec.prod == 'rhel9' or spec.prod == 'rhel10' or spec.prod == 'c9s' or spec.prod == 'c10s' %}${NAME}-${MYSQL_SHORT_VERSION}-container{% else %}${NAME}{% endif %}" \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can be simplified to if spec.prod != 'fedora'

Comment thread src/Dockerfile
com.redhat.component="{% if spec.prod == 'rhel8' or spec.prod == 'rhel9' or spec.prod == 'rhel10' or spec.prod == 'c9s' or spec.prod == 'c10s' %}${NAME}-${MYSQL_SHORT_VERSION}-container{% else %}${NAME}{% endif %}" \
name="{{ spec.img_name }}" \
{% if spec.prod == 'fedora' %} version="${MYSQL_VERSION}" \
{% elif spec.prod != 'rhel10' %} version="1" \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This makes no sense to me, why would the version be 1? Shouldn't it always be MYSQL_VERSION? Guessing this is a mistake in the .rhelx Dockerfiles?

Comment thread src/Dockerfile
Comment on lines +58 to +59
{% endif %}{% if spec.license_terms %} com.redhat.license_terms="{{ spec.license_terms }}" \
{% endif %} usage="{% if spec.prod == 'c9s' or spec.prod == 'c10s' %}podman run -d -e MYSQL_USER=user -e MYSQL_PASSWORD=pass -e MYSQL_DATABASE=db -p 3306:3306 quay.io/{{ spec.img_name }}:{{ spec.prod }}{% elif spec.prod == 'fedora' %}docker run -d -e MYSQL_USER=user -e MYSQL_PASSWORD=pass -e MYSQL_DATABASE=db -p 3306:3306 quay.io/{{ spec.img_name }}{% else %}podman run -d -e MYSQL_USER=user -e MYSQL_PASSWORD=pass -e MYSQL_DATABASE=db -p 3306:3306 {{ spec.img_name }}{% endif %}" \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: these endif's at the starts of lines are ugly

Comment thread src/Dockerfile
Comment on lines +78 to +97
{% elif spec.prod == 'rhel8' and spec.version == '8.0' %}RUN yum -y module enable mysql:$MYSQL_VERSION && \
INSTALL_PKGS="{{ spec.pkgs }}" && \
yum install -y --setopt=tsflags=nodocs $INSTALL_PKGS && \
rpm -V $INSTALL_PKGS && \
yum -y clean all --enablerepo='*' && \
mkdir -p /var/lib/mysql/data && chown -R mysql.0 /var/lib/mysql && \
test "$(id mysql)" = "uid=27(mysql) gid=27(mysql) groups=27(mysql)"
{% elif (spec.prod == 'rhel9' or spec.prod == 'c9s') and spec.version == '8.4' %}RUN yum -y module enable ${NAME}:${MYSQL_VERSION} && \
INSTALL_PKGS="{{ spec.pkgs }}" && \
dnf install -y --setopt=tsflags=nodocs ${INSTALL_PKGS} && \
rpm -V ${INSTALL_PKGS} && \
dnf -y clean all --enablerepo='*' && \
mkdir -p ${HOME}/data && chown -R mysql:{{ spec.chown_group }} ${HOME} && \
test "$(id mysql)" = "uid=27(mysql) gid=27(mysql) groups=27(mysql)"
{% elif spec.version == '8.0' %}RUN INSTALL_PKGS="{{ spec.pkgs }}" && \
yum install -y --setopt=tsflags=nodocs ${INSTALL_PKGS} && \
rpm -V ${INSTALL_PKGS} && \
yum -y clean all --enablerepo='*' && \
mkdir -p ${HOME}/data && chown -R mysql.0 ${HOME} && \
test "$(id mysql)" = "uid=27(mysql) gid=27(mysql) groups=27(mysql)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These modular installs seem very similar, maybe the if can be divided into something like
if spec.prod in ['rhel8', ... 'c9s'] for modules where the module is enabled, and then a more common way to install the packages?

Comment thread src/Dockerfile Outdated
Comment on lines +115 to +121
{% if spec.prod == 'rhel8' and spec.version == '8.0' %}COPY {{ spec.version }}/root-common /
COPY {{ spec.version }}/s2i-common/bin/ $STI_SCRIPTS_PATH
COPY {{ spec.version }}/root /
{% else %}COPY ${MYSQL_VERSION}/root-common /
COPY ${MYSQL_VERSION}/s2i-common/bin/ ${STI_SCRIPTS_PATH}
COPY ${MYSQL_VERSION}/root /
{% endif %}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Aren't these the exact same?

Comment thread src/Dockerfile Outdated
Comment on lines +126 to +128
{% if spec.prod == 'rhel8' and spec.version == '8.0' %}RUN ln -s /bin/run-mysqld $STI_SCRIPTS_PATH/run
{% else %}RUN ln -s /bin/run-mysqld ${STI_SCRIPTS_PATH}/run
{% endif %}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also the exact same.

Comment thread src/Dockerfile Outdated
Comment on lines +134 to +142
{% if spec.prod == 'rhel8' and spec.version == '8.0' %}RUN rm -rf /etc/my.cnf.d/* && \
/usr/libexec/container-setup && \
rpm-file-permissions && \
/usr/libexec/mysqld -V | grep -qe "$MYSQL_VERSION\." && echo "Found VERSION $MYSQL_VERSION"
{% else %}RUN rm -rf /etc/my.cnf.d/* && \
/usr/libexec/container-setup && \
rpm-file-permissions && \
/usr/libexec/mysqld -V | grep -qe "${MYSQL_VERSION}\." && echo "Found VERSION ${MYSQL_VERSION}"
{% endif %}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also the exact same.

Comment thread src/Dockerfile Outdated
Comment on lines +13 to +61
{% if spec.prod == 'rhel8' and spec.version == '8.0' %}ENV MYSQL_VERSION={{ spec.version }} \
APP_DATA=/opt/app-root/src \
HOME=/var/lib/mysql

ENV SUMMARY="MySQL {{ spec.version }} SQL database server" \
DESCRIPTION="MySQL is a multi-user, multi-threaded SQL database server. The container \
image provides a containerized packaging of the MySQL mysqld daemon and client application. \
The mysqld server daemon accepts connections from clients and provides access to content from \
MySQL databases on behalf of the clients."

LABEL summary="$SUMMARY" \
description="$DESCRIPTION" \
io.k8s.description="$DESCRIPTION" \
io.k8s.display-name="MySQL {{ spec.version }}" \
io.openshift.expose-services="3306:mysql" \
io.openshift.tags="database,mysql,mysql{{ spec.short }},mysql-{{ spec.short }}" \
com.redhat.component="mysql-{{ spec.short }}-container" \
name="{{ spec.img_name }}" \
version="1" \
{% if spec.license_terms %} com.redhat.license_terms="{{ spec.license_terms }}" \
{% endif %} usage="podman run -d -e MYSQL_USER=user -e MYSQL_PASSWORD=pass -e MYSQL_DATABASE=db -p 3306:3306 {{ spec.img_name }}" \
maintainer="SoftwareCollections.org <[email protected]>"
{% else %}# Standalone ENV call so these values can be re-used in the other ENV calls
ENV MYSQL_VERSION={{ spec.version }} \
MYSQL_SHORT_VERSION={{ spec.short }}

ENV APP_DATA=/opt/app-root/src \
HOME=/var/lib/mysql \
NAME=mysql \
SUMMARY="MySQL ${MYSQL_VERSION} SQL database server" \
DESCRIPTION="MySQL is a multi-user, multi-threaded SQL database server. The container \
image provides a containerized packaging of the MySQL mysqld daemon and client application. \
The mysqld server daemon accepts connections from clients and provides access to content from \
MySQL databases on behalf of the clients."

LABEL summary="${SUMMARY}" \
description="${DESCRIPTION}" \
io.k8s.description="${DESCRIPTION}" \
io.k8s.display-name="MySQL ${MYSQL_VERSION}" \
io.openshift.expose-services="3306:mysql" \
io.openshift.tags="database,${NAME},${NAME}${MYSQL_SHORT_VERSION}{% if spec.prod == 'fedora' and spec.version == '8.0' %},rh-${NAME}${MYSQL_SHORT_VERSION}{% elif spec.prod != 'fedora' %},${NAME}-${MYSQL_SHORT_VERSION}{% endif %}" \
com.redhat.component="{% if spec.prod == 'rhel8' or spec.prod == 'rhel9' or spec.prod == 'rhel10' or spec.prod == 'c9s' or spec.prod == 'c10s' %}${NAME}-${MYSQL_SHORT_VERSION}-container{% else %}${NAME}{% endif %}" \
name="{{ spec.img_name }}" \
{% if spec.prod == 'fedora' %} version="${MYSQL_VERSION}" \
{% elif spec.prod != 'rhel10' %} version="1" \
{% endif %}{% if spec.license_terms %} com.redhat.license_terms="{{ spec.license_terms }}" \
{% endif %} usage="{% if spec.prod == 'c9s' or spec.prod == 'c10s' %}podman run -d -e MYSQL_USER=user -e MYSQL_PASSWORD=pass -e MYSQL_DATABASE=db -p 3306:3306 quay.io/{{ spec.img_name }}:{{ spec.prod }}{% elif spec.prod == 'fedora' %}docker run -d -e MYSQL_USER=user -e MYSQL_PASSWORD=pass -e MYSQL_DATABASE=db -p 3306:3306 quay.io/{{ spec.img_name }}{% else %}podman run -d -e MYSQL_USER=user -e MYSQL_PASSWORD=pass -e MYSQL_DATABASE=db -p 3306:3306 {{ spec.img_name }}{% endif %}" \
maintainer="SoftwareCollections.org <[email protected]>"
{% endif %}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Extremely long if and the only differences I see are the use of {{spec.version}} instead of ${MYSQL_VERSION} and similar small things that evaluate to the same thing during buildtime. Maybe this can be simplified somehow?

@ndavidova

Copy link
Copy Markdown
Contributor Author

Would it also make sense to include the s2i-common directories in the sources? Or just make them symlinks to the one already present in the project root

I believe this should already be present.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@ndavidova, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 53 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d9436642-36bf-464b-b8cb-87fb79b92e30

📥 Commits

Reviewing files that changed from the base of the PR and between 78a998b and 0eda79b.

📒 Files selected for processing (19)
  • .github/workflows/container-tests.yml
  • 8.0/Dockerfile.c9s
  • 8.0/Dockerfile.fedora
  • 8.0/Dockerfile.rhel8
  • 8.0/Dockerfile.rhel9
  • 8.0/README.md
  • 8.0/root/usr/share/container-scripts/mysql/README.md
  • 8.4/Dockerfile.c10s
  • 8.4/Dockerfile.c9s
  • 8.4/Dockerfile.fedora
  • 8.4/Dockerfile.rhel10
  • 8.4/Dockerfile.rhel9
  • 8.4/README.md
  • 8.4/root/usr/share/container-scripts/mysql/README.md
  • 8.4/s2i-common
  • 8.4/test
  • manifest.yml
  • specs/multispec.yml
  • src/Dockerfile
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ndavidov/distgen

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ndavidova

Copy link
Copy Markdown
Contributor Author

[test]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants