mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 14:02:56 +09:00
fix(seaweedfs): point S3 consumers at the reachable S3 service (#3119)
## What this PR does The S3 Service is named with the release-independent `seaweedfs.name` helper (always `seaweedfs-s3`), but several in-chart consumers still resolved it through `componentName` (`<fullname>-s3`). Since the shipped release is `seaweedfs-system`, those consumers pointed at `seaweedfs-system-s3` — a Service that does not exist — so ingress-nginx returns 503 on the external S3 endpoint and the COSI provisioner cannot reach S3 in-cluster. This aligns every in-chart S3 consumer with the actual Service: - S3 ingress backend and iceberg ingress backend → `seaweedfs-s3` (fixes the 503 on the default `s3.ingress.enabled: true` path). - COSI provisioner in-cluster `ENDPOINT` (both the s3-gateway and filer-S3 branches) → `seaweedfs-s3` on the configured S3 port. These were also portless (defaulting to 443 against an S3 that listens on 8333), and the filer branch additionally targeted the filer Service, which serves no S3. The all-in-one ingress branch is intentionally left on `componentName "all-in-one"` (a different Service). Changes are captured as a downstream patch (`s3-service-name-consumers.patch`) wired into `make update`, and pinned by helm-unittest rendered under the production release name so the mismatch cannot recur. ### Release note ```release-note fix(seaweedfs): point the S3 ingress, iceberg ingress, and COSI provisioner endpoint at the actual seaweedfs-s3 service, fixing 503 on the external S3 endpoint ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated S3-related routing and in-cluster endpoints to use the correct service name and explicit port values. * Improved ingress behavior for newer Kubernetes versions while keeping compatibility with older versions. * Preserved the existing all-in-one routing path for S3 deployments. * **Tests** * Added coverage for S3 service naming, ingress backend routing, and COSI endpoint resolution to prevent regressions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
6 files changed
+153
-5
No files matched your search
@@ -18,5 +18,6 @@ update:
|
||||
patch --no-backup-if-mismatch -p4 < patches/cosi-provisioner-sa-name.patch
|
||||
patch --no-backup-if-mismatch -p4 < patches/cosi-bucket-class-lock-readonly.patch
|
||||
patch --no-backup-if-mismatch -p4 < patches/s3-service-name.patch
|
||||
patch --no-backup-if-mismatch -p4 < patches/s3-service-name-consumers.patch
|
||||
#patch --no-backup-if-mismatch -p4 < patches/retention-policy-delete.yaml
|
||||
|
||||
@@ -79,9 +79,9 @@ spec:
|
||||
{{- else if .Values.s3.ingress.enabled }}
|
||||
value: "{{ printf "https://%s" .Values.s3.ingress.host }}"
|
||||
{{- else if .Values.s3.enabled }}
|
||||
value: "{{ printf "https://%s.%s.svc" (include "seaweedfs.componentName" (list . "s3")) .Release.Namespace }}"
|
||||
value: "{{ printf "https://%s-s3.%s.svc:%v" (include "seaweedfs.name" .) .Release.Namespace .Values.s3.port }}"
|
||||
{{- else }}
|
||||
value: "{{ printf "https://%s.%s.svc" (include "seaweedfs.componentName" (list . "filer")) .Release.Namespace }}"
|
||||
value: "{{ printf "https://%s-s3.%s.svc:%v" (include "seaweedfs.name" .) .Release.Namespace .Values.filer.s3.port }}"
|
||||
{{- end }}
|
||||
{{- with .Values.cosi.region }}
|
||||
- name: REGION
|
||||
|
||||
@@ -5,11 +5,11 @@ paths:
|
||||
backend:
|
||||
{{- if semverCompare ">=1.19-0" .Capabilities.KubeVersion.GitVersion }}
|
||||
service:
|
||||
name: {{ include "seaweedfs.componentName" (list . "s3") }}
|
||||
name: {{ template "seaweedfs.name" . }}-s3
|
||||
port:
|
||||
number: {{ .Values.s3.icebergPort }}
|
||||
{{- else }}
|
||||
serviceName: {{ include "seaweedfs.componentName" (list . "s3") }}
|
||||
serviceName: {{ template "seaweedfs.name" . }}-s3
|
||||
servicePort: {{ .Values.s3.icebergPort }}
|
||||
{{- end }}
|
||||
{{- end -}}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
{{- $s3Enabled := or .Values.s3.enabled (and .Values.filer.s3.enabled (not .Values.allInOne.enabled)) (and .Values.allInOne.enabled .Values.allInOne.s3.enabled) }}
|
||||
{{- if and $s3Enabled .Values.s3.ingress.enabled }}
|
||||
{{- /* Determine service name based on deployment mode */}}
|
||||
{{- $serviceName := ternary (include "seaweedfs.componentName" (list . "all-in-one")) (include "seaweedfs.componentName" (list . "s3")) .Values.allInOne.enabled }}
|
||||
{{- $serviceName := ternary (include "seaweedfs.componentName" (list . "all-in-one")) (printf "%s-s3" (include "seaweedfs.name" .)) .Values.allInOne.enabled }}
|
||||
{{- $s3Port := .Values.allInOne.s3.port | default .Values.s3.port }}
|
||||
{{- /* Build hosts list - support both legacy .host (string) and new .hosts (array) for backwards compatibility */}}
|
||||
{{- $hosts := list }}
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
diff --git a/packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml b/packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml
|
||||
index d485e0678..d581b085d 100644
|
||||
--- a/packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml
|
||||
+++ b/packages/system/seaweedfs/charts/seaweedfs/templates/cosi/cosi-deployment.yaml
|
||||
@@ -79,9 +79,9 @@ spec:
|
||||
{{- else if .Values.s3.ingress.enabled }}
|
||||
value: "{{ printf "https://%s" .Values.s3.ingress.host }}"
|
||||
{{- else if .Values.s3.enabled }}
|
||||
- value: "{{ printf "https://%s.%s.svc" (include "seaweedfs.componentName" (list . "s3")) .Release.Namespace }}"
|
||||
+ value: "{{ printf "https://%s-s3.%s.svc:%v" (include "seaweedfs.name" .) .Release.Namespace .Values.s3.port }}"
|
||||
{{- else }}
|
||||
- value: "{{ printf "https://%s.%s.svc" (include "seaweedfs.componentName" (list . "filer")) .Release.Namespace }}"
|
||||
+ value: "{{ printf "https://%s-s3.%s.svc:%v" (include "seaweedfs.name" .) .Release.Namespace .Values.filer.s3.port }}"
|
||||
{{- end }}
|
||||
{{- with .Values.cosi.region }}
|
||||
- name: REGION
|
||||
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
|
||||
index 5a760d7e9..68db538dc 100644
|
||||
--- a/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
|
||||
+++ b/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
|
||||
@@ -5,11 +5,11 @@ paths:
|
||||
backend:
|
||||
{{- if semverCompare ">=1.19-0" .Capabilities.KubeVersion.GitVersion }}
|
||||
service:
|
||||
- name: {{ include "seaweedfs.componentName" (list . "s3") }}
|
||||
+ name: {{ template "seaweedfs.name" . }}-s3
|
||||
port:
|
||||
number: {{ .Values.s3.icebergPort }}
|
||||
{{- else }}
|
||||
- serviceName: {{ include "seaweedfs.componentName" (list . "s3") }}
|
||||
+ serviceName: {{ template "seaweedfs.name" . }}-s3
|
||||
servicePort: {{ .Values.s3.icebergPort }}
|
||||
{{- end }}
|
||||
{{- end -}}
|
||||
diff --git a/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml b/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml
|
||||
index bc4eee164..c92063f8c 100644
|
||||
--- a/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml
|
||||
+++ b/packages/system/seaweedfs/charts/seaweedfs/templates/s3/s3-ingress.yaml
|
||||
@@ -2,7 +2,7 @@
|
||||
{{- $s3Enabled := or .Values.s3.enabled (and .Values.filer.s3.enabled (not .Values.allInOne.enabled)) (and .Values.allInOne.enabled .Values.allInOne.s3.enabled) }}
|
||||
{{- if and $s3Enabled .Values.s3.ingress.enabled }}
|
||||
{{- /* Determine service name based on deployment mode */}}
|
||||
-{{- $serviceName := ternary (include "seaweedfs.componentName" (list . "all-in-one")) (include "seaweedfs.componentName" (list . "s3")) .Values.allInOne.enabled }}
|
||||
+{{- $serviceName := ternary (include "seaweedfs.componentName" (list . "all-in-one")) (printf "%s-s3" (include "seaweedfs.name" .)) .Values.allInOne.enabled }}
|
||||
{{- $s3Port := .Values.allInOne.s3.port | default .Values.s3.port }}
|
||||
{{- /* Build hosts list - support both legacy .host (string) and new .hosts (array) for backwards compatibility */}}
|
||||
{{- $hosts := list }}
|
||||
@@ -0,0 +1,100 @@
|
||||
suite: seaweedfs s3 consumers resolve the renamed s3 service
|
||||
|
||||
# Pins the regression where consumers of the S3 service 503 / fail to connect
|
||||
# because they still resolve a service name that no longer exists.
|
||||
#
|
||||
# The s3-service-name.patch renamed the S3 Service to `<name>-s3` (seaweedfs-s3),
|
||||
# but several consumers still resolved it via componentName -> `<fullname>-s3`:
|
||||
# - the S3 ingress backend,
|
||||
# - the iceberg ingress backend,
|
||||
# - the COSI provisioner ENDPOINT (in-cluster, s3.ingress.enabled=false path).
|
||||
# name and fullname only coincide when the release name equals the chart name;
|
||||
# the shipped release is `seaweedfs-system`, so those consumers pointed at
|
||||
# `seaweedfs-system-s3` while the real service is `seaweedfs-s3`. With no
|
||||
# matching endpoints, ingress-nginx returns 503 and COSI cannot reach S3.
|
||||
#
|
||||
# All tests render under the real release name so name != fullname, the exact
|
||||
# condition that exposes the bug. The all-in-one case documents the other
|
||||
# ternary branch, which is unchanged and correct.
|
||||
|
||||
templates:
|
||||
- charts/seaweedfs/templates/s3/s3-service.yaml
|
||||
- charts/seaweedfs/templates/s3/s3-ingress.yaml
|
||||
- charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
|
||||
- charts/seaweedfs/templates/cosi/cosi-deployment.yaml
|
||||
|
||||
release:
|
||||
name: seaweedfs-system
|
||||
namespace: cozy-seaweedfs
|
||||
|
||||
tests:
|
||||
- it: s3 service is named with the name helper, not componentName
|
||||
template: charts/seaweedfs/templates/s3/s3-service.yaml
|
||||
set:
|
||||
seaweedfs.s3.enabled: true
|
||||
asserts:
|
||||
- equal:
|
||||
path: metadata.name
|
||||
value: seaweedfs-s3
|
||||
|
||||
- it: s3 ingress backend points at the s3 service, not a fullname-derived name
|
||||
template: charts/seaweedfs/templates/s3/s3-ingress.yaml
|
||||
set:
|
||||
seaweedfs.s3.enabled: true
|
||||
seaweedfs.s3.ingress.enabled: true
|
||||
seaweedfs.s3.ingress.host: s3.example.com
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.rules[0].http.paths[0].backend.service.name
|
||||
value: seaweedfs-s3
|
||||
|
||||
- it: all-in-one ingress backend still resolves to the all-in-one service
|
||||
template: charts/seaweedfs/templates/s3/s3-ingress.yaml
|
||||
set:
|
||||
seaweedfs.allInOne.enabled: true
|
||||
seaweedfs.allInOne.s3.enabled: true
|
||||
seaweedfs.s3.ingress.enabled: true
|
||||
seaweedfs.s3.ingress.host: s3.example.com
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.rules[0].http.paths[0].backend.service.name
|
||||
value: seaweedfs-system-all-in-one
|
||||
|
||||
- it: iceberg ingress backend points at the s3 service too
|
||||
template: charts/seaweedfs/templates/s3/s3-iceberg-ingress.yaml
|
||||
set:
|
||||
seaweedfs.s3.enabled: true
|
||||
seaweedfs.s3.icebergPort: 8334
|
||||
seaweedfs.s3.icebergIngress.enabled: true
|
||||
seaweedfs.s3.icebergIngress.host: iceberg.example.com
|
||||
asserts:
|
||||
- equal:
|
||||
path: spec.rules[0].http.paths[0].backend.service.name
|
||||
value: seaweedfs-s3
|
||||
|
||||
- it: COSI in-cluster ENDPOINT resolves the s3 service when ingress is disabled
|
||||
template: charts/seaweedfs/templates/cosi/cosi-deployment.yaml
|
||||
set:
|
||||
seaweedfs.cosi.enabled: true
|
||||
seaweedfs.s3.enabled: true
|
||||
seaweedfs.s3.ingress.enabled: false
|
||||
asserts:
|
||||
- contains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
name: ENDPOINT
|
||||
value: https://seaweedfs-s3.cozy-seaweedfs.svc:8333
|
||||
|
||||
- it: COSI in-cluster ENDPOINT (filer S3 mode) targets the s3 service on the filer s3 port
|
||||
template: charts/seaweedfs/templates/cosi/cosi-deployment.yaml
|
||||
set:
|
||||
seaweedfs.cosi.enabled: true
|
||||
seaweedfs.s3.enabled: false
|
||||
seaweedfs.filer.s3.enabled: true
|
||||
seaweedfs.s3.ingress.enabled: false
|
||||
asserts:
|
||||
- contains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
name: ENDPOINT
|
||||
value: https://seaweedfs-s3.cozy-seaweedfs.svc:8333
|
||||
Reference in new issue
Block a user