Added service wrapper code #6220

angrycub · 2019-08-28T18:25:51Z

This is the basic code to add the Windows Service Manager hooks to Nomad. There is still a need to add logging to file in order to fully support this feature.

endocrimes · 2019-10-28T13:02:56Z

@angrycub What's left to get this ready for review? Would be nice to ship service support in 10.1

angrycub · 2019-10-29T16:03:39Z

This looks good to go based on prelim. testing. I was able to start Nomad as a service and manage it from the native Windows tooling.

endocrimes

🎉

endocrimes · 2019-10-31T10:15:39Z

website/source/docs/configuration/index.html.md

@@ -179,6 +183,22 @@ testing.

 - `log_json` `(bool: false)` - Output logs in a JSON format.

+- `log_file` `(string: "")` - Specifies the path for logging. If the path


thanks for handling this 👍

service_os/service_windows.go

schmichael · 2019-10-31T19:31:59Z

service_os/service_windows.go

+
+func (serviceWindows) Execute(args []string, r <-chan wsvc.ChangeRequest, s chan<- wsvc.Status) (svcSpecificEC bool, exitCode uint32) {
+	const accCommands = wsvc.AcceptStop | wsvc.AcceptShutdown
+	s <- wsvc.Status{State: wsvc.StartPending}


Is this necessary since the next line sends Running? If Windows requires us sending it first we should comment to that effect.

Looking at the win32 API docs for this concept, it looks like the expectation is that services set SERVICE_START_PENDING at the very beginning, and then wait to set SERVICE_STARTED only once we're ready to receive any shutdown signals. But the way this Execute gets invoked, the change request channel already exists by the time we're called, so we're good-to-go at that point. Compare to https://docs.microsoft.com/en-us/windows/win32/services/writing-a-servicemain-function where they have to set up the stop signal handler first.

So I don't think there's any particular reason to have this line here. Removed in aa0b3f7

(That being said, all signal handling is racy during startup, including on Linux. If we get a signal before the signal handler has been created, we die without being able to gracefully shut down. The API for Windows implies they don't send you a shutdown notice until you've indicated you're ready to receive one via SERVICE_START_PENDING, but that's just from the service manager. Some other process could come along and SIGINT you and you'd be just as dead as under Unix.)

schmichael · 2019-10-31T19:35:27Z

service_os/service_windows.go

+		c := <-r
+		switch c.Cmd {
+		case wsvc.Interrogate:
+			s <- c.CurrentStatus


Any clue why Go's example service does a wacky send/sleep/send instead of just sending back CurrentStatus?! https://github.com/golang/sys/blob/master/windows/svc/example/service.go#L38-L42

From the background in https://code.google.com/archive/p/winsvc/issues/4 it looks like the comment they have here "Testing deadlock" literally means "we've left testing code in here in order to see if we can trigger this deadlock." I don't think we want to keep that.

schmichael · 2019-10-31T19:39:30Z

service_os/service_windows.go

+			s <- c.CurrentStatus
+		case wsvc.Stop, wsvc.Shutdown:
+			chanGraceExit <- 1
+			s <- wsvc.Status{State: wsvc.StopPending}


I think we should swap the order of these 2 lines to notify Windows we're intending to stop before signally to Nomad to stop. Either that or make the chan buffered so L35 doesn't block.

I agree. It looks like this was introduced in hashicorp/consul@319a0ae#diff-b58928334218b555d4fbe9d2b431a706 but there was a giant sleep in that commit, so I suspect the order wasn't well-defined.

Done in f107b52

service_os/service.go

website/source/docs/configuration/index.html.md

schmichael · 2019-10-31T19:51:04Z

website/source/docs/configuration/index.html.md

+  log should be written to before it needs to be rotated. Must be a duration
+  value such as 30s.
+
+- `log_rotate_max_files` `(int: 0)` - Specifies the maximum number of older log


Hm, I totally didn't think about the defaults of these from #6429 -- I filed #6603 as a followup

website/source/guides/install/windows-service.html.md

website/source/layouts/guides.erb

tgross · 2019-11-06T20:52:58Z

I've rebased this on master and I think I've addressed the open code review comments. Next I'm going to take it for a spin on the Windows e2e infra to verify it still works.

tgross · 2019-11-08T15:36:51Z

Success!

PS C:\Users\Administrator> sc.exe query Nomad

SERVICE_NAME: Nomad
        TYPE               : 10  WIN32_OWN_PROCESS
        STATE              : 4  RUNNING
                                (STOPPABLE, NOT_PAUSABLE, ACCEPTS_SHUTDOWN)
        WIN32_EXIT_CODE    : 0  (0x0)
        SERVICE_EXIT_CODE  : 0  (0x0)
        CHECKPOINT         : 0x0
        WAIT_HINT          : 0x0

tgross · 2019-11-08T16:05:47Z

@schmichael this was previously approved by @endocrimes but you had some open questions on it, so I want to make sure you're happy with my answers before we merge this.

schmichael

Just a couple unimportant nitpicks. Feel free to merge whenever. Thanks @angrycub and @tgross!

helper/winsvc/service_windows.go

helper/winsvc/service.go

tgross · 2019-11-11T18:29:33Z

Just a couple unimportant nitpicks. Feel free to merge whenever.

👍 I'll pick those up as well as the CHANGELOG conflict.

* guide for installing as a windows service. * configuration for logging to file from PR #6429

* fix layout on guide for sidebar * link downloads * correct md formatting for links inside of monospace * shorten title

tgross · 2019-11-11T19:06:25Z

I've made those updates and rebased on master. Once the tests pass I'll squash-and-merge this.

github-actions · 2023-01-21T02:16:18Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

angrycub changed the base branch from master to 0.8.7 September 24, 2019 17:41

angrycub changed the base branch from 0.8.7 to master September 24, 2019 17:41

angrycub mentioned this pull request Sep 24, 2019

Support running as a Windows Service #6372

Closed

2 tasks

angrycub force-pushed the f-win-service branch from 258c310 to 7c27b45 Compare October 14, 2019 14:51

angrycub force-pushed the f-win-service branch from 7c27b45 to 460178b Compare October 23, 2019 20:52

angrycub marked this pull request as ready for review October 29, 2019 19:52

angrycub requested review from schmichael and endocrimes October 29, 2019 19:52

endocrimes approved these changes Oct 31, 2019

View reviewed changes

schmichael reviewed Nov 1, 2019

View reviewed changes

tgross self-assigned this Nov 6, 2019

tgross force-pushed the f-win-service branch from 94e302f to f107b52 Compare November 6, 2019 20:20

tgross force-pushed the f-win-service branch from aa0b3f7 to b2e283f Compare November 7, 2019 16:30

tgross added this to the near-term milestone Nov 7, 2019

tgross force-pushed the f-win-service branch from 2e4d85f to e5156da Compare November 11, 2019 15:24

schmichael approved these changes Nov 11, 2019

View reviewed changes

helper/winsvc/service_windows.go Outdated Show resolved Hide resolved

helper/winsvc/service.go Outdated Show resolved Hide resolved

angrycub and others added 8 commits November 11, 2019 14:05

Added service wrapper code

20b8448

Vendored golang.org/x/sys/windows/svc

c1af8f6

Added Docs

42c9b1d

* guide for installing as a windows service. * configuration for logging to file from PR #6429

Incorporate review feedback from @schmichael

698d538

* fix layout on guide for sidebar * link downloads * correct md formatting for links inside of monospace * shorten title

rename package

0bb81e4

send StopPending to windows before sending graceful exit

54e2f60

removed extra start pending signal

a7b4874

Fix changelog

f2a4c41

tgross force-pushed the f-win-service branch from 87858d0 to 5b5f7ce Compare November 11, 2019 19:06

fixup naming

608f4da

tgross force-pushed the f-win-service branch from 5b5f7ce to 608f4da Compare November 11, 2019 19:34

tgross merged commit 1ec6388 into master Nov 11, 2019

tgross deleted the f-win-service branch November 11, 2019 20:16

tgross removed this from the near-term milestone Jan 9, 2020

github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added service wrapper code #6220

Added service wrapper code #6220

angrycub commented Aug 28, 2019

endocrimes commented Oct 28, 2019

angrycub commented Oct 29, 2019

endocrimes left a comment

endocrimes Oct 31, 2019

schmichael Oct 31, 2019

tgross Nov 6, 2019

schmichael Oct 31, 2019

tgross Nov 6, 2019

schmichael Oct 31, 2019

tgross Nov 6, 2019

tgross Nov 6, 2019 •

edited

Loading

schmichael Oct 31, 2019

tgross commented Nov 6, 2019

tgross commented Nov 8, 2019

tgross commented Nov 8, 2019

schmichael left a comment

tgross commented Nov 11, 2019

tgross commented Nov 11, 2019

github-actions bot commented Jan 21, 2023

		@@ -179,6 +183,22 @@ testing.

		- `log_json` `(bool: false)` - Output logs in a JSON format.

		- `log_file` `(string: "")` - Specifies the path for logging. If the path

Added service wrapper code #6220

Added service wrapper code #6220

Conversation

angrycub commented Aug 28, 2019

endocrimes commented Oct 28, 2019

angrycub commented Oct 29, 2019

endocrimes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgross Nov 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgross commented Nov 6, 2019

tgross commented Nov 8, 2019

tgross commented Nov 8, 2019

schmichael left a comment

Choose a reason for hiding this comment

tgross commented Nov 11, 2019

tgross commented Nov 11, 2019

github-actions bot commented Jan 21, 2023

tgross Nov 6, 2019 •

edited

Loading