backup cronjob related helm chart problems

[yteraoka]

Overview of the Issue

When I tried to enable backup cron then I found 2 problems in helm chart.

Reproduction Steps

Steps to reproduce this issue:

Problem 1 : nil pointer evaluating interface {}.cron

1. Append below config to examples/helm/101_initial_cluster.yaml

   config:
     backup:
       enabled: true

2. Execute helm template to check generated yaml

   cd examples/helm
   helm template ../../helm/vitess -f 101_initial_cluster.yaml
   

   Then, you get an error.

   Error: render error in "vitess/templates/vitess.yaml": template: vitess/templates/vitess.yaml:69:11: executing "vitess/templates/vitess.yaml" at <include "vttablet" (tuple $.Values.topology $cell $keyspace $shard $tablet $.Values.vttablet $.Values.vtctlclient $.Release.Namespace $.Values.config $.Values.pmm $.Values.orchestrator)>: error calling include: template: vitess/templates/_vttablet.tpl:156:3: executing "vttablet" at <include "vttablet-backup-cron" (tuple $cellClean $keyspaceClean $shardClean $shardName $keyspace $shard $vitessTag $config.backup $namespace $defaultVtctlclient)>: error calling include: template: vitess/templates/_cron-jobs.tpl:33:21: executing "vttablet-backup-cron" at <$shard.backup.cron.schedule>: nil pointer evaluating interface {}.cron
   

   $shard object doesn't have cron object. Whare is per shard cron config defined ?

Problem 2 : Duplicated CronJob

1. Edit helm/vitess/templates/_cron-jobs.tpl to do workarround for problem 1

   diff --git a/helm/vitess/templates/_cron-jobs.tpl b/helm/vitess/templates/_cron-jobs.tpl
   index 3b4ffd138..2b3977b96 100644
   --- a/helm/vitess/templates/_cron-jobs.tpl
   +++ b/helm/vitess/templates/_cron-jobs.tpl
   @@ -30,9 +30,9 @@ metadata:
        backupJob: "true"
   
    spec:
   -  schedule: {{ $shard.backup.cron.schedule | default $backup.cron.schedule | quote }}
   +  schedule: {{ $backup.cron.schedule | quote }}
      concurrencyPolicy: Forbid
   -  suspend: {{ $shard.backup.cron.suspend | default $backup.cron.suspend }}
   +  suspend: {{ $backup.cron.suspend }}
      successfulJobsHistoryLimit: 3
      failedJobsHistoryLimit: 20

2. Execute helm template again

   cd example/helm
   helm template ../../helm/vitess -f 101_initial_cluster.yaml | grep -A 10 CronJob
   

   Outputs:

   kind: CronJob
   metadata:
     name: zone1-commerce-0-backup
     labels:
       app: vitess
       component: vttablet
       cell: "zone1"
       keyspace: "commerce"
       shard: "0"
       backupJob: "true"
   
   --
   kind: CronJob
   metadata:
     name: zone1-commerce-0-backup
     labels:
       app: vitess
       component: vttablet
       cell: "zone1"
       keyspace: "commerce"
       shard: "0"
       backupJob: "true"
   

   2 same CronJob are generated.

3. Execute helm install

   helm install ../../helm/vitess -f 101_initial_cluster.yaml
   

   Got an error.

   Error: release romping-gibbon failed: cronjobs.batch "zone1-commerce-0-backup" already exists
   

   include "vttablet-backup-cron" is called in each replica and rdonly types.
   Therefore, it generate same CronJob named {cell}-{keyspace}-{shard}-backup twice.

   I think this cronjob should be generated for replica tablet only. like below.

   {{ if eq $tablet.type "replica" }}
   {{ include "vttablet-backup-cron" (tuple $cellClean $keyspaceClean $shardClean $shardName $keyspace $shard $vitessTag $config.backup $namespace $defaultVtctlclient) }}
   {{ end}}
   

   At
   

   vitess/helm/vitess/templates/_vttablet.tpl

   Lines 155 to 156 in edb59bd

   	# conditionally add cron job
   	{{ include "vttablet-backup-cron" (tuple $cellClean $keyspaceClean $shardClean $shardName $keyspace $shard $vitessTag $config.backup $namespace $defaultVtctlclient) }}

Operating system and Environment details

Minikube on macOS Catalina.

❯ uname -a       
Darwin YoshinorinoMacBook-Pro.local 19.3.0 Darwin Kernel Version 19.3.0: Thu Jan  9 20:58:23 PST 2020; root:xnu-6153.81.5~1/RELEASE_X86_64 x86_64

❯ minikube version
minikube version: v1.7.3
commit: 436667c819c324e35d7e839f8116b968a2d0a3ff

❯ helm version
Client: &version.Version{SemVer:"v2.16.1", GitCommit:"bbdfe5e7803a12bbdf97e94cd847859890cf4050", GitTreeState:"clean"}
Server: &version.Version{SemVer:"v2.16.1", GitCommit:"bbdfe5e7803a12bbdf97e94cd847859890cf4050", GitTreeState:"clean"}

❯ kubectl version --short 
Client Version: v1.15.7
Server Version: v1.15.7

minikube start \
  --kubernetes-version=v1.15.7 \
  --cpus=4 \
  --memory=6gb \
  --disk-size=20gb \
  --vm-driver=hyperkit

[mattlord]

I'm closing this as the Helm charts were deprecated in Vitess 7.0.

I opened a PR to remove the related code from the Vitess repo. Thanks!