Skip to content

Commit b8f7eb5

Browse files
authored
fix(targets) deduplicate nil-weight targets (#5946)
Assume the Kong default 100 weight for targets that have nil weight when being deduplicated.
1 parent b014561 commit b8f7eb5

File tree

2 files changed

+15
-2
lines changed

2 files changed

+15
-2
lines changed

internal/dataplane/translator/translate_upstreams.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,16 @@ func getEndpoints(
331331
return upstreamServers
332332
}
333333

334+
// targetWeightOrDefault returns the effective value of a target weight pointer. If the pointer is non-nil, it returns
335+
// the pointee. If the pointer is nil, it returns 100, the default Kong target weight. This allows us to sum
336+
// deduplicated targets' weights if one happens to be unset in the controller.
337+
func targetWeightOrDefault(in *int) int {
338+
if in != nil {
339+
return *in
340+
}
341+
return 100
342+
}
343+
334344
func updateTargetMap(targetMap map[string]kongstate.Target, t kongstate.Target) map[string]kongstate.Target {
335345
// See https://github.com/Kong/kubernetes-ingress-controller/issues/5761:
336346
// Duplicate targets will appear in configurations that use Services with the same selector, which are used
@@ -341,7 +351,7 @@ func updateTargetMap(targetMap map[string]kongstate.Target, t kongstate.Target)
341351
// instead requires t.Target.Target. For consistency, everything below explicitly includes the nested object
342352
// name, so t.Target.Weight instead of t.Weight.
343353
if existing, ok := targetMap[*t.Target.Target]; ok {
344-
sum := *existing.Target.Weight + *t.Target.Weight
354+
sum := targetWeightOrDefault(existing.Target.Weight) + targetWeightOrDefault(t.Target.Weight)
345355
existing.Target.Weight = &sum
346356
targetMap[*t.Target.Target] = existing
347357
} else {

test/integration/ingress_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ func TestIngressClassNameSpec(t *testing.T) {
297297
helpers.EventuallyExpectHTTP404WithNoRoute(t, proxyHTTPURL, proxyHTTPURL.Host, "/test_ingressclassname_spec", ingressWait, waitTick, nil)
298298
}
299299

300-
func TestIngressNamespaces(t *testing.T) {
300+
func TestIngressServiceUpstream(t *testing.T) {
301301
t.Parallel()
302302

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

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

0 commit comments

Comments
 (0)