allthingssecurity opened a new pull request, #29196: URL: https://github.com/apache/camel/pull/29196
# CAMEL-26571: camel-consul - ConsulClusterView must not be leader after it was stopped https://issues.apache.org/jira/browse/CAMEL-26571 A stopped `ConsulClusterView` could still be leader, in three ways: - **Stop (or failed start) while Consul cannot be reached.** `doStop` skipped `setMaster(false)` when `releaseLock` or `destroySession` threw, and the stop failed. When `releaseLock` threw, the session id was kept as well, so a later start was a no-op (`doStart` only runs without a session id): no new session, no watch, and the old leadership. Consul expires the session that is no longer renewed and another node takes the lock, so both nodes are leader. A start that fails after creating the session is stopped by `BaseService`, with the same result. For example a `master:` route stopped and started again while the Consul agent restarts. - **Answer handled while the view stops.** `Watcher.onComplete` checked the status, acquired the free key under `sessionIdLock` and called `setMaster` after releasing it. A `doStop` waiting for that lock (destroy, `setMaster(false)`) ran in between and was followed by `setMaster(true)`: the stopped view is leader and fires a leadership event (after a JMX `stopView` the clustered routes start on this node). - **Answer to a query of an earlier start.** After a quick stop and start, the old answer (or failure) was applied to the new session and the old callback kept querying: two watches. ## Changes - `doStop` releases the lock and destroys the session, each best effort (a failure is logged at WARN, Consul invalidates the session after its TTL) and always clears the session id and the leadership. The release now happens under `sessionIdLock` too, so a watch answer cannot acquire the lock again after it. - Each start creates a new `Watcher`. An answer decides and sets the leadership under `sessionIdLock`, and only if its watch is the current one and the view runs; otherwise it does nothing and does not query again. `onFailure` gets the same check. The leadership events of a watch answer are now fired while holding `sessionIdLock`, as `doStop` and `renewSession` already did. ## Tests `ConsulClusterViewRecoveryTest` (mocked `KeyValueClient`/`SessionClient`, no Consul) gets six tests: - `givesUpTheLeadershipWhenStoppedWhileConsulCannotBeReached` - `destroysTheSessionWhenTheLockCannotBeReleased` - `createsANewSessionWhenStartedAgainAfterAFailedStart` - `ignoresTheFailureOfAQueryFromBeforeARestart` - `ignoresTheAnswerOfAQueryFromBeforeARestart` - `doesNotTakeTheLeadershipAfterTheViewWasStopped`: the stop runs from a log hook on the callback thread right after the answer acquired the lock (no other call sits in that window) All six fail without the change. Found with a TLA+ model of the view (two nodes; Consul answers, session expiry and failing calls chosen nondeterministically): with the old code a stopped view can be leader and two nodes can be leader; the changed model keeps at most one leader, and every running view has a watch. No upgrade guide entry: no option or API changes. Stopping a view no longer fails when Consul cannot be reached (it logs a WARN). Backports: the change builds on CAMEL-25286 and applies to `camel-4.23.x`; 4.22.x and older have the same defects in different code and need their own patch. Follow-ups (already on main, separate JIRA): the `Consul` client created by each `doStart` is never destroyed (its OkHttp resources leak per restart), and `onFailure` can hit a `RejectedExecutionException` when the stop shuts the executor down between the check and `schedule`. _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]
