Skip to content

Commit 907a695

Browse files
committed
avoid weight adjustment on managed routes owned header rule
1 parent 0356d30 commit 907a695

File tree

6 files changed

+91
-2
lines changed

6 files changed

+91
-2
lines changed

pkg/plugin/httproute.go

+51-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/argoproj-labs/rollouts-plugin-trafficrouter-gatewayapi/internal/utils"
1010
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
1111
pluginTypes "github.com/argoproj/argo-rollouts/utils/plugin/types"
12+
"github.com/sirupsen/logrus"
1213
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1314
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
1415
)
@@ -19,10 +20,12 @@ const (
1920

2021
func (r *RpcPlugin) setHTTPRouteWeight(rollout *v1alpha1.Rollout, desiredWeight int32, gatewayAPIConfig *GatewayAPITrafficRouting) pluginTypes.RpcError {
2122
ctx := context.TODO()
23+
clientset := r.TestClientset
2224
httpRouteClient := r.HTTPRouteClient
2325
if !r.IsTest {
2426
gatewayClientV1 := r.GatewayAPIClientset.GatewayV1()
2527
httpRouteClient = gatewayClientV1.HTTPRoutes(gatewayAPIConfig.Namespace)
28+
clientset = r.Clientset.CoreV1().ConfigMaps(gatewayAPIConfig.Namespace)
2629
}
2730
httpRoute, err := httpRouteClient.Get(ctx, gatewayAPIConfig.HTTPRoute, metav1.GetOptions{})
2831
if err != nil {
@@ -32,16 +35,63 @@ func (r *RpcPlugin) setHTTPRouteWeight(rollout *v1alpha1.Rollout, desiredWeight
3235
}
3336
canaryServiceName := rollout.Spec.Strategy.Canary.CanaryService
3437
stableServiceName := rollout.Spec.Strategy.Canary.StableService
38+
39+
// Retrieve the managed routes from the configmap to determine which rules were added via setHTTPHeaderRoute
40+
managedRouteMap := make(ManagedRouteMap)
41+
configMap, err := utils.GetOrCreateConfigMap(gatewayAPIConfig.ConfigMap, utils.CreateConfigMapOptions{
42+
Clientset: clientset,
43+
Ctx: ctx,
44+
})
45+
if err != nil {
46+
return pluginTypes.RpcError{
47+
ErrorString: err.Error(),
48+
}
49+
}
50+
err = utils.GetConfigMapData(configMap, HTTPConfigMapKey, &managedRouteMap)
51+
if err != nil {
52+
return pluginTypes.RpcError{
53+
ErrorString: err.Error(),
54+
}
55+
}
56+
managedRuleIndices := make(map[int]bool)
57+
for _, managedRoute := range managedRouteMap {
58+
if idx, ok := managedRoute[httpRoute.Name]; ok {
59+
managedRuleIndices[idx] = true
60+
}
61+
}
62+
3563
routeRuleList := HTTPRouteRuleList(httpRoute.Spec.Rules)
36-
canaryBackendRefs, err := getBackendRefs(canaryServiceName, routeRuleList)
64+
indexedCanaryBackendRefs, err := getIndexedBackendRefs(canaryServiceName, routeRuleList)
3765
if err != nil {
3866
return pluginTypes.RpcError{
3967
ErrorString: err.Error(),
4068
}
4169
}
70+
canaryBackendRefs := make([]*HTTPBackendRef, 0)
71+
for _, indexedCanaryBackendRef := range indexedCanaryBackendRefs {
72+
// TODO - when setMirrorRoute is implemented, we would need to update the weight of the managed
73+
// canary backendRefs for mirror routes.
74+
// Ideally - these would be stored differently in the configmap from the managed header based routes
75+
// but that would mean a breaking change to the configmap structure
76+
if managedRuleIndices[indexedCanaryBackendRef.RuleIndex] {
77+
r.LogCtx.WithFields(logrus.Fields{
78+
"rule": httpRoute.Spec.Rules[indexedCanaryBackendRef.RuleIndex],
79+
"index": indexedCanaryBackendRef.RuleIndex,
80+
"managedRouteMap": managedRouteMap,
81+
}).Info("Skipping matched canary backendRef for weight adjustment since it is a part of a rule marked as a managed route")
82+
continue
83+
}
84+
canaryBackendRefs = append(canaryBackendRefs, indexedCanaryBackendRef.Refs...)
85+
}
86+
87+
// Update the weight of the canary backendRefs not owned by a rule marked as a managed route
4288
for _, ref := range canaryBackendRefs {
4389
ref.Weight = &desiredWeight
4490
}
91+
92+
// Noted above, but any managed routes that would have a stableBackendRef would be updated with weight here.
93+
// Since this is not yet possible (all managed routes will only have a single canary backendRef),
94+
// we can avoid checking for managed route rule indexes here.
4595
stableBackendRefs, err := getBackendRefs(stableServiceName, routeRuleList)
4696
if err != nil {
4797
return pluginTypes.RpcError{

pkg/plugin/plugin.go

+31
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,37 @@ func getBackendRefs[T1 GatewayAPIBackendRef, T2 GatewayAPIRouteRule[T1], T3 Gate
280280
return nil, routeRuleList.Error()
281281
}
282282

283+
// This will return the backendRefs slices that matched the backendRefName but split by the routeRule index to be used for updating the weight
284+
// under consideration of the managedRoute rules present from the configmap (identifiable by the index of the rule on an HttpRoute)
285+
func getIndexedBackendRefs[T1 GatewayAPIBackendRef, T2 GatewayAPIRouteRule[T1], T3 GatewayAPIRouteRuleList[T1, T2]](backendRefName string, routeRuleList T3) ([]IndexedBackendRefs[T1], error) {
286+
var results []IndexedBackendRefs[T1]
287+
var backendRef T1
288+
var routeRule T2
289+
ruleIndex := 0
290+
for next, hasNext := routeRuleList.Iterator(); hasNext; {
291+
// Has to be inside of the for loop since it will be reset to 0 for each routeRule
292+
var matchedRefs []T1
293+
routeRule, hasNext = next()
294+
for nextRef, hasNextRef := routeRule.Iterator(); hasNextRef; {
295+
backendRef, hasNextRef = nextRef()
296+
if backendRefName == backendRef.GetName() {
297+
matchedRefs = append(matchedRefs, backendRef)
298+
}
299+
}
300+
if len(matchedRefs) > 0 {
301+
results = append(results, IndexedBackendRefs[T1]{
302+
RuleIndex: ruleIndex,
303+
Refs: matchedRefs,
304+
})
305+
}
306+
ruleIndex++
307+
}
308+
if len(results) > 0 {
309+
return results, nil
310+
}
311+
return nil, routeRuleList.Error()
312+
}
313+
283314
func isConfigHasRoutes(config *GatewayAPITrafficRouting) bool {
284315
return len(config.HTTPRoutes) > 0 || len(config.TCPRoutes) > 0 || len(config.GRPCRoutes) > 0
285316
}

pkg/plugin/plugin_test.go

+2
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ func TestRunSuccessfully(t *testing.T) {
106106
rollout := newRollout(mocks.StableServiceName, mocks.CanaryServiceName, &GatewayAPITrafficRouting{
107107
Namespace: mocks.RolloutNamespace,
108108
HTTPRoute: mocks.HTTPRouteName,
109+
ConfigMap: mocks.ConfigMapName,
109110
})
110111
err := pluginInstance.SetWeight(rollout, desiredWeight, []v1alpha1.WeightDestination{})
111112

@@ -155,6 +156,7 @@ func TestRunSuccessfully(t *testing.T) {
155156
UseHeaderRoutes: true,
156157
},
157158
},
159+
ConfigMap: mocks.ConfigMapName,
158160
})
159161
err := pluginInstance.SetWeight(rollout, desiredWeight, []v1alpha1.WeightDestination{})
160162

pkg/plugin/types.go

+5
Original file line numberDiff line numberDiff line change
@@ -129,3 +129,8 @@ type GatewayAPIBackendRef interface {
129129
type GatewayAPIRouteRuleListIterator[T1 GatewayAPIBackendRef, T2 GatewayAPIRouteRule[T1]] func() (T2, bool)
130130

131131
type GatewayAPIRouteRuleIterator[T1 GatewayAPIBackendRef] func() (T1, bool)
132+
133+
type IndexedBackendRefs[T GatewayAPIBackendRef] struct {
134+
RuleIndex int
135+
Refs []T
136+
}

test/e2e/constants.go

+1
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const (
3333

3434
FIRST_CANARY_ROUTE_WEIGHT = 0
3535
LAST_CANARY_ROUTE_WEIGHT = 30
36+
DEFAULT_ROUTE_WEIGHT = 1 // HTTPRoute rules that are managed by the rollout should never update their weight to the setWeight value. It should stay as the default 1
3637

3738
RESOURCES_MAP_KEY contextKey = "resourcesMap"
3839

test/e2e/single_header_based_httproute_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ func getMatchHeaderBasedHTTPRouteFetcher(t *testing.T, targetWeight int32, targe
272272
}
273273
headerBasedRouteValue := rules[HEADER_BASED_RULE_INDEX].Matches[HEADER_BASED_MATCH_INDEX].Headers[HEADER_BASED_HEADER_INDEX]
274274
weight := *rules[HEADER_BASED_RULE_INDEX].BackendRefs[HEADER_BASED_BACKEND_REF_INDEX].Weight
275-
return weight == targetWeight && isHeaderBasedHTTPRouteValuesEqual(headerBasedRouteValue, targetHeaderBasedRouteValue)
275+
return weight == DEFAULT_ROUTE_WEIGHT && isHeaderBasedHTTPRouteValuesEqual(headerBasedRouteValue, targetHeaderBasedRouteValue)
276276
}
277277
}
278278

0 commit comments

Comments
 (0)