Add Leader Election module to noobaa core - #9765

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/test/unit_tests/util_functions_tests/test_core_lease.js`:
- Around line 8-9: The KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT
environment variable assignments on lines 8-9 are mutating global process state
at module load time without restoration, causing test order dependencies. Move
these assignments into individual test setup/teardown (using
beforeEach/afterEach hooks) so that each test can independently control these
environment variables and restore them to their original state after the test
completes, preventing state leakage between tests.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cmd/core_init.js`:
- Around line 178-196: The issue is that if run_server_upgrade() throws an
error, the start() function exits without releasing the lease held by
_lease_client, causing the renewal loop from start_core_lease_renewal() to
continue and delaying failover. To fix this, wrap the code starting from after
await _lease_client.acquire_lease() through the run_supervisord() call in a
try/finally block, ensuring that stop_serving_and_release_lease() is called in
the finally clause whenever a lease was acquired, regardless of whether
run_server_upgrade() or any subsequent operation succeeds or fails.

In `@src/util/core_lease_utils.js`:
- Around line 330-349: The 409 conflict handling in the lease renewal loop uses
a continue statement that skips the _interruptible_sleep at the end of the loop,
causing a busy-loop without backoff that hammers the API server. Add an
_interruptible_sleep call with an appropriate retry delay before the continue
statement in the 409 conflict handling block (around line 342). Apply the same
fix to the release_lease method around line 254 where the same unthrottled-retry
pattern exists.
- Around line 116-133: In the _request method, the options object passed to
make_https_request contains rejectUnauthorized: false which disables TLS
certificate verification. Remove this line entirely to enable certificate
validation by default, allowing the make_https_request function to use the CA
certificates from INTERNAL_CA_CERTS or EXTERNAL_CA_CERTS paths automatically.
This will ensure the service account bearer token is protected by proper TLS
verification when communicating with the Kubernetes API server.

---

Nitpick comments:
In `@src/util/core_lease_utils.js`:
- Around line 191-207: The acquire_lease method loops indefinitely without any
timeout or maximum retry limit, which can cause silent blocking of core startup
if the API consistently returns non-200 status codes. Add a timeout mechanism to
the acquire_lease method by tracking the start time of the acquisition attempt
and comparing it against a configured maximum timeout duration (similar to how
config.CORE_LEASE_ACQUIRE_RETRY_MS is used). When the timeout is exceeded, throw
an error with a descriptive message indicating the lease acquisition failed due
to timeout. Optionally, add periodic error-level logging after each failed
attempt to make stuck pods observable before the timeout is reached.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/test/unit_tests/util_functions_tests/test_core_lease.js`:
- Around line 35-49: Clear the cached lease module after restoring the
Kubernetes environment variables in the test setup/teardown around lease_utils.
The issue is that core_lease_utils reads KUBERNETES_SERVICE_HOST and
KUBERNETES_SERVICE_PORT at import time, so resetting process.env alone still
leaves the cached module using test values. Update the mocha.after block in
test_core_lease.js to restore the env first and then delete
require.cache[LEASE_UTILS], so later tests re-require core_lease_utils with the
real environment.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cmd/core_init.js`:
- Around line 74-80: The shutdown fallback in kill_supervisord is using
proc.killed as the guard, but that flag is set as soon as SIGTERM is sent, not
when supervisord actually exits. Update the guard to track real child
termination state via the proc close/exit events (or an equivalent explicit
closed flag) so the SIGKILL fallback still runs if the process remains alive;
keep the change localized to kill_supervisord and the shutdown path that sends
SIGTERM.

In `@src/test/unit_tests/internal/test_core_init.js`:
- Around line 65-68: The supervisord child stub in the test setup does not mimic
real ChildProcess kill() behavior, so branches that check
_supervisord_proc.killed can be wrong. Update the child stub in the test around
the EventEmitter/kill setup so that child.kill() returns true and also flips
child.killed to true, keeping the stub behavior aligned with the code paths
exercised by test_core_init and any logic relying on the killed flag.

In `@src/util/core_lease_utils.js`:
- Around line 116-129: The Lease API request path in the _request method
currently calls make_https_request without any timeout, so stalled GET/PUT calls
can hang indefinitely and block lease renewal. Update _request in
core_lease_utils.js to pass an explicit timeout when invoking
make_https_request, or enforce the timeout inside that helper, and keep the
change scoped to the lease renewal request flow so the renew loop can fail fast
between deadline checks.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cmd/core_init.js`:
- Around line 21-25: The shutdown flow in start(), run_supervisord(),
step_down_from_lease(), and handle_sigterm() can race so supervisord’s close
event returns before lease cleanup finishes. Add coordination for the active
shutdown path (for example, track a shutdown promise or equivalent) and have
start() await it before resolving. Make sure the lease release logic in
step_down_from_lease() and the SIGTERM handler completes before main() can
proceed to process.exit().

In `@src/test/unit_tests/internal/test_core_init.js`:
- Around line 344-356: The test for start_core_lease_renewal only verifies
process.exit on lost leadership, so it does not cover the release contract.
Update the existing test around core_init.start_core_lease_renewal and
step_down_from_lease to also assert that mock_client.release_lease is called
when renew_lease_loop invokes the on_lost callback, so the test fails if
leadership loss exits without releasing the lease.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/util/leader_election.js`:
- Around line 141-158: Update the renewal deadline calculation and flow around
the GET/PUT requests in the leader-election renewal logic to reserve both
CORE_LEASE_REQUEST_TIMEOUT_MS intervals. Either subtract two request timeouts
from the deadline or recheck the deadline after the GET before issuing the PUT,
while preserving existing request behavior.
- Around line 228-270: Serialize the lifecycle in start(), stop(), and
acquire_lease() so shutdown cancels and awaits any pending acquisition before
releasing the lease. Track the acquisition task with a cancellation mechanism,
prevent stop() from returning until acquisition and renewal tasks are settled,
and ensure a cancelled acquisition cannot emit LEADERSHIP_ACQUIRED or start
renewal. Guard repeated start() calls so they cannot overwrite
_renew_loop_promise or launch concurrent lifecycle operations.

---

Duplicate comments:
In `@src/test/unit_tests/util_functions_tests/test_leader_election.js`:
- Around line 266-276: Update the “steps down when another holder is observed”
test to configure client._lease_duration_ms to a positive duration before
calling renew_lease_loop, ensuring the deadline check does not trigger first.
Keep the stubbed read_lease_state response and LEADERSHIP_LOST assertion so the
test specifically exercises the another-holder path.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/test/unit_tests/util_functions_tests/test_leader_election.js`:
- Around line 203-218: Extract a shared with_config_overrides helper in
test_leader_election.js that applies overrides and restores original config
values in a finally block, then use it in the five affected tests: lines
203-218, 279-304, 306-328, 330-350, and 409-438. Replace each manual
save/mutate/restore block while preserving each test’s existing overrides and
assertions.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/test/unit_tests/util_functions_tests/test_leader_election.js`:
- Around line 222-231: Update the test for acquire_lease to retain a reference
to the read_lease_state stub and assert it is called exactly once after
_stop_requested is set. Keep the existing update_lease assertion, ensuring the
test verifies that the stop request prevents retrying rather than only
preventing lease updates.

In `@src/util/leader_election.js`:
- Around line 415-428: Update _handle_put_conflict and its callers around the
409 renewal path to accept and propagate the renewal deadline or abort signal
into read_lease_state, so an expired renewal deadline interrupts the reread and
triggers leadership-loss handling without waiting for the independent request
timeout.
- Around line 409-413: Update _clear_renew_deadline and the renewal restart path
used by start() so _cycle_abort is recreated before the new renewal loop
accesses its signal. Preserve clearing the existing timer and abort state, while
ensuring every stop()-then-start() cycle has a fresh AbortController.
- Around line 362-388: Update _interruptible_sleep to retain the abort listener
and remove it whenever the sleep resolves, including normal timer completion,
abort handling, and _wake_interruptible_sleep. Ensure cleanup is performed once
and preserve the existing wake-up behavior.

---

Nitpick comments:
In `@src/test/unit_tests/util_functions_tests/test_leader_election.js`:
- Around line 178-184: Update the `logs warning on non-200 status without
retrying` test to spy on the client’s warning logger before calling
`release_lease()`, then assert that it was invoked for the 409 response while
preserving the existing single-call assertion for `_put_release_lease`.
- Around line 305-330: Make the timing-sensitive leader-election tests
deterministic or less narrow, including the test around renew_lease_loop and the
additional tests at the referenced ranges. Prefer sinon.useFakeTimers() when
compatible with the abort-signal and timeout paths; otherwise widen the
configured sleep, request-timeout, and deadline-margin values and adjust
assertions such as the elapsed-time bound to provide CI scheduling slack while
preserving the tested behavior.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/util/leader_election.js`:
- Around line 467-469: Update the successful renewal branch in the lease-renewal
flow around _reset_renew_deadline so it refreshes the holder’s lease
duration/deadline from the renewed Lease response, rather than retaining the
acquisition-time _lease_duration_ms. Ensure later reductions to
leaseDurationSeconds are reflected before the next renewal deadline is
calculated.

---

Outside diff comments:
In `@src/util/leader_election.js`:
- Around line 423-431: Update _handle_put_conflict and the corresponding renewal
paths around the referenced logic to require current ownership via
is_lease_held_by(lease, this._holder), rather than state.can_take. When the
lease is vacant, expired, or held by another identity, emit LEADERSHIP_LOST and
stop serving; preserve renewal only when the current holder still owns the
lease.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/util/leader_election.js`:
- Around line 432-433: Remove the redundant _stop_requested = false assignment
from renew_lease_loop(), relying on start() to initialize the flag and
preserving stop() requests that occur before renewal begins.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/test/unit_tests/util_functions_tests/test_leader_election.js`:
- Line 301: Update the affected leader-election tests to target the current API:
replace client._cycle_abort with client._renew_deadline_abort, and configure
config.LEASE_RENEW_DEADLINE_MS to create the intended short renewal deadline
instead of setting removed lease-duration or margin/timeout fields. Apply this
consistently across the deadline-related cases so each test exercises the timer
path and avoids undefined-property failures.

In `@src/util/leader_election.js`:
- Around line 144-163: Update _request to avoid AbortSignal.any() and enforce
the lease request deadline with an explicit timeout controller compatible with
Node 24.13.0. Combine the caller’s abort signal with the controller’s timeout
while preserving cancellation from either source, and ensure the timeout is
cleaned up after make_https_request completes.

---

Duplicate comments:
In `@src/util/leader_election.js`:
- Around line 227-241: Guard LeaderElector.start() against concurrent or
repeated invocations before resetting _stop_requested or beginning acquisition:
if an existing _running_promise represents an active start/acquisition/renewal
flow, reuse or return it. Ensure _running_promise is assigned once for the
complete lifecycle and cleared only when that lifecycle finishes, preventing
concurrent renew_lease_loop executions and preserving the existing stop()
cancellation behavior.