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.

Sign up for GitHub

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

Jump to bottom

Windows service handling improvements #319

Closed
mpfz0r opened this issue Dec 12, 2018 · 2 comments
Closed

Windows service handling improvements #319

mpfz0r opened this issue Dec 12, 2018 · 2 comments
Labels
bug triaged
Milestone
3.0.0

Comments

@mpfz0r
Copy link
Contributor

mpfz0r commented Dec 12, 2018

We need to think about how users are supposed to upgrade their sidecar to the
new version.

Right now a user cannot simply install the new sidecar (1.0) over an existing
older installation.
One problem is that the windows service name got changed, but not the
Display name.
That's why the new sidecar is not able to register itself as a service,
nor is it able to uninstall the old sidecar's service.

There are probably more issues, we can add here...
@mpfz0r mpfz0r added this to the 3.0.0 milestone Dec 12, 2018
@bernd
Copy link
Member

bernd commented Dec 12, 2018

@mpfz0r Can we change the Display name?

@mpfz0r
Copy link
Contributor Author

mpfz0r commented Dec 13, 2018 •
edited
Loading

@mpfz0r Can we change the Display name?

I think that would improve things slightly.
However, I just noticed a similar problem if you rename a collector:

time="2018-12-13T05:17:37-08:00" level=info msg="Adding process runner for: nxlog-windows-old" 
time="2018-12-13T05:17:37-08:00" level=info msg="[nxlog-windows-old] Configuration change detected, rewriting configuration file." 
time="2018-12-13T05:17:37-08:00" level=error msg="[nxlog-windows-old] Can't get status of service graylog-collector-nxlog-windows-old cause it doesn't exist: The specified service does not exist as an installed service." 
time="2018-12-13T05:17:37-08:00" level=error msg="[nxlog-windows-old] Failed to install service: The name is already in use as either a service name or a service display name." 
panic: runtime error: invalid memory address or nil pointer dereference
        panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x10 pc=0x52c459]
goroutine 8 [running]:
github.com/Graylog2/collector-sidecar/vendor/golang.org/x/sys/windows/svc/mgr.(*Service).Close(0x0, 0xc042016d50, 0x1)
        /home/mpf/go/src/github.com/Graylog2/collector-sidecar/vendor/golang.org/x/sys/windows/svc/mgr/service.go:33 +0x29
panic(0x782780, 0x9f5980)
        /usr/lib/go-1.10/src/runtime/panic.go:502 +0x237
github.com/Graylog2/collector-sidecar/vendor/golang.org/x/sys/windows/svc/mgr.(*Service).Delete(0x0, 0x23, 0x7)
        /home/mpf/go/src/github.com/Graylog2/collector-sidecar/vendor/golang.org/x/sys/windows/svc/mgr/service.go:28 +0x29
github.com/Graylog2/collector-sidecar/daemon.(*SvcRunner).ValidateBeforeStart(0xc0422d4000, 0x0, 0x0)
        /home/mpf/go/src/github.com/Graylog2/collector-sidecar/daemon/svc_runner_windows.go:169 +0x634
github.com/Graylog2/collector-sidecar/daemon.(*SvcRunner).start(0xc0422d4000, 0x0, 0x0)
        /home/mpf/go/src/github.com/Graylog2/collector-sidecar/daemon/svc_runner_windows.go:200 +0x52                                                                                                                                                                 
github.com/Graylog2/collector-sidecar/daemon.(*SvcRunner).restart(0xc0422d4000, 0x7f7715, 0x2c)                                                                                                                                                                       
        /home/mpf/go/src/github.com/Graylog2/collector-sidecar/daemon/svc_runner_windows.go:289 +0x137                                                                                                                                                                
github.com/Graylog2/collector-sidecar/daemon.(*SvcRunner).signalProcessor.func1(0xc0422d4000)                                                                                                                                                                         
        /home/mpf/go/src/github.com/Graylog2/collector-sidecar/daemon/svc_runner_windows.go:304 +0x2c4                                                                                                                                                                
created by github.com/Graylog2/collector-sidecar/daemon.(*SvcRunner).signalProcessor                                                                                                                                                                                  
        /home/mpf/go/src/github.com/Graylog2/collector-sidecar/daemon/svc_runner_windows.go:296 +0x46

@mpfz0r mpfz0r changed the title Windows upgrade path Windows service handling improvements Dec 19, 2018
mpfz0r added a commit that referenced this issue Dec 19, 2018
@mpfz0r
Fix and improve Windows service handling
82da916 
This change addresses several issues how we handle windows
services:
 - Change the DisplayName of the Sidecar itself so it does not conflict
   with old sidecars.
 - If a sidecar backend (aka collector) gets renamed, update the
   DisplayName to avoid a conflict with the existing service.
 - If backends get renamed, or deleted while the sidecar isn't running
   there was no way we could cleanup those.
   Therefore update golang/x/sys to a newer version that supports
   `ListServices()` so we can search for stale registered services
   and clean those.
 - Improve error handling in `ValidateBeforeStart()` which could
   lead to a panic if a service could not be created.

Fixes #319
@mpfz0r mpfz0r mentioned this issue Dec 19, 2018
Merged
mariussturm pushed a commit that referenced this issue Dec 21, 2018
@mpfz0r
Fix and improve Windows service handling (#321)
acfcc9b 
* Fix and improve Windows service handling

This change addresses several issues how we handle windows
services:
 - Change the DisplayName of the Sidecar itself so it does not conflict
   with old sidecars.
 - If a sidecar backend (aka collector) gets renamed, update the
   DisplayName to avoid a conflict with the existing service.
 - If backends get renamed, or deleted while the sidecar isn't running
   there was no way we could cleanup those.
   Therefore update golang/x/sys to a newer version that supports
   `ListServices()` so we can search for stale registered services
   and clean those.
 - Improve error handling in `ValidateBeforeStart()` which could
   lead to a panic if a service could not be created.

Fixes #319

* Fix commandline parsing for Windows

On Windows, use CommandLineToArgv() to run foreground processes.
In the previous change I've only used it to run validation commands.

With this change it is possible to run collectors
without the need for services on Windows.

Fixes #290  (2nd fix)

* Fix intial Windows service startup

`CreateService` would not create the service with command line
arguments.  Run an immediate `UpdateConfig` afterwards to
fixup the `BinaryPathName`.

Also remove the unecessarily verbose error message when
we just probe whether a service is running.
It's ok if it does not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug triaged
Projects
None yet
Milestone
3.0.0
Development

No branches or pull requests
3 participants
@bernd @jalogisch @mpfz0r