fix: modify upstream name generation method (#273)

Signed-off-by: Ashing Zheng <axingfly@gmail.com>

Author: ronething

api/adc/types.go

@@ -819,38 +819,23 @@ var (
 	}
 )
 
-// ComposeUpstreamName uses namespace, name, subset (optional), port, resolveGranularity info to compose
+// ComposeUpstreamName uses namespace, name, ruleIndex, backendIndex, serviceName info to compose
 // the upstream name.
 // the resolveGranularity is not composited in the upstream name when it is endpoint.
 // ref: https://github.com/apache/apisix-ingress-controller/blob/10059afe3e84b693cc61e6df7a0040890a9d16eb/pkg/types/apisix/v1/types.go#L595-L598
-func ComposeUpstreamName(namespace, name, subset string, port int32, resolveGranularity string) string {
-	pstr := strconv.Itoa(int(port))
-	// FIXME Use sync.Pool to reuse this buffer if the upstream
-	// name composing code path is hot.
+func ComposeUpstreamName(namespace, name, ruleIndex, backendIndex string) string {
 	var p []byte
-	plen := len(namespace) + len(name) + len(pstr) + 2
-	if subset != "" {
-		plen = plen + len(subset) + 1
-	}
-	if resolveGranularity == ResolveGranularity.Service {
-		plen = plen + len(resolveGranularity) + 1
-	}
+	plen := len(namespace) + len(name) + len(ruleIndex) + len(backendIndex) + 3
 
 	p = make([]byte, 0, plen)
 	buf := bytes.NewBuffer(p)
 	buf.WriteString(namespace)
 	buf.WriteByte('_')
 	buf.WriteString(name)
 	buf.WriteByte('_')
-	if subset != "" {
-		buf.WriteString(subset)
-		buf.WriteByte('_')
-	}
-	buf.WriteString(pstr)
-	if resolveGranularity == ResolveGranularity.Service {
-		buf.WriteByte('_')
-		buf.WriteString(resolveGranularity)
-	}
+	buf.WriteString(ruleIndex)
+	buf.WriteByte('_')
+	buf.WriteString(backendIndex)
 
 	return buf.String()
 }

internal/adc/translator/apisixroute.go

@@ -73,7 +73,7 @@ func (t *Translator) translateHTTPRule(tctx *provider.TranslateContext, ar *apiv
 
 	service := t.buildService(ar, rule, ruleIndex)
 	t.buildRoute(ar, service, rule, plugins, timeout, vars)
-	t.buildUpstream(tctx, service, ar, rule)
+	t.buildUpstream(tctx, service, ar, rule, ruleIndex)
 
 	return service, nil
 }
@@ -203,13 +203,13 @@ func (t *Translator) buildRoute(ar *apiv2.ApisixRoute, service *adc.Service, rul
 	service.Routes = []*adc.Route{route}
 }
 
-func (t *Translator) buildUpstream(tctx *provider.TranslateContext, service *adc.Service, ar *apiv2.ApisixRoute, rule apiv2.ApisixRouteHTTP) {
+func (t *Translator) buildUpstream(tctx *provider.TranslateContext, service *adc.Service, ar *apiv2.ApisixRoute, rule apiv2.ApisixRouteHTTP, ruleIndex int) {
 	var (
 		upstreams         = make([]*adc.Upstream, 0)
 		weightedUpstreams = make([]adc.TrafficSplitConfigRuleWeightedUpstream, 0)
 	)
 
-	for _, backend := range rule.Backends {
+	for backendIndex, backend := range rule.Backends {
 		var backendErr error
 		upstream := adc.NewDefaultUpstream()
 		// try to get the apisixupstream with the same name as the backend service to be upstream config.
@@ -236,7 +236,7 @@ func (t *Translator) buildUpstream(tctx *provider.TranslateContext, service *adc
 			upstream.Labels["meta_weight"] = strconv.FormatInt(int64(*backend.Weight), 10)
 		}
 
-		upstreamName := adc.ComposeUpstreamName(ar.Namespace, backend.ServiceName, backend.Subset, int32(backend.ServicePort.IntValue()), backend.ResolveGranularity)
+		upstreamName := adc.ComposeUpstreamName(ar.Namespace, ar.Name, fmt.Sprintf("%d", ruleIndex), fmt.Sprintf("%d", backendIndex))
 		upstream.Name = upstreamName
 		upstream.ID = id.GenID(upstreamName)
 		upstreams = append(upstreams, upstream)

test/e2e/crds/v2/route.go

@@ -2128,4 +2128,75 @@ spec:
 			})
 		})
 	})
+
+	Context("Test ApisixRoute with multiple backends", func() {
+		It("create ApisixRoute with multiple backends", func() {
+			var httpService = `
+apiVersion: v1
+kind: Service
+metadata:
+  name: httpbin-service-e2e-test2
+spec:
+  selector:
+    app: httpbin-deployment-e2e-test
+  ports:
+    - name: http
+      port: 80
+      protocol: TCP
+      targetPort: 80
+  type: ClusterIP
+`
+			err := s.CreateResourceFromString(httpService)
+			Expect(err).ShouldNot(HaveOccurred())
+			s.EnsureNumEndpointsReady(GinkgoT(), "httpbin-service-e2e-test2", 1)
+
+			const apisixRouteSpec = `
+apiVersion: apisix.apache.org/v2
+kind: ApisixRoute
+metadata:
+  name: httpbin
+spec:
+  ingressClassName: %s
+  http:
+    - name: get
+      match:
+        paths:
+          - /get
+      backends:
+        - serviceName: httpbin-service-e2e-test
+          servicePort: 80
+        - serviceName: httpbin-service-e2e-test2
+          servicePort: 80
+    - name: ip
+      match:
+        paths:
+          - /ip
+      backends:
+        - serviceName: httpbin-service-e2e-test
+          servicePort: 80
+        - serviceName: httpbin-service-e2e-test2
+          servicePort: 80
+`
+			By("apply ApisixRoute")
+			applier.MustApplyAPIv2(types.NamespacedName{Namespace: s.Namespace(), Name: "httpbin"},
+				&apiv2.ApisixRoute{}, fmt.Sprintf(apisixRouteSpec, s.Namespace()))
+
+			By("check upstreams")
+			upstreams, err := s.DefaultDataplaneResource().Upstream().List(context.Background())
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(upstreams).Should(HaveLen(4))
+
+			By("verify ApisixRoute works")
+			s.RequestAssert(&scaffold.RequestAssert{
+				Method: "GET",
+				Path:   "/get",
+				Check:  scaffold.WithExpectedStatus(http.StatusOK),
+			})
+			s.RequestAssert(&scaffold.RequestAssert{
+				Method: "GET",
+				Path:   "/ip",
+				Check:  scaffold.WithExpectedStatus(http.StatusOK),
+			})
+		})
+	})
 })

test/e2e/scaffold/adc.go

@@ -56,6 +56,7 @@ func init() {
 // DataplaneResource defines the interface for accessing dataplane resources
 type DataplaneResource interface {
 	Route() RouteResource
+	Upstream() UpstreamResource
 	Service() ServiceResource
 	SSL() SSLResource
 	Consumer() ConsumerResource
@@ -76,6 +77,11 @@ type SSLResource interface {
 	List(ctx context.Context) ([]*adctypes.SSL, error)
 }
 
+// UpstreamResource defines the interface for upstream resources
+type UpstreamResource interface {
+	List(ctx context.Context) ([]*adctypes.Upstream, error)
+}
+
 // ConsumerResource defines the interface for consumer resources
 type ConsumerResource interface {
 	List(ctx context.Context) ([]*adctypes.Consumer, error)
@@ -117,8 +123,11 @@ func (a *adcDataplaneResource) Consumer() ConsumerResource {
 	return &adcConsumerResource{a}
 }
 
+func (a *adcDataplaneResource) Upstream() UpstreamResource {
+	return &adcUpstreamResource{a}
+}
+
 func init() {
-	// dashboard sdk log
 	l, err := log.NewLogger(
 		log.WithOutputFile("stderr"),
 		log.WithLogLevel("debug"),
@@ -192,14 +201,7 @@ func (a *adcDataplaneResource) dumpResources(ctx context.Context) (*translator.T
 		return nil, err
 	}
 
-	// Extract routes from services
-	var routes []*adctypes.Route
-	for _, service := range resources.Services {
-		routes = append(routes, service.Routes...)
-	}
-
 	result := &translator.TranslateResult{
-		Routes:         routes,
 		Services:       resources.Services,
 		SSL:            resources.SSLs,
 		GlobalRules:    resources.GlobalRules,
@@ -220,7 +222,11 @@ func (r *adcRouteResource) List(ctx context.Context) ([]*adctypes.Route, error)
 	if err != nil {
 		return nil, err
 	}
-	return result.Routes, nil
+	var routes []*adctypes.Route
+	for _, service := range result.Services {
+		routes = append(routes, service.Routes...)
+	}
+	return routes, nil
 }
 
 // adcServiceResource implements ServiceResource
@@ -261,3 +267,25 @@ func (c *adcConsumerResource) List(ctx context.Context) ([]*adctypes.Consumer, e
 	}
 	return result.Consumers, nil
 }
+
+type adcUpstreamResource struct {
+	*adcDataplaneResource
+}
+
+func (r *adcUpstreamResource) List(ctx context.Context) ([]*adctypes.Upstream, error) {
+	result, err := r.dumpResources(ctx)
+	if err != nil {
+		return nil, err
+	}
+	upstreams := make([]*adctypes.Upstream, 0, len(result.Services))
+	for _, svc := range result.Services {
+		if svc.Upstream != nil {
+			upstreams = append(upstreams, svc.Upstream)
+		}
+		if svc.Upstreams != nil {
+			upstreams = append(upstreams, svc.Upstreams...)
+		}
+	}
+
+	return upstreams, nil
+}