Skip to content

Commit

Permalink
feat(runner conf): Runner conf allows mergin of timeouts with default
Browse files Browse the repository at this point in the history
- Accounts for zero value configurations in the merging process
    Use nullable timeout values on the runner conf, so if timeouts are
    nil, values from default conf can be used (before this commit zero
    values could not be used properly).

- Split the merging of configuration in 'GetConfRunner' into a separate
  function 'MergedConfRunners' for simpler code and potential future
  reusability

By changing the configuration assignments to reference assignments, we
have more flexibility when passing runner configurations (i.e., we can
use nil to specify a non-override). By splitting the
configuration merging logic into a separate function, we enhance
readability and promote reusability. The adjustments in the test
functions account for these changes. The update to the HTTP response
generation simplifies the existing implementation by removing
redundancy.
  • Loading branch information
farzadghanei committed May 3, 2024
1 parent d9b2a32 commit a5b7f6b
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 114 deletions.
3 changes: 2 additions & 1 deletion examples/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ runners:
default:
timeout: 5m
listen_address: "127.0.0.1:51234"
http:
cli: {} # override default runner only for CLI mode
http: # override default runner only for HTTP mode
# shutdown_signal_header is mainly useful for testing http mode, do not set it in production
# if set, better be treated like a secret, and a secure transport layer should be used.
# this is the value set on "X-Shutdown-Signal" header in the http request
Expand Down
120 changes: 57 additions & 63 deletions internal/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ type Conf struct {

// ConfRunner is config for the check runners
type ConfRunner struct {
Timeout time.Duration
ShutdownSignalHeader *string `yaml:"shutdown_signal_header"`
ListenAddress string `yaml:"listen_address"`
RequestReadTimeout time.Duration `yaml:"request_read_timeout"`
ResponseWriteTimeout time.Duration `yaml:"response_write_timeout"`
ResponseOK string `yaml:"response_ok"`
ResponseFailed string `yaml:"response_failed"`
ResponseTimeout string `yaml:"response_timeout"`
Timeout *time.Duration
ShutdownSignalHeader *string `yaml:"shutdown_signal_header"`
ListenAddress string `yaml:"listen_address"`
RequestReadTimeout *time.Duration `yaml:"request_read_timeout"`
ResponseWriteTimeout *time.Duration `yaml:"response_write_timeout"`
ResponseOK *string `yaml:"response_ok"`
ResponseFailed *string `yaml:"response_failed"`
ResponseTimeout *string `yaml:"response_timeout"`
}

// ConfCheckSpec is the spec for each check configuration
Expand Down Expand Up @@ -59,45 +59,25 @@ func ReadConf(path string) (*Conf, error) {

// GetDefaultConfRunner returns a ConfRunner based on the default configuration
func GetDefaultConfRunner(runners *ConfRunners) ConfRunner {
defaultRunner := ConfRunner{
Timeout: 0,
var timeout, readTimeout, writeTimout time.Duration = 0 * time.Second, 30 * time.Second, 30 * time.Second
var respOK, respFailed, respTimeout string = "OK", "FAILED", "TIMEOUT"

baseConf := ConfRunner{
Timeout: &timeout,
ShutdownSignalHeader: nil,
ListenAddress: "127.0.0.1:8880",
RequestReadTimeout: 30 * time.Second,
ResponseWriteTimeout: 30 * time.Second,
ResponseOK: "OK",
ResponseFailed: "FAILED",
ResponseTimeout: "TIMEOUT",
RequestReadTimeout: &readTimeout,
ResponseWriteTimeout: &writeTimout,
ResponseOK: &respOK,
ResponseFailed: &respFailed,
ResponseTimeout: &respTimeout,
}

if defaultConf, defaultExists := (*runners)["default"]; defaultExists {
if defaultConf.Timeout != 0 {
defaultRunner.Timeout = defaultConf.Timeout
}
if defaultConf.ShutdownSignalHeader != nil {
defaultRunner.ShutdownSignalHeader = defaultConf.ShutdownSignalHeader
}
if defaultConf.ListenAddress != "" {
defaultRunner.ListenAddress = defaultConf.ListenAddress
}
if defaultConf.RequestReadTimeout != 0 {
defaultRunner.RequestReadTimeout = defaultConf.RequestReadTimeout
}
if defaultConf.ResponseWriteTimeout != 0 {
defaultRunner.ResponseWriteTimeout = defaultConf.ResponseWriteTimeout
}
if defaultConf.ResponseOK != "" {
defaultRunner.ResponseOK = defaultConf.ResponseOK
}
if defaultConf.ResponseFailed != "" {
defaultRunner.ResponseFailed = defaultConf.ResponseFailed
}
if defaultConf.ResponseTimeout != "" {
defaultRunner.ResponseTimeout = defaultConf.ResponseTimeout
}
baseConf = MergedConfRunners(&baseConf, &defaultConf)
}

return defaultRunner
return baseConf
}

// GetConfRunner returns the runner config for the name merged with the default, and bool if it exists
Expand All @@ -110,41 +90,55 @@ func GetConfRunner(runners *ConfRunners, name string) (ConfRunner, bool) {
}

// Merge the requested runner with the default runner
mergedConf := MergedConfRunners(&defaultConf, &namedConf)

return mergedConf, true
}

// MergedConfRunners merges the baseConf with the overrideConf and returns the merged ConfRunner
func MergedConfRunners(baseConf, overrideConf *ConfRunner) ConfRunner {
mergedConf := ConfRunner{
Timeout: namedConf.Timeout,
ShutdownSignalHeader: namedConf.ShutdownSignalHeader,
ListenAddress: namedConf.ListenAddress,
RequestReadTimeout: namedConf.RequestReadTimeout,
ResponseWriteTimeout: namedConf.ResponseWriteTimeout,
ResponseOK: namedConf.ResponseOK,
ResponseFailed: namedConf.ResponseFailed,
ResponseTimeout: namedConf.ResponseTimeout,
Timeout: overrideConf.Timeout,
ShutdownSignalHeader: overrideConf.ShutdownSignalHeader,
ListenAddress: overrideConf.ListenAddress,
RequestReadTimeout: overrideConf.RequestReadTimeout,
ResponseWriteTimeout: overrideConf.ResponseWriteTimeout,
ResponseOK: overrideConf.ResponseOK,
ResponseFailed: overrideConf.ResponseFailed,
ResponseTimeout: overrideConf.ResponseTimeout,
}

if mergedConf.Timeout == 0 {
mergedConf.Timeout = defaultConf.Timeout
if mergedConf.Timeout == nil {
mergedConf.Timeout = baseConf.Timeout
}

if mergedConf.ShutdownSignalHeader == nil {
mergedConf.ShutdownSignalHeader = defaultConf.ShutdownSignalHeader
mergedConf.ShutdownSignalHeader = baseConf.ShutdownSignalHeader
}

if mergedConf.ListenAddress == "" {
mergedConf.ListenAddress = defaultConf.ListenAddress
mergedConf.ListenAddress = baseConf.ListenAddress
}
if mergedConf.RequestReadTimeout == 0 {
mergedConf.RequestReadTimeout = defaultConf.RequestReadTimeout

if mergedConf.RequestReadTimeout == nil {
mergedConf.RequestReadTimeout = baseConf.RequestReadTimeout
}
if mergedConf.ResponseWriteTimeout == 0 {
mergedConf.ResponseWriteTimeout = defaultConf.ResponseWriteTimeout

if mergedConf.ResponseWriteTimeout == nil {
mergedConf.ResponseWriteTimeout = baseConf.ResponseWriteTimeout
}
if mergedConf.ResponseOK == "" {
mergedConf.ResponseOK = defaultConf.ResponseOK

if mergedConf.ResponseOK == nil {
mergedConf.ResponseOK = baseConf.ResponseOK
}
if mergedConf.ResponseFailed == "" {
mergedConf.ResponseFailed = defaultConf.ResponseFailed

if mergedConf.ResponseFailed == nil {
mergedConf.ResponseFailed = baseConf.ResponseFailed
}
if mergedConf.ResponseTimeout == "" {
mergedConf.ResponseTimeout = defaultConf.ResponseTimeout

if mergedConf.ResponseTimeout == nil {
mergedConf.ResponseTimeout = baseConf.ResponseTimeout
}

return mergedConf, true
return mergedConf
}
Loading

0 comments on commit a5b7f6b

Please sign in to comment.