distribute services between 2 namespaces in e2e DNS scale services test #65599
Conversation
k8s-ci-robot
added
release-note-none
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/test pull-kubernetes-e2e-gce |
| ns1, _ := f.CreateNamespace(f.BaseName, nil) | ||
| ns2, _ := f.CreateNamespace(f.BaseName, nil) | ||
|
|
||
| services := generateServicesInNamespaces([]string{ns1.Name, ns2.Name}, maxServicesPerCluster/2) |
There was a problem hiding this comment.
Instead of just introducing this maginc constant "2", can you please introduce a constant:
"maxServicesPerNamespace" (for now set it to 5000)
and then:
- create necessary number of namesapces (maxServicesPerCluster + maxServicesPerNamespace - 1 / maxServicesPerNamespace)
- in each of namespaces create necessary number of services?
k8s-ci-robot
removed
the
lgtm
|
/test pull-kubernetes-bazel-test |
| @@ -50,10 +51,22 @@ var _ = SIGDescribe("[Feature:PerformanceDNS]", func() { | |||
|
|
|||
| // answers dns for service - creates the maximum number of services, and then check dns record for one | |||
| It("Should answer DNS query for maximum number of services per cluster", func() { | |||
| services := generateServicesInNamespace(f.Namespace.Name, maxServicesPerCluster) | |||
|
|
|||
| numNs := maxServicesPerCluster / maxServicesPerNamespace | |||
There was a problem hiding this comment.
Why not
numNs := (maxServicesPerCluster + maxServicesPerNamespace - 1) / maxServicesPerNamespace
There was a problem hiding this comment.
Can you help me understand the reasoning/logic behind your formula?
Common sense would say ...
( 10000 + 5000 + 1 ) / 5000 = 3.0002
But AFAIU, in Go this is instead "3", since it's an integer division, not a float division.
Why split between 3 namespaces? If we can fit them in 2...
And why the +1? To protect against float representation of 3 of 2.9999... ? I don't think thats necessary, since AFAIU the evaluation is all integer.
There was a problem hiding this comment.
Not +1, but -1.
Then you get:
(10000 + 5000 - 1) / 5000 = 2.99.. which will be 2 [updated].
(A+B-1)/B is integer ceiling of A/B
|
/test pull-kubernetes-e2e-gce |
| @@ -50,10 +51,21 @@ var _ = SIGDescribe("[Feature:PerformanceDNS]", func() { | |||
|
|
|||
| // answers dns for service - creates the maximum number of services, and then check dns record for one | |||
| It("Should answer DNS query for maximum number of services per cluster", func() { | |||
| services := generateServicesInNamespace(f.Namespace.Name, maxServicesPerCluster) | |||
|
|
|||
| } | ||
|
|
||
| services := generateServicesInNamespaces(namespaces, maxServicesPerCluster) | ||
|
|
|
/retest |
|
/test pull-kubernetes-e2e-gce |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chrisohaver, rajansandeep, wojtek-t The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/retest Review the full test history for this PR. Silence the bot with an |
|
/test pull-kubernetes-e2e-gce |
|
Automatic merge from submit-queue (batch tested with PRs 65348, 65599, 65635, 65688, 65691). If you want to cherry-pick this change to another branch, please follow the instructions here. |
What this PR does / why we need it:
What: Alters the dns scale test, to distribute the scale load of 10K services between 2 namespaces, so that the test does not fail to create the services.
Why: To allow the dns test to proceed.
Expect to Fix #64774 but wont know until it's actually run in e2e tests, so not marking that issue to auto-close on merge. FWIW, it does pass in local tests using hack/local-up-cluster.sh.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Special notes for your reviewer:
Release note: