diff --git a/docs/ldap_sync.md b/docs/ldap_sync.md index 5d08cf8bbab..789a6a1efc0 100644 --- a/docs/ldap_sync.md +++ b/docs/ldap_sync.md @@ -32,7 +32,7 @@ litellm_settings: The example uses anonymous LDAP reads over TLS. For a service account, add `bind_dn: os.environ/LDAP_BIND_DN` and `bind_password: os.environ/LDAP_BIND_PASSWORD`. Every enabled sync requires TLS, including anonymous reads and dry runs. Use `ldaps://` or set `start_tls: true` with `ldap://`. LDAPS and StartTLS verify the server certificate, using the system trust store or the optional `ca_cert_path` CA bundle. -Start with `dry_run: true`, restart the proxy, and review the planned team creation and member additions/removals in its normal logs. Set `dry_run: false` and restart to apply changes. The first run starts shortly after startup and later runs start every `interval_minutes`, which must be at least 5. Logs also report matched and unmatched users and skipped groups. There is no separate sync UI or run-history table. +Start with `dry_run: true`, restart the proxy, and review the planned team creation and member additions/removals in its normal logs. Set `dry_run: false` and restart to apply changes. The first run starts shortly after startup and later runs start every `interval_minutes`, which must be at least 5. Only one replica syncs per interval: the job lock lasts the whole interval, so a restart soon after a run, for example to switch off `dry_run`, takes effect at the next interval rather than immediately. Logs also report matched and unmatched users and skipped groups. There is no separate sync UI or run-history table. ## Identity and membership diff --git a/litellm/proxy/common_utils/ldap_sync.py b/litellm/proxy/common_utils/ldap_sync.py index 126063e0906..0ed26c74010 100644 --- a/litellm/proxy/common_utils/ldap_sync.py +++ b/litellm/proxy/common_utils/ldap_sync.py @@ -209,11 +209,8 @@ class LDAPSync: is not True ): return - try: - await self._sync() - finally: - # Release on completion so a restart (e.g. dry_run -> apply) syncs immediately. - await self.pod_lock_manager.release_lock(cronjob_id=LDAP_SYNC_JOB_ID) + # The lock is left to expire, so one replica syncs per interval; others firing within it skip. + await self._sync() def ldap_sync_settings(general_settings: Mapping[str, object]) -> LDAPSyncSettings | None: diff --git a/tests/unit/proxy/common_utils/test_ldap_sync.py b/tests/unit/proxy/common_utils/test_ldap_sync.py index 8e1f9fdf15f..26f0d129cdb 100644 --- a/tests/unit/proxy/common_utils/test_ldap_sync.py +++ b/tests/unit/proxy/common_utils/test_ldap_sync.py @@ -107,9 +107,6 @@ class LeaseRedis: async def async_get_cache(self, key: str) -> object | None: return self.values.get(key) - async def async_delete_cache(self, key: str) -> int: - return 1 if self.values.pop(key, None) is not None else 0 - def job_for( prisma: _FakePrisma, @@ -189,24 +186,23 @@ async def test_second_worker_cannot_sync_while_first_workers_run_holds_the_lock( release.set() await running assert not second_connection.closed - assert redis.values == {} @pytest.mark.asyncio -async def test_finished_run_releases_the_lock_so_a_restarted_worker_syncs_immediately(): +async def test_lock_outlives_a_finished_run_so_other_workers_skip_the_rest_of_the_interval(): prisma: Final = _FakePrisma(teams=[_team()]) redis: Final = LeaseRedis() first: Final = Connection([group(TEAM, member=[])]) - restarted: Final = Connection([group(TEAM, member=[])]) + later: Final = Connection([group(TEAM, member=[])]) await job_for(prisma, first, redis=redis).run() - assert redis.values == {} - await job_for(prisma, restarted, redis=redis).run() + await job_for(prisma, later, redis=redis).run() assert first.closed - assert restarted.closed + assert not later.closed + assert redis.values @pytest.mark.asyncio -async def test_failed_run_still_releases_the_lock(monkeypatch: pytest.MonkeyPatch): +async def test_failed_run_keeps_the_lock_until_the_next_interval(monkeypatch: pytest.MonkeyPatch): redis: Final = LeaseRedis() job: Final = job_for(_FakePrisma(teams=[_team()]), Connection([group(TEAM, member=[])]), redis=redis) @@ -216,7 +212,7 @@ async def test_failed_run_still_releases_the_lock(monkeypatch: pytest.MonkeyPatc monkeypatch.setattr(job, "_sync", failing_sync) with pytest.raises(RuntimeError): await job.run() - assert redis.values == {} + assert redis.values @pytest.mark.asyncio