From c81133bfc79e8eb2efb729a5f6e3f972195967e4 Mon Sep 17 00:00:00 2001 From: WenChih Lo Date: Thu, 28 Mar 2024 05:06:58 +0800 Subject: [PATCH] Fix separateGrpcIngress flag not working in flyte-binary helm chart (#4946) * [BUG] separateGrpcIngress flag not working in flyte-binary helm chart Signed-off-by: Ryan Lo * make helm Signed-off-by: Ryan Lo * revise Signed-off-by: Ryan Lo * separate service Signed-off-by: Ryan Lo * make helm Signed-off-by: Ryan Lo * update README.md Signed-off-by: Ryan Lo * move paths in values.yaml to _helpers.tpl Signed-off-by: Ryan Lo * remove anno Signed-off-by: Ryan Lo * fix doc Signed-off-by: Ryan Lo * set ingress.separateGrpcIngress true by default Signed-off-by: Ryan Lo --------- Signed-off-by: Ryan Lo Signed-off-by: Eduardo Apolinario Co-authored-by: Eduardo Apolinario --- charts/flyte-binary/README.md | 1 + charts/flyte-binary/templates/_helpers.tpl | 18 +++ .../flyte-binary/templates/ingress/grpc.yaml | 103 +++--------------- .../flyte-binary/templates/ingress/http.yaml | 19 ++++ .../flyte-binary/templates/service/grpc.yaml | 2 + .../flyte-binary/templates/service/http.yaml | 10 ++ charts/flyte-binary/values.yaml | 2 + 7 files changed, 70 insertions(+), 85 deletions(-) diff --git a/charts/flyte-binary/README.md b/charts/flyte-binary/README.md index 99aa1c40b1..d7d50e092e 100644 --- a/charts/flyte-binary/README.md +++ b/charts/flyte-binary/README.md @@ -138,6 +138,7 @@ Chart for basic single Flyte executable deployment | ingress.httpTls | list | `[]` | | | ingress.ingressClassName | string | `""` | | | ingress.labels | object | `{}` | | +| ingress.separateGrpcIngress | bool | `true` | | | ingress.tls | list | `[]` | | | nameOverride | string | `""` | | | rbac.annotations | object | `{}` | | diff --git a/charts/flyte-binary/templates/_helpers.tpl b/charts/flyte-binary/templates/_helpers.tpl index 92e8fa4636..85184a45db 100644 --- a/charts/flyte-binary/templates/_helpers.tpl +++ b/charts/flyte-binary/templates/_helpers.tpl @@ -176,6 +176,24 @@ Get the Flyte service GRPC port. {{- default 8089 .Values.service.ports.grpc -}} {{- end -}} +{{/* +Get the Flyte service GRPC paths. +*/}} +{{- define "flyte-binary.ingress.grpcPaths" -}} +- /flyteidl.service.AdminService +- /flyteidl.service.AdminService/* +- /flyteidl.service.AuthMetadataService +- /flyteidl.service.AuthMetadataService/ +- /flyteidl.service.DataProxyService +- /flyteidl.service.DataProxyService/* +- /flyteidl.service.IdentityService +- /flyteidl.service.IdentityService/* +- /flyteidl.service.SignalService +- /flyteidl.service.SignalService/* +- /grpc.health.v1.Health +- /grpc.health.v1.Health/* +{{- end -}} + {{/* Get the Flyte agent service GRPC port. */}} diff --git a/charts/flyte-binary/templates/ingress/grpc.yaml b/charts/flyte-binary/templates/ingress/grpc.yaml index 427600336b..bd43185e50 100644 --- a/charts/flyte-binary/templates/ingress/grpc.yaml +++ b/charts/flyte-binary/templates/ingress/grpc.yaml @@ -1,4 +1,5 @@ -{{- if .Values.ingress.create }} +{{- if and .Values.ingress.create .Values.ingress.separateGrpcIngress }} +{{- $paths := (include "flyte-binary.ingress.grpcPaths" .) | fromYamlArray }} apiVersion: networking.k8s.io/v1 kind: Ingress metadata: @@ -38,90 +39,22 @@ spec: {{- if .Values.ingress.grpcExtraPaths.prepend }} {{- tpl ( .Values.ingress.grpcExtraPaths.prepend | toYaml ) . | nindent 6 }} {{- end }} - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.AdminService - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.AdminService/* - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.DataProxyService - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.DataProxyService/* - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.AuthMetadataService - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.AuthMetadataService/* - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.IdentityService - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.IdentityService/* - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /grpc.health.v1.Health - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /grpc.health.v1.Health/* - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.SignalService - pathType: ImplementationSpecific - - backend: - service: - name: {{ include "flyte-binary.service.grpc.name" . }} - port: - number: {{ include "flyte-binary.service.grpc.port" . }} - path: /flyteidl.service.SignalService/* - pathType: ImplementationSpecific + {{- range $path := $paths }} + - path: {{ $path }} + {{- if semverCompare ">=1.18-0" $.Capabilities.KubeVersion.GitVersion }} + pathType: ImplementationSpecific + {{- end }} + backend: + {{- if semverCompare ">=1.19-0" $.Capabilities.KubeVersion.GitVersion }} + service: + name: {{ include "flyte-binary.service.grpc.name" $ }} + port: + number: {{ include "flyte-binary.service.grpc.port" $ }} + {{- else }} + serviceName: {{ include "flyte-binary.service.grpc.name" $ }} + servicePort: {{ include "flyte-binary.service.grpc.port" $ }} + {{- end }} + {{- end }} {{- if .Values.ingress.grpcExtraPaths.append }} {{- tpl ( .Values.ingress.grpcExtraPaths.append | toYaml ) . | nindent 6 }} {{- end }} diff --git a/charts/flyte-binary/templates/ingress/http.yaml b/charts/flyte-binary/templates/ingress/http.yaml index 5c2a7ad1c7..cdce87047b 100644 --- a/charts/flyte-binary/templates/ingress/http.yaml +++ b/charts/flyte-binary/templates/ingress/http.yaml @@ -171,6 +171,25 @@ spec: number: {{ include "flyte-binary.service.http.port" . }} path: /oauth2/* pathType: ImplementationSpecific + {{- if not .Values.ingress.separateGrpcIngress }} + {{- $paths := (include "flyte-binary.ingress.grpcPaths" .) | fromYamlArray }} + {{- range $path := $paths }} + - path: {{ $path }} + {{- if semverCompare ">=1.18-0" $.Capabilities.KubeVersion.GitVersion }} + pathType: ImplementationSpecific + {{- end }} + backend: + {{- if semverCompare ">=1.19-0" $.Capabilities.KubeVersion.GitVersion }} + service: + name: {{ include "flyte-binary.service.http.name" $ }} + port: + number: {{ include "flyte-binary.service.grpc.port" $ }} + {{- else }} + serviceName: {{ include "flyte-binary.service.http.name" $ }} + servicePort: {{ include "flyte-binary.service.grpc.port" $ }} + {{- end }} + {{- end }} + {{- end }} {{- if .Values.ingress.httpExtraPaths.append }} {{- tpl ( .Values.ingress.httpExtraPaths.append | toYaml ) . | nindent 6 }} {{- end }} diff --git a/charts/flyte-binary/templates/service/grpc.yaml b/charts/flyte-binary/templates/service/grpc.yaml index eb8d3d0063..1feafa1c5c 100644 --- a/charts/flyte-binary/templates/service/grpc.yaml +++ b/charts/flyte-binary/templates/service/grpc.yaml @@ -1,3 +1,4 @@ +{{- if .Values.ingress.separateGrpcIngress }} apiVersion: v1 kind: Service metadata: @@ -47,3 +48,4 @@ spec: nodePort: null {{- end }} selector: {{- include "flyte-binary.selectorLabels" . | nindent 4 }} +{{- end }} diff --git a/charts/flyte-binary/templates/service/http.yaml b/charts/flyte-binary/templates/service/http.yaml index 189ea1e040..3b47e72340 100644 --- a/charts/flyte-binary/templates/service/http.yaml +++ b/charts/flyte-binary/templates/service/http.yaml @@ -46,6 +46,16 @@ spec: {{- else if eq .Values.service.type "ClusterIP" }} nodePort: null {{- end }} + {{- if not .Values.ingress.separateGrpcIngress }} + - name: grpc + port: {{ include "flyte-binary.service.grpc.port" . }} + targetPort: grpc + {{- if and (or (eq .Values.service.type "NodePort") (eq .Values.service.type "LoadBalancer")) (not (empty .Values.service.nodePorts.grpc)) }} + nodePort: {{ .Values.service.nodePorts.grpc }} + {{- else if eq .Values.service.type "ClusterIP" }} + nodePort: null + {{- end }} + {{- end }} {{- if .Values.service.extraPorts }} {{- tpl ( .Values.service.extraPorts | toYaml ) . | nindent 4 }} {{- end }} diff --git a/charts/flyte-binary/values.yaml b/charts/flyte-binary/values.yaml index f3f8e79528..2211e14f85 100644 --- a/charts/flyte-binary/values.yaml +++ b/charts/flyte-binary/values.yaml @@ -329,6 +329,8 @@ ingress: labels: {} # host Hostname to bind to ingress resources host: "" + # separateGrpcIngress Create a separate ingress resource for GRPC if true. Required for certain ingress controllers like nginx. + separateGrpcIngress: true # commonAnnotations Add common annotations to all ingress resources commonAnnotations: {} # httpAnnotations Add annotations to http ingress resource