Skip to content

Bug 1837251: e2e: Rework http/2 e2e test for ingresscontroller precedence #405

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 1 commit into from
May 26, 2020
Merged

Bug 1837251: e2e: Rework http/2 e2e test for ingresscontroller precedence #405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 25 additions & 9 deletions pkg/operator/controller/ingress/deployment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,31 @@ func (r *reconciler) ensureRouterDeleted(ci *operatorv1.IngressController) error
}

// HTTP2IsDisabledByAnnotation returns true if the map m has the key
// RouterDisableHTTP2Annotation present and it has "true" as its
// value.
func HTTP2IsDisabledByAnnotation(m map[string]string) bool {
if mval, ok := m[RouterDisableHTTP2Annotation]; ok {
if val, err := strconv.ParseBool(mval); err == nil && val {
return true
}
// RouterDisableHTTP2Annotation present and true|false depending on
// the annotation's value that is parsed by strconv.ParseBool.
func HTTP2IsDisabledByAnnotation(m map[string]string) (bool, bool) {
if val, ok := m[RouterDisableHTTP2Annotation]; ok {
v, _ := strconv.ParseBool(val)
return true, v
}
return false, false
}

// HTTP2IsDisabled returns true if the ingress controller disables
// http/2 via the RouterDisableHTTP2Annotation, or if the ingress
// config disables http/2. It will return false for the case where the
// ingress config has been disabled but the ingress controller
// explicitly overrides that by having the annotation present (even if
// its value is "false").
func HTTP2IsDisabled(ic *operatorv1.IngressController, ingressConfig *configv1.Ingress) bool {
controllerHasHTTP2Annotation, controllerHasHTTP2Disabled := HTTP2IsDisabledByAnnotation(ic.Annotations)
_, configHasHTTP2Disabled := HTTP2IsDisabledByAnnotation(ingressConfig.Annotations)

if controllerHasHTTP2Annotation {
return controllerHasHTTP2Disabled
}
return false

return configHasHTTP2Disabled
}

// desiredRouterDeployment returns the desired router deployment.
Expand Down Expand Up @@ -514,7 +530,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
env = append(env, corev1.EnvVar{Name: WildcardRouteAdmissionPolicy, Value: "false"})
}

if HTTP2IsDisabledByAnnotation(ci.Annotations) || HTTP2IsDisabledByAnnotation(ingressConfig.Annotations) {
if HTTP2IsDisabled(ci, ingressConfig) {
env = append(env, corev1.EnvVar{Name: RouterDisableHTTP2EnvName, Value: "true"})
}

Expand Down
147 changes: 78 additions & 69 deletions test/e2e/http2_disable_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,89 +25,126 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
)

const routerDeployTimeout time.Duration = 1 * time.Minute

// NOTE: This test will mutate the default ingresscontroller and the
// "cluster" ingress configuration.
func TestRouteHTTP2EnableAndDisable(t *testing.T) {
routerDeployTimeout := 1 * time.Minute

func TestRouteHTTP2EnableAndDisableIngressController(t *testing.T) {
if err := waitForIngressControllerCondition(kclient, 5*time.Minute, defaultName, defaultAvailableConditions...); err != nil {
t.Fatalf("failed to observe expected conditions: %v", err)
}
ic, ingressConfig, routerDeployment, err := http2RefreshTestObjects(t, kclient, 1*time.Minute)

ic, err := http2GetIngressController(t, kclient, 1*time.Minute)
if err != nil {
t.Fatalf("failed to get test objects: %v", err)
t.Fatalf("failed to get ingress controller: %v", err)
}
if err := checkHTTP2IsEnabled(ic, ingressConfig, routerDeployment); err != nil {
t.Fatalf("test assertions failed: %v", err)

// By default the router should not have http/2 disabled
if err := waitForRouterDeploymentHTTP2Disabled(kclient, routerDeployTimeout, ic, false); err != nil {
t.Fatalf("expected router deployment to have http/2 enabled: %v", err)
}

// Disable http/2 on the default ingresscontroller
if err := setHTTP2DisabledForIngressController(t, kclient, 1*time.Minute, true, ic); err != nil {
t.Fatalf("failed to update ingresscontroller: %v", err)
}

if err := waitForIngressControllerCondition(kclient, 5*time.Minute, defaultName, defaultAvailableConditions...); err != nil {
t.Fatalf("failed to observe expected conditions: %v", err)
}

if err := waitForRouterDeploymentHTTP2Disabled(kclient, routerDeployTimeout, ic, true); err != nil {
t.Fatalf("expected router deployment to have http/2 disabled: %v", err)
}

// Revert the previous change to the default ingresscontroller
if err := setHTTP2DisabledForIngressController(t, kclient, 1*time.Minute, false, ic); err != nil {
t.Fatalf("failed to update ingresscontroller: %v", err)
}

if err := waitForRouterDeploymentHTTP2Disabled(kclient, routerDeployTimeout, ic, false); err != nil {
t.Fatalf("expected router deployment to have http/2 enabled: %v", err)
}
}

func TestRouteHTTP2EnableAndDisableIngressConfig(t *testing.T) {
if err := waitForIngressControllerCondition(kclient, 5*time.Minute, defaultName, defaultAvailableConditions...); err != nil {
t.Fatalf("failed to observe expected conditions: %v", err)
}
if err := waitForRouterDeploymentHTTP2Disabled(kclient, routerDeployTimeout, ic, false); err != nil {
t.Fatalf("expected router deployment to have http/2 enabled: %v", err)

ic, err := http2GetIngressController(t, kclient, 1*time.Minute)
if err != nil {
t.Fatalf("failed to get ingress controller: %v", err)
}

//
// Now assert that changing the "cluster" ingress
// configuration will also disable/enable http/2 for all
// ingress controllers.
ic, ingressConfig, routerDeployment, err = http2RefreshTestObjects(t, kclient, 1*time.Minute)
if err := setHTTP2DisabledForIngressController(t, kclient, 1*time.Minute, false, ic); err != nil {
t.Fatalf("failed to update ingress config: %v", err)
}

ingressConfig, err := http2GetIngressConfig(t, kclient, 1*time.Minute)
if err != nil {
t.Fatalf("failed to get test objects: %v", err)
t.Fatalf("failed to get ingress config: %v", err)
}
if err := checkHTTP2IsEnabled(ic, ingressConfig, routerDeployment); err != nil {
t.Fatalf("test assertions failed: %v", err)

// By default the router should not have http/2 disabled
if err := waitForRouterDeploymentHTTP2Disabled(kclient, routerDeployTimeout, ic, false); err != nil {
t.Fatalf("expected router deployment to have http/2 enabled: %v", err)
}

// Disable http/2 on the ingress config "cluster"
if err := setHTTP2DisabledForIngressConfig(t, kclient, 1*time.Minute, true, ingressConfig); err != nil {
t.Fatalf("failed to update ingress config: %v", err)
}

if err := waitForIngressControllerCondition(kclient, 5*time.Minute, defaultName, defaultAvailableConditions...); err != nil {
t.Fatalf("failed to observe expected conditions: %v", err)
}

if err := waitForRouterDeploymentHTTP2Disabled(kclient, routerDeployTimeout, ic, true); err != nil {
t.Fatalf("expected router deployment to have http/2 disabled: %v", err)
}

// Revert the previous change to the ingress config
ic, ingressConfig, routerDeployment, err = http2RefreshTestObjects(t, kclient, 1*time.Minute)
if err != nil {
t.Fatalf("failed to get test objects: %v", err)
}
if err := setHTTP2DisabledForIngressConfig(t, kclient, 1*time.Minute, false, ingressConfig); err != nil {
t.Fatalf("failed to update ingress config: %v", err)
// Now set the ingress controller to also have the annotation
// but with an explicit value of "false" which will take
// precedence over the ingress config which is currently
// "true". The net result should be that the router deployment
// transitions from its current state of http/2 disabled
// (which we just asserted) back to http/2 enabled.
//
// This update is different to
// setHTTP2DisabledForIngressController() as we preserve the
// annotation if the value is "false".
if err := wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) {
if err := kclient.Get(context.TODO(), defaultName, ic); err != nil {
t.Logf("Get failed: %v, retrying...", err)
return false, nil
}
if ic.Annotations == nil {
ic.Annotations = map[string]string{}
}
ic.Annotations[ingress.RouterDisableHTTP2Annotation] = "false"
if err := kclient.Update(context.TODO(), ic); err != nil {
t.Logf("failed to update ingress controller: %v", err)
return false, nil
}
return true, nil
}); err != nil {
t.Fatalf("failed to update ingress controller: %v", err)
}

if err := waitForIngressControllerCondition(kclient, 5*time.Minute, defaultName, defaultAvailableConditions...); err != nil {
t.Fatalf("failed to observe expected conditions: %v", err)
}

if err := waitForRouterDeploymentHTTP2Disabled(kclient, routerDeployTimeout, ic, false); err != nil {
t.Fatalf("expected router deployment to have http/2 enabled: %v", err)
t.Fatalf("expected router deployment to have http/2 disabled: %v", err)
}

// Assert all our mutations are back to their initial state
ic, ingressConfig, routerDeployment, err = http2RefreshTestObjects(t, kclient, 1*time.Minute)
if err != nil {
t.Fatalf("failed to get test objects: %v", err)
// cleanup

if err := setHTTP2DisabledForIngressController(t, kclient, 1*time.Minute, false, ic); err != nil {
t.Fatalf("failed to update ingress config: %v", err)
}
if err := checkHTTP2IsEnabled(ic, ingressConfig, routerDeployment); err != nil {
t.Fatalf("test assertions failed: %v", err)

if err := setHTTP2DisabledForIngressConfig(t, kclient, 1*time.Minute, false, ingressConfig); err != nil {
t.Fatalf("failed to update ingress config: %v", err)
}
}

Expand All @@ -122,7 +159,7 @@ func http2IsDisabledInEnv(env []corev1.EnvVar) bool {
return false
}

func http2RefreshTestObjects(t *testing.T, client client.Client, timeout time.Duration) (*operatorv1.IngressController, *configv1.Ingress, *appsv1.Deployment, error) {
func http2GetIngressController(t *testing.T, client client.Client, timeout time.Duration) (*operatorv1.IngressController, error) {
ic := operatorv1.IngressController{}
if err := wait.PollImmediate(1*time.Second, timeout, func() (bool, error) {
if err := client.Get(context.TODO(), defaultName, &ic); err != nil {
Expand All @@ -131,20 +168,12 @@ func http2RefreshTestObjects(t *testing.T, client client.Client, timeout time.Du
}
return true, nil
}); err != nil {
return nil, nil, nil, fmt.Errorf("failed to get %q: %v", defaultName, err)
}

deployment := appsv1.Deployment{}
if err := wait.PollImmediate(1*time.Second, timeout, func() (bool, error) {
if err := client.Get(context.TODO(), controller.RouterDeploymentName(&ic), &deployment); err != nil {
t.Logf("Get %q failed: %v, retrying...", controller.RouterDeploymentName(&ic), err)
return false, nil
}
return true, nil
}); err != nil {
return nil, nil, nil, fmt.Errorf("failed to get default ingresscontroller router deployment: %v", err)
return nil, fmt.Errorf("failed to get %q: %v", defaultName, err)
}
return &ic, nil
}

func http2GetIngressConfig(t *testing.T, client client.Client, timeout time.Duration) (*configv1.Ingress, error) {
ingressConfig := configv1.Ingress{}
if err := wait.PollImmediate(1*time.Second, timeout, func() (bool, error) {
name := types.NamespacedName{Name: "cluster"}
Expand All @@ -154,30 +183,10 @@ func http2RefreshTestObjects(t *testing.T, client client.Client, timeout time.Du
}
return true, nil
}); err != nil {
return nil, nil, nil, fmt.Errorf("failed to get ingress configuration: %v", err)
return nil, fmt.Errorf("failed to get ingress configuration: %v", err)
}

return &ic, &ingressConfig, &deployment, nil
}

func checkHTTP2IsEnabled(ic *operatorv1.IngressController, ingressConfig *configv1.Ingress, d *appsv1.Deployment) error {
icName := fmt.Sprintf("%s/%s", ic.Namespace, ic.Name)
deploymentName := fmt.Sprintf("%s/%s", d.Namespace, d.Name)
ingressConfigName := fmt.Sprintf("%s/%s", ingressConfig.Namespace, ingressConfig.Name)

if ingress.HTTP2IsDisabledByAnnotation(ic.Annotations) {
return fmt.Errorf("expected ingress controller %q to have HTTP/2 enabled by default", icName)
}
if ingress.HTTP2IsDisabledByAnnotation(ingressConfig.Annotations) {
return fmt.Errorf("expected ingress config %q to have HTTP/2 enabled by default", ingressConfigName)
}
if len(d.Spec.Template.Spec.Containers) < 1 {
return fmt.Errorf("expected deployment %q to have at least one container", deploymentName)
}
if http2IsDisabledInEnv(d.Spec.Template.Spec.Containers[0].Env) {
return fmt.Errorf("expected deployment %q to have http/2 enabled by default", deploymentName)
}
return nil
return &ingressConfig, nil
}

func waitForRouterDeploymentHTTP2Disabled(client client.Client, timeout time.Duration, c *operatorv1.IngressController, expectedDisabledStatus bool) error {
Expand Down