Update mysql chart from 6.x to 9.x #412

Closed
pat-s wants to merge 2 commits from update-mysql into main
Member

In line with #391 and #407

In line with #391 and #407
pat-s added the
kind
breaking
label 2023-03-09 09:04:12 +00:00
pat-s added 2 commits 2023-03-09 09:04:13 +00:00
All checks were successful
continuous-integration/drone/push Build is passing
6ec445c3bc
update mysql chart
All checks were successful
continuous-integration/drone/push Build is passing
continuous-integration/drone/pr Build is passing
b2eab21714
revert yaml style changes
pat-s requested review from justusbunsi 2023-03-09 09:04:22 +00:00
justusbunsi requested changes 2023-03-23 18:25:17 +00:00
justusbunsi left a comment
Member

Please see my comments below.

Please see my comments below.
@ -6,2 +6,2 @@
repository: https://raw.githubusercontent.com/bitnami/charts/pre-2022/bitnami
version: 6.14.10
repository: oci://registry-1.docker.io/bitnamicharts
version: 9.5.2
Member

Current version would be 9.7.0. Sorry for the delay. :D

Current version would be 9.7.0. Sorry for the delay. :D
@ -455,4 +455,1 @@
name: gitea
service:
port: 3306
persistence:
Member

Based on version 6.14.10, this persistence override was never applied. It should've been master.persistence.size and slave.persistence.size. Since it was never applied the default 8Gi are used.

Specifying primary.persistence.size and secondary.persistence.size as 10Gi will change the size of the allocated storage. But since the Chart also changes their default (master+slave) to only one primary instance as default installation, they also use a different PVC name and everything must be migrated. So I am not fully sure if this is an actual breaking. But definitely noteworthy, I'd say.

Based on version 6.14.10, this persistence override was never applied. It should've been `master.persistence.size` and `slave.persistence.size`. Since it was never applied the default 8Gi are used. Specifying `primary.persistence.size` and `secondary.persistence.size` as 10Gi will change the size of the allocated storage. But since the Chart also changes their default (master+slave) to only one primary instance as default installation, they also use a different PVC name and everything must be migrated. So I am not fully sure if this is an actual breaking. But definitely noteworthy, I'd say.
@ -451,3 +450,1 @@
password: gitea
db:
user: gitea
auth:
Member

The current drone build seems to only render the Chart template for Postgres, which is enabled by default.
Running helm template --set postgresql.enabled=false --set mysql.enabled=true gitea ./ on this branch causes an error:

Error: template: gitea/templates/gitea/statefulset.yaml:23:28: executing "gitea/templates/gitea/statefulset.yaml" at <include (print $.Template.BasePath "/gitea/config.yaml") .>: error calling include: template: gitea/templates/gitea/config.yaml:9:6: executing "gitea/templates/gitea/config.yaml" at <include "gitea.inline_configuration" .>: error calling include: template: gitea/templates/_helpers.tpl:173:6: executing "gitea.inline_configuration" at <include "gitea.inline_configuration.defaults" .>: error calling include: template: gitea/templates/_helpers.tpl:226:6: executing "gitea.inline_configuration.defaults" at <include "gitea.inline_configuration.defaults.database" .>: error calling include: template: gitea/templates/_helpers.tpl:303:67: executing "gitea.inline_configuration.defaults.database" at <.Values.mysql.db.name>: nil pointer evaluating interface {}.name

A unittest like below would for each DB type identifies rendering issues immediately, so we don't need an explicit helm template at all. But that is something for #409.

suite: Statefulset template (mysql)
release:
  name: gitea-unittests
  namespace: testing
templates:
  - templates/gitea/statefulset.yaml
  - templates/gitea/config.yaml
tests:
  - it: renders a statefulset
    template: templates/gitea/statefulset.yaml
    set:
      postgresql.enabled: false
      mysql.enabled: true
    asserts:
      - hasDocuments:
          count: 1
      - containsDocument:
          kind: StatefulSet  
          apiVersion: apps/v1
          name: gitea-unittests
The current drone build seems to only render the Chart template for Postgres, which is enabled by default. Running `helm template --set postgresql.enabled=false --set mysql.enabled=true gitea ./` on this branch causes an error: ``` Error: template: gitea/templates/gitea/statefulset.yaml:23:28: executing "gitea/templates/gitea/statefulset.yaml" at <include (print $.Template.BasePath "/gitea/config.yaml") .>: error calling include: template: gitea/templates/gitea/config.yaml:9:6: executing "gitea/templates/gitea/config.yaml" at <include "gitea.inline_configuration" .>: error calling include: template: gitea/templates/_helpers.tpl:173:6: executing "gitea.inline_configuration" at <include "gitea.inline_configuration.defaults" .>: error calling include: template: gitea/templates/_helpers.tpl:226:6: executing "gitea.inline_configuration.defaults" at <include "gitea.inline_configuration.defaults.database" .>: error calling include: template: gitea/templates/_helpers.tpl:303:67: executing "gitea.inline_configuration.defaults.database" at <.Values.mysql.db.name>: nil pointer evaluating interface {}.name ``` A unittest like below would for each DB type identifies rendering issues immediately, so we don't need an explicit `helm template` at all. But that is something for #409. ```yaml suite: Statefulset template (mysql) release: name: gitea-unittests namespace: testing templates: - templates/gitea/statefulset.yaml - templates/gitea/config.yaml tests: - it: renders a statefulset template: templates/gitea/statefulset.yaml set: postgresql.enabled: false mysql.enabled: true asserts: - hasDocuments: count: 1 - containsDocument: kind: StatefulSet apiVersion: apps/v1 name: gitea-unittests ```
@ -458,2 +456,2 @@
persistence:
size: 10Gi
ports:
mysql: 3306
Member

mysql.service.ports.mysql must be mysql.primary.service.ports.mysql and secondary respectively. And the helpers.tpl files must be updated as well. Otherwise, the database configuration for app.ini would now render HOST=gitea-mysql.default.svc.cluster.local:%!g(<nil>) instead of HOST=gitea-mysql.default.svc.cluster.local:3306.

`mysql.service.ports.mysql` must be `mysql.primary.service.ports.mysql` and _secondary_ respectively. And the _helpers.tpl_ files must be updated as well. Otherwise, the database configuration for app.ini would now render `HOST=gitea-mysql.default.svc.cluster.local:%!g(<nil>)` instead of `HOST=gitea-mysql.default.svc.cluster.local:3306`.
Author
Member

Closed in favor of #417.

Closed in favor of #417.
pat-s closed this pull request 2023-03-25 10:07:37 +00:00
pat-s deleted branch update-mysql 2023-03-27 17:36:34 +00:00
Some checks are pending
continuous-integration/drone/push Build is passing
continuous-integration/drone/pr Build is passing
check-and-test / check-and-test (pull_request)
Required

Pull request closed

Sign in to join this conversation.
No description provided.