Skip to content

Commit 5deb9cf

Browse files
myasnikovdaniilmyasnikovdaniil
committed
fix(site-router): fail early on a loadBalancerClass change of the tunnel
spec.loadBalancerClass is immutable while a Service is type LoadBalancer: setting, changing or clearing it is rejected. The tunnel Service takes the class from the platform's publishing.loadBalancerClass, so an operator changing that value later wedged the upgrade of every installed instance on an opaque "may not change once set", and finding the way out meant knowing that the Service had to be recreated. The chart now looks the Service up and fails with the existing and requested class and the command that recreates it, the guard extra/ingress already carries for the same value. Nothing changes on a fresh install, where the lookup is empty. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent b135c27 commit 5deb9cf

2 files changed

Lines changed: 94 additions & 0 deletions

File tree

‎packages/apps/site-router/templates/service.yaml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,20 @@
88
gateway virt-launcher pod via the KubeVirt-managed vm.kubevirt.io/name label.
99
*/ -}}
1010
{{- $loadBalancerClass := (index (.Values._cluster | default dict) "load-balancer-class") | default "" }}
11+
{{- /*
12+
spec.loadBalancerClass is immutable once a Service is type LoadBalancer: the
13+
API rejects setting, changing or clearing it, so a later change of
14+
publishing.loadBalancerClass would wedge every installed instance's upgrade on
15+
an opaque "may not change once set". Fail early with the way out instead, the
16+
same guard extra/ingress carries for the same platform value. lookup returns
17+
nothing on a fresh install, a dry-run or in CI, so the Service is born with the
18+
requested class there.
19+
*/ -}}
20+
{{- $tunnelSvcName := printf "%s-tunnel" .Release.Name }}
21+
{{- $existingTunnelSvc := lookup "v1" "Service" .Release.Namespace $tunnelSvcName }}
22+
{{- if and $existingTunnelSvc (eq (dig "spec" "type" "" $existingTunnelSvc) "LoadBalancer") (ne (dig "spec" "loadBalancerClass" "" $existingTunnelSvc) $loadBalancerClass) }}
23+
{{- fail (printf "publishing.loadBalancerClass cannot be changed on the existing tunnel Service %s/%s: spec.loadBalancerClass is immutable once the Service is type LoadBalancer (existing: %q, requested: %q). Delete the Service once so Flux recreates it with the new setting; its external address is allocated again and may change, so the remote peer may need the new one: kubectl -n %s delete service %s" .Release.Namespace $tunnelSvcName (dig "spec" "loadBalancerClass" "" $existingTunnelSvc) $loadBalancerClass .Release.Namespace $tunnelSvcName) }}
24+
{{- end }}
1125
apiVersion: v1
1226
kind: Service
1327
metadata:

‎packages/apps/site-router/tests/service_test.yaml‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,83 @@ tests:
8686
- equal:
8787
path: spec.selector["vm.kubevirt.io/name"]
8888
value: site-router-test
89+
90+
# spec.loadBalancerClass is immutable once the Service is type LoadBalancer, so
91+
# setting, changing or clearing it on an installed instance must fail early with
92+
# the recreate command instead of wedging the upgrade on the API's rejection.
93+
- it: fails when the class is set on an installed tunnel Service that has none
94+
kubernetesProvider:
95+
scheme: &svcscheme
96+
"v1/Service":
97+
gvr:
98+
version: v1
99+
resource: services
100+
namespaced: true
101+
objects:
102+
- apiVersion: v1
103+
kind: Service
104+
metadata:
105+
name: site-router-test-tunnel
106+
namespace: tenant-test
107+
spec:
108+
type: LoadBalancer
109+
asserts:
110+
- failedTemplate:
111+
errorPattern: "spec\\.loadBalancerClass is immutable once the Service is type LoadBalancer"
112+
- failedTemplate:
113+
errorPattern: "kubectl -n tenant-test delete service site-router-test-tunnel"
114+
115+
- it: fails when the class changes on an installed tunnel Service
116+
kubernetesProvider:
117+
scheme: *svcscheme
118+
objects:
119+
- apiVersion: v1
120+
kind: Service
121+
metadata:
122+
name: site-router-test-tunnel
123+
namespace: tenant-test
124+
spec:
125+
type: LoadBalancer
126+
loadBalancerClass: cilium.io/l2-announcer
127+
asserts:
128+
- failedTemplate:
129+
errorPattern: 'existing: "cilium.io/l2-announcer", requested: "metallb.io/metallb"'
130+
131+
- it: fails when the class is cleared on an installed tunnel Service that carries one
132+
set:
133+
_cluster:
134+
load-balancer-class: ""
135+
kubernetesProvider:
136+
scheme: *svcscheme
137+
objects:
138+
- apiVersion: v1
139+
kind: Service
140+
metadata:
141+
name: site-router-test-tunnel
142+
namespace: tenant-test
143+
spec:
144+
type: LoadBalancer
145+
loadBalancerClass: metallb.io/metallb
146+
asserts:
147+
- failedTemplate:
148+
errorPattern: 'existing: "metallb.io/metallb", requested: ""'
149+
150+
- it: renders when the installed tunnel Service already has the requested class
151+
kubernetesProvider:
152+
scheme: *svcscheme
153+
objects:
154+
- apiVersion: v1
155+
kind: Service
156+
metadata:
157+
name: site-router-test-tunnel
158+
namespace: tenant-test
159+
spec:
160+
type: LoadBalancer
161+
loadBalancerClass: metallb.io/metallb
162+
documentSelector:
163+
path: kind
164+
value: Service
165+
asserts:
166+
- equal:
167+
path: spec.loadBalancerClass
168+
value: metallb.io/metallb

0 commit comments

Comments
 (0)