WINA-747 Complete solution to solve startup/shutdown race condition

[iglendd]

What does this PR do?

It is the second and final part of the solution based on the first part partial solution (#25282). This PR is based on the same testing framework and its major contribution is detecting various parts of the race condition mostly based on the fact that ControlService with SERVICE_CONTROL_STOP code would fail if any dependent service is running. Accordingly, second or more iterations to stop dependent services will be required, unless total timeout is exceeded.

This will make Agent restart-service robust enough to handle race conditions when the Agent is just starting, but starting concurrently to the Agent restarting via Agent CLI command. This will not help Powershell restart-service cmdlet which would have non insignificant chances to fail if by the time core agent service is about to stop one of its dependent services is concurrently started just before attempting to stop main service.

In addition, a modest refactoring had been done to remove duplicate calls and reuse more recent Go runtime svc.ListDependentServices() function instead of handcrafted version.

In addition to the race condition handled by part #1 PR (#25282)

1. [Process A] datadogagent service is starting in the end of Agent installation (no Agent services are running at this moment)
2. [Process B] Externally and concurrently (after Agent configuration change) agent.exe restart-service CLI command is started.
3. [Process B] agent.exe restart-service CLI command will get list of datadogagent dependent services to be stopped before stopping datadogagent service itself.
4. [Process B] agent.exe restart-service CLI command enumerate all dependent services. Some of them running and some of them are not running or not running yet (depending on configuration). All running dependent services will be stopped.
5. [Process A] datadog-trace-agent service (or other dependent services) is started by the still starting datadogagent service (from step no.1).
6. [Process B] agent.exe restart-service CLI command "presumes" that all dependent services are stopped and will finally call stop for datadogagent service itself. It will fail since just before this call [Process A] started datadog-trace-agent service.
7. [PR key fix] [Process B] will recognize error condition that a dependent service or services are still running, expect that and will iterate one or more time attempting to stop dependents services until all of them are finally be stopped.

Motivation

Eliminate race condition

Additional Notes

Possible Drawbacks / Trade-offs

This PR fixes race condition for Agent restart-service CLI command. It does not address fundamental and inherited race condition to unsuspected mechanism dealing strictly with Windows services, such as Powershell restart-service cmdlet.

Describe how to test/QA your changes

No needs to manually QA because all edge case tests are automated in this PR

[buraizu]

Thanks, approving with a minor rephrasing suggested for the release notes

[pr-commenter]

Test changes on VM

Use this command from test-infra-definitions to manually test this PR changes on a VM:

inv create-vm --pipeline-id=33882450 --os-family=ubuntu

[agent-platform-auto-pr]

[Fast Unit Tests Report]

On pipeline 33882450 (CI Visibility). The following jobs did not run any unit tests:

Jobs:

• tests_deb-arm64-py3
• tests_deb-x64-py3
• tests_flavor_dogstatsd_deb-x64
• tests_flavor_heroku_deb-x64
• tests_flavor_iot_deb-x64
• tests_rpm-arm64-py3
• tests_rpm-x64-py3

If you modified Go files and expected unit tests to run in these jobs, please double check the job logs. If you think tests should have been executed reach out to #agent-developer-experience

[pducolin]

❓ question
Would it make sense to retry on error? Or errors here are not recoverable?

[iglendd]

It depends, this is low-level function. Retry exists in the callers

[pducolin]

❓ question
have you considered using out of the box retry libraries like backoff.Retry? Wonder if we should sleep between each retry to reduce CPU impact

[iglendd]

I do not think so in this case. First it is capped by timeout, currently 30 seconds - it will be short and anyway and second ability for restart to be "tight" is relatively important, Second, doStopService I think waits 300 ms on each individual service stop, external, meaning this loop is to "detect" race condition which happens and immediately try to address it.