-
Notifications
You must be signed in to change notification settings - Fork 222
NE-577: Support a Configurable ROUTER_MAX_CONNECTIONS in HAproxy #735
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
Merged
openshift-merge-robot
merged 5 commits into
openshift:master
from
frobware:haproxy-maxconn
May 9, 2022
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
62e8849
ingress: change deployment to use spec.tuningOptions.MaxConnections
frobware bcbad4b
test/e2e: add test for tuningOptions.MaxConnections
frobware c6a01f4
Drop maxConnections from unsupportedConfigOverrides
frobware 8d52365
vendor: https://github.com/openshift/api/pull/1161
frobware 2c8f12e
Regenerate CRD
frobware File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| //go:build e2e | ||
| // +build e2e | ||
|
|
||
| package e2e | ||
|
|
||
| import ( | ||
| "context" | ||
| "strings" | ||
| "testing" | ||
| "time" | ||
|
|
||
| operatorv1 "github.com/openshift/api/operator/v1" | ||
| "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" | ||
| "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/ingress" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
|
|
||
| appsv1 "k8s.io/api/apps/v1" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| "k8s.io/apimachinery/pkg/util/wait" | ||
| ) | ||
|
|
||
| func TestTunableMaxConnectionsValidValues(t *testing.T) { | ||
| updateMaxConnections := func(t *testing.T, client client.Client, timeout time.Duration, maxConnections int32, name types.NamespacedName) error { | ||
| return wait.PollImmediate(time.Second, timeout, func() (bool, error) { | ||
| ic := operatorv1.IngressController{} | ||
| if err := client.Get(context.TODO(), name, &ic); err != nil { | ||
| t.Logf("Get %q failed: %v, retrying...", name, err) | ||
| return false, nil | ||
| } | ||
| ic.Spec.TuningOptions.MaxConnections = maxConnections | ||
| if err := client.Update(context.TODO(), &ic); err != nil { | ||
| t.Logf("Update %q failed: %v, retrying...", name, err) | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| }) | ||
| } | ||
|
|
||
| icName := types.NamespacedName{Namespace: operatorNamespace, Name: "maxconnections-valid-values"} | ||
| domain := icName.Name + "." + dnsConfig.Spec.BaseDomain | ||
| ic := newPrivateController(icName, domain) | ||
| if err := kclient.Create(context.TODO(), ic); err != nil { | ||
| t.Fatalf("failed to create ingresscontroller %s: %v", icName, err) | ||
| } | ||
| defer assertIngressControllerDeleted(t, kclient, ic) | ||
| t.Logf("waiting for ingresscontroller %v", icName) | ||
| if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, icName, availableConditionsForPrivateIngressController...); err != nil { | ||
| t.Fatalf("failed to observe expected conditions: %v", err) | ||
| } | ||
|
|
||
| ic, err := getIngressController(t, kclient, icName, 1*time.Minute) | ||
| if err != nil { | ||
| t.Fatalf("failed to get ingress controller: %v", err) | ||
| } | ||
|
|
||
| deployment := &appsv1.Deployment{} | ||
| if err := kclient.Get(context.TODO(), controller.RouterDeploymentName(ic), deployment); err != nil { | ||
| t.Fatalf("failed to get ingresscontroller deployment: %v", err) | ||
| } | ||
|
|
||
| if err := waitForDeploymentEnvVar(t, kclient, deployment, time.Minute, ingress.RouterMaxConnectionsEnvName, ""); err != nil { | ||
| t.Fatalf("expected router deployment to have %s unset: %v", ingress.RouterMaxConnectionsEnvName, err) | ||
| } | ||
|
|
||
| for _, testCase := range []struct { | ||
| description string | ||
| maxConnections int32 | ||
| expectedEnvVar string | ||
| }{ | ||
| {"set maxconn 12345", 12345, "12345"}, | ||
| {"set maxconn auto", -1, "auto"}, | ||
frobware marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| {"set maxconn to default", 0, ""}, | ||
| } { | ||
| t.Log(testCase.description) | ||
| if err := updateMaxConnections(t, kclient, time.Minute, testCase.maxConnections, icName); err != nil { | ||
| t.Fatalf("failed to update ingresscontroller with maxConnections=%v: %v", testCase.maxConnections, err) | ||
| } | ||
| if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, icName, availableConditionsForPrivateIngressController...); err != nil { | ||
| t.Fatalf("failed to observe expected conditions: %v", err) | ||
| } | ||
| if err := waitForDeploymentEnvVar(t, kclient, deployment, time.Minute, ingress.RouterMaxConnectionsEnvName, testCase.expectedEnvVar); err != nil { | ||
| t.Fatalf("router deployment not updated with %s=%v: %v", ingress.RouterMaxConnectionsEnvName, testCase.expectedEnvVar, err) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // TestTunableMaxConnectionsInvalidValues tests that invalid values | ||
| // cannot be set for tuningOptions.maxConnections. | ||
| // | ||
| // Valid values are: | ||
| // 0 | ||
| // -1 | ||
| // 2000-2000000 | ||
| // | ||
| // so we test outside of those value and expect validation failures | ||
| // when we attempt to set them. | ||
| func TestTunableMaxConnectionsInvalidValues(t *testing.T) { | ||
| updateMaxConnections := func(t *testing.T, client client.Client, maxConnections int32, name types.NamespacedName) error { | ||
| ic := operatorv1.IngressController{} | ||
| if err := client.Get(context.TODO(), name, &ic); err != nil { | ||
| t.Logf("Get %q failed: %v, retrying...", name, err) | ||
| return err | ||
| } | ||
| ic.Spec.TuningOptions.MaxConnections = maxConnections | ||
| return client.Update(context.TODO(), &ic) | ||
| } | ||
|
|
||
| icName := types.NamespacedName{Namespace: operatorNamespace, Name: "maxconnections-invalid-values"} | ||
| domain := icName.Name + "." + dnsConfig.Spec.BaseDomain | ||
| ic := newPrivateController(icName, domain) | ||
| if err := kclient.Create(context.TODO(), ic); err != nil { | ||
| t.Fatalf("failed to create ingresscontroller %s: %v", icName, err) | ||
| } | ||
| defer assertIngressControllerDeleted(t, kclient, ic) | ||
| t.Logf("waiting for ingresscontroller %v", icName) | ||
| if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, icName, availableConditionsForPrivateIngressController...); err != nil { | ||
| t.Fatalf("failed to observe expected conditions: %v", err) | ||
| } | ||
|
|
||
| ic, err := getIngressController(t, kclient, icName, 1*time.Minute) | ||
| if err != nil { | ||
| t.Fatalf("failed to get ingress controller: %v", err) | ||
| } | ||
|
|
||
| deployment := &appsv1.Deployment{} | ||
| if err := kclient.Get(context.TODO(), controller.RouterDeploymentName(ic), deployment); err != nil { | ||
| t.Fatalf("failed to get ingresscontroller deployment: %v", err) | ||
| } | ||
|
|
||
| if err := waitForDeploymentEnvVar(t, kclient, deployment, time.Minute, ingress.RouterMaxConnectionsEnvName, ""); err != nil { | ||
| t.Fatalf("expected router deployment to have %s unset: %v", ingress.RouterMaxConnectionsEnvName, err) | ||
| } | ||
|
|
||
| for _, testCase := range []struct { | ||
| description string | ||
| maxConnections int32 | ||
| }{ | ||
| {"set maxconn -2", -2}, | ||
| {"set maxconn 1999", 1999}, | ||
| {"set maxconn 2000001", 2000001}, | ||
| } { | ||
| t.Log(testCase.description) | ||
| const expectedErr string = `"spec.tuningOptions" must validate at least one schema (anyOf), spec.tuningOptions.maxConnections` | ||
| err := updateMaxConnections(t, kclient, testCase.maxConnections, icName) | ||
| if err == nil { | ||
| t.Fatal("expected an error") | ||
| } | ||
| if !strings.Contains(err.Error(), expectedErr) { | ||
| t.Fatalf("expected error message %q, got %v", expectedErr, err) | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
7 changes: 7 additions & 0 deletions
7
vendor/github.com/openshift/api/config/v1/types_cluster_version.go
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
23 changes: 0 additions & 23 deletions
23
vendor/github.com/openshift/api/machine/v1beta1/types_awsprovider.go
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
TestDesiredRouterDeploymentSpecAndNetworkfeels like it should be at least two separate tests, but that can be a further cleanup for another day.