Skip to content

Conversation

@parmstro
Copy link

@parmstro parmstro commented Sep 15, 2025

tested with 4 replica environment:
existing master
existing replica
uninstalled replica
new replica

Summary by Sourcery

Bug Fixes:

  • Use 'not options.no_ntp' in NTP configuration check to avoid blocking installations when NTP is explicitly disabled

…all configurations. Adding "not options.no_ntp" ensures replicas install when explicitly telling role not to configure ntp.

tested with 4 replica environment:
existing master
existing replica
uninstalled replica
new replica
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@t-woerner
Copy link
Member

Hello, thanks for the PR.
Please explain what this change is fixing.
The NTP configuration can not be touched on a pre-installed client. There is no way to turn off ntp if it was enabled for the client before.

@parmstro
Copy link
Author

parmstro commented Sep 16, 2025 via email

@t-woerner
Copy link
Member

FreeIPA is not supporting to turn off previously enabled features for clients and servers. The deployment roles for client, replica and server in ansible-freeipa are using the FreeIPA code and therefore they are following that rule.

If ntp or any other client feature was enabled or configured for a client and later on the client is promoted to become a server, there is no way to deactivate any of the features. Therefore there is a check if any ipaclient setting is given for the replica promotion.

For no_ntp it might work to ignore it in this case, but what do we do with the other ntp or ipaclient settings? The ipareplica role is not able to change any client settings in the "client is already deployed" case.

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.

2 participants