diff --git a/backend/.sqlx/query-a41c4cbaffdb714e4a963557de5a4011744d684eb24e03cb4beae6a512613159.json b/backend/.sqlx/query-0c18351237816fe0c56e23801fcb8e70dbffcf08ed121e55c871f727c4ddf626.json similarity index 80% rename from backend/.sqlx/query-a41c4cbaffdb714e4a963557de5a4011744d684eb24e03cb4beae6a512613159.json rename to backend/.sqlx/query-0c18351237816fe0c56e23801fcb8e70dbffcf08ed121e55c871f727c4ddf626.json index 0141c379b4..ceb05001e4 100644 --- a/backend/.sqlx/query-a41c4cbaffdb714e4a963557de5a4011744d684eb24e03cb4beae6a512613159.json +++ b/backend/.sqlx/query-0c18351237816fe0c56e23801fcb8e70dbffcf08ed121e55c871f727c4ddf626.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "UPDATE worker_ping SET ping_at = now(), jobs_executed = $1, custom_tags = $2,\n occupancy_rate = $3, memory_usage = $4, wm_memory_usage = $5, vcpus = COALESCE($7, vcpus),\n memory = COALESCE($8, memory), occupancy_rate_15s = $9, occupancy_rate_5m = $10, occupancy_rate_30m = $11, native_mode = $12 WHERE worker = $6", + "query": "UPDATE worker_ping SET ping_at = now(), jobs_executed = $1, custom_tags = $2,\n occupancy_rate = $3, memory_usage = $4, wm_memory_usage = $5, vcpus = COALESCE($7, vcpus),\n memory = COALESCE($8, memory), occupancy_rate_15s = $9, occupancy_rate_5m = $10, occupancy_rate_30m = $11, native_mode = $12, ip = COALESCE($13, ip) WHERE worker = $6", "describe": { "columns": [], "parameters": { @@ -16,10 +16,11 @@ "Float4", "Float4", "Float4", - "Bool" + "Bool", + "Varchar" ] }, "nullable": [] }, - "hash": "a41c4cbaffdb714e4a963557de5a4011744d684eb24e03cb4beae6a512613159" + "hash": "0c18351237816fe0c56e23801fcb8e70dbffcf08ed121e55c871f727c4ddf626" } diff --git a/backend/.sqlx/query-9d9fbcb598c582a29d65be87a1e35baa410f93be3cac4b8dfd33ddbef446d3fe.json b/backend/.sqlx/query-9d9fbcb598c582a29d65be87a1e35baa410f93be3cac4b8dfd33ddbef446d3fe.json deleted file mode 100644 index 2c95e06391..0000000000 --- a/backend/.sqlx/query-9d9fbcb598c582a29d65be87a1e35baa410f93be3cac4b8dfd33ddbef446d3fe.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO worker_ping (worker_instance, worker, ip, custom_tags, worker_group, dedicated_worker, dedicated_workers, wm_version, vcpus, memory, job_isolation, native_mode) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT (worker)\n DO UPDATE set ping_at = now(), worker_instance = EXCLUDED.worker_instance, ip = EXCLUDED.ip, custom_tags = EXCLUDED.custom_tags, worker_group = EXCLUDED.worker_group, dedicated_worker = EXCLUDED.dedicated_worker, dedicated_workers = EXCLUDED.dedicated_workers, wm_version = EXCLUDED.wm_version, vcpus = COALESCE(EXCLUDED.vcpus, worker_ping.vcpus), memory = COALESCE(EXCLUDED.memory, worker_ping.memory), job_isolation = EXCLUDED.job_isolation, native_mode = EXCLUDED.native_mode, current_job_id = NULL, current_job_workspace_id = NULL\n RETURNING jobs_executed", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "jobs_executed", - "type_info": "Int4" - } - ], - "parameters": { - "Left": [ - "Varchar", - "Varchar", - "Varchar", - "TextArray", - "Varchar", - "Varchar", - "TextArray", - "Varchar", - "Int8", - "Int8", - "Text", - "Bool" - ] - }, - "nullable": [ - false - ] - }, - "hash": "9d9fbcb598c582a29d65be87a1e35baa410f93be3cac4b8dfd33ddbef446d3fe" -} diff --git a/backend/.sqlx/query-c748617e060bc41b5922df4b433f20b6971b993e0671b6ccb45db5ef028550bc.json b/backend/.sqlx/query-c748617e060bc41b5922df4b433f20b6971b993e0671b6ccb45db5ef028550bc.json new file mode 100644 index 0000000000..7f6fe7a81c --- /dev/null +++ b/backend/.sqlx/query-c748617e060bc41b5922df4b433f20b6971b993e0671b6ccb45db5ef028550bc.json @@ -0,0 +1,33 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO worker_ping (worker_instance, worker, ip, custom_tags, worker_group, dedicated_worker, dedicated_workers, wm_version, vcpus, memory, job_isolation, native_mode) VALUES ($1, $2, COALESCE($3, 'NO IP'), $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT (worker)\n DO UPDATE set ping_at = now(), worker_instance = EXCLUDED.worker_instance, ip = COALESCE($3, worker_ping.ip), custom_tags = EXCLUDED.custom_tags, worker_group = EXCLUDED.worker_group, dedicated_worker = EXCLUDED.dedicated_worker, dedicated_workers = EXCLUDED.dedicated_workers, wm_version = EXCLUDED.wm_version, vcpus = COALESCE(EXCLUDED.vcpus, worker_ping.vcpus), memory = COALESCE(EXCLUDED.memory, worker_ping.memory), job_isolation = EXCLUDED.job_isolation, native_mode = EXCLUDED.native_mode, current_job_id = NULL, current_job_workspace_id = NULL\n RETURNING jobs_executed", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "jobs_executed", + "type_info": "Int4" + } + ], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Text", + "TextArray", + "Varchar", + "Varchar", + "TextArray", + "Varchar", + "Int8", + "Int8", + "Text", + "Bool" + ] + }, + "nullable": [ + false + ] + }, + "hash": "c748617e060bc41b5922df4b433f20b6971b993e0671b6ccb45db5ef028550bc" +} diff --git a/backend/src/main.rs b/backend/src/main.rs index 43a92ce4bf..b625f440f4 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -2183,12 +2183,7 @@ pub async fn run_workers( // #[cfg(tokio_unstable)] // let monitor = tokio_metrics::TaskMonitor::new(); - let ip = windmill_common::external_ip::get_ip() - .await - .unwrap_or_else(|e| { - tracing::warn!(error = e.to_string(), "failed to get external IP"); - "unretrievable IP".to_string() - }); + windmill_common::external_ip::resolve_ip_in_background(); let mut handles = Vec::with_capacity(num_workers as usize); @@ -2232,7 +2227,6 @@ pub async fn run_workers( let conn1 = wk_conf.conn.clone(); let worker_name = wk_conf.worker_name.clone(); WORKERS_NAMES.write().await.push(worker_name.clone()); - let ip = ip.clone(); let rx = killpill_rxs.pop().unwrap(); let tx = tx.clone(); let base_internal_url = base_internal_url.clone(); @@ -2249,7 +2243,6 @@ pub async fn run_workers( worker_name, i as u64, num_workers as u32, - &ip, rx, tx, &base_internal_url, diff --git a/backend/tests/nativets_stress.rs b/backend/tests/nativets_stress.rs index 32057b41d2..21d3911778 100644 --- a/backend/tests/nativets_stress.rs +++ b/backend/tests/nativets_stress.rs @@ -241,7 +241,6 @@ fn spawn_workers( worker_name, i as u64, n as u32, - "127.0.0.1", rx, tx2, &base_internal_url, diff --git a/backend/tests/worker_ping_ip.rs b/backend/tests/worker_ping_ip.rs new file mode 100644 index 0000000000..8f98398b40 --- /dev/null +++ b/backend/tests/worker_ping_ip.rs @@ -0,0 +1,44 @@ +use sqlx::{Pool, Postgres}; +use windmill_common::{external_ip::UNKNOWN_IP, worker::insert_ping_query}; + +async fn insert_ping(db: &Pool, worker: &str, ip: Option<&str>) -> anyhow::Result<()> { + insert_ping_query( + "test-instance", + worker, + "default", + ip, + &[], + None, + None, + "test", + None, + None, + None, + false, + db, + ) + .await?; + Ok(()) +} + +/// The external IP resolves in the background, so the initial ping often has none yet. That must +/// not blank the address a previous process wrote to the row this one reclaims — worker names are +/// stable across restarts under EXIT_AFTER_N_JOBS. +#[sqlx::test] +async fn unresolved_ip_keeps_the_reclaimed_rows_address(db: Pool) -> anyhow::Result<()> { + insert_ping(&db, "wk-reclaimed", Some("1.2.3.4")).await?; + insert_ping(&db, "wk-reclaimed", None).await?; + let ip: String = sqlx::query_scalar("SELECT ip FROM worker_ping WHERE worker = $1") + .bind("wk-reclaimed") + .fetch_one(&db) + .await?; + assert_eq!(ip, "1.2.3.4"); + + insert_ping(&db, "wk-fresh", None).await?; + let ip: String = sqlx::query_scalar("SELECT ip FROM worker_ping WHERE worker = $1") + .bind("wk-fresh") + .fetch_one(&db) + .await?; + assert_eq!(ip, UNKNOWN_IP); + Ok(()) +} diff --git a/backend/windmill-common/src/external_ip.rs b/backend/windmill-common/src/external_ip.rs index b9713f67fd..5ffd713f5b 100644 --- a/backend/windmill-common/src/external_ip.rs +++ b/backend/windmill-common/src/external_ip.rs @@ -12,8 +12,58 @@ //! connections to be from whitelisted IP addresses. use crate::utils::configure_client; +use std::sync::OnceLock; use std::time::Duration; +/// No address has ever been established for the row. Matches the `worker_ping.ip` column default, +/// and doubles as what an agent sends while its lookup is in flight, since a server that predates +/// the lookup being asynchronous rejects an initial ping carrying nothing. +pub const UNKNOWN_IP: &str = "NO IP"; + +/// The lookup ran and could not produce an address. Distinct from [`UNKNOWN_IP`] because it tells +/// an operator the difference between "never asked" and "asked, and this instance cannot reach the +/// hub", which is the actionable one. Both are filtered out of the addresses the frontend offers +/// for whitelisting. +pub const UNRETRIEVABLE_IP: &str = "unretrievable IP"; + +/// `worker_ping.ip` is `VARCHAR(50)`, and a failed initial ping takes the worker down, so an +/// overlong value must not reach the insert. +const MAX_IP_LEN: usize = 50; + +static EXTERNAL_IP: OnceLock = OnceLock::new(); + +/// The external IP of this process, [`UNRETRIEVABLE_IP`] once the lookup has failed, or `None` +/// while it is still in flight. +pub fn cached_ip() -> Option<&'static str> { + EXTERNAL_IP.get().map(String::as_str) +} + +/// Resolves the external IP into the process-wide cache without blocking the caller. The value is +/// informational, and behind a firewall the lookup burns its whole 5s connect timeout on every +/// process start, so nothing on the worker startup path may wait on it. +pub fn resolve_ip_in_background() { + tokio::spawn(async { + let ip = get_ip() + .await + .map(|ip| { + if ip.len() > MAX_IP_LEN { + tracing::error!("external IP lookup returned an overlong value, ignoring it"); + UNRETRIEVABLE_IP.to_string() + } else { + ip + } + }) + .unwrap_or_else(|e| { + tracing::warn!( + error = e.to_string(), + "failed to get external IP, workers of this process will report it as unretrievable" + ); + UNRETRIEVABLE_IP.to_string() + }); + let _ = EXTERNAL_IP.set(ip); + }); +} + pub async fn get_ip() -> anyhow::Result { tokio::select! { biased; diff --git a/backend/windmill-common/src/worker.rs b/backend/windmill-common/src/worker.rs index a94160be33..3fbe1fc2be 100644 --- a/backend/windmill-common/src/worker.rs +++ b/backend/windmill-common/src/worker.rs @@ -30,6 +30,7 @@ use crate::{ agent_workers::PingJobStatusResponse, cache::{unwrap_or_error, RawNode, RawScript}, error::{self, to_anyhow}, + external_ip::UNKNOWN_IP, global_settings::CUSTOM_TAGS_SETTING, indexer::TantivyIndexerSettings, server::Smtp, @@ -1749,25 +1750,23 @@ pub async fn update_ping_http( insert_ping.occupancy_rate_5m, insert_ping.occupancy_rate_30m, insert_ping.native_mode.unwrap_or(false), + insert_ping.ip.as_deref(), db, ) .await? } PingType::Initial => { - if insert_ping.worker_instance.is_none() - || insert_ping.version.is_none() - || insert_ping.ip.is_none() - { - return Err(anyhow::anyhow!( - "Worker instance, version and ip are required" - )); + if insert_ping.worker_instance.is_none() || insert_ping.version.is_none() { + return Err(anyhow::anyhow!("Worker instance and version are required")); } insert_ping_query( &insert_ping.worker_instance.unwrap(), &worker_name, worker_group, - &insert_ping.ip.unwrap(), + // An agent worker sends the sentinel rather than nothing, to stay acceptable to + // servers that still require an IP here; both mean "not resolved yet". + insert_ping.ip.as_deref().filter(|ip| *ip != UNKNOWN_IP), insert_ping.tags.unwrap_or_default().as_slice(), insert_ping.dw, insert_ping.dws.as_deref(), @@ -1904,12 +1903,13 @@ pub async fn fetch_raw_script_from_app_query( /// `wm_version`, hold the instance-wide `MIN_VERSION` back forever, and one still naming the /// job that process was killed mid-way through skews the zombie/OOM diagnostics that read it. /// `started_at` and `jobs_executed` are the only two columns carried over, being the -/// continuity itself. +/// continuity itself — plus `ip` for as long as `ip` is `None`, which means the external IP +/// lookup has not resolved yet and the predecessor's address is still the best guess. pub async fn insert_ping_query( worker_instance: &str, worker_name: &str, worker_group: &str, - ip: &str, + ip: Option<&str>, tags: &[String], dw: Option, dws: Option<&[String]>, @@ -1920,9 +1920,13 @@ pub async fn insert_ping_query( native_mode: bool, db: &DB, ) -> anyhow::Result { + // A NULL `ip` means the external IP lookup is still in flight; a later ping fills it in, and + // meanwhile the value a previous process wrote to a reclaimed row is the best guess we have. A + // lookup that has failed reports `external_ip::UNRETRIEVABLE_IP`, which does overwrite it. The + // literal below must stay equal to `external_ip::UNKNOWN_IP`. let previous_jobs_executed = sqlx::query_scalar!( - "INSERT INTO worker_ping (worker_instance, worker, ip, custom_tags, worker_group, dedicated_worker, dedicated_workers, wm_version, vcpus, memory, job_isolation, native_mode) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT (worker) - DO UPDATE set ping_at = now(), worker_instance = EXCLUDED.worker_instance, ip = EXCLUDED.ip, custom_tags = EXCLUDED.custom_tags, worker_group = EXCLUDED.worker_group, dedicated_worker = EXCLUDED.dedicated_worker, dedicated_workers = EXCLUDED.dedicated_workers, wm_version = EXCLUDED.wm_version, vcpus = COALESCE(EXCLUDED.vcpus, worker_ping.vcpus), memory = COALESCE(EXCLUDED.memory, worker_ping.memory), job_isolation = EXCLUDED.job_isolation, native_mode = EXCLUDED.native_mode, current_job_id = NULL, current_job_workspace_id = NULL + "INSERT INTO worker_ping (worker_instance, worker, ip, custom_tags, worker_group, dedicated_worker, dedicated_workers, wm_version, vcpus, memory, job_isolation, native_mode) VALUES ($1, $2, COALESCE($3, 'NO IP'), $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT (worker) + DO UPDATE set ping_at = now(), worker_instance = EXCLUDED.worker_instance, ip = COALESCE($3, worker_ping.ip), custom_tags = EXCLUDED.custom_tags, worker_group = EXCLUDED.worker_group, dedicated_worker = EXCLUDED.dedicated_worker, dedicated_workers = EXCLUDED.dedicated_workers, wm_version = EXCLUDED.wm_version, vcpus = COALESCE(EXCLUDED.vcpus, worker_ping.vcpus), memory = COALESCE(EXCLUDED.memory, worker_ping.memory), job_isolation = EXCLUDED.job_isolation, native_mode = EXCLUDED.native_mode, current_job_id = NULL, current_job_workspace_id = NULL RETURNING jobs_executed", worker_instance, worker_name, @@ -2027,12 +2031,13 @@ pub async fn update_worker_ping_main_loop_query( occupancy_rate_5m: Option, occupancy_rate_30m: Option, native_mode: bool, + ip: Option<&str>, db: &DB, ) -> anyhow::Result<()> { timeout(Duration::from_secs(10), sqlx::query!( "UPDATE worker_ping SET ping_at = now(), jobs_executed = $1, custom_tags = $2, occupancy_rate = $3, memory_usage = $4, wm_memory_usage = $5, vcpus = COALESCE($7, vcpus), - memory = COALESCE($8, memory), occupancy_rate_15s = $9, occupancy_rate_5m = $10, occupancy_rate_30m = $11, native_mode = $12 WHERE worker = $6", + memory = COALESCE($8, memory), occupancy_rate_15s = $9, occupancy_rate_5m = $10, occupancy_rate_30m = $11, native_mode = $12, ip = COALESCE($13, ip) WHERE worker = $6", jobs_executed, tags, occupancy_rate, @@ -2045,6 +2050,7 @@ pub async fn update_worker_ping_main_loop_query( occupancy_rate_5m, occupancy_rate_30m, native_mode, + ip, ) .execute(db)) .await??; diff --git a/backend/windmill-test-utils/src/lib.rs b/backend/windmill-test-utils/src/lib.rs index 060e8af983..c09ad2147d 100644 --- a/backend/windmill-test-utils/src/lib.rs +++ b/backend/windmill-test-utils/src/lib.rs @@ -442,7 +442,6 @@ pub fn spawn_test_worker( let (tx, rx) = KillpillSender::new(1); let worker_instance: &str = "test worker instance"; let worker_name: String = next_worker_name(); - let ip: &str = Default::default(); let conn = conn.to_owned(); let tx2 = tx.clone(); @@ -465,7 +464,6 @@ pub fn spawn_test_worker( worker_name, 1, 1, - ip, rx, tx2, &base_internal_url, @@ -494,7 +492,6 @@ pub fn spawn_test_worker_dedicated( let (tx, rx) = KillpillSender::new(1); let worker_instance: &str = "test worker instance"; let worker_name: String = next_worker_name(); - let ip: &str = Default::default(); let conn = conn.to_owned(); let tx2 = tx.clone(); @@ -548,7 +545,6 @@ pub fn spawn_test_worker_dedicated( worker_name, 1, 1, - ip, rx, tx2, &base_internal_url, diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index eb35b38d4c..8590c4c763 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -41,6 +41,7 @@ use windmill_common::{ agent_workers::DECODED_AGENT_TOKEN, apps::AppScriptId, cache::{future::FutureCachedExt, ScriptData, ScriptMetadata}, + external_ip::cached_ip, schema::{should_validate_schema, SchemaValidator}, utils::{create_directory_async, WarnAfterExt}, worker::{ @@ -2487,7 +2488,6 @@ pub async fn run_worker( worker_name: String, i_worker: u64, num_workers: u32, - ip: &str, mut killpill_rx: tokio::sync::broadcast::Receiver<()>, killpill_tx: KillpillSender, base_internal_url: &str, @@ -2583,7 +2583,8 @@ pub async fn run_worker( let mut last_ping = Instant::now() - Duration::from_secs(NUM_SECS_PING + 1); - let previous_jobs_executed = insert_ping(hostname, &worker_name, ip, conn) + let mut reported_ip = cached_ip(); + let previous_jobs_executed = insert_ping(hostname, &worker_name, reported_ip, conn) .await .expect("initial ping could be sent"); @@ -3066,7 +3067,26 @@ pub async fn run_worker( otel_set_worker_uptime(&worker_name, start_time.elapsed().as_secs_f64()); - if last_ping.elapsed().as_secs() > NUM_SECS_PING { + // The external IP resolves in the background, after the initial ping. Pinging on the very + // next iteration rather than the next periodic one is what gets it into the row of a worker + // whose process is short-lived (EXIT_AFTER_N_JOBS). + let ip = cached_ip(); + let ip_just_resolved = reported_ip.is_none() && ip.is_some(); + if ip_just_resolved || last_ping.elapsed().as_secs() > NUM_SECS_PING { + // Servers older than the background lookup take an IP from the initial ping only, so an + // agent has to register a second time to deliver whatever the lookup settled on, an + // address or the unretrievable marker. Registering also clears the row's job columns, + // which costs at most the last job's id here: no job of this worker is in flight at this + // point in the loop, and the next one refills them. + if ip_just_resolved && conn.as_sql().is_none() { + if let Err(e) = insert_ping(hostname, &worker_name, ip, &conn).await { + tracing::warn!( + worker = %worker_name, hostname = %hostname, + "failed to re-register with the resolved external IP: {e}" + ); + } + } + let read_cgroups = *REFRESH_CGROUP_READINGS && last_reading.elapsed().as_secs() > NUM_SECS_READINGS; update_worker_ping_full( @@ -3077,6 +3097,7 @@ pub async fn run_worker( &hostname, &mut occupancy_metrics, &killpill_tx, + ip, ) .await; @@ -3084,6 +3105,7 @@ pub async fn run_worker( last_reading = Instant::now(); } last_ping = Instant::now(); + reported_ip = ip; } if (jobs_executed as u32 + vacuum_shift) % VACUUM_PERIOD == 0 { diff --git a/backend/windmill-worker/src/worker_utils.rs b/backend/windmill-worker/src/worker_utils.rs index 6f47b2d863..67e9831481 100644 --- a/backend/windmill-worker/src/worker_utils.rs +++ b/backend/windmill-worker/src/worker_utils.rs @@ -4,6 +4,7 @@ use uuid::Uuid; use windmill_common::{ agent_workers::{PingJobStatus, PingJobStatusResponse}, cache, + external_ip::UNKNOWN_IP, worker::{ get_memory, get_vcpus, get_windmill_memory_usage, get_worker_memory_usage, insert_ping_query, update_job_ping_query, update_worker_ping_from_job_query, @@ -26,6 +27,7 @@ pub(crate) async fn update_worker_ping_full( hostname: &str, occupancy_metrics: &mut OccupancyMetrics, killpill_tx: &KillpillSender, + ip: Option<&str>, ) { let wc = WORKER_CONFIG.load(); let tags = wc.worker_tags.clone(); @@ -64,6 +66,7 @@ pub(crate) async fn update_worker_ping_full( occupancy_rate_5m, occupancy_rate_30m, native_mode, + ip, ) }) .retry( @@ -110,6 +113,7 @@ async fn update_worker_ping_full_inner( occupancy_rate_5m: Option, occupancy_rate_30m: Option, native_mode: bool, + ip: Option<&str>, ) -> anyhow::Result<()> { match conn { Connection::Sql(db) => { @@ -126,6 +130,7 @@ async fn update_worker_ping_full_inner( occupancy_rate_5m, occupancy_rate_30m, native_mode, + ip, db, ) .await?; @@ -139,7 +144,7 @@ async fn update_worker_ping_full_inner( last_job_executed: None, last_job_workspace_id: None, worker_instance: None, - ip: None, + ip: ip.map(str::to_string), tags: Some(tags.to_vec()), dw: None, dws: None, @@ -169,7 +174,7 @@ async fn update_worker_ping_full_inner( pub async fn insert_ping( worker_instance: &str, worker_name: &str, - ip: &str, + ip: Option<&str>, db: &Connection, ) -> anyhow::Result { let (tags, dw, dws, native_mode) = { @@ -228,7 +233,10 @@ pub async fn insert_ping( last_job_executed: None, last_job_workspace_id: None, worker_instance: Some(worker_instance.to_string()), - ip: Some(ip.to_string()), + // Servers older than the background lookup reject an initial ping with + // no IP, and an agent worker routinely runs against one, so the + // not-resolved-yet case goes over the wire as the sentinel. + ip: Some(ip.unwrap_or(UNKNOWN_IP).to_string()), tags: Some(tags.to_vec()), dw: dw, dws: dws, diff --git a/frontend/src/lib/components/WhitelistIp.svelte b/frontend/src/lib/components/WhitelistIp.svelte index 8d8bac1a5d..be2f8f56dd 100644 --- a/frontend/src/lib/components/WhitelistIp.svelte +++ b/frontend/src/lib/components/WhitelistIp.svelte @@ -4,12 +4,16 @@ let ips: string[] | undefined = $state(undefined) + // Sentinels the backend stores when a worker has no external IP to report: 'NO IP' while the + // lookup is still in flight, 'unretrievable IP' once it has failed. + const UNKNOWN_IPS = ['NO IP', 'unretrievable IP'] + WorkerService.listWorkers({ pingSince: 300 }).then((workers) => { ips = [ ...new Set( workers .filter((worker) => { - return worker.ip != 'unretrievable IP' && worker.last_ping && worker.last_ping < 300 + return !UNKNOWN_IPS.includes(worker.ip) && worker.last_ping && worker.last_ping < 300 }) .map((worker) => worker.ip) )