-
Notifications
You must be signed in to change notification settings - Fork 440
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
DontReview Garnitin/add gke load testing/v1 #2225
base: garnitin/add-gke-load-testing/enhancements/remove-mash-dependency
Are you sure you want to change the base?
DontReview Garnitin/add gke load testing/v1 #2225
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2225 +/- ##
==========================================
+ Coverage 77.02% 77.04% +0.01%
==========================================
Files 113 113
Lines 15910 15918 +8
==========================================
+ Hits 12255 12264 +9
+ Misses 3129 3128 -1
Partials 526 526
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
fi | ||
|
||
function validateMachineConfig() { | ||
echo "Validiting input parameters ..." |
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.
typo
local cluster_name=${1} | ||
local zone=${2} | ||
local node_pool=${3} | ||
if [ $(gcloud container node-pools list --cluster=${cluster_name} --zone=${zone} | grep -ow ${node_pool} | wc -l) -gt 0 ] ; then |
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.
simplify this with grep -q
if ClusterExists ${cluster_name} ; then | ||
gcloud container clusters update ${cluster_name} --location=${zone} --workload-pool=${project_id}.svc.id.goog | ||
else | ||
# gcloud container --project "${project_id}" clusters create ${cluster_name} --zone "${zone}" --cluster-version "${cluster_version}" --workload-pool=${project_id}.svc.id.goog |
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.
remove commented code
echo "Enabling/disabling csi add-on ..." | ||
# By default, disable the managed csi driver. | ||
if ${useCustomCsiDriver}; then | ||
# gcloud -q container clusters update ${cluster_name} \ |
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.
re-enable this
gcloud container clusters get-credentials ${cluster_name} --location=${zone} | ||
kubectl create namespace ${appnamespace} | ||
kubectl create serviceaccount ${ksa} --namespace ${appnamespace} | ||
for workload_bucket in ${buckets} ; 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.
add code to get buckets from somewhere.
} | ||
|
||
# validateMachineConfig ${machine_type} ${num_nodes} ${num_ssd} | ||
# installDependencies |
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.
re-enable all these disabled steps.
def get_cpu(pod_name: str, start: str, end: str) -> Tuple[float, float]: | ||
# for some reason, the mash filter does not always work, so we fetch all the metrics for all the pods and filter later. | ||
result = subprocess.run(["mash", "--namespace=cloud_prod", "--output=csv", | ||
f"Query(Fetch(Raw('cloud.kubernetes.K8sContainer', 'kubernetes.io/container/cpu/core_usage_time'), {{'project': '927584127901'}})| Window(Rate('10m'))| GroupBy(['pod_name', 'container_name'], Max()), TimeInterval('{start}', '{end}'), '5s')"], |
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.
put the project-number back to the original and change it by code during runtime and then revert it back when done during runtime.
function updateMachineTypeInPodConfigs() { | ||
for file in ${gke_testing_dir}/examples/fio/loading-test/values.yaml ${gke_testing_dir}/examples/dlio/unet3d-loading-test/values.yaml ; do | ||
test -f ${file} | ||
sed -i -E "s/nodeType: [0-9a-z_-]+$/nodeType: ${machine_type}/g" ${file} |
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.
add code to revert this back
for file in ${gke_testing_dir}/examples/fio/loading-test/values.yaml ${gke_testing_dir}/examples/dlio/unet3d-loading-test/values.yaml ; do | ||
test -f ${file} | ||
# sed -i -E "s/mountOptions: [0-9a-zA-Z,\:\"-_]+$/mountOptions: \"${gcsfuse_mount_options}\"/g" ${file} | ||
sed -i -E "s/mountOptions:[ \t]*\"?[a-zA-Z0-9,:_-]+\"? *$/mountOptions: \"${gcsfuse_mount_options}\"/g" "${file}" |
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.
add code to revert this back when done
2958a3c
to
ad97f82
Compare
return utc_timestamp_string | ||
|
||
def standard_timestamp(timestamp: int) -> str: | ||
return timestamp.split('.')[0].replace('T', ' ') + " UTC" |
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.
insert newline after this line
@@ -0,0 +1,425 @@ | |||
#!/bin/bash | |||
|
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.
Add a header and a help
option at the top.
ff7ee55
to
478921c
Compare
1e4353a
to
b6a0e76
Compare
b62b517
to
d008a84
Compare
0daf723
to
6dbb552
Compare
53def3b
to
d423275
Compare
ace404a
to
841048c
Compare
d423275
to
711aa0f
Compare
841048c
to
c1f5947
Compare
711aa0f
to
789b5f4
Compare
c1f5947
to
db9edf9
Compare
789b5f4
to
a3feb0d
Compare
db9edf9
to
1ea03c4
Compare
a3feb0d
to
8294243
Compare
1ea03c4
to
fab819c
Compare
8294243
to
dcfb13b
Compare
fab819c
to
137aed1
Compare
dcfb13b
to
924758e
Compare
137aed1
to
8c4d392
Compare
924758e
to
f2f6b68
Compare
8c4d392
to
e3ce844
Compare
f2f6b68
to
44243a3
Compare
e3ce844
to
e8828d2
Compare
44243a3
to
7c135ea
Compare
e8828d2
to
6f6bf16
Compare
7c135ea
to
5d01790
Compare
6f6bf16
to
45b39ce
Compare
5d01790
to
8d14955
Compare
45b39ce
to
02f144c
Compare
8d14955
to
b9dbc4e
Compare
02f144c
to
06cc29b
Compare
b9dbc4e
to
b4bd35d
Compare
06cc29b
to
3496cd4
Compare
Description
Link to the issue in case of a bug fix.
NA
Testing details