allthingssecurity opened a new pull request, #29583: URL: https://github.com/apache/camel/pull/29583
# Description [CAMEL-26802](https://issues.apache.org/jira/browse/CAMEL-26802) `MasterRoutePolicy` joins the ZooKeeper group in `doStart` and leaves it in `doStop`. The route builder adds the policy to the CamelContext as a service, so it only stops with the CamelContext, and it overrode only `onInit`. That left these cases: - **A removed route kept the mastership.** A route removed on the master (`removeRoute`, or a route reload, which removes the route and adds it again) left the policy in the group as the master, so no other node started the route: it ran nowhere in the cluster. On a slave, the policy of a removed route also stayed in the group, and when it was elected later it started the consumer of the removed route. - **A route could not be added again.** When a route is added to a running CamelContext, the route builder starts the policy before it calls `onInit`, so `doStart` failed with a `NullPointerException` (`this.route` is null) and the route was not created: a route reload with a new `MasterRoutePolicy` failed. With the same policy instance the route was added, but its consumer was never started, as the policy was still the master and had already started the consumer of the removed route. - **A slave started its consumer when its route was started or resumed.** Only `autoStartup=false` kept the route of a slave stopped. A `startRoute` or `resumeRoute` on a slave (JMX, controlbus, a scheduled route policy; `resumeRoute` also starts the consumer of a route that is not started) started its consumer, and nothing stopped it again, so the slave consumed next to the master. A route left at `autoStartup=true` consumed on every node. The JIRA said the start/resume case was still to be confirmed with a test; `routeOfASlaveIsKeptStoppedWhenStarted` and `routeOfASlaveIsKeptStoppedWhenResumed` below confirm it on main. The `NullPointerException` on a re-add with a new policy was not in the JIRA; the test found it. Change (the same gaps were fixed in `HazelcastRoutePolicy` with CAMEL-26784, #29532; this follows its shape where the two policies are alike): - `onRemove` of the policy's route leaves the group (the group listener is stopped, as in `doStop`), so another node becomes the master. The policy keeps running as a CamelContext service; - `doStart` joins the group only when the policy has a route. When a route is added to the running CamelContext (a route added again with the same policy, or a new policy, which the route builder starts before it calls `onInit`), the policy joins the group on the `RouteAddedEvent` of its route, which comes after the consumer of the route was created: a node elected before that would have had no consumer to start. The event notifier ignores all but route events, so it adds no cost to the exchanges. `onInit`, `onRemove` and the join hold the service lock of the policy, as `start()` and `stop()` do; - `onStart` and `onResume` stop the consumer on a node that is not the master and has not started it (`masterConsumer` not set and the group does not say it is the master), under the leadership lock that the election and the disconnect already use; the consumer is started when the node is elected, as before. On a node that is the master but has not started its consumer yet (the CHANGED event or a retry is pending) the consumer is left running and the pending start marks it as started; - the election and disconnect callbacks read the route once, and do nothing when it was removed or has no consumer yet; the JMX attributes (`isConnected`, `isMaster`, `slaves`, `lastEvent`) and the leadership check read the group listener once, as it is now cleared when the route is removed; - with a `CuratorFramework` of the user (set on the policy or found in the registry), the group listener, which registers itself as a connection state listener on it, is removed from it when the policy leaves the group (a new group listener registers itself on every join). What stays as it was: stopping a route (without removing it) keeps the mastership, as in `HazelcastRoutePolicy`; on a slave the consumer runs for a moment after a start or resume before `onStart`/`onResume` stops it again (the route controller starts it before it calls the policy), so it can take a message in that moment. The documentation still asks for `noAutoStartup()`, which avoids that moment when the CamelContext starts (not after a later `startRoute` or `resumeRoute`). A policy is still meant for one route: as before, it follows the route it was initialized with last. Leaving the group is logged at INFO when a route is removed while the CamelContext runs, and at DEBUG when the CamelContext stops. Tests, new `MasterRoutePolicyRouteLifecycleTest` (two CamelContexts as two nodes, and a ZooKeeper server in the test JVM, as `ZooKeeperConsumerSessionExpiryTest` in camel-zookeeper does; that needs the `metrics-core` and `snappy-java` test dependencies, "provided" dependencies of zookeeper, as in camel-zookeeper; Awaitility and mock waits only): - `removedRouteGivesUpTheMastership`: the route is removed on the master, the other node must take over and consume; - `removedRouteOfASlaveLeavesTheGroup`: the route is removed on the slave, and the group in ZooKeeper (read with a Curator client of the test) must have one member left; - `routeAddedAgainWithANewPolicyRunsAgain`: the route is removed and added again on the master with a new policy, and must consume; - `routeAddedAgainWithTheSamePolicyRunsAgain`: the same with the same policy instance; - `routeAddedToARunningContextJoinsTheGroupWithItsConsumer`: a route with a new policy is added to a running CamelContext; the policy must not be in the group yet when the consumer of the route is created, and the node must then become the master and start the consumer; - `routeOfASlaveIsKeptStoppedWhenStarted`: `startRoute` on the slave keeps its consumer stopped, and it consumes once it takes over; - `routeOfASlaveIsKeptStoppedWhenResumed`: `resumeRoute` of the slave's route, and start, suspend and resume, keep the consumer stopped, and it consumes once it takes over (a SEDA consumer that is resumed after the policy stopped it is marked as started without consuming, so the election would not start it); - `routeWithAutoStartupRunsOnlyOnTheMaster`: both nodes with `autoStartup=true`, only the master consumes, and the slave takes over; - `restartedContextRunsTheRouteAgain`: a control (it passes on main too): the CamelContext is stopped and started again, and the route consumes again. "Consumes" is checked by sending a message to the node and waiting for it at a mock, not only by the consumer status. On main 8 of the 9 fail (all but the restart control); without the `onResume` override `routeOfASlaveIsKeptStoppedWhenResumed` fails; against a first version of this change, which joined the group in `onInit`, `routeAddedToARunningContextJoinsTheGroupWithItsConsumer` fails. With the change the 9 pass, 180/180 over 20 runs in one JVM (and the existing `MasterRoutePolicyLeadershipTest` 100/100), and the camel-zookeeper-master unit tests pass (32 run, 0 failures). Upgrade guide: a 4.24 entry, as a route reloaded on the master can now move to another node, and a slave no longer consumes after a start or resume; a route left at `autoStartup=true` now consumes only on the master, also at startup. The class javadoc now says to use one policy instance per route. Follow-ups I noticed next to this, not changed here (I can file JIRAs if wanted): after a reload with a new policy instance the old instance stays a service of the CamelContext until it stops (the route builder adds every policy as a service and nothing removes it; it is out of the group, so it does no harm besides the object, its route-added event notifier, which does nothing, and an idle retry executor); `MasterConsumer` in the same module registers a connection state listener on a shared CuratorFramework on every start; and with a shared CuratorFramework, a route removed while the connection to ZooKeeper is lost leaves its ephemeral znode in the group until the session of that client ends (a client that reconnects keeps it), so if that node was the oldest member, the route runs nowhere meanwhile. Main only: this changes what a route reload and a route at `autoStartup=true` do (upgrade-guide entry); the maintenance branches also lack CAMEL-26540, which this builds on. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it): [CAMEL-26802](https://issues.apache.org/jira/browse/CAMEL-26802). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built the core modules and camel-zookeeper-master with the formatter and import-sort plugins and ran the camel-zookeeper-master tests; no generated file changes. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
