-
Notifications
You must be signed in to change notification settings - Fork 69
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 Virtual Machines related dashboards #1613
Add Virtual Machines related dashboards #1613
Conversation
5bae7a4
to
766ee1f
Compare
Hi @moadz , please review the following dashboards. Thank you. |
name: grafana-dashboard-acm-virtual-machines-inventory | ||
namespace: open-cluster-management-observability | ||
annotations: | ||
observability.open-cluster-management.io/dashboard-folder: "ACM / OpenShift Virtualization" |
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.
@moadz is there another place I need to create the folder or this is enough?
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.
Is just "OpenShift Virtualisation"
sufficient afaik there are only ACM dashboards in that grafana anyway, so the top level folder is redundant.
766ee1f
to
f7972f8
Compare
aa6056f
to
e4db282
Compare
46acad7
to
3b1b00e
Compare
/hold |
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.
LGTM pending we can go through this together @sradco let's organise some time to view them live :)
"id": 29, | ||
"uid": "OmpD1ZoSk", | ||
"iteration": 1709210609633, | ||
"id": 45, |
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.
One note is all the dashboards need a unique UUID as per this PR
https://github.com/stolostron/multicluster-observability-operator/pull/1566/files if they ever link back to themselves in the dashboard definition.
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.
All dashboards have a uid
- kubevirt_vmi_info | ||
- kubevirt_vm_running_status_last_transition_timestamp_seconds | ||
- kubevirt_vm_non_running_status_last_transition_timestamp_seconds | ||
- kubevirt_vm_error_status_last_transition_timestamp_seconds | ||
- kubevirt_vm_starting_status_last_transition_timestamp_seconds | ||
- kubevirt_vm_migrating_status_last_transition_timestamp_seconds | ||
- kubevirt_vmi_memory_available_bytes | ||
- kubevirt_vmi_phase_count | ||
- kubevirt_vmi_cpu_usage_seconds_total | ||
- kubevirt_vmi_network_receive_bytes_total | ||
- kubevirt_vmi_network_transmit_bytes_total | ||
- kubevirt_vmi_storage_iops_read_total | ||
- kubevirt_vmi_storage_iops_write_total | ||
|
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.
Not relevant to this PR, but we probably want to maintain independent allowlists in the future as we support more than one allowlist.
} | ||
] | ||
}, | ||
"description": "This dashboard provides a comprehensive list of all virtual machines across clusters, offering detailed insights into each VM’s configuration. Users can easily search and filter VMs based on various attributes, enabling quick access to specific information for effective management and troubleshooting.", |
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.
Might be worth getting @dockerymick to do a review of the copy on these dashboards. I think there's a style guide from the doc writers they would probably like to maintain here as well.
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.
"description": "This dashboard provides a comprehensive list of all virtual machines across clusters, offering detailed insights into each VM’s configuration. Users can easily search and filter VMs based on various attributes, enabling quick access to specific information for effective management and troubleshooting.", | |
"description": "This dashboard provides a comprehensive list of all virtual machines deployed in a given managed cluster, offering detailed insights into each VM’s configuration. Users can easily search and filter VMs based on their attributes, enabling quick access to specific information for effective management and troubleshooting.", |
} | ||
] | ||
}, | ||
"description": "This dashboard provides a quick overview of the health status of Virtual Machines (VMs) across clusters in the KubeVirt environment. It helps users identify VMs that are currently in unhealthy states and those that have been in such states for an extended period, potentially making them candidates for cleanup. Use the filters to customize the view based on cluster, namespace, VM name, and duration in an unhealthy state for efficient monitoring and management.", |
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.
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.
Some tags for @dockerymick
} | ||
] | ||
}, | ||
"description": "This dashboard provides detailed insights into a specific virtual machine, including its configuration, resource consumption, and any active alerts. It allows users to easily locate the virtual machine across multiple clusters, facilitating efficient monitoring and troubleshooting.", |
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.
@@ -21,44 +21,41 @@ data: | |||
} | |||
] | |||
}, | |||
"description": "This dashboard shows details related to the OpenShift Virtualization operator and Virtual machines", | |||
"description": "This dashboard provides insights into the health of the OpenShift Virtualization operator and displays key metrics for virtual machines at the cluster level across multiple clusters. It helps monitor and assess the status of virtualization resources, ensuring smooth operation and quick identification of issues.", |
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.
} | ||
] | ||
}, | ||
"description": "This dashboard provides comprehensive insights into the cluster with OpenShift Virtualization installed. It offers detailed information about the virtual machines (VMs), associated resources, and any active alerts, enabling efficient monitoring and management of virtualization within the cluster.", |
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 @moadz for tagging in me in areas that need a review. I will get to this ASAP |
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 suggested to use sentence-case for titles because we use sentence-case for the RHACM console (which follows PatternFly guidance). I also did not do a full review of each file because the number of lines. I did a complete review of the first .yaml file. Essentially, most of my suggestions would probably be similar to `dash-acm-openshift-virtualization-overview.yaml
...iclusterobservability/manifests/base/grafana/dash-acm-openshift-virtualization-overview.yaml
Show resolved
Hide resolved
...iclusterobservability/manifests/base/grafana/dash-acm-openshift-virtualization-overview.yaml
Show resolved
Hide resolved
...iclusterobservability/manifests/base/grafana/dash-acm-openshift-virtualization-overview.yaml
Show resolved
Hide resolved
...iclusterobservability/manifests/base/grafana/dash-acm-openshift-virtualization-overview.yaml
Show resolved
Hide resolved
} | ||
], | ||
"title": "OpenShift Virtualization Health by Cluster", | ||
"title": "Number of VMs started in the last 7 days", |
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 notice that this title is sentenced-cased
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.
Hi @dockerymick , What did you mean here?
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 just agree with this change. This is basically what I am asking for when I say sentence case
...rs/multiclusterobservability/manifests/base/grafana/dash-acm-virtual-machines-inventory.yaml
Show resolved
Hide resolved
...rs/multiclusterobservability/manifests/base/grafana/dash-acm-virtual-machines-inventory.yaml
Show resolved
Hide resolved
...rs/multiclusterobservability/manifests/base/grafana/dash-acm-virtual-machines-inventory.yaml
Show resolved
Hide resolved
...rs/multiclusterobservability/manifests/base/grafana/dash-acm-virtual-machines-inventory.yaml
Show resolved
Hide resolved
...rs/multiclusterobservability/manifests/base/grafana/dash-acm-virtual-machines-inventory.yaml
Outdated
Show resolved
Hide resolved
3b1b00e
to
a2ca0c1
Compare
69d4853
to
39dfe5e
Compare
/unhold |
/lgtm |
/retest |
39dfe5e
to
a70a4e6
Compare
Hi @moadz, I had to rebase so there LGTM was removed |
/lgtm |
Before merging, can we address my suggestions or at least have a discussion about them ? |
Can we address them in a separate PR? I believe this is urgent. @moadz ? |
Yes discussed it with @dockerymick we can do a follow up with the docs fixes i'm just keen to get the dashboards QE'ed, Mikela are you able to remove the request changes? |
a70a4e6
to
12322c5
Compare
In this PR we introduce 4 dashboard related to Virtual Machines: 1. Service Level dashboards - Virtual Machines by Time in Status 2. Inventory dashboards - Virtual Machines Inventory 3. Executive dashboards - Single Cluster View 4. Executive dashboards - Single Virtual Machine View And update the "ACM - OpenShift Virtualization Overview" name to "Executive dashboards - Clusters Overview" and enriched it with additional details. Also moved all dashboards under the "ACM / OpenShift Virtualization" folder. Signed-off-by: Shirly Radco <sradco@redhat.com>
12322c5
to
aaaa718
Compare
@moadz Im sorry, I had to get an important fix that would impact future support of the dashboards |
I think you can just merge without doing the request or just resolve them. I am on PTO. I can go through the resolve if yall are unable to merge |
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.
resolved my suggestions
Quality Gate passedIssues Measures |
/retest |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: moadz, sradco The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@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. |
In this PR we introduce 4 dashboards related to Virtual Machines:
And update the "ACM - OpenShift Virtualization Overview"
name to "Executive dashboards - Clusters Overview"
and enriched it with additional details.
Also moved all dashboards under the "ACM / OpenShift Virtualization"
folder.
Signed-off-by: Shirly Radco sradco@redhat.com