diff --git a/go.mod b/go.mod index 45942a431..60043035e 100644 --- a/go.mod +++ b/go.mod @@ -21,8 +21,8 @@ require ( github.com/klauspost/cpuid/v2 v2.4.0 github.com/miekg/dns v1.1.72 github.com/mulgadc/northstar v1.13.0 - github.com/mulgadc/predastore v1.13.0 - github.com/mulgadc/viperblock v1.13.0 + github.com/mulgadc/predastore v1.13.1-0.20260722023246-c0a90d23b3a0 + github.com/mulgadc/viperblock v1.13.1-0.20260722023228-f8cfd7740bf4 github.com/nats-io/nats-server/v2 v2.14.3 github.com/nats-io/nats.go v1.52.0 github.com/opencontainers/runtime-spec v1.3.0 diff --git a/go.sum b/go.sum index 1011987b0..323bf035f 100644 --- a/go.sum +++ b/go.sum @@ -1321,8 +1321,12 @@ github.com/mulgadc/northstar v1.13.0 h1:L9PyYZ9Klg7dv2kE5+DHOcgPHelaZRDIvnwBsbHs github.com/mulgadc/northstar v1.13.0/go.mod h1:erQE/PzK9UMtgH8Y+uI5RdiboO/nqe1CUJbIpkae/FM= github.com/mulgadc/predastore v1.13.0 h1:jDPpLJP/GA4WK2t+9lksTSFDuQiV8vgDl6ISnPJRUuo= github.com/mulgadc/predastore v1.13.0/go.mod h1:38YSuk9zvYmbmgyM13WTkIQPJtvo2KmGJBYJul6NeAE= +github.com/mulgadc/predastore v1.13.1-0.20260722023246-c0a90d23b3a0 h1:29g+uPZ0LAANVA1IPnE/ogxJzJBJnBZvnTl51icp378= +github.com/mulgadc/predastore v1.13.1-0.20260722023246-c0a90d23b3a0/go.mod h1:38YSuk9zvYmbmgyM13WTkIQPJtvo2KmGJBYJul6NeAE= github.com/mulgadc/viperblock v1.13.0 h1:NuqmPRm26/j58GKbzcyshm0uZwdobzdjKK1qEsb2pCY= github.com/mulgadc/viperblock v1.13.0/go.mod h1:sW0hxZjN7WDQQZ39L5mE58fVLbSv3mBP3/ifFPdwJtU= +github.com/mulgadc/viperblock v1.13.1-0.20260722023228-f8cfd7740bf4 h1:FpNYQoqP3jF5XOu/gxsaasVDCSueg74mznlUkpiuEyo= +github.com/mulgadc/viperblock v1.13.1-0.20260722023228-f8cfd7740bf4/go.mod h1:sW0hxZjN7WDQQZ39L5mE58fVLbSv3mBP3/ifFPdwJtU= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= diff --git a/scripts/images/eks-node/mulga-vpc-mtu.sh b/scripts/images/eks-node/mulga-vpc-mtu.sh index 20ad95855..ee9d4060f 100755 --- a/scripts/images/eks-node/mulga-vpc-mtu.sh +++ b/scripts/images/eks-node/mulga-vpc-mtu.sh @@ -4,13 +4,27 @@ # # Spinifex carries tenant traffic over an OVN geneve underlay (~1442B usable on a # 1500B host link). The EKS node's primary ENI (eth0) comes up at the default -# 1500 — cloud-init's Ec2 datasource has no MTU leaf and Alpine's udhcpc ignores -# DHCP option 26 — so flannel derives flannel.1 = 1500-50 = 1450. VXLAN frames up -# to 1500 then exceed the underlay and are silently dropped: small packets (TCP -# handshakes) pass while large ones (TLS, kubelet streams, pod-to-pod payloads) -# blackhole. Pinning eth0 down to 1320 (flannel.1 = 1270) leaves margin under the -# underlay and restores the datapath. This must run before k3s starts flannel, -# which reads the iface MTU once at startup. +# 1500 — cloud-init's Ec2 datasource has no MTU leaf — so flannel derives +# flannel.1 = 1500-50 = 1450. VXLAN frames up to 1500 then exceed the underlay and +# are silently dropped: small packets (TCP handshakes) pass while large ones (TLS, +# kubelet streams, pod-to-pod payloads) blackhole. Pinning eth0 down to 1320 +# (flannel.1 = 1270) leaves margin under the underlay and restores the datapath. +# This must run before k3s starts flannel, which reads the iface MTU once at +# startup. +# +# Pinning the LINK is not sufficient on its own. This image runs dhcpcd, not +# udhcpc (an earlier version of this comment had that backwards), and dhcpcd +# honours DHCP option 26 by stamping the advertised MTU onto every route it +# installs: +# +# default via 10.32.101.1 dev eth0 proto dhcp src 10.32.101.7 metric 1002 mtu 1442 +# +# Linux prefers a route's MTU metric over the device MTU (dst_mtu()), so the node +# still advertises an MSS derived from 1442 no matter what the link says, and +# large inbound segments blackhole in the HOST netns — which is where containerd +# pulls run, so image pulls fail with `TLS handshake timeout` while pod traffic +# (correctly sized off cni0) is fine. Pod netns working while the node netns does +# not is the signature. Re-stamp the routes to match the pinned link. # # eth0 is always the primary data ENI (cloud-init set-name; mgmt0, when present, # is a separate NIC and is left at its default). The default-route interface is @@ -30,9 +44,47 @@ pin() { fi } +# Rewrite the MTU metric dhcpcd stamped onto the routes of one interface, so no +# route can advertise a larger MSS than the pinned link carries. Routes with no +# MTU metric inherit the device MTU and are left alone. +# +# `ip route show dev X` omits the "dev X" token from each line, so it is added +# back before replacing; a "default" line already carries its own dev. +restamp() { + iface="$1" + [ -n "$iface" ] || return 0 + ip -4 route show dev "$iface" 2>/dev/null | while read -r line; do + case "$line" in + *" mtu "*) ;; + *) continue ;; + esac + cur=$(echo "$line" | sed -n 's/.* mtu \([0-9]*\).*/\1/p') + [ -n "$cur" ] || continue + [ "$cur" != "$MTU" ] || continue + spec=$(echo "$line" | sed "s/ mtu $cur/ mtu $MTU/") + case "$spec" in + *" dev "*) ;; + *) spec="$spec dev $iface" ;; + esac + # Word splitting is intended: $spec is a route specification, not a path. + # shellcheck disable=SC2086 + if ip route replace $spec; then + echo "[mulga-vpc-mtu] $iface route MTU $cur -> $MTU ($line)" + else + echo "[mulga-vpc-mtu] WARNING: failed to restamp $iface route: $line" >&2 + fi + done +} + pin eth0 route_iface=$(ip -4 route show default 2>/dev/null | awk '{print $5; exit}') [ "$route_iface" = "eth0" ] || pin "$route_iface" +# setup.sh writes `nooption interface_mtu` to dhcpcd.conf so no future lease can +# re-stamp a route MTU; this pass fixes routes installed before that took effect +# (an already-running dhcpcd, or an image built before the fix). +restamp eth0 +[ "$route_iface" = "eth0" ] || restamp "$route_iface" + exit 0 diff --git a/scripts/images/eks-node/mulga-vpc-mtu_test.sh b/scripts/images/eks-node/mulga-vpc-mtu_test.sh new file mode 100644 index 000000000..5ecaa4a63 --- /dev/null +++ b/scripts/images/eks-node/mulga-vpc-mtu_test.sh @@ -0,0 +1,118 @@ +#!/bin/sh +# Self-contained POSIX test for mulga-vpc-mtu.sh route re-stamping. No root or +# real interfaces needed: `ip` is stubbed on PATH, serving fixture route output +# and recording every mutating call. +# +# Covers the fault this script exists to prevent: dhcpcd stamps the DHCP MTU +# (option 26) onto its routes, Linux prefers a route's MTU metric over the device +# MTU, so pinning only the link leaves the node advertising an MSS too large for +# the path and large inbound segments blackhole in the host netns. +# +# Run: sh scripts/images/eks-node/mulga-vpc-mtu_test.sh +set -eu + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +PINNER="${SCRIPT_DIR}/mulga-vpc-mtu.sh" +WORK=$(mktemp -d) +trap 'rm -rf "${WORK}"' EXIT + +STUBBIN="${WORK}/bin" +mkdir -p "${STUBBIN}" + +# Stub `ip`: serves ${ROUTES_DEFAULT} / ${ROUTES_DEV} for show, records mutations +# ("link set …" / "route replace …") to ${CALLS}. Anything else is a no-op. +cat > "${STUBBIN}/ip" <<'STUB' +#!/bin/sh +args="$*" +case "$args" in + *"route show default"*) cat "${ROUTES_DEFAULT}"; exit 0 ;; + *"route show dev "*) cat "${ROUTES_DEV}"; exit 0 ;; + "link show dev "*) exit 0 ;; + "link set dev "*) echo "$args" >> "${CALLS}"; exit 0 ;; + "route replace "*) echo "$args" >> "${CALLS}"; exit 0 ;; +esac +exit 0 +STUB +chmod +x "${STUBBIN}/ip" + +PASS=0 +FAIL=0 +check() { # check + if [ "$2" = "$3" ]; then + PASS=$((PASS + 1)) + else + FAIL=$((FAIL + 1)) + echo "FAIL: $1" + echo " expected: [$2]" + echo " actual: [$3]" + fi +} + +# run_case : runs the pinner against fixture +# route tables and leaves the recorded mutations in ${CALLS}. +run_case() { + CASE="${WORK}/$1" + mkdir -p "${CASE}" + CALLS="${CASE}/calls" + ROUTES_DEFAULT="${CASE}/default" + ROUTES_DEV="${CASE}/dev" + : > "${CALLS}" + printf '%s\n' "$2" > "${ROUTES_DEFAULT}" + printf '%s\n' "$3" > "${ROUTES_DEV}" + export CALLS ROUTES_DEFAULT ROUTES_DEV + PATH="${STUBBIN}:${PATH}" sh "${PINNER}" >/dev/null 2>&1 +} + +replaces() { grep -c '^route replace' "${CALLS}" 2>/dev/null | head -1; } + +# 1. The real-world fault: dhcpcd's 1442 on the default route is rewritten to the +# pinned link MTU, and the rest of the route spec is preserved verbatim. +run_case dhcpcd-1442 \ + 'default via 10.32.101.1 dev eth0 proto dhcp src 10.32.101.7 metric 1002 mtu 1442' \ + 'default via 10.32.101.1 proto dhcp src 10.32.101.7 metric 1002 mtu 1442' +check "link is pinned to 1320" \ + "link set dev eth0 mtu 1320" \ + "$(grep '^link set' "${CALLS}" | head -1)" +check "route MTU rewritten to the pinned link MTU" \ + "route replace default via 10.32.101.1 proto dhcp src 10.32.101.7 metric 1002 mtu 1320 dev eth0" \ + "$(grep '^route replace' "${CALLS}" | head -1)" + +# 2. `ip route show dev X` omits the dev token; it must be added back or the +# replace would be rejected (and the fault would silently persist). +run_case dev-token-restored \ + 'default via 10.32.101.1 dev eth0 proto dhcp metric 1002 mtu 1442' \ + '10.32.101.0/24 proto kernel scope link src 10.32.101.7 metric 1002 mtu 1442' +check "dev token re-added to a dev-scoped route" \ + "route replace 10.32.101.0/24 proto kernel scope link src 10.32.101.7 metric 1002 mtu 1320 dev eth0" \ + "$(grep '^route replace' "${CALLS}" | head -1)" + +# 3. Idempotent: a route already at the pinned MTU is left alone, so a re-run (or +# a boot after the dhcpcd.conf fix) issues no route changes. +run_case already-pinned \ + 'default via 10.32.101.1 dev eth0 proto dhcp metric 1002 mtu 1320' \ + 'default via 10.32.101.1 proto dhcp metric 1002 mtu 1320' +check "no replace when the route MTU already matches" "0" "$(replaces)" + +# 4. A route with no MTU metric inherits the device MTU — leave it alone rather +# than pinning a value the link may later change. +run_case no-mtu-metric \ + 'default via 10.32.101.1 dev eth0 proto dhcp metric 1002' \ + 'default via 10.32.101.1 proto dhcp metric 1002' +check "no replace when the route carries no MTU metric" "0" "$(replaces)" + +# 5. The pinned value is overridable, and the override reaches the routes too. +run_case custom-mtu \ + 'default via 10.32.101.1 dev eth0 proto dhcp metric 1002 mtu 1442' \ + 'default via 10.32.101.1 proto dhcp metric 1002 mtu 1442' +CASE="${WORK}/custom-mtu-run" +mkdir -p "${CASE}" +CALLS="${CASE}/calls" +: > "${CALLS}" +export CALLS +PATH="${STUBBIN}:${PATH}" MULGA_VPC_NIC_MTU=1408 sh "${PINNER}" >/dev/null 2>&1 +check "MULGA_VPC_NIC_MTU overrides both link and route" \ + "route replace default via 10.32.101.1 proto dhcp metric 1002 mtu 1408 dev eth0" \ + "$(grep '^route replace' "${CALLS}" | head -1)" + +echo "pass=${PASS} fail=${FAIL}" +[ "${FAIL}" -eq 0 ] diff --git a/scripts/images/eks-node/setup.sh b/scripts/images/eks-node/setup.sh index b8d5680c7..9ca06b95d 100755 --- a/scripts/images/eks-node/setup.sh +++ b/scripts/images/eks-node/setup.sh @@ -142,4 +142,16 @@ if ! grep -qxF 'noipv4ll' /etc/dhcpcd.conf 2>/dev/null; then echo 'noipv4ll' >> /etc/dhcpcd.conf fi +# Ignore the DHCP MTU (option 26). dhcpcd honours it by stamping the advertised +# value onto every route it installs, and Linux prefers a route's MTU metric over +# the device MTU — so the subnet's advertised MTU wins over the link MTU that +# mulga-vpc-mtu pins, and the node advertises an MSS too large for the real path. +# Large inbound segments then blackhole in the host netns (image pulls fail with +# `TLS handshake timeout`) while pod traffic, sized off cni0, is unaffected. +# mulga-vpc-mtu is the single source of truth for this node's MTU; this stops +# dhcpcd re-stamping a different one on every lease renewal. +if ! grep -qxF 'nooption interface_mtu' /etc/dhcpcd.conf 2>/dev/null; then + echo 'nooption interface_mtu' >> /etc/dhcpcd.conf +fi + echo "[eks-node-setup] done" diff --git a/spinifex/handlers/eks/cp_vpc.go b/spinifex/handlers/eks/cp_vpc.go index 274fb7e59..7e41605c9 100644 --- a/spinifex/handlers/eks/cp_vpc.go +++ b/spinifex/handlers/eks/cp_vpc.go @@ -103,6 +103,9 @@ type natGatewayProvisioner interface { // managed control-plane VPC from. All calls run under the system account. type CPVPCDeps struct { VPC vpcProvisioner + // SG is used only by teardown, to clear security groups left in the managed + // CP VPC that the tag-driven sweep cannot see. Nil is tolerated. + SG sgProvisioner IGW igwProvisioner RT routeTableProvisioner NGW natGatewayProvisioner @@ -117,6 +120,7 @@ type CPVPCDeps struct { func (s *EKSServiceImpl) cpVPCDeps() CPVPCDeps { return CPVPCDeps{ VPC: s.deps.VPCMgr, + SG: s.deps.VPCSG, IGW: s.deps.IGW, RT: s.deps.RouteTable, NGW: s.deps.NATGW, @@ -537,6 +541,55 @@ func DeleteClusterCPVPC(ctx context.Context, deps CPVPCDeps, accountID, clusterN if err := DeleteClusterIGW(ctx, deps.IGW, accountID, vpcID, clusterName); err != nil { slog.WarnContext(ctx, "DeleteClusterCPVPC: delete IGW failed", "vpc", vpcID, "err", err) } + + // Sweep residual subnets by VPC identity, not by role tag. + // + // The tagged sweep above only finds subnets that survived long enough to be + // tagged. A create that fails partway — the first control-plane server + // failing to launch, say — leaves untagged subnets behind, which the tagged + // sweep cannot see. DeleteVpc then rejects with DependencyViolation, and + // because teardown is re-driven on a timer the cluster sits in DELETING + // forever, retrying the same doomed delete every couple of minutes. The + // cluster name stays unusable and terraform's delete wait can never pass. + if snOut, err := deps.VPC.DescribeSubnets(ctx, &ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{{Name: aws.String("vpc-id"), Values: aws.StringSlice([]string{vpcID})}}, + }, accountID); err != nil { + slog.WarnContext(ctx, "DeleteClusterCPVPC: describe residual subnets failed", "vpc", vpcID, "err", err) + } else if snOut != nil { + for _, sn := range snOut.Subnets { + snID := aws.StringValue(sn.SubnetId) + slog.InfoContext(ctx, "DeleteClusterCPVPC: removing residual subnet the tagged sweep missed", + "cluster", clusterName, "vpc", vpcID, "subnet", snID) + if _, err := deps.VPC.DeleteSubnet(ctx, &ec2.DeleteSubnetInput{SubnetId: aws.String(snID)}, accountID); err != nil { + slog.WarnContext(ctx, "DeleteClusterCPVPC: delete residual subnet failed", "subnet", snID, "err", err) + } + } + } + + // Same for security groups: DeleteVpc rejects any non-default SG, and the + // tagged CP-SG teardown misses ones a partial create never tagged. + if deps.SG != nil { + if sgOut, err := deps.SG.DescribeSecurityGroups(ctx, &ec2.DescribeSecurityGroupsInput{ + Filters: []*ec2.Filter{{Name: aws.String("vpc-id"), Values: aws.StringSlice([]string{vpcID})}}, + }, accountID); err != nil { + slog.WarnContext(ctx, "DeleteClusterCPVPC: describe residual SGs failed", "vpc", vpcID, "err", err) + } else if sgOut != nil { + for _, sg := range sgOut.SecurityGroups { + // The default SG goes with the VPC cascade and cannot be + // deleted on its own; it is not what blocks DeleteVpc. + if aws.StringValue(sg.GroupName) == "default" { + continue + } + sgID := aws.StringValue(sg.GroupId) + slog.InfoContext(ctx, "DeleteClusterCPVPC: removing residual SG the tagged sweep missed", + "cluster", clusterName, "vpc", vpcID, "sg", sgID) + if _, err := deps.SG.DeleteSecurityGroup(ctx, &ec2.DeleteSecurityGroupInput{GroupId: aws.String(sgID)}, accountID); err != nil { + slog.WarnContext(ctx, "DeleteClusterCPVPC: delete residual SG failed", "sg", sgID, "err", err) + } + } + } + } + if _, err := deps.VPC.DeleteVpc(ctx, &ec2.DeleteVpcInput{VpcId: aws.String(vpcID)}, accountID); err != nil && !awserrors.IsNotFound(err) { return fmt.Errorf("eks: delete cp vpc %s: %w", vpcID, err) } diff --git a/spinifex/network/reconcile/testdata/kitchen_sink_nb.golden b/spinifex/network/reconcile/testdata/kitchen_sink_nb.golden index da2d71fb7..b67a3a403 100644 --- a/spinifex/network/reconcile/testdata/kitchen_sink_nb.golden +++ b/spinifex/network/reconcile/testdata/kitchen_sink_nb.golden @@ -1,5 +1,5 @@ -dhcp {UUID:dhcp:10.0.1.0/24 CIDR:10.0.1.0/24 Options:map[dns_server:{8.8.8.8, 1.1.1.1} lease_time:3600 mtu:1442 router:10.0.1.1 server_id:10.0.1.1 server_mac:02:42:e1:1f:07:a8] ExternalIDs:map[spinifex:subnet_id:subnet-pub spinifex:vpc_id:vpc-a]} -dhcp {UUID:dhcp:10.0.2.0/24 CIDR:10.0.2.0/24 Options:map[dns_server:{8.8.8.8, 1.1.1.1} lease_time:3600 mtu:1442 router:10.0.2.1 server_id:10.0.2.1 server_mac:02:56:6c:1d:90:2a] ExternalIDs:map[spinifex:subnet_id:subnet-priv spinifex:vpc_id:vpc-a]} +dhcp {UUID:dhcp:10.0.1.0/24 CIDR:10.0.1.0/24 Options:map[dns_server:{8.8.8.8, 1.1.1.1} lease_time:3600 mtu:1408 router:10.0.1.1 server_id:10.0.1.1 server_mac:02:42:e1:1f:07:a8] ExternalIDs:map[spinifex:subnet_id:subnet-pub spinifex:vpc_id:vpc-a]} +dhcp {UUID:dhcp:10.0.2.0/24 CIDR:10.0.2.0/24 Options:map[dns_server:{8.8.8.8, 1.1.1.1} lease_time:3600 mtu:1408 router:10.0.2.1 server_id:10.0.2.1 server_mac:02:56:6c:1d:90:2a] ExternalIDs:map[spinifex:subnet_id:subnet-priv spinifex:vpc_id:vpc-a]} policy {UUID:X Priority:1000 Match:inport == "rtr-subnet-pub" && ip4.dst == 0.0.0.0/0 && ip4.dst != 10.0.0.0/16 && ip4.dst != 169.254.0.0/16 Action:reroute Nexthop:PTR Nexthops:[] Options:map[] ExternalIDs:map[spinifex:output_port:gw-vpc-a spinifex:subnet:subnet-pub]} policy {UUID:X Priority:1100 Match:inport == "rtr-subnet-priv" && ip4.dst == 0.0.0.0/0 && ip4.dst != 10.0.0.0/16 && ip4.dst != 169.254.0.0/16 && ip4.dst != 224.0.0.0/4 Action:drop Nexthop: Nexthops:[] Options:map[] ExternalIDs:map[spinifex:gate:drop spinifex:subnet:subnet-priv]} policy {UUID:X Priority:1100 Match:inport == "rtr-subnet-pub" && ip4.dst == 0.0.0.0/0 && ip4.dst != 10.0.0.0/16 && ip4.dst != 169.254.0.0/16 && ip4.dst != 224.0.0.0/4 Action:drop Nexthop: Nexthops:[] Options:map[] ExternalIDs:map[spinifex:gate:drop spinifex:subnet:subnet-pub]} diff --git a/spinifex/network/topology/dhcp_options.go b/spinifex/network/topology/dhcp_options.go index 4bd946e3b..bbc292d13 100644 --- a/spinifex/network/topology/dhcp_options.go +++ b/spinifex/network/topology/dhcp_options.go @@ -10,6 +10,16 @@ func BuildSubnetDHCPOptions(gwIP, routerMAC, dnsServer string) map[string]string "lease_time": "3600", "router": gwIP, "dns_server": dnsServer, - "mtu": "1442", + // 1442 is the geneve-only figure (1500 - 58) and overstates the usable + // MTU wherever the egress path adds encapsulation: on an IPsec-protected + // NAT path the measured path MTU is 1408, so a guest that believes 1442 + // advertises an MSS ~34B too large. Small packets pass; large inbound + // segments (a TLS ServerHello + certificate chain, image layers) are + // silently dropped, which surfaces as `TLS handshake timeout` rather than + // as anything MTU-shaped. Advertise the conservative figure — a few bytes + // of payload per segment is cheaper than a PMTU blackhole, and PMTU + // discovery still raises the effective size per destination where the + // path allows it. + "mtu": "1408", } }