1. Lack of Proper Error Handling
• Original Issue: Many functions did not properly handle or return errors. For example, getArgsFromPid, isInteractive, and Uninstall didn’t adequately report issues.
• Fix: Ensured all functions return errors when necessary and added descriptive error messages.
2. Missing or Inconsistent Formatting
• Original Issue: Code formatting was inconsistent, particularly around template definitions, string concatenation, and variable declarations.
• Fix: Improved formatting for readability and consistency.
3. Unsafe Regex Handling
• Original Issue: Service name strings were directly embedded in regular expressions, which could cause issues with special characters.
• Fix: Used regexp.QuoteMeta to escape service names safely.
4. Incorrect Status Parsing
• Original Issue: Parsing the lssrc output for service status relied on weak assumptions and could result in incorrect matches.
• Fix: Improved the regex and explicitly handled unknown statuses with an appropriate error.
5. Overlooked File and Path Errors
• Original Issue: File path checks and operations like os.Stat or os.Remove didn’t always handle errors or check preconditions properly.
• Fix: Added error checks to ensure paths and files are correctly verified before performing actions like creation or deletion.
6. Race Condition in Restart Logic
• Original Issue: The Restart method had no delay between Stop and Start, potentially causing race conditions.
• Fix: Introduced a short sleep (50ms) to ensure the stop operation completes before starting.
7. Poor Handling of Interactive Mode
• Original Issue: Interactive mode detection lacked robustness and could panic on errors.
• Fix: Improved the logic in isInteractive and wrapped it in a more descriptive error message.
8. Missing Default Logger for Interactive Mode
• Original Issue: The Logger method didn’t properly handle interactive environments in a structured way.
• Fix: Added a fallback to ConsoleLogger for interactive environments.
9. Hardcoded Paths and Symlink Errors
• Original Issue: The script creation logic for /etc/rc and symlinks lacked checks for existing files and didn’t handle errors in creating symlinks robustly.
• Fix: Enhanced error handling for path operations and skipped symlinks on failure without interrupting the process.
10. Template Logic Issues
• Original Issue: The template method didn’t handle cases where optionSysvScript was unset properly.
• Fix: Added a fallback to default configuration when no custom template was provided.
11. Miscellaneous Issues
• Incorrect Case Logic: Fixed missing * cases in shell script templates to ensure the default behavior is handled.
• Magic Numbers: Replaced arbitrary values with meaningful comments and constants where appropriate.
* validate systemctl command
* validate systemctl command
* add rcS support
* only attempt to set environment if list is not empty
* check if service bin exists
* Update XML prolog and DTD
Also converts single-quotes to double-quotes
* Remove whitespace from parent dict
* Only populate EnvironmentVariables when available
* Only add additional arguments when defined
Also cleans up whitespace around each argument
* Cleanup whitespace and only populate when defined
* Convert remaining spaces to tabs
* Sort keys. Similar to `plutil -convert xml1`
Using the service manager from an remote ssh command promt fails
with
`The service process could not connect to the service controller`
Thanks to the detailed analysis from @kelseyma the fix was very straight
forward.
Using IsWindowsService() solved this problem for me.
The mentioned issue at https://github.com/golang/go/issues/44921
has also been fixed in the meantime, so there is no reason not to
use IsWindowsService() instead.
Fixes#300
Doing so will create services that are broken and will fail
when started:
`Error 87: The parameter is incorrect`
This is a regression from https://github.com/kardianos/service/pull/312
* fix for the openrc status func
Signed-off-by: Karen Almog <kalmog@mirantis.com>
* openrc: exitCode parsing for better status
Signed-off-by: Karen Almog <kalmog@mirantis.com>
The library is using inconsistent names when calling `systemctl`
commands.
Let's say that the service is named `name`. The unit file will be
then named `name.service` - this is the popular pattern used also by
this library.
While for most common commands like `systemctl start` there is no
difference whether `name` or `name.service` will be used as the service
identifier, the `list-unit-files` command expects explicitly the unit
file name.
Currently just the name - without the `.service` suffix - is being used,
which causes the command to return no output, not match the name of the
service and finally respond the status of installed but inactive service
as `StatusUnknown` with `ErrNotInstalled` error. While `StatusStopped`
without any error is expected.
Considering `systemctl` behavior it's just easier to:
- have one method that constructs the `name.service` name,
- use it for all `systemctl` commands calls.
And this is the change that this commit is adding.
* Shutdowner interface and Windows trigger
* Updating Shutdown doc
Stop won't be called when Shutdown is.
Co-authored-by: Juan Hernandez <jhernandez@newrelic.com>
* service(windows): added option flag for delayed automatic start
* service(windows): added option flag for delayed automatic start
Co-authored-by: Utkarsh Dixit <utkarsh.dixit@siemens.com>
