-
Notifications
You must be signed in to change notification settings - Fork 242
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
test:[NPM] Adding service deployment for the scale test #2007
Conversation
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.
This is awesome. Minor comments
local numDeployments=$2 | ||
local serviceKind=$3 | ||
|
||
for i in $(seq -f "%05g" 1 $numServices); do |
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.
to make --num-services
optional, I think you'll need to set a default value of 0 somewhere
test/scale/test-scale.sh
Outdated
@@ -406,6 +427,7 @@ set -x | |||
$KUBECTL $KUBECONFIG_ARG create ns scale-test | |||
$KUBECTL $KUBECONFIG_ARG apply -f generated/kwok-nodes/ | |||
$KUBECTL $KUBECONFIG_ARG apply -f generated/deployments/real/ | |||
$KUBECTL $KUBECONFIG_ARG apply -f generated/services/real/ |
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.
We'll need to apply only if numServices > 0
. Like here:
https://github.com/Azure/azure-container-networking/pull/1975/files/24467195c5ac9edd90793a3befd95ebe029098f4#diff-7f8f18cf7c38d320fefc80745a20c806a8b4f626e8fb27bf7d009a0daf96de4f
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.
Cool, will add it.
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.
Could we also modify these NPM calculations:
- increase number of NPM IPSets by numRealDeployments when numServices > 0
- increase number of IPSet members by numRealPods
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.
So for:
- numIPSetsAddedByNPM : I can add the
numRealDeployments
ifnumServices > 0
- numIPSetMembersAddedByNPM: Add
numRealPods
ifnumServices > 0
? @huntergregory
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.
ya exactly
f04d1a7
to
0f340e0
Compare
1c0437e
to
1db3b50
Compare
Reason for Change:
Adding the option to deploy service for each set of deployments for scale test. Each service will be mapped to the respective deployment(having certain number of pods per deployment)
Issue Fixed:
Requirements:
Notes: