Skip to content

Commit

Permalink
Changed the reload constants to be Durations rather than integers
Browse files Browse the repository at this point in the history
The reload constants were previously integers that were multipled by a
duration constant later.  This is confusing since the units are
different, and the environment variables are duration strings.

This patch also changes the way the constants are used and moves them
inline in the parameter definitions with a helper function to
translate to durations.
  • Loading branch information
knobunc committed Sep 29, 2017
1 parent d95ecd9 commit 4099687
Showing 1 changed file with 11 additions and 43 deletions.
54 changes: 11 additions & 43 deletions pkg/cmd/infra/router/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,6 @@ import (
"github.com/openshift/origin/pkg/version"
)

// defaultReloadInterval is how often to do reloads in seconds.
const defaultReloadInterval = 5

// defaultReloadGap is the minimum gap between a reload end and start in milliseconds.
const defaultReloadGap = 100

// defaultReloadEventWait is how long to wait for more events before triggering a reload. In milliseconds
const defaultReloadEventWait = 10

var routerLong = templates.LongDesc(`
Start a router
Expand Down Expand Up @@ -89,38 +80,15 @@ type TemplateRouter struct {
MetricsType string
}

// reloadInterval returns how often to run the router reloads. The interval
// value is based on an environment variable or the default.
func reloadInterval() time.Duration {
interval := util.Env("RELOAD_INTERVAL", fmt.Sprintf("%vs", defaultReloadInterval))
value, err := time.ParseDuration(interval)
if err != nil {
glog.Warningf("Invalid RELOAD_INTERVAL %q, using default value %vs ...", interval, defaultReloadInterval)
value = time.Duration(defaultReloadInterval * time.Second)
}
return value
}

// reloadGap returns how often to wait between a reload end and the start of the next reload.
// The gap value is based on an environment variable or the default.
func reloadGap() time.Duration {
gap := util.Env("RELOAD_GAP", fmt.Sprintf("%vms", defaultReloadGap))
value, err := time.ParseDuration(gap)
if err != nil {
glog.Warningf("Invalid RELOAD_GAP %q, using default value %vms ...", gap, defaultReloadGap)
value = time.Duration(defaultReloadGap * time.Millisecond)
}
return value
}

// reloadEventWait returns how often to wait after getting the first event after a reload before triggering the next reload.
// The event wait value is based on an environment variable or the default.
func reloadEventWait() time.Duration {
eventWait := util.Env("RELOAD_EVENT_WAIT", fmt.Sprintf("%vms", defaultReloadEventWait))
value, err := time.ParseDuration(eventWait)
// getDurationEnv() gets retrieves the value of the named environment
// variable and checks that is a valid duration. If it is not, or it
// is not set, it returns the given default.
func getDurationEnv(envName string, defaultDuration time.Duration) time.Duration {
valueString := util.Env(envName, defaultDuration.String())
value, err := time.ParseDuration(valueString)
if err != nil {
glog.Warningf("Invalid RELOAD_EVENT_WAIT %q, using default value %vms ...", eventWait, defaultReloadEventWait)
value = time.Duration(defaultReloadEventWait * time.Millisecond)
glog.Warningf("Invalid %s %q, using default value %s ...", envName, valueString, defaultDuration.String())
value = defaultDuration
}
return value
}
Expand All @@ -135,9 +103,9 @@ func (o *TemplateRouter) Bind(flag *pflag.FlagSet) {
flag.StringVar(&o.DefaultDestinationCAPath, "default-destination-ca-path", util.Env("DEFAULT_DESTINATION_CA_PATH", "/var/run/secrets/kubernetes.io/serviceaccount/service-ca.crt"), "A path to a PEM file containing the default CA bundle to use with re-encrypt routes. This CA should sign for certificates in the Kubernetes DNS space (service.namespace.svc).")
flag.StringVar(&o.TemplateFile, "template", util.Env("TEMPLATE_FILE", ""), "The path to the template file to use")
flag.StringVar(&o.ReloadScript, "reload", util.Env("RELOAD_SCRIPT", ""), "The path to the reload script to use")
flag.DurationVar(&o.ReloadInterval, "interval", reloadInterval(), "Controls how often router reloads are invoked. Mutiple router reload requests are coalesced for the duration of this interval since the time the last reload started.")
flag.DurationVar(&o.ReloadGap, "reload-gap", reloadGap(), "Controls how often router reloads are invoked. Mutiple router reload requests are coalesced for the duration of this interval since the time the last reload ended.")
flag.DurationVar(&o.ReloadEventWait, "reload-event-wait", reloadEventWait(), "Controls how often router reloads are invoked. Mutiple router reload events are gathered for the given duration before a reload is triggered.")
flag.DurationVar(&o.ReloadInterval, "interval", getDurationEnv("RELOAD_INTERVAL", 5*time.Second), "Controls how often router reloads are invoked. Mutiple router reload requests are coalesced for the duration of this interval since the time the last reload started.")
flag.DurationVar(&o.ReloadGap, "reload-gap", getDurationEnv("RELOAD_GAP", 100*time.Millisecond), "Controls how often router reloads are invoked. Mutiple router reload requests are coalesced for the duration of this interval since the time the last reload ended.")
flag.DurationVar(&o.ReloadEventWait, "reload-event-wait", getDurationEnv("RELOAD_EVENT_WAIT", 10*time.Millisecond), "Controls how often router reloads are invoked. Mutiple router reload events are gathered for the given duration before a reload is triggered.")
flag.BoolVar(&o.ExtendedValidation, "extended-validation", util.Env("EXTENDED_VALIDATION", "true") == "true", "If set, then an additional extended validation step is performed on all routes admitted in by this router. Defaults to true and enables the extended validation checks.")
flag.BoolVar(&o.BindPortsAfterSync, "bind-ports-after-sync", util.Env("ROUTER_BIND_PORTS_AFTER_SYNC", "") == "true", "Bind ports only after route state has been synchronized")
flag.StringVar(&o.MaxConnections, "max-connections", util.Env("ROUTER_MAX_CONNECTIONS", ""), "Specifies the maximum number of concurrent connections.")
Expand Down

0 comments on commit 4099687

Please sign in to comment.