Skip to content

Commit 8fbf558

Browse files
authored
feat: allow configuring timeout and retries for upstream with ingress (apache#1876)
* feat: allow configuring timeout and retries for upstream with ingress annotations Signed-off-by: revolyssup <[email protected]> --------- Signed-off-by: revolyssup <[email protected]> Signed-off-by: Ashish Tiwari <[email protected]>
1 parent 589bae6 commit 8fbf558

File tree

24 files changed

+584
-105
lines changed

24 files changed

+584
-105
lines changed

.github/workflows/e2e-test-ci-v2-cron-dev.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ jobs:
113113
${REGISTRY}/apisix-ingress-controller:dev \
114114
${REGISTRY}/httpbin:dev \
115115
${REGISTRY}/test-backend:dev \
116+
${REGISTRY}/test-timeout:dev \
116117
${REGISTRY}/echo-server:dev \
117118
${REGISTRY}/busybox:dev \
118119
| pigz > docker-dev.tar.gz

.github/workflows/e2e-test-ci-v2-cron.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ jobs:
113113
${REGISTRY}/apisix-ingress-controller:dev \
114114
${REGISTRY}/httpbin:dev \
115115
${REGISTRY}/test-backend:dev \
116+
${REGISTRY}/test-timeout:dev \
116117
${REGISTRY}/echo-server:dev \
117118
${REGISTRY}/busybox:dev \
118119
| pigz > docker-v2.tar.gz

.github/workflows/e2e-test-ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ jobs:
114114
${REGISTRY}/apisix-ingress-controller:dev \
115115
${REGISTRY}/httpbin:dev \
116116
${REGISTRY}/test-backend:dev \
117+
${REGISTRY}/test-timeout:dev \
117118
${REGISTRY}/echo-server:dev \
118119
${REGISTRY}/busybox:dev \
119120
| pigz > docker.tar.gz

.github/workflows/k8s-timer-ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ jobs:
103103
${REGISTRY}/apisix-ingress-controller:dev \
104104
${REGISTRY}/httpbin:dev \
105105
${REGISTRY}/test-backend:dev \
106+
${REGISTRY}/test-timeout:dev \
106107
${REGISTRY}/echo-server:dev \
107108
${REGISTRY}/busybox:dev \
108109
| pigz > docker.tar.gz

Makefile

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,9 @@ ifeq ($(E2E_SKIP_BUILD), 0)
103103
docker tag kennethreitz/httpbin $(REGISTRY)/httpbin:$(IMAGE_TAG)
104104

105105
docker build -t test-backend:$(IMAGE_TAG) --build-arg ENABLE_PROXY=$(ENABLE_PROXY) ./test/e2e/testbackend
106+
docker build -t test-timeout:$(IMAGE_TAG) --build-arg ENABLE_PROXY=$(ENABLE_PROXY) ./test/e2e/testtimeout
106107
docker tag test-backend:$(IMAGE_TAG) $(REGISTRY)/test-backend:$(IMAGE_TAG)
107-
108+
docker tag test-timeout:$(IMAGE_TAG) $(REGISTRY)/test-timeout:$(IMAGE_TAG)
108109
docker tag apache/apisix-ingress-controller:$(IMAGE_TAG) $(REGISTRY)/apisix-ingress-controller:$(IMAGE_TAG)
109110

110111
docker pull jmalloc/echo-server:latest
@@ -122,6 +123,7 @@ ifeq ($(E2E_SKIP_BUILD), 0)
122123
docker push $(REGISTRY)/etcd:$(IMAGE_TAG)
123124
docker push $(REGISTRY)/httpbin:$(IMAGE_TAG)
124125
docker push $(REGISTRY)/test-backend:$(IMAGE_TAG)
126+
docker push $(REGISTRY)/test-timeout:$(IMAGE_TAG)
125127
docker push $(REGISTRY)/apisix-ingress-controller:$(IMAGE_TAG)
126128
docker push $(REGISTRY)/echo-server:$(IMAGE_TAG)
127129
docker push $(REGISTRY)/busybox:$(IMAGE_TAG)
@@ -305,6 +307,7 @@ kind-load-images:
305307
$(REGISTRY)/apisix-ingress-controller:dev \
306308
$(REGISTRY)/httpbin:dev \
307309
$(REGISTRY)/test-backend:dev \
310+
$(REGISTRY)/test-timeout:dev \
308311
$(REGISTRY)/echo-server:dev \
309312
$(REGISTRY)/busybox:dev
310313

docs/en/latest/concepts/annotations.md

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,3 +413,63 @@ spec:
413413
port:
414414
number: 80
415415
```
416+
417+
## Upstream retries
418+
419+
This annotation can be used to configure retries among multiple nodes in an upstream. You may want the proxy to retry when requests occur faults like transient network errors or service unavailable, By default the retry count is 1. You can change it by specifying the retries field.
420+
421+
The following configuration configures the retries to 3, which indicates there'll be at most 3 requests sent to Kubernetes service httpbin's endpoints.
422+
423+
One should bear in mind that passing a request to the next endpoint is only possible if nothing has been sent to a client yet. That is, if an error or timeout occurs in the middle of the transferring of a response, fixing this is impossible.
424+
425+
```yaml
426+
apiVersion: networking.k8s.io/v1
427+
kind: Ingress
428+
metadata:
429+
annotations:
430+
k8s.apisix.apache.org/upstream-retries: "3"
431+
name: ingress-ext-v1beta1
432+
spec:
433+
ingressClassName: apisix
434+
rules:
435+
- host: httpbin.org
436+
http:
437+
paths:
438+
- path: /ip
439+
pathType: Exact
440+
backend:
441+
service:
442+
name: httpbin
443+
port:
444+
number: 80
445+
```
446+
447+
## Upstream timeout
448+
449+
This annotation can be used to configure different types of timeout on an upstream. The default connect, read and send timeout are 60s, which might not be proper for some applications.
450+
451+
The below example sets the read, connect and send timeout to 5s, 10s, 10s respectively.
452+
453+
```yaml
454+
apiVersion: networking.k8s.io/v1
455+
kind: Ingress
456+
metadata:
457+
annotations:
458+
k8s.apisix.apache.org/upstream-read-timeout.: "5s"
459+
k8s.apisix.apache.org/upstream-connect-timeout: "10s"
460+
k8s.apisix.apache.org/upstream-send-timeout: "10s"
461+
name: ingress-ext-v1beta1
462+
spec:
463+
ingressClassName: apisix
464+
rules:
465+
- host: httpbin.org
466+
http:
467+
paths:
468+
- path: /ip
469+
pathType: Exact
470+
backend:
471+
service:
472+
name: httpbin
473+
port:
474+
number: 80
475+
```

pkg/providers/ingress/translation/annotations.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations/plugins"
2525
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations/regex"
2626
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations/servicenamespace"
27-
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations/upstreamscheme"
27+
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations/upstream"
2828
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations/websocket"
2929
apisix "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
3030
)
@@ -36,7 +36,7 @@ type Ingress struct {
3636
EnableWebSocket bool
3737
PluginConfigName string
3838
ServiceNamespace string
39-
UpstreamScheme string
39+
Upstream upstream.Upstream
4040
}
4141

4242
var (
@@ -46,7 +46,7 @@ var (
4646
"EnableWebSocket": websocket.NewParser(),
4747
"PluginConfigName": pluginconfig.NewParser(),
4848
"ServiceNamespace": servicenamespace.NewParser(),
49-
"UpstreamScheme": upstreamscheme.NewParser(),
49+
"Upstream": upstream.NewParser(),
5050
}
5151
)
5252

pkg/providers/ingress/translation/annotations/types.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ const (
2727
AnnotationsEnableWebSocket = AnnotationsPrefix + "enable-websocket"
2828
AnnotationsPluginConfigName = AnnotationsPrefix + "plugin-config-name"
2929
AnnotationsUpstreamScheme = AnnotationsPrefix + "upstream-scheme"
30+
31+
//support retries and timeouts on upstream
32+
AnnotationsUpstreamRetry = AnnotationsPrefix + "upstream-retries"
33+
AnnotationsUpstreamTimeoutConnect = AnnotationsPrefix + "upstream-connect-timeout"
34+
AnnotationsUpstreamTimeoutRead = AnnotationsPrefix + "upstream-read-timeout"
35+
AnnotationsUpstreamTimeoutSend = AnnotationsPrefix + "upstream-send-timeout"
3036
)
3137

3238
const (
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one or more
2+
// contributor license agreements. See the NOTICE file distributed with
3+
// this work for additional information regarding copyright ownership.
4+
// The ASF licenses this file to You under the Apache License, Version 2.0
5+
// (the "License"); you may not use this file except in compliance with
6+
// the License. You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
package upstream
16+
17+
import (
18+
"fmt"
19+
"strconv"
20+
"strings"
21+
22+
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
23+
apisixv1 "github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
24+
)
25+
26+
func NewParser() annotations.IngressAnnotationsParser {
27+
return &Upstream{}
28+
}
29+
30+
type Upstream struct {
31+
Scheme string
32+
Retry int
33+
TimeoutRead int
34+
TimeoutConnect int
35+
TimeoutSend int
36+
}
37+
38+
func (u *Upstream) Parse(e annotations.Extractor) (interface{}, error) {
39+
scheme := strings.ToLower(e.GetStringAnnotation(annotations.AnnotationsUpstreamScheme))
40+
if scheme != "" {
41+
_, ok := apisixv1.ValidSchemes[scheme]
42+
if !ok {
43+
keys := make([]string, 0, len(apisixv1.ValidSchemes))
44+
for key := range apisixv1.ValidSchemes {
45+
keys = append(keys, key)
46+
}
47+
return nil, fmt.Errorf("scheme %s is not supported, Only { %s } are supported", scheme, strings.Join(keys, ", "))
48+
}
49+
u.Scheme = scheme
50+
}
51+
52+
retry := e.GetStringAnnotation(annotations.AnnotationsUpstreamRetry)
53+
if retry != "" {
54+
t, err := strconv.Atoi(retry)
55+
if err != nil {
56+
return nil, fmt.Errorf("could not parse retry as an integer: %s", err.Error())
57+
}
58+
u.Retry = t
59+
}
60+
61+
timeoutConnect := strings.TrimSuffix(e.GetStringAnnotation(annotations.AnnotationsUpstreamTimeoutConnect), "s")
62+
if timeoutConnect != "" {
63+
t, err := strconv.Atoi(timeoutConnect)
64+
if err != nil {
65+
return nil, fmt.Errorf("could not parse timeout as an integer: %s", err.Error())
66+
}
67+
u.TimeoutConnect = t
68+
}
69+
70+
timeoutRead := strings.TrimSuffix(e.GetStringAnnotation(annotations.AnnotationsUpstreamTimeoutRead), "s")
71+
if timeoutRead != "" {
72+
t, err := strconv.Atoi(timeoutRead)
73+
if err != nil {
74+
return nil, fmt.Errorf("could not parse timeout as an integer: %s", err.Error())
75+
}
76+
u.TimeoutRead = t
77+
}
78+
79+
timeoutSend := strings.TrimSuffix(e.GetStringAnnotation(annotations.AnnotationsUpstreamTimeoutSend), "s")
80+
if timeoutSend != "" {
81+
t, err := strconv.Atoi(timeoutSend)
82+
if err != nil {
83+
return nil, fmt.Errorf("could not parse timeout as an integer: %s", err.Error())
84+
}
85+
u.TimeoutSend = t
86+
}
87+
88+
return *u, nil
89+
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one or more
2+
// contributor license agreements. See the NOTICE file distributed with
3+
// this work for additional information regarding copyright ownership.
4+
// The ASF licenses this file to You under the Apache License, Version 2.0
5+
// (the "License"); you may not use this file except in compliance with
6+
// the License. You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
package upstream_test
16+
17+
import (
18+
"testing"
19+
20+
"github.com/stretchr/testify/assert"
21+
22+
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations"
23+
"github.com/apache/apisix-ingress-controller/pkg/providers/ingress/translation/annotations/upstream"
24+
)
25+
26+
func TestIPRestrictionHandler(t *testing.T) {
27+
anno := map[string]string{
28+
annotations.AnnotationsUpstreamScheme: "grpcs",
29+
}
30+
u := upstream.NewParser()
31+
32+
out, err := u.Parse(annotations.NewExtractor(anno))
33+
ups, ok := out.(upstream.Upstream)
34+
if !ok {
35+
t.Fatalf("could not parse upstream")
36+
}
37+
assert.Nil(t, err, "checking given error")
38+
assert.Equal(t, "grpcs", ups.Scheme)
39+
40+
anno[annotations.AnnotationsUpstreamScheme] = "gRPC"
41+
out, err = u.Parse(annotations.NewExtractor(anno))
42+
ups, ok = out.(upstream.Upstream)
43+
if !ok {
44+
t.Fatalf("could not parse upstream")
45+
}
46+
assert.Nil(t, err, "checking given error")
47+
assert.Equal(t, "grpc", ups.Scheme)
48+
49+
anno[annotations.AnnotationsUpstreamScheme] = "nothing"
50+
out, err = u.Parse(annotations.NewExtractor(anno))
51+
assert.NotNil(t, err, "checking given error")
52+
assert.Nil(t, out, "checking given output")
53+
}
54+
55+
func TestRetryParsing(t *testing.T) {
56+
anno := map[string]string{
57+
annotations.AnnotationsUpstreamRetry: "2",
58+
}
59+
u := upstream.NewParser()
60+
out, err := u.Parse(annotations.NewExtractor(anno))
61+
if err != nil {
62+
t.Fatalf(err.Error())
63+
}
64+
ups, ok := out.(upstream.Upstream)
65+
if !ok {
66+
t.Fatalf("could not parse upstream")
67+
}
68+
assert.Nil(t, err, "checking given error")
69+
assert.Equal(t, 2, ups.Retry)
70+
71+
anno[annotations.AnnotationsUpstreamRetry] = "asdf"
72+
out, err = u.Parse(annotations.NewExtractor(anno))
73+
assert.NotNil(t, err, "checking given error")
74+
}
75+
76+
func TestTimeoutParsing(t *testing.T) {
77+
anno := map[string]string{
78+
annotations.AnnotationsUpstreamTimeoutConnect: "2s",
79+
annotations.AnnotationsUpstreamTimeoutRead: "3s",
80+
annotations.AnnotationsUpstreamTimeoutSend: "4s",
81+
}
82+
u := upstream.NewParser()
83+
out, err := u.Parse(annotations.NewExtractor(anno))
84+
if err != nil {
85+
t.Fatalf(err.Error())
86+
}
87+
ups, ok := out.(upstream.Upstream)
88+
if !ok {
89+
t.Fatalf("could not parse upstream")
90+
}
91+
assert.Nil(t, err, "checking given error")
92+
assert.Equal(t, 2, ups.TimeoutConnect)
93+
assert.Equal(t, 3, ups.TimeoutRead)
94+
assert.Equal(t, 4, ups.TimeoutSend)
95+
anno[annotations.AnnotationsUpstreamRetry] = "asdf"
96+
out, err = u.Parse(annotations.NewExtractor(anno))
97+
assert.NotNil(t, err, "checking given error")
98+
}

0 commit comments

Comments
 (0)