From 9cab34f31cf2771a85d6fab31250d79204bc6510 Mon Sep 17 00:00:00 2001 From: Kenny Udovic Date: Thu, 23 Jul 2026 14:49:59 -0400 Subject: [PATCH 1/5] fix: endpoint failover and blank screen --- crates/talos-pilot-tui/src/app.rs | 76 +++++++++- .../talos-pilot-tui/src/components/cluster.rs | 139 ++++++++++++++++-- crates/talos-rs/examples/failover_check.rs | 87 +++++++++++ crates/talos-rs/src/auth.rs | 88 ++++++++--- crates/talos-rs/src/config.rs | 102 ++++++++++--- 5 files changed, 438 insertions(+), 54 deletions(-) create mode 100644 crates/talos-rs/examples/failover_check.rs diff --git a/crates/talos-pilot-tui/src/app.rs b/crates/talos-pilot-tui/src/app.rs index 9bbe67c..8b53fb7 100644 --- a/crates/talos-pilot-tui/src/app.rs +++ b/crates/talos-pilot-tui/src/app.rs @@ -10,7 +10,7 @@ use crate::components::{ }; use crate::tui::{self, Tui}; use color_eyre::Result; -use crossterm::event::{self, Event, KeyEventKind}; +use crossterm::event::{self, Event, KeyCode, KeyEventKind, KeyModifiers}; use std::time::Duration; use tokio::sync::mpsc; @@ -93,6 +93,40 @@ impl Default for App { } } +/// Draw the animated, cancellable startup "connecting" screen. +fn draw_connecting(frame: &mut ratatui::Frame, spinner: usize) { + use ratatui::layout::Alignment; + use ratatui::style::{Color, Style}; + use ratatui::text::{Line, Span}; + use ratatui::widgets::{Block, Borders, Paragraph}; + + const FRAMES: [&str; 4] = ["|", "/", "-", "\\"]; + let spin = FRAMES[spinner % FRAMES.len()]; + + let area = frame.area(); + if area.width == 0 || area.height == 0 { + return; + } + let para = Paragraph::new(vec![ + Line::from(""), + Line::from(Span::styled( + format!("{} Connecting to Talos cluster...", spin), + Style::default().fg(Color::Cyan), + )), + Line::from(Span::styled( + "Trying configured endpoints (press q or Ctrl-C to cancel)", + Style::default().fg(Color::DarkGray), + )), + ]) + .block( + Block::default() + .borders(Borders::ALL) + .title(" talos-pilot "), + ) + .alignment(Alignment::Center); + frame.render_widget(para, area); +} + impl App { pub fn new( config_path: Option, @@ -482,13 +516,49 @@ impl App { /// Main event loop async fn main_loop(&mut self, terminal: &mut Tui) -> Result<()> { - // Connect on startup - self.cluster.connect().await?; + // Connect on startup while showing an animated, cancellable "connecting" + // screen. connect() dials all configured endpoints concurrently and the + // first reachable one wins; we drive it alongside an input poll so the + // UI never looks frozen and the user can bail out with q / Esc / Ctrl-C + // instead of feeling like the terminal is stuck. + let connect_result = { + let mut connect_fut = Box::pin(self.cluster.connect()); + let mut spinner: usize = 0; + loop { + terminal.draw(|frame| draw_connecting(frame, spinner))?; + spinner = spinner.wrapping_add(1); + + tokio::select! { + res = &mut connect_fut => break res, + _ = tokio::time::sleep(Duration::from_millis(120)) => { + // Non-blocking drain of pending input; abort keys exit cleanly. + while event::poll(Duration::from_millis(0))? { + if let Event::Key(key) = event::read()? + && key.kind == KeyEventKind::Press + { + let abort = matches!(key.code, KeyCode::Char('q') | KeyCode::Esc) + || (key.code == KeyCode::Char('c') + && key.modifiers.contains(KeyModifiers::CONTROL)); + if abort { + return Ok(()); + } + } + } + } + } + } + }; + connect_result?; loop { // Draw current view terminal.draw(|frame| { let area = frame.area(); + // Skip drawing into a zero-sized area (e.g. a 0x0 terminal or a + // momentary 0-height during resize) — ratatui panics otherwise. + if area.width == 0 || area.height == 0 { + return; + } match self.view { View::Cluster => { let _ = self.cluster.draw(frame, area); diff --git a/crates/talos-pilot-tui/src/components/cluster.rs b/crates/talos-pilot-tui/src/components/cluster.rs index 0421032..3e75dfc 100644 --- a/crates/talos-pilot-tui/src/components/cluster.rs +++ b/crates/talos-pilot-tui/src/components/cluster.rs @@ -119,6 +119,8 @@ struct ClusterData { connected: bool, /// Error message if connection failed error: Option, + /// Configured endpoints for this context (shown in connection-error details) + endpoints: Vec, /// Version info from nodes versions: Vec, /// Services from nodes @@ -177,6 +179,19 @@ impl Default for ClusterComponent { } } +/// Extract the deepest, most actionable message from an error chain. +/// +/// tonic's transport-error `Display` is just "transport error"; the real cause +/// (e.g. "Connection refused", "i/o timeout") lives at the bottom of the +/// `source()` chain. +fn root_cause(e: &dyn std::error::Error) -> String { + let mut deepest: &dyn std::error::Error = e; + while let Some(src) = deepest.source() { + deepest = src; + } + deepest.to_string() +} + impl ClusterComponent { pub fn new(config_path: Option, context_filter: Option) -> Self { Self { @@ -391,16 +406,20 @@ impl ClusterComponent { // Try to connect to each cluster using the loaded config match config.get_context(name) { - Ok(ctx) => match TalosClient::from_context(ctx).await { - Ok(client) => { - cluster.client = Some(client); - cluster.connected = true; - } - Err(e) => { - cluster.error = Some(e.to_string()); - cluster.connected = false; + Ok(ctx) => { + cluster.endpoints = ctx.endpoints.clone(); + match TalosClient::from_context(ctx).await { + Ok(client) => { + cluster.client = Some(client); + cluster.connected = true; + cluster.error = None; + } + Err(e) => { + cluster.error = Some(format!("Could not connect — {}", root_cause(&e))); + cluster.connected = false; + } } - }, + } Err(e) => { cluster.error = Some(e.to_string()); cluster.connected = false; @@ -622,6 +641,27 @@ impl ClusterComponent { } } } + + // Reflect real reachability so the UI shows a clear error instead of an + // empty/blank cluster when no endpoint can be reached. Healthy clusters + // have versions/etcd/discovery data; a transient failure keeps the + // previously cached data, so this only trips when there is nothing at all. + if let Some(cluster) = self.clusters.get_mut(cluster_idx) { + let has_any_data = !cluster.versions.is_empty() + || !cluster.etcd_members.is_empty() + || !cluster.discovery_members.is_empty(); + if has_any_data { + cluster.connected = true; + cluster.error = None; + } else { + cluster.connected = false; + cluster.error = Some( + "Unable to reach any configured Talos endpoint. Check network \ + connectivity and the endpoints in your talosconfig." + .to_string(), + ); + } + } } /// Refresh only the selected node's stats (memory, load, services) @@ -1513,6 +1553,16 @@ impl ClusterComponent { cluster_line.extend(etcd_spans); lines.push(Line::from(cluster_line)); + // Show a connection error under the cluster header (always, even + // when collapsed) so failures are visible instead of a blank list. + if let Some(err) = &cluster.error { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled("⚠ ", Style::default().fg(Color::Red)), + Span::styled(err.as_str(), Style::default().fg(Color::Red)), + ])); + } + // Skip if cluster is collapsed if !cluster.expanded { continue; @@ -1795,6 +1845,77 @@ impl ClusterComponent { let cluster_idx = self.active_cluster; let cluster = self.clusters.get(cluster_idx); + // A disconnected cluster that failed to connect has no nodes to select, + // so show the failure details here (endpoints tried + cause + hint). + if let Some(c) = cluster + && !c.connected + && let Some(err) = &c.error + { + let block = Block::default() + .title(" Connection Error ") + .title_style(Style::default().fg(Color::Red)) + .borders(Borders::ALL) + .border_style(Style::default().fg(border_color)); + + let mut lines = vec![ + Line::from(""), + Line::from(vec![ + Span::raw(" Cluster: "), + Span::styled( + c.name.clone(), + Style::default() + .fg(Color::White) + .add_modifier(Modifier::BOLD), + ), + ]), + Line::from(vec![ + Span::raw(" Status: "), + Span::styled("Disconnected", Style::default().fg(Color::Red)), + ]), + ]; + + if !c.endpoints.is_empty() { + lines.push(Line::from("")); + lines.push(Line::from(Span::styled( + " Endpoints tried:", + Style::default().fg(Color::Yellow), + ))); + for ep in &c.endpoints { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled("- ", Style::default().fg(Color::DarkGray)), + Span::styled(ep.clone(), Style::default().fg(Color::White)), + ])); + } + } + + lines.push(Line::from("")); + lines.push(Line::from(Span::styled( + " Error:", + Style::default().fg(Color::Yellow), + ))); + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled(err.clone(), Style::default().fg(Color::Red)), + ])); + + lines.push(Line::from("")); + lines.push(Line::from(Span::styled( + " Verify the endpoint(s) are reachable from this", + Style::default().dim(), + ))); + lines.push(Line::from(Span::styled( + " machine and that your talosconfig is correct.", + Style::default().dim(), + ))); + + let msg = Paragraph::new(lines) + .block(block) + .wrap(ratatui::widgets::Wrap { trim: false }); + frame.render_widget(msg, area); + return; + } + // Check if cluster is connected but has no etcd members (not bootstrapped) let needs_bootstrap = cluster .map(|c| c.connected && c.etcd_members.is_empty() && c.versions.is_empty()) diff --git a/crates/talos-rs/examples/failover_check.rs b/crates/talos-rs/examples/failover_check.rs new file mode 100644 index 0000000..64b4579 --- /dev/null +++ b/crates/talos-rs/examples/failover_check.rs @@ -0,0 +1,87 @@ +//! Smoke-test harness for multi-endpoint failover. +//! +//! Loads a talosconfig context and times a real `version()` RPC through +//! `TalosClient` (which goes via `create_channel`). Prints the endpoints, the +//! wall-clock time, and whether the call succeeded. A 30s bound keeps a dead +//! endpoint from stalling the run so old-vs-new behavior is easy to compare. +//! +//! Usage: cargo run -p talos-rs --example failover_check -- + +use std::time::{Duration, Instant}; +use talos_rs::{TalosClient, TalosConfig}; + +#[tokio::main] +async fn main() { + rustls::crypto::ring::default_provider() + .install_default() + .expect("install rustls crypto provider"); + + let args: Vec = std::env::args().collect(); + if args.len() < 3 { + eprintln!("usage: failover_check "); + std::process::exit(2); + } + let cfg_path = std::path::PathBuf::from(&args[1]); + let ctx_name = &args[2]; + + let config = TalosConfig::load_from(&cfg_path).expect("load talosconfig"); + let ctx = config.get_context(ctx_name).expect("context not found"); + + println!("context : {}", ctx_name); + println!("endpoints : {:?}", ctx.endpoints); + println!("nodes : {:?}", ctx.nodes); + + // Time the connect (now eager: races all endpoints, first reachable wins). + let start = Instant::now(); + let built = tokio::time::timeout(Duration::from_secs(35), TalosClient::from_context(ctx)).await; + let client = match built { + Err(_) => { + println!( + "RESULT : CONNECT TIMED OUT after {:.2?} (>35s)", + start.elapsed() + ); + std::process::exit(1); + } + Ok(Err(e)) => { + println!( + "RESULT : NO ENDPOINT REACHABLE after {:.2?} — {}", + start.elapsed(), + e + ); + std::process::exit(1); + } + Ok(Ok(c)) => { + println!("connected : in {:.2?}", start.elapsed()); + c + } + }; + + let outcome = tokio::time::timeout(Duration::from_secs(30), client.version()).await; + let elapsed = start.elapsed(); + + match outcome { + Ok(Ok(versions)) => { + let first = versions + .first() + .map(|v| format!("{} ({})", v.node, v.version)) + .unwrap_or_else(|| "".to_string()); + println!( + "RESULT : OK in {:.2?} — {} node(s), first = {}", + elapsed, + versions.len(), + first + ); + } + Ok(Err(e)) => { + println!("RESULT : RPC ERROR after {:.2?} — {}", elapsed, e); + std::process::exit(1); + } + Err(_) => { + println!( + "RESULT : TIMED OUT after {:.2?} (>30s, no failover)", + elapsed + ); + std::process::exit(1); + } + } +} diff --git a/crates/talos-rs/src/auth.rs b/crates/talos-rs/src/auth.rs index b52c0fa..e8a98a8 100644 --- a/crates/talos-rs/src/auth.rs +++ b/crates/talos-rs/src/auth.rs @@ -6,8 +6,17 @@ use crate::config::Context; use crate::error::TalosError; use rustls::pki_types::{CertificateDer, PrivateKeyDer}; use std::sync::Arc; +use std::time::Duration; use tonic::transport::{Certificate, Channel, ClientTlsConfig, Identity}; +/// Per-endpoint connection timeout. +/// +/// Bounds each dial attempt so an unreachable endpoint is abandoned quickly +/// instead of hanging on the OS-default TCP connect timeout (~21s+). All +/// endpoints are dialed concurrently, so this also bounds how long startup +/// waits when every configured endpoint is unreachable. +const CONNECT_TIMEOUT: Duration = Duration::from_secs(10); + /// Convert Ed25519 private key PEM to standard PKCS8 format /// Talos uses "ED25519 PRIVATE KEY" header but tonic/rustls expects "PRIVATE KEY" fn convert_ed25519_key_to_pkcs8(pem: &[u8]) -> Vec { @@ -28,13 +37,20 @@ fn convert_ed25519_key_to_pkcs8(pem: &[u8]) -> Vec { } } -/// Create a TLS-enabled gRPC channel from a talosconfig context +/// Create a TLS-enabled gRPC channel from a talosconfig context. +/// +/// When the context lists multiple endpoints, all of them are dialed +/// concurrently and the first one that connects wins (talosctl-style failover). +/// Each dial is bounded by [`CONNECT_TIMEOUT`], so an unreachable endpoint is +/// skipped in favor of a reachable one instead of hanging the client. Returns +/// an error only when every configured endpoint is unreachable. pub async fn create_channel(ctx: &Context) -> Result { - let endpoint_url = ctx - .endpoint_url() - .ok_or_else(|| TalosError::ConfigInvalid("No endpoints configured".to_string()))?; - - tracing::debug!("Connecting to endpoint: {}", endpoint_url); + let endpoint_urls = ctx.endpoint_urls(); + if endpoint_urls.is_empty() { + return Err(TalosError::ConfigInvalid( + "No endpoints configured".to_string(), + )); + } // Decode certificates from base64 let ca_pem = ctx.ca_pem()?; @@ -49,25 +65,59 @@ pub async fn create_channel(ctx: &Context) -> Result { // Tonic expects "PRIVATE KEY" not "ED25519 PRIVATE KEY" let client_key_pem = convert_ed25519_key_to_pkcs8(&client_key_pem); - // Create TLS config + // Create TLS config (shared across every endpoint - Talos uses one + // cluster-wide PKI, so the same CA/identity authenticates all of them). let ca = Certificate::from_pem(&ca_pem); let identity = Identity::from_pem(&client_cert_pem, &client_key_pem); - let tls_config = ClientTlsConfig::new().ca_certificate(ca).identity(identity); - // Build the channel (use connect_lazy to defer TLS handshake) - let endpoint = Channel::from_shared(endpoint_url.clone()) - .map_err(|e| { - TalosError::Connection(format!("Invalid endpoint URL '{}': {}", endpoint_url, e)) - })? - .tls_config(tls_config) - .map_err(|e| TalosError::Tls(format!("TLS config error: {:?}", e)))?; + // Build one Endpoint per configured endpoint, all sharing the mTLS config + // and a bounded connect timeout. + let mut endpoints = Vec::with_capacity(endpoint_urls.len()); + for url in &endpoint_urls { + let endpoint = Channel::from_shared(url.clone()) + .map_err(|e| TalosError::Connection(format!("Invalid endpoint URL '{}': {}", url, e)))? + .tls_config(tls_config.clone()) + .map_err(|e| TalosError::Tls(format!("TLS config error: {:?}", e)))? + .connect_timeout(CONNECT_TIMEOUT) + .tcp_keepalive(Some(Duration::from_secs(60))); + endpoints.push((url.clone(), endpoint)); + } - // Use connect_lazy - connection happens on first request - let channel = endpoint.connect_lazy(); + tracing::debug!( + "Dialing {} endpoint(s), first reachable wins: {:?}", + endpoints.len(), + endpoint_urls + ); + + // Eagerly connect to every endpoint concurrently and use the first that + // succeeds. This is true failover: a lazy `balance_list` reports its + // subchannels ready before they have actually connected, so a request can + // be routed to a dead endpoint and time out; racing real `connect()` calls + // guarantees we land on a reachable endpoint. Losing in-flight dials are + // dropped (cancelled) once the first one wins. + let attempts: Vec<_> = endpoints + .into_iter() + .map(|(url, endpoint)| { + Box::pin(async move { + match endpoint.connect().await { + Ok(channel) => Ok((url, channel)), + Err(e) => { + tracing::warn!("Endpoint {} not reachable: {}", url, e); + Err(e) + } + } + }) + }) + .collect(); - tracing::debug!("Successfully connected to {}", endpoint_url); - Ok(channel) + match futures::future::select_ok(attempts).await { + Ok(((url, channel), _losers)) => { + tracing::debug!("Connected via endpoint {}", url); + Ok(channel) + } + Err(e) => Err(e.into()), + } } /// Parse PEM-encoded certificates into rustls types diff --git a/crates/talos-rs/src/config.rs b/crates/talos-rs/src/config.rs index 0f96439..26742eb 100644 --- a/crates/talos-rs/src/config.rs +++ b/crates/talos-rs/src/config.rs @@ -109,29 +109,19 @@ impl Context { /// Get the first endpoint URL pub fn endpoint_url(&self) -> Option { - self.endpoints.first().map(|e| { - if e.starts_with("https://") || e.starts_with("http://") { - e.clone() - } else if e.starts_with('[') { - // IPv6 address with brackets - check if port is specified - if e.contains("]:") { - // Has port specified: [::1]:50000 - format!("https://{}", e) - } else { - // No port: [::1] -> add default port - format!("https://{}:50000", e) - } - } else if e.contains("::") || e.matches(':').count() > 1 { - // Raw IPv6 address without brackets - add brackets and default port - format!("https://[{}]:50000", e) - } else if e.contains(':') { - // IPv4 or hostname with port specified - format!("https://{}", e) - } else { - // IPv4 or hostname without port - add default Talos API port - format!("https://{}:50000", e) - } - }) + self.endpoints.first().map(|e| normalize_endpoint(e)) + } + + /// Get all endpoint URLs (normalized). + /// + /// Used to build a load-balanced channel that fails over across every + /// configured endpoint, mirroring `talosctl`, which load balances and fails + /// over between the endpoints in a context. + pub fn endpoint_urls(&self) -> Vec { + self.endpoints + .iter() + .map(|e| normalize_endpoint(e)) + .collect() } /// Get target nodes, falling back to endpoints if not specified @@ -144,6 +134,34 @@ impl Context { } } +/// Normalize an endpoint string into a full `https://host:port` URL. +/// +/// Handles bare IPv4/hostname (adds the default Talos API port 50000), IPv6 +/// with or without brackets, an existing `http(s)://` scheme, and explicit ports. +fn normalize_endpoint(e: &str) -> String { + if e.starts_with("https://") || e.starts_with("http://") { + e.to_string() + } else if e.starts_with('[') { + // IPv6 address with brackets - check if port is specified + if e.contains("]:") { + // Has port specified: [::1]:50000 + format!("https://{}", e) + } else { + // No port: [::1] -> add default port + format!("https://{}:50000", e) + } + } else if e.contains("::") || e.matches(':').count() > 1 { + // Raw IPv6 address without brackets - add brackets and default port + format!("https://[{}]:50000", e) + } else if e.contains(':') { + // IPv4 or hostname with port specified + format!("https://{}", e) + } else { + // IPv4 or hostname without port - add default Talos API port + format!("https://{}:50000", e) + } +} + #[cfg(test)] mod tests { use super::*; @@ -230,6 +248,44 @@ contexts: ); } + #[test] + fn test_endpoint_urls_multiple() { + // Multiple endpoints in mixed formats should all be normalized and returned. + let ctx = Context { + endpoints: vec![ + "100.64.0.9".to_string(), + "192.168.178.216:50000".to_string(), + "https://talos.example.com:6443".to_string(), + ], + nodes: vec![], + ca: "YQ==".to_string(), + crt: "Yg==".to_string(), + key: "Yw==".to_string(), + }; + assert_eq!( + ctx.endpoint_urls(), + vec![ + "https://100.64.0.9:50000".to_string(), + "https://192.168.178.216:50000".to_string(), + "https://talos.example.com:6443".to_string(), + ] + ); + // endpoint_url() still returns just the first, matching endpoint_urls[0]. + assert_eq!(ctx.endpoint_url(), Some(ctx.endpoint_urls()[0].clone())); + } + + #[test] + fn test_endpoint_urls_empty() { + let ctx = Context { + endpoints: vec![], + nodes: vec![], + ca: "YQ==".to_string(), + crt: "Yg==".to_string(), + key: "Yw==".to_string(), + }; + assert!(ctx.endpoint_urls().is_empty()); + } + #[test] fn test_endpoint_url_with_ipv6() { // IPv6 with brackets and port From 1ce1e01754f14a3ca3c1ee41d00bd94e64458c5a Mon Sep 17 00:00:00 2001 From: Kenny Udovic Date: Thu, 23 Jul 2026 15:09:25 -0400 Subject: [PATCH 2/5] chore: version fetch non-fatal --- CLAUDE.md | 376 ++++++++++++++++++ .../src/components/lifecycle.rs | 94 ++++- 2 files changed, 462 insertions(+), 8 deletions(-) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..7cb7c88 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,376 @@ +# CLAUDE.md - Talos Pilot Development Guide + +## Project Overview + +**Talos Pilot** is a terminal UI (TUI) for managing and monitoring Talos Linux Kubernetes clusters. It provides real-time diagnostics, log streaming, network analysis, and cluster health monitoring. + +### Crate Structure + +``` +crates/ +├── talos-rs/ # Low-level Talos gRPC client library +├── talos-pilot-core/ # Shared business logic (~1,760 lines, 47 tests) +└── talos-pilot-tui/ # Terminal UI application (ratatui-based) +``` + +### Key Technologies +- **Rust 2024 edition** +- **Async runtime:** tokio +- **TUI framework:** ratatui + crossterm + tachyonfx +- **gRPC client:** tonic + prost +- **Kubernetes client:** kube-rs +- **Error handling:** color-eyre + thiserror + +--- + +## Feature Status + +### Implemented Features + +| Feature | Component | Status | +|---------|-----------|--------| +| Multi-cluster overview | `cluster.rs` | Complete | +| Node details (CPU, memory, load) | `cluster.rs` | Complete | +| Service logs with search | `logs.rs` | Complete | +| Multi-service logs (Stern-style) | `multi_logs.rs` | Complete | +| Process tree view | `processes.rs` | Complete | +| Network stats, KubeSpan, packet capture | `network.rs` | Complete | +| etcd status & quorum | `etcd.rs` | Complete | +| K8s workload health | `workloads.rs` | Complete | +| System diagnostics | `diagnostics/` | Complete | +| CNI detection (Flannel/Cilium/Calico) | `diagnostics/cni/` | Complete | +| Addon detection | `diagnostics/addons/` | Complete | +| Security/PKI audit | `security.rs` | Complete | +| Lifecycle (versions, config drift) | `lifecycle.rs` | Complete | +| Node operations (drain/reboot) | `node_operations.rs` | Complete | +| Rolling operations | `rolling_operations.rs` | Complete | +| Audit logging | `audit.rs` | Complete | + +### Planned Features + +| Feature | Priority | Notes | +|---------|----------|-------| +| Container namespace support | Medium | Show pod/container names for connections | +| Upgrade availability alerts | Low | Check for new Talos/K8s versions | + +--- + +## Core Modules (talos-pilot-core) + +| Module | Lines | Tests | Purpose | +|--------|-------|-------|---------| +| `indicators` | ~300 | 6 | HealthIndicator, HasHealth trait, QuorumState, SafetyStatus | +| `formatting` | ~280 | 10 | format_bytes, format_duration, format_percent, pluralize | +| `selection` | ~320 | 6 | SelectableList, MultiSelectList for UI navigation | +| `async_state` | ~200 | 7 | AsyncState for loading/error/refresh management | +| `errors` | ~180 | 4 | format_talos_error, ErrorCategory, user-friendly messages | +| `network` | ~180 | 6 | port_to_service, connection classification | +| `diagnostics` | ~200 | 5 | CheckStatus, CniType, CniInfo, PodHealthInfo | +| `constants` | ~100 | 3 | Thresholds, CRD lists, refresh intervals | + +--- + +## Talos Linux Reference + +### Network Ports + +| Port | Protocol | Service | Used By | +|------|----------|---------|---------| +| 50000 | TCP | apid (Talos API) | talosctl, control plane nodes | +| 50001 | TCP | trustd | Worker nodes for TLS certs | +| 6443 | TCP | kube-apiserver | kubectl, kubelets | +| 2379 | TCP | etcd client | kube-apiserver | +| 2380 | TCP | etcd peer | etcd cluster members | +| 10250 | TCP | kubelet | kube-apiserver | +| 10259 | TCP | kube-scheduler | Health checks | +| 10257 | TCP | kube-controller-manager | Health checks | + +### Talos Machine API Methods + +Key gRPC methods we use (all in `MachineService`): + +| Method | Description | +|--------|-------------| +| `Version` | Get Talos version | +| `ServiceList` | List all services | +| `ServiceRestart` | Restart a service | +| `Logs` | Stream service logs | +| `Memory` | Memory usage | +| `LoadAvg` | CPU load averages | +| `CPUInfo` | CPU info | +| `Processes` | Process list | +| `NetworkDeviceStats` | Network interface stats | +| `Netstat` | Network connections | +| `Read` | Read file from node | +| `Dmesg` | Kernel ring buffer | +| `EtcdStatus` | etcd member status | +| `EtcdMemberList` | etcd cluster members | +| `EtcdAlarmList` | etcd alarms | +| `Kubeconfig` | Get kubeconfig | +| `ApplyConfiguration` | Apply config patch | +| `PacketCapture` | Capture packets (pcap) | +| `Reboot` | Reboot node | +| `Shutdown` | Shutdown node | + +### COSI Resource API - NOT EXTERNALLY ACCESSIBLE + +> **IMPORTANT:** The COSI State API is an **internal service** and is **NOT exposed** through port 50000. + +**Do NOT attempt to implement COSI gRPC client code** - it will fail with `PermissionDenied`. Use `talosctl get` as a subprocess instead. + +--- + +## Core Philosophies + +### 1. State Over Logs + +**The most important principle for diagnostics:** + +> Check actual system state, not log messages. Logs are history; APIs and files are truth. + +```rust +// GOOD: Direct state check +let healthy = client.read_file("/run/flannel/subnet.env").await.is_ok(); + +// GOOD: K8s API query for current state +let pods = kube_client.list::(¶ms).await?; + +// BAD: Log parsing for health determination +let logs = client.logs("kubelet", 100).await?; +let healthy = !logs.contains("error"); // DON'T DO THIS +``` + +### 2. Reliability Hierarchy + +When implementing any check, prefer data sources in this order: + +| Tier | Source Type | Example | Reliability | +|------|-------------|---------|-------------| +| 1 | File/procfs state | `/run/flannel/subnet.env` | Highest | +| 2 | API responses | Talos API, K8s API | High | +| 3 | Log parsing | kubelet logs | Last resort | + +### 3. Graceful Degradation + +When data sources are unavailable, degrade gracefully: + +```rust +// Good: Show unknown state, don't crash +match client.read_file("/some/path").await { + Ok(content) => DiagnosticCheck::pass(...), + Err(_) => DiagnosticCheck::unknown("check_id", "Check Name"), +} +``` + +### 4. No False Positives + +A diagnostic showing failure for a healthy system is worse than showing unknown. + +--- + +## Code Patterns + +### Using AsyncState + +All data-holding components use `AsyncState` for consistent loading/error handling: + +```rust +pub struct MyComponent { + state: AsyncState, + // ... UI state (selection, scroll, etc.) +} + +#[derive(Debug, Clone, Default)] +pub struct MyData { + // All async-loaded data goes here +} + +impl MyComponent { + pub async fn refresh(&mut self, client: &TalosClient) -> Result<()> { + self.state.start_loading(); + + match load_data(client).await { + Ok(data) => self.state.set_data(data), + Err(e) => self.state.set_error_with_retry(format_talos_error(&e)), + } + Ok(()) + } + + fn draw(&self, frame: &mut Frame, area: Rect) { + if self.state.is_loading() && !self.state.has_data() { + // Show loading spinner + } else if let Some(error) = self.state.error() { + // Show error message + } else if let Some(data) = self.state.data() { + // Render data + } + } +} +``` + +### Using HasHealth Trait + +Implement `HasHealth` for health-related enums, then use extension traits for UI colors: + +```rust +// In core +impl HasHealth for MyStatus { + fn health(&self) -> HealthIndicator { + match self { + MyStatus::Good => HealthIndicator::Healthy, + MyStatus::Bad => HealthIndicator::Error, + } + } +} + +// In TUI - use HealthIndicatorExt for colors +let (symbol, color) = my_status.health().symbol_and_color(); +``` + +### Diagnostic Checks + +```rust +// Creating checks +DiagnosticCheck::pass("memory", "Memory", "2.1 GB / 4.0 GB (52%)") +DiagnosticCheck::fail("cni", "CNI", "Not initialized", Some(fix)) +DiagnosticCheck::warn("cpu_load", "CPU Load", "High load: 4.5") +DiagnosticCheck::unknown("etcd", "Etcd") // When data unavailable +``` + +--- + +## Adding New Features + +### Adding a New Component + +1. Create `components/myfeature.rs` +2. Define `MyFeatureData` struct for async-loaded data +3. Use `AsyncState` for state management +4. Implement `Component` trait with `init()`, `update()`, `draw()` +5. Add keyboard handling in `update()` → `Action` +6. Register in `components/mod.rs` +7. Add view switching in `app.rs` + +### Adding a New Diagnostic Check + +1. Identify the **source of truth** (file, API, not logs) +2. Add check function to appropriate module (`core.rs` or CNI/addon provider) +3. Use `DiagnosticCheck::pass/fail/warn/unknown` constructors +4. Provide actionable `DiagnosticFix` when possible +5. Handle unavailable data gracefully (return `unknown`, don't crash) + +### Adding CNI Support + +1. Create `diagnostics/cni/.rs` with provider-specific checks +2. Add detection logic to `cni/mod.rs` (K8s API first, file fallback) +3. Document CNI-specific requirements (kernel modules, etc.) +4. Add pod health checks for CNI pods + +### Adding Addon Support + +1. Add CRD names to `constants.rs` (e.g., `MY_ADDON_CRDS`) +2. Add detection in `addons/mod.rs` +3. Create `addons/.rs` with specific checks if needed + +--- + +## Project Structure + +### Key Files + +| File | Purpose | +|------|---------| +| `talos-rs/src/client.rs` | Talos gRPC client wrapper | +| `core/src/async_state.rs` | Loading/error state management | +| `core/src/indicators.rs` | Health indicator types | +| `core/src/diagnostics.rs` | Diagnostic types (CheckStatus, CniType) | +| `core/src/constants.rs` | Shared constants | +| `tui/src/ui_ext.rs` | Extension traits for ratatui colors | +| `tui/src/components/` | All UI components | + +### Diagnostics Module Structure + +``` +components/diagnostics/ +├── mod.rs # DiagnosticsComponent (UI orchestrator) +├── types.rs # DiagnosticCheck, DiagnosticFix, DiagnosticContext +├── core.rs # Core checks (memory, cpu, services, etcd) +├── k8s.rs # K8s client helper, CNI detection via K8s API +├── cni/ +│ ├── mod.rs # CNI detection + provider dispatch +│ └── flannel.rs # Flannel-specific checks +└── addons/ + ├── mod.rs # Addon detection (CRDs + pods) + └── cert_manager.rs +``` + +--- + +## Quick Reference + +### Do + +- Check actual system state (files, APIs) +- Use `AsyncState` for component data +- Implement `HasHealth` for health enums +- Provide actionable fix suggestions +- Degrade gracefully when sources unavailable +- Add tests for new core functionality +- Use constants from `talos_pilot_core::constants` + +### Don't + +- Parse logs to determine health status +- Use string matching without timestamp validation +- Crash when data sources are unavailable +- Show "failed" when "unknown" is more accurate +- Use VIP for Talos API endpoint (won't work if etcd is down) +- Duplicate health indicator logic (use HasHealth trait) +- Store UI state mixed with async data + +### Reliability Checklist for New Checks + +- [ ] What is the source of truth for this state? +- [ ] Can we check that source directly (file, API)? +- [ ] If using logs, do we validate timestamps? +- [ ] What happens if the data source is unavailable? +- [ ] Can this check produce false positives? +- [ ] Is the failure mode graceful? + +--- + +## Testing + +```bash +# Run all tests +cargo test --all + +# Run specific crate tests +cargo test --package talos-pilot-core + +# Current test counts +# - Core: 47 unit tests + 10 doc tests +# - TUI: 8 tests +# - talos-rs: 32 tests + 1 doc test (includes gRPC metadata format tests) +``` + +### Before Merging + +1. **No false positives:** Create error condition, fix it, verify check shows healthy +2. **Graceful degradation:** Disconnect API, verify no crashes +3. **Stale log immunity:** Ensure old log messages don't affect current state +4. **Zero warnings:** `cargo clippy --all --all-targets -- -D warnings` should pass clean +5. **Formatting:** `cargo fmt --all -- --check` should pass + +--- + +## Documentation + +| Doc | Purpose | +|-----|---------| +| `README.md` | User-facing documentation | +| `internal-docs/talos-pilot-design-doc.md` | Original design document | +| `internal-docs/phase-2-features.md` | Feature implementation tracking | +| `internal-docs/refactoring_report.md` | Code refactoring progress | +| `internal-docs/*-plan.md` | Feature planning documents | +| `internal-docs/*-progress.md` | Feature implementation progress | diff --git a/crates/talos-pilot-tui/src/components/lifecycle.rs b/crates/talos-pilot-tui/src/components/lifecycle.rs index 603e04a..b90b505 100644 --- a/crates/talos-pilot-tui/src/components/lifecycle.rs +++ b/crates/talos-pilot-tui/src/components/lifecycle.rs @@ -228,8 +228,14 @@ impl LifecycleComponent { // Get or create data let mut data = self.state.take_data().unwrap_or_default(); - // Fetch version information - match client.version().await { + // Fetch version information. + // + // Non-fatal: a version RPC failure must NOT abort the refresh. The K8s, + // etcd, and pod/PDB pre-operation checks below use independent data + // sources and can still succeed. Previously this early-returned after + // set_data() (which clears the just-set error), leaving every section + // stuck on "unknown"/"unavailable" with no error and no further fetches. + let version_error = match client.version().await { Ok(versions) => { // Get context name from talosconfig if not already set if !versions.is_empty() && data.context_name.is_empty() { @@ -246,14 +252,17 @@ impl LifecycleComponent { } data.versions = versions; + None } Err(e) => { - self.state - .set_error(format!("Failed to fetch versions: {}", e)); - self.state.set_data(data); - return Ok(()); + tracing::warn!( + "Failed to fetch versions: {} (continuing with etcd/K8s pre-op checks)", + e + ); + // Preserve any cached versions for display; do not abort. + Some(format!("Failed to fetch versions: {}", e)) } - } + }; // Fetch time sync status match client.time().await { @@ -352,11 +361,33 @@ impl LifecycleComponent { // Generate alerts Self::generate_alerts_into(&mut data); - // Store the data + // Store the data (this clears any prior error). self.state.set_data(data); + + // If the version RPC failed AND nothing else could be gathered, surface + // the error instead of a silent all-"unknown" screen. If any pre-op + // check succeeded we keep the partial view (graceful degradation). + if let Some(err) = version_error { + let nothing_loaded = self.data().map(Self::gathered_nothing).unwrap_or(true); + if nothing_loaded { + self.state.set_error(err); + } + } Ok(()) } + /// True when a refresh produced no usable data at all (no versions, no node + /// rows, and no pre-operation check succeeded). Used to decide whether a + /// version-fetch failure should surface as an error banner rather than a + /// silent all-"unknown" screen. Kept pure so it can be unit-tested. + fn gathered_nothing(data: &LifecycleData) -> bool { + data.versions.is_empty() + && data.node_statuses.is_empty() + && data.pre_op_checks.pod_health.is_none() + && data.pre_op_checks.pdb_health.is_none() + && data.pre_op_checks.etcd_quorum.is_none() + } + /// Fetch pre-operation health checks into data async fn fetch_pre_op_checks_into(&mut self, data: &mut LifecycleData, client: &TalosClient) { // Get a control plane node IP for kubeconfig fetch @@ -1032,3 +1063,50 @@ impl Component for LifecycleComponent { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + /// A totally empty refresh result (e.g. version RPC failed and no pre-op + /// check succeeded) must be reported as "nothing gathered" so the caller + /// surfaces an error instead of a silent all-"unknown" screen. This is the + /// regression guard for issue #2 (lifecycle view stuck on "unknown"). + #[test] + fn gathered_nothing_is_true_for_empty_data() { + let data = LifecycleData::default(); + assert!(LifecycleComponent::gathered_nothing(&data)); + } + + /// If any single pre-op check succeeded, the refresh produced usable data, + /// so we keep the partial view rather than replacing it with an error + /// (graceful degradation — the whole point of not aborting on version()). + #[test] + fn gathered_nothing_is_false_when_etcd_quorum_present() { + let mut data = LifecycleData::default(); + data.pre_op_checks.etcd_quorum = Some(EtcdQuorumInfo { + total_members: 3, + healthy_members: 3, + can_lose: 1, + is_healthy: true, + }); + assert!(!LifecycleComponent::gathered_nothing(&data)); + } + + /// Versions present (the happy path) is likewise not "nothing". + #[test] + fn gathered_nothing_is_false_when_versions_present() { + let mut data = LifecycleData::default(); + data.versions.push(VersionInfo { + node: "10.5.0.2".to_string(), + version: "v1.12.1".to_string(), + sha: String::new(), + built: String::new(), + go_version: String::new(), + os: String::new(), + arch: String::new(), + platform: "container".to_string(), + }); + assert!(!LifecycleComponent::gathered_nothing(&data)); + } +} From fd5080fc9c6daa0f76a9734f3cbe9652768da99e Mon Sep 17 00:00:00 2001 From: Kenny Udovic Date: Thu, 23 Jul 2026 15:18:26 -0400 Subject: [PATCH 3/5] fix: missing workers bug squashed via talosctl properly recieving config in internal function alls --- .../talos-pilot-tui/src/components/cluster.rs | 206 ++++++++++++++++-- crates/talos-rs/src/talosctl.rs | 83 +++++-- 2 files changed, 250 insertions(+), 39 deletions(-) diff --git a/crates/talos-pilot-tui/src/components/cluster.rs b/crates/talos-pilot-tui/src/components/cluster.rs index 3e75dfc..455e45b 100644 --- a/crates/talos-pilot-tui/src/components/cluster.rs +++ b/crates/talos-pilot-tui/src/components/cluster.rs @@ -145,6 +145,9 @@ struct ClusterData { controlplane_expanded: bool, /// Whether workers group is expanded workers_expanded: bool, + /// Non-fatal warning when worker discovery is unavailable, so the node list + /// (control-plane-only in that case) isn't silently misleading. + discovery_warning: Option, } /// Cluster component showing overview with node list @@ -209,7 +212,30 @@ impl ClusterComponent { } } - /// Get control plane nodes for a cluster (nodes with etcd service) + /// Whether a node is a control plane node. + /// + /// Discovery `machineType` is authoritative when available; otherwise we + /// fall back to the etcd-service heuristic (only control plane nodes run + /// etcd). Keeping these two in one place ensures the control-plane and + /// worker groups stay complementary (every node lands in exactly one). + fn node_is_controlplane(&self, cluster_idx: usize, node: &str) -> bool { + if let Some(cluster) = self.clusters.get(cluster_idx) { + let key = node.split(':').next().unwrap_or(node); + if let Some(member) = cluster.discovery_members.iter().find(|m| { + m.hostname.eq_ignore_ascii_case(node) + || m.hostname.eq_ignore_ascii_case(key) + || m.addresses.iter().any(|a| a == node || a == key) + }) { + return member.machine_type.eq_ignore_ascii_case("controlplane"); + } + } + // Fallback: presence of the etcd service marks a control plane node. + self.get_node_services_for(cluster_idx, node) + .map(|s| s.iter().any(|svc| svc.id == "etcd")) + .unwrap_or(false) + } + + /// Get control plane nodes for a cluster (discovery machineType, else etcd service) fn controlplane_nodes_for(&self, cluster_idx: usize) -> Vec<(usize, &VersionInfo)> { let Some(cluster) = self.clusters.get(cluster_idx) else { return Vec::new(); @@ -218,15 +244,11 @@ impl ClusterComponent { .versions .iter() .enumerate() - .filter(|(_, v)| { - self.get_node_services_for(cluster_idx, &v.node) - .map(|s| s.iter().any(|svc| svc.id == "etcd")) - .unwrap_or(false) - }) + .filter(|(_, v)| self.node_is_controlplane(cluster_idx, &v.node)) .collect() } - /// Get worker nodes for a cluster (nodes without etcd service) + /// Get worker nodes for a cluster (every node that isn't a control plane node) fn worker_nodes_for(&self, cluster_idx: usize) -> Vec<(usize, &VersionInfo)> { let Some(cluster) = self.clusters.get(cluster_idx) else { return Vec::new(); @@ -235,11 +257,7 @@ impl ClusterComponent { .versions .iter() .enumerate() - .filter(|(_, v)| { - self.get_node_services_for(cluster_idx, &v.node) - .map(|s| !s.iter().any(|svc| svc.id == "etcd")) - .unwrap_or(true) - }) + .filter(|(_, v)| !self.node_is_controlplane(cluster_idx, &v.node)) .collect() } @@ -497,7 +515,7 @@ impl ClusterComponent { ) .await { - Ok(members) => { + Ok(members) if !members.is_empty() => { cluster.node_ips.clear(); for member in &members { if let Some(ip) = member.addresses.first() { @@ -505,6 +523,24 @@ impl ClusterComponent { } } cluster.discovery_members = members; + cluster.discovery_warning = None; + } + Ok(_) => { + // Discovery succeeded but returned no members. This usually means + // the cluster discovery service is disabled, so only nodes known + // via etcd (control plane) can be enumerated. + tracing::warn!( + "Discovery returned no members for {} (discovery service likely disabled); \ + worker nodes cannot be enumerated", + cluster.name + ); + if cluster.discovery_members.is_empty() { + cluster.discovery_warning = Some( + "Worker nodes unavailable: cluster discovery returned no members \ + (the discovery service may be disabled)." + .to_string(), + ); + } } Err(e) => { tracing::warn!( @@ -512,7 +548,15 @@ impl ClusterComponent { cluster.name, e ); - // DO NOT clear - preserve existing data for resilience + // DO NOT clear discovery_members - preserve existing data for resilience. + // Only warn if we have no discovery data at all (would fall back to + // control-plane-only), so the user knows workers may be missing. + if cluster.discovery_members.is_empty() { + cluster.discovery_warning = Some(format!( + "Worker nodes may be missing: could not reach the discovery service \ + ({e}). Check that talosctl is installed and on PATH." + )); + } } } @@ -878,8 +922,17 @@ impl ClusterComponent { } } - /// Determine node role based on services (etcd = controlplane) + /// Determine the selected node's role. + /// + /// The selection already reflects how the node was grouped (which now uses + /// discovery machineType with an etcd-service fallback), so read that + /// directly; only fall back to the service heuristic for non-node selections. fn current_node_role(&self) -> String { + match &self.selected_item { + NodeListItem::ControlPlaneNode(..) => return "controlplane".to_string(), + NodeListItem::WorkerNode(..) => return "worker".to_string(), + _ => {} + } let service_ids = self.current_service_ids(); if service_ids.iter().any(|s| s == "etcd") { "controlplane".to_string() @@ -1746,6 +1799,17 @@ impl ClusterComponent { } } } + + // Discovery warning: worker enumeration failed, so the list + // above may be control-plane-only. Surface it instead of + // silently hiding workers. + if let Some(warning) = &cluster.discovery_warning { + lines.push(Line::from(vec![ + Span::raw(" "), + Span::styled("⚠ ", Style::default().fg(Color::Yellow)), + Span::styled(warning.clone(), Style::default().fg(Color::Yellow)), + ])); + } } frame.render_widget(Paragraph::new(lines), pane_layout[0]); @@ -2190,3 +2254,115 @@ impl ClusterComponent { } } } + +#[cfg(test)] +mod tests { + use super::*; + use talos_rs::{DiscoveryMember, NodeServices, ServiceInfo, VersionInfo}; + + fn ver(node: &str) -> VersionInfo { + VersionInfo { + node: node.to_string(), + version: String::new(), + sha: String::new(), + built: String::new(), + go_version: String::new(), + os: String::new(), + arch: String::new(), + platform: String::new(), + } + } + + fn member(hostname: &str, ip: &str, machine_type: &str) -> DiscoveryMember { + DiscoveryMember { + id: hostname.to_string(), + addresses: vec![ip.to_string()], + hostname: hostname.to_string(), + machine_type: machine_type.to_string(), + operating_system: String::new(), + } + } + + fn services_with(node: &str, ids: &[&str]) -> NodeServices { + NodeServices { + node: node.to_string(), + services: ids + .iter() + .map(|id| ServiceInfo { + id: id.to_string(), + state: "Running".to_string(), + health: None, + }) + .collect(), + } + } + + fn names(nodes: &[(usize, &VersionInfo)]) -> Vec { + nodes.iter().map(|(_, v)| v.node.clone()).collect() + } + + /// Discovery machineType is authoritative: a discovered worker is classified + /// as a worker even when no per-node service data is available. This is the + /// regression guard for "workers missing / shown only as control plane" + /// (the "worker nodes missing" reports) — workers must land in the worker group. + #[test] + fn classifies_by_discovery_machine_type() { + let mut comp = ClusterComponent::default(); + comp.clusters.push(ClusterData { + name: "test".to_string(), + versions: vec![ver("cp1"), ver("worker1")], + discovery_members: vec![ + member("cp1", "10.0.0.1", "controlplane"), + member("worker1", "10.0.0.2", "worker"), + ], + ..Default::default() + }); + + assert_eq!(names(&comp.controlplane_nodes_for(0)), vec!["cp1"]); + assert_eq!(names(&comp.worker_nodes_for(0)), vec!["worker1"]); + } + + /// When discovery is unavailable (empty members), fall back to the + /// etcd-service heuristic so control plane nodes are still identified. + #[test] + fn falls_back_to_etcd_service_without_discovery() { + let mut comp = ClusterComponent::default(); + comp.clusters.push(ClusterData { + name: "test".to_string(), + versions: vec![ver("n1"), ver("n2")], + services: vec![ + services_with("n1", &["etcd", "kubelet"]), + services_with("n2", &["kubelet"]), + ], + ..Default::default() + }); + + assert_eq!(names(&comp.controlplane_nodes_for(0)), vec!["n1"]); + assert_eq!(names(&comp.worker_nodes_for(0)), vec!["n2"]); + } + + /// Control-plane and worker groups must partition the node set: every node + /// lands in exactly one group, never both or neither. + #[test] + fn groups_are_complementary() { + let mut comp = ClusterComponent::default(); + comp.clusters.push(ClusterData { + name: "test".to_string(), + versions: vec![ver("a"), ver("b"), ver("c")], + discovery_members: vec![ + member("a", "10.0.0.1", "controlplane"), + member("b", "10.0.0.2", "worker"), + // "c" is intentionally absent from discovery and has no services + ], + ..Default::default() + }); + + let cp = names(&comp.controlplane_nodes_for(0)); + let workers = names(&comp.worker_nodes_for(0)); + assert_eq!(cp.len() + workers.len(), 3, "every node classified once"); + // Unknown node "c" defaults to worker (safe default, still visible). + assert!(workers.contains(&"c".to_string())); + assert!(cp.contains(&"a".to_string())); + assert!(workers.contains(&"b".to_string())); + } +} diff --git a/crates/talos-rs/src/talosctl.rs b/crates/talos-rs/src/talosctl.rs index a41dc92..378f5f1 100644 --- a/crates/talos-rs/src/talosctl.rs +++ b/crates/talos-rs/src/talosctl.rs @@ -533,41 +533,49 @@ pub async fn get_discovery_members_for_context( return Err(TalosError::NoEndpoints(context.to_string())); } - let output = exec_talosctl_async(&[ - "--context", - context, - "-n", - &node_ip, - "get", - "members", - "-o", - "yaml", - ]) - .await?; - parse_discovery_members_yaml(&output) + get_discovery_members_for_node_async(context, &node_ip, config_path).await } /// Get discovery members for a specific node IP using context certificates (async). /// /// This allows querying a specific control plane node directly instead of going through the VIP. +/// +/// Executes: talosctl --context [--talosconfig ] -n get members -o yaml +/// +/// The `--talosconfig` flag is essential for users who run with `--config `: +/// without it talosctl reads the default `~/.talos/config`, where the context +/// (and its certs) may not exist, so discovery fails and the node list silently +/// degrades to control-plane-only (the "worker nodes missing" reports). async fn get_discovery_members_for_node_async( context: &str, node_ip: &str, + config_path: Option<&str>, ) -> Result, TalosError> { - let output = exec_talosctl_async(&[ - "--context", - context, - "-n", - node_ip, - "get", - "members", - "-o", - "yaml", - ]) - .await?; + let args = members_command_args(context, node_ip, config_path); + let output = exec_talosctl_async(&args).await?; parse_discovery_members_yaml(&output) } +/// Build the `talosctl ... get members` argument list. +/// +/// Extracted as a pure function so the `--talosconfig` handling can be +/// unit-tested: omitting it made talosctl read the default `~/.talos/config`, +/// which breaks `--config ` users (their context isn't there) and +/// silently degraded the node list to control-plane-only (workers missing). +fn members_command_args<'a>( + context: &'a str, + node_ip: &'a str, + config_path: Option<&'a str>, +) -> Vec<&'a str> { + let mut args = vec!["--context", context]; + if let Some(path) = config_path { + args.push("--talosconfig"); + args.push(path); + } + args.extend_from_slice(&["-n", node_ip, "get", "members", "-o", "yaml"]); + args +} + /// Get discovery members with automatic retry and fallback to specific nodes. /// /// First tries the VIP endpoint (via context), then falls back to querying @@ -617,7 +625,7 @@ pub async fn get_discovery_members_with_retry( fastrand::shuffle(&mut shuffled_ips); for node_ip in shuffled_ips { - match get_discovery_members_for_node_async(context, node_ip).await { + match get_discovery_members_for_node_async(context, node_ip, config_path).await { Ok(members) => { tracing::debug!( "Successfully fetched discovery members from fallback node {}", @@ -1362,6 +1370,33 @@ spec: assert!(members.is_empty()); } + #[test] + fn members_args_include_talosconfig_when_config_path_set() { + let args = members_command_args("mycluster", "10.0.0.1", Some("/etc/talos/config")); + assert_eq!( + args, + vec![ + "--context", + "mycluster", + "--talosconfig", + "/etc/talos/config", + "-n", + "10.0.0.1", + "get", + "members", + "-o", + "yaml", + ] + ); + } + + #[test] + fn members_args_omit_talosconfig_when_none() { + let args = members_command_args("mycluster", "10.0.0.1", None); + assert!(!args.contains(&"--talosconfig")); + assert_eq!(&args[..2], &["--context", "mycluster"]); + } + #[test] fn test_parse_discovery_members_invalid_yaml() { // Should skip invalid documents and not panic From 49814c723131b29c3b939ff897fb66929cc29be3 Mon Sep 17 00:00:00 2001 From: Kenny Udovic Date: Thu, 23 Jul 2026 15:25:01 -0400 Subject: [PATCH 4/5] fix: fetch the kubeconfig directly from a control-plane node over the Talos API --- .../src/components/node_operations.rs | 102 +++++++++++++++++- 1 file changed, 98 insertions(+), 4 deletions(-) diff --git a/crates/talos-pilot-tui/src/components/node_operations.rs b/crates/talos-pilot-tui/src/components/node_operations.rs index 973a8b7..6ef623a 100644 --- a/crates/talos-pilot-tui/src/components/node_operations.rs +++ b/crates/talos-pilot-tui/src/components/node_operations.rs @@ -5,7 +5,7 @@ use crate::action::Action; use crate::components::Component; use crate::components::diagnostics::k8s::{ - DrainOptions, PdbHealthInfo, check_pdb_health, create_k8s_client, + DrainOptions, PdbHealthInfo, check_pdb_health, create_k8s_client_with_source, }; use crate::ui_ext::SafetyStatusExt; use color_eyre::Result; @@ -486,6 +486,10 @@ pub struct NodeOperationsComponent { client: Option, /// K8s client for PDB checks k8s_client: Option, + /// Why the K8s client couldn't be created, if it couldn't. Surfaced when an + /// operation that needs it (drain/reboot) is attempted, instead of a + /// generic "No K8s client available". + k8s_error: Option, /// Async state for loaded data state: AsyncState, @@ -519,6 +523,7 @@ impl NodeOperationsComponent { is_controlplane, client: None, k8s_client: None, + k8s_error: None, state: AsyncState::new(), selected_op: 0, operation_state: OperationState::Ready, @@ -593,6 +598,26 @@ impl NodeOperationsComponent { /// Start a background operation pub fn start_operation(&mut self, op_type: OperationType) { + // Both drain and reboot require a Kubernetes client (to cordon/drain the + // node). If we couldn't create one, fail fast with the *specific* reason + // instead of spawning a task that reports a generic "No K8s client + // available". + if self.k8s_client.is_none() { + let reason = self.k8s_error.clone().unwrap_or_else(|| { + "could not reach a control plane node for the kubeconfig, and no \ + usable KUBECONFIG was found" + .to_string() + }); + let message = format!( + "Cannot {}: no Kubernetes client — {}", + op_type.name().to_lowercase(), + reason + ); + tracing::error!("{}", message); + self.operation_state = OperationState::Completed(op_type, false, message); + return; + } + // Reset progress { let mut progress = self.operation_progress.lock().unwrap(); @@ -743,14 +768,31 @@ impl NodeOperationsComponent { /// Fetch PDB information async fn fetch_pdb_info(&mut self, client: &TalosClient) { - // Initialize K8s client if needed + // Initialize K8s client if needed. + // + // Target a control plane node for the kubeconfig fetch (via etcd member + // IPs). This mirrors the diagnostics/lifecycle path and matters because + // the environment kubeconfig can be unusable here — kube-rs is built + // without the exec/oidc auth features, so an EKS/OIDC kubeconfig that + // `kubectl` accepts fails — whereas the Talos-served kubeconfig uses + // client-cert auth that always works. Without a CP target the fetch + // fell back to the VIP and failed, leaving no client (and a later + // drain/reboot reporting the generic "No K8s client available"). if self.k8s_client.is_none() { - match create_k8s_client(client).await { - Ok(k8s) => { + let cp_node_ip = match client.etcd_members().await { + Ok(members) => members.first().and_then(|m| m.ip_address()), + Err(_) => None, + }; + + match create_k8s_client_with_source(client, cp_node_ip.as_deref(), None).await { + Ok((k8s, source)) => { + tracing::info!("K8s client created from: {:?}", source); self.k8s_client = Some(k8s); + self.k8s_error = None; } Err(e) => { tracing::warn!("Failed to create K8s client: {}", e); + self.k8s_error = Some(e.to_string()); return; } } @@ -1345,3 +1387,55 @@ impl Component for NodeOperationsComponent { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + /// When no K8s client could be created, starting an operation must fail with + /// the *specific* reason (captured during refresh), not the old generic + /// "No K8s client available". Regression guard for the reboot/drain report. + #[test] + fn start_operation_without_k8s_surfaces_specific_reason() { + let mut comp = + NodeOperationsComponent::new("node1".to_string(), "10.0.0.1:50000".to_string(), false); + comp.k8s_error = Some("kubeconfig fetch failed: exec plugin not supported".to_string()); + + comp.start_operation(OperationType::Reboot); + + match &comp.operation_state { + OperationState::Completed(op, success, msg) => { + assert_eq!(*op, OperationType::Reboot); + assert!(!success); + assert!(msg.contains("Cannot reboot"), "msg: {msg}"); + assert!(msg.contains("exec plugin not supported"), "msg: {msg}"); + } + other => panic!("expected Completed, got {other:?}"), + } + // Must not have spawned a background task. + assert!(comp.operation_task.is_none()); + } + + /// With no captured error, still fail fast with an actionable default reason + /// rather than spawning a doomed task. + #[test] + fn start_operation_without_k8s_uses_default_reason() { + let mut comp = + NodeOperationsComponent::new("node1".to_string(), "10.0.0.1:50000".to_string(), true); + + comp.start_operation(OperationType::Drain); + + match &comp.operation_state { + OperationState::Completed(op, success, msg) => { + assert_eq!(*op, OperationType::Drain); + assert!(!success); + assert!(msg.contains("Cannot drain"), "msg: {msg}"); + assert!( + msg.contains("control plane") || msg.to_lowercase().contains("kubeconfig"), + "msg: {msg}" + ); + } + other => panic!("expected Completed, got {other:?}"), + } + } +} From 85a6922b6e908a3796eee5bcf16326d4bd037304 Mon Sep 17 00:00:00 2001 From: Kenny Udovic Date: Thu, 23 Jul 2026 15:30:28 -0400 Subject: [PATCH 5/5] chore: bump version to 0.1.10 Release notes in CHANGELOG.md cover fixes for #22, #23, #25, #26, #27. Co-Authored-By: Claude Opus 4.8 (1M context) fix: resolve clippy lints (sort_by_key, redundant format ref) Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 27 +++++++++++++++++++ Cargo.lock | 8 +++--- Cargo.toml | 2 +- .../talos-pilot-tui/src/components/network.rs | 2 +- .../src/components/processes.rs | 4 +-- .../talos-pilot-tui/src/components/storage.rs | 2 +- 6 files changed, 36 insertions(+), 9 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..a586204 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,27 @@ +# Changelog + +## 0.1.10 + +Reliability release focused on connectivity, node discovery, and node operations. + +### Bug Fixes + +**Multi-endpoint failover — app no longer hangs silently ([#27](https://github.com/Handfish/talos-pilot/issues/27))** + +talos-pilot now fails over across endpoints like `talosctl`: it dials all of a context's endpoints concurrently and uses the first that actually connects (bounded by a 10s timeout), so a dead first endpoint is skipped instead of hanging the session. The old "Successfully connected" log was just tonic's lazy channel, not a real handshake — hence the ~21s wait before the first RPC failed. Startup no longer blocks on a blank screen: there's now a cancellable "Connecting…" screen (q/Esc/Ctrl-C), and an all-unreachable cluster shows a clear Connection Error panel. Verified on a multi-endpoint cluster with the dead endpoint first — connects in ~2ms where it used to hang, so the reorder-endpoints workaround is no longer needed. (commit 9cab34f) + +**Lifecycle/Versions view now fetches data ([#26](https://github.com/Handfish/talos-pilot/issues/26))** + +The Lifecycle view fetched the Talos version first and treated it as a hard gate: on failure it cleared its own error and returned early, skipping the etcd/K8s/pod checks entirely (hence zero follow-up requests and the silent "unknown" fields). Now a version-fetch failure just logs a warning and continues; the other checks fire regardless, and you only get an error banner if the whole refresh comes back empty. Regression tests added. (commit 1ce1e01) + +**Worker nodes no longer missing ([#25](https://github.com/Handfish/talos-pilot/issues/25), [#22](https://github.com/Handfish/talos-pilot/issues/22))** + +talos-pilot fetches the full node list (workers included) via `talosctl get members`, but it wasn't passing `--talosconfig`, so if you ran with `--config ` talosctl fell back to the default `~/.talos/config`, couldn't find your context, and the list silently collapsed to just the control-plane nodes. It now passes your config through correctly. Node roles are now determined from the discovery `machineType` (not inferred from the etcd service), and if worker discovery genuinely can't run (discovery service disabled, or `talosctl` not on PATH) you'll now see a visible warning instead of a silently short list. Verified end-to-end + tests added. (commit fd5080f) + +**"No K8s client available" when rebooting a node ([#23](https://github.com/Handfish/talos-pilot/issues/23))** + +When you open node operations, talos-pilot builds a Kubernetes client to run the pre-drain safety checks. It was only trying your environment kubeconfig and then a VIP fallback — and since we build kube-rs without the exec/OIDC auth plugins, a kubeconfig that `kubectl` accepts (e.g. EKS/OIDC) could fail to load, leaving no client, so the reboot bailed out with the generic "No K8s client available". It now fetches the kubeconfig directly from a control-plane node over the Talos API (certificate auth, always available), so it no longer depends on your local kubeconfig's auth method. And if a client genuinely can't be created, you'll now see the specific reason instead of the generic message. Verified that drain/reboot safety checks load even with no usable local kubeconfig. (commit 49814c7) + +## 0.1.9 + +See the git history for changes prior to 0.1.10. diff --git a/Cargo.lock b/Cargo.lock index 83fca31..6f07a85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2778,7 +2778,7 @@ dependencies = [ [[package]] name = "talos-pilot" -version = "0.1.9" +version = "0.1.10" dependencies = [ "clap", "color-eyre", @@ -2791,7 +2791,7 @@ dependencies = [ [[package]] name = "talos-pilot-core" -version = "0.1.9" +version = "0.1.10" dependencies = [ "chrono", "serde", @@ -2802,7 +2802,7 @@ dependencies = [ [[package]] name = "talos-pilot-tui" -version = "0.1.9" +version = "0.1.10" dependencies = [ "arboard", "base64", @@ -2829,7 +2829,7 @@ dependencies = [ [[package]] name = "talos-rs" -version = "0.1.9" +version = "0.1.10" dependencies = [ "base64", "dirs-next", diff --git a/Cargo.toml b/Cargo.toml index 5b8d0a5..2b8ea9d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ members = ["crates/*"] resolver = "2" [workspace.package] -version = "0.1.9" +version = "0.1.10" edition = "2024" authors = ["Ken Udovic"] license = "MIT" diff --git a/crates/talos-pilot-tui/src/components/network.rs b/crates/talos-pilot-tui/src/components/network.rs index 8a24e29..2213481 100644 --- a/crates/talos-pilot-tui/src/components/network.rs +++ b/crates/talos-pilot-tui/src/components/network.rs @@ -681,7 +681,7 @@ impl NetworkStatsComponent { }); } ConnSortBy::Port => { - conns.sort_by(|a, b| a.local_port.cmp(&b.local_port)); + conns.sort_by_key(|a| a.local_port); } } diff --git a/crates/talos-pilot-tui/src/components/processes.rs b/crates/talos-pilot-tui/src/components/processes.rs index f808b64..090bca7 100644 --- a/crates/talos-pilot-tui/src/components/processes.rs +++ b/crates/talos-pilot-tui/src/components/processes.rs @@ -224,7 +224,7 @@ impl ProcessesComponent { } SortBy::Mem => { data.processes - .sort_by(|a, b| b.resident_memory.cmp(&a.resident_memory)); + .sort_by_key(|p| std::cmp::Reverse(p.resident_memory)); } } } @@ -579,7 +579,7 @@ impl ProcessesComponent { } SortBy::Mem => { data.processes - .sort_by(|a, b| b.resident_memory.cmp(&a.resident_memory)); + .sort_by_key(|p| std::cmp::Reverse(p.resident_memory)); } } } diff --git a/crates/talos-pilot-tui/src/components/storage.rs b/crates/talos-pilot-tui/src/components/storage.rs index f544004..76b224c 100644 --- a/crates/talos-pilot-tui/src/components/storage.rs +++ b/crates/talos-pilot-tui/src/components/storage.rs @@ -347,7 +347,7 @@ impl StorageComponent { Span::styled("Size: ", Style::default().fg(Color::Gray)), Span::raw(format!( "{} ({})", - &disk.size_pretty, + disk.size_pretty, format_bytes(disk.size) )), ]),