Skip to content

Commit

Permalink
fix(targets) deduplicate nil-weight targets (#5946)
Browse files Browse the repository at this point in the history
Assume the Kong default 100 weight for targets that have nil weight when
being deduplicated.
  • Loading branch information
rainest authored Apr 29, 2024
1 parent b014561 commit b8f7eb5
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
12 changes: 11 additions & 1 deletion internal/dataplane/translator/translate_upstreams.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,16 @@ func getEndpoints(
return upstreamServers
}

// targetWeightOrDefault returns the effective value of a target weight pointer. If the pointer is non-nil, it returns
// the pointee. If the pointer is nil, it returns 100, the default Kong target weight. This allows us to sum
// deduplicated targets' weights if one happens to be unset in the controller.
func targetWeightOrDefault(in *int) int {
if in != nil {
return *in
}
return 100
}

func updateTargetMap(targetMap map[string]kongstate.Target, t kongstate.Target) map[string]kongstate.Target {
// See https://github.com/Kong/kubernetes-ingress-controller/issues/5761:
// Duplicate targets will appear in configurations that use Services with the same selector, which are used
Expand All @@ -341,7 +351,7 @@ func updateTargetMap(targetMap map[string]kongstate.Target, t kongstate.Target)
// instead requires t.Target.Target. For consistency, everything below explicitly includes the nested object
// name, so t.Target.Weight instead of t.Weight.
if existing, ok := targetMap[*t.Target.Target]; ok {
sum := *existing.Target.Weight + *t.Target.Weight
sum := targetWeightOrDefault(existing.Target.Weight) + targetWeightOrDefault(t.Target.Weight)
existing.Target.Weight = &sum
targetMap[*t.Target.Target] = existing
} else {
Expand Down
5 changes: 4 additions & 1 deletion test/integration/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func TestIngressClassNameSpec(t *testing.T) {
helpers.EventuallyExpectHTTP404WithNoRoute(t, proxyHTTPURL, proxyHTTPURL.Host, "/test_ingressclassname_spec", ingressWait, waitTick, nil)
}

func TestIngressNamespaces(t *testing.T) {
func TestIngressServiceUpstream(t *testing.T) {
t.Parallel()

ctx := context.Background()
Expand All @@ -308,12 +308,15 @@ func TestIngressNamespaces(t *testing.T) {
t.Log("deploying a minimal HTTP container deployment to test Ingress routes")
container := generators.NewContainer("httpbin", test.HTTPBinImage, test.HTTPBinPort)
deployment := generators.NewDeploymentForContainer(container)
deployment.Spec.Replicas = lo.ToPtr(int32(3))
deployment, err := env.Cluster().Client().AppsV1().Deployments(ns.Name).Create(ctx, deployment, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(deployment)

t.Logf("exposing deployment %s via service", deployment.Name)
service := generators.NewServiceForDeployment(deployment, corev1.ServiceTypeLoadBalancer)
service.ObjectMeta.Annotations = map[string]string{}
service.ObjectMeta.Annotations["ingress.kubernetes.io/service-upstream"] = "true"
service, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service, metav1.CreateOptions{})
require.NoError(t, err)
cleaner.Add(service)
Expand Down

0 comments on commit b8f7eb5

Please sign in to comment.