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-2992] Spec for deployment and route object #500

Open
wants to merge 3 commits into
base: jlink-dev
Choose a base branch
from

Conversation

jhuttana
Copy link
Contributor

JIRA: https://issues.redhat.com/browse/OPENJDK-2992

NOTE: My stage-2 is failing with CRI-O failed msg which is known one with our crc setup. I will have to restart or do a fresh setup to get it working fine :) Meanwhile I thought I can raise a PR so that you can take look and commented if its syntactically fine.

Could you please review the changes?

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

jmtd commented Jun 11, 2024

I've just tried this out.

the deployment object starts to log errors as soon as the template is instantiated, and before the builds have completed.

Once the builds have completed, the route is not yet working (Application is not available)

I'm creating a deployment manually now to compare the objects in the OCP instance

@jmtd
Copy link
Member

jmtd commented Jun 11, 2024

app
one difference, the manual deployment has some kind of "Application" enclosing it

@jmtd
Copy link
Member

jmtd commented Jun 11, 2024

I think the route object needs .spec.port.targetPort: 8080-tcp added (possibly also via a template variable)

spec:
containers:
- name: jlinked-app-container
image: lightweight-image:latest
Copy link
Member

Choose a reason for hiding this comment

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

I think this might be an issue. Comparing this to a manually-created Deployment, the working one has, instead,

- image: image-registry.openshift-image-registry.svc:5000/jlink2/lightweight-image@sha256:19538848d2f06ee00d7462f8290982191fe9acb63c6ecea026d8df484e840ecc

Copy link
Member

Choose a reason for hiding this comment

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

...but that's also specific to my OpenShift cluster. I'm not sure what to put in a template. re-reading https://docs.openshift.com/container-platform/4.15/openshift_images/using-templates.html, the example they have of a template with a Deployment in it does not name the image at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I referred this template : https://docs.openshift.com/container-platform/4.15/applications/deployments/what-deployments-are.html#deployments-kube-deployments_what-deployments-are
DeploymentConfig is marked as deprecated and they recommend to use Deployment object.

Copy link
Member

Choose a reason for hiding this comment

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

I hadn't noticed the discrepancy between Deployment and DeploymentConfig. Hmm. I see (from that example) that DeploymentConfig supports specifying an ImageStreamTag. It doesn't look like Deployment does (which makes sense, since Deployment comes from Kubernetes but ImageStreams are an OpenShift addition). So I'm not really sure how we would specify the image to use for the Deployment in a safe/generic way.

Even though DCs are deprecated, I think I'd be happy to merge a PR which used them if it worked today!

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

Successfully merging this pull request may close these issues.

2 participants