From da2431f11fcf71fbe905f6bc129b6f7902f3d44e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arpad=20M=C3=BCller?= <arpad-m@users.noreply.github.com>
Date: Wed, 12 Mar 2025 03:30:56 +0100
Subject: [PATCH] storcon: add --control-plane-url config option (#11173)

Adds the `--control-plane-url` config option to the storcon, which we
want to migrate to instead of using `notify-attach`.

Part of #11163
---
 docs/storage_controller.md             |  2 +-
 storage_controller/src/compute_hook.rs | 11 ++++++++++-
 storage_controller/src/main.rs         | 13 ++++++++++---
 storage_controller/src/service.rs      |  9 +++++++++
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/docs/storage_controller.md b/docs/storage_controller.md
index 6d2ef929a4..cf00cd8e33 100644
--- a/docs/storage_controller.md
+++ b/docs/storage_controller.md
@@ -101,7 +101,7 @@ changes such as a pageserver node becoming unavailable, or the tenant's shard co
 postgres clients to handle such changes, the storage controller calls an API hook when a tenant's pageserver
 location changes.
 
-The hook is configured using the storage controller's `--compute-hook-url` CLI option. If the hook requires
+The hook is configured using the storage controller's `--control-plane-url` CLI option. If the hook requires
 JWT auth, the token may be provided with `--control-plane-jwt-token`. The hook will be invoked with a `PUT` request.
 
 In the Neon cloud service, this hook is implemented by Neon's internal cloud control plane. In `neon_local` systems
diff --git a/storage_controller/src/compute_hook.rs b/storage_controller/src/compute_hook.rs
index b602af362d..5ce4d63d77 100644
--- a/storage_controller/src/compute_hook.rs
+++ b/storage_controller/src/compute_hook.rs
@@ -624,7 +624,16 @@ impl ComputeHook {
             MaybeSendResult::Transmit((request, lock)) => (request, lock),
         };
 
-        let result = if let Some(notify_url) = &self.config.compute_hook_url {
+        let compute_hook_url = if let Some(control_plane_url) = &self.config.control_plane_url {
+            Some(if control_plane_url.ends_with('/') {
+                format!("{control_plane_url}notify-attach")
+            } else {
+                format!("{control_plane_url}/notify-attach")
+            })
+        } else {
+            self.config.compute_hook_url.clone()
+        };
+        let result = if let Some(notify_url) = &compute_hook_url {
             self.do_notify(notify_url, &request, cancel).await
         } else {
             self.do_notify_local(&request).await.map_err(|e| {
diff --git a/storage_controller/src/main.rs b/storage_controller/src/main.rs
index 46ac1cd7ca..6e3c70c42b 100644
--- a/storage_controller/src/main.rs
+++ b/storage_controller/src/main.rs
@@ -71,6 +71,10 @@ struct Cli {
     #[arg(long)]
     compute_hook_url: Option<String>,
 
+    /// URL to control plane storage API prefix
+    #[arg(long)]
+    control_plane_url: Option<String>,
+
     /// URL to connect to postgres, like postgresql://localhost:1234/storage_controller
     #[arg(long)]
     database_url: Option<String>,
@@ -313,11 +317,13 @@ async fn async_main() -> anyhow::Result<()> {
                 "Insecure config!  One or more secrets is not set.  This is only permitted in `--dev` mode"
             );
         }
-        StrictMode::Strict if args.compute_hook_url.is_none() => {
-            // Production systems should always have a compute hook set, to prevent falling
+        StrictMode::Strict
+            if args.compute_hook_url.is_none() && args.control_plane_url.is_none() =>
+        {
+            // Production systems should always have a control plane URL set, to prevent falling
             // back to trying to use neon_local.
             anyhow::bail!(
-                "`--compute-hook-url` is not set: this is only permitted in `--dev` mode"
+                "neither `--compute-hook-url` nor `--control-plane-url` are set: this is only permitted in `--dev` mode"
             );
         }
         StrictMode::Strict => {
@@ -343,6 +349,7 @@ async fn async_main() -> anyhow::Result<()> {
         control_plane_jwt_token: secrets.control_plane_jwt_token,
         peer_jwt_token: secrets.peer_jwt_token,
         compute_hook_url: args.compute_hook_url,
+        control_plane_url: args.control_plane_url,
         max_offline_interval: args
             .max_offline_interval
             .map(humantime::Duration::into)
diff --git a/storage_controller/src/service.rs b/storage_controller/src/service.rs
index a06748abc6..96b67fa81e 100644
--- a/storage_controller/src/service.rs
+++ b/storage_controller/src/service.rs
@@ -363,6 +363,15 @@ pub struct Config {
     /// assume it is running in a test environment and try to update neon_local.
     pub compute_hook_url: Option<String>,
 
+    /// Prefix for storage API endpoints of the control plane. We use this prefix to compute
+    /// URLs that we use to send pageserver and safekeeper attachment locations.
+    /// If this is None, the compute hook will assume it is running in a test environment
+    /// and try to invoke neon_local instead.
+    ///
+    /// For now, there is also `compute_hook_url` which allows configuration of the pageserver
+    /// specific endpoint, but it is in the process of being phased out.
+    pub control_plane_url: Option<String>,
+
     /// Grace period within which a pageserver does not respond to heartbeats, but is still
     /// considered active. Once the grace period elapses, the next heartbeat failure will
     /// mark the pagseserver offline.