From d9b39f5d2ba512d9874dd22e9274cc99d152ec93 Mon Sep 17 00:00:00 2001 From: Diego Abelenda Date: Tue, 3 Sep 2024 13:44:52 +0200 Subject: [PATCH 1/3] Allow to pass allocateLoadBalancerNodePorts in Service for SSH Fixes #5721 --- charts/gitlab/charts/gitlab-shell/templates/service.yaml | 3 +++ charts/gitlab/charts/gitlab-shell/values.schema.json | 4 ++++ charts/gitlab/charts/gitlab-shell/values.yaml | 1 + doc/charts/gitlab/gitlab-shell/index.md | 1 + 4 files changed, 9 insertions(+) diff --git a/charts/gitlab/charts/gitlab-shell/templates/service.yaml b/charts/gitlab/charts/gitlab-shell/templates/service.yaml index af75b49709..049b6e66b6 100644 --- a/charts/gitlab/charts/gitlab-shell/templates/service.yaml +++ b/charts/gitlab/charts/gitlab-shell/templates/service.yaml @@ -29,6 +29,9 @@ spec: {{- if or (eq .Values.service.type "NodePort") (eq .Values.service.type "LoadBalancer") }} externalTrafficPolicy: {{ .Values.service.externalTrafficPolicy }} {{- end }} + {{- if and (eq .Values.service.type "LoadBalancer") (hasKey .Values.service "allocateLoadBalancerNodePorts") }} + allocateLoadBalancerNodePorts: {{ .Values.service.allocateLoadBalancerNodePorts }} + {{- end }} {{- if .Values.service.loadBalancerIP }} loadBalancerIP: {{ .Values.service.loadBalancerIP }} {{- end }} diff --git a/charts/gitlab/charts/gitlab-shell/values.schema.json b/charts/gitlab/charts/gitlab-shell/values.schema.json index e6d346ecfc..5eb139d47e 100644 --- a/charts/gitlab/charts/gitlab-shell/values.schema.json +++ b/charts/gitlab/charts/gitlab-shell/values.schema.json @@ -447,6 +447,10 @@ "title": "IP address to assign to LoadBalancer", "type": "string" }, + "allocateLoadBalancerNodePorts": { + "title": "If the LoadBalancer Service should also have a NodePort", + "type": "boolean" + }, "loadBalancerSourceRanges": { "title": "List of IP CIDRs allowed access to LoadBalancer", "type": "array" diff --git a/charts/gitlab/charts/gitlab-shell/values.yaml b/charts/gitlab/charts/gitlab-shell/values.yaml index 9ee90e3748..578b603179 100644 --- a/charts/gitlab/charts/gitlab-shell/values.yaml +++ b/charts/gitlab/charts/gitlab-shell/values.yaml @@ -12,6 +12,7 @@ service: internalPort: 2222 externalTrafficPolicy: Cluster # nodePort: xxx + # allocateLoadBalancerNodePorts: true # loadBalancerIP: x.x.x.x # loadBalancerSourceRanges: # - x.x.x.x/yy diff --git a/doc/charts/gitlab/gitlab-shell/index.md b/doc/charts/gitlab/gitlab-shell/index.md index 49cd1c7fd0..aada5da39f 100644 --- a/doc/charts/gitlab/gitlab-shell/index.md +++ b/doc/charts/gitlab/gitlab-shell/index.md @@ -109,6 +109,7 @@ controlled by `global.shell.port`. | `priorityClassName` | | [Priority class](https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/) assigned to pods. | | `replicaCount` | `1` | Shell replicas | | `serviceLabels` | `{}` | Supplemental service labels | +| `service.allocateLoadBalancerNodePorts` | Not set, to use kubernetes default value. | Allows to disable NodePort allocation on LoadBalancer service, see the [documentation](https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation) | | `service.externalTrafficPolicy` | `Cluster` | Shell service external traffic policy (Cluster or Local) | | `service.internalPort` | `2222` | Shell internal port | | `service.nodePort` | | Sets shell nodePort if set | -- GitLab From 436d031366478602a387113bc09ea7d0373e480d Mon Sep 17 00:00:00 2001 From: Diego Abelenda Date: Tue, 17 Sep 2024 05:43:32 +0000 Subject: [PATCH 2/3] Apply 1 suggestion(s) to 1 file(s) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Alexandre Cunha --- doc/charts/gitlab/gitlab-shell/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/charts/gitlab/gitlab-shell/index.md b/doc/charts/gitlab/gitlab-shell/index.md index aada5da39f..6fa7177590 100644 --- a/doc/charts/gitlab/gitlab-shell/index.md +++ b/doc/charts/gitlab/gitlab-shell/index.md @@ -109,7 +109,7 @@ controlled by `global.shell.port`. | `priorityClassName` | | [Priority class](https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/) assigned to pods. | | `replicaCount` | `1` | Shell replicas | | `serviceLabels` | `{}` | Supplemental service labels | -| `service.allocateLoadBalancerNodePorts` | Not set, to use kubernetes default value. | Allows to disable NodePort allocation on LoadBalancer service, see the [documentation](https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation) | +| `service.allocateLoadBalancerNodePorts` | Not set, to use Kubernetes default value. | Allows to disable NodePort allocation on LoadBalancer service, see the [documentation](https://kubernetes.io/docs/concepts/services-networking/service/#load-balancer-nodeport-allocation) | | `service.externalTrafficPolicy` | `Cluster` | Shell service external traffic policy (Cluster or Local) | | `service.internalPort` | `2222` | Shell internal port | | `service.nodePort` | | Sets shell nodePort if set | -- GitLab From 782ab1fb94eb337fa48a6c3f070fd32f6cd140c6 Mon Sep 17 00:00:00 2001 From: Diego Abelenda Date: Wed, 18 Sep 2024 07:40:20 +0200 Subject: [PATCH 3/3] Add rspec tests as proposed by @Alexand --- spec/configuration/gitlab_shell_spec.rb | 37 +++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/spec/configuration/gitlab_shell_spec.rb b/spec/configuration/gitlab_shell_spec.rb index 19804a8b6b..8db825143a 100644 --- a/spec/configuration/gitlab_shell_spec.rb +++ b/spec/configuration/gitlab_shell_spec.rb @@ -23,6 +23,43 @@ describe 'gitlab-shell configuration' do expect(t.exit_code).to eq(0), "Unexpected error code #{t.exit_code} -- #{t.stderr}" end + context 'when service.type is LoadBalancer' do + let(:values) do + YAML.safe_load(%( + gitlab: + gitlab-shell: + service: + type: LoadBalancer + )).deep_merge(default_values) + end + + it 'renders the type' do + expect_successful_exit_code + + expect(t.dig('Service/test-gitlab-shell', 'spec', 'type')).to eq('LoadBalancer') + expect(t.dig('Service/test-gitlab-shell', 'spec').keys).to_not include('allocateLoadBalancerNodePorts') + end + + context 'when allocateLoadBalancerNodePorts is set' do + let(:values) do + YAML.safe_load(%( + gitlab: + gitlab-shell: + service: + type: LoadBalancer + allocateLoadBalancerNodePorts: false + )).deep_merge(default_values) + end + + it 'renders allocateLoadBalancerNodePorts' do + expect_successful_exit_code + + expect(t.dig('Service/test-gitlab-shell', 'spec', 'type')).to eq('LoadBalancer') + expect(t.dig('Service/test-gitlab-shell', 'spec', 'allocateLoadBalancerNodePorts')).to be(false) + end + end + end + context 'when gitlab-sshd is enabled' do using RSpec::Parameterized::TableSyntax -- GitLab