jboss-container-images / openjdk

Source To Image (S2I) image for Red Hat OpenShift providing OpenJDK
Apache License 2.0
53 stars 58 forks source link

[OPENJDK-2735] Phase-3 BuildConfig template for multistage build for jlinked image #461

Closed jhuttana closed 3 months ago

jhuttana commented 3 months ago

This is still WIP and an initial attempt to write a BuildConfig template.

jmtd commented 3 months ago

This is looking good! I'll leave some comments.

jmtd commented 3 months ago

I don't think switching to dockerfilePath will address the problem that OpenShift will substitute the last FROM instruction in the multi-stage build, rather than the first: in other words, whether the Dockerfile is embedded or not, OpenShift will (optionally) replace FROM registry.access.redhat.com/ubi9/ubi-micro, but not FROM ubi9-jlinked-image, which is the one we need substituted.

Two approaches that might work are moving the first stage work into Phase 1, or wrapping the whole thing up in an OpenShift template, and using the template parameters to substitute the FROM. We are going to be using OpenShift templates for other aspects, so it would be worth looking at that now.

jhuttana commented 3 months ago

On my local crc setup I tried to create the template as follows:

$ oc create -f templates/multistage-buildconfig.yaml
template.template.openshift.io/jlinked-image-template created

$ oc get template
NAME                     DESCRIPTION                                                              PARAMETERS    OBJECTS
jlinked-image-template   Template to produce an imagestream and buildconfig for a Jlinked image   0 (all set)   3

Further I tried to process the template and end-up with some errors :)

$ oc process jlinked-image-template | oc create -f -
imagestream.image.openshift.io/jlinked-image created
Error from server (Invalid): error when creating "STDIN": BuildConfig.build.openshift.io "ubi9-jlinked-image-buildconfig" is invalid: spec.strategy: Invalid value: build.BuildStrategy{DockerStrategy:(*build.DockerBuildStrategy)(nil), SourceStrategy:(*build.SourceBuildStrategy)(nil), CustomStrategy:(*build.CustomBuildStrategy)(nil), JenkinsPipelineStrategy:(*build.JenkinsPipelineBuildStrategy)(nil)}: must provide a value for exactly one of sourceStrategy, customStrategy, dockerStrategy, or jenkinsPipelineStrategy
Error from server (Invalid): error when creating "STDIN": BuildConfig.build.openshift.io "lean-runtime-buildconfig" is invalid: spec.strategy: Invalid value: build.BuildStrategy{DockerStrategy:(*build.DockerBuildStrategy)(nil), SourceStrategy:(*build.SourceBuildStrategy)(nil), CustomStrategy:(*build.CustomBuildStrategy)(nil), JenkinsPipelineStrategy:(*build.JenkinsPipelineBuildStrategy)(nil)}: must provide a value for exactly one of sourceStrategy, customStrategy, dockerStrategy, or jenkinsPipelineStrategy

Looks like some wrong stuff is fed for build strategy !. Which I need to figure out.

jmtd commented 3 months ago

These are just indentation errors I think: It's saying the buildconfigs don't have .spec.strategy keys. At the moment the strategy keys are indented such that they're under spec.source.

jmtd commented 3 months ago

This is looking great. I'm not 100% sure that we should split the build into two buildConfigs here or not; it might be that we can use an openshift template parameter for the FROM in a dockerfile multi-stage build, if we could determine the correct value based on template parameters. (I'm not sure if we can or not!) But if we move ahead with two buildConfigs, as per the current status, we need to do something to fix up the COPY --from lines.

jhuttana commented 3 months ago

This is looking great. I'm not 100% sure that we should split the build into two buildConfigs here or not; it might be that we can use an openshift template parameter for the FROM in a dockerfile multi-stage build, if we could determine the correct value based on template parameters. (I'm not sure if we can or not!) But if we move ahead with two buildConfigs, as per the current status, we need to do something to fix up the COPY --from lines.

Thanks for the review. I will try to brainstorm and experiment few options and update my findings :)

jhuttana commented 3 months ago

With the recent commit able to create two separate BuildConfigs for stage-1 and stage-2 in our multi-stage Dockerfile

$ oc create -f templates/multistage-buildconfig.yaml
template.template.openshift.io/jlinked-image-template created

$ oc get templates
NAME                     DESCRIPTION                                                              PARAMETERS    OBJECTS
jlinked-image-template   Template to produce an imagestream and buildconfig for a Jlinked image   0 (all set)   3

$ oc process jlinked-image-template | oc create -f -
imagestream.image.openshift.io/jlinked-image created
buildconfig.build.openshift.io/ubi9-jlinked-image-buildconfig created
buildconfig.build.openshift.io/lean-runtime-buildconfig created
jmtd commented 3 months ago

Some notes on the current status:

The first buildConfig cannot be run because of a dependency issue

oc start-build ubi9-jlinked-image-buildconfig
The ImageStreamTag "jlinked-image:latest" is invalid: from: Error resolving ImageStreamTag jlinked-image:latest in namespace jlink: unable to find latest tagged image

The ImageStream exists, but there is no :latest tag, because no builds have run yet. The buildConfig outputs to that ImageStream, so we cannot also depend upon it. Removing dockerStrategy.from resolves that problem.

Next problem, builds from that build Config still fail, " Output image could not be resolved.". The output is described as ubi9-jlinked-image:latest which does not exist. The imageStream defined here is jlinked-image instead. Changing that and I can start builds

The builds fail because quay.io/repository/jdowland/jlink:ubi9-jlinked-image cannot be pulled. the correct name is quay.io/jdowland/jlink:ubi9-jlinked-image (no "repository"). Fixing that, and the build can fetch the image OK, and the build succeeds!

The next problem is, after the build succeeds, it updates the jlinked-image imagestream, which causes the trigger for the buildconfig to fire, so it builds again... a potentially infinite loop. The imageChange trigger for this buildConfig should be removed.

jmtd commented 3 months ago

On to the second buildConfig, lean-runtime-buildconfig. This gets triggered by the imagestream jlinked-image getting updated (which the first buildConfig does), great.

This second buildConfig is defined to build from, and also write to, jlinked-image imageStream, which cannot work. We need a separate imagStream for each buildConfig to read or write to.

builds from the second buildConfig fail at the COPY --from=ubi9-jlinked-image instructions: it does not know what ubi9-jlinked-image is. I tried to change it to COPY --from=jlinked-image, matching the input imageStream, but that doesn't work either yet.

jhuttana commented 3 months ago

With the recent changes FirstBuildConfig completed successfully. The secondBuildConfig when I see on the web console as developer its state is still new but as Administrator -> Build -> Phase-3 -> Activity -> view evernts

Generated from buildconfig-controller
14 times in the last 13 minutes
error instantiating Build from BuildConfig phase-3/lean-runtime-buildconfig (0): Error resolving ImageStreamTag ubi9-jlinked-image:latest in namespace phase-3: unable to find latest tagged image

But the FirstBuildConfig has successully created ubi9-jlinked-image:latest

$ oc get is
NAME                 IMAGE REPOSITORY                                                                     TAGS     UPDATED
ubi9-jlinked-image   default-route-openshift-image-registry.apps-crc.testing/phase-3/ubi9-jlinked-image   latest   5 minutes ago
jhuttana commented 3 months ago

I am not sure whether custom build strategy is useful to handle COPY instructions in the secondBuildConfig. If yes, then we have to use script. Instead the better way would be to use strategy.dockerStrategy.dockerfilePath as used in #471

jhuttana commented 3 months ago

Closing this PR as we are focusing on #471