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

feat(config): Update remove defaults #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kjkeane
Copy link

@kjkeane kjkeane commented Jun 15, 2019

To be merged after #19

This updates some of the default values to match with the current documentation here.

  • Removes NTP servers being set by default, because Chrony is a client first and server second
  • Update maxupdateskew to current defaults
  • Remove commandkey and generatecommandkey since these was deprecated
  • Update makestep to current defaults
  • Update logchange to current defaults

@aboe76 aboe76 changed the title Update remove defaults feat(config): Update remove defaults Aug 19, 2019
@aboe76
Copy link
Member

aboe76 commented Aug 19, 2019

@kjkeane You are right that chrony is a client first, that's why there aren't any allow rules in the defaults.
But chrony needs ntpservers or pool parameters otherwise it won't get ntp sync...

Copy link
Member

@aboe76 aboe76 left a comment

Choose a reason for hiding this comment

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

please see comments

chrony/defaults.yaml Show resolved Hide resolved
pillar.example Outdated
@@ -4,6 +4,11 @@ chrony:
- '1.centos.pool.ntp.org'
- '2.arch.pool.ntp.org'
- '3.gentoo.pool.ntp.org'
pool:
Copy link
Member

Choose a reason for hiding this comment

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

nice documentation added

@@ -3,22 +3,30 @@
# Your changes will be overwritten.
########################################################################

{%- if chrony.ntpservers is defined %}
Copy link
Member

Choose a reason for hiding this comment

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

this will make it cleaner thanks

@kjkeane kjkeane force-pushed the update-remove-defaults branch 2 times, most recently from b5cab2f to d2d5953 Compare August 25, 2019 00:36
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