Skip to content

Commit

Permalink
fix(trafficrouting): patch VirtualService when there is only one name…
Browse files Browse the repository at this point in the history
…d route

Signed-off-by: Jean Morais <[email protected]>
  • Loading branch information
jeanmorais authored and zachaller committed Feb 21, 2025
1 parent 5e6936e commit e5d9e21
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
7 changes: 6 additions & 1 deletion rollout/trafficrouting/istio/istio.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,8 +904,13 @@ func (r *Reconciler) VerifyWeight(desiredWeight int32, additionalDestinations ..
// getHttpRouteIndexesToPatch returns array indices of the httpRoutes which need to be patched when updating weights
func getHttpRouteIndexesToPatch(routeNames []string, httpRoutes []VirtualServiceHTTPRoute) ([]int, error) {
//We have no routes listed in spec.strategy.canary.trafficRouting.istio.virtualService.routes so find index
//of the first empty named route
//of the first empty named route.
// If there is only one HTTPRoute defined in the VirtualService, then we can patch it without a name.
if len(routeNames) == 0 {
if len(httpRoutes) == 1 {
return []int{0}, nil
}

for i, route := range httpRoutes {
if route.Name == "" {
return []int{i}, nil
Expand Down
42 changes: 42 additions & 0 deletions rollout/trafficrouting/istio/istio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3156,3 +3156,45 @@ spec:
actions := client.Actions()
assert.Len(t, actions, 0)
}

func TestGetHttpRouteIndexesToPatch(t *testing.T) {

// Test case when the rollout has no managed routes defined
httpRoutes := []VirtualServiceHTTPRoute{
{Name: "foo", Match: nil},
{Name: "", Match: nil},
}

indexes, err := getHttpRouteIndexesToPatch([]string{}, httpRoutes)
assert.NoError(t, err)
assert.Equal(t, []int{1}, indexes)

// Test case when the rollout has managed routes defined
httpRoutes = []VirtualServiceHTTPRoute{
{Name: "foo", Match: nil},
{Name: "bar", Match: nil},
}

indexes, err = getHttpRouteIndexesToPatch([]string{"foo", "bar"}, httpRoutes)
assert.NoError(t, err)
assert.Equal(t, []int{0, 1}, indexes)

// Test case when the rollout has only one managed route defined
httpRoutes = []VirtualServiceHTTPRoute{
{Name: "foo", Match: nil},
}

indexes, err = getHttpRouteIndexesToPatch([]string{}, httpRoutes)
assert.NoError(t, err)
assert.Equal(t, []int{0}, indexes)

// Test case when http route is not found
httpRoutes = []VirtualServiceHTTPRoute{
{Name: "foo", Match: nil},
}

indexes, err = getHttpRouteIndexesToPatch([]string{"bar"}, httpRoutes)
assert.Equal(t, "HTTP Route 'bar' is not found in the defined Virtual Service.", err.Error())
assert.Nil(t, indexes)

}

0 comments on commit e5d9e21

Please sign in to comment.