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

Add Helm Operator for OpenJ9 JITServer #81

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

agarwalrounak
Copy link

Signed-off-by: Rounak Agarwal [email protected]

@mpirvu mpirvu self-assigned this Feb 7, 2022
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

Can a user get the two yaml file without cloning this entire openj9-utils repo?
We need some documentation on how to change the Java version for JITServer. Moreover, what happens if a user needs to have a JITServer with Java8 and a JITServer with Java11? We need two different JITServer deployments.

@agarwalrounak agarwalrounak requested a review from mpirvu February 10, 2022 14:28
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

Could you please add license information much like the helm files have?

@agarwalrounak agarwalrounak requested a review from mpirvu February 15, 2022 13:26
Copy link
Contributor

@mpirvu mpirvu left a comment

Choose a reason for hiding this comment

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

LGTM. One I have is about the particle 'sample' in the name of the JITServer service/deployment/pod. E.g. service/openj9jitserverchart-sample-openj9-jitserver-chart
How is the name being constructed?
I also feel there is a bit of redundancy between "openj9jitserverchart" and "openj9-jitserver-chart"

@agarwalrounak
Copy link
Author

In service.yaml from the jitserver helm chart templates, name is defined as name: {{ include "openj9-jitserver-chart.fullname" . }} which translates to release-name-chart-name.

release-name is defined here in the file included in this PR as openj9jitserverchart-sample and chart-name comes from chart.yaml file in the jitserver helm chart where its value is openj9-jitserver-chart.

Thus, the name translates to openj9jitserverchart-sample-openj9-jitserver-chart.

@agarwalrounak
Copy link
Author

The release-name can be changed to openj9jitserver and name in service.yaml from the jitserver helm chart templates can be changed to {{ .Release.Name }}-service which would then translate to openj9jitserver-service. The same thing could then happen for deployment template as {{ .Release.Name }}-deployment.

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