[Backport release/3.1.x] deduplicate nil-weight targets (#5946) (#6025)

* fix(targets) deduplicate nil-weight targets (#5946)

Assume the Kong default 100 weight for targets that have nil weight when
being deduplicated.

(cherry picked from commit b8f7eb5)

* add changelog entry

---------

Co-authored-by: Travis Raines <[email protected]>

Author: randmonkey

CHANGELOG.md

@@ -93,7 +93,9 @@ Adding a new version? You'll need three changes:
 - Support to apply licenses to DB backed Kong gateway from `KongLicense`.
   [#5648](https://github.com/Kong/kubernetes-ingress-controller/pull/5648)
 - Redacted values no longer cause collisions in configuration reported to Konnect.
-  [5964](https://github.com/Kong/kubernetes-ingress-controller/pull/5964)
+  [#5964](https://github.com/Kong/kubernetes-ingress-controller/pull/5964)
+- Assign a default value for `weight` in Kong target if the `weight` is nil.
+  [#5946](https://github.com/Kong/kubernetes-ingress-controller/pull/5946)
 
 ## [3.1.4]
 

internal/dataplane/translator/translator.go

@@ -449,6 +449,16 @@ func (t *Translator) getUpstreams(serviceMap map[string]kongstate.Service) ([]ko
 	return upstreams, serviceMap
 }
 
+// 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
@@ -459,7 +469,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 {

test/integration/ingress_test.go

@@ -296,7 +296,7 @@ func TestIngressClassNameSpec(t *testing.T) {
 	helpers.EventuallyExpectHTTP404WithNoRoute(t, proxyURL, proxyURL.Host, "/test_ingressclassname_spec", ingressWait, waitTick, nil)
 }
 
-func TestIngressNamespaces(t *testing.T) {
+func TestIngressServiceUpstream(t *testing.T) {
 	t.Parallel()
 
 	ctx := context.Background()
@@ -307,12 +307,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)