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

check whether service exists (before stopping) #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

check whether service exists (before stopping) #1

wants to merge 1 commit into from

Conversation

chipitsine
Copy link
Contributor

No description provided.

@mattock
Copy link
Owner

mattock commented Nov 10, 2016

I assume the idea is to prevent nasty-looking errors showing up OpenVPNService and/or OpenVPNServiceLegacy are not registered as Windows services. I did some really quick testing with try-catch blocks, but those did not seem to suppress the errors. However, these seem to do the trick:

Stop-Service -Erroraction Ignore OpenVPNService
Stop-Service -Erroraction Ignore OpenVPNServiceLegacy

If this solves your problem, I can push this change to the repository. Thoughts?

@chipitsine
Copy link
Contributor Author

one part is to avoid stopping non registered service. there're several ways to achive that

  1. try/catch (powershell is strange with that, you need to specify proper -ErrorAction, otherwise, it is not caught)
  2. trap { ....}
  3. -EA Ignore
  4. check for existance

another part is that service stop is asynchronous. you send a command to service and it might not stop immediately. so, I wait for service to stop.

@mattock
Copy link
Owner

mattock commented Nov 14, 2016

Here are a couple of tricks that should force Stop-Service finish before starting anything:

Stop-Service -Erroraction Ignore OpenVPNService|Out-Null

The above would be a minimal change, and the piping trick works for external commands at least - I've used that in the past.

Another approach that should work alternatively start the service as a job and wait for the job to finish:

$job = Start-Job { Stop-Service OpenVPNService }
Wait-Job $job|Out-Null
Stop-Job $job

This would be quite readable, and could be made into a loop easily.

Then there's the rather dummy approach of just sleeping (say, 1 sec) after Stop-Service.

@chipitsine
Copy link
Contributor Author

no, when you issue Stop-Service, you actually tell a service to stop gracefully.
it does not mean it will stop right after Stop-Service is done

it might not be noticeable on simple services, but it is clearly seen, for example on sql server.

@mattock
Copy link
Owner

mattock commented Nov 14, 2016

Ok, I was probably a bit confusing. I attempted to say that the above tricks should ensure that Stop-Service finishes before any other command does anything else. Basically the tricks should emulate a synchronous Stop-Service.

@chipitsine
Copy link
Contributor Author

Stop-Service is not synchoronous.
it is not possible to emulate that

@mattock
Copy link
Owner

mattock commented Nov 14, 2016

Let's suppose, for the sake of argument, that "Stop-Service OpenVPNService" takes 10 seconds to finish. Then we run this code:

$job = Start-Job { Stop-Service OpenVPNService }
Wait-Job $job|Out-Null
Write-Host "I should run after OpenVPNService has stopped"
Stop-Job $job

Would Write-Host actually run before OpenVPNService has stopped?

@chipitsine
Copy link
Contributor Author

yes, it could happen

@mattock
Copy link
Owner

mattock commented Nov 14, 2016

Do you know of a slow-running "installed by default" asynchronous CmdLet I could do some testing with?

Also, wouldn't a "Sleep -Seconds 2" be good enough a workaround? The changes you propose are rather large in comparison.

@chipitsine
Copy link
Contributor Author

http://www.powershellmagazine.com/2013/04/10/pstip-wait-for-a-service-to-reach-a-specified-status/

it seems we can either switch to "while" (it saves extra "sleep"), or use .WaitForStatus('Stopped')

@mattock
Copy link
Owner

mattock commented Nov 14, 2016

Ah yes, sounds good. I did run through "Get-Service|Get-Member", but somehow managed to overlook the WaitForStatus method. It would definitely be more readable than the original PR.

@mattock
Copy link
Owner

mattock commented Nov 15, 2016

@chipitsine : I forked this repo to https://github.com/OpenVPN/openvpn-windows-test, and I suggest we start using it as the upstream repository. I opened a PR already, if you want to have a look:

@mattock
Copy link
Owner

mattock commented Nov 15, 2016

I created OpenVPN/openvpn-windows-test PR#2 that based on my testing solves both of the problems you identified and fixed here, just in a more readable fashion. Does that PR look good to you?

mattock added a commit that referenced this pull request Dec 19, 2016
Make openvpnserv2's openvpn respawn tests optional
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants