Use manifests specific to kruize-demos which runs load internally for techempower and petclinic benchmarks#177
Merged
dinogun merged 7 commits intokruize:mainfrom Apr 7, 2026
Conversation
Signed-off-by: kusuma chalasani <kchalasa@redhat.com>
Reviewer's GuideAdjusts local monitoring demo setup and benchmark helpers to use kruize-demos manifests with in-cluster load generation for TechEmpower and Petclinic, while simplifying benchmark arguments and centralizing Quarkus labeling/setup logic. Sequence diagram for updated benchmark install with in-cluster load generationsequenceDiagram
actor Developer
participant LocalMonitoringScripts as local_monitoring_common_sh
participant CommonHelper as common_common_helper_sh
participant Kubernetes as KubernetesCluster
Developer->>LocalMonitoringScripts: run kruize_local_demo_setup(demo=runtimes, experiments)
LocalMonitoringScripts->>Kubernetes: create_namespace APP_NAMESPACE
LocalMonitoringScripts->>Kubernetes: delete job petclinic-load-generator
LocalMonitoringScripts->>Kubernetes: delete job tfb-qrh-load-generator
Note over LocalMonitoringScripts,CommonHelper: Install TechEmpower with kruize-demos manifests
LocalMonitoringScripts->>CommonHelper: benchmarks_install(APP_NAMESPACE, tfb, kruize-demos, bench2)
CommonHelper->>Kubernetes: kubectl apply -f manifests/default_manifests for tfb
Kubernetes-->>CommonHelper: tfb pods ready
Note over LocalMonitoringScripts,CommonHelper: Install Petclinic with kruize-demos manifests
LocalMonitoringScripts->>CommonHelper: benchmarks_install(APP_NAMESPACE, petclinic, kruize-demos)
CommonHelper->>Kubernetes: kubectl apply -f manifests/kruize-demos for petclinic
Kubernetes-->>CommonHelper: petclinic pods and internal load jobs ready
Note over LocalMonitoringScripts,Kubernetes: Quarkus pod labeling and monitoring enablement
alt CLUSTER_TYPE is minikube or kind
LocalMonitoringScripts->>Kubernetes: kubectl get pod | grep tfb-qrh-sample
LocalMonitoringScripts->>Kubernetes: kubectl label pod quarkus_label
LocalMonitoringScripts->>LocalMonitoringScripts: enable_kube_state_metrics_labels.sh
else CLUSTER_TYPE is openshift
LocalMonitoringScripts->>Kubernetes: oc get pod | grep tfb-qrh-sample
LocalMonitoringScripts->>Kubernetes: oc label pod quarkus_label
LocalMonitoringScripts->>LocalMonitoringScripts: enable_user_workload_monitoring_openshift.sh
end
LocalMonitoringScripts-->>Developer: demo environment with in-cluster load ready
Updated class diagram for benchmark helper and demo setup functionsclassDiagram
class LocalMonitoringCommonSh {
+kruize_local_demo_setup()
}
class CommonHelperSh {
+benchmarks_install(APP_NAMESPACE, BENCHMARK, MANIFESTS)
+apply_benchmark_load(APP_NAMESPACE, BENCHMARK, LOAD_DURATION)
}
LocalMonitoringCommonSh --> CommonHelperSh : uses
class benchmarks_install_changes {
- BENCHMARK2 parameter removed
+ BENCHMARK controls both tfb and petclinic install
+ kubectl apply -f manifests/MANIFESTS for petclinic
}
class apply_benchmark_load_changes {
- BENCHMARK2 parameter removed
+ BENCHMARK selects tfb or petclinic load
+ petclinic load path keyed on BENCHMARK==petclinic
}
CommonHelperSh .. benchmarks_install_changes : updated behavior
CommonHelperSh .. apply_benchmark_load_changes : updated behavior
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
kusumachalasani
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Since
apply_benchmark_loadno longer takesBENCHMARK2, double-check all other call sites to ensure they only pass the updated argument list and are not still relying on a fourth parameter that will now be interpreted asLOAD_DURATION. - If tfb load is now always run inside the container and
apply_benchmark_loadis only used for petclinic, consider removing or clearly deprecating the tfb-specific branch inapply_benchmark_loadto avoid confusion about how load is triggered for tfb.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since `apply_benchmark_load` no longer takes `BENCHMARK2`, double-check all other call sites to ensure they only pass the updated argument list and are not still relying on a fourth parameter that will now be interpreted as `LOAD_DURATION`.
- If tfb load is now always run inside the container and `apply_benchmark_load` is only used for petclinic, consider removing or clearly deprecating the tfb-specific branch in `apply_benchmark_load` to avoid confusion about how load is triggered for tfb.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: kusuma chalasani <kchalasa@redhat.com>
kusumachalasani
changed the title
Contributor
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="monitoring/local_monitoring/common.sh" line_range="549" />
<code_context>
+ kubectl delete job petclinic-load-generator -n ${APP_NAMESPACE} --ignore-not-found >> "${LOG_FILE}" 2>&1
+ kubectl delete job tfb-qrh-load-generator -n ${APP_NAMESPACE} --ignore-not-found >> "${LOG_FILE}" 2>&1
+
+ benchmarks_install ${APP_NAMESPACE} ${bench} "kruize-demos" ${bench2} >> "${LOG_FILE}" 2>&1
+ benchmarks_install ${APP_NAMESPACE} ${bench2} "kruize-demos" >> "${LOG_FILE}" 2>&1
echo "✅ Completed!"
fi
</code_context>
<issue_to_address>
**suggestion:** Calls to `benchmarks_install` still pass a 4th argument that the function no longer uses.
These calls still pass a 4th argument (`${bench2}`) even though the function now only accepts three parameters. Please remove the extra argument so the call sites match the updated signature and don’t imply unused semantics.
```suggestion
benchmarks_install ${APP_NAMESPACE} ${bench} "kruize-demos" >> "${LOG_FILE}" 2>&1
```
</issue_to_address>
### Comment 2
<location path="common/common_helper.sh" line_range="368-372" />
<code_context>
popd >/dev/null
fi
- if [ ${BENCHMARK2} == "petclinic" ]; then
+ if [ ${BENCHMARK} == "petclinic" ]; then
echo "5. Installing spring petclinic benchmark into cluster"
pushd spring-petclinic >/dev/null
- kubectl apply -f manifests -n ${APP_NAMESPACE}
+ kubectl apply -f manifests/${MANIFESTS} -n ${APP_NAMESPACE}
check_err "ERROR: spring petclinic failed to start, exiting"
popd >/dev/null
</code_context>
<issue_to_address>
**issue (bug_risk):** Switching petclinic to use `manifests/${MANIFESTS}` may break existing non–`kruize-demos` layouts.
Previously, petclinic always applied from `manifests/`. Now it uses `manifests/${MANIFESTS}` (defaulting to `default_manifests`), which will fail when only `manifests/` exists for non–`kruize-demos` layouts. Consider either appending `/${MANIFESTS}` only when `MANIFESTS` is not the default, or checking for `manifests/${MANIFESTS}` and falling back to `manifests/` to keep existing setups working.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
chandrams
reviewed
Mar 3, 2026
| kubectl delete job petclinic-load-generator -n ${APP_NAMESPACE} --ignore-not-found >> "${LOG_FILE}" 2>&1 | ||
| kubectl delete job tfb-qrh-load-generator -n ${APP_NAMESPACE} --ignore-not-found >> "${LOG_FILE}" 2>&1 | ||
|
|
||
| benchmarks_install ${APP_NAMESPACE} ${bench} "kruize-demos" ${bench2} >> "${LOG_FILE}" 2>&1 |
Contributor
There was a problem hiding this comment.
Remove bench2 here as we are calling benchmarks_install again
| kubectl delete job tfb-qrh-load-generator -n ${APP_NAMESPACE} --ignore-not-found >> "${LOG_FILE}" 2>&1 | ||
|
|
||
| benchmarks_install ${APP_NAMESPACE} ${bench} "kruize-demos" ${bench2} >> "${LOG_FILE}" 2>&1 | ||
| benchmarks_install ${APP_NAMESPACE} ${bench2} "kruize-demos" >> "${LOG_FILE}" 2>&1 |
Contributor
There was a problem hiding this comment.
Also now that load is made internal, do you want to remove the benchmark and load related options in runtimes_demo script?
Signed-off-by: kusuma chalasani <kchalasa@redhat.com>
Signed-off-by: kusuma chalasani <kchalasa@redhat.com>
chandrams
reviewed
Mar 12, 2026
| @@ -78,26 +75,15 @@ export KRUIZE_OPERATOR_IMAGE="" | |||
| while getopts bc:d:kfi:lm:no:pstu: gopts | |||
Contributor
There was a problem hiding this comment.
@kusumachalasani Can you please update the readme with the missing options
Signed-off-by: kusuma chalasani <kchalasa@redhat.com>
Signed-off-by: kusuma chalasani <kchalasa@redhat.com>
Signed-off-by: kusuma chalasani <kchalasa@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Instead of running load externally in a tfb-benchmark, use the manifests of the benchmark which runs the load internally in the tfb-qrh pod.
Uses the manifests from kruize/benchmarks#71
Summary by Sourcery
Update local monitoring benchmark setup to run TechEmpower load generation from within the tfb container/manifests instead of externally and align helper script parameters with the new behavior.
Enhancements:
Summary by Sourcery
Switch local monitoring demo benchmarks to use kruize-demos-specific manifests and internal load generation for TechEmpower and Petclinic, while centralizing Quarkus pod labeling and monitoring setup.
Enhancements: