Allow building images to any Docker registry and image builds in containers #1545

lewijacn · 2025-05-28T14:58:44Z

Description

This change introduces a new pattern for building images to our tooling. It makes use of Jib for our Java applications (except for RFS currently as it has additional Dockerfile steps) and BuildKit for our other images, to allow building images to any specified docker registry, whether that be a local registry or a remote registry like AWS ECR. This pattern also allows for building the images with a container, which the bootstrapK8s helm chart utilizes to be able to pull this Github repository and build images within a K8s pod.

At a top level this can be executed, after following the setup steps in buildImages.md, like so:

./gradlew buildImagesToRegistry

And includes options for specifying the registry endpoint as well as the architecture to build with:

./gradlew buildImagesToRegistry -PregistryEndpoint=123456789012.dkr.ecr.us-west-2.amazonaws.com/my-ecr-repo -PimageArch=arm64

TODO:

References to this branch should be replaced with main before merging

Issues Resolved

https://opensearch.atlassian.net/browse/MIGRATIONS-2523

Testing

Local and EKS testing

Check List

New functionality includes testing
Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Tanner Lewis <[email protected]>

# Conflicts: # build.gradle

Signed-off-by: Tanner Lewis <[email protected]>

peternied

Haven't go into much depth on jib or buildKit, but had commends on the overall organization

TrafficCapture/trafficCaptureProxyServer/docker/runJavaWithClasspath.sh

build.gradle

peternied · 2025-05-28T17:56:03Z

buildSrc/src/main/groovy/org/opensearch/migrations/common/RegistryImageBuildUtils.groovy

+    static def getFullBaseImageIdentifier(String baseImageRegistryEndpoint, String baseImageGroup, String baseImageName,
+                                      String baseImageTag) {
+        def baseImage = ""
+        def isEcr = baseImageRegistryEndpoint.contains(".ecr.") && baseImageRegistryEndpoint.contains(".amazonaws.com")


Code smell: this looks like what a interface with a Docker and ECR classes would be good at solving.

Let me investigate an interface for this

Added an interface for this now thanks 👍

peternied · 2025-05-28T17:57:38Z

buildSrc/src/main/groovy/org/opensearch/migrations/common/RegistryImageBuildUtils.groovy

+    }
+
+    void applyJibConfigurations(Project rootProject) {
+        def projectsToConfigure = [


Lets shift how this is declared to the place (/build.gradle?) where the image is defined.

Not sure I'm following this comment. I'd like the keep this common logic together, as well as keep the top level build.gradle as small as I can. Was this geared at trying to add some of this logic to the different subprojects?

Html formatting got me, I meant {project}/build.gradle e.g. replayer's build.gradle file would include those details instead of having them spread over the project.

{ baseImageName : "amazoncorretto", baseImageTag : "17-al2023-headless", imageName : "traffic_replayer", imageTag : "latest" }

I was going to say +1 - but I really like the idea of keeping consistency for base images. I would be good to get those consolidated to one place and have the image names/tags defined within the subprojects.

After going through the rest of the PR... I think keeping more of the docker stuff outside the scope of the java projects makes sense. If somebody comes along and wants to build RFS as a java application for their own image (or AMI, etc) they shouldn't be concerned with the docker configs at all - even the image name. Those aren't details that matter for a java project in isolation.

Maybe we could rename or lift the configurations for all of the docker projects rather than w/in the Utils script. Would it make sense to have a new gradle project just to track the images?

Co-authored-by: Peter Nied <[email protected]> Signed-off-by: Tanner Lewis <[email protected]>

…on build process Signed-off-by: Tanner Lewis <[email protected]>

Signed-off-by: Tanner Lewis <[email protected]>

peternied · 2025-05-29T19:06:47Z

build.gradle

@@ -1,4 +1,6 @@
+import com.google.cloud.tools.jib.gradle.JibTask


We discussed this over a call but capturing it here. I'm apresenhive to approve this PR as is, it adds a new way to create docker images and I suppose I am asking why not replace the exist docker plugin with this change as it is. I'd rather we only use one tool at a time.

I know this might come at odds with the coupling in this PR with the registry publication, but I'd rather we replace com.bmuschko:gradle-docker-plugin with jib all at once and then introduce the container registry copying seperately.

We shouldn't make a wholesale switch because the new images aren't binary compatible with the old ones. They need to be invoked in a totally separate way. To do that, we'd need to add a large amount of code to other systems that are on the deprecation path (docker compose and ECS).

Leaving the two coexist for a little bit lets us track upstream changes more easily while we transition. Yes, it's a task, but given the staffing/importance of making the switch, it seems like a good tradeoff.

gregschohn

Thanks Tanner - there's tons of exciting stuff in here.
I've left a lot of feedback, much of it just so that I can understand more about what is going on. Please provide responses where you know the answers and if you don't, feel free to ignore.
I'd like to get some more understanding, then figure out what should be changed now, what doesn't matter, and what we can do later.
Thanks again!

gregschohn · 2025-05-30T10:57:48Z

TrafficCapture/dockerSolution/build.gradle

@@ -28,7 +28,9 @@ dockerFilesForExternalServices.each { dockerImageName, projectName ->
                project(":CreateSnapshot"),
                project(":MetadataMigration")
            ]
-            def syncTask = getMigrationConsoleSyncTask(project, dockerImageName, escapedProjectName, libraries, applications)
+            // Setup additional sync task which doesn't depend on the base image being built, for other projects to use


Can you explain this one some more? Is this for any base image, or the test console one? Either way, I don't understand how this would be useful/how it wouldn't be problematic in some circumstances.

gregschohn · 2025-05-30T10:59:37Z

TrafficCapture/dockerSolution/build.gradle

     // Create a single sync task to copy the required files
    def destDir = "build/docker/${dockerImageName}_${escapedProjectName}"
-    def syncTask = project.tasks.create("syncArtifact_${dockerImageName}_${escapedProjectName}", Sync) {
+    def taskName = "syncArtifact_${dockerImageName}_${escapedProjectName}"


I recall putting escapedProjectName in place for some workaround. Do we still need it?

gregschohn · 2025-05-30T11:03:08Z

TrafficCapture/dockerSolution/src/main/docker/migrationConsole/Dockerfile

@@ -1,4 +1,5 @@
-FROM migrations/elasticsearch_client_test_console:latest AS migration-console-base
+ARG BASE_IMAGE=migrations/elasticsearch_client_test_console:latest


Why do you define this and why is it defined twice in the file (albeit with the same values)?

gregschohn · 2025-05-30T12:39:50Z

TrafficCapture/trafficCaptureProxyServer/docker/runJavaWithClasspath.sh

@@ -0,0 +1,7 @@
+java \


What is this file used for now?

gregschohn · 2025-05-30T12:49:10Z

build.gradle

@@ -1,4 +1,6 @@
+import com.google.cloud.tools.jib.gradle.JibTask


We shouldn't make a wholesale switch because the new images aren't binary compatible with the old ones. They need to be invoked in a totally separate way. To do that, we'd need to add a large amount of code to other systems that are on the deprecation path (docker compose and ECS).

Leaving the two coexist for a little bit lets us track upstream changes more easily while we transition. Yes, it's a task, but given the staffing/importance of making the switch, it seems like a good tradeoff.

gregschohn · 2025-05-30T15:09:14Z

deployment/k8s/charts/components/bootstrapK8s/templates/bootstrap-job.yaml

+              {{- if .Values.keepJobAlive }}
+              tail -f /dev/null
+              {{- end }}


would anybody NOT working on this job ever use this flag?

gregschohn · 2025-05-30T15:12:57Z

deployment/k8s/charts/components/bootstrapK8s/templates/buildkitd-pod.yaml

+  ports:
+    - name: buildkit
+      protocol: TCP
+      port: 1234


does buildkit have a default port? 1234 sounds made-up :)

gregschohn · 2025-05-30T15:13:32Z

deployment/k8s/charts/components/bootstrapK8s/templates/buildkitd-pod.yaml

+      ports:
+        - containerPort: 1234
+      securityContext:
+        privileged: true


Again, just curious - if this gets turned off, what does the user see

gregschohn · 2025-05-30T15:16:12Z

deployment/k8s/charts/components/bootstrapK8s/templates/role.yaml

+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: migration-role


Should this be more targeted to the bootstrapping phase or is it used afterwards for migrations?

gregschohn · 2025-05-30T15:16:53Z

deployment/k8s/minikubeLocal.sh

@@ -18,8 +18,14 @@ start() {
  helm repo add opensearch-operator https://opensearch-project.github.io/opensearch-k8s-operator/
  helm repo add strimzi https://strimzi.io/charts/

-  minikube start
+  # Development setup to allow using an insecure registry
+  minikube start --insecure-registry="0.0.0.0/0"


lewijacn added 30 commits May 10, 2025 10:14

Checkpoint gradle setup

10b3d7b

Signed-off-by: Tanner Lewis <[email protected]>

Checkpoint 2

79e4e7d

Signed-off-by: Tanner Lewis <[email protected]>

Checkpoint 3

521b0f9

Signed-off-by: Tanner Lewis <[email protected]>

Checkpoint 3

690b2a1

Signed-off-by: Tanner Lewis <[email protected]>

Checkpoint 4

dd7178e

Signed-off-by: Tanner Lewis <[email protected]>

Checkpoint may13

7caf0e3

Signed-off-by: Tanner Lewis <[email protected]>

Sync may13

8fb2b98

Signed-off-by: Tanner Lewis <[email protected]>

Change k8 check command

06b2eaf

Signed-off-by: Tanner Lewis <[email protected]>

Update kubectl command

e0f0bfc

Signed-off-by: Tanner Lewis <[email protected]>

Sync may13 pvc

491a3a8

Signed-off-by: Tanner Lewis <[email protected]>

Test not uninstall

a376abd

Signed-off-by: Tanner Lewis <[email protected]>

Revert test change

dd65aef

Signed-off-by: Tanner Lewis <[email protected]>

Minor adjustments

7811aff

Signed-off-by: Tanner Lewis <[email protected]>

Test change yaml

a7a6fbd

Signed-off-by: Tanner Lewis <[email protected]>

Add another test2

7beea77

Signed-off-by: Tanner Lewis <[email protected]>

Sync up registry endpoint and ecr image format

22c517e

Signed-off-by: Tanner Lewis <[email protected]>

Add test service account name

a37e5f0

Signed-off-by: Tanner Lewis <[email protected]>

Remove helm uninstall for kaniko for testing

e4505ea

Signed-off-by: Tanner Lewis <[email protected]>

Update image build gradle for ECR

d2c3678

Signed-off-by: Tanner Lewis <[email protected]>

Update jib for ECR

d964b9e

Signed-off-by: Tanner Lewis <[email protected]>

Add helm chart for bootstrap

143aaa8

Signed-off-by: Tanner Lewis <[email protected]>

Automate bootstrap for EKS

9eb1116

Signed-off-by: Tanner Lewis <[email protected]>

Minor fixes from testing

05a1995

Signed-off-by: Tanner Lewis <[email protected]>

Minor testing 2

534b298

Signed-off-by: Tanner Lewis <[email protected]>

Update jib auth

320a603

Signed-off-by: Tanner Lewis <[email protected]>

Up wait time for kaniko task

9e350a3

Signed-off-by: Tanner Lewis <[email protected]>

Update from cloudshell testing

56e8162

Signed-off-by: Tanner Lewis <[email protected]>

Change logging for wait

bf8d5ed

Signed-off-by: Tanner Lewis <[email protected]>

Update end to temp spin up migration console

5a6f67f

Signed-off-by: Tanner Lewis <[email protected]>

Updates for better ux

d9cf8c7

Signed-off-by: Tanner Lewis <[email protected]>

lewijacn added 3 commits May 28, 2025 00:57

Remove unintended changes

88f35a8

Signed-off-by: Tanner Lewis <[email protected]>

Merge remote-tracking branch 'origin/main' into build-images-in-k8s

164bb1f

# Conflicts: # build.gradle

Remove unneeded changes

def5cce

Signed-off-by: Tanner Lewis <[email protected]>

lewijacn requested review from AndreKurait, chelma, gregschohn, mikaylathompson, peternied, sumobrian and jugal-chauhan as code owners May 28, 2025 14:58

lewijacn temporarily deployed to migrations-cicd May 28, 2025 14:59 — with GitHub Actions Inactive

Doc updates and sonarqube fixes

15b7349

Signed-off-by: Tanner Lewis <[email protected]>

lewijacn temporarily deployed to migrations-cicd May 28, 2025 16:37 — with GitHub Actions Inactive

peternied reviewed May 28, 2025

View reviewed changes

Apply suggestions from code review

ea000c2

Co-authored-by: Peter Nied <[email protected]> Signed-off-by: Tanner Lewis <[email protected]>

lewijacn temporarily deployed to migrations-cicd May 28, 2025 19:56 — with GitHub Actions Inactive

Address PR feedback and automatically do no dependency on docker daem…

b2c861a

…on build process Signed-off-by: Tanner Lewis <[email protected]>

lewijacn temporarily deployed to migrations-cicd May 28, 2025 20:00 — with GitHub Actions Inactive

Increase buildkitd resource limits

eac6a5c

Signed-off-by: Tanner Lewis <[email protected]>

lewijacn temporarily deployed to migrations-cicd May 28, 2025 20:40 — with GitHub Actions Inactive

Update per PR feedback and proper link comment

ecfe470

Signed-off-by: Tanner Lewis <[email protected]>

lewijacn temporarily deployed to migrations-cicd May 29, 2025 16:50 — with GitHub Actions Inactive

peternied reviewed May 29, 2025

View reviewed changes

gregschohn reviewed May 30, 2025

View reviewed changes

		@@ -1,4 +1,6 @@
		import com.google.cloud.tools.jib.gradle.JibTask

		@@ -1,4 +1,5 @@
		FROM migrations/elasticsearch_client_test_console:latest AS migration-console-base
		ARG BASE_IMAGE=migrations/elasticsearch_client_test_console:latest

Allow building images to any Docker registry and image builds in containers #1545

Are you sure you want to change the base?

Allow building images to any Docker registry and image builds in containers #1545

Uh oh!

Conversation

lewijacn commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Testing

Check List

Uh oh!

peternied left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peternied May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregschohn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lewijacn commented May 28, 2025 •

edited

Loading

peternied May 28, 2025 •

edited

Loading