-
Notifications
You must be signed in to change notification settings - Fork 60
[OPENJDK-2992] Create DeploymentConfig and Route for jlink apps #514
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
Conversation
Signed-off-by: Jayashree Huttanagoudar <[email protected]>
Signed-off-by: Jayashree Huttanagoudar <[email protected]>
Signed-off-by: Jayashree Huttanagoudar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! Some notes
- can we get a default value for
TARGET_PORT
? - please add
TARGET_PORT
to the examples intemplates/jlink/README.md
- The Port parameter in the service object needs templating too; the default of 80 doesn't work with the quickstart we use in README.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The structure looks correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding those; this is much closer. I've just tried it again in CRC and it's still not quite there. One more difference I notice between what this creates and what is created if I do "+Add" by hand in the console, is the latter creates a Pod object as well. Perhaps we need to add that.
Here's a dump of the pod object that was created in my CRC instance when I did a manual "Add". I'm guessing the vast majority of fields here aren't needed in the template, and somehow we 'll have to resolve the ImageStream/Image discrepancy for this
|
Thanks for continuing to work on this. We're not quite there yet. Here's the
With the current state ( Once that's resolved, the final state is a DeploymentConfig and a Pod that Where we want to get to is to have the objects combined like in the manual Clicking on the "external link" icon, to visit the URI for the route, The acceptance criteria is thus
|
The latest push fixes the label issue. The problem doesn't seem to be with the route, rather it's with the container itself. Looking at the logs before it crashes I'm seeing ERROR: Failed to start application (with profile [prod]) It looks like quarkus isn't able to create any temporary files/directories inside the container when built through the template. I don't see any differences in the pod specs that should be causing this, do you have any ideas @jmtd ? |
Upon further investigation it looks like the user we switch to for running the java command (USER 185) doesn't have write permissions on the file system so it's unable to create the cache for quarkus. This causes quarkus to crash and the pod to enter a CrashLoopBackoff which is why the application is unavailable. Removing the USER 185 and presumably running as root everything works as expected, but this probably isn't the best solution here, do you have any suggestions for a different user and/or how to fix this permissions issue? |
@jmtd any thoughts on the above? |
Hi Josh, I've just been looking at this a bit more and trying to reproduce your experience but I haven't been able to. When I load the template on a clean cluster, once the three builds have finished, I can see (in the "Topology" view) a single Application containing a DeploymentConfig and an associated Route. Trying to follow the Route shows "application not available". There are 0/1 pods running associated with the DeploymentConfig, and I cannot find logs or events to explain what's going on. Since nothing is running, there's no issue with permissions in /tmp, at least yet. Meanwhile, if i click "+Add" and create a new Deployment from the final image stream, that Deployment starts up a Pod and following the relevant route leads me to the "hello world" page. So the app has deployed successfully and hasn't complained about any permissions issues. If I access a shell on the running Pod, I can see the permissions for /tmp are to be expected (anyone can write) and indeed the running user has written some stuff:
|
After nuking the old crc cluster
and updating to the most recent version of crc
The deploymentconfig seems to work as expected now: Template was given the following parameters:
Make sure the builder containers used are correctly named (I updated the README accordingly)
|
Hi Josh,
Are you sure you specified JDK 17 for the template? With our move towards only doing a 21 tech preview image, I would expect this not to work. |
I tested both. JDK 21 works as well for me.
|
I'm still getting undeployable results. I'll update my crc (current is CRC version: 2.31.0+6d23b6) |
Did you ever get a chance to re-test this with an updated CRC? I honestly have no idea where the disconnect is between what we're seeing. I can't reproduce what you're seeing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are a couple items that should be updated. 1. JDK_VERSION. We are only releasing this for JDK21 so there is no reason to confuse the customer with a lower default verison. 2. Update to using Deployment rather than DeploymentConfig.
DeploymentConfig has been deprecated since OCP version 4.14.
Updated the DeploymentConfig to a Deployment object. From the documentation there's one notable limitation here:
In other words, for the Deployment to resolve the ImageStream correctly it needs to be run in a non default project, running everything in a newly created project/namespace it works as expected and deploys properly for me. If you're getting an undeployable result, check the deployment pod to see if it's having difficulty resolving the $APPNAME-lightweight-image. If it's an ErrImagePull looking for a URL like docker.io/quarkus-lightweight-image or something similar, then it's not able to resolve the ImageStream and you should double check the project you're running it in. |
I'll have to double check that when I get home. My understanding is users should never use the default project. In the case I ran on Thursday, I used a project called jlink1. But I need to try it again to make sure it is reproducable. |
On Today's call I said I'd share my experiment with dumping a Deployment object from CRC after creating it manually. It's at https://github.com/jmtd/redhat-openjdk-container-images/tree/514-jmtd-dump-deployment . Note that the focus of this work is still on Josh's PR; I need to catch up on #514 (comment) |
@jmtd I re-ran the procedure on our OCP cluster and it worked this time. If you run @Josh-Matsuoka command above, remove the -n openshift as I think this caused some problems. I ran it again today and the deployment came automatically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes with the readme. Otherwise looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
fantastic, it worked! |
Addresses https://issues.redhat.com/browse/OPENJDK-2992
Cleanup/Continuation of #500
This cleans up the original PR, bringing it in line with the new naming convention for created objects, adding in the missing target port, and converting the Deployment to a DeploymentConfig to work with ImageStreamTags