Skip to content

Commit

Permalink
Fix and improve Windows service handling (#321)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
mpfz0r authored and Marius Sturm committed Dec 21, 2018
1 parent 5e1aead commit acfcc9b
Show file tree
Hide file tree
Showing 7 changed files with 171 additions and 78 deletions.
15 changes: 11 additions & 4 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewConfig() *DaemonConfig {

dc := &DaemonConfig{
Name: "graylog-sidecar",
DisplayName: "Graylog collector sidecar",
DisplayName: "Graylog Sidecar",
Description: "Wrapper service for Graylog controlled collector",
Dir: rootDir,
Env: []string{},
Expand Down Expand Up @@ -130,11 +130,18 @@ func (dc *DaemonConfig) SyncWithAssignments(configChecksums map[string]string, c
configChecksums[backend.Id] = ""
}
}

// add new backends to registry
assignedBackends := []*backends.Backend{}
for backendId := range assignments.Store.GetAll() {
backend := backends.Store.GetBackendById(backendId)
if backend != nil && dc.Runner[backend.Id] == nil {
if backend != nil {
assignedBackends = append(assignedBackends, backend)
}
}
CleanOldServices(assignedBackends)

// add new backends to registry
for _, backend := range assignedBackends {
if dc.Runner[backend.Id] == nil {
log.Info("Adding process runner for: " + backend.Name)
dc.AddRunner(*backend, context)
}
Expand Down
8 changes: 7 additions & 1 deletion daemon/exec_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,13 @@ func (r *ExecRunner) start() error {
}

// setup process environment
quotedArgs, err := shlex.Split(r.args)
var err error
var quotedArgs []string
if runtime.GOOS == "windows" {
quotedArgs = common.CommandLineToArgv(r.args)
} else {
quotedArgs, err = shlex.Split(r.args)
}
if err != nil {
return err
}
Expand Down
11 changes: 11 additions & 0 deletions daemon/svc_helper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// +build !windows

package daemon

import (
"github.com/Graylog2/collector-sidecar/backends"
)

// Dummy function. Only used on Windows
func CleanOldServices(assignedBackends []*backends.Backend) {
}
113 changes: 113 additions & 0 deletions daemon/svc_helper_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package daemon

import (
"fmt"
"github.com/Graylog2/collector-sidecar/backends"
"golang.org/x/sys/windows/svc"
"golang.org/x/sys/windows/svc/eventlog"
"golang.org/x/sys/windows/svc/mgr"
"strings"
"time"
)

func CleanOldServices(assignedBackends []*backends.Backend) {
registeredServices, err := getRegisteredServices()
if err != nil {
log.Warn("Failed to get registered services. Skipping cleanup. ", err)
return
}
for _, service := range registeredServices {
if strings.Contains(service, ServiceNamePrefix) {
log.Debugf("Found graylog service %s", service)
if !serviceIsAssigned(assignedBackends, service) {
log.Infof("Removing stale graylog service %s", service)
uninstallService(service)
}
}
}
}

func getRegisteredServices() ([]string, error) {
m, err := mgr.Connect()
if err != nil {
return nil, err
}
defer m.Disconnect()
registeredServices, err := m.ListServices()
if err != nil {
return nil, err
}
return registeredServices, nil
}

func serviceIsAssigned(assignedBackends []*backends.Backend, serviceName string) bool {
for _, backend := range assignedBackends {
if backend.ServiceType == "svc" && ServiceNamePrefix+backend.Name == serviceName {
return true
}
}
return false
}

func uninstallService(name string) {
log.Infof("Uninstalling service %s", name)
stopService(name)

m, err := mgr.Connect()
if err != nil {
log.Errorf("Failed to connect to service manager: %v", err)
return
}
defer m.Disconnect()

s, err := m.OpenService(name)
// service exist so we try to uninstall it
if err != nil {
log.Debugf("Service %s doesn't exist, no uninstall action needed", name)
return
}

defer s.Close()
err = s.Delete()
if err != nil {
log.Errorf("Can't delete service %s: %v", name, err)
}

err = eventlog.Remove(s.Name)
if err != nil {
log.Errorf("RemoveEventLogSource() failed: %s", err)
}
return
}

func stopService(serviceName string) error {
m, err := mgr.Connect()
if err != nil {
return fmt.Errorf("Failed to connect to service manager: %v", err)
}
defer m.Disconnect()

ws, err := m.OpenService(serviceName)
if err != nil {
return fmt.Errorf("Could not access service %s: %v", serviceName, err)
}
defer ws.Close()

status, err := ws.Control(svc.Stop)
if err != nil {
return fmt.Errorf("Could not send stop control: %v", err)
}

timeout := time.Now().Add(10 * time.Second)
for status.State != svc.Stopped {
if timeout.Before(time.Now()) {
return fmt.Errorf("Timeout waiting for service to go to stopped state: %v", err)
}
time.Sleep(300 * time.Millisecond)
status, err = ws.Query()
if err != nil {
return fmt.Errorf("Could not retrieve service status: %v", err)
}
}
return nil
}
62 changes: 24 additions & 38 deletions daemon/svc_runner_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
"github.com/Graylog2/collector-sidecar/context"
)

const ServiceNamePrefix = "graylog-collector-"

type SvcRunner struct {
RunnerCommon
exec string
Expand All @@ -54,7 +56,7 @@ func NewSvcRunner(backend backends.Backend, context *context.Ctx) Runner {
exec: backend.ExecutablePath,
args: backend.ExecuteParameters,
signals: make(chan string),
serviceName: "graylog-collector-" + backend.Name,
serviceName: ServiceNamePrefix + backend.Name,
}

// set default state
Expand All @@ -79,16 +81,15 @@ func (r *SvcRunner) Running() bool {
defer m.Disconnect()

s, err := m.OpenService(r.serviceName)
// service exist so we only update the properties
if err != nil {
r.backend.SetStatusLogErrorf("Can't get status of service %s cause it doesn't exist: %v", r.serviceName, err)
return false
}
defer s.Close()

status, err := s.Query()
if err != nil {
r.backend.SetStatusLogErrorf("Can't query status of service %s: %v", r.serviceName, err)
return false
}

return status.State == svc.Running
Expand All @@ -113,6 +114,7 @@ func (r *SvcRunner) GetBackend() *backends.Backend {
func (r *SvcRunner) SetBackend(b backends.Backend) {
r.backend = b
r.name = b.Name
r.serviceName = ServiceNamePrefix + b.Name
r.exec = b.ExecutablePath
r.args = b.ExecuteParameters
}
Expand All @@ -124,8 +126,7 @@ func (r *SvcRunner) ValidateBeforeStart() error {
return err
}

execPath, err := exec.LookPath(r.exec)
if err != nil {
if _, err := exec.LookPath(r.exec); err != nil {
return r.backend.SetStatusLogErrorf("Failed to find collector executable %s", r.exec)
}

Expand All @@ -137,14 +138,14 @@ func (r *SvcRunner) ValidateBeforeStart() error {

serviceConfig := mgr.Config{
DisplayName: "Graylog collector sidecar - " + r.name + " backend",
Description: "Wrapper service for the NXLog backend",
Description: "Wrapper service for the " + r.name + " backend",
BinaryPathName: "\"" + r.exec + "\" " + r.args}

s, err := m.OpenService(r.serviceName)
// service exist so we only update the properties
if err == nil {
defer s.Close()
log.Debugf("[%s] service %s already exists, updating properties", r.name)
log.Debugf("Service %s already exists, updating properties", r.name)
currentConfig, err := s.Config()
if err == nil {
currentConfig.DisplayName = serviceConfig.DisplayName
Expand All @@ -158,16 +159,26 @@ func (r *SvcRunner) ValidateBeforeStart() error {
// service needs to be created
} else {
s, err = m.CreateService(r.serviceName,
execPath,
r.exec,
serviceConfig)
if err != nil {
r.backend.SetStatusLogErrorf("Failed to install service: %v", err)
return r.backend.SetStatusLogErrorf("Failed to install service: %v", err)
}
// It seems impossible to create a service with properly quoted arguments :-(
// Updating the BinaryPathName afterwards does the trick
currentConfig, err := s.Config()
if err == nil {
currentConfig.BinaryPathName = serviceConfig.BinaryPathName
}
err = s.UpdateConfig(currentConfig)
if err != nil {
r.backend.SetStatusLogErrorf("Failed to update the created service: %v", err)
}
defer s.Close()
err = eventlog.InstallAsEventCreate(r.serviceName, eventlog.Error|eventlog.Warning|eventlog.Info)
if err != nil {
s.Delete()
r.backend.SetStatusLogErrorf("SetupEventLogSource() failed: %v", err)
return r.backend.SetStatusLogErrorf("SetupEventLogSource() failed: %v", err)
}
}

Expand Down Expand Up @@ -213,7 +224,7 @@ func (r *SvcRunner) start() error {

ws, err := m.OpenService(r.serviceName)
if err != nil {
return r.backend.SetStatusLogErrorf("Could not access service: %v", err)
return r.backend.SetStatusLogErrorf("Could not access service %s: %v", r.serviceName, err)
}
defer ws.Close()

Expand All @@ -239,35 +250,10 @@ func (r *SvcRunner) stop() error {
// deactivate supervisor
r.setSupervised(false)

m, err := mgr.Connect()
if err != nil {
return r.backend.SetStatusLogErrorf("Failed to connect to service manager: %v", err)
}
defer m.Disconnect()

ws, err := m.OpenService(r.serviceName)
if err != nil {
return r.backend.SetStatusLogErrorf("Could not access service: %v", err)
}
defer ws.Close()

status, err := ws.Control(svc.Stop)
err := stopService(r.serviceName)
if err != nil {
return r.backend.SetStatusLogErrorf("Could not send stop control: %v", err)
return r.backend.SetStatusLogErrorf("%s", err)
}

timeout := time.Now().Add(10 * time.Second)
for status.State != svc.Stopped {
if timeout.Before(time.Now()) {
return r.backend.SetStatusLogErrorf("Timeout waiting for service to go to stopped state: %v", err)
}
time.Sleep(300 * time.Millisecond)
status, err = ws.Query()
if err != nil {
return r.backend.SetStatusLogErrorf("Could not retrieve service status: %v", err)
}
}

r.backend.SetStatus(backends.StatusStopped, "Stopped", "")

return nil
Expand Down
2 changes: 1 addition & 1 deletion glide.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 4 additions & 34 deletions services/control_handler_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@
package services

import (
"github.com/Graylog2/collector-sidecar/backends"
"github.com/Graylog2/collector-sidecar/daemon"
"github.com/kardianos/service"

"golang.org/x/sys/windows/svc/eventlog"
"golang.org/x/sys/windows/svc/mgr"
)

func ControlHandler(action string) {
Expand All @@ -37,35 +36,6 @@ func ControlHandler(action string) {
}

func cleanupInstalledServices() {
// backends are not initialized yet, let's use names directly
uninstallService("graylog-collector-nxlog")
}

func uninstallService(name string) {
log.Infof("Uninstalling service %s", name)
m, err := mgr.Connect()
if err != nil {
log.Errorf("Failed to connect to service manager: %v", err)
}
defer m.Disconnect()

s, err := m.OpenService(name)
// service exist so we try to uninstall it
if err != nil {
log.Debugf("Service %s doesn't exist, no uninstall action needed", name)
return
}

defer s.Close()
err = s.Delete()
if err != nil {
log.Errorf("Can't delete service %s: %v", name, err)
}

err = eventlog.Remove(s.Name)
if err != nil {
log.Errorf("RemoveEventLogSource() failed: %s", err)
}

return
// Cleans all services starting with "graylog-collector-"
daemon.CleanOldServices([]*backends.Backend{})
}

0 comments on commit acfcc9b

Please sign in to comment.