bzp2010 commented on code in PR #2540:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2540#discussion_r2318400577


##########
internal/adc/translator/httproute.go:
##########
@@ -466,32 +467,89 @@ func (t *Translator) TranslateHTTPRoute(tctx 
*provider.TranslateContext, httpRou
        labels := label.GenLabel(httpRoute)
 
        for ruleIndex, rule := range rules {
-               upstream := adctypes.NewDefaultUpstream()
-               var backendErr error
+               service := adctypes.NewDefaultService()
+               service.Labels = labels
+
+               service.Name = 
adctypes.ComposeServiceNameWithRule(httpRoute.Namespace, httpRoute.Name, 
fmt.Sprintf("%d", ruleIndex))
+               service.ID = id.GenID(service.Name)
+               service.Hosts = hosts
+
+               var (
+                       upstreams         = make([]*adctypes.Upstream, 0)
+                       weightedUpstreams = 
make([]adctypes.TrafficSplitConfigRuleWeightedUpstream, 0)
+                       backendErr        error
+               )
+
                for _, backend := range rule.BackendRefs {
                        if backend.Namespace == nil {
                                namespace := 
gatewayv1.Namespace(httpRoute.Namespace)
                                backend.Namespace = &namespace
                        }
+                       upstream := adctypes.NewDefaultUpstream()
                        upNodes, err := t.translateBackendRef(tctx, 
backend.BackendRef, DefaultEndpointFilter)
                        if err != nil {
                                backendErr = err
                                continue
                        }
+                       if len(upNodes) == 0 {
+                               continue
+                       }
+
                        
t.AttachBackendTrafficPolicyToUpstream(backend.BackendRef, 
tctx.BackendTrafficPolicies, upstream)
-                       upstream.Nodes = append(upstream.Nodes, upNodes...)
+                       upstream.Nodes = upNodes
+                       upstreams = append(upstreams, upstream)
                }
 
-               // todo: support multiple backends
-               service := adctypes.NewDefaultService()
-               service.Labels = labels
+               // Handle multiple backends with traffic-split plugin
+               if len(upstreams) == 0 {
+                       // Create a default upstream if no valid backends
+                       upstream := adctypes.NewDefaultUpstream()
+                       service.Upstream = upstream
+               } else if len(upstreams) == 1 {
+                       // Single backend - use directly as service upstream
+                       service.Upstream = upstreams[0]
+               } else {
+                       // Multiple backends - use traffic-split plugin
+                       service.Upstream = upstreams[0]
+                       upstreams = upstreams[1:]
+
+                       // Set weight in traffic-split for the default upstream
+                       weight := apiv2.DefaultWeight
+                       if rule.BackendRefs[0].Weight != nil {
+                               weight = int(*rule.BackendRefs[0].Weight)
+                       }
+                       weightedUpstreams = append(weightedUpstreams, 
adctypes.TrafficSplitConfigRuleWeightedUpstream{
+                               Weight: weight,
+                       })

Review Comment:
   The problem is the same as the previous one: it's best to avoid using inline 
upstreams. Otherwise, any change to the endpoints will trigger a complete 
rebuild of the routing tree.
   
   Instead, ADC provides full capabilities to define separate upstream 
resources and assign IDs. You can then reference those IDs in traffic-split. 
This functionality is available out of the box for every ADC backend.
   
   ```yaml
   services:
     - name: demo
       upstream:
         nodes:
           - host: inlined upstream
             port: 443
             weight: 5
       upstreams:
          - name: upstream2
            bala: bala
            # id: 293fcc9825f66b97b257289235c6f0334befe769 # by default 
sha1(name)
          - id: custom-id-for-upstream3 # It is recommended that the ingress 
controller generate this ID using built-in rules.
            name: upstream3
            bala: bala
       plugins:
         traffic-split:
           rules:
             - weight: 5
             - upstream_id: 293fcc9825f66b97b257289235c6f0334befe769
               weight: 10
             - upstream_id: custom-id-for-upstream3
               weight: 5
   ```
   
   Changes to each individual upstream are independent and do not cause changes 
to the service or route. This avoids unnecessary rebuilds of the routing tree 
and meaningless changes to the conf version.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to