[BUG] separateGrpcIngress flag not working in flyte-binary helm chart

Signed-off-by: Ryan Lo <[email protected]>
flyteorg · Feb 24, 2024 · 75e595a · 75e595a
1 parent 94e433b
commit 75e595a
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 86 deletions.
diff --git a/charts/flyte-binary/templates/_helpers.tpl b/charts/flyte-binary/templates/_helpers.tpl
@@ -176,6 +176,96 @@ Get the Flyte service GRPC port.
 {{- default 8089 .Values.service.ports.grpc -}}
 {{- end -}}
 
+{{/*
+Get the Flyte ingress GRPC paths
+*/}}
+{{- define "flyte-binary.ingress.grpc.paths" -}}
+- 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
+  backend:
+    service:
+      name: {{ include "flyte-binary.service.grpc.name" . }}
+      port:
+        number: {{ include "flyte-binary.service.grpc.port" . }}
+{{- 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
@@ -1,4 +1,4 @@
-{{- if .Values.ingress.create }}
+{{- if and .Values.ingress.create .Values.ingress.separateGrpcIngress }}
 apiVersion: networking.k8s.io/v1
 kind: Ingress
 metadata:
@@ -38,90 +38,7 @@ 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
+      {{- include "flyte-binary.ingress.grpc.paths" . | nindent 6 }}
       {{- 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
@@ -21,6 +21,9 @@ metadata:
     {{- if .Values.ingress.httpAnnotations }}
     {{- tpl ( .Values.ingress.httpAnnotations | toYaml ) . | nindent 4 }}
     {{- end }}
+    {{- if not .Values.ingress.separateGrpcIngress }}
+    nginx.ingress.kubernetes.io/backend-protocol: "GRPC"
+    {{- end }}
 spec:
   {{- if .Values.ingress.httpIngressClassName }}
   ingressClassName: {{ .Values.ingress.httpIngressClassName | quote }}
@@ -171,6 +174,9 @@ spec:
               number: {{ include "flyte-binary.service.http.port" . }}
         path: /oauth2/*
         pathType: ImplementationSpecific
+      {{- if not .Values.ingress.separateGrpcIngress }}
+      {{- include "flyte-binary.ingress.grpc.paths" . | nindent 6 }}
+      {{- end }}
       {{- if .Values.ingress.httpExtraPaths.append }}
       {{- tpl ( .Values.ingress.httpExtraPaths.append | toYaml ) . | nindent 6 }}
       {{- end }}

diff --git a/charts/flyte-binary/values.yaml b/charts/flyte-binary/values.yaml
@@ -324,11 +324,13 @@ service:
 # ingress Configure ingress for Flyte
 ingress:
   # create Create ingress resources
-  create: false
+  create: true
   # labels Add labels to ingress resources
   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