Allow creating config for attached tenant

pageserver/src/tenant.rs

@@ -1807,7 +1807,6 @@ impl Tenant {
         let _enter = info_span!("saving tenantconf").entered();
         info!("persisting tenantconf to {}", target_config_path.display());
 
-        // TODO this will prepend comments endlessly ?
         let mut conf_content = r#"# This file contains a specific per-tenant's config.
 #  It is read in case of pageserver restart.
 
@@ -1823,7 +1822,10 @@ impl Tenant {
             OpenOptions::new()
                 .truncate(true) // This needed for overwriting with small config files
                 .write(true)
-                .create_new(first_save),
+                .create_new(first_save)
+                // when initializing a new tenant, first_save will be true, thus this flag will be
+                // ignored (per rust std docs). later, don't care.
+                .create(true),
         )?;
 
         target_config_file
@@ -1841,26 +1843,26 @@ impl Tenant {
                 )
             })?;
 
-        // fsync the parent directory to ensure the directory entry is durable
-        if first_save {
-            target_config_path
-                .parent()
-                .context("Config file does not have a parent")
-                .and_then(|target_config_parent| {
-                    File::open(target_config_parent).context("Failed to open config parent")
-                })
-                .and_then(|tenant_dir| {
-                    tenant_dir
-                        .sync_all()
-                        .context("Failed to fsync config parent")
-                })
-                .with_context(|| {
-                    format!(
-                        "Failed to fsync on first save for config {}",
-                        target_config_path.display()
-                    )
-                })?;
-        }
+        // fsync the parent directory to ensure the directory entry is durable.
+        // before this was done conditionally on first_save, but these management actions are rare
+        // enough to just fsync it always.
+        target_config_path
+            .parent()
+            .context("Config file does not have a parent")
+            .and_then(|target_config_parent| {
+                File::open(target_config_parent).context("Failed to open config parent")
+            })
+            .and_then(|tenant_dir| {
+                tenant_dir
+                    .sync_all()
+                    .context("Failed to fsync config parent")
+            })
+            .with_context(|| {
+                format!(
+                    "Failed to fsync on first save for config {}",
+                    target_config_path.display()
+                )
+            })?;
 
         Ok(())
     }

test_runner/regress/test_tenant_conf.py

@@ -1,8 +1,16 @@
+import shutil
 from contextlib import closing
+from pathlib import Path
 
 import psycopg2.extras
 from fixtures.log_helper import log
-from fixtures.neon_fixtures import NeonEnvBuilder
+from fixtures.neon_fixtures import (
+    LocalFsStorage,
+    NeonEnvBuilder,
+    RemoteStorageKind,
+    assert_tenant_status,
+)
+from fixtures.utils import wait_until
 
 
 def test_tenant_config(neon_env_builder: NeonEnvBuilder):
@@ -158,3 +166,53 @@ def test_tenant_config(neon_env_builder: NeonEnvBuilder):
                     "pitr_interval": 60,
                 }.items()
             )
+
+
+def test_creating_tenant_conf_after_attach(neon_env_builder: NeonEnvBuilder):
+    neon_env_builder.enable_remote_storage(
+        remote_storage_kind=RemoteStorageKind.LOCAL_FS,
+        test_name="test_creating_tenant_conf_after_attach",
+    )
+
+    env = neon_env_builder.init_start()
+    assert isinstance(env.remote_storage, LocalFsStorage)
+
+    # tenant is created with defaults, as in without config file
+    (tenant_id, timeline_id) = env.neon_cli.create_tenant()
+    config_path = env.repo_dir / "tenants" / str(tenant_id) / "config"
+    assert config_path.exists(), "config file is always initially created"
+
+    env.pageserver.stop()
+
+    remote_dir = (
+        env.remote_storage.root / "tenants" / str(tenant_id) / "timelines" / str(timeline_id)
+    )
+    found_any = False
+    for path in Path.iterdir(remote_dir):
+        if not path.is_file():
+            continue
+        found_any = True
+        break
+
+    assert found_any, "should had have files under timeline directory"
+
+    # now prepare for attaching it once more
+    shutil.rmtree(Path(env.repo_dir) / "tenants" / str(tenant_id))
+
+    env.pageserver.start()
+    http_client = env.pageserver.http_client()
+    http_client.tenant_attach(tenant_id)
+    wait_until(
+        number_of_iterations=5,
+        interval=1,
+        func=lambda: assert_tenant_status(http_client, tenant_id, "Active"),
+    )
+
+    env.neon_cli.config_tenant(tenant_id, {"gc_horizon": "1000000"})
+    contents_first = config_path.read_text("utf-8")
+    env.neon_cli.config_tenant(tenant_id, {"gc_horizon": "0"})
+    contents_later = config_path.read_text("utf-8")
+
+    # dont test applying the setting here, we have that another test case to show it
+    # we just care about being able to create the file
+    assert len(contents_first) > len(contents_later)