Feature Request: Show ready/not ready, node type via node icons

[mowings]

First great job on the dashboard -- seems a lot more lightweight and less buggy than the standard dashboard. We have a couple of minor UI requests:

On the nodes page it would be nice if unready nodes showed up by default on top, and maybe with a red icon, instead of the text 'READY' column. Even without the icon change, it would be good to float unready nodes to the top by default (we run on bare metal, so unready nodes are a big deal for us). We have alerts, obviously, but it still seems like a logical change to the UI.

It would also be great if on that same page it was a bit more obvious which nodes were masters. You can look at the labels, but an icon change (or replacing the READY column with a MASTER column) would be pretty nice.

Thanks again for all your great work!

[olivergg]

1. One could change the READY column to display some kind of a checkmark (instead of just rendering the boolean value) (would need an icon for that)
2. I would change the icon in the type column to symbolize the fact that it is a master, but not the READY column which should just display the status, not the nature of the node 😉
   @herbrandson I'm willing to do that if it's okay for you 😅

[olivergg]

What about :
?
(I've just used two utf-8 symbols for that : U+2713 and U+1F6C7)

[herbrandson]

@olivergg really like this :)

I do believe there are actually 3 states here. Ready, Not Ready, and Unknown. Where "Not Ready" means the k8s can communicate with the server, but it's not ready yet. And "Unknown" means the server is unreachable.

Also, I think it might look a little nicer if the icons were centered horizontally. What do you think?

@mowings Currently, you can sort by the "Ready" column to place the "not ready" nodes at the top of the list (which I do a lot). Will that work for you for the short term?

[olivergg]

@herbrandson Yes, you're right, I thought the "Ready" state was a boolean.... I should have read the documentation https://kubernetes.io/docs/concepts/architecture/nodes/#condition 🙈
I'm not a CSS expert, but I'll try do to my best 😅

As for the sorts, one could sort by default by the "Ready" column and then by the "Name" column. Therefore, if all nodes are ready, it's the same as the current sort 😉

[olivergg]

@mowings I would need more time to understand how the sort works (and to properly implement it in an idiomatic way), but for now, the #60 PR makes the things a little bit prettier;)

[mowings]

Thanks @olivergg -- I will have a look when I get a chance!

[mowings]

Just to add an additional note, at least for us it's probably more important that unready or unknown status nodes float to the top. Sorting by master or worker node is not really that big a deal here

[mowings]

Just built and deployed with @olivergg s pr. Definitely a big improvement. If the sort could float unready nodes to the top it would be perfect. Thanks again for your work!

[mowings]

@olivergg

Actually, it looks like the PR causes breaks the namespace pane. When navigating to the namespace pane, I get an error message. While there are no api errors, the javascript console shows:

TypeError: Cannot convert undefined or null to object
    at Function.entries (<anonymous>)
    at dt (listViewHelpers.js:76)

The code in question is:

 {/** If the node is a master, display a simple "MASTER" label below the item.kind icon */}
                {Object.entries(item.metadata.labels).some(([key, _]) => key === "node-role.kubernetes.io/master")
                    && <span className='td_iconLabel node-master'>MASTER</span>}

This seems to be broken on the namespace pane. I am guessing this is because namespaces (or at least mine) don't have any labels.

EDIT: Same issue with the config pane

[mowings]

I left a comment in the PR with the fix -- you do need to check that labels actually exists, Thanxs!

[herbrandson]

Yikes! I missed this comment before merging that PR :(

@olivergg I ran into similar issues in various other places and as a result had started using lodash functions instead of things like Object.entries because it deals with null/undefined values.

@mowings I saw you had another PR. Was it the patch for this issue?

[olivergg]

@herbrandson thanks, I'll have a look at lodash. Meanwhile, f2ee674 was the actual fix for this missing check. Regarding the other PR, I guess, an interractive rebase should be necessary from @mowings 😉

[mowings]

@herbrandson @olivergg -- I closed out that PR and submitted a clean one just to fix the ApiVersion issue.

My master has diverged a bit from yours -- so I resubmitted from a clean branch. Should be good to go.

[olivergg]

@herbrandson I've added a fix to my branch 8de135d to use lodash function ._has

[mowings]

Hey guys -- I am going to close this issue out and create another one just for the sorting unready masters to the top part.

[herbrandson]

Perfect. Thanks @mowings