diff --git a/charts/gitlab/charts/migrations/templates/_helpers.tpl b/charts/gitlab/charts/migrations/templates/_helpers.tpl index c7ff8dccdb601b70099246819c34e9295f157210..802251a7a74b5a31d9fe7053fd521b34022a9e4a 100644 --- a/charts/gitlab/charts/migrations/templates/_helpers.tpl +++ b/charts/gitlab/charts/migrations/templates/_helpers.tpl @@ -2,12 +2,23 @@ {{/* Create a default fully qualified job name. -Due to the job only being allowed to run once, we add the chart revision so helm -upgrades don't cause errors trying to create the already ran job. -Due to the helm delete not cleaning up these jobs, we add a randome value to -reduce collision +The name contains a hash that is based on the chart's app version +and the chart's values (which also might contain the global.gitlabVersion) +to make sure that the job is run at least once everytime GitLab is updated. + +In order to make sure that the hash is stable for `helm template` +and `helm upgrade --install`, we need to remove the `local` block injected +by the template file `charts/gitlab/templates/_databaseDatamodel.tpl`. + +This local block contains the values of the Helm "built-in object" +(see https://helm.sh/docs/chart_template_guide/builtin_objects) which would +result in different hash values due to fields like `Release.IsUpgrade`, +`Release.IsInstall` and especially `Release.Revision`. */}} {{- define "migrations.jobname" -}} +{{- $values := deepCopy .Values -}} +{{- $values := unset $values "local" -}} {{- $name := include "fullname" . | trunc 55 | trimSuffix "-" -}} -{{- printf "%s-%d" $name .Release.Revision | trunc 63 | trimSuffix "-" -}} +{{- $hash := printf "%s-%s" .Chart.AppVersion ( $values | toYaml | b64enc ) | sha256sum | trunc 7 }} +{{- printf "%s-%s" $name $hash | trunc 63 | trimSuffix "-" -}} {{- end -}} diff --git a/charts/gitlab/charts/migrations/templates/_jobspec.yaml b/charts/gitlab/charts/migrations/templates/_jobspec.yaml index 581e01e1e661aec97fdebaae7aee02c957b607b2..69aa00aebc1bb2d0d2b2ee46052ab6f4e6f58a5f 100644 --- a/charts/gitlab/charts/migrations/templates/_jobspec.yaml +++ b/charts/gitlab/charts/migrations/templates/_jobspec.yaml @@ -19,6 +19,9 @@ metadata: spec: activeDeadlineSeconds: {{ .Values.activeDeadlineSeconds }} backoffLimit: {{ .Values.backoffLimit }} + {{- with .Values.ttlSecondsAfterFinished }} + ttlSecondsAfterFinished: {{ . }} + {{- end }} template: metadata: {{- if or .Values.PodAnnotations .Values.annotations}} diff --git a/charts/gitlab/charts/migrations/values.yaml b/charts/gitlab/charts/migrations/values.yaml index 766faf68ff9226f244a8c62b7045a61af017f1af..b8ffa2b3d07881e560b8697081fb79b089735dde 100644 --- a/charts/gitlab/charts/migrations/values.yaml +++ b/charts/gitlab/charts/migrations/values.yaml @@ -74,9 +74,13 @@ resources: requests: cpu: 250m memory: 200Mi + activeDeadlineSeconds: 3600 backoffLimit: 6 +## Define time after which job gets deleted after finishing +# ttlSecondsAfterFinished: 60 + ## Allow to overwrite under which User and Group we're running. securityContext: runAsUser: 1000 diff --git a/spec/configuration/migrations_spec.rb b/spec/configuration/migrations_spec.rb index 550b15dec8541732081caed53ee123d8951fecf6..73e2f90dc26e1d53e0caa82c98e1619135f48c91 100644 --- a/spec/configuration/migrations_spec.rb +++ b/spec/configuration/migrations_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' require 'helm_template_helper' require 'yaml' require 'hash_deep_merge' +# require 'digest' +# require 'base64' describe 'migrations configuration' do let(:default_values) do @@ -17,6 +19,11 @@ describe 'migrations configuration' do )) end + let(:expected_hash) do + b64_encoded_yaml = Base64.encode64(YAML.dump(values)) + OpenSSL::Digest::SHA256.hexdigest("master-#{b64_encoded_yaml}")[0, 7] + end + context 'When customer provides additional labels' do let(:values) do YAML.safe_load(%( @@ -39,16 +46,20 @@ describe 'migrations configuration' do global: pod )).deep_merge(default_values) end + # let(:expected_hash) do + # Digest::SHA256.hexdigest(Base64.encode64 YAML.dump values)[0, 7] + # end + it 'Populates the additional labels in the expected manner' do t = HelmTemplate.new(values) expect(t.exit_code).to eq(0), "Unexpected error code #{t.exit_code} -- #{t.stderr}" expect(t.dig('ConfigMap/test-migrations', 'metadata', 'labels')).to include('global' => 'migrations') - expect(t.dig('Job/test-migrations-1', 'metadata', 'labels')).to include('foo' => 'global') - expect(t.dig('Job/test-migrations-1', 'metadata', 'labels')).to include('global' => 'migrations') - expect(t.dig('Job/test-migrations-1', 'metadata', 'labels')).not_to include('global' => 'global') - expect(t.dig('Job/test-migrations-1', 'spec', 'template', 'metadata', 'labels')).to include('global' => 'pod') - expect(t.dig('Job/test-migrations-1', 'spec', 'template', 'metadata', 'labels')).to include('pod' => 'true') - expect(t.dig('Job/test-migrations-1', 'spec', 'template', 'metadata', 'labels')).to include('global_pod' => 'true') + expect(t.dig("Job/test-migrations-#{expected_hash}", 'metadata', 'labels')).to include('foo' => 'global') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'metadata', 'labels')).to include('global' => 'migrations') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'metadata', 'labels')).not_to include('global' => 'global') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'spec', 'template', 'metadata', 'labels')).to include('global' => 'pod') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'spec', 'template', 'metadata', 'labels')).to include('pod' => 'true') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'spec', 'template', 'metadata', 'labels')).to include('global_pod' => 'true') expect(t.dig('ServiceAccount/test-migrations', 'metadata', 'labels')).to include('global' => 'migrations') end end @@ -66,15 +77,18 @@ describe 'migrations configuration' do baz: baz )).deep_merge(default_values) end + # let(:expected_hash) do + # Digest::SHA256.hexdigest(Base64.encode64 YAML.dump values)[0, 7] + # end it 'Populates the additional annotations in the expected manner' do t = HelmTemplate.new(values) expect(t.exit_code).to eq(0), "Unexpected error code #{t.exit_code} -- #{t.stderr}" - expect(t.dig('Job/test-migrations-1', 'metadata', 'annotations')).to include('foo' => 'bar') - expect(t.dig('Job/test-migrations-1', 'metadata', 'annotations')).to include('bar' => 'foo') - expect(t.dig('Job/test-migrations-1', 'metadata', 'annotations')).not_to include('baz' => 'baz') - expect(t.dig('Job/test-migrations-1', 'spec', 'template', 'metadata', 'annotations')).to include('foo' => 'foo') - expect(t.dig('Job/test-migrations-1', 'spec', 'template', 'metadata', 'annotations')).to include('bar' => 'foo') - expect(t.dig('Job/test-migrations-1', 'spec', 'template', 'metadata', 'annotations')).to include('baz' => 'baz') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'metadata', 'annotations')).to include('foo' => 'bar') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'metadata', 'annotations')).to include('bar' => 'foo') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'metadata', 'annotations')).not_to include('baz' => 'baz') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'spec', 'template', 'metadata', 'annotations')).to include('foo' => 'foo') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'spec', 'template', 'metadata', 'annotations')).to include('bar' => 'foo') + expect(t.dig("Job/test-migartions-#{expected_hash}", 'spec', 'template', 'metadata', 'annotations')).to include('baz' => 'baz') end end end