From 5ef9cb691c51beec57bc74d8bcce67e2c5be8dc7 Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Mon, 7 Nov 2022 11:39:38 +0100 Subject: [PATCH 1/2] Support configuring continuous profiling for Workhorse This was so far done by setting an environment variable at the pod level, which does not allow us to configure this for multiple containers and services. Moving this to the container level prevents these variables from clashing. Changelog: added --- charts/gitlab/charts/webservice/templates/deployment.yaml | 4 ++++ doc/charts/gitlab/webservice/index.md | 1 + 2 files changed, 5 insertions(+) diff --git a/charts/gitlab/charts/webservice/templates/deployment.yaml b/charts/gitlab/charts/webservice/templates/deployment.yaml index c30ab6683d..92c5437860 100644 --- a/charts/gitlab/charts/webservice/templates/deployment.yaml +++ b/charts/gitlab/charts/webservice/templates/deployment.yaml @@ -362,6 +362,10 @@ spec: - name: GITLAB_WORKHORSE_SENTRY_DSN value: {{ .workhorse.sentryDSN }} {{- end }} + {{- if .workhorse.continuousProfiling }} + - name: GITLAB_CONTINUOUS_PROFILING + value: {{ .workhorse.continuousProfiling }} + {{- end }} {{- include "gitlab.tracing.env" $ | nindent 12 }} {{- include "webservice.extraEnv" (dict "global" $.Values.global "local" .) | nindent 12 }} {{- include "gitlab.extraEnvFrom" (dict "root" $ "local" .) | nindent 12 }} diff --git a/doc/charts/gitlab/webservice/index.md b/doc/charts/gitlab/webservice/index.md index 6a4c1aac34..41b460299a 100644 --- a/doc/charts/gitlab/webservice/index.md +++ b/doc/charts/gitlab/webservice/index.md @@ -158,6 +158,7 @@ to the `helm install` command using the `--set` flags. | `workhorse.tls.verify` | `true` | When set to `true` forces NGINX Ingress to verify the TLS certificate of Workhorse. For custom CA you need to set `workhorse.tls.caSecretName` as well. Must be set to `false` for self-signed certificates. | | `workhorse.tls.secretName` | `{Release.Name}-workhorse-tls` | The name of the [TLS Secret](https://kubernetes.io/docs/concepts/configuration/secret/#tls-secrets) that contains the TLS key and certificate pair. This is required when Workhorse TLS is enabled. | | `workhorse.tls.caSecretName` | | The name of the Secret that contains the CA certificate. This **is not** a [TLS Secret](https://kubernetes.io/docs/concepts/configuration/secret/#tls-secrets), and must have only `ca.crt` key. This is used for TLS verification by NGINX. | +| `workhorse.continuousProfiling` | | A configuration string to enable [GitLab Continuous Profiling](https://gitlab.com/gitlab-org/labkit/-/blob/master/monitoring/doc.go) support. | | `webServer` | `puma` | Selects web server (Webservice/Puma) that would be used for request handling | | `priorityClassName` | `""` | Allow configuring pods `priorityClassName`, this is used to control pod priority in case of eviction | -- GitLab From 931cd515d242f5973ad141bade6375553fc5a37e Mon Sep 17 00:00:00 2001 From: Matthias Kaeppler Date: Mon, 7 Nov 2022 14:01:22 +0100 Subject: [PATCH 2/2] Add unit test --- .../webservice_deployments_spec.rb | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/spec/configuration/webservice_deployments_spec.rb b/spec/configuration/webservice_deployments_spec.rb index f06cd24f9c..b45545cf43 100644 --- a/spec/configuration/webservice_deployments_spec.rb +++ b/spec/configuration/webservice_deployments_spec.rb @@ -682,6 +682,55 @@ describe 'Webservice Deployments configuration' do end end + context 'workhorse continuous profiling' do + let(:expected_config) { { 'name' => 'GITLAB_CONTINUOUS_PROFILING', 'value' => 'stackdriver?service=workhorse' } } + let(:template) { HelmTemplate.new(deployments_values) } + let(:workhorse_env) do + containers = template.dig('Deployment/test-webservice-default', 'spec', 'template', 'spec', 'containers') + workhorse = containers.find { |c| c['name'] == 'gitlab-workhorse' } + workhorse['env'] + end + + context 'when not configured' do + let(:deployments_values) do + YAML.safe_load(%( + gitlab: + webservice: + deployments: + default: + ingress: + path: / + )).deep_merge(default_values) + end + + it 'does not set the GITLAB_CONTINUOUS_PROFILING environment variable' do + expect(template.exit_code).to eq(0), "Unexpected error code #{template.exit_code} -- #{template.stderr}" + expect(workhorse_env).not_to include(expected_config) + end + end + + context 'when configured' do + let(:deployments_values) do + YAML.safe_load(%( + gitlab: + webservice: + deployments: + default: + workhorse: + continuousProfiling: stackdriver?service=workhorse + ingress: + path: / + )).deep_merge(default_values) + end + + it 'sets the GITLAB_CONTINUOUS_PROFILING environment variable' do + expect(template.exit_code).to eq(0), "Unexpected error code #{template.exit_code} -- #{template.stderr}" + + expect(workhorse_env).to include(expected_config) + end + end + end + context 'when emptyDir is customized' do let(:deployments_values) do YAML.safe_load(%( -- GitLab