Skip to content

Commit

Permalink
Fix and improve Windows service handling
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mpfz0r committed Dec 19, 2018
1 parent eda0771 commit 82da916
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 72 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
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
}
44 changes: 11 additions & 33 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 Down Expand Up @@ -113,6 +115,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 @@ -137,14 +140,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 @@ -161,13 +164,13 @@ func (r *SvcRunner) ValidateBeforeStart() error {
execPath,
serviceConfig)
if err != nil {
r.backend.SetStatusLogErrorf("Failed to install service: %v", err)
return r.backend.SetStatusLogErrorf("Failed to install 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 +216,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 +242,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 82da916

Please sign in to comment.