[kong] rework migration job configuration

[rainest]

What this PR does / why we need it:

• Create a new migrations section in values.yaml.
• Add configurable annotations to migration jobs, with default annotations to disable Istio and Kuma sidecars.
• Splits runMigrations into per-migration toggles under migrations.
• Points NOTES.txt deprecation warnings to the relevant UPGRADE.md section rather than explaining the issue in place.
• Add documentation for split deployments and init conflict issue.

This addresses several different issues:

• Migration annotations address an issue with sidecar containers blocking job completion (kong: Ability to add custom annotations to jobs #19).
• Avoiding the init job conflict ("helm upgrade" fails due to init-migrations job conflict #43)
• Allowing users to disable pre/post-upgrade migrations is needed for deployments that split different role nodes across multiple releases.

Which issue this PR fixes

Fixes #19
Fixes #43

Special notes for your reviewer:

This supersedes #101

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

• PR is based off the current tip of the next branch and targets next, not master
• New or modified sections of values.yaml are documented in the README.md
• Title of the PR and commit headers start with chart name (e.g. [kong])

[rainest]

20f853f is mostly doc changes requested above. It has one major functionality change: the init job handling is now fully automated and avoids the conflict issue. It should be squashed into 857215e with an update to the original message.

Edit: git commit --fixup does not work how I think. 6913d3d should be squashed also.

I tried to make migrations.init effectively a hidden setting, i.e. it's not listed in values.yaml but does exist, and if set to false in values.yaml would indeed disable that job. For some reason that's not clear to me, the default in the template appears to always apply even when that is set, so the job cannot be disabled.

I've left the default in there since it'd otherwise rely on runMigrations only, with the net effect that the job is never disabled, even if you have runMigrations: false. However, with the addition of the .Release.IsInstall condition, I don't think this should be a concern: there really shouldn't be any reason to ever disable the init migrations job on install, and worst case (you have an already-initialized database), it simply exits without doing anything.

[hbagdi]

Question: Do we need to clarify with a doc line saying this is only valid for Kong installations backed by DB?
The reason I ask is because it is not totally obvious from just reading this.

[rainest]

Not really--the job templates already disable themselves if DB-less is in use, and no changes are required to this config for it.

[chenguang-Steve]

Hi @hbagdi @rainest
I see the below changes are closed and not included in latest release. I want to trigger migrations-post-upgrade.yaml and migrations-pre-upgrade.yaml when using helm install. But currently these are only triggered by helm upgrade. Is it possible add another hook annotations like helm.sh/hook: "pre-install" in migrations-pre-upgrade.yaml and helm.sh/hook: "post-install" in migrations-pre-upgrade.yaml

annotations:
helm.sh/hook: "post-upgrade"
helm.sh/hook-delete-policy: "before-hook-creation"
{{- range $key, $value := .Values.migrationAnnotations }}
{{ $key }}: {{ $value | quote }}
{{- end }}

[rainest]

Could you elaborate? That's by design, as there's no reason for migrations up and migrations finish to roun outside the context of an upgrade.

Installs are handled using a different command (migrations bootstrap) with its own Job: https://github.com/Kong/charts/blob/master/charts/kong/templates/migrations.yaml. That's essentially always enabled (it does in fact have its own enable/disable toggle, but we don't list it in values.yaml, as it should never be necessary to disable that Job).

[chenguang-Steve]

Hi, thanks for your quick response.
The kong is integrated into my project. When project execute "stop" commands, then stop kong(helm delete), when project start then start kong(helm install)
The steps:

1. project start(helm install kong), migrations command execute successfully. (migrations bootstrap)
2. project stop(helm delete kong)
3. Add additional plugin migration.
4. project start(helm install kong)
   The kong failed to start because of below errors.

waiting for db
Error: /usr/local/share/lua/5.1/kong/cmd/start.lua:64: nginx: [error] init_by_lua error: /usr/local/share/lua/5.1/kong/cmd/utils/migrations.lua:20: New migrations available; run 'kong migrations up' to proceed
stack traceback:
[C]: in function 'error'
/usr/local/share/lua/5.1/kong/cmd/utils/migrations.lua:20: in function 'check_state'
/usr/local/share/lua/5.1/kong/init.lua:411: in function 'init'
init_by_lua:3: in main chunk
lua:3: in main chunk

I guess when add new plugin migrations based on already bootstrapped , we have to execute "migrations up"?
That is why I want execute to pre-update job when install. But this may does not work.
So for my case, possibly always execute helm install and helm upgrade when start my project?

[rainest]

@chenguang-Steve yeah--that's an atypical strategy we wouldn't normally expect to support: we'd normally expect users to have any custom plugins present when running helm install (which would then run their migrations during the kong bootstrap job) or to run helm upgrade when adding new plugins that require migrations.

Is there any reason you perform a full delete and re-install for this? I'd normally expect a helm upgrade in the scenario you describe, and don't see any reason it wouldn't achieve the same end.

You could probably perform a full delete and re-install when adding the additional plugin if you also reset your database or delete its PVC after deletion (note: don't do this if you aren't using the ingress controller or decK to manage configuration--it will destroy all configuration added manually through the admin API). However, it normally shouldn't be necessary to do so--an upgrade should suffice for most use cases. If there's a case where it doesn't, I'd be curious to know--it may be something we can handle better with improvements elsewhere.