Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Show all sbd devices #801

Merged
merged 6 commits into from
Feb 18, 2022

Conversation

arbulu89
Copy link
Contributor

@arbulu89 arbulu89 commented Feb 16, 2022

Fix the lack of some SBD devices visualization. When the SBD_DEVICES entry in /etc/sysconfic/sbd has quotes, the agent was not getting the sbd values correctly. This is fixed now.

Besides that, as the current frontend templates were pretty horrible when multiple devices are there, I have changed it to a table, and added the status of the device itself.

This status is not totally OK, as it only shows the sbd status of the DC node. This should be a matrix with the status of all nodes. If you agree I can open a tech-debt issue and we can work it in the future, as it is not that simple.

image

PD: @abravosuse how do you see this new visualization?

@diegoakechi
Copy link
Contributor

@arbulu89 the SBD device is the same shared to all the nodes (each node has an slot on it). Based on recent feedbacks, I would try to keep the visualization as clean as possible, like it is right now on this PR, instead of expanding a matrix that shows green multiple times. In case some node loses its SBD, we mark red and show somehow the failed node name and error. This way the user spends time only on nodes demanding actions.

@arbulu89
Copy link
Contributor Author

@arbulu89 the SBD device is the same shared to all the nodes (each node has an slot on it). Based on recent feedbacks, I would try to keep the visualization as clean as possible, like it is right now on this PR, instead of expanding a matrix that shows green multiple times. In case some node loses its SBD, we mark red and show somehow the failed node name and error. This way the user spends time only on nodes demanding actions.

Thank you for the feedback! Really valuable!
In any case, the tech-debt is still there, as we don't store the status per host (it is not only visualization) we couldn't have this aggregates status information.

@diegoakechi
Copy link
Contributor

The current makes sense. The tech debit could be implemented in the future in my POV

Copy link
Member

@nelsonkopliku nelsonkopliku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@arbulu89 arbulu89 merged commit 979b17c into trento-project:main Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

3 participants