[WIP] Add Cluster Memory Load Perses dashboard #3835
Conversation
Signed-off-by: Shirly Radco <[email protected]>
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
kubevirt-bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubevirt-bot
added
dco-signoff: yes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `controllers/observability/perses.go:23` </location>
<code_context>
+
+// ReconcilePersesResources applies all Perses resources shipped with the operator under assets/perses.
+// It uses server-side apply to create/update resources idempotently without relying on cache reads.
+func (r *Reconciler) ReconcilePersesResources(ctx context.Context) error {
+ wd, err := os.Getwd()
+ if err != nil {
</code_context>
<issue_to_address>
**issue (complexity):** Consider embedding asset files at compile time and refactoring the logic into smaller functions for improved clarity and efficiency.
```go
import (
"context"
"embed"
"fmt"
"io/fs"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"
)
// 1) Embed all YAMLs at compile time, drop os.Getwd/fs.DirFS entirely.
//go:embed assets/dashboards/perses/*.yaml
var persesAssets embed.FS
func (r *Reconciler) ReconcilePersesResources(ctx context.Context) error {
// 2) Single WalkDir + tiny helper to remove nesting
return fs.WalkDir(persesAssets, ".", func(path string, d fs.DirEntry, err error) error {
if err != nil || d.IsDir() {
return err
}
return r.applyAsset(ctx, path)
})
}
func (r *Reconciler) applyAsset(ctx context.Context, path string) error {
// 3) Read + unmarshal YAML straight into Unstructured.Object
data, err := persesAssets.ReadFile(path)
if err != nil {
return fmt.Errorf("read %s: %w", path, err)
}
u := &unstructured.Unstructured{}
if err := yaml.Unmarshal(data, &u.Object); err != nil {
return fmt.Errorf("parse %s: %w", path, err)
}
// 4) Override namespace consistently
u.SetNamespace(r.namespace)
// 5) Apply via SSA as before
if err := r.Patch(ctx, u, client.Apply, client.FieldOwner("hyperconverged-operator"), client.ForceOwnership); err != nil {
return fmt.Errorf("apply %s: %w", path, err)
}
return nil
}
```
- Embeds assets to avoid runtime `os.Getwd` + `fs.DirFS` boilerplate.
- Splits walking, loading/parsing, and applying into two small functions.
- Uses `yaml.Unmarshal` directly into `Unstructured.Object` to drop the YAML→JSON→map round-trip.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Pull Request Test Coverage Report for Build 19075895534Details
💛 - Coveralls |
|
hco-e2e-upgrade-operator-sdk-sno-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-operator-sdk-sno-aws In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
hco-e2e-operator-sdk-sno-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-sno-aws In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
hco-e2e-operator-sdk-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-aws, ci/prow/hco-e2e-operator-sdk-gcp In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
hco-e2e-upgrade-prev-operator-sdk-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-operator-sdk-aws, ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
hco-e2e-kv-smoke-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@sradco: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
hco-e2e-upgrade-operator-sdk-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-operator-sdk-aws, ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
4 participants
What this PR does / why we need it:
Reviewer Checklist
Jira Ticket:
Release note: