-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add fio-standalone Helm chart #1
base: main
Are you sure you want to change the base?
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.
Thanks for the PR! I do want to include both single storage class and comparison benchmark in the chart. I found the comparison benchmark is very useful.
README.md
Outdated
@@ -28,7 +28,7 @@ Latency in ns (Read/Write) | |||
CPU Idleness: 72% | |||
``` | |||
|
|||
### Example Result of Comparsion Benchmark | |||
### Example Result of Comparison Benchmark |
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.
I've cleaned up the README.md and fix a few typos.
README.md
Outdated
To deploy with all defaults: | ||
```bash | ||
helm upgrade --install fio-benchmark-default \ | ||
yasker-benchmark/fio-standalone \ |
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 can just call it fio
. I think we should be able to cover both single and compare cases in the chart. Comparison is very useful, don't want to just leave it to yaml, lol.
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.
My thought was we could move a comparison benchmark into "install the helm chart multiple times" to allow N comparisons easily - maybe we could rework the README to have a YAML-based comparison and a Helm-based comparison? The Helm comparison would be something like helm install ... -f values-local.yaml; helm install ... -f values-longhorn.yaml
.
helm upgrade --install fio-benchmark-default \ | ||
yasker-benchmark/fio-standalone \ | ||
-n fio-benchmarks \ | ||
--version v0.1.0 |
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.
Yeah, I should release a v0.1.0.
-n fio-benchmarks \ | ||
--version v0.1.0 \ | ||
--set pvc.size=11Gi \ | ||
--set benchmark.size=10G |
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.
Can we automatically set PVC size to benchmark size * 1.1? Not sure if the chart can do that though.
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.
Good call, will update templates and values
@@ -0,0 +1,5 @@ | |||
apiVersion: v2 | |||
name: fio-standalone |
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.
just fio
.
@@ -0,0 +1,5 @@ | |||
apiVersion: v2 | |||
name: fio-standalone | |||
description: Benchmark a single Kubernetes StorageClass |
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.
E.g. Benchmark and compare Kubernetes StorageClass performance.
image: | ||
repository: "yasker/benchmark" | ||
pullPolicy: "IfNotPresent" | ||
tag: "latest" |
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.
should be v0.1.0
.
README.md
Outdated
@@ -92,9 +92,9 @@ Something is *wrong* when: | |||
* For **Latency**, the **CPU Idleness** should be at least 40% to guarantee the test won't be impacted by CPU starving. | |||
* If this happens, adding more CPUs to the node, or move to a beefer machine. | |||
|
|||
### Deploy the benchmark | |||
## Deploy the FIO benchmark |
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.
I am keeping it nested since I think we can add more benchmarks later.
Allow Helm-based installation for easier repeated deployments; add instructions and examples to README.