From d0e9934789d5f07699788d5fa1f3819ebc0a8919 Mon Sep 17 00:00:00 2001 From: Anddd7 Date: Mon, 27 May 2024 00:37:11 +0800 Subject: [PATCH] feat: Add grpc timeouts annotations (#11258) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * โœจ feat: add grpc timeouts with proxy settings if backend is grpc * ๐Ÿ“ docs: Documentation only changes * ๐Ÿ› fix: uppercase for protocol * ๐Ÿ“ docs: grpc timeouts example * ๐Ÿ“ docs: add links and default values for proxy timeout * ๐Ÿงช test: add e2e test for timeout * ๐Ÿ› fix: upgrade to 1.0.6 to fix nil pointer * ๐Ÿ› fix: lint * ๐Ÿงช test: trigger ci --- docs/examples/grpc/README.md | 6 +- .../nginx-configuration/annotations.md | 6 + .../nginx-configuration/configmap.md | 6 + go.mod | 1 + go.sum | 2 + rootfs/etc/nginx/template/nginx.tmpl | 7 ++ test/e2e/annotations/grpc.go | 94 ++++++++++++++- test/e2e/framework/grpc_delay.go | 109 ++++++++++++++++++ 8 files changed, 224 insertions(+), 7 deletions(-) create mode 100644 test/e2e/framework/grpc_delay.go diff --git a/docs/examples/grpc/README.md b/docs/examples/grpc/README.md index 508b23fb8f..23126c3453 100644 --- a/docs/examples/grpc/README.md +++ b/docs/examples/grpc/README.md @@ -166,11 +166,9 @@ This example demonstrates how to route traffic to a gRPC service through the Ing ### Notes on using response/request streams +> `grpc_read_timeout` and `grpc_send_timeout` will be set as `proxy_read_timeout` and `proxy_send_timeout` when you set backend protocol to `GRPC` or `GRPCS`. + 1. If your server only does response streaming and you expect a stream to be open longer than 60 seconds, you will have to change the `grpc_read_timeout` to accommodate this. 2. If your service only does request streaming and you expect a stream to be open longer than 60 seconds, you have to change the `grpc_send_timeout` and the `client_body_timeout`. 3. If you do both response and request streaming with an open stream longer than 60 seconds, you have to change all three timeouts: `grpc_read_timeout`, `grpc_send_timeout` and `client_body_timeout`. - -Values for the timeouts must be specified as e.g. `"1200s"`. - -> On the most recent versions of ingress-nginx, changing these timeouts requires using the `nginx.ingress.kubernetes.io/server-snippet` annotation. There are plans for future releases to allow using the Kubernetes annotations to define each timeout separately. diff --git a/docs/user-guide/nginx-configuration/annotations.md b/docs/user-guide/nginx-configuration/annotations.md index cf4902c1eb..0ea54a29fe 100755 --- a/docs/user-guide/nginx-configuration/annotations.md +++ b/docs/user-guide/nginx-configuration/annotations.md @@ -698,6 +698,12 @@ In some scenarios is required to have different values. To allow this we provide - `nginx.ingress.kubernetes.io/proxy-next-upstream-tries` - `nginx.ingress.kubernetes.io/proxy-request-buffering` +If you indicate [Backend Protocol](#backend-protocol) as `GRPC` or `GRPCS`, the following grpc values will be set and inherited from proxy timeouts: + +- [`grpc_connect_timeout=5s`](https://nginx.org/en/docs/http/ngx_http_grpc_module.html#grpc_connect_timeout), from `nginx.ingress.kubernetes.io/proxy-connect-timeout` +- [`grpc_send_timeout=60s`](https://nginx.org/en/docs/http/ngx_http_grpc_module.html#grpc_send_timeout), from `nginx.ingress.kubernetes.io/proxy-send-timeout` +- [`grpc_read_timeout=60s`](https://nginx.org/en/docs/http/ngx_http_grpc_module.html#grpc_read_timeout), from `nginx.ingress.kubernetes.io/proxy-read-timeout` + Note: All timeout values are unitless and in seconds e.g. `nginx.ingress.kubernetes.io/proxy-read-timeout: "120"` sets a valid 120 seconds proxy read timeout. ### Proxy redirect diff --git a/docs/user-guide/nginx-configuration/configmap.md b/docs/user-guide/nginx-configuration/configmap.md index 3127f73a45..48ab316cbb 100644 --- a/docs/user-guide/nginx-configuration/configmap.md +++ b/docs/user-guide/nginx-configuration/configmap.md @@ -1101,14 +1101,20 @@ See NGINX [client_max_body_size](https://nginx.org/en/docs/http/ngx_http_core_mo Sets the timeout for [establishing a connection with a proxied server](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_connect_timeout). It should be noted that this timeout cannot usually exceed 75 seconds. +It will also set the [grpc_connect_timeout](https://nginx.org/en/docs/http/ngx_http_grpc_module.html#grpc_connect_timeout) for gRPC connections. + ## proxy-read-timeout Sets the timeout in seconds for [reading a response from the proxied server](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_read_timeout). The timeout is set only between two successive read operations, not for the transmission of the whole response. +It will also set the [grpc_read_timeout](https://nginx.org/en/docs/http/ngx_http_grpc_module.html#grpc_read_timeout) for gRPC connections. + ## proxy-send-timeout Sets the timeout in seconds for [transmitting a request to the proxied server](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_send_timeout). The timeout is set only between two successive write operations, not for the transmission of the whole request. +It will also set the [grpc_send_timeout](https://nginx.org/en/docs/http/ngx_http_grpc_module.html#grpc_send_timeout) for gRPC connections. + ## proxy-buffers-number Sets the number of the buffer used for [reading the first part of the response](https://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_buffers) received from the proxied server. This part usually contains a small response header. diff --git a/go.mod b/go.mod index cec60ef9c9..75782885ae 100644 --- a/go.mod +++ b/go.mod @@ -48,6 +48,7 @@ require ( ) require ( + github.com/Anddd7/pb v0.0.0-20240425032658-369b0f6a404c github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 // indirect github.com/BurntSushi/toml v1.3.2 // indirect github.com/beorn7/perks v1.0.1 // indirect diff --git a/go.sum b/go.sum index 1ad706885a..d62944e1f5 100644 --- a/go.sum +++ b/go.sum @@ -597,6 +597,8 @@ cloud.google.com/go/workflows v1.10.0/go.mod h1:fZ8LmRmZQWacon9UCX1r/g/DfAXx5VcP dario.cat/mergo v1.0.0 h1:AGCNq9Evsj31mOgNPcLyXc+4PNABt905YmuqPYYpBWk= dario.cat/mergo v1.0.0/go.mod h1:uNxQE+84aUszobStD9th8a29P2fMDhsBdgRYvZOxGmk= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= +github.com/Anddd7/pb v0.0.0-20240425032658-369b0f6a404c h1:uhBf0CHXi7nCFZXxHV7l1cBcYFEEVRK4FYxvm1l9lKg= +github.com/Anddd7/pb v0.0.0-20240425032658-369b0f6a404c/go.mod h1:vYWKbnXd2KAZHUECLPzSE0Er3FgiEmOdPtxwSIRihck= gioui.org v0.0.0-20210308172011-57750fc8a0a6/go.mod h1:RSH6KIUZ0p2xy5zHDxgAM4zumjgTw83q2ge/PI+yyw8= git.sr.ht/~sbinet/gg v0.3.1/go.mod h1:KGYtlADtqsqANL9ueOFkWymvzUvLMQllU5Ixo+8v3pc= github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 h1:L/gRVlceqvL25UVaW/CKtUDjefjrs0SPonmDGUVOYP0= diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index 4c0da2eb95..93a04e3e62 100644 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -1481,6 +1481,13 @@ stream { proxy_next_upstream_timeout {{ $location.Proxy.NextUpstreamTimeout }}; proxy_next_upstream_tries {{ $location.Proxy.NextUpstreamTries }}; + {{ if or (eq $location.BackendProtocol "GRPC") (eq $location.BackendProtocol "GRPCS") }} + # Grpc settings + grpc_connect_timeout {{ $location.Proxy.ConnectTimeout }}s; + grpc_send_timeout {{ $location.Proxy.SendTimeout }}s; + grpc_read_timeout {{ $location.Proxy.ReadTimeout }}s; + {{ end }} + {{/* Add any additional configuration defined */}} {{ $location.ConfigurationSnippet }} diff --git a/test/e2e/annotations/grpc.go b/test/e2e/annotations/grpc.go index b3be82c2a7..60696e1395 100644 --- a/test/e2e/annotations/grpc.go +++ b/test/e2e/annotations/grpc.go @@ -22,11 +22,13 @@ import ( "fmt" "strings" + delaypb "github.com/Anddd7/pb/grpcbin" pb "github.com/moul/pb/grpcbin/go-grpc" "github.com/onsi/ginkgo/v2" "github.com/stretchr/testify/assert" "google.golang.org/grpc" "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/metadata" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,7 +37,10 @@ import ( "k8s.io/ingress-nginx/test/e2e/framework" ) -const echoHost = "echo" +const ( + echoHost = "echo" + host = "grpc" +) var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { f := framework.NewDefaultFramework("grpc", framework.WithHTTPBunEnabled()) @@ -43,8 +48,6 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { ginkgo.It("should use grpc_pass in the configuration file", func() { f.NewGRPCFortuneTellerDeployment() - host := "grpc" - annotations := map[string]string{ "nginx.ingress.kubernetes.io/backend-protocol": "GRPC", } @@ -259,4 +262,89 @@ var _ = framework.DescribeAnnotation("backend-protocol - GRPC", func() { metadata := res.GetMetadata() assert.Equal(ginkgo.GinkgoT(), metadata["content-type"].Values[0], "application/grpc") }) + + ginkgo.It("should return OK when request not exceed timeout", func() { + f.NewGRPCBinDelayDeployment() + + proxyTimeout := "10" + ingressName := "grpcbin-delay" + + annotations := make(map[string]string) + annotations["nginx.ingress.kubernetes.io/backend-protocol"] = "GRPC" + annotations["nginx.ingress.kubernetes.io/proxy-connect-timeout"] = proxyTimeout + annotations["nginx.ingress.kubernetes.io/proxy-send-timeout"] = proxyTimeout + annotations["nginx.ingress.kubernetes.io/proxy-read-timeout"] = proxyTimeout + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, ingressName, 50051, annotations) + + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, fmt.Sprintf("grpc_connect_timeout %ss;", proxyTimeout)) && + strings.Contains(server, fmt.Sprintf("grpc_send_timeout %ss;", proxyTimeout)) && + strings.Contains(server, fmt.Sprintf("grpc_read_timeout %ss;", proxyTimeout)) + }) + + conn, err := grpc.Dial( + f.GetNginxIP()+":80", + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithAuthority(host), + ) + assert.Nil(ginkgo.GinkgoT(), err, "error creating a connection") + defer conn.Close() + + client := delaypb.NewGrpcbinServiceClient(conn) + + res, err := client.Unary(context.Background(), &delaypb.UnaryRequest{ + Data: "hello", + }) + assert.Nil(ginkgo.GinkgoT(), err) + + metadata := res.GetResponseAttributes().RequestHeaders + assert.Equal(ginkgo.GinkgoT(), metadata["content-type"], "application/grpc") + assert.Equal(ginkgo.GinkgoT(), metadata[":authority"], host) + }) + + ginkgo.It("should return Error when request exceed timeout", func() { + f.NewGRPCBinDelayDeployment() + + proxyTimeout := "10" + ingressName := "grpcbin-delay" + + annotations := make(map[string]string) + annotations["nginx.ingress.kubernetes.io/backend-protocol"] = "GRPC" + annotations["nginx.ingress.kubernetes.io/proxy-connect-timeout"] = proxyTimeout + annotations["nginx.ingress.kubernetes.io/proxy-send-timeout"] = proxyTimeout + annotations["nginx.ingress.kubernetes.io/proxy-read-timeout"] = proxyTimeout + + ing := framework.NewSingleIngress(host, "/", host, f.Namespace, ingressName, 50051, annotations) + + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, fmt.Sprintf("grpc_connect_timeout %ss;", proxyTimeout)) && + strings.Contains(server, fmt.Sprintf("grpc_send_timeout %ss;", proxyTimeout)) && + strings.Contains(server, fmt.Sprintf("grpc_read_timeout %ss;", proxyTimeout)) + }) + + conn, err := grpc.Dial( + f.GetNginxIP()+":80", + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithAuthority(host), + ) + assert.Nil(ginkgo.GinkgoT(), err, "error creating a connection") + defer conn.Close() + + client := delaypb.NewGrpcbinServiceClient(conn) + + _, err = client.Unary(context.Background(), &delaypb.UnaryRequest{ + Data: "hello", + RequestAttributes: &delaypb.RequestAttributes{ + Delay: 15, + }, + }) + assert.Error(ginkgo.GinkgoT(), err) + }) }) diff --git a/test/e2e/framework/grpc_delay.go b/test/e2e/framework/grpc_delay.go new file mode 100644 index 0000000000..58d10b2e97 --- /dev/null +++ b/test/e2e/framework/grpc_delay.go @@ -0,0 +1,109 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package framework + +import ( + "github.com/onsi/ginkgo/v2" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/util/intstr" +) + +// NewGRPCBinDelayDeployment creates a new single replica +// deployment of the grpcbin image in a particular namespace +func (f *Framework) NewGRPCBinDelayDeployment() { + f.NewNewGRPCBinDelayDeploymentWithReplicas(1) +} + +// NewNewGRPCBinDelayDeploymentWithReplicas creates a new deployment of the +// grpcbin image in a particular namespace. Number of replicas is configurable +func (f *Framework) NewNewGRPCBinDelayDeploymentWithReplicas(replicas int32) { + name := "grpcbin-delay" + + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: f.Namespace, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: NewInt32(replicas), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": name, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": name, + }, + }, + Spec: corev1.PodSpec{ + TerminationGracePeriodSeconds: NewInt64(0), + Containers: []corev1.Container{ + { + Name: name, + Image: "ghcr.io/anddd7/grpcbin:v1.0.6", + Env: []corev1.EnvVar{}, + Ports: []corev1.ContainerPort{ + { + Name: "grpc", + ContainerPort: 50051, + }, + }, + }, + }, + }, + }, + }, + } + + d := f.EnsureDeployment(deployment) + + err := waitForPodsReady(f.KubeClientSet, DefaultTimeout, int(replicas), f.Namespace, &metav1.ListOptions{ + LabelSelector: fields.SelectorFromSet(fields.Set(d.Spec.Template.ObjectMeta.Labels)).String(), + }) + assert.Nil(ginkgo.GinkgoT(), err, "failed to wait for to become ready") + + service := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: f.Namespace, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "grpc", + Port: 50051, + TargetPort: intstr.FromInt(50051), + Protocol: "TCP", + }, + }, + Selector: map[string]string{ + "app": name, + }, + }, + } + + f.EnsureService(service) + + err = WaitForEndpoints(f.KubeClientSet, DefaultTimeout, name, f.Namespace, int(replicas)) + assert.Nil(ginkgo.GinkgoT(), err, "waiting for endpoints to become ready") +}