-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Change the router reload supression, and add some more controls #16564
Changes from all commits
6006275
af1a03a
d95ecd9
4099687
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,9 +34,6 @@ import ( | |
"github.com/openshift/origin/pkg/version" | ||
) | ||
|
||
// defaultReloadInterval is how often to do reloads in seconds. | ||
const defaultReloadInterval = 5 | ||
|
||
var routerLong = templates.LongDesc(` | ||
Start a router | ||
|
||
|
@@ -68,6 +65,8 @@ type TemplateRouter struct { | |
TemplateFile string | ||
ReloadScript string | ||
ReloadInterval time.Duration | ||
ReloadGap time.Duration | ||
ReloadEventWait time.Duration | ||
DefaultCertificate string | ||
DefaultCertificatePath string | ||
DefaultCertificateDir string | ||
|
@@ -81,14 +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) | ||
// 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_INTERVAL %q, using default value %v ...", interval, defaultReloadInterval) | ||
value = time.Duration(defaultReloadInterval * time.Second) | ||
glog.Warningf("Invalid %s %q, using default value %s ...", envName, valueString, defaultDuration.String()) | ||
value = defaultDuration | ||
} | ||
return value | ||
} | ||
|
@@ -103,7 +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 last reload time.") | ||
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.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How often are we really going to tune these? Adding lots of extra tunables comes with a cost - do these really need to be changeable or can they just be proportional to the overall duration? Every tunable we add is more choices that someone can get wrong. Also, I couldn't understand how the flags are used from the descriptions, which is a problem. |
||
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.") | ||
|
@@ -205,6 +207,14 @@ func (o *TemplateRouterOptions) Complete() error { | |
return fmt.Errorf("invalid reload interval: %v - must be a positive duration", nsecs) | ||
} | ||
|
||
if o.ReloadGap < 0 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be in validate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the other checks in here be in validate too? Or is there some pattern that I don't grok fully. |
||
return fmt.Errorf("invalid reload gap: %s - must not be negative", o.ReloadGap.String()) | ||
} | ||
|
||
if o.ReloadEventWait < 0 { | ||
return fmt.Errorf("invalid reload event wait: %s - must not be negative", o.ReloadEventWait.String()) | ||
} | ||
|
||
if len(routerCanonicalHostname) > 0 { | ||
if errs := validation.IsDNS1123Subdomain(routerCanonicalHostname); len(errs) != 0 { | ||
return fmt.Errorf("invalid canonical hostname: %s", routerCanonicalHostname) | ||
|
@@ -324,6 +334,8 @@ func (o *TemplateRouterOptions) Run() error { | |
TemplatePath: o.TemplateFile, | ||
ReloadScriptPath: o.ReloadScript, | ||
ReloadInterval: o.ReloadInterval, | ||
ReloadGap: o.ReloadGap, | ||
ReloadEventWait: o.ReloadEventWait, | ||
DefaultCertificate: o.DefaultCertificate, | ||
DefaultCertificatePath: o.DefaultCertificatePath, | ||
DefaultCertificateDir: o.DefaultCertificateDir, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typos