New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ingress-nginx version 0.13 for grpc not work #2444

Closed
kinglion811 opened this Issue Apr 28, 2018 · 12 comments

Comments

Projects
None yet
6 participants
@kinglion811

kinglion811 commented Apr 28, 2018

I deploy my ingress-nginx fllow https://github.com/kubernetes/ingress-nginx/blob/master/docs/deploy/index.md
and I deploy the example fllow https://github.com/kubernetes/ingress-nginx/tree/master/docs/examples https://github.com/kubernetes/ingress-nginx/tree/master/docs/examples the example can work with http curl.
but when I deploy myself ingress by useing grpc it not work.
the deployment and service is :

kubernetes version is:v1.10

$ cat serving.yaml
apiVersion: extensions/v1beta1
kind: Deployment
metadata:
name: tensorflowserving
spec:
replicas: 2
selector:
matchLabels:
app: tensorflowserving
template:
metadata:
labels:
app: tensorflowserving
spec:
containers:
- name: tensorflowserving
image: xxxxxx
#command: ["tensorflow_model_server"]
args:
- model_server
- --model_base_path=/serving/cchess
- --port=9000
- --enable_batching=true
resources:
limits:
nvidia.com/gpu: 1
ports:
- containerPort: 9000

apiVersion: v1
kind: Service
metadata:
name: serving-svc
spec:
ports:

  • port: 9000
    targetPort: 9000
    protocol: TCP
    name: http
    selector:
    app: tensorflowserving

the ingress is :
$ cat serving-ingress.yaml
apiVersion: extensions/v1beta1
kind: Ingress
metadata:
name: serving-ingress
annotations:
nginx.ingress.kubernetes.io/grpc-backend: "true"
spec:
rules:

  • host: tensorflow.serving.com
    http:
    paths:
    • backend:
      serviceName: serving-svc
      servicePort: 9000

the nginx config is
$nginx -v
nginx version: nginx/1.13.12

and the grpc is configed:

$cat /etc/nginx/nginx.conf |grep grpc
grpc_pass grpc://default-serving-svc-9000;

but when I make request,it not work,the nginx pod log is :

PRI * HTTP/2.0" 400 174 "-" "-" 0 0.001 [] - - - -

what's the matter?

/sig network

@kinglion811

This comment has been minimized.

kinglion811 commented Apr 28, 2018

I check my nginx config, I find

listen 80 default_server backlog=511;

listen [::]:80 default_server backlog=511;

set $proxy_upstream_name "-";

listen 443 default_server backlog=511 ssl http2;

listen [::]:443 default_server backlog=511 ssl http2;

why the port of 80 don't http2?
should set http2 for port 80?
I deploy my nginx fllow https://www.nginx.com/blog/nginx-1-13-10-grpc/ and only port 80 work,then I deploy my service by type of nodeport,it can work.

@aledbf

This comment has been minimized.

Member

aledbf commented Apr 28, 2018

You need to use SSL. Using http2 on port 80 breaks http traffic

@chalvern

This comment has been minimized.

chalvern commented May 2, 2018

@aledbf I have the same problem , but as far as I learn, gRPC should work without SSL. gRPC use h2c(without ssl) instead of h2 (with ssl)

@kinglion811

This comment has been minimized.

kinglion811 commented May 2, 2018

@aledbf do I have to use ssl?If I don’t want to use ssl. which method can set the ingress nginx config to set

listen 80 default_server backlog=511 http2;

listen [::]:80 default_server backlog=511 http2;

I set http2 of port 80 in ingress-controller container by myself,but the controller will clean http2 for port 80.

@aledbf

This comment has been minimized.

Member

aledbf commented May 2, 2018

@kinglion811 you need to use a custom template

@kinglion811

This comment has been minimized.

kinglion811 commented May 2, 2018

@aledbf I change the template like this:
listen {{ $address }}:{{ $all.ListenPorts.HTTP }}{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}{{ if eq $server.Hostname ""}} default_server {{ if $all.Cfg.ReusePort }}reuseport{{ end }} backlog={{ $all.BacklogSize }}{{end}} http2;
{{ else }}
listen {{ $all.ListenPorts.HTTP }}{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}{{ if eq $server.Hostname "
"}} default_server {{ if $all.Cfg.ReusePort }}reuseport{{ end }} backlog={{ $all.BacklogSize }}{{end}} http2;
{{ end }}
{{ if $all.IsIPV6Enabled }}
{{ range $address := $all.Cfg.BindAddressIpv6 }}
listen {{ $address }}:{{ $all.ListenPorts.HTTP }}{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}{{ if eq $server.Hostname ""}} default_server {{ if $all.Cfg.ReusePort }}reuseport{{ end }} backlog={{ $all.BacklogSize }}{{ end }} http2;
{{ else }}
listen [::]:{{ $all.ListenPorts.HTTP }}{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}{{ if eq $server.Hostname "
"}} default_server {{ if $all.Cfg.ReusePort }}reuseport{{ end }} backlog={{ $all.BacklogSize }}{{ end }} http2;
it work

@aledbf

This comment has been minimized.

Member

aledbf commented May 7, 2018

Closing. I am considering enabling http2 on port 80 if all the paths in the server are grpc services but this could break if you start with just one grpc service and then add another path/s for http

@aledbf aledbf closed this May 7, 2018

@mumoshu

This comment has been minimized.

mumoshu commented May 8, 2018

@aledbf Can we alternatively introduce a dedicated annotation like nginx.ingress.kubernetes.io/http2-listener: "true" to delegate decision of forcing http2 to users? This way, no one will break it unexpectedly anyway 🤔

@aledbf

This comment has been minimized.

Member

aledbf commented May 8, 2018

@mumoshu the problem with that proposal is that you could have two ingresses for the same host and different paths, with true in one and false in the other and we end up in the same situation we have now

@aledbf

This comment has been minimized.

Member

aledbf commented May 8, 2018

Just in case, there is a PR to provide autodetection but is not merged yet https://forum.nginx.org/read.php?29,278877,278929#msg-278929

@mumoshu

This comment has been minimized.

mumoshu commented May 9, 2018

@aledbf Ah, made sense!

So, even my proposed annotation was in-place, one needs to ensure separating nginx-ingress-controller by customizing and differentiating ingress classes? Sounds like too much compared to benefit...

I'll check the ong PR for nginx. Thanks for the pointer

@wendorf

This comment has been minimized.

wendorf commented Aug 14, 2018

Just to provide additional user info, I ran into roughly this issue and have implemented the workarounds mentioned above by creating a second ingress-nginx (with a different class) that sets the custom NGINX template to force http2 for our listeners.

In our use case, we're terminating TLS at a load balancer that forwards to ingress-nginx and we want to support both gRPC and "normal" HTTP 1.x traffic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment