-
Notifications
You must be signed in to change notification settings - Fork 448
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
[artifactory-ha] Add Annotations to Primary Artifactory Service and Resources to Nginx Deployment #1862
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
@@ -79,6 +79,8 @@ spec: | |||
volumeMounts: | |||
- mountPath: {{ .Values.nginx.persistence.mountPath | quote }} | |||
name: nginx-volume | |||
resources: | |||
{{ toYaml .Values.nginx.resources | indent 10 }} |
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.
maybe "{{ toYaml .Values.initContainers.resources | indent 10 }}" better? this is init container
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.
Yes, you're right. Just updated it, thanks for pointing it out
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.
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.
this is already fixed and avialble in current main branch
https://github.com/jfrog/charts/blob/master/stable/artifactory-ha/templates/nginx-deployment.yaml#L79
I used it already and it works fine for me.
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.
Hi @se-kai , thank you for the update. Appreciate having resources block added to nginx-deployment.
We also need annotations
added to artifactory-primary-service.yaml
's metadata
. It is not originally added and we can't use this chart as an upstream until that's added unfortunately. Can that be taken into account for the next release? Link for reference : https://github.com/jfrog/charts/blob/master/stable/artifactory-ha/templates/artifactory-primary-service.yaml#L6
I can rebase and update this PR with just the artifactory-primary-service.yaml
changes required if you need.
…into sr-add-annotations merge with main
…esources to Nginx Deployment (#1862)
Add Annotations to primary artifactory
service
Add resources to
setup
container in the nginx deployment yamlPR Checklist
[artifactory]
)What this PR does / why we need it:
Add Annotations to Primary Artifactory Service: This change adds annotations to the primary Artifactory service.
Add resources block to setup container in nginx deployment yaml: This change gives the ability to change limits and requests for the
setup
init container within the nginx deployment.