Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/dedicated machine account for cluster #8205

Merged
merged 8 commits into from
Jul 16, 2024

Conversation

stgmsa
Copy link
Contributor

@stgmsa stgmsa commented Jul 3, 2024

Cluster support for domain.conf

  • allow PacketFence nodes in a cluster have their own dedicated settings in domain.conf
  • adds HA support when establishing secure connections.

Impacts

  • changed the domain.conf structure - For now the section name will always be hostname domain_id
  • changed the steps when init secure connection - perform a dns query and obtain all available domain controllers, choose one to connect.

Delete branch after merge

YES

Checklist

  • Document the feature
  • Add OpenAPI specification
  • Add unit tests
  • Add acceptance tests (TestLink)

Enhancements

  • allow different settings in domain.conf for each nodes in a cluster. e.g., each node has its own machine account name and password.
  • supports domain controller high availibity now with a SRV query of DNS

@stgmsa stgmsa requested a review from fdurand July 3, 2024 17:47
@satkunas satkunas added this to the PacketFence-14.0 milestone Jul 8, 2024
addons/upgrade/to-14.0-update-domain-config-section.pl Outdated Show resolved Hide resolved
lib/pf/UnifiedApi/Controller/Config/Domains.pm Outdated Show resolved Hide resolved
bin/pyntlm_auth/utils.py Outdated Show resolved Hide resolved
lib/pf/UnifiedApi/Controller/Config/Domains.pm Outdated Show resolved Hide resolved
lib/pf/UnifiedApi/Controller/Config/Domains.pm Outdated Show resolved Hide resolved
@stgmsa stgmsa self-assigned this Jul 11, 2024
@stgmsa stgmsa force-pushed the fix/dedicated-machine-account-for-cluster branch from 59b0d07 to d32a2b9 Compare July 15, 2024 19:41
@satkunas satkunas merged commit 565fab6 into devel Jul 16, 2024
13 checks passed
@leonardosecci
Copy link

Hello,

I have seen the changes made, but in my opinion, they could create problems in the event that a domain controller becomes unavailable.
In fact, if such a scenario occurs, it would be queried cyclically, generating an error.
This scenario is not uncommon in a production environment, as domain controllers are often taken down for maintenance, but this is not evident just by reading the SRV record from DNS.
For example, Samba, using the net.finddc method, first retrieves the list of domain controllers reading SRV record but returns the information of the first one that passes the operational check.

Another issue that concerns me is the performance in a production environment.
I presume that because Samba is not declared compatible with multi-threading, you have implemented a lock that, in addition to atomicizing access to global variables, effectively serializes all requests.
Given my limited knowledge of Samba, it seems that there could be compatibility with a multi-process architecture. Wouldn't it be simpler to integrate a pre-fork system, similar to Gunicorn?

Best regards

@satkunas
Copy link
Contributor

@leonardosecci thank you for your feedback

We currently handle fail-over, and LB - which isn't useful due to SMB single-thread.

We have multi-threading on our road-map that will monitor alive/dead of all hosts from SRV DNS and manage a pool used for multi-threaded LB.

@leonardosecci
Copy link

Thank you @satkunas, do you have an idea when the update that will monitor the health of domain controllers and for multi-threading will be released?

Best regards

@satkunas
Copy link
Contributor

No ETA, but we are likely to continue to prioritizing work on Domains in the future

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

Successfully merging this pull request may close these issues.

4 participants