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

Skip ESIPP [Slow] suite of networking tests for huge clusters #55275

Merged
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
4 changes: 4 additions & 0 deletions test/e2e/network/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,10 @@ var _ = SIGDescribe("ESIPP [Slow]", func() {
// requires cloud load-balancer support - this feature currently supported only on GCE/GKE
framework.SkipUnlessProviderIs("gce", "gke")

// Skipping this test for too large clusters due to issue #52495.
// TODO(MrHohn): Get rid of this when gce-side load-balancer improvements are done.
framework.SkipUnlessNodeCountIsAtMost(framework.GCPMaxInstancesInInstanceGroup)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

I am hesitate about which exact tests should we skip --- not all of ESIPP tests will create LB (like should work for type=NodePort does not). Also, should we skip other non-ESIPP tests that also create LB (like should only allow access from service loadbalancer source ranges and should be able to change the type and ports of a service)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the tests that seem to be failing (https://k8s-testgrid.appspot.com/sig-scalability-gce#gce-scale-correctness) are also the ones in this suite. So I think this is fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh.. But I think you're right about the type=NodePort one.
Wrt the others on LB, let's keep them for now as they're passing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So do you suggest adding this check for each of the e2e's except that one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine :) It may be too messy to separate them.


cs = f.ClientSet
if nodes := framework.GetReadySchedulableNodesOrDie(cs); len(nodes.Items) > framework.LargeClusterMinNodesNumber {
loadBalancerCreateTimeout = framework.LoadBalancerCreateTimeoutLarge
Expand Down