Skip ESIPP [Slow] suite of networking tests for huge clusters #55275
Conversation
|
@shyamjvs: Adding do-not-merge/release-note-label-needed because the release note process has not been followed. One of the following labels is required "release-note", "release-note-action-required", or "release-note-none". Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
sig/network
shyamjvs
added
release-note-none
shyamjvs
force-pushed
the
skip-esipp-slow-tests-on-large-cluster
branch
from
fe2c627
to
9356c78
Compare
k8s-ci-robot
added
the
cncf-cla: yes
| @@ -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) | |||
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
So do you suggest adding this check for each of the e2e's except that one?
There was a problem hiding this comment.
I think it is fine :) It may be too messy to separate them.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MrHohn, shyamjvs Associated issue: 52495 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
|
/test pull-kubernetes-unit |
|
/retest |
1 similar comment
|
/retest |
|
/test pull-kubernetes-unit |
|
/retest Review the full test history for this PR. |
|
/test pull-kubernetes-e2e-kops-aws |
|
cc @kubernetes/test-infra-maintainers Seems like kops is down? |
|
ref #55303, working on it |
|
I manually merged kubernetes/test-infra#5402, prow was not responsive last night, rekicking all the jobs /test pull-kubernetes-e2e-kops-aws |
|
/retest Review the full test history for this PR. |
|
Automatic merge from submit-queue (batch tested with PRs 54177, 55203, 55120, 55275, 55260). If you want to cherry-pick this change to another branch, please follow the instructions here. |
Ref #52495 (comment)
/cc @MrHohn @kubernetes/sig-network-misc