This is an automated email from the ASF dual-hosted git repository.
tokers 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 af0dec4 fix: handle upstream caching delete correctly (#422)
af0dec4 is described below
commit af0dec4f12dd5f2edacee4184d5190d745bc497d
Author: Alex Zhang <[email protected]>
AuthorDate: Sat May 8 13:47:22 2021 +0800
fix: handle upstream caching delete correctly (#422)
---
pkg/ingress/manifest.go | 21 ++++++--
test/e2e/ingress/resourcepushing.go | 102 ++++++++++++++++++++++++++++++++++++
2 files changed, 118 insertions(+), 5 deletions(-)
diff --git a/pkg/ingress/manifest.go b/pkg/ingress/manifest.go
index 42cfb61..d597714 100644
--- a/pkg/ingress/manifest.go
+++ b/pkg/ingress/manifest.go
@@ -19,7 +19,10 @@ import (
"reflect"
"github.com/hashicorp/go-multierror"
+ "go.uber.org/zap"
+ "github.com/apache/apisix-ingress-controller/pkg/apisix/cache"
+ "github.com/apache/apisix-ingress-controller/pkg/log"
apisixv1
"github.com/apache/apisix-ingress-controller/pkg/types/apisix/v1"
)
@@ -149,16 +152,24 @@ func (c *Controller) syncManifests(ctx context.Context,
added, updated, deleted
merr = multierror.Append(merr, err)
}
}
- for _, u := range deleted.upstreams {
- if err :=
c.apisix.Cluster(clusterName).Upstream().Delete(ctx, u); err != nil {
- merr = multierror.Append(merr, err)
- }
- }
for _, sr := range deleted.streamRoutes {
if err :=
c.apisix.Cluster(clusterName).StreamRoute().Delete(ctx, sr); err != nil {
merr = multierror.Append(merr, err)
}
}
+ for _, u := range deleted.upstreams {
+ if err :=
c.apisix.Cluster(clusterName).Upstream().Delete(ctx, u); err != nil {
+ // Upstream might be referenced by other routes.
+ if err != cache.ErrStillInUse {
+ merr = multierror.Append(merr, err)
+ } else {
+ log.Infow("upstream was referenced by
other routes",
+ zap.String("upstream_id", u.ID),
+ zap.String("upstream_name",
u.Name),
+ )
+ }
+ }
+ }
}
if added != nil {
// Should create upstreams firstly due to the dependencies.
diff --git a/test/e2e/ingress/resourcepushing.go
b/test/e2e/ingress/resourcepushing.go
index 99e201c..2c459fc 100644
--- a/test/e2e/ingress/resourcepushing.go
+++ b/test/e2e/ingress/resourcepushing.go
@@ -389,4 +389,106 @@ spec:
resp.Status(http.StatusNotFound)
resp.Body().Contains("404 Route Not Found")
})
+
+ ginkgo.It("service is referenced by two ApisixRoutes", func() {
+ backendSvc, backendSvcPort := s.DefaultHTTPBackend()
+ ar1 := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2alpha1
+kind: ApisixRoute
+metadata:
+ name: httpbin-route-1
+spec:
+ http:
+ - name: rule1
+ priority: 1
+ match:
+ hosts:
+ - httpbin.com
+ paths:
+ - /ip
+ backend:
+ serviceName: %s
+ servicePort: %d
+`, backendSvc, backendSvcPort[0])
+ ar2 := fmt.Sprintf(`
+apiVersion: apisix.apache.org/v2alpha1
+kind: ApisixRoute
+metadata:
+ name: httpbin-route-2
+spec:
+ http:
+ - name: rule1
+ priority: 1
+ match:
+ hosts:
+ - httpbin.com
+ paths:
+ - /status/200
+ backend:
+ serviceName: %s
+ servicePort: %d
+`, backendSvc, backendSvcPort[0])
+
+ err := s.CreateResourceFromString(ar1)
+ assert.Nil(ginkgo.GinkgoT(), err)
+ err = s.CreateResourceFromString(ar2)
+ assert.Nil(ginkgo.GinkgoT(), err)
+
+ time.Sleep(3 * time.Second)
+
+ routes, err := s.ListApisixRoutes()
+ assert.Nil(ginkgo.GinkgoT(), err, "listing routes")
+ assert.Len(ginkgo.GinkgoT(), routes, 2)
+
+ ups, err := s.ListApisixUpstreams()
+ assert.Nil(ginkgo.GinkgoT(), err, "listing upstreams")
+ assert.Len(ginkgo.GinkgoT(), ups, 1)
+ assert.Equal(ginkgo.GinkgoT(), ups[0].ID, routes[0].UpstreamId)
+ assert.Equal(ginkgo.GinkgoT(), ups[0].ID, routes[1].UpstreamId)
+
+ resp := s.NewAPISIXClient().GET("/ip").WithHeader("Host",
"httpbin.com").Expect()
+ resp.Status(http.StatusOK)
+ resp.Body().Contains("origin")
+
+ resp =
s.NewAPISIXClient().GET("/status/200").WithHeader("Host",
"httpbin.com").Expect()
+ resp.Status(http.StatusOK)
+
+ // Delete ar1
+ err = s.RemoveResourceByString(ar1)
+ assert.Nil(ginkgo.GinkgoT(), err)
+ time.Sleep(3 * time.Second)
+
+ routes, err = s.ListApisixRoutes()
+ assert.Nil(ginkgo.GinkgoT(), err, "listing routes")
+ assert.Len(ginkgo.GinkgoT(), routes, 1)
+ name := s.Namespace() + "_" + "httpbin-route-2" + "_" + "rule1"
+ assert.Equal(ginkgo.GinkgoT(), routes[0].Name, name)
+
+ // As httpbin service is referenced by ar2, the corresponding
upstream still exists.
+ ups, err = s.ListApisixUpstreams()
+ assert.Nil(ginkgo.GinkgoT(), err, "listing upstreams")
+ assert.Len(ginkgo.GinkgoT(), ups, 1)
+ assert.Equal(ginkgo.GinkgoT(), ups[0].ID, routes[0].UpstreamId)
+
+ resp = s.NewAPISIXClient().GET("/ip").WithHeader("Host",
"httpbin.com").Expect()
+ resp.Status(http.StatusNotFound)
+ resp =
s.NewAPISIXClient().GET("/status/200").WithHeader("Host",
"httpbin.com").Expect()
+ resp.Status(http.StatusOK)
+
+ // Delete ar2
+ err = s.RemoveResourceByString(ar2)
+ assert.Nil(ginkgo.GinkgoT(), err)
+ time.Sleep(3 * time.Second)
+
+ routes, err = s.ListApisixRoutes()
+ assert.Nil(ginkgo.GinkgoT(), err, "listing routes")
+ assert.Len(ginkgo.GinkgoT(), routes, 0)
+
+ ups, err = s.ListApisixUpstreams()
+ assert.Nil(ginkgo.GinkgoT(), err, "listing upstreams")
+ assert.Len(ginkgo.GinkgoT(), ups, 0)
+
+ resp =
s.NewAPISIXClient().GET("/status/200").WithHeader("Host",
"httpbin.com").Expect()
+ resp.Status(http.StatusNotFound)
+ })
})
