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

Move the call to reload ntp settings to the server only #14208

Merged
merged 1 commit into from
Mar 7, 2017

Conversation

carbonin
Copy link
Member

@carbonin carbonin commented Mar 6, 2017

Previously, when the ntp settings were changed every worker would enqueue a message for the server to reload the settings when they went through the Config::Activator.

Multiple calls to restart chronyd within a matter of seconds causes systemd to back off and fail the unit causing chronyd not to run until the appliance is rebooted or it is started manually.

This change makes the server reload the ntp settings every time its config is changed, but removes the call to reload from the activator which removes all the calls generated when the workers refresh their settings. This results in just one call to reload the settings when the config is changed rather than tens.

https://bugzilla.redhat.com/show_bug.cgi?id=1429714

Previously, when the ntp settings were changed every worker would
enqueue a message for the server to reload the settings when they
went through the Config::Activator.

Multiple calls to restart chronyd within a matter of seconds causes
systemd to backoff and fail the unit causing chronyd not to run until
the appliance is rebooted or it is started manually.

This change makes the server reload the ntp settings every time its
config is changed, but removes the call to reload from the activator
which removes all the calls generated when the workers refresh their
settings. This results in just one call to reload the settings when
the config is changed rather than tens.
@miq-bot
Copy link
Member

miq-bot commented Mar 6, 2017

Checked commit carbonin@939524f with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🏆

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

👍 Looks good. Tested this yesterday with @carbonin

@gtanzillo gtanzillo added this to the Sprint 56 Ending Mar 13, 2017 milestone Mar 7, 2017
@gtanzillo gtanzillo merged commit 515b4a0 into ManageIQ:master Mar 7, 2017
simaishi pushed a commit that referenced this pull request Mar 7, 2017
Move the call to reload ntp settings to the server only
(cherry picked from commit 515b4a0)

https://bugzilla.redhat.com/show_bug.cgi?id=1429999
@simaishi
Copy link
Contributor

simaishi commented Mar 7, 2017

Euwe backport details:

$ git log -1
commit fa7e8750e688ff73ca5863636924cf40131c9dcd
Author: Gregg Tanzillo <[email protected]>
Date:   Tue Mar 7 10:27:46 2017 -0500

    Merge pull request #14208 from carbonin/fix_chrony_start_issue
    
    Move the call to reload ntp settings to the server only
    (cherry picked from commit 515b4a0c9d769330e749fbc0a3cef48f98d8ac70)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1429999

@carbonin carbonin deleted the fix_chrony_start_issue branch March 7, 2017 16:30
Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

Very nice. I believe set_config is called throughout the UI, although I don't think you have to in the backend. This should be good enough to solve this problem.

agrare pushed a commit to agrare/manageiq that referenced this pull request Apr 19, 2017
Move the call to reload ntp settings to the server only
(cherry picked from commit 515b4a0)

https://bugzilla.redhat.com/show_bug.cgi?id=1429999
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.

5 participants