[stable/artifactory] Postgresql requirement / ingress support / nginx optional #3242
Conversation
…stgresql. Updated artifactory version number to 5.8.0. Updated notes and added ingress documentation. Added Ingress support to artifactory. Made the NGINX deployment optional by adding an enabled flag. Fixed naming of components.
k8s-ci-robot
added
needs-ok-to-test
|
@jainishshah17 @eldada please review. |
|
@joosthof - can you wait until we merge #3200 ? It has a few changes that you'll need to merge with before applying your PR (which looks like a good path to take). |
|
@joosthof - #3200 is merged. Please update your PR. |
|
@eldada PR updated. |
|
@jainishshah17 Fixed the chart version |
stable/artifactory/values.yaml
Outdated
| postgresPassword: "artifactory" | ||
| postgresDatabase: "artifactory" | ||
| persistence: | ||
| enabled: false |
There was a problem hiding this comment.
Since Artifactory is configure with persistence by default, I'd like the DB to also set like this.
stable/artifactory/Chart.yaml
Outdated
| @@ -1,7 +1,7 @@ | |||
| apiVersion: v1 | |||
| name: artifactory | |||
| home: https://www.jfrog.com/artifactory/ | |||
| version: 6.2.5 | |||
| version: 6.3.5 | |||
stable/artifactory/values.yaml
Outdated
| @@ -94,6 +70,7 @@ artifactory: | |||
|
|
|||
| # Nginx | |||
| nginx: | |||
| enabled: false | |||
There was a problem hiding this comment.
I think either nginx or ingress should be enabled. Both are set to enabled: false. Keeping with current chart, I suggest you leave the nginx enabled: true.
stable/artifactory/values.yaml
Outdated
| @@ -65,7 +41,7 @@ artifactory: | |||
| internalPort: 8081 | |||
| persistence: | |||
| mountPath: "/var/opt/jfrog/artifactory" | |||
| enabled: true | |||
| enabled: false | |||
There was a problem hiding this comment.
Artifactory in its nature is still a stateful app (although we hope to change this in the future). I think the default should stay true here.
| * Deploy a PostgreSQL database | ||
| * Deploy an Nginx server | ||
| * Deploy a PostgreSQL database using the stable/postgresql chart | ||
| * Deploy an optional Nginx server |
There was a problem hiding this comment.
Add the optional ingress.
Also, add a documentation section on the ingress and its supported parameters.
- What it does
- Using the TLS secret
- Whitelisting IPs
There was a problem hiding this comment.
I added information about TLS secrets and how to use.
For the Ingress and whitelisting part I added a link to the kubernetes documentation.
stable/artifactory/requirements.yaml
Outdated
| @@ -0,0 +1,4 @@ | |||
| dependencies: | |||
| - name: postgresql | |||
| version: 0.8.3 | |||
There was a problem hiding this comment.
You can update to a later version
…istence default on true
|
/assign @sameersbn |
|
/approve |
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
@mattfarina Yes!! |
|
@jainishshah17 Then please submit a PR and go through the member process. You can use me as one of your references. The process for that is documented at https://github.com/kubernetes/community/blob/master/community-membership.md#member |
stable/artifactory/Chart.yaml
Outdated
| @@ -1,7 +1,7 @@ | |||
| apiVersion: v1 | |||
| name: artifactory | |||
| home: https://www.jfrog.com/artifactory/ | |||
| version: 6.2.5 | |||
| version: 6.3.1 | |||
There was a problem hiding this comment.
Because the API to the chart change (e.g., values.yaml file structure) in a backwards breaking way this should be 7.0.0 instead of a minor bump. That is to follow semver rules.
|
/approve |
k8s-ci-robot
added
the
approved
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eldada, jainishshah17, joosthof, sameersbn The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Removed Postgresql own deployment and created dependency to stable/Postgresql.
Updated artifactory version number to 5.8.0.
Updated notes and added ingress documentation.
Added Ingress support to artifactory.
Made the NGINX deployment optional by adding an enabled flag.
Fixed naming of components.