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

[OPENJDK-2911] Merge all 3-templates into one #477

Merged
merged 4 commits into from
Apr 12, 2024

Conversation

jhuttana
Copy link
Contributor

@jhuttana jhuttana commented Apr 8, 2024

JIRA : https://issues.redhat.com/browse/OPENJDK-2911
Could you please review the changes?
My local crc is again giving some expected problems like space issue.
I will fix that. Meanwhile thought to raise a PR.

Also for time being I have commented triggers section.

Signed-off-by: Jayashree Huttanagoudar <[email protected]>
@jhuttana jhuttana requested a review from jmtd April 8, 2024 10:56
@jmtd
Copy link
Member

jmtd commented Apr 8, 2024

Syntactically valid, loads into OpenShift OK; processing OK, creating the objects OK. Manually triggering the first buildConfig...

@jmtd
Copy link
Member

jmtd commented Apr 8, 2024

First build fine; manually starting second build, completed fine; starting third build, fail: Error "Error resolving ImageStreamTag intermediate:latest in namespace jlink2: unable to find latest tagged image" for field "from".

@jmtd
Copy link
Member

jmtd commented Apr 8, 2024

The stage 2 buildConfig output: key is indented wrong, it should belong under spec: but is at the same indent level.
The same problem exists in jlink/jlink-builder/jlink-builder-template.yaml, I thought we'd fixed it but that must have been missed. Please fix it in the merged version. Then, please delete jlink/jlink-builder/jlink-builder-template.yaml and templates/multistage-dockerfile-buildconfig.yaml.

@jhuttana
Copy link
Contributor Author

jhuttana commented Apr 8, 2024

The stage 2 buildConfig output: key is indented wrong, it should belong under spec: but is at the same indent level. The same problem exists in jlink/jlink-builder/jlink-builder-template.yaml, I thought we'd fixed it but that must have been missed. Please fix it in the merged version. Then, please delete jlink/jlink-builder/jlink-builder-template.yaml and templates/multistage-dockerfile-buildconfig.yaml.

Sure Jon I will do that.

jhuttana added 2 commits April 8, 2024 18:35
- Indent `output` section in stage-2
- Indent `output` section in merged version of jlink-builder-template.yaml
- Uncomment `trigger` section

Signed-off-by: Jayashree Huttanagoudar <[email protected]>
@jmtd
Copy link
Member

jmtd commented Apr 8, 2024

Once I'd addressed the indenting, and I side-tracked to commit a suitable test application somewhere (rh-openjdk/openjdk-container-test-applications#5), it works!

@jhuttana
Copy link
Contributor Author

jhuttana commented Apr 8, 2024

Once I'd addressed the indenting, and I side-tracked to commit a suitable test application somewhere (jboss-container-images/openjdk-test-applications#5), it works!

Actually I just wanted to add QUARKUS_PACKAGE_TYPE=uber-jar to the parameter list. Its good that you have added that to .s2i/environment in the application itself. Thank you!

@jhuttana
Copy link
Contributor Author

jhuttana commented Apr 8, 2024

@jmtd To move templates/jlink/jlink-builder/README.md to templates/jlink/ there is one conflict, because already we have a README.md there which was created for multistage Docker build. If we want to retain that then we will have to name templates/jlink/jlink-builder/README.md to ``templates/jlink/README_3PHASE.md` is that fine?

@jhuttana
Copy link
Contributor Author

jhuttana commented Apr 8, 2024

The template in this PR worked fine for all the three stages and created the expected outputs.

$ oc get builds
NAME                       TYPE     FROM          STATUS     STARTED          DURATION
jlink-builder-jdk-17-1     Docker   Dockerfile    Complete   19 minutes ago   1m9s
jlink-s2i-jdk-17-1         Source   Git@0635f38   Complete   16 minutes ago   6m23s
multistage-buildconfig-2   Docker   Dockerfile    Complete   5 minutes ago    46s

$ oc get is
NAME                    IMAGE REPOSITORY                                                                        TAGS                                                      UPDATED
intermediate            default-route-openshift-image-registry.apps-crc.testing/phase-1/intermediate            latest                                                    10 minutes ago
java                    default-route-openshift-image-registry.apps-crc.testing/phase-1/java                    11,8,latest,openjdk-11-ubi8,openjdk-17-ubi8 + 1 more...   4 hours ago
lightweight-image       default-route-openshift-image-registry.apps-crc.testing/phase-1/lightweight-image       latest                                                    5 minutes ago
ubi9-openjdk-11         default-route-openshift-image-registry.apps-crc.testing/phase-1/ubi9-openjdk-11         1.18,1.13,1.14,1.15,1.16,1.17                             4 hours ago
ubi9-openjdk-17         default-route-openshift-image-registry.apps-crc.testing/phase-1/ubi9-openjdk-17         1.18,1.13,1.14,1.15,1.16,1.17                             21 minutes ago
ubi9-openjdk-17-jlink   default-route-openshift-image-registry.apps-crc.testing/phase-1/ubi9-openjdk-17-jlink   latest                                                    18 minutes ago
ubi9-openjdk-21         default-route-openshift-image-registry.apps-crc.testing/phase-1/ubi9-openjdk-21         1.17,1.18                                                 4 hours ago
ubimicro                default-route-openshift-image-registry.apps-crc.testing/phase-1/ubimicro                latest                                                    20 minutes ago

$ oc get bc
NAME                     TYPE     FROM                  LATEST
jlink-builder-jdk-17     Docker   Dockerfile            1
jlink-s2i-jdk-17         Source   Git@quarkus-uberjar   1
multistage-buildconfig   Docker   Dockerfile            2

$ oc get templates
NAME                 DESCRIPTION                                                                 PARAMETERS    OBJECTS
jlink-app-template   Template to produce imagestreams and buildconfigs for jlinked application   4 (3 blank)   7

@jmtd
Copy link
Member

jmtd commented Apr 10, 2024

To move templates/jlink/jlink-builder/README.md to templates/jlink/ there is one conflict, because already we have a README.md there which was created for multistage Docker build.

The existing templates/jlink/README.md for the multistage docker build (as well as the multistage docker build sources) can be removed.

@jmtd
Copy link
Member

jmtd commented Apr 10, 2024

The process worked for me! Please remove the old templates/jlink/README.md, move the other the other one into place and I can merge.

… templates/jlink/jlink-builder/

Signed-off-by: Jayashree Huttanagoudar <[email protected]>
@jhuttana
Copy link
Contributor Author

Do we need to update README.md to match new template jlinked-app.yaml ? Or is it good to do that in another PR?

@jmtd
Copy link
Member

jmtd commented Apr 10, 2024

Do we need to update README.md to match new template jlinked-app.yaml ? Or is it good to do that in another PR?

Another PR

@jmtd jmtd merged commit a903061 into rh-openjdk:jlink-dev Apr 12, 2024
0 of 6 checks passed
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