KEMBAR78
Improve installer (Windows) by thiagoftsm · Pull Request #21090 · netdata/netdata · GitHub
Skip to content

Conversation

@thiagoftsm
Copy link
Contributor

@thiagoftsm thiagoftsm commented Oct 3, 2025

Summary

The Netdata installer failed to stop the Netdata service during installation. It also did not raise errors or provide feedback about this failure.

This PR addresses these issues by modifying the service management mechanism.

Test Plan
  1. Install Netdata version 2.7.1.
  2. Compile this branch.
  3. Run the installer. You will receive a warning, which is expected. For the specific features we are working on, ServiceConfig functions correctly.
  4. Open Task Manager and search for netdata.
  5. Install this branch. You will now observe that the installer displays warnings and successfully terminates the Netdata software.
  6. Let Netdata run for a short while, and then run the installer again. You will see that it is faster now.
Additional Information
For users: How does this change affect me?

@github-actions github-actions bot added the area/packaging Packaging and operating systems support label Oct 3, 2025
@thiagoftsm thiagoftsm marked this pull request as ready for review October 3, 2025 04:33
@thiagoftsm thiagoftsm requested a review from a team as a code owner October 3, 2025 04:33
@thiagoftsm thiagoftsm requested a review from stelfrag October 3, 2025 04:33
@ilyam8 ilyam8 requested a review from Copilot October 3, 2025 05:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Improves the Windows installer's service management by fixing issues where the Netdata service failed to stop properly during installation and provided no feedback about failures.

  • Replaced unreliable net stop command with process termination using taskkill.exe
  • Added service recovery configuration to automatically restart on failures
  • Refactored service control to use separate install/uninstall actions with proper timing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +247 to +250
ResetPeriodInDays="0"
FirstFailureActionType="restart"
SecondFailureActionType="restart"
ThirdFailureActionType="restart"
Copy link

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

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

The service recovery configuration has aggressive restart behavior with no reset period. Consider adding a reasonable reset period (e.g., 1 day) and escalating failure actions (e.g., restart -> restart -> none) to prevent infinite restart loops if the service consistently fails.

Suggested change
ResetPeriodInDays="0"
FirstFailureActionType="restart"
SecondFailureActionType="restart"
ThirdFailureActionType="restart"
ResetPeriodInDays="1"
FirstFailureActionType="restart"
SecondFailureActionType="restart"
ThirdFailureActionType="none"

Copilot uses AI. Check for mistakes.

/>
</ServiceInstall>

<ServiceControl Id="StartService" Name="Netdata" Start="install" Wait="no" />
Copy link

Copilot AI Oct 3, 2025

Choose a reason for hiding this comment

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

Setting Wait="no" for service start could lead to race conditions where subsequent installation steps might execute before the service is fully started. Consider using Wait="yes" to ensure proper sequencing.

Suggested change
<ServiceControl Id="StartService" Name="Netdata" Start="install" Wait="no" />
<ServiceControl Id="StartService" Name="Netdata" Start="install" Wait="yes" />

Copilot uses AI. Check for mistakes.

@thiagoftsm thiagoftsm merged commit 1d9ca4d into netdata:master Oct 3, 2025
106 checks passed
@thiagoftsm thiagoftsm deleted the fix_service_installer branch October 3, 2025 11:50
stelfrag pushed a commit to stelfrag/netdata that referenced this pull request Oct 3, 2025
@stelfrag stelfrag mentioned this pull request Oct 3, 2025
Ferroin pushed a commit that referenced this pull request Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/packaging Packaging and operating systems support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants