[stable/mattemost-team-edition] Add initial charts for Mattermost Team Edition #5987
Conversation
k8s-ci-robot
added
needs-ok-to-test
| "DataRetentionSettings": { | ||
| "Enable": false | ||
| } | ||
| } |
There was a problem hiding this comment.
Might be a good idea to remove any of the enterprise related settings so it's a little cleaner as those won't be needed in the chart
| # Mattermost configuration: | ||
| config: | ||
| SiteUrl: "http://mattermost.example.com" | ||
| SiteName: "MMTE" |
There was a problem hiding this comment.
Maybe default this to just Mattermost
|
|
||
| ## Introduction | ||
|
|
||
| This chart creates a [Mattermost](https://mattermost.com/) deployment on a [Kubernetes](http://kubernetes.io) |
There was a problem hiding this comment.
Might be a good idea to stress that it's team edition so:
This chart creates a Mattermost deployment...
to
This chart creates a Mattermost Team Edition deployment...
|
/assign @lachie83 |
cpanato
changed the title
|
Thanks for the chart! I wonder if it is possible to define a configmap generated by the |
| subPath: config.json | ||
| resources: | ||
| {{ toYaml .Values.resources | indent 12 }} | ||
| volumes: |
There was a problem hiding this comment.
should there be an option for /mattermost/data be persistent with a pvc?
There was a problem hiding this comment.
for the data we are using minio
There was a problem hiding this comment.
ahh, sorry minio is for the other edition. Will add the persistence for that 😄 thanks for the catch!
There was a problem hiding this comment.
For people using hosted sql, it'd be useful if this chart provides extraContainers and extraVolumes to configure cloud proxy. See the keycloak chart for example
| FilesAccessKey: | ||
| FilesSecretKey: | ||
| FileBucketName: | ||
| SMTPHost: |
There was a problem hiding this comment.
it seem this should be SMTPServer according to config.tpl
|
hello @clkao thanks for all review and feedback!
|
|
@clkao for this request:
I think it is better to add in a follow up, we need to change how we handle our configuration and this change will allow better manipulation. For now I will put this as TODO. is that ok for you? |
|
I've added a few things including postgresql support, recreate on config changes, and the required hooks for cloud sql proxy. Let me push somewhere for you to see if the changes can be incorporated. |
|
@clkao Hello, I was reading more about the cloud sql proxy that you mentioned, and the problem for that it is limited only to Google cloud i thought that was more generic. |
|
Hi @cpanato I made a few improvements in separate commits so it should be easier for you to review and cherry-pick from: pull/5987/head...clkao:mattermost |
|
hi @clkao sorry for the delay I was in vacations. I cherry pick all suggestions thanks!! |
There was a problem hiding this comment.
Please use a consistent list indentation style throughout the chart. Either use zero or two spaces.
Why does the chart have dependencies on kube-lego and nginx-ingress? You don't install these on a per-chart basis. You usually have multiple services use the same ingress controller. Also, the ingress that's included is tied to nginx-ingress. What if someone wants to use a different ingress controller? kube-lego is deprecated. I would rather use cert-manager, but again, not on a per-chart basis. I'd suggest you remove these dependencies.
| protocol: TCP | ||
| name: {{ template "mattermost-team-edition.name" . }} | ||
| selector: | ||
| app: {{ template "mattermost-team-edition.name" . }} |
There was a problem hiding this comment.
Add release label to selector.
There was a problem hiding this comment.
my fault added only in the label.
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{template "mattermost-team-edition.fullname" .}}-tests |
| {{- if .Values.persistence.data.enabled }} | ||
| kind: PersistentVolumeClaim | ||
| apiVersion: v1 | ||
| metadata: |
| name: {{template "mattermost-team-edition.fullname" .}}-config-json | ||
| labels: | ||
| chart: "{{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}" | ||
| app: {{ template "mattermost-team-edition.fullname" . }} |
There was a problem hiding this comment.
Add release and heritage labels.
| kind: Service | ||
| metadata: | ||
| name: {{ template "mattermost-team-edition.name" . }} | ||
| labels: |
There was a problem hiding this comment.
Add release and heritage labels.
| chart: "{{ .Chart.Name }}-{{ .Chart.Version | replace "+" "_" }}" | ||
| spec: | ||
| selector: | ||
| app: {{ template "mattermost-team-edition.name" . }} |
There was a problem hiding this comment.
Add release label to selector.
| apiVersion: v1 | ||
| kind: Pod | ||
| metadata: | ||
| name: "{{template "mattermost-team-edition.fullname" .}}-test-{{ randAlphaNum 5 | lower }}" |
|
/assign |
|
@unguiculus thanks so much for your review and the ingress/kubelego makes sense. removed. ptal |
| description: Mattermost Team Edition server. | ||
| name: mattermost-team-edition | ||
| version: 0.1.0 | ||
| appVersion: 5.0.0 |
|
|
||
| # Mattermost configuration: | ||
| config: | ||
| SiteUrl: "http://mattermost.example.com" |
There was a problem hiding this comment.
Convention is to use lower camel case.
| volumes: | ||
| - name: config-json | ||
| configMap: | ||
| name: {{template "mattermost-team-edition.fullname" .}}-config-json |
There was a problem hiding this comment.
Please be consistent. Add a space after {{ am before }}. Apply throughout the chart.
| app: {{ template "mattermost-team-edition.fullname" . }} | ||
| release: {{ .Release.Name }} | ||
| heritage: {{ .Release.Service }} | ||
| annotations: |
There was a problem hiding this comment.
Don't set nginx-ingress specific annotations by default. Instead provide an option to configure annotations. You may add these defaults to values.yaml and comment them out.
| heritage: {{ .Release.Service }} | ||
| spec: | ||
| selector: | ||
| app: {{ template "mattermost-team-edition.name" . }} |
There was a problem hiding this comment.
Add release label to selector.
| protocol: TCP | ||
| name: {{ template "mattermost-team-edition.name" . }} | ||
| selector: | ||
| app: {{ template "mattermost-team-edition.name" . }} |
| enabled: false | ||
| path: / | ||
| annotations: | ||
| kubernetes.io/ingress.class: nginx |
mattfarina
added
the
Contribution Allowed
cpanato
changed the title
ey-bot
added
Contribution Allowed
ey-bot
added
the
Contribution Allowed
| @@ -0,0 +1,76 @@ | |||
| apiVersion: apps/v1beta2 | |||
| mysqlRootPassword: root_password | ||
| mysqlUser: mmuser | ||
| mysqlPassword: mmpasswd | ||
| mysqlDatabase: mattermost |
There was a problem hiding this comment.
Don't set a default password.
| image: | ||
| repository: mattermost/mattermost-team-edition | ||
| tag: 5.2.1 | ||
| imagePullPolicy: Always |
| ## Default: volume.alpha.kubernetes.io/storage-class: default | ||
| ## | ||
| # storageClass: | ||
| accessMode: ReadWriteOnce |
There was a problem hiding this comment.
What if I run multiple replicas? How would Mattermost behave?
There was a problem hiding this comment.
For team edition, you can run just one instance.
There was a problem hiding this comment.
made note in the Readme.md as limitation.
ey-bot
added
the
Contribution Allowed
Signed-off-by: cpanato <ctadeu@gmail.com>
Signed-off-by: cpanato <ctadeu@gmail.com>
ey-bot
added
the
Contribution Allowed
Signed-off-by: cpanato <ctadeu@gmail.com>
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cpanato, unguiculus The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
@unguiculus thanks for all the review! |
* superset-annotations: (27 commits) [stable/superset] Bump version to 0.1.3 [stable/superset] Add service annotations support [stable/drupal] Fix chart not being upgradable (helm#7825) [stable/parse] Fix chart not being upgradable (helm#7824) [stable/jasperreports] Fix chart not being upgradable (helm#7818) [stable/osclass] Fix chart not being upgradable (helm#7817) [stable/phpbb] Fix chart not being upgradable (helm#7820) [stable/ghost] Fix chart not being upgradable (helm#7814) [stable/suitecrm] Fix chart not being upgradable (helm#7816) [stable/phpmyadmin] Fix chart not being upgradable (helm#7830) [stable/magento] Release 2.0.6 (helm#7810) [stable/wordpress] Fix chart not being upgradable (helm#7831) [stable/mongodb] Use .Values.existingSecret for standalone deployments (helm#7839) [incubator/kafka]: add reassignPartitions to topic configuration (helm#7623) Changed syntax error in custom-metrics-apiserver-service and secret (label to labels) (helm#7295) [stable/spinnaker] Changing email of dwardu89 (helm#7838) fix: mongo init issues (helm#7772) [stable/mattemost-team-edition] Add initial charts for Mattermost Team Edition (helm#5987) Fixed TLS Ingress, updated mongo dep requirements and app to latest version (helm#7636) Adding dwardu89 to owners and chart maintainers of stable/spinnaker (helm#7751) ...
…m Edition (helm#5987) * Add initial charts for Mattermost Team Edition Signed-off-by: cpanato <ctadeu@gmail.com> * update based on feedback Signed-off-by: cpanato <ctadeu@gmail.com> * update dependencies Signed-off-by: cpanato <ctadeu@gmail.com> Signed-off-by: jenkin-x <jicowan@hotmail.com>
…m Edition (helm#5987) * Add initial charts for Mattermost Team Edition Signed-off-by: cpanato <ctadeu@gmail.com> * update based on feedback Signed-off-by: cpanato <ctadeu@gmail.com> * update dependencies Signed-off-by: cpanato <ctadeu@gmail.com> Signed-off-by: Jakob Niggel <info@jakobniggel.de>
…m Edition (helm#5987) * Add initial charts for Mattermost Team Edition Signed-off-by: cpanato <ctadeu@gmail.com> * update based on feedback Signed-off-by: cpanato <ctadeu@gmail.com> * update dependencies Signed-off-by: cpanato <ctadeu@gmail.com>
What this PR does / why we need it:
Add Mattermost Team Edition to Helm charts
Special notes for your reviewer:
Please let me know if need any clarifications