Skip to content

Commit 8a14de9

Browse files
lexfreilexfrei
committed
fix(seaweedfs): point S3 consumers at the reachable S3 service
The s3-service-name.patch renamed the S3 Service to <name>-s3 (seaweedfs-s3), but several consumers still resolved it through componentName -> <fullname>-s3. name and fullname only coincide when the release name equals the chart name; the shipped release is seaweedfs-system, so they pointed at seaweedfs-system-s3 while the real service stayed seaweedfs-s3: - the S3 ingress backend and the iceberg ingress backend, which left ingress-nginx returning 503 on the external S3 endpoint. The COSI provisioner in-cluster ENDPOINT was wrong on the same axis and also on the port. Both the s3-gateway branch and the filer-S3 branch rendered a portless URL that defaults to 443, while the s3 service only listens on the S3 port (8333); the filer branch additionally targeted the filer service, which serves no S3. Both in-cluster branches now resolve seaweedfs-s3 on the configured S3 port. The default path (s3.ingress.enabled=true) is repaired by the ingress fix; the COSI in-cluster paths are latent until an operator disables the ingress. Resolve every consumer through the same name helper the Service uses, and pin the contract with unittests rendered under the production release name so the mismatch cannot recur. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent f5c408d commit 8a14de9

6 files changed

Lines changed: 153 additions & 5 deletions

File tree

‎packages/system/seaweedfs/Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,5 +18,6 @@ update:
1818
patch --no-backup-if-mismatch -p4 < patches/cosi-provisioner-sa-name.patch
1919
patch --no-backup-if-mismatch -p4 < patches/cosi-bucket-class-lock-readonly.patch
2020
patch --no-backup-if-mismatch -p4 < patches/s3-service-name.patch
21+
patch --no-backup-if-mismatch -p4 < patches/s3-service-name-consumers.patch
2122
#patch --no-backup-if-mismatch -p4 < patches/retention-policy-delete.yaml
2223

‎packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ spec:
7979
{{- else if .Values.s3.ingress.enabled }}
8080
value: "{{ printf "https://%s" .Values.s3.ingress.host }}"
8181
{{- else if .Values.s3.enabled }}
82-
value: "{{ printf "https://%s.%s.svc" (include "seaweedfs.componentName" (list . "s3")) .Release.Namespace }}"
82+
value: "{{ printf "https://%s-s3.%s.svc:%v" (include "seaweedfs.name" .) .Release.Namespace .Values.s3.port }}"
8383
{{- else }}
84-
value: "{{ printf "https://%s.%s.svc" (include "seaweedfs.componentName" (list . "filer")) .Release.Namespace }}"
84+
value: "{{ printf "https://%s-s3.%s.svc:%v" (include "seaweedfs.name" .) .Release.Namespace .Values.filer.s3.port }}"
8585
{{- end }}
8686
{{- with .Values.cosi.region }}
8787
- name: REGION

‎packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ paths:
55
backend:
66
{{- if semverCompare ">=1.19-0" .Capabilities.KubeVersion.GitVersion }}
77
service:
8-
name: {{ include "seaweedfs.componentName" (list . "s3") }}
8+
name: {{ template "seaweedfs.name" . }}-s3
99
port:
1010
number: {{ .Values.s3.icebergPort }}
1111
{{- else }}
12-
serviceName: {{ include "seaweedfs.componentName" (list . "s3") }}
12+
serviceName: {{ template "seaweedfs.name" . }}-s3
1313
servicePort: {{ .Values.s3.icebergPort }}
1414
{{- end }}
1515
{{- end -}}

‎packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{{- $s3Enabled := or .Values.s3.enabled (and .Values.filer.s3.enabled (not .Values.allInOne.enabled)) (and .Values.allInOne.enabled .Values.allInOne.s3.enabled) }}
33
{{- if and $s3Enabled .Values.s3.ingress.enabled }}
44
{{- /* Determine service name based on deployment mode */}}
5-
{{- $serviceName := ternary (include "seaweedfs.componentName" (list . "all-in-one")) (include "seaweedfs.componentName" (list . "s3")) .Values.allInOne.enabled }}
5+
{{- $serviceName := ternary (include "seaweedfs.componentName" (list . "all-in-one")) (printf "%s-s3" (include "seaweedfs.name" .)) .Values.allInOne.enabled }}
66
{{- $s3Port := .Values.allInOne.s3.port | default .Values.s3.port }}
77
{{- /* Build hosts list - support both legacy .host (string) and new .hosts (array) for backwards compatibility */}}
88
{{- $hosts := list }}

‎packages/system/seaweedfs/patches/s3-service-name-consumers.patch‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
diff --git a/packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml b/packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml
2+
index d485e0678..d581b085d 100644
3+
--- a/packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml
4+
+++ b/packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml
5+
@@ -79,9 +79,9 @@ spec:
6+
{{- else if .Values.s3.ingress.enabled }}
7+
value: "{{ printf "https://%s" .Values.s3.ingress.host }}"
8+
{{- else if .Values.s3.enabled }}
9+
- value: "{{ printf "https://%s.%s.svc" (include "seaweedfs.componentName" (list . "s3")) .Release.Namespace }}"
10+
+ value: "{{ printf "https://%s-s3.%s.svc:%v" (include "seaweedfs.name" .) .Release.Namespace .Values.s3.port }}"
11+
{{- else }}
12+
- value: "{{ printf "https://%s.%s.svc" (include "seaweedfs.componentName" (list . "filer")) .Release.Namespace }}"
13+
+ value: "{{ printf "https://%s-s3.%s.svc:%v" (include "seaweedfs.name" .) .Release.Namespace .Values.filer.s3.port }}"
14+
{{- end }}
15+
{{- with .Values.cosi.region }}
16+
- name: REGION
17+
diff --git a/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml b/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
18+
index 5a760d7e9..68db538dc 100644
19+
--- a/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
20+
+++ b/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
21+
@@ -5,11 +5,11 @@ paths:
22+
backend:
23+
{{- if semverCompare ">=1.19-0" .Capabilities.KubeVersion.GitVersion }}
24+
service:
25+
- name: {{ include "seaweedfs.componentName" (list . "s3") }}
26+
+ name: {{ template "seaweedfs.name" . }}-s3
27+
port:
28+
number: {{ .Values.s3.icebergPort }}
29+
{{- else }}
30+
- serviceName: {{ include "seaweedfs.componentName" (list . "s3") }}
31+
+ serviceName: {{ template "seaweedfs.name" . }}-s3
32+
servicePort: {{ .Values.s3.icebergPort }}
33+
{{- end }}
34+
{{- end -}}
35+
diff --git a/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml b/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml
36+
index bc4eee164..c92063f8c 100644
37+
--- a/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml
38+
+++ b/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml
39+
@@ -2,7 +2,7 @@
40+
{{- $s3Enabled := or .Values.s3.enabled (and .Values.filer.s3.enabled (not .Values.allInOne.enabled)) (and .Values.allInOne.enabled .Values.allInOne.s3.enabled) }}
41+
{{- if and $s3Enabled .Values.s3.ingress.enabled }}
42+
{{- /* Determine service name based on deployment mode */}}
43+
-{{- $serviceName := ternary (include "seaweedfs.componentName" (list . "all-in-one")) (include "seaweedfs.componentName" (list . "s3")) .Values.allInOne.enabled }}
44+
+{{- $serviceName := ternary (include "seaweedfs.componentName" (list . "all-in-one")) (printf "%s-s3" (include "seaweedfs.name" .)) .Values.allInOne.enabled }}
45+
{{- $s3Port := .Values.allInOne.s3.port | default .Values.s3.port }}
46+
{{- /* Build hosts list - support both legacy .host (string) and new .hosts (array) for backwards compatibility */}}
47+
{{- $hosts := list }}

‎packages/system/seaweedfs/tests/s3_service_name_consumers_test.yaml‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
suite: seaweedfs s3 consumers resolve the renamed s3 service
2+
3+
# Pins the regression where consumers of the S3 service 503 / fail to connect
4+
# because they still resolve a service name that no longer exists.
5+
#
6+
# The s3-service-name.patch renamed the S3 Service to `<name>-s3` (seaweedfs-s3),
7+
# but several consumers still resolved it via componentName -> `<fullname>-s3`:
8+
# - the S3 ingress backend,
9+
# - the iceberg ingress backend,
10+
# - the COSI provisioner ENDPOINT (in-cluster, s3.ingress.enabled=false path).
11+
# name and fullname only coincide when the release name equals the chart name;
12+
# the shipped release is `seaweedfs-system`, so those consumers pointed at
13+
# `seaweedfs-system-s3` while the real service is `seaweedfs-s3`. With no
14+
# matching endpoints, ingress-nginx returns 503 and COSI cannot reach S3.
15+
#
16+
# All tests render under the real release name so name != fullname, the exact
17+
# condition that exposes the bug. The all-in-one case documents the other
18+
# ternary branch, which is unchanged and correct.
19+
20+
templates:
21+
- charts/seaweedfs/templates/s3/s3-service.yaml
22+
- charts/seaweedfs/templates/s3/s3-ingress.yaml
23+
- charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
24+
- charts/seaweedfs/templates/cosi/cosi-deployment.yaml
25+
26+
release:
27+
name: seaweedfs-system
28+
namespace: cozy-seaweedfs
29+
30+
tests:
31+
- it: s3 service is named with the name helper, not componentName
32+
template: charts/seaweedfs/templates/s3/s3-service.yaml
33+
set:
34+
seaweedfs.s3.enabled: true
35+
asserts:
36+
- equal:
37+
path: metadata.name
38+
value: seaweedfs-s3
39+
40+
- it: s3 ingress backend points at the s3 service, not a fullname-derived name
41+
template: charts/seaweedfs/templates/s3/s3-ingress.yaml
42+
set:
43+
seaweedfs.s3.enabled: true
44+
seaweedfs.s3.ingress.enabled: true
45+
seaweedfs.s3.ingress.host: s3.example.com
46+
asserts:
47+
- equal:
48+
path: spec.rules[0].http.paths[0].backend.service.name
49+
value: seaweedfs-s3
50+
51+
- it: all-in-one ingress backend still resolves to the all-in-one service
52+
template: charts/seaweedfs/templates/s3/s3-ingress.yaml
53+
set:
54+
seaweedfs.allInOne.enabled: true
55+
seaweedfs.allInOne.s3.enabled: true
56+
seaweedfs.s3.ingress.enabled: true
57+
seaweedfs.s3.ingress.host: s3.example.com
58+
asserts:
59+
- equal:
60+
path: spec.rules[0].http.paths[0].backend.service.name
61+
value: seaweedfs-system-all-in-one
62+
63+
- it: iceberg ingress backend points at the s3 service too
64+
template: charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
65+
set:
66+
seaweedfs.s3.enabled: true
67+
seaweedfs.s3.icebergPort: 8334
68+
seaweedfs.s3.icebergIngress.enabled: true
69+
seaweedfs.s3.icebergIngress.host: iceberg.example.com
70+
asserts:
71+
- equal:
72+
path: spec.rules[0].http.paths[0].backend.service.name
73+
value: seaweedfs-s3
74+
75+
- it: COSI in-cluster ENDPOINT resolves the s3 service when ingress is disabled
76+
template: charts/seaweedfs/templates/cosi/cosi-deployment.yaml
77+
set:
78+
seaweedfs.cosi.enabled: true
79+
seaweedfs.s3.enabled: true
80+
seaweedfs.s3.ingress.enabled: false
81+
asserts:
82+
- contains:
83+
path: spec.template.spec.containers[0].env
84+
content:
85+
name: ENDPOINT
86+
value: https://seaweedfs-s3.cozy-seaweedfs.svc:8333
87+
88+
- it: COSI in-cluster ENDPOINT (filer S3 mode) targets the s3 service on the filer s3 port
89+
template: charts/seaweedfs/templates/cosi/cosi-deployment.yaml
90+
set:
91+
seaweedfs.cosi.enabled: true
92+
seaweedfs.s3.enabled: false
93+
seaweedfs.filer.s3.enabled: true
94+
seaweedfs.s3.ingress.enabled: false
95+
asserts:
96+
- contains:
97+
path: spec.template.spec.containers[0].env
98+
content:
99+
name: ENDPOINT
100+
value: https://seaweedfs-s3.cozy-seaweedfs.svc:8333

0 commit comments

Comments
 (0)