Conversation
Add a new 'forced-host' label/annotation that can be set on Pods and ReplicaSets to configure Velocity's forced hosts functionality. Players connecting via a specific hostname will be routed to the server with the matching forced-host label. See: https://docs.papermc.io/velocity/configuration/\#forced-hosts-section
A backend whose Deployment rolls could stay registered in Velocity (and as a load-balancer endpoint) pointing at the dead pod's IP, with no health check to evict it. Players routed there get "Unable to connect you to <server>", and the only remedy today is a manual `kubectl rollout restart` of the proxy. Three defects combine to produce this: 1. KuvelServiceHandler.unregisterPod early-returned whenever the server name was still mapped to another pod UID, leaving the Velocity ServerInfo / LB endpoint bound to the departed pod's IP and never verifying the surviving UID was actually a live pod. 2. The 5s RedisServerDiscovery poll's delete path ran unregisterPodOrIgnore outside the ReentrantLock that guards registration in processUpdatedPod (TOCTOU), and nothing re-derived state from the live pod list each tick. 3. updateLoadBalancerEndpoints (which rebuilds LB membership from the live pod list) was only ever called from registerLoadBalancer, never from the poll, so a stale endpoint survived in the round-robin rotation. getServersForStartup already prunes dead entries and rebuilds from live Running pods on boot -- which is exactly what a proxy restart does. This change runs that reconciliation continuously. Changes: - Level-triggered reconcile in the 5s poll (RedisServerDiscovery.reconcile): after diff/delete processing each tick, evict any podUidAndServerNameMap UID that is not in the current live-Running labelled-pod set, then call KuvelServiceHandler.reconcileLoadBalancerEndpoints() to rebuild every load balancer's membership from live pods. The delete path and the sweep now run under the same ReentrantLock as registration, closing the TOCTOU. - KuvelServiceHandler.unregisterPod now reconciles instead of early- returning: when the name is still mapped to another UID it verifies that UID is a live (Running) pod and rebinds the Velocity ServerInfo / LB endpoint to the live pod's IP; only when a genuinely-live pod holds the name is teardown skipped. A name is never left bound to a departed UID. Minimal change, reuses existing helpers (registerPod, updateLoadBalancerEndpoints) and mirrors getServersForStartup's Running- phase semantics. The forced-host behaviour from c17652b is preserved. Co-authored-by: Chip Wolf <hello@chipwolf.uk> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…suite from stale branches (#2) * feat: add workflow_dispatch trigger and upload artifact step for Kuvel build * Update Java version to 21 and improve build workflow - Update Java version from 17 to 21 in pom.xml and build.yml - Switch JDK distribution from adopt to temurin - Add workflow_dispatch trigger - Add artifact upload step * test(forced-host): add unit tests for forced-host concurrency and routing Rescued from stale branch test/k8s-integration (418bcc0). Adds the JUnit 5 + Mockito test-scope dependencies and two test classes: - ForcedHostConcurrencyTest: validates the ConcurrentHashMap + CopyOnWriteArrayList pattern used by KuvelServiceHandler for forced hosts is safe under concurrent add/remove/iterate. - ChooseInitialServerListenerTest: covers forced-host routing, fall through to initial servers, unregistered-server skip, and empty-list cases in ChooseInitialServerListener. The forced-host source changes from that branch are already present on develop/v3 (ConcurrentHashMap + CopyOnWriteArrayList, if/return listener), so only the tests are carried; they compile and pass against the current source unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(k8s): add forced-host integration harness Rescued from stale branch test/k8s-integration (418bcc0). Adds a Kubernetes harness to exercise forced-host routing end to end: - Dockerfile.kuvel-plugin packages target/Kuvel.jar (matches the pom finalName) into a sidecar image copied into the proxy plugins dir. - backends.yaml, dragonfly.yaml, velocity-config.yaml, velocity-proxy.yaml deploy a Paper backend, a Redis (dragonfly) instance, and a Velocity proxy wired via the KUVEL_* env overrides that KuvelConfig reads, using the kuvel.azisaba.net label prefix and the forced-host label key the current plugin expects. - test_forced_host.py drives the Minecraft handshake against the proxy NodePort (30577) to assert forced-host routing. Labels, jar name, config env vars, and ports are internally consistent with the current plugin, but this harness has NOT been validated against a live cluster in this change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Chip Wolf <hello@chipwolf.uk> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a new 'forced-host' label/annotation that can be set on Pods and ReplicaSets to configure Velocity's forced hosts functionality. Players connecting via a specific hostname will be routed to the server with the matching forced-host label.
See: https://docs.papermc.io/velocity/configuration/\#forced-hosts-section
Resolves #70