Added Helm Chart - #7
Conversation
mlorenzofr
left a comment
mlorenzofr
left a comment
There was a problem hiding this comment.
it looks good
There are a few minor things that could be added, but they aren't important.
I understand that more specific gitops features, like the Argo sync-wave annotations, aren't relevant to this chart
| database: | ||
| name: tasks | ||
| user: qtodo_user | ||
| password: "RedH@tPassw0rd" |
There was a problem hiding this comment.
Here I would add a comment suggesting that ESO be used in production
Something like:
| password: "RedH@tPassw0rd" | |
| password: "RedH@tPassw0rd" # WARNING: For local testing only. Use External Secrets Operator in production. |
There was a problem hiding this comment.
I'd prefer to see an ExternalSecret object here, so that users actually could deploy the external secret that way. Personally, I'd make the ES the default path and provide an option to provide a secret using just values for someone who wants to do that.
| applicationType: "web-app" | ||
|
|
||
| # OpenShift Route configuration | ||
| route: |
There was a problem hiding this comment.
In route.yaml we reference a variable route.host in lines L9-11
do we want to add some examples here?
| | qtodo.resources.requests.memory | string | `"256Mi"` | | | ||
| | qtodo.securePort | int | `8443` | | | ||
| | route.enabled | bool | `true` | | | ||
| | route.path | string | `"/"` | | |
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ include "qtodo.fullname" . }} |
There was a problem hiding this comment.
We can set it with helm install --namespace, but adding the namespace avoids problems if some tool don't pass the namespace argument.
| name: {{ include "qtodo.fullname" . }} | |
| name: {{ include "qtodo.fullname" . }} | |
| namespace: {{ .Release.Namespace }} |
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: {{ include "qtodo.fullname" . }}-postgresql |
There was a problem hiding this comment.
| name: {{ include "qtodo.fullname" . }}-postgresql | |
| name: {{ include "qtodo.fullname" . }}-postgresql | |
| namespace: {{ .Release.Namespace }} |
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
| name: {{ include "qtodo.fullname" . }}-qtodo |
There was a problem hiding this comment.
| name: {{ include "qtodo.fullname" . }}-qtodo | |
| name: {{ include "qtodo.fullname" . }}-qtodo | |
| namespace: {{ .Release.Namespace }} |
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: {{ include "qtodo.fullname" . }}-qtodo |
There was a problem hiding this comment.
| name: {{ include "qtodo.fullname" . }}-qtodo | |
| name: {{ include "qtodo.fullname" . }}-qtodo | |
| namespace: {{ .Release.Namespace }} |
| apiVersion: v1 | ||
| kind: ServiceAccount | ||
| metadata: | ||
| name: {{ include "qtodo.serviceAccountName" . }} |
There was a problem hiding this comment.
| name: {{ include "qtodo.serviceAccountName" . }} | |
| name: {{ include "qtodo.serviceAccountName" . }} | |
| namespace: {{ .Release.Namespace }} |
| apiVersion: route.openshift.io/v1 | ||
| kind: Route | ||
| metadata: | ||
| name: {{ include "qtodo.fullname" . }} |
There was a problem hiding this comment.
| name: {{ include "qtodo.fullname" . }} | |
| name: {{ include "qtodo.fullname" . }} | |
| namespace: {{ .Release.Namespace }} |
| capabilities: | ||
| drop: | ||
| - ALL | ||
| livenessProbe: |
There was a problem hiding this comment.
do we want to make livenessProbes/readinessProbes configurable in values.yaml, just like with PostgreSQL?
Signed-off-by: Andrew Block <andy.block@gmail.com>
|
@mlorenzofr thanks for your comments. i have made the requested updates. @mhjacks @day0hero if you guys could review the contents of this PR, it would be appreciated |
|
I'm curious what you would think about the sanity/desirability of adding an external-secret object as an example. I know that could be a thorny thing with dependencies. Is the idea then that the chart here would replace the one currently in the LZT pattern, or is the idea that this would be comepletely separate? @sabre1041 @mlorenzofr |
Right now, this chart would be separate from the LZT pattern. Longer term, we could assess the feasibility of a single chart. But the goal of this chart is to demonstrate a traditional microservices deployment without security features implemented where we can then demonstrate the value emphasized by the LZT pattern |
3 participants
Added Helm Chart