(#21641) Windows puppet service should log to the eventlog #2025
Conversation
…t need to be fixed.
Cleaned up some whitespace issues Changed the wording on some of the log entries to avoid confusion between 'Service resuming' and a Service Resume control message Added logging for the Pause, Resume and Shutdown service control messages. Previously there was none Noted Bug #11 in the win32-service ruby code Added a commented out handler for the Interrogation event for debuggin purposes Changed the name of the event log function from raise_windows_event to report_windows_event to avoid connotations with raise Changed the event source to Puppet so that the correct event source DLL is used. However it may be a good idea to split the event source between the service and the actual agent so it's easier to read in the Eventlog. This would require a change to the puppet_for_the_win repo as well. The eventlog.close command is within an ensure block to stop leaked handles if an error occurs
The --logtofile argument was working for the service but was causing the puppet agent run to fail as it didn't know what to do with it. Used a simple text substitution to remove it from the args string.
|
Hi, you have one commit with a message that does not conform to the policy (a3fb129). A small issue; we like the one line commit message to be phrased in a manner that it describes what the commit does to the code as if you think "This commit when merged will..." before your sentence - i.e. start with a word like "Fix", "Add", "Correct". Your longer commit text explanations are fine. |
| begin | ||
| eventlog = Win32::EventLog.open("Application") | ||
| eventlog.report_event( | ||
| :source => "Puppet", |
There was a problem hiding this comment.
formatting off by a space (being picky...)
|
I can't retract my commit messages so I guess you would need me to close this Pull Request, refork, commit and raise another pull request? |
|
@glennsarti that would be fine. It's to what level you feel comfortable. If you are super comfortable with git and rebasing, you can update the original PR (pull request) with the newest stuff. This is done with a combination of rebase and push -f. Be aware that this does require a high level of comfort as you would effectively be rewriting your git history and could potentially lose some code. NOTE: Also push with force (-f) would never be done against a code base that others were forking or watching, like puppetlabs/puppet, but in my case I do with ferventcoder/puppet as I'm fixing up PRs because it is not the official repository. |
|
I'm not that comfortable with Git yet and it's only 1 file with 3 commits I can recreate easily. I'm closing this pull request and creating a new one |
|
Raised Pull Request 2161 for these changes. |
The windows service code (daemon.rb) logs to a file, since that was implemented prior to eventlog support. Now that support has been added, the daemon code should use it.
The service code has been modified so that any log_notice, log_debug etc. call will log the event to the Windows Application Event log (assuming it is of the right level) and optionally, it can log to the windows.log file as it was previously. The optional text logging is there for legacy reasons or in case the event log code is broken, then log files can still be generated. To use the log file method, the --logtofile service start argument should be set e.g. sc.exe start pe-puppet --logtofile --debug
Some of the wording of the logging messages has been changed e.g. removing ambiguity about the service resuming and additional logging for the Pause, Continue and Shutdown service control messages. Previously there was no logging for these events.
There is an issue with the win32-service gem regarding services coming out of Paused state and is fixed in version 0.8.3 (See chef/win32-service#11). I am not sure which version Puppet uses, but in the 3.0.1 Puppet Enterprise Windows MSI, v0.7.2 is used. While I could write code to work around this issue, I decided that it is not warranted as most people never put Puppet into a Paused state.