From 91c5fbfa4d41cd185c52fa1e6fbf7532b1f255d8 Mon Sep 17 00:00:00 2001 From: James Liu Date: Mon, 13 Jan 2025 16:29:43 +1100 Subject: [PATCH 1/2] gitaly: Migrate cgroups to `config` key Migrate the subset of cgroups keys that are used directly by Gitaly's config.toml into the new `config` section. Remove the `cgroups.enabled` option in favour of setting a positive integer for `cgroups.repositories.count`. This aligns with how cgroups are enabeld in Omnibus and via Gitaly's `config.toml`. Default values and placeholders are left as-is. Manual templating of the keys is also left untouched. A subsequent commit in this series will remove the manual templating and instead directly translate keys under `config` into Gitaly's `config.toml`. --- .../gitaly/templates/_configmap_spec.yaml | 38 +++++++++---------- .../gitaly/templates/_statefulset_spec.yaml | 6 +-- charts/gitlab/charts/gitaly/values.yaml | 26 +++++++------ spec/configuration/gitaly_spec.rb | 29 +++++++------- 4 files changed, 51 insertions(+), 48 deletions(-) diff --git a/charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml b/charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml index 92c75a7299..1fa4d68c74 100644 --- a/charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml +++ b/charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml @@ -176,42 +176,42 @@ data: go_cloud_url = "{{ .Values.backup.goCloudUrl }}" {{- end }} - {{- if .Values.cgroups.enabled }} - {{- with .Values.cgroups }} + {{- if gt (int64 .Values.config.cgroups.repositories.count) 0 }} + {{- with .Values.config.cgroups }} [cgroups] {{- if .mountpoint }} mountpoint = "{{ .mountpoint }}" {{- end }} - {{- if .hierarchyRoot }} - hierarchy_root = {{ .hierarchyRoot | quote }} + {{- if .hierarchy_root }} + hierarchy_root = {{ .hierarchy_root | quote }} {{- end }} - {{- if .memoryBytes }} - memory_bytes = {{ .memoryBytes | int64 }} + {{- if .memory_bytes }} + memory_bytes = {{ .memory_bytes | int64 }} {{- end }} - {{- if .cpuShares }} - cpu_shares = {{ .cpuShares | int64 }} + {{- if .cpu_shares }} + cpu_shares = {{ .cpu_shares | int64 }} {{- end }} - {{- if .cpuQuotaUs }} - cpu_quota_us = {{ .cpuQuotaUs | int64 }} + {{- if .cpu_quota_us }} + cpu_quota_us = {{ .cpu_quota_us | int64 }} {{- end }} {{- end }} - {{- with .Values.cgroups.repositories }} + {{- with .Values.config.cgroups.repositories }} [cgroups.repositories] {{- if .count }} count = {{ .count | int64 }} {{- end }} - {{- if .memoryBytes }} - memory_bytes = {{ .memoryBytes | int64 }} + {{- if .memory_bytes }} + memory_bytes = {{ .memory_bytes | int64 }} {{- end }} - {{- if .cpuShares }} - cpu_shares = {{ .cpuShares | int64 }} + {{- if .cpu_shares }} + cpu_shares = {{ .cpu_shares | int64 }} {{- end }} - {{- if .cpuQuotaUs }} - cpu_quota_us = {{ .cpuQuotaUs | int64 }} + {{- if .cpu_quota_us }} + cpu_quota_us = {{ .cpu_quota_us | int64 }} {{- end }} - {{- if .maxCgroupsPerRepo }} - max_cgroups_per_repo = {{ .maxCgroupsPerRepo | int64 }} + {{- if .max_cgroups_per_repo }} + max_cgroups_per_repo = {{ .max_cgroups_per_repo | int64 }} {{- end }} {{- end }} {{- end }} \ No newline at end of file diff --git a/charts/gitlab/charts/gitaly/templates/_statefulset_spec.yaml b/charts/gitlab/charts/gitaly/templates/_statefulset_spec.yaml index 0655eed4ca..089cd2b5f3 100644 --- a/charts/gitlab/charts/gitaly/templates/_statefulset_spec.yaml +++ b/charts/gitlab/charts/gitaly/templates/_statefulset_spec.yaml @@ -59,7 +59,7 @@ spec: {{- include "gitlab.priorityClassName" . | nindent 6 }} terminationGracePeriodSeconds: {{ .Values.gracefulRestartTimeout | int | add 5 }} initContainers: - {{- if .Values.cgroups.enabled }} + {{- if gt (int64 .Values.config.cgroups.repositories.count) 0 }} - name: init-cgroups image: {{ include "gitlab.configure.image" (dict "root" $ "image" .Values.cgroups.initContainer.image) | quote }} {{- include "gitlab.image.pullPolicy" $initCgroupsImageCfg | indent 10 }} @@ -155,7 +155,7 @@ spec: volumeMounts: {{- include "gitlab.extraVolumeMounts" . | nindent 12 }} {{- include "gitlab.certificates.volumeMount" . | nindent 12 }} - {{- if .Values.cgroups.enabled }} + {{- if gt (int64 .Values.config.cgroups.repositories.count) 0 }} - name: cgroup mountPath: '/run/gitaly/cgroup' {{- end }} @@ -217,7 +217,7 @@ spec: {{- include "gitlab.nodeSelector" . | nindent 6 }} volumes: {{- include "gitlab.extraVolumes" . | nindent 8 }} - {{- if .Values.cgroups.enabled }} + {{- if gt (int64 .Values.config.cgroups.repositories.count) 0 }} - name: cgroup hostPath: path: /sys/fs/cgroup diff --git a/charts/gitlab/charts/gitaly/values.yaml b/charts/gitlab/charts/gitaly/values.yaml index 4051387a1a..236f5c7d28 100644 --- a/charts/gitlab/charts/gitaly/values.yaml +++ b/charts/gitlab/charts/gitaly/values.yaml @@ -53,24 +53,12 @@ init: # Gitaly built-in cgroups, spawns Git processes into a cgroup per repository, protecting the service/pod from OOM events # ref: https://docs.gitlab.com/ee/administration/gitaly/configure_gitaly.html#control-groups cgroups: - enabled: false initContainer: image: repository: registry.gitlab.com/gitlab-org/build/cng/gitaly-init-cgroups securityContext: runAsUser: 0 runAsGroup: 0 - mountpoint: '{% file.Read "/etc/gitlab-secrets/gitaly-pod-cgroup" | strings.TrimSpace %}' - hierarchyRoot: gitaly - # memoryBytes: - # cpuShares: - # cpuQuotaUs: - repositories: {} - # count: - # memoryBytes: - # cpuShares: - # cpuQuotaUs: - # maxCgroupsPerRepo: ## Support for tolerations for pod scheduling tolerations: [] @@ -245,3 +233,17 @@ backup: {} # default enable gomemlimit to avoid gc related OOM errors gomemlimit: enabled: true + +config: + cgroups: + mountpoint: '{% file.Read "/etc/gitlab-secrets/gitaly-pod-cgroup" | strings.TrimSpace %}' + hierarchy_root: gitaly + # memory_bytes: + # cpu_shares: + # cpu_quota_us: + repositories: + count: 0 + # memory_bytes: + # cpu_shares: + # cpu_quota_us: + # max_cgroups_per_repo: diff --git a/spec/configuration/gitaly_spec.rb b/spec/configuration/gitaly_spec.rb index 98a9bc28a4..c6df14079d 100644 --- a/spec/configuration/gitaly_spec.rb +++ b/spec/configuration/gitaly_spec.rb @@ -569,30 +569,31 @@ describe 'Gitaly configuration' do gitlab: gitaly: cgroups: - enabled: #{cgroups_enabled} initContainer: image: repository: registry.gitlab.com/gitlab-org/build/cng/gitaly-init-cgroups tag: master pullPolicy: IfNotPresent - mountpoint: '{% file.Read "/etc/gitlab-secrets/gitaly-pod-cgroup" | strings.TrimSpace %}' - hierarchyRoot: gitaly - memoryBytes: 64424509440 - cpuShares: 1024 - cpuQuotaUs: 400000 - repositories: - count: 1000 - memoryBytes: 32212254720 - cpuShares: 512 - cpuQuotaUs: 200000 - maxCgroupsPerRepo: 2 + config: + cgroups: + mountpoint: '{% file.Read "/etc/gitlab-secrets/gitaly-pod-cgroup" | strings.TrimSpace %}' + hierarchy_root: gitaly + memory_bytes: 64424509440 + cpu_shares: 1024 + cpu_quota_us: 400000 + repositories: + count: #{cgroups_count} + memory_bytes: 32212254720 + cpu_shares: 512 + cpu_quota_us: 200000 + max_cgroups_per_repo: 2 )).deep_merge(default_values) end let(:gitaly_stateful_set) { 'StatefulSet/test-gitaly' } context 'when enabled' do - let(:cgroups_enabled) { true } + let(:cgroups_count) { 1000 } let(:template) { HelmTemplate.new(values) } let(:gitaly_config) { template.dig('ConfigMap/test-gitaly', 'data', 'config.toml.tpl') } @@ -633,7 +634,7 @@ describe 'Gitaly configuration' do end context 'when disabled' do - let(:cgroups_enabled) { false } + let(:cgroups_count) { 0 } let(:template) { HelmTemplate.new(values) } -- GitLab From b91f80068d2a11ebe1c6e510e430b54debe52f4d Mon Sep 17 00:00:00 2001 From: James Liu Date: Mon, 13 Jan 2025 17:17:10 +1100 Subject: [PATCH 2/2] gitaly: Migrate backup to `config` key Migrate the backup keys that are used directly by Gitaly's config.toml into the new `config` section. Default values and placeholders are left as-is. Manual templating of the keys is also left untouched. A subsequent commit in this series will remove the manual templating and instead directly translate keys under `config` into Gitaly's `config.toml`. --- charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml | 4 ++-- charts/gitlab/charts/gitaly/values.yaml | 5 ++--- spec/configuration/gitaly_spec.rb | 5 +++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml b/charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml index 1fa4d68c74..ea5eec2c86 100644 --- a/charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml +++ b/charts/gitlab/charts/gitaly/templates/_configmap_spec.yaml @@ -171,9 +171,9 @@ data: {{- end }} {{- end }} - {{- if .Values.backup.goCloudUrl }} + {{- if .Values.config.backup.go_cloud_url }} [backup] - go_cloud_url = "{{ .Values.backup.goCloudUrl }}" + go_cloud_url = "{{ .Values.config.backup.go_cloud_url }}" {{- end }} {{- if gt (int64 .Values.config.cgroups.repositories.count) 0 }} diff --git a/charts/gitlab/charts/gitaly/values.yaml b/charts/gitlab/charts/gitaly/values.yaml index 236f5c7d28..1d23f1f7d9 100644 --- a/charts/gitlab/charts/gitaly/values.yaml +++ b/charts/gitlab/charts/gitaly/values.yaml @@ -227,14 +227,13 @@ gpgSigning: {} # secret: glGPG # key: -backup: {} - # goCloudUrl: - # default enable gomemlimit to avoid gc related OOM errors gomemlimit: enabled: true config: + backup: {} + # go_cloud_url: cgroups: mountpoint: '{% file.Read "/etc/gitlab-secrets/gitaly-pod-cgroup" | strings.TrimSpace %}' hierarchy_root: gitaly diff --git a/spec/configuration/gitaly_spec.rb b/spec/configuration/gitaly_spec.rb index c6df14079d..34681bd363 100644 --- a/spec/configuration/gitaly_spec.rb +++ b/spec/configuration/gitaly_spec.rb @@ -463,8 +463,9 @@ describe 'Gitaly configuration' do YAML.safe_load(%( gitlab: gitaly: - backup: - goCloudUrl: 'gs://gitaly-backups' + config: + backup: + go_cloud_url: 'gs://gitaly-backups' )).deep_merge(default_values) end -- GitLab