Skip to content

Add LeaseLock for leader election (upstream #1877) #1

Description

@chala2001

Goal

Add LeaseLock to kubernetes/base/leaderelection/resourcelock/, so leader election can use the
Kubernetes coordination.k8s.io/v1 Lease resource instead of only ConfigMap.

Upstream issue: kubernetes-client#1877

Why

client-go moved leader election to Lease and removed ConfigMap support entirely
(kubernetes/client-go commit 276ea3ed). The Python client still only ships ConfigMapLock.
Three separate people have asked for this, including one porting it to kubernetes_asyncio.

Prior art — this is a takeover, not a fresh start

PR kubernetes-client#2314 by @rvlane (Dec 2024, +139/-7) implemented this.

  • roycaihw reviewed it: "LGTM overall. Could you add a test?"
  • The test was never added.
  • k8s-triage-robot auto-closed it 2025-11-21 after lifecycle/stale -> lifecycle/rotten.

The approach was approved. Only the test was missing. rvlane's work is Apache-2.0 (CLA signed,
confirmed in the thread) and must be credited with a Co-authored-by trailer.

Reference implementation: kubernetes/client-go tools/leaderelection/resourcelock/leaselock.go.

Contract a lock must satisfy

Read from leaderelection.py:

  • get(name, namespace) -> (bool, LeaderElectionRecord | ApiException)
  • create(name, namespace, election_record) -> bool
  • update(name, namespace, updated_record) -> bool
  • .name, .namespace, .identity attributes

LeaderElectionRecord fields as written by try_acquire_or_renew:

field written as
holder_identity str
lease_duration str(config.lease_duration)
acquire_time str(datetime) -> "YYYY-MM-DD HH:MM:SS.ffffff"
renew_time same

Critical: leaderelection.py compares old_election_record.__dict__ != self.observed_record.__dict__.
So a value written and then read back must compare equal. The record round-trip through
V1LeaseSpec (which stores real datetimes, not strings) is the main correctness risk and is what
the test must cover.

Changes needed on top of rvlane's version

  1. Named logger. rvlane's file calls logging.basicConfig(level=logging.INFO) at import and
    uses logging.info(...). Since that PR, this package moved to logger = logging.getLogger("leaderelection") (see configmaplock.py:21, leaderelection.py). Calling
    basicConfig from a library reconfigures the root logger of the importing application — it
    must not ship. Use the named logger.
  2. Set lease_reference after a successful create, so a following update in the same cycle
    has a reference to work from.
  3. Do not silently swallow a time parse failuretime_str_to_iso currently logs and returns
    None, which would write a null acquireTime.
  4. Leave configmaplock.py alone. rvlane's diff also removed unused imports there; that is
    unrelated to the feature and widens the diff. Keep this PR focused.

Test plan (the missing piece roycaihw asked for)

New tests in leaderelection_test.py, mocking CoordinationV1Api:

  1. create builds a V1Lease with the right name and a spec carrying holder identity, duration
    and both times.
  2. get on a missing lease returns (False, ApiException).
  3. get on an existing lease returns a LeaderElectionRecord with the right holder identity.
  4. Round-trip: a record written by create/update and read back by get compares equal
    field-for-field to what leaderelection.py would have written. This is the test that protects
    the __dict__ comparison above.
  5. update returns False (not raising) when the API raises ApiException.

Out of scope

  • Deprecating or removing ConfigMapLock.
  • Any change to leaderelection.py itself.
  • The asyncio client.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions