From ef50dc921f567bff8b42f2a660a29d70d3018c51 Mon Sep 17 00:00:00 2001 From: Levalicious Date: Tue, 13 Oct 2026 13:10:49 -0400 Subject: [PATCH] lb: DNAT rules carry their protocol; end-to-end test against a cluster iptables refuses a DNAT to ip:port unless the same rule names the protocol, so every per-port chain's DNAT now has -p. Live on the SG cluster the first sync (no endpoints yet) applied; every later one failed with "Need TCP, UDP, SCTP or DCCP with port specification" and left the chains empty. The unit test now rejects any DNAT line without its protocol; the rendered nat and mangle payloads were also checked with the nodes' own iptables-restore --test (nf_tables). reweave/test/lb/run.sh is the end-to-end test: from outside, both a pod on the holder and one on another node see the client's own address; status is [{ip, ipMode: Proxy}] and kube-proxy has nothing for the address; a pod on a third node reaches it; an unheld address clears the status and stops the forward; deleting the Services removes the holder's chains. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UHXzGrsxfkGk8LmwrBbHNZ --- lb/datapath.go | 7 +-- lb/model_test.go | 12 ++++-- reweave/test/lb/lb-test.yaml | 75 ++++++++++++++++++++++++++++++++ reweave/test/lb/run.sh | 83 ++++++++++++++++++++++++++++++++++++ 4 files changed, 171 insertions(+), 6 deletions(-) create mode 100644 reweave/test/lb/lb-test.yaml create mode 100755 reweave/test/lb/run.sh diff --git a/lb/datapath.go b/lb/datapath.go index 3fb06b2a..35a93b2a 100644 --- a/lb/datapath.go +++ b/lb/datapath.go @@ -62,12 +62,13 @@ func Render(d Desired, podNet *net.IPNet, stale []string) Rules { ForwardChain, f.Address, proto, proto, p.Port, f.Namespace+"/"+f.Name, c) n := len(p.Endpoints) for i, e := range p.Endpoints { + // DNAT to an ip:port needs the protocol in the same rule if i < n-1 { // the i-th endpoint takes 1/(n-i) of what the earlier ones left: uniform overall - fmt.Fprintf(&body, "-A %s -m statistic --mode random --probability %.10f -j DNAT --to-destination %s\n", - c, 1/float64(n-i), e) + fmt.Fprintf(&body, "-A %s -p %s -m statistic --mode random --probability %.10f -j DNAT --to-destination %s\n", + c, proto, 1/float64(n-i), e) } else { - fmt.Fprintf(&body, "-A %s -j DNAT --to-destination %s\n", c, e) + fmt.Fprintf(&body, "-A %s -p %s -j DNAT --to-destination %s\n", c, proto, e) } } } diff --git a/lb/model_test.go b/lb/model_test.go index 4cc965d1..56d37779 100644 --- a/lb/model_test.go +++ b/lb/model_test.go @@ -209,15 +209,21 @@ func TestRenderedRules(t *testing.T) { "*nat\n:REWEAVE-LB - [0:0]\n", ":REWEAVE-LBS-DEADBEEF0000 - [0:0]\n", // stale chain emptied "-A REWEAVE-LB -d 86.107.168.15/32 -p tcp -m tcp --dport 22 -m comment --comment \"gitea/gitea\" -j REWEAVE-LBS-", - "-m statistic --mode random --probability 0.3333333333 -j DNAT --to-destination 10.40.0.7:2222\n", - "-m statistic --mode random --probability 0.5000000000 -j DNAT --to-destination 10.44.0.3:2222\n", - "-j DNAT --to-destination 10.44.0.4:2222\n", + "-p tcp -m statistic --mode random --probability 0.3333333333 -j DNAT --to-destination 10.40.0.7:2222\n", + "-p tcp -m statistic --mode random --probability 0.5000000000 -j DNAT --to-destination 10.44.0.3:2222\n", + "-p tcp -j DNAT --to-destination 10.44.0.4:2222\n", "COMMIT\n", } { if !strings.Contains(r.Nat, want) { t.Fatalf("nat lacks %q:\n%s", want, r.Nat) } } + for _, line := range strings.Split(r.Nat, "\n") { + // iptables refuses a DNAT to ip:port without the protocol in the same rule + if strings.Contains(line, "-j DNAT") && !strings.Contains(line, " -p tcp ") { + t.Fatalf("DNAT without its protocol: %q", line) + } + } if strings.Contains(r.Nat, "SNAT") || strings.Contains(r.Nat, "MASQUERADE") { t.Fatalf("the client's address must survive:\n%s", r.Nat) } diff --git a/reweave/test/lb/lb-test.yaml b/reweave/test/lb/lb-test.yaml new file mode 100644 index 00000000..3b5bc787 --- /dev/null +++ b/reweave/test/lb/lb-test.yaml @@ -0,0 +1,75 @@ +# End-to-end test of weave-npc's LoadBalancer agent (lb). Placeholders are +# filled by run.sh: ADDR (an address HOLDER holds), HOLDER (that node), +# REMOTE (another node: its pod's replies must be steered back to HOLDER), +# OTHER (a third node: a pod there connects to ADDR from inside the cluster). +apiVersion: v1 +kind: List +items: + - apiVersion: v1 + kind: Namespace + metadata: + name: lbtest + # /clientip answers with the address the connection came from + - apiVersion: v1 + kind: Pod + metadata: + name: echo-remote + namespace: lbtest + labels: {app: echo-remote} + spec: + nodeName: REMOTE + containers: + - name: echo + image: registry.k8s.io/e2e-test-images/agnhost:2.53 + args: ["netexec", "--http-port=8080"] + ports: [{name: http, containerPort: 8080}] + readinessProbe: {httpGet: {path: /healthz, port: 8080}} + - apiVersion: v1 + kind: Pod + metadata: + name: echo-local + namespace: lbtest + labels: {app: echo-local} + spec: + nodeName: HOLDER + containers: + - name: echo + image: registry.k8s.io/e2e-test-images/agnhost:2.53 + args: ["netexec", "--http-port=8080"] + ports: [{name: http, containerPort: 8080}] + readinessProbe: {httpGet: {path: /healthz, port: 8080}} + - apiVersion: v1 + kind: Pod + metadata: + name: client + namespace: lbtest + spec: + nodeName: OTHER + containers: + - name: client + image: registry.k8s.io/e2e-test-images/agnhost:2.53 + args: ["pause"] + - apiVersion: v1 + kind: Service + metadata: + name: echo-remote + namespace: lbtest + spec: + type: LoadBalancer + loadBalancerClass: reweave.levk.dev/node-address + loadBalancerIP: ADDR + allocateLoadBalancerNodePorts: false + selector: {app: echo-remote} + ports: [{name: http, port: 18080, targetPort: http}] + - apiVersion: v1 + kind: Service + metadata: + name: echo-local + namespace: lbtest + spec: + type: LoadBalancer + loadBalancerClass: reweave.levk.dev/node-address + loadBalancerIP: ADDR + allocateLoadBalancerNodePorts: false + selector: {app: echo-local} + ports: [{name: http, port: 18081, targetPort: http}] diff --git a/reweave/test/lb/run.sh b/reweave/test/lb/run.sh new file mode 100755 index 00000000..d9383947 --- /dev/null +++ b/reweave/test/lb/run.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# End-to-end test of weave-npc's LoadBalancer agent against a live cluster. +# +# CP="ssh ... root@" HOLDER_SSH="ssh ... root@" \ +# ADDR=
HOLDER= REMOTE= OTHER= \ +# reweave/test/lb/run.sh +# +# CP runs kubectl (admin.conf); HOLDER_SSH reads the holder's iptables. The +# machine running this script is the outside client. Asserts, in order: +# 1. each Service's status is exactly [{ip: ADDR, ipMode: Proxy}] +# 2. kube-proxy installed nothing for ADDR (ipMode Proxy leaves it to us) +# 3. from outside, both ports answer, and the pod sees the client's own +# address: from the holder's own pod, and from a pod on another node +# (whose replies must be steered back through the holder) +# 4. a pod on a third node reaches ADDR too +# 5. fail closed: naming an address nobody holds clears the status and +# stops the forward +# 6. deleting the Services removes the holder's chains +# Leaves nothing behind (namespace lbtest is deleted, also on failure). +set -euo pipefail + +: "${CP:?}" "${HOLDER_SSH:?}" "${ADDR:?}" "${HOLDER:?}" "${REMOTE:?}" "${OTHER:?}" +here=$(cd "$(dirname "$0")" && pwd) +k() { $CP "KUBECONFIG=/etc/kubernetes/admin.conf kubectl $*"; } +fail() { echo "FAIL: $*" >&2; exit 1; } +ok() { echo "ok: $*"; } +cleanup() { k delete namespace lbtest --ignore-not-found --wait=false >/dev/null 2>&1 || true; } +trap cleanup EXIT + +# eventually : retry once a second +eventually() { + local n=$1; shift + for _ in $(seq "$n"); do + if "$@" >/dev/null 2>&1; then return 0; fi + sleep 1 + done + return 1 +} + +sed -e "s/\bADDR\b/$ADDR/g; s/\bHOLDER\b/$HOLDER/g; s/\bREMOTE\b/$REMOTE/g; s/\bOTHER\b/$OTHER/g" \ + "$here/lb-test.yaml" | $CP "KUBECONFIG=/etc/kubernetes/admin.conf kubectl apply -f -" >/dev/null +k -n lbtest wait --for=condition=Ready pod --all --timeout=180s >/dev/null || fail "test pods not ready" + +status() { k -n lbtest get svc "$1" -o "jsonpath='{.status.loadBalancer.ingress}'"; } +for s in echo-remote echo-local; do + want="[{\"ip\":\"$ADDR\",\"ipMode\":\"Proxy\"}]" + eventually 30 test "$(status $s)" = "$want" || fail "$s status: $(status $s), want $want" +done +ok "1. status [{ip: $ADDR, ipMode: Proxy}] on both Services" + +if $HOLDER_SSH "iptables-save -t nat | grep -E '^-A KUBE-' | grep -F -- '-d $ADDR/32'"; then + fail "kube-proxy has rules for $ADDR" +fi +ok "2. kube-proxy installed nothing for $ADDR" + +# the client's own address, as the holder sees this machine's SSH session +me=$($HOLDER_SSH 'echo ${SSH_CLIENT%% *}') +for port in 18080 18081; do + got="" + for _ in $(seq 30); do + got=$(curl -s --max-time 3 "http://$ADDR:$port/clientip" || true) + [ -n "$got" ] && break + sleep 1 + done + [ "${got%:*}" = "$me" ] || fail "port $port: pod saw '${got}', want this client ($me)" +done +ok "3. from outside, both pods see this client's own address ($me), remote pod's replies steered back" + +k -n lbtest exec client -- /agnhost connect "$ADDR:18080" --timeout 5s >/dev/null || fail "pod on $OTHER cannot reach $ADDR:18080" +ok "4. a pod on $OTHER reaches $ADDR:18080" + +k -n lbtest patch svc echo-remote --type merge -p "'{\"spec\":{\"loadBalancerIP\":\"192.0.2.1\"}}'" >/dev/null +eventually 30 test "$(status echo-remote)" = "" || fail "status not cleared for an unheld address: $(status echo-remote)" +if curl -s --max-time 3 "http://$ADDR:18080/clientip" >/dev/null; then + fail "$ADDR:18080 still forwards after its Service moved to an unheld address" +fi +ok "5. unheld address: status cleared, forward gone (fail closed)" + +k -n lbtest delete svc echo-remote echo-local >/dev/null +eventually 30 test "$($HOLDER_SSH "iptables-save -t nat | grep -c '^:REWEAVE-LBS-'" || true)" = 0 \ + || fail "holder kept per-port chains after the Services went away" +ok "6. Services deleted: holder's chains removed" +echo PASS