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.

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

Cpbr 1867 ubi9 minimal #618

Open
wants to merge 51 commits into
base: master
Choose a base branch
from
Open

Cpbr 1867 ubi9 minimal #618

wants to merge 51 commits into from

Conversation

hk10111
Copy link
Member

@hk10111 hk10111 commented Jan 15, 2025

Change Description

a new cp-base image that is built on ubi9-minimal
this image contains only jre and would be used as the base for CP component image

Testing

@hk10111 hk10111 marked this pull request as ready for review January 15, 2025 11:25
@hk10111 hk10111 requested a review from a team as a code owner January 15, 2025 11:25
gpgkey=https://packages.adoptium.net/artifactory/api/gpg/key/public \n\
" > /etc/yum.repos.d/adoptium.repo
RUN echo "installing temurin-17-jre:${TEMURIN_JDK_VERSION}"
RUN yum --nodocs install -y --setopt=install_weak_deps=False temurin-17-jre
Copy link
Member

Choose a reason for hiding this comment

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

we have to use JDK 21 with CP 8.0, let update it as part of this PR itself


<packaging>pom</packaging>

<artifactId>cp-base-jre17</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

let's rename this image as cp-java21

@@ -0,0 +1,78 @@
ARG MICRODIR=/microdir
ARG UBI_MICRO_VERSION=8.10-13
Copy link
Member

Choose a reason for hiding this comment

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

please remove unused args in the Dockerfile

ARG MICRODIR=/microdir
ARG UBI_MICRO_VERSION=8.10-13
ARG TEMURIN_JDK_VERSION="21.0.6.0.0.7-1"
ARG DOCKER_UPSTREAM_REGISTRY="519856050701.dkr.ecr.us-west-2.amazonaws.com/docker/prod/"
Copy link
Member

Choose a reason for hiding this comment

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

please remove default values for these args so that the build fails if the corrects args are not provided. This will ensure that args are properly propagated during docker build

# base image that supports it
ENV LANG="C.UTF-8"

#ARG MICRODIR
Copy link
Member

Choose a reason for hiding this comment

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

please remove this commented line

RUN go test ./...

FROM registry.access.redhat.com/ubi8/ubi-minimal:${UBI_MINIMAL_VERSION}
FROM ${DOCKER_UPSTREAM_REGISTRY}confluentinc/cp-base-java:${DOCKER_TAG}
Copy link
Member

Choose a reason for hiding this comment

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

please use new arg/env for the base image name as there are symlinks also added in the Dockerfile

pom.xml Outdated
@@ -34,6 +35,8 @@
<io.confluent.common-docker.version>8.0.0-0</io.confluent.common-docker.version>
<!-- Versions-->
<ubi.image.version>8.10-1154</ubi.image.version>
<ubi9.image.version>9.5-1736404155</ubi9.image.version>
<ubi.micro.image.version>8.10-13</ubi.micro.image.version>
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed as this is not getting used right now.

<artifactId>dockerfile-maven-plugin</artifactId>
<configuration>
<buildArgs>
<UBI_MINIMAL_VERSION>${ubi.image.version}</UBI_MINIMAL_VERSION>
Copy link
Member

Choose a reason for hiding this comment

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

should the variable be ubi9.image.version instead of ubi.image.version

<image>
<build>
<args>
<UBI_MINIMAL_VERSION>${ubi.image.version}</UBI_MINIMAL_VERSION>
Copy link
Member

Choose a reason for hiding this comment

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

should the variable be ubi9.image.version instead of ubi.image.version

@@ -4,10 +4,10 @@
import confluent.docker_utils as utils
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep this file in base-lite also to make sure base-lite is getting built properly also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants