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

Bug 1920421: Add "ingress.operator.openshift.io/hard-stop-after" annotation #538

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 78 additions & 0 deletions pkg/operator/controller/ingress/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"sort"
"strconv"
"strings"
"time"

"github.com/davecgh/go-spew/spew"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -42,6 +43,9 @@ const (

RouterDisableHTTP2EnvName = "ROUTER_DISABLE_HTTP2"
RouterDefaultEnableHTTP2Annotation = "ingress.operator.openshift.io/default-enable-http2"

RouterHardStopAfterEnvName = "ROUTER_HARD_STOP_AFTER"
RouterHardStopAfterAnnotation = "ingress.operator.openshift.io/hard-stop-after"
)

// ensureRouterDeployment ensures the router deployment exists for a given
Expand Down Expand Up @@ -112,6 +116,33 @@ func HTTP2IsEnabled(ic *operatorv1.IngressController, ingressConfig *configv1.In
return configHasHTTP2Enabled
}

// HardStopAfterIsEnabledByAnnotation returns true if the map m has
// the key RouterHardStopAfterEnvName and its value is a valid HAProxy
// time duration.
func HardStopAfterIsEnabledByAnnotation(m map[string]string) (bool, string) {
if val, ok := m[RouterHardStopAfterAnnotation]; ok && len(val) > 0 {
if clippedVal, err := clipHAProxyTimeoutValue(val); err != nil {
log.Error(err, "invalid HAProxy time value", "annotation", RouterHardStopAfterAnnotation, "value", val)
return false, ""
} else {
return true, clippedVal
}
}
return false, ""
}

// HardStopAfterIsEnabled returns true if either the ingress
// controller or the ingress config has the "hard-stop-after"
// annotation. The presence of the annotation on the ingress
// controller, irrespective of its value, always overrides any setting
// on the ingress config.
func HardStopAfterIsEnabled(ic *operatorv1.IngressController, ingressConfig *configv1.Ingress) (bool, string) {
if controllerAnnotation, controllerValue := HardStopAfterIsEnabledByAnnotation(ic.Annotations); controllerAnnotation {
return controllerAnnotation, controllerValue
}
return HardStopAfterIsEnabledByAnnotation(ingressConfig.Annotations)
}

// desiredRouterDeployment returns the desired router deployment.
func desiredRouterDeployment(ci *operatorv1.IngressController, ingressControllerImage string, infraConfig *configv1.Infrastructure, ingressConfig *configv1.Ingress, apiConfig *configv1.APIServer, networkConfig *configv1.Network) (*appsv1.Deployment, error) {
deployment := manifests.RouterDeployment()
Expand Down Expand Up @@ -535,6 +566,10 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
env = append(env, corev1.EnvVar{Name: RouterDisableHTTP2EnvName, Value: "true"})
}

if enabled, value := HardStopAfterIsEnabled(ci, ingressConfig); enabled {
env = append(env, corev1.EnvVar{Name: RouterHardStopAfterEnvName, Value: value})
}

deployment.Spec.Template.Spec.Volumes = volumes
deployment.Spec.Template.Spec.Containers[0].VolumeMounts = routerVolumeMounts
deployment.Spec.Template.Spec.Containers[0].Env = append(deployment.Spec.Template.Spec.Containers[0].Env, env...)
Expand Down Expand Up @@ -937,3 +972,46 @@ func deploymentConfigChanged(current, expected *appsv1.Deployment) (bool, *appsv
updated.Spec.Replicas = &replicas
return true, updated
}

// clipHAProxyTimeoutValue prevents the HAProxy config file from using
// timeout values that exceed the maximum value allowed by HAProxy.
// Returns an error in the event that a timeout string value is not
// parsable as a valid time duration, or the clipped time duration
// otherwise.
//
// TODO: this is a modified copy from openshift/router but returns ""
// if there's any error.
//
// Ideally we need to share this utility function via:
// https://github.com/openshift/library-go/blob/master/pkg/route/routeapihelpers
func clipHAProxyTimeoutValue(val string) (string, error) {
const haproxyMaxTimeout = "2147483647ms" // max timeout allowable by HAProxy

endIndex := len(val) - 1
maxTimeout, err := time.ParseDuration(haproxyMaxTimeout)
if err != nil {
return "", err
}
// time.ParseDuration doesn't work with days
// despite HAProxy accepting timeouts that specify day units
if val[endIndex] == 'd' {
days, err := strconv.Atoi(val[:endIndex])
if err != nil {
return "", err
}
if maxTimeout.Hours() < float64(days*24) {
log.V(7).Info("Route annotation timeout exceeds maximum allowable by HAProxy, clipping to max")
return haproxyMaxTimeout, nil
}
} else {
duration, err := time.ParseDuration(val)
if err != nil {
return "", err
}
if maxTimeout.Milliseconds() < duration.Milliseconds() {
log.V(7).Info("Route annotation timeout exceeds maximum allowable by HAProxy, clipping to max")
return haproxyMaxTimeout, nil
}
}
return val, nil
}
1 change: 1 addition & 0 deletions pkg/operator/controller/ingress/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ func TestDesiredRouterDeployment(t *testing.T) {

checkDeploymentHasEnvVar(t, deployment, "ROUTER_IP_V4_V6_MODE", true, "v6")
checkDeploymentHasEnvVar(t, deployment, RouterDisableHTTP2EnvName, true, "true")
checkDeploymentHasEnvVar(t, deployment, RouterHardStopAfterEnvName, false, "")
}

func TestInferTLSProfileSpecFromDeployment(t *testing.T) {
Expand Down