From 7fa7362ddc0e8a0b85cffcaebc38abd772b355e2 Mon Sep 17 00:00:00 2001 From: Utku Ozdemir Date: Thu, 4 Jan 2024 12:05:32 +0100 Subject: [PATCH] fix: fix nodes on dashboard footer when node names are used in `--nodes` When the dashboard is used via the CLI through a proxy, e.g., through Omni, node names or IDs can be used in the `--nodes` flag instead of the IPs. This caused rendering inconsistencies in the dashboard, as some parts of it used the IPs and some used the names passed in the context. Fix this by collecting all node IPs on dashboard start, and map these IPs to the respective nodes passed as the `--nodes` flag. On the dashboard footer, we always display the node names as they are passed in the `--nodes` flag. As part of it, remove the node list change reactivity from the dashboard, so it will always take the passed nodes as the truth. The IP to node mapping collection at dashboard startup also solves another issue where the first API call by the dashboard triggered the interactive API authentication (e.g., the OIDC flow). Previously, because the terminal was already switched to the raw mode, it was not possible to authenticate properly. Signed-off-by: Utku Ozdemir --- internal/pkg/dashboard/components/footer.go | 10 +- internal/pkg/dashboard/dashboard.go | 194 ++++++++++++++------ 2 files changed, 139 insertions(+), 65 deletions(-) diff --git a/internal/pkg/dashboard/components/footer.go b/internal/pkg/dashboard/components/footer.go index b60594f860..2dafefcf08 100644 --- a/internal/pkg/dashboard/components/footer.go +++ b/internal/pkg/dashboard/components/footer.go @@ -27,7 +27,7 @@ type Footer struct { } // NewFooter initializes Footer. -func NewFooter(screenKeyToName map[string]string) *Footer { +func NewFooter(screenKeyToName map[string]string, nodes []string) *Footer { var initialScreen string for _, name := range screenKeyToName { initialScreen = name @@ -39,6 +39,7 @@ func NewFooter(screenKeyToName map[string]string) *Footer { TextView: *tview.NewTextView(), screenKeyToName: screenKeyToName, selectedScreen: initialScreen, + nodes: nodes, } widget.SetDynamicColors(true) @@ -65,13 +66,6 @@ func NewFooter(screenKeyToName map[string]string) *Footer { return widget } -// OnNodeSetChange implements the NodeSetListener interface. -func (widget *Footer) OnNodeSetChange(nodes []string) { - widget.nodes = nodes - - widget.refresh() -} - // OnNodeSelect implements the NodeSelectListener interface. func (widget *Footer) OnNodeSelect(node string) { widget.selectedNode = node diff --git a/internal/pkg/dashboard/dashboard.go b/internal/pkg/dashboard/dashboard.go index ce0c75bf28..94d776d207 100644 --- a/internal/pkg/dashboard/dashboard.go +++ b/internal/pkg/dashboard/dashboard.go @@ -9,7 +9,9 @@ import ( "context" "errors" "fmt" - "sort" + "net/netip" + "slices" + "strings" "time" "github.com/gdamore/tcell/v2" @@ -18,7 +20,9 @@ import ( "github.com/rivo/tview" "github.com/siderolabs/gen/maps" "github.com/siderolabs/gen/xslices" + "github.com/siderolabs/go-api-signature/pkg/message" "golang.org/x/sync/errgroup" + "google.golang.org/grpc/metadata" "github.com/siderolabs/talos/internal/pkg/dashboard/apidata" "github.com/siderolabs/talos/internal/pkg/dashboard/components" @@ -69,11 +73,6 @@ type LogDataListener interface { OnLogDataChange(node string, logLine string) } -// NodeSetListener is a listener which is notified when the set of nodes changes. -type NodeSetListener interface { - OnNodeSetChange(nodes []string) -} - // NodeSelectListener is a listener which is notified when a node is selected. type NodeSelectListener interface { OnNodeSelect(node string) @@ -103,11 +102,10 @@ type Dashboard struct { resourceDataSource *resourcedata.Source logDataSource *logdata.Source - apiDataListeners []APIDataListener - resourceDataListeners []ResourceDataListener - logDataListeners []LogDataListener - nodeSelectListeners []NodeSelectListener - nodeSetChangeListeners []NodeSetListener + apiDataListeners []APIDataListener + resourceDataListeners []ResourceDataListener + logDataListeners []LogDataListener + nodeSelectListeners []NodeSelectListener app *tview.Application @@ -124,6 +122,7 @@ type Dashboard struct { selectedNodeIndex int selectedNode string nodeSet map[string]struct{} + ipsToNodeAliases map[string]string nodes []string } @@ -137,11 +136,22 @@ func buildDashboard(ctx context.Context, cli *client.Client, opts ...Option) (*D opt(defOptions) } + // map node IPs to their aliases (names/IPs - as specified "nodes" in context). + // this will also trigger the interactive API authentication if needed - e.g., when the API is used through Omni. + ipsToNodeAliases, err := collectNodeIPsToNodeAliases(ctx, cli) + if err != nil { + return nil, err + } + + nodes := getSortedNodeAliases(ipsToNodeAliases) + dashboard := &Dashboard{ - cli: cli, - interval: defOptions.interval, - app: tview.NewApplication(), - nodeSet: make(map[string]struct{}), + cli: cli, + interval: defOptions.interval, + app: tview.NewApplication(), + nodeSet: make(map[string]struct{}), + nodes: nodes, + ipsToNodeAliases: ipsToNodeAliases, } dashboard.mainGrid = tview.NewGrid(). @@ -155,8 +165,7 @@ func buildDashboard(ctx context.Context, cli *client.Client, opts ...Option) (*D header := components.NewHeader() dashboard.mainGrid.AddItem(header, 0, 0, 1, 1, 0, 0, false) - err := dashboard.initScreenConfigs(ctx, defOptions.screens) - if err != nil { + if err = dashboard.initScreenConfigs(ctx, defOptions.screens); err != nil { return nil, err } @@ -168,7 +177,7 @@ func buildDashboard(ctx context.Context, cli *client.Client, opts ...Option) (*D return config.keyCode, config }) - dashboard.footer = components.NewFooter(screenKeyToName) + dashboard.footer = components.NewFooter(screenKeyToName, nodes) dashboard.app.SetInputCapture(func(event *tcell.EventKey) *tcell.EventKey { config, screenOk := screenConfigByKeyCode[event.Key()] @@ -214,10 +223,6 @@ func buildDashboard(ctx context.Context, cli *client.Client, opts ...Option) (*D dashboard.footer, } - dashboard.nodeSetChangeListeners = []NodeSetListener{ - dashboard.footer, - } - for _, config := range dashboard.screenConfigs { screenPrimitive := config.primitive @@ -240,11 +245,6 @@ func buildDashboard(ctx context.Context, cli *client.Client, opts ...Option) (*D if ok { dashboard.nodeSelectListeners = append(dashboard.nodeSelectListeners, nodeSelectListener) } - - nodeSetListener, ok := screenPrimitive.(NodeSetListener) - if ok { - dashboard.nodeSetChangeListeners = append(dashboard.nodeSetChangeListeners, nodeSetListener) - } } dashboard.apiDataSource = &apidata.Source{ @@ -313,6 +313,8 @@ func Run(ctx context.Context, cli *client.Client, opts ...Option) (runErr error) return err } + dashboard.selectNodeByIndex(0) + // handle panic & stop dashboard gracefully on exit defer func() { if r := recover(); r != nil { @@ -387,18 +389,24 @@ func (d *Dashboard) startDataHandler(ctx context.Context) func() error { case <-ctx.Done(): return ctx.Err() case nodeLog := <-d.logDataSource.LogCh: + nodeAlias := d.attemptResolveIPToAlias(nodeLog.Node) + if time.Since(lastLogTime) < 50*time.Millisecond { d.app.QueueUpdate(func() { - d.processLog(nodeLog.Node, nodeLog.Log) + d.processLog(nodeAlias, nodeLog.Log) }) } else { d.app.QueueUpdateDraw(func() { - d.processLog(nodeLog.Node, nodeLog.Log) + d.processLog(nodeAlias, nodeLog.Log) }) } lastLogTime = time.Now() case d.data = <-dataCh: + d.data.Nodes = maps.Map(d.data.Nodes, func(key string, v *apidata.Node) (string, *apidata.Node) { + return d.attemptResolveIPToAlias(key), v + }) + d.app.QueueUpdateDraw(func() { d.processAPIData() }) @@ -440,10 +448,6 @@ func (d *Dashboard) processAPIData() { return } - for _, node := range maps.Keys(d.data.Nodes) { - d.processSeenNode(node) - } - for _, component := range d.apiDataListeners { component.OnAPIDataChange(d.selectedNode, d.data) } @@ -451,8 +455,6 @@ func (d *Dashboard) processAPIData() { // processNodeResource re-renders the components with new resource data. func (d *Dashboard) processNodeResource(nodeResource resourcedata.Data) { - d.processSeenNode(nodeResource.Node) - for _, component := range d.resourceDataListeners { component.OnResourceDataChange(nodeResource) } @@ -465,28 +467,6 @@ func (d *Dashboard) processLog(node, line string) { } } -func (d *Dashboard) processSeenNode(node string) { - _, exists := d.nodeSet[node] - if exists { - return - } - - d.nodeSet[node] = struct{}{} - - nodes := maps.Keys(d.nodeSet) - - sort.Strings(nodes) - - d.nodes = nodes - - for _, listener := range d.nodeSetChangeListeners { - listener.OnNodeSetChange(nodes) - } - - // we received a new node, so we re-select the first node - d.selectNodeByIndex(0) -} - func (d *Dashboard) selectScreen(screen Screen) { for _, info := range d.screenConfigs { info := info @@ -506,3 +486,103 @@ func (d *Dashboard) selectScreen(screen Screen) { d.footer.SelectScreen(string(screen)) } + +// attemptResolveIPToAlias attempts to resolve the given node IP to its alias as it appears in "nodes" in the context. +// If the IP is not found in the context, the IP is returned as-is. +func (d *Dashboard) attemptResolveIPToAlias(node string) string { + if resolved, ok := d.ipsToNodeAliases[node]; ok { + return resolved + } + + return node +} + +// collectNodeIPsToNodeAliases probes all nodes in the context for their IP addresses by calling their .Version endpoint and maps them to the node aliases in the context. +// +// Sample output: +// +// 172.20.0.6 -> node-1 +// +// 10.42.0.1 -> node-1 +// +// 172.20.0.7 -> node-2 +// +// 10.42.0.2 -> node-2. +func collectNodeIPsToNodeAliases(ctx context.Context, c *client.Client) (map[string]string, error) { + ipsToNodeAliases := make(map[string]string) + + nodes := nodeAliasesInContext(ctx) + for _, node := range nodes { + ctx = client.WithNodes(ctx, node) // do not replace this with "WithNode" - it would not return the IP in the response metadata + + resp, err := c.Version(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get node %q version: %w", node, err) + } + + if len(resp.GetMessages()) == 0 { + return nil, fmt.Errorf("node %q returned no messages in version response", node) + } + + nodeIP := resp.GetMessages()[0].GetMetadata().GetHostname() + if nodeIP == "" { + return nil, fmt.Errorf("node %q returned no IP in version response", node) + } + + ipsToNodeAliases[nodeIP] = node + } + + return ipsToNodeAliases, nil +} + +// nodeAliasesInContext extracts the node aliases (IP, name etc.) from the given context which are stored in the "node" or "nodes" GRPC metadata. +func nodeAliasesInContext(ctx context.Context) []string { + md, mdOk := metadata.FromOutgoingContext(ctx) + if !mdOk { + return nil + } + + nodeVal := md.Get("node") + if len(nodeVal) > 0 { + return []string{nodeVal[0]} + } + + nodesVal := md.Get(message.NodesHeaderKey) + + return xslices.FlatMap(nodesVal, func(node string) []string { + return strings.Split(node, ",") + }) +} + +// getSortedNodeAliases returns the unique node aliases sorted by their IP address. +func getSortedNodeAliases(ipToNodeAliases map[string]string) []string { + if len(ipToNodeAliases) == 0 { // assume that it is the local node (running on TTY) + return []string{""} + } + + nodeAliases := maps.Keys(xslices.ToSet(maps.Values(ipToNodeAliases))) // eliminate duplicates + + // if the aliases are IP addresses, compare them as IPs + // otherwise, compare them as strings + // all IPs come before non-IPs + slices.SortFunc(nodeAliases, func(a, b string) int { + addrA, aErr := netip.ParseAddr(a) + addrB, bErr := netip.ParseAddr(b) + + if aErr != nil && bErr != nil { + return strings.Compare(a, b) + } + + if aErr != nil { + return 1 + } + + if bErr != nil { + return -1 + } + + return addrA.Compare(addrB) + }) + + return nodeAliases +}