Make the endpoint watcher the sole active-endpoint annotation writer so a Service snapshot captured during startup cannot replace a newer endpoint. Preserve intentionally empty snapshots across endpoint-to-zero-to-same transitions, select the cleared annotation from the configured egress family, reject malformed or wrong-family endpoints, and preserve the other family in dual-stack state.
Fixes#1673.
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(endpoints): recompute and withdraw only when no longer referenced
Track EndpointSlice/Endpoints deletions in provider state instead of stopping
the watcher or clearing unconditionally. Recompute the aggregate endpoint set
after each deletion and only withdraw local routing/BGP/DNAT state once no
tracked object still references it, matching the existing behaviour for a
Modified event that reduces endpoints to zero.
Remove the now-unreachable per-mode delete()/deleteAction() hooks, which acted
on stale pre-deletion state and could withdraw prematurely while another
object still referenced the same address. Fix the WireGuard control-plane
endpoint watcher to drop deleted objects from its cache instead of reloading
them as still current.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* feat(worker): support global leader election for BGP and WireGuard
Wire the existing global leader-election machinery into BGP.StartServices and
WireGuard.StartServices so both modes can run with a single node advertising
all services, matching the option already available for ARP and RoutingTable.
WireGuard has no multipath mechanism, so it must always run some form of
leader election (per-service or global); remove its no-election path and the
redundant services watcher previously started from OnStartedLeading, which
would have run a second time alongside the global election path.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(services): drop unnecessary election guard on route teardown
route.Manager.Delete is reference-counted per route and safe to call even
when nothing is tracked for it, exactly like the existing BGP host teardown.
Remove the leader-election guard on the RoutingTable branch of deleteService
so both branches read identically and route cleanup runs consistently across
all leader-election configurations.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(endpoints): treat an unknown ready condition as ready
Deferring a nil Serving to Ready matched the Serving godoc but stopped
half way: per the EndpointConditions.Ready godoc a nil Ready is an unknown
state that consumers should interpret as ready, which is what kube-proxy
does. An endpoint with both conditions unset, common in manually managed
EndpointSlices for external workloads, was therefore skipped and never
selected as a local endpoint.
Only skip the endpoint when the resolved condition is explicitly false,
and cover the condition permutations in the provider tests.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(endpoints): reconcile fully on endpoint object deletion
Delete only mirrored part of AddOrModify's zero-endpoint handling, so a
deleted EndpointSlice left the datapath inconsistent in three ways.
In ARP global-leader mode the VIP clusters were left running with no
backends, because the cluster stop that AddOrModify performs was missing.
When endpoints remained, lastKnownGoodEndpoint was never revisited, so a
last known endpoint that lived in the deleted slice stayed stale: egress
rules were neither torn down nor moved and the active-endpoint annotations
were not refreshed. The EndpointSlice controller can delete a whole slice
during consolidation without touching the survivor, so no follow-up event
is guaranteed to repair this. In WireGuard mode processInstance never ran,
leaving DNAT rules pointing at removed addresses.
Extract the shared decisions into shouldProcessInstance and
handleNoEndpoints and use them from both paths, and run the same
lastKnownGoodEndpoint and annotation reconciliation after a deletion.
handleNoEndpoints also guards against a missing instance, which the
deletion path can legitimately hit. The getEndpoints error is now wrapped
with the provider label like every other error here.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* docs(bgp): call out the services election behaviour change
Enabling leader election in BGP mode now runs a global services leader.
Nothing in the configuration changed for existing deployments, but a user
who enabled it for the control plane previously got the no-leader path,
where every node advertised the service VIP over ECMP, and now gets a
single advertising node.
Record this in the changelog and warn once on startup when this path is
taken, pointing at the settings that restore the previous datapath.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* refactor(endpoints): deduplicate the control plane endpoint watcher
The watch.Deleted case duplicated the Added/Modified body except for the
provider mutation, which meant maintaining the log message and the DNAT
update in two places. Pick the mutation from the event type and share the
rest.
Also state the single-object invariant on the Endpoints provider's
DeleteObject, so its asymmetry with the slice provider is not mistaken for
an oversight.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(endpoints): only ever consider active endpoints
GetLocalEndpoints skipped endpoints whose conditions said they were not
serving, but GetAllEndpoints applied no filtering at all. A service using
externalTrafficPolicy: Cluster therefore advertised backends that had
already been marked as not ready, and the two policies disagreed about
which endpoints back the same service.
Extract the condition evaluation into isServing and apply it on both
paths, and assert in the tests that Cluster and Local agree.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* refactor(endpoints): reconcile every endpoint event through one path
Now that a deletion recomputes the remaining endpoints, AddOrModify and
Delete differed only in how the event mutated the provider. Keeping two
near-identical bodies is what let their behaviour drift apart in the first
place, so fold them into a single Reconcile that dispatches the mutation
in applyEvent.
Deletions consequently pick up the parts of the add path they were still
missing: the IPv6 egress guard, the leader election start, and the
endpointless opt-in branch, which now keeps an opted-in Cluster service up
after its last EndpointSlice disappears instead of tearing it down.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
---------
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
The endpoint worker's delete() path calls clear(nil, nil, service), and
clear() unconditionally dereferenced svcCtx.LeaderCancel, panicking on
every service or endpoint deletion in WireGuard mode (reached via the
endpoint watcher on a Deleted event). Guard the dereference with a nil
check on svcCtx. Adds a regression test that reproduces the panic.
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* fix(e2e/services): pass metrics address as container arg
The prometheusHTTPServer env var is never read by kube-vip; environment
parsing only knows prometheus_server and ignores empty values, so every
parallel DaemonSet still tried to bind the :2112 flag default on the
shared host network. Pass the address as --prometheusHTTPServer instead,
where an empty value really disables the metrics server.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* fix(egress): restore CIDR fallback and cluster-wide SNAT exclusion
Auto-discovery dropped the kube-controller-manager fallback, breaking
clusters whose CNI does its own IPAM without Node PodCIDR allocation.
Fall back to the --cluster-cidr/--service-cluster-ip-range flags when
the ServiceCIDR or Node APIs yield nothing.
Per-node PodCIDR discovery combined with getSameFamilyCidr also
narrowed the SNAT exclusion to the local node's pod CIDR, so egress
traffic to pods on other nodes was SNAT'd to the VIP. Exclude every
same-family pod CIDR in both the nftables and iptables paths.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* test(e2e/services): fail failover tests when the VIP never recovers
leaderFailover and podFailover returned nil when the watch drained
without any post-kill update passing httpTest, so an unreachable VIP
after failover passed CI. Keep the warn-and-continue polling, but turn
the drain path into an error.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* test(e2e/services): require sustained quiet window in lease error check
A single quiet 2 s interval can coincide with the pause between failing
election retries and produce a false pass for the #1664 guard. Require
three consecutive intervals without counter growth and reset the streak
whenever the counter grows.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* fix(svcs): log service deletion only when a tracked service is removed
deleteTrackedService logged '(svcs) deleted' for every watch event of
every non-LoadBalancer service, even when nothing was tracked. Move the
log inside the svcCtx guard.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* fix(svcs): skip API refresh when egress endpoints are unchanged
updateEgressConfiguration issued a Services Get on every no-op watch
event before short-circuiting. Compare the ActiveEndpoint annotations
first and only refresh the service from the API when they differ.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* test(e2e/services): drop dead StartServiceTest and restore run summary
StartServiceTest was dead code once main.go inlined the phases, and the
E2E_KEEP_LOGS-conditional temp-dir cleanup plus the 'Testing Complete'
summary silently disappeared with it. Reinstate both in run() and
collect every test result so parallel failures are all reported instead
of collapsing to the errgroup's first error.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* style(e2e): gofmt test files
gofmt -l flagged e2e_bgp_healthcheck_test.go (the Mode == ModeBGP block
body was never re-indented) and a trailing space in e2e_sigusr1_test.go.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* ci: write ginkgo JSON reports where the artifact upload finds them
Without --output-dir ginkgo resolves the report path relative to the
suite directory, so /tmp/kube-vip-test-report-*.json landed under
testing/e2e/tmp/ and the /tmp/kube-vip-test* artifact glob missed it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* ci: scope cancel-in-progress to PRs and restore tag builds
cancel-in-progress: true also cancelled in-progress main builds on
back-to-back merges; limit cancellation to pull_request runs. The
branches: [main] filter added in the refactor dropped the tag pushes
the bare push trigger previously covered, and the release workflow
only publishes images, so restore lint/test coverage for v* tags.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* ci: fail the validation job on unformatted Go code
make check only formats '*.go pkg cmd' and merely warns, which let
unformatted files under testing/ land. Add a blocking gofmt -l check
over the whole repo that prints the offending files.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* test(e2e/services): kill kube-vip pods in the test namespace during failover
Since the parallel-phase refactor the kube-vip DaemonSet runs in the
per-test namespace, but the failover killer still listed pods in
kube-system, so it never deleted a single pod and no failover was ever
induced. The lenient drain path masked this; the strict assertion
exposed it.
Target the test namespace, gate success on events observed after at
least one actual pod kill, extend the watch window to 60s to leave
convergence margin after the ~25s kill churn, and return distinct
errors for aborted watch, zero kills, and unreachable VIP. Detach test
cleanup from the errgroup context (context.WithoutCancel) and demote
cleanup-path Fatals to errors so one failing test no longer kills
sibling teardown.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
* test(e2e/bgp): raise route-destination timeout for parallel runs
With ginkgo --procs=4, sibling processes create kind clusters and load
images on the same runner; the 30s window for the first route check
overlaps that contention and flakes (observed >13s convergence even in
passing specs). Use 120s, matching the BGP health-check suite's
re-announcement timeout. Eventually returns on success, so passing
specs are unaffected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
---------
Signed-off-by: Maximilian Rink <maximilian.rink@telekom.de>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Use one control-plane node for basic route checks and three for health-check scenarios so stopping one API server preserves quorum. Four Ginkgo workers now peak at eight control-plane containers instead of twelve.
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
Scope each egress DaemonSet to its test namespace, pin Kind's dual-stack Pod CIDRs, retain per-instance nftables tables, and grant the test service account ServiceCIDR discovery access.
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
Read ServiceCIDRs and Node PodCIDRs through the Processor client, retain a Service-only controller-manager fallback, and grant generated ClusterRoles get/list/watch access to ServiceCIDRs.
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
Base teardown on tracked UID state so LoadBalancer-to-ClusterIP transitions cancel the service context and remove the active instance even when the new object is ignored.
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
Run ARP, routing-table, and BGP modes on separate runners, shard scenario resources by Ginkgo worker, and cap routing-table mode at two workers for Kind stability.
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
Run independent service scenarios in dedicated namespaces, add per-namespace RBAC and cleanup barriers, make goroutine cleanup cancellation-safe, and parameterize CI execution with phase timing.
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
Replace fixed waits with polling, remove obsolete serialization, and align the BGP health-check mode guard with the rest of the BGP suite.
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* refactor(errors): centralize fatal error handling
Detect wrapped PanicError values consistently and preserve their underlying causes when adding fatal context. Apply the helpers to manager, cluster, and IPVS error paths.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(watchers): restart after terminal watch failures
Propagate fatal endpoint watcher failures through the owning service watcher so kube-vip releases leadership instead of remaining active with a stale watch. Treat terminal service, node, and annotation watch failures as errors while preserving clean context cancellation.
Return exhausted authorization failures to RetryWatcher, safely decode watch error objects, and replace direct go-spew diagnostics with structured logging.
Fixes#1685
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(services): replace state after traffic policy changes
Recreate the service context and instance as one generation when a Service change requires teardown. Ignore delayed leadership cleanup from superseded contexts so it cannot remove replacement state.
This prevents a stale Cluster-policy endpoint watcher from winning the service lease after externalTrafficPolicy changes to Local.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(cli): return command errors to container runtime
Propagate manager and service command failures through Cobra so the process exits with status 1. Show usage for invocation errors while keeping runtime failures concise.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* refactor(logging): use structured errors
Replace direct stdout error output with slog records for command failures and traffic mirror qdisc lookup failures.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
* fix(watchers): continue after endpoint deletion
Keep EndpointSlice watchers active when an individual endpoint object is deleted so replacement objects can be observed and service traffic can recover.
Assisted-by: GitHub-Copilot:unspecified
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
---------
Signed-off-by: Marcel Fest <marcel.fest@telekom.de>
kube-vip's DHCP client uses nclient4, whose BroadcastRawUDPConn.ReadFrom could
compute a negative DHCP length and panic on a malformed reply. That was fixed
in insomniacslk/dhcp#583; this bumps the module past that commit.
Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
the connmark policy-based routing was broken by tailscale, as we collide
with their connmark range (bits 16 to 23)
Signed-off-by: Daniel Nägele <daniel@naegele.dev>
