fix: multiple fixes for dashboard/no data

There were multiple issues: * `hostname` in the header is coming from resources, and the one in API data was unused * we should update to `n/a` if the value is not available * if the node is unavailable, the response contains a message with placeholders with a proxy error, and that should be honored to ignore the response; if instead we use it as it is, it puts zero value, which results in funny output. Fixes #10383 Signed-off-by: Andrey Smirnov <andrey.smirnov@siderolabs.com>
siderolabs · Feb 25, 2025 · 468e318 · 468e318
1 parent 3dd8d9a
commit 468e318
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 131 deletions.
diff --git a/.github/ISSUE_TEMPLATE/BUG_REPORT.md b/.github/ISSUE_TEMPLATE/BUG_REPORT.md
@@ -15,5 +15,5 @@ assignees: ""
 ### Environment
 
 - Talos version: [`talosctl version --nodes <problematic nodes>`]
-- Kubernetes version: [`kubectl version --short`]
+- Kubernetes version: [`kubectl version`]
 - Platform:
diff --git a/internal/pkg/dashboard/apidata/node.go b/internal/pkg/dashboard/apidata/node.go
@@ -13,7 +13,6 @@ import (
 // Node represents data gathered from a single node.
 type Node struct {
 	// These fields are directly API responses.
-	Hostname      *machine.Hostname
 	LoadAvg       *machine.LoadAvg
 	Version       *machine.Version
 	Memory        *machine.Memory

diff --git a/internal/pkg/dashboard/apidata/source.go b/internal/pkg/dashboard/apidata/source.go
@@ -14,6 +14,7 @@ import (
 
 	"github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers"
 	"github.com/siderolabs/talos/internal/pkg/dashboard/resolver"
+	"github.com/siderolabs/talos/pkg/machinery/api/machine"
 	"github.com/siderolabs/talos/pkg/machinery/client"
 )
 
@@ -85,7 +86,30 @@ func (source *Source) run(dataCh chan<- *Data) {
 	}
 }
 
-//nolint:gocyclo,cyclop
+type protoMsg[T any] interface {
+	GetMessages() []T
+}
+
+func unpack[T helpers.Message](source *Source, nodes map[string]*Node, resultLock *sync.Mutex, resp protoMsg[T], setter func(node *Node, value T)) {
+	resultLock.Lock()
+	defer resultLock.Unlock()
+
+	for _, msg := range resp.GetMessages() {
+		node := source.node(msg)
+
+		if _, ok := nodes[node]; !ok {
+			nodes[node] = &Node{}
+		}
+
+		if msg.GetMetadata().GetError() != "" {
+			continue
+		}
+
+		setter(nodes[node], msg)
+	}
+}
+
+//nolint:gocyclo
 func (source *Source) gather() *Data {
 	result := &Data{
 		Timestamp: time.Now(),
@@ -101,18 +125,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].LoadAvg = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.LoadAvg) {
+				node.LoadAvg = value
+			})
 
 			return nil
 		},
@@ -122,18 +137,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].Version = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.Version) {
+				node.Version = value
+			})
 
 			return nil
 		},
@@ -143,18 +149,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].Memory = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.Memory) {
+				node.Memory = value
+			})
 
 			return nil
 		},
@@ -164,18 +161,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].SystemStat = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.SystemStat) {
+				node.SystemStat = value
+			})
 
 			return nil
 		},
@@ -185,18 +173,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].CPUsFreqStats = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.CPUsFreqStats) {
+				node.CPUsFreqStats = value
+			})
 
 			return nil
 		},
@@ -206,18 +185,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].CPUsInfo = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.CPUsInfo) {
+				node.CPUsInfo = value
+			})
 
 			return nil
 		},
@@ -227,18 +197,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].NetDevStats = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.NetworkDeviceStats) {
+				node.NetDevStats = value
+			})
 
 			return nil
 		},
@@ -248,18 +209,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].DiskStats = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.DiskStats) {
+				node.DiskStats = value
+			})
 
 			return nil
 		},
@@ -269,18 +221,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].Processes = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.Process) {
+				node.Processes = value
+			})
 
 			return nil
 		},
@@ -290,18 +233,9 @@ func (source *Source) gather() *Data {
 				return err
 			}
 
-			resultLock.Lock()
-			defer resultLock.Unlock()
-
-			for _, msg := range resp.GetMessages() {
-				node := source.node(msg)
-
-				if _, ok := result.Nodes[node]; !ok {
-					result.Nodes[node] = &Node{}
-				}
-
-				result.Nodes[node].ServiceList = msg
-			}
+			unpack(source, result.Nodes, &resultLock, resp, func(node *Node, value *machine.ServiceList) {
+				node.ServiceList = value
+			})
 
 			return nil
 		},

diff --git a/internal/pkg/dashboard/components/header.go b/internal/pkg/dashboard/components/header.go
@@ -81,9 +81,9 @@ func (widget *Header) OnResourceDataChange(data resourcedata.Data) {
 
 // OnAPIDataChange implements the APIDataListener interface.
 func (widget *Header) OnAPIDataChange(node string, data *apidata.Data) {
-	nodeAPIData := data.Nodes[node]
-
-	widget.updateNodeAPIData(node, nodeAPIData)
+	for node, nodeData := range data.Nodes {
+		widget.updateNodeAPIData(node, nodeData)
+	}
 
 	if node == widget.selectedNode {
 		widget.redraw()
@@ -132,16 +132,16 @@ func (widget *Header) updateNodeAPIData(node string, data *apidata.Node) {
 	nodeData.cpuUsagePercent = fmt.Sprintf("%.1f%%", data.CPUUsageByName("usage")*100.0)
 	nodeData.memUsagePercent = fmt.Sprintf("%.1f%%", data.MemUsage()*100.0)
 
-	if data.Hostname != nil {
-		nodeData.hostname = data.Hostname.GetHostname()
-	}
-
 	if data.Version != nil {
 		nodeData.version = data.Version.GetVersion().GetTag()
+	} else {
+		nodeData.version = notAvailable
 	}
 
-	if data.SystemStat != nil {
+	if data.SystemStat != nil && data.SystemStat.BootTime != 0 {
 		nodeData.uptime = time.Since(time.Unix(int64(data.SystemStat.GetBootTime()), 0)).Round(time.Second).String()
+	} else {
+		nodeData.uptime = notAvailable
 	}
 
 	if data.CPUsInfo != nil {
@@ -150,6 +150,8 @@ func (widget *Header) updateNodeAPIData(node string, data *apidata.Node) {
 		if numCPUs > 0 {
 			nodeData.cpuFreq = fmt.Sprintf("%dx%s", numCPUs, widget.humanizeCPUFrequency(data.CPUsInfo.GetCpuInfo()[0].GetCpuMhz()))
 		}
+	} else {
+		nodeData.cpuFreq = notAvailable
 	}
 
 	if data.CPUsFreqStats != nil && data.CPUsFreqStats.CpuFreqStats != nil {
@@ -183,14 +185,20 @@ func (widget *Header) updateNodeAPIData(node string, data *apidata.Node) {
 
 			nodeData.cpuFreq += fmt.Sprintf("%dx%s", uniqMhz[mhz], widget.humanizeCPUFrequency(float64(mhz)/1000.0))
 		}
+	} else {
+		nodeData.cpuFreq = notAvailable
 	}
 
 	if data.Processes != nil {
 		nodeData.numProcesses = strconv.Itoa(len(data.Processes.GetProcesses()))
+	} else {
+		nodeData.numProcesses = notAvailable
 	}
 
 	if data.Memory != nil {
 		nodeData.totalMem = humanize.IBytes(data.Memory.GetMeminfo().GetMemtotal() << 10)
+	} else {
+		nodeData.totalMem = notAvailable
 	}
 }