Skip to content

Enhance LDAP provider handling during node removal#1201

Open
stephdl wants to merge 1 commit into
mainfrom
sdl-7841
Open

Enhance LDAP provider handling during node removal#1201
stephdl wants to merge 1 commit into
mainfrom
sdl-7841

Conversation

@stephdl

@stephdl stephdl commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Improve the handling of LDAP providers when a node is removed from the cluster. This change ensures that stale entries are purged and surviving providers are notified for reconfiguration.

NethServer/dev#7841

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves cluster node removal by adding LDAP-provider cleanup steps so that providers hosted on a removed node are de-registered and remaining providers can reconfigure, reducing the risk of stale LDAP upstream entries after a node is removed.

Changes:

  • Detect LDAP providers that belong to the node being removed and group remaining providers by domain.
  • Notify a surviving LDAP provider (per domain) to run leave-replica for each removed provider’s serverid.
  • Publish cluster/event/service-ldap-changed to trigger ldapproxy reconfiguration after provider removal.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread core/imageroot/var/lib/nethserver/cluster/actions/remove-node/50remove_node Outdated
Comment thread core/imageroot/var/lib/nethserver/cluster/actions/remove-node/50remove_node Outdated
Comment thread core/imageroot/var/lib/nethserver/cluster/actions/remove-node/50remove_node Outdated
@stephdl
stephdl requested a review from DavidePrincipi June 11, 2026 07:14
Comment thread core/imageroot/var/lib/nethserver/cluster/actions/remove-node/50remove_node Outdated
@DavidePrincipi

Copy link
Copy Markdown
Member

Thanks for tackling this, @stephdl — the stale-provider problem is real, but I'd push back on where the fix lives.

50remove_node already publishes cluster/event/module-domain-changed with the affected domains/modules, and that event fires on every domain-membership change (remove-node, remove-module, remove-internal-domain, bind-user-domains) — not just node removal. I think the OpenLDAP-specific cleanup belongs in that event's handler inside ns8-openldap (and ns8-samba, if applicable), not in ns8-core:

  • Keeps core agnostic. Right now 50remove_node hardcodes the rfc2307 schema filter and the remove-server RPC action name — that's LDAP-specific knowledge leaking into a generic cluster action.
  • Covers more cases. A handler reacting to module-domain-changed catches stale providers from any code path that changes domain membership, not just node removal. The current fix only kicks in when a whole node is removed.
  • Can be simpler. By the time the event fires, remove-node has already deleted the node/module keys, so list_service_providers for the domain reflects only survivors. The module's handler can just diff its own configured peers against that list and prune anything missing — no need to track "removed" vs "surviving" providers the way this diff does.

Could we trim 50remove_node back to just publishing the event (as it did before), and move the provider-reconciliation logic into the OpenLDAP module's module-domain-changed handler instead?

@DavidePrincipi DavidePrincipi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants