This is an automated email from the ASF dual-hosted git repository.
kvn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
The following commit(s) were added to refs/heads/master by this push:
new fe57c75 fix: use not strictly translation when remote ApisixRoute
(#544)
fe57c75 is described below
commit fe57c7509505ee1474ae19010d137d4f765d4825
Author: kv <[email protected]>
AuthorDate: Sun Jun 13 00:43:18 2021 +0800
fix: use not strictly translation when remote ApisixRoute (#544)
* fix: use not strictly translation when remote ApisixRoute
* fix: lint & comments
* add comments
* fix: add upstreams to ctx when translation
* use upstraemId instead of upstream in tcproute
* fix: check upstream exist
* test: add e2e test
* CI: [Temporary changes]use APISIX:2.6 instead of APISIX:dev
* use Apisix:dev image & use intstring type for counter
* fix: counter
* fix: use json.Unmarshal instead
* fix: the count in cache should be modify too
* recover test case
* fix: style
* fix: count compare
* fix: style
* fix: remove ginkgo focus test case
---
pkg/apisix/resource.go | 20 +++++++-
pkg/ingress/apisix_route.go | 9 +++-
pkg/kube/translation/apisix_route.go | 68 +++++++++++++++++++++++++-
pkg/kube/translation/apisix_route_test.go | 62 ++++++++++++++++++++++++
pkg/kube/translation/translator.go | 3 ++
pkg/kube/translation/util.go | 8 ++++
test/e2e/ingress/resourcepushing.go | 80 +++++++++++++++++++++++++++++++
test/e2e/scaffold/k8s.go | 19 ++++----
8 files changed, 255 insertions(+), 14 deletions(-)
diff --git a/pkg/apisix/resource.go b/pkg/apisix/resource.go
index 9efde6c..3792de6 100644
--- a/pkg/apisix/resource.go
+++ b/pkg/apisix/resource.go
@@ -18,6 +18,7 @@ import (
"encoding/json"
"errors"
"fmt"
+ "strconv"
"strings"
"github.com/apache/apisix-ingress-controller/pkg/log"
@@ -30,8 +31,23 @@ type getResponse struct {
// listResponse is the unified LIST response mapping of APISIX.
type listResponse struct {
- Count string `json:"count"`
- Node node `json:"node"`
+ Count IntOrString `json:"count"`
+ Node node `json:"node"`
+}
+
+// IntOrString processing number and string types, after json deserialization
will output int
+type IntOrString struct {
+ IntValue int `json:"int_value"`
+}
+
+func (ios *IntOrString) UnmarshalJSON(p []byte) error {
+ result := strings.Trim(string(p), "\"")
+ count, err := strconv.Atoi(result)
+ if err != nil {
+ return err
+ }
+ ios.IntValue = count
+ return nil
}
type createResponse struct {
diff --git a/pkg/ingress/apisix_route.go b/pkg/ingress/apisix_route.go
index 1172c73..5a1a6c3 100644
--- a/pkg/ingress/apisix_route.go
+++ b/pkg/ingress/apisix_route.go
@@ -139,7 +139,14 @@ func (c *apisixRouteController) sync(ctx context.Context,
ev *types.Event) error
return err
}
} else {
- tctx, err =
c.controller.translator.TranslateRouteV2alpha1(ar.V2alpha1())
+ if ev.Type != types.EventDelete {
+ tctx, err =
c.controller.translator.TranslateRouteV2alpha1(ar.V2alpha1())
+ } else {
+ // Use TranslateRouteV2alpha1NotStrictly in EventDelete.
+ // if K8S service has been removed before ApisixRoute
resource, the translation about nodes
+ // of upstream will be failed.
+ tctx, err =
c.controller.translator.TranslateRouteV2alpha1NotStrictly(ar.V2alpha1())
+ }
if err != nil {
log.Errorw("failed to translate ApisixRoute v2alpha1",
zap.Error(err),
diff --git a/pkg/kube/translation/apisix_route.go
b/pkg/kube/translation/apisix_route.go
index ba2ef2b..a8ea661 100644
--- a/pkg/kube/translation/apisix_route.go
+++ b/pkg/kube/translation/apisix_route.go
@@ -78,6 +78,21 @@ func (t *translator) TranslateRouteV1(ar
*configv1.ApisixRoute) (*TranslateConte
return ctx, nil
}
+// TranslateRouteV2alpha1NotStrictly translates route v2alpha1 with a loose
way, only generate ID and Name for delete Event.
+func (t *translator) TranslateRouteV2alpha1NotStrictly(ar
*configv2alpha1.ApisixRoute) (*TranslateContext, error) {
+ ctx := &TranslateContext{
+ upstreamMap: make(map[string]struct{}),
+ }
+
+ if err := t.translateHTTPRouteNotStrictly(ctx, ar); err != nil {
+ return nil, err
+ }
+ if err := t.translateTCPRouteNotStrictly(ctx, ar); err != nil {
+ return nil, err
+ }
+ return ctx, nil
+}
+
func (t *translator) TranslateRouteV2alpha1(ar *configv2alpha1.ApisixRoute)
(*TranslateContext, error) {
ctx := &TranslateContext{
upstreamMap: make(map[string]struct{}),
@@ -92,6 +107,32 @@ func (t *translator) TranslateRouteV2alpha1(ar
*configv2alpha1.ApisixRoute) (*Tr
return ctx, nil
}
+// translateHTTPRouteNotStrictly translates http route with a loose way, only
generate ID and Name for delete Event.
+func (t *translator) translateHTTPRouteNotStrictly(ctx *TranslateContext, ar
*configv2alpha1.ApisixRoute) error {
+ for _, part := range ar.Spec.HTTP {
+ backends := part.Backends
+ backend := part.Backend
+ if len(backends) > 0 {
+ // Use the first backend as the default backend in
Route,
+ // others will be configured in traffic-split plugin.
+ backend = backends[0]
+ } // else use the deprecated Backend.
+ upstreamName := apisixv1.ComposeUpstreamName(ar.Namespace,
backend.ServiceName, backend.Subset, backend.ServicePort.IntVal)
+ route := apisixv1.NewDefaultRoute()
+ route.Name = apisixv1.ComposeRouteName(ar.Namespace, ar.Name,
part.Name)
+ route.ID = id.GenID(route.Name)
+ ctx.addRoute(route)
+ if !ctx.checkUpstreamExist(upstreamName) {
+ ups, err :=
t.translateUpstreamNotStrictly(ar.Namespace, backend.ServiceName,
backend.Subset, backend.ServicePort.IntVal)
+ if err != nil {
+ return err
+ }
+ ctx.addUpstream(ups)
+ }
+ }
+ return nil
+}
+
func (t *translator) translateHTTPRoute(ctx *TranslateContext, ar
*configv2alpha1.ApisixRoute) error {
ruleNameMap := make(map[string]struct{})
for _, part := range ar.Spec.HTTP {
@@ -304,6 +345,27 @@ func (t *translator) translateRouteMatchExprs(nginxVars
[]configv2alpha1.ApisixR
return vars, nil
}
+// translateTCPRouteNotStrictly translates tcp route with a loose way, only
generate ID and Name for delete Event.
+func (t *translator) translateTCPRouteNotStrictly(ctx *TranslateContext, ar
*configv2alpha1.ApisixRoute) error {
+ for _, part := range ar.Spec.TCP {
+ backend := &part.Backend
+ sr := apisixv1.NewDefaultStreamRoute()
+ name := apisixv1.ComposeStreamRouteName(ar.Namespace, ar.Name,
part.Name)
+ sr.ID = id.GenID(name)
+ sr.ServerPort = part.Match.IngressPort
+ ups, err := t.translateUpstreamNotStrictly(ar.Namespace,
backend.ServiceName, backend.Subset, backend.ServicePort.IntVal)
+ if err != nil {
+ return err
+ }
+ sr.UpstreamId = ups.ID
+ ctx.addStreamRoute(sr)
+ if !ctx.checkUpstreamExist(ups.Name) {
+ ctx.addUpstream(ups)
+ }
+ }
+ return nil
+}
+
func (t *translator) translateTCPRoute(ctx *TranslateContext, ar
*configv2alpha1.ApisixRoute) error {
ruleNameMap := make(map[string]struct{})
for _, part := range ar.Spec.TCP {
@@ -329,8 +391,12 @@ func (t *translator) translateTCPRoute(ctx
*TranslateContext, ar *configv2alpha1
if err != nil {
return err
}
- sr.Upstream = ups
+ sr.UpstreamId = ups.ID
ctx.addStreamRoute(sr)
+ if !ctx.checkUpstreamExist(ups.Name) {
+ ctx.addUpstream(ups)
+ }
+
}
return nil
}
diff --git a/pkg/kube/translation/apisix_route_test.go
b/pkg/kube/translation/apisix_route_test.go
index 24f7c43..8106ebf 100644
--- a/pkg/kube/translation/apisix_route_test.go
+++ b/pkg/kube/translation/apisix_route_test.go
@@ -16,6 +16,7 @@ package translation
import (
"context"
+ "fmt"
"testing"
"github.com/stretchr/testify/assert"
@@ -26,6 +27,7 @@ import (
"k8s.io/client-go/kubernetes/fake"
"k8s.io/client-go/tools/cache"
+ "github.com/apache/apisix-ingress-controller/pkg/id"
configv2alpha1
"github.com/apache/apisix-ingress-controller/pkg/kube/apisix/apis/config/v2alpha1"
fakeapisix
"github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/clientset/versioned/fake"
apisixinformers
"github.com/apache/apisix-ingress-controller/pkg/kube/apisix/client/informers/externalversions"
@@ -320,3 +322,63 @@ func TestTranslateApisixRouteV2alpha1WithDuplicatedName(t
*testing.T) {
_, err = tr.TranslateRouteV2alpha1(ar)
assert.Equal(t, err.Error(), "duplicated route rule name")
}
+
+func TestTranslateApisixRouteV2alpha1NotStrictly(t *testing.T) {
+ tr := &translator{
+ &TranslatorOptions{},
+ }
+ ar := &configv2alpha1.ApisixRoute{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "ar",
+ Namespace: "test",
+ },
+ Spec: &configv2alpha1.ApisixRouteSpec{
+ HTTP: []*configv2alpha1.ApisixRouteHTTP{
+ {
+ Name: "rule1",
+ Match:
&configv2alpha1.ApisixRouteHTTPMatch{
+ Paths: []string{
+ "/*",
+ },
+ },
+ Backend:
&configv2alpha1.ApisixRouteHTTPBackend{
+ ServiceName: "svc1",
+ ServicePort: intstr.IntOrString{
+ IntVal: 81,
+ },
+ },
+ },
+ {
+ Name: "rule2",
+ Match:
&configv2alpha1.ApisixRouteHTTPMatch{
+ Paths: []string{
+ "/*",
+ },
+ },
+ Backend:
&configv2alpha1.ApisixRouteHTTPBackend{
+ ServiceName: "svc2",
+ ServicePort: intstr.IntOrString{
+ IntVal: 82,
+ },
+ },
+ },
+ },
+ },
+ }
+
+ tx, err := tr.TranslateRouteV2alpha1NotStrictly(ar)
+ fmt.Println(tx)
+ assert.NoError(t, err, "translateRoute not strictly should be no error")
+ assert.Equal(t, len(tx.Routes), 2, "There should be 2 routes")
+ assert.Equal(t, len(tx.Upstreams), 2, "There should be 2 upstreams")
+ assert.Equal(t, tx.Routes[0].Name, "test_ar_rule1", "route1 name error")
+ assert.Equal(t, tx.Routes[1].Name, "test_ar_rule2", "route2 name error")
+ assert.Equal(t, tx.Upstreams[0].Name, "test_svc1_81", "upstream1 name
error")
+ assert.Equal(t, tx.Upstreams[1].Name, "test_svc2_82", "upstream2 name
error")
+
+ assert.Equal(t, tx.Routes[0].ID, id.GenID("test_ar_rule1"), "route1 id
error")
+ assert.Equal(t, tx.Routes[1].ID, id.GenID("test_ar_rule2"), "route2 id
error")
+ assert.Equal(t, tx.Upstreams[0].ID, id.GenID("test_svc1_81"),
"upstream1 id error")
+ assert.Equal(t, tx.Upstreams[1].ID, id.GenID("test_svc2_82"),
"upstream2 id error")
+
+}
diff --git a/pkg/kube/translation/translator.go
b/pkg/kube/translation/translator.go
index 5f833f5..8682213 100644
--- a/pkg/kube/translation/translator.go
+++ b/pkg/kube/translation/translator.go
@@ -70,6 +70,9 @@ type Translator interface {
// TranslateRouteV2alpha1 translates the configv2alpha1.ApisixRoute
object into several Route
// and Upstream resources.
TranslateRouteV2alpha1(*configv2alpha1.ApisixRoute) (*TranslateContext,
error)
+ // TranslateRouteV2alpha1NotStrictly translates the
configv2alpha1.ApisixRoute object into several Route
+ // and Upstream resources not strictly, only used for delete event.
+ TranslateRouteV2alpha1NotStrictly(*configv2alpha1.ApisixRoute)
(*TranslateContext, error)
// TranslateSSL translates the configv2alpha1.ApisixTls object into the
APISIX SSL resource.
TranslateSSL(*configv1.ApisixTls) (*apisixv1.Ssl, error)
// TranslateClusterConfig translates the
configv2alpha1.ApisixClusterConfig object into the APISIX
diff --git a/pkg/kube/translation/util.go b/pkg/kube/translation/util.go
index 0dff70d..c57d809 100644
--- a/pkg/kube/translation/util.go
+++ b/pkg/kube/translation/util.go
@@ -110,6 +110,14 @@ loop:
return svc.Spec.ClusterIP, svcPort, nil
}
+// translateUpstreamNotStrictly translates Upstream nodes with a loose way,
only generate ID and Name for delete Event.
+func (t *translator) translateUpstreamNotStrictly(namespace, svcName, subset
string, svcPort int32) (*apisixv1.Upstream, error) {
+ ups := &apisixv1.Upstream{}
+ ups.Name = apisixv1.ComposeUpstreamName(namespace, svcName, subset,
svcPort)
+ ups.ID = id.GenID(ups.Name)
+ return ups, nil
+}
+
func (t *translator) translateUpstream(namespace, svcName, subset,
svcResolveGranularity, svcClusterIP string, svcPort int32) (*apisixv1.Upstream,
error) {
ups, err := t.TranslateUpstream(namespace, svcName, subset, svcPort)
if err != nil {
diff --git a/test/e2e/ingress/resourcepushing.go
b/test/e2e/ingress/resourcepushing.go
index 6f432bb..555b2f8 100644
--- a/test/e2e/ingress/resourcepushing.go
+++ b/test/e2e/ingress/resourcepushing.go
@@ -150,6 +150,86 @@ spec:
assert.Contains(ginkgo.GinkgoT(), body, "404 Route Not Found")
})
+ ginkgo.It("create, update, remove k8s service, remove ApisixRoute",
func() {
+ backendSvc, backendSvcPort := s.DefaultHTTPBackend()
+ apisixRoute := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2alpha1
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+ http:
+ - name: rule1
+ match:
+ hosts:
+ - httpbin.com
+ paths:
+ - /ip
+ backend:
+ serviceName: %s
+ servicePort: %d
+`, backendSvc, backendSvcPort[0])
+
+ assert.Nil(ginkgo.GinkgoT(),
s.CreateResourceFromString(apisixRoute), "creating ApisixRoute")
+ err := s.EnsureNumApisixRoutesCreated(1)
+ assert.Nil(ginkgo.GinkgoT(), err, "Checking number of routes")
+ err = s.EnsureNumApisixUpstreamsCreated(1)
+ assert.Nil(ginkgo.GinkgoT(), err, "Checking number of
upstreams")
+
+ s.NewAPISIXClient().GET("/ip").WithHeader("Host",
"httpbin.com").Expect().Status(http.StatusOK)
+
+ // update
+ apisixRoute = fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2alpha1
+kind: ApisixRoute
+metadata:
+ name: httpbin-route
+spec:
+ http:
+ - name: rule1
+ match:
+ hosts:
+ - httpbin.com
+ paths:
+ - /ip
+ exprs:
+ - subject:
+ scope: Header
+ name: X-Foo
+ op: Equal
+ value: "barbaz"
+ backend:
+ serviceName: %s
+ servicePort: %d
+`, backendSvc, backendSvcPort[0])
+
+ assert.Nil(ginkgo.GinkgoT(),
s.CreateResourceFromString(apisixRoute))
+ // TODO When ingress controller can feedback the lifecycle of
CRDs to the
+ // status field, we can poll it rather than sleeping.
+ time.Sleep(10 * time.Second)
+
+ err = s.EnsureNumApisixRoutesCreated(1)
+ assert.Nil(ginkgo.GinkgoT(), err, "Checking number of routes")
+ err = s.EnsureNumApisixUpstreamsCreated(1)
+ assert.Nil(ginkgo.GinkgoT(), err, "Checking number of
upstreams")
+
+ s.NewAPISIXClient().GET("/ip").WithHeader("Host",
"httpbin.com").Expect().Status(http.StatusNotFound)
+ s.NewAPISIXClient().GET("/ip").WithHeader("Host",
"httpbin.com").WithHeader("X-Foo", "barbaz").Expect().Status(http.StatusOK)
+ // remove k8s service first
+ s.DeleteHTTPBINService()
+ // remove
+ assert.Nil(ginkgo.GinkgoT(),
s.RemoveResourceByString(apisixRoute))
+ // TODO When ingress controller can feedback the lifecycle of
CRDs to the
+ // status field, we can poll it rather than sleeping.
+ time.Sleep(10 * time.Second)
+ ups, err := s.ListApisixUpstreams()
+ assert.Nil(ginkgo.GinkgoT(), err, "list upstreams error")
+ assert.Len(ginkgo.GinkgoT(), ups, 0, "upstreams nodes not
expect")
+
+ body := s.NewAPISIXClient().GET("/ip").WithHeader("Host",
"httpbin.com").Expect().Status(http.StatusNotFound).Body().Raw()
+ assert.Contains(ginkgo.GinkgoT(), body, "404 Route Not Found")
+ })
+
ginkgo.It("change route rule name", func() {
backendSvc, backendSvcPort := s.DefaultHTTPBackend()
apisixRoute := fmt.Sprintf(`
diff --git a/test/e2e/scaffold/k8s.go b/test/e2e/scaffold/k8s.go
index 1412f48..40e6c76 100644
--- a/test/e2e/scaffold/k8s.go
+++ b/test/e2e/scaffold/k8s.go
@@ -17,9 +17,9 @@ package scaffold
import (
"context"
"encoding/json"
+ "io/ioutil"
"net/http"
"net/url"
- "strconv"
"time"
"github.com/apache/apisix-ingress-controller/pkg/apisix"
@@ -33,7 +33,7 @@ import (
)
type counter struct {
- Count string `json:"count"`
+ Count apisix.IntOrString `json:"count"`
}
// ApisixRoute is the ApisixRoute CRD definition.
@@ -150,19 +150,18 @@ func (s *Scaffold) ensureNumApisixCRDsCreated(url string,
desired int) error {
ginkgo.GinkgoT().Logf("got status code %d from APISIX",
resp.StatusCode)
return false, nil
}
- var c counter
- dec := json.NewDecoder(resp.Body)
- if err := dec.Decode(&c); err != nil {
+ c := &counter{}
+ b, err := ioutil.ReadAll(resp.Body)
+ if err != nil {
return false, err
}
- // NOTE count field is a string.
- count, err := strconv.Atoi(c.Count)
+ err = json.Unmarshal(b, c)
if err != nil {
return false, err
}
- // 1 for dir.
- if count != desired+1 {
- ginkgo.GinkgoT().Logf("mismatched number of items,
expected %d but found %d", desired, count-1)
+ count := c.Count.IntValue
+ if count != desired {
+ ginkgo.GinkgoT().Logf("mismatched number of items,
expected %d but found %d", desired, count)
return false, nil
}
return true, nil
