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

Improve process shutdown handling #462

Merged
merged 22 commits into from
Jan 12, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix build for windows
This needed a bigger refactoring to deal with import cycles:

 - Move exec_helpers to daemon package
 - Extract some helper functions into new helper package
mpfz0r committed Jan 3, 2023
commit ecc35a7bd6bb251d4db4dc915af111d9a0d96ef0
5 changes: 3 additions & 2 deletions api/graylog.go
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ import (
"bytes"
"crypto/tls"
"encoding/json"
"github.com/Graylog2/collector-sidecar/helpers"
"io"
"net/http"
"strconv"
@@ -164,15 +165,15 @@ func UpdateRegistration(httpClient *http.Client, checksum string, ctx *context.C
registration := graylog.RegistrationRequest{}

registration.NodeName = ctx.UserConfig.NodeName
registration.NodeDetails.OperatingSystem = common.GetSystemName()
registration.NodeDetails.OperatingSystem = helpers.GetSystemName()

if ctx.UserConfig.SendStatus {
metrics := &graylog.MetricsRequest{
Disks75: common.GetFileSystemList75(ctx.UserConfig.WindowsDriveRange),
CpuIdle: common.GetCpuIdle(),
Load1: common.GetLoad1(),
}
registration.NodeDetails.IP = common.GetHostIP()
registration.NodeDetails.IP = helpers.GetHostIP()
registration.NodeDetails.Status = status
registration.NodeDetails.Metrics = metrics
if len(ctx.UserConfig.ListLogFiles) > 0 {
4 changes: 2 additions & 2 deletions assignments/assignment.go
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@
package assignments

import (
"github.com/Graylog2/collector-sidecar/common"
"github.com/Graylog2/collector-sidecar/helpers"
"reflect"
)

@@ -92,7 +92,7 @@ func (as *assignmentStore) Update(assignments []ConfigurationAssignment) bool {

func (as *assignmentStore) cleanup(validBackendIds []string) {
for backendId := range as.assignments {
if !common.IsInList(backendId, validBackendIds) {
if !helpers.IsInList(backendId, validBackendIds) {
delete(as.assignments, backendId)
}
}
14 changes: 7 additions & 7 deletions backends/backend.go
Original file line number Diff line number Diff line change
@@ -18,6 +18,7 @@ package backends
import (
"bytes"
"fmt"
"github.com/Graylog2/collector-sidecar/helpers"
"os/exec"
"path/filepath"
"reflect"
@@ -27,7 +28,6 @@ import (
"github.com/flynn-archive/go-shlex"

"github.com/Graylog2/collector-sidecar/api/graylog"
"github.com/Graylog2/collector-sidecar/common"
"github.com/Graylog2/collector-sidecar/context"
"github.com/Graylog2/collector-sidecar/system"
)
@@ -50,7 +50,7 @@ type Backend struct {

func BackendFromResponse(response graylog.ResponseCollectorBackend, configId string, ctx *context.Ctx) *Backend {
return &Backend{
Enabled: common.NewTrue(),
Enabled: helpers.NewTrue(),
Id: response.Id + "-" + configId,
CollectorId: response.Id,
ConfigId: configId,
@@ -74,10 +74,10 @@ func (b *Backend) Equals(a *Backend) bool {
}

func (b *Backend) EqualSettings(a *Backend) bool {
executeParameters, _ := common.Sprintf(
executeParameters, _ := helpers.Sprintf(
a.ExecuteParameters,
a.ConfigurationPath)
validationParameters, _ := common.Sprintf(
validationParameters, _ := helpers.Sprintf(
a.ValidationParameters,
a.ConfigurationPath)

@@ -104,7 +104,7 @@ func (b *Backend) CheckExecutableAgainstAccesslist(context *context.Ctx) error {
if len(context.UserConfig.CollectorBinariesAccesslist) <= 0 {
return nil
}
isListed, err := common.PathMatch(b.ExecutablePath, context.UserConfig.CollectorBinariesAccesslist)
isListed, err := helpers.PathMatch(b.ExecutablePath, context.UserConfig.CollectorBinariesAccesslist)
if err != nil {
return fmt.Errorf("Can not validate binary path: %s", err)
}
@@ -121,7 +121,7 @@ func (b *Backend) CheckExecutableAgainstAccesslist(context *context.Ctx) error {
}

func (b *Backend) CheckConfigPathAgainstAccesslist(context *context.Ctx) bool {
configuration, err := common.PathMatch(b.ConfigurationPath, context.UserConfig.CollectorBinariesAccesslist)
configuration, err := helpers.PathMatch(b.ConfigurationPath, context.UserConfig.CollectorBinariesAccesslist)
if err != nil {
log.Errorf("Can not validate configuration path: %s", err)
return false
@@ -145,7 +145,7 @@ func (b *Backend) ValidateConfigurationFile(context *context.Ctx) (error, string
var err error
var quotedArgs []string
if runtime.GOOS == "windows" {
quotedArgs = common.CommandLineToArgv(b.ValidationParameters)
quotedArgs = helpers.CommandLineToArgv(b.ValidationParameters)
} else {
quotedArgs, err = shlex.Split(b.ValidationParameters)
}
8 changes: 4 additions & 4 deletions backends/registry.go
Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@
package backends

import (
"github.com/Graylog2/collector-sidecar/common"
"github.com/Graylog2/collector-sidecar/helpers"
"github.com/Graylog2/collector-sidecar/logger"
)

@@ -32,13 +32,13 @@ type backendStore struct {

func (bs *backendStore) SetBackend(backend Backend) {
bs.backends[backend.Id] = &backend
executeParameters, err := common.Sprintf(backend.ExecuteParameters, backend.ConfigurationPath)
executeParameters, err := helpers.Sprintf(backend.ExecuteParameters, backend.ConfigurationPath)
if err != nil {
log.Errorf("Invalid execute parameters, skip adding backend: %s", backend.Name)
return
}
bs.backends[backend.Id].ExecuteParameters = executeParameters
validationParameters, err := common.Sprintf(backend.ValidationParameters, backend.ConfigurationPath)
validationParameters, err := helpers.Sprintf(backend.ValidationParameters, backend.ConfigurationPath)
if err != nil {
log.Errorf("Invalid validation parameters, skip adding backend: %s", backend.Name)
return
@@ -84,7 +84,7 @@ func (bs *backendStore) Update(backends []Backend) {

func (bs *backendStore) Cleanup(validBackendIds []string) {
for _, backend := range bs.backends {
if !common.IsInList(backend.Id, validBackendIds) {
if !helpers.IsInList(backend.Id, validBackendIds) {
log.Debug("Cleaning up backend: " + backend.Name)
delete(bs.backends, backend.Id)
}
3 changes: 2 additions & 1 deletion backends/render.go
Original file line number Diff line number Diff line change
@@ -20,14 +20,15 @@ import (
"fmt"
"github.com/Graylog2/collector-sidecar/common"
"github.com/Graylog2/collector-sidecar/context"
"github.com/Graylog2/collector-sidecar/helpers"
"io/ioutil"
)

func (b *Backend) render() []byte {
var result bytes.Buffer
result.WriteString(b.Template)

return common.ConvertLineBreak(result.Bytes())
return helpers.ConvertLineBreak(result.Bytes())
}

func (b *Backend) renderToFile(context *context.Ctx) error {
7 changes: 4 additions & 3 deletions context/context.go
Original file line number Diff line number Diff line change
@@ -16,6 +16,8 @@
package context

import (
"github.com/Graylog2/collector-sidecar/common"
"github.com/Graylog2/collector-sidecar/helpers"
"github.com/docker/go-units"
"net/url"
"os"
@@ -25,7 +27,6 @@ import (
"time"

"github.com/Graylog2/collector-sidecar/cfgfile"
"github.com/Graylog2/collector-sidecar/common"
"github.com/Graylog2/collector-sidecar/logger"
"github.com/Graylog2/collector-sidecar/system"
)
@@ -71,15 +72,15 @@ func (ctx *Ctx) LoadConfig(path *string) error {
if ctx.UserConfig.NodeId == "" {
log.Fatal("No node ID was configured.")
}
ctx.NodeId = common.GetCollectorId(ctx.UserConfig.NodeId)
ctx.NodeId = helpers.GetCollectorId(ctx.UserConfig.NodeId)
if ctx.NodeId == "" {
log.Fatal("Empty node-id, exiting! Make sure a valid id is configured.")
}

// node_name
if ctx.UserConfig.NodeName == "" {
log.Info("No node name was configured, falling back to hostname")
ctx.UserConfig.NodeName, err = common.GetHostname()
ctx.UserConfig.NodeName, err = helpers.GetHostname()
if err != nil {
log.Fatal("No node name configured and not able to obtain hostname as alternative.")
}
4 changes: 2 additions & 2 deletions daemon/action_handler.go
Original file line number Diff line number Diff line change
@@ -18,7 +18,7 @@ package daemon
import (
"github.com/Graylog2/collector-sidecar/api/graylog"
"github.com/Graylog2/collector-sidecar/backends"
"github.com/Graylog2/collector-sidecar/common"
"github.com/Graylog2/collector-sidecar/helpers"
)

func HandleCollectorActions(actions []graylog.ResponseCollectorAction) {
@@ -37,7 +37,7 @@ func HandleCollectorActions(actions []graylog.ResponseCollectorAction) {
case action.Properties["stop"] == true:
stopAction(backend)
default:
log.Infof("Got unsupported collector command: %s", common.Inspect(action.Properties))
log.Infof("Got unsupported collector command: %s", helpers.Inspect(action.Properties))
}
}
}
4 changes: 2 additions & 2 deletions daemon/daemon.go
Original file line number Diff line number Diff line change
@@ -18,8 +18,8 @@ package daemon
import (
"github.com/Graylog2/collector-sidecar/assignments"
"github.com/Graylog2/collector-sidecar/backends"
"github.com/Graylog2/collector-sidecar/common"
"github.com/Graylog2/collector-sidecar/context"
"github.com/Graylog2/collector-sidecar/helpers"
"github.com/Graylog2/collector-sidecar/logger"
)

@@ -45,7 +45,7 @@ func init() {
}

func NewConfig() *DaemonConfig {
rootDir, err := common.GetRootPath()
rootDir, err := helpers.GetRootPath()
if err != nil {
log.Error("Can not access root directory")
}
44 changes: 44 additions & 0 deletions daemon/exec_helper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (C) 2020 Graylog, Inc.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the Server Side Public License, version 1,
// as published by MongoDB, Inc.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// Server Side Public License for more details.
//
// You should have received a copy of the Server Side Public License
// along with this program. If not, see
// <http://www.mongodb.com/licensing/server-side-public-license>.

//go:build !windows
// +build !windows

package daemon

import (
"os"
"os/exec"
"syscall"
"time"
)

func Setpgid(cmd *exec.Cmd) {
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
}
func KillProcess(r *ExecRunner, proc *os.Process) {
mpfz0r marked this conversation as resolved.
Show resolved Hide resolved
log.Debugf("[%s] PID SIGHUP ignored, sending SIGHUP to process group", r.Name())
mpfz0r marked this conversation as resolved.
Show resolved Hide resolved
err := syscall.Kill(-proc.Pid, syscall.SIGHUP)
if err != nil {
log.Debugf("[%s] Failed to HUP process group %s", r.Name(), err)
}
time.Sleep(2 * time.Second)
if r.Running() {
err := syscall.Kill(-proc.Pid, syscall.SIGKILL)
if err != nil {
log.Debugf("[%s] Failed to kill process group %s", r.Name(), err)
}
}
}
61 changes: 61 additions & 0 deletions daemon/exec_helper_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright (C) 2020 Graylog, Inc.

// This program is free software: you can redistribute it and/or modify
// it under the terms of the Server Side Public License, version 1,
// as published by MongoDB, Inc.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// Server Side Public License for more details.
//
// You should have received a copy of the Server Side Public License
// along with this program. If not, see
// <http://www.mongodb.com/licensing/server-side-public-license>.

// Copyright 2009 The Go Authors. All rights reserved.
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are
// met:
//
// * Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above
// copyright notice, this list of conditions and the following disclaimer
// in the documentation and/or other materials provided with the
// distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

// extracted from github.com/golang/go/blob/master/src/os/exec_windows.go

package daemon

import (
"os"
"os/exec"
)

func Setpgid(cmd *exec.Cmd) {
panic("not implemented on this platform")
}
func KillProcess(r *ExecRunner, proc *os.Process) {
err := proc.Kill()
if err != nil {
log.Debugf("[%s] Failed to kill process %s", r.Name(), err)
}
}
22 changes: 6 additions & 16 deletions daemon/exec_runner.go
Original file line number Diff line number Diff line change
@@ -17,6 +17,7 @@ package daemon

import (
"errors"
"github.com/Graylog2/collector-sidecar/helpers"
"io/ioutil"
"os"
"os/exec"
@@ -198,7 +199,7 @@ func (r *ExecRunner) start() error {
var err error
var quotedArgs []string
if runtime.GOOS == "windows" {
quotedArgs = common.CommandLineToArgv(r.args)
quotedArgs = helpers.CommandLineToArgv(r.args)
} else {
quotedArgs, err = shlex.Split(r.args)
}
@@ -208,7 +209,9 @@ func (r *ExecRunner) start() error {
r.cmd = exec.Command(r.exec, quotedArgs...)
r.cmd.Dir = r.daemon.Dir
r.cmd.Env = append(os.Environ(), r.daemon.Env...)
r.cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
if runtime.GOOS != "windows" {
Setpgid(r.cmd)
}

// start the actual process and don't block
r.startTime = time.Now()
@@ -241,20 +244,7 @@ func (r *ExecRunner) stop() error {
}

// in doubt kill the process
if r.Running() && runtime.GOOS != "windows" {
log.Debugf("[%s] PID SIGHUP ignored, sending SIGHUP to process group", r.Name())
err := syscall.Kill(-r.cmd.Process.Pid, syscall.SIGHUP)
if err != nil {
log.Debugf("[%s] Failed to HUP process group %s", r.Name(), err)
}
time.Sleep(2 * time.Second)
}
if r.Running() {
err := syscall.Kill(-r.cmd.Process.Pid, syscall.SIGKILL)
if err != nil {
log.Debugf("[%s] Failed to kill process group %s", r.Name(), err)
}
}
KillProcess(r, r.cmd.Process)
mpfz0r marked this conversation as resolved.
Show resolved Hide resolved

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

Loading