mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
Route Git scratch through configured volume (#1884)
Signed-off-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co> Co-authored-by: npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757 <5217c5c2f7bfb4333e46d17c98a9255a52dadee18dcd43a43536b95e6776dfa0@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
co-authored by
npub12gtutshhh76rx0jx697f32f9tffd4hhp3hx58fp4x6u4uemkm7sqf8f757
parent
c772ac08ca
commit
2318b3096c
@@ -225,15 +225,16 @@ impl ParentState {
|
||||
/// HEAD or no HEAD yields an empty string.
|
||||
async fn snapshot_workspace_state(
|
||||
repo_path: &Path,
|
||||
scratch_dir: &Path,
|
||||
) -> Result<(BTreeMap<String, String>, String), CasError> {
|
||||
const MAX_REF_SNAPSHOT_BYTES: u64 = 4 * 1024 * 1024;
|
||||
|
||||
let refs_stdout_tmp = tempfile::NamedTempFile::new()
|
||||
let refs_stdout_tmp = tempfile::NamedTempFile::new_in(scratch_dir)
|
||||
.map_err(|e| CasError::PackCapture(format!("for-each-ref stdout tempfile: {e}")))?;
|
||||
let refs_stdout_file = refs_stdout_tmp
|
||||
.reopen()
|
||||
.map_err(|e| CasError::PackCapture(format!("for-each-ref stdout reopen: {e}")))?;
|
||||
let refs_stderr_tmp = tempfile::NamedTempFile::new()
|
||||
let refs_stderr_tmp = tempfile::NamedTempFile::new_in(scratch_dir)
|
||||
.map_err(|e| CasError::PackCapture(format!("for-each-ref stderr tempfile: {e}")))?;
|
||||
let refs_stderr_file = refs_stderr_tmp
|
||||
.reopen()
|
||||
@@ -325,9 +326,11 @@ async fn write_idx_sidecar(
|
||||
store: &GitStore,
|
||||
pack_key: &str,
|
||||
pack_bytes: &[u8],
|
||||
scratch_dir: &Path,
|
||||
) -> Result<(), CasError> {
|
||||
let pack_digest = digest_from_pack_key(pack_key)?;
|
||||
let tempdir = TempDir::new().map_err(|e| CasError::PackCapture(format!("idx tempdir: {e}")))?;
|
||||
let tempdir = TempDir::new_in(scratch_dir)
|
||||
.map_err(|e| CasError::PackCapture(format!("idx tempdir in {scratch_dir:?}: {e}")))?;
|
||||
let pack_path = tempdir.path().join(format!("pack-{pack_digest}.pack"));
|
||||
tokio::fs::write(&pack_path, pack_bytes)
|
||||
.await
|
||||
@@ -380,6 +383,7 @@ async fn capture_pack(
|
||||
refs_before: &BTreeMap<String, String>,
|
||||
refs_after: &BTreeMap<String, String>,
|
||||
max_pack_bytes: u64,
|
||||
scratch_dir: &Path,
|
||||
) -> Result<Option<Vec<u8>>, CasError> {
|
||||
// Build rev-spec stdin: positive new tips, negative old tips.
|
||||
// Deduplicate against the same-oid case — no point feeding `X ^X`.
|
||||
@@ -401,12 +405,12 @@ async fn capture_pack(
|
||||
stdin_lines.push('\n');
|
||||
}
|
||||
|
||||
let stdout_tmp = tempfile::NamedTempFile::new()
|
||||
let stdout_tmp = tempfile::NamedTempFile::new_in(scratch_dir)
|
||||
.map_err(|e| CasError::PackCapture(format!("pack-objects stdout tempfile: {e}")))?;
|
||||
let stdout_file = stdout_tmp
|
||||
.reopen()
|
||||
.map_err(|e| CasError::PackCapture(format!("pack-objects stdout reopen: {e}")))?;
|
||||
let stderr_tmp = tempfile::NamedTempFile::new()
|
||||
let stderr_tmp = tempfile::NamedTempFile::new_in(scratch_dir)
|
||||
.map_err(|e| CasError::PackCapture(format!("pack-objects stderr tempfile: {e}")))?;
|
||||
let stderr_file = stderr_tmp
|
||||
.reopen()
|
||||
@@ -558,10 +562,17 @@ pub async fn cas_publish(
|
||||
) -> Result<CasSuccess, CasError> {
|
||||
let pkey = pointer_key(ctx.community(), owner, repo);
|
||||
|
||||
// Hydrated repositories are direct children of the configured Git scratch
|
||||
// root. Reuse that parent for publication tempfiles so they remain on the
|
||||
// mounted scratch volume without adding another independent path argument.
|
||||
let scratch_dir = repo_path
|
||||
.parent()
|
||||
.ok_or_else(|| CasError::PackCapture("repository path has no scratch parent".into()))?;
|
||||
|
||||
// Snapshot post-receive-pack state from disk. `parent_state.parent.refs`
|
||||
// are the refs the workspace was hydrated from — `pack-objects --revs`
|
||||
// below uses them as the "negative" set to produce the delta pack.
|
||||
let (refs_after, head_observed) = snapshot_workspace_state(repo_path).await?;
|
||||
let (refs_after, head_observed) = snapshot_workspace_state(repo_path, scratch_dir).await?;
|
||||
|
||||
// HEAD fallback: a bare repo serving pushes shouldn't have detached
|
||||
// HEAD, but if `git symbolic-ref` failed (or returned empty), inherit
|
||||
@@ -583,6 +594,7 @@ pub async fn cas_publish(
|
||||
&parent_state.parent.refs,
|
||||
&refs_after,
|
||||
limits.max_pack_bytes,
|
||||
scratch_dir,
|
||||
)
|
||||
.await?;
|
||||
let new_pack_key = if let Some(bytes) = pack_bytes {
|
||||
@@ -599,7 +611,7 @@ pub async fn cas_publish(
|
||||
}
|
||||
debug!(bytes = bytes.len(), "captured push pack");
|
||||
let pack_key = store.put_pack(&bytes).await?;
|
||||
if let Err(e) = write_idx_sidecar(store, &pack_key, &bytes).await {
|
||||
if let Err(e) = write_idx_sidecar(store, &pack_key, &bytes, scratch_dir).await {
|
||||
warn!(
|
||||
pack_key = %pack_key,
|
||||
error = %e,
|
||||
|
||||
@@ -105,6 +105,7 @@ pub async fn hydrate_for_read(
|
||||
ctx: &TenantContext,
|
||||
owner: &str,
|
||||
repo: &str,
|
||||
scratch_dir: &Path,
|
||||
max_pack_bytes: u64,
|
||||
max_repo_bytes: u64,
|
||||
) -> Result<Option<HydratedRepo>, HydrateError> {
|
||||
@@ -112,7 +113,14 @@ pub async fn hydrate_for_read(
|
||||
return Ok(None);
|
||||
};
|
||||
Ok(Some(
|
||||
materialize_manifest(store, &manifest, max_pack_bytes, max_repo_bytes).await?,
|
||||
materialize_manifest(
|
||||
store,
|
||||
&manifest,
|
||||
scratch_dir,
|
||||
max_pack_bytes,
|
||||
max_repo_bytes,
|
||||
)
|
||||
.await?,
|
||||
))
|
||||
}
|
||||
|
||||
@@ -158,13 +166,20 @@ pub async fn hydrate_for_write(
|
||||
ctx: &TenantContext,
|
||||
owner: &str,
|
||||
repo: &str,
|
||||
scratch_dir: &Path,
|
||||
max_pack_bytes: u64,
|
||||
max_repo_bytes: u64,
|
||||
) -> Result<(HydratedRepo, ParentState), HydrateError> {
|
||||
match load_pointer(store, ctx, owner, repo).await? {
|
||||
Some((etag, digest, manifest)) => {
|
||||
let repo =
|
||||
materialize_manifest(store, &manifest, max_pack_bytes, max_repo_bytes).await?;
|
||||
let repo = materialize_manifest(
|
||||
store,
|
||||
&manifest,
|
||||
scratch_dir,
|
||||
max_pack_bytes,
|
||||
max_repo_bytes,
|
||||
)
|
||||
.await?;
|
||||
let parent = ParentState::from_loaded(etag, digest, manifest);
|
||||
Ok((repo, parent))
|
||||
}
|
||||
@@ -172,8 +187,8 @@ pub async fn hydrate_for_write(
|
||||
// First push: empty bare repo. No packs to fetch, no refs to
|
||||
// install. `receive-pack` will accept whatever the client
|
||||
// sends; `cas_publish` will use `If-None-Match: *`.
|
||||
let tempdir =
|
||||
TempDir::new().map_err(|e| HydrateError::Hydrate(format!("tempdir: {e}")))?;
|
||||
let tempdir = TempDir::new_in(scratch_dir)
|
||||
.map_err(|e| HydrateError::Hydrate(format!("tempdir in {scratch_dir:?}: {e}")))?;
|
||||
let path = tempdir.path().to_path_buf();
|
||||
run_git(&path, &["init", "--bare", "--quiet"]).await?;
|
||||
Ok((
|
||||
@@ -242,11 +257,13 @@ async fn get_verified_limited(
|
||||
async fn materialize_manifest(
|
||||
store: &GitStore,
|
||||
manifest: &Manifest,
|
||||
scratch_dir: &Path,
|
||||
max_pack_bytes: u64,
|
||||
max_repo_bytes: u64,
|
||||
) -> Result<HydratedRepo, HydrateError> {
|
||||
// Init bare repo.
|
||||
let tempdir = TempDir::new().map_err(|e| HydrateError::Hydrate(format!("tempdir: {e}")))?;
|
||||
let tempdir = TempDir::new_in(scratch_dir)
|
||||
.map_err(|e| HydrateError::Hydrate(format!("tempdir in {scratch_dir:?}: {e}")))?;
|
||||
let path = tempdir.path().to_path_buf();
|
||||
run_git(&path, &["init", "--bare", "--quiet"]).await?;
|
||||
|
||||
@@ -443,6 +460,26 @@ mod tests {
|
||||
)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn materialized_repo_is_created_under_configured_scratch_dir() {
|
||||
let scratch = TempDir::new().unwrap();
|
||||
let store = GitStore::new("http://localhost:9000", "x", "x", "x", "us-east-1")
|
||||
.expect("construct store");
|
||||
let manifest = Manifest {
|
||||
version: 1,
|
||||
head: "refs/heads/main".into(),
|
||||
refs: BTreeMap::new(),
|
||||
packs: Vec::new(),
|
||||
parent: None,
|
||||
};
|
||||
|
||||
let hydrated = materialize_manifest(&store, &manifest, scratch.path(), u64::MAX, u64::MAX)
|
||||
.await
|
||||
.expect("materialize empty repo");
|
||||
|
||||
assert!(hydrated.path().starts_with(scratch.path()));
|
||||
}
|
||||
|
||||
// -------- Live MinIO + real git roundtrip ----------------------------------
|
||||
//
|
||||
// Run manually:
|
||||
@@ -557,10 +594,12 @@ mod tests {
|
||||
}
|
||||
|
||||
// Hydrate.
|
||||
let hydrated = hydrate_for_read(&st, &ctx, &owner, repo, u64::MAX, u64::MAX)
|
||||
.await
|
||||
.expect("hydrate")
|
||||
.expect("hydrate Some");
|
||||
let scratch = TempDir::new().unwrap();
|
||||
let hydrated =
|
||||
hydrate_for_read(&st, &ctx, &owner, repo, scratch.path(), u64::MAX, u64::MAX)
|
||||
.await
|
||||
.expect("hydrate")
|
||||
.expect("hydrate Some");
|
||||
eprintln!("hydrated to {}", hydrated.path().display());
|
||||
|
||||
// The hydrated repo must list the same ref with the same oid.
|
||||
@@ -630,9 +669,18 @@ mod tests {
|
||||
let st = store();
|
||||
let owner = format!("nope-{}", uuid::Uuid::new_v4());
|
||||
let ctx = tenant();
|
||||
let result = hydrate_for_read(&st, &ctx, &owner, "ghost", u64::MAX, u64::MAX)
|
||||
.await
|
||||
.expect("ok");
|
||||
let scratch = TempDir::new().unwrap();
|
||||
let result = hydrate_for_read(
|
||||
&st,
|
||||
&ctx,
|
||||
&owner,
|
||||
"ghost",
|
||||
scratch.path(),
|
||||
u64::MAX,
|
||||
u64::MAX,
|
||||
)
|
||||
.await
|
||||
.expect("ok");
|
||||
assert!(result.is_none(), "missing pointer must surface as None");
|
||||
}
|
||||
|
||||
@@ -677,10 +725,19 @@ mod tests {
|
||||
super::super::store::CasOutcome::LostRace => panic!("first INM* must win"),
|
||||
}
|
||||
|
||||
let hydrated = hydrate_for_read(&st, &ctx, &owner, "void", u64::MAX, u64::MAX)
|
||||
.await
|
||||
.expect("hydrate")
|
||||
.expect("hydrate Some");
|
||||
let scratch = TempDir::new().unwrap();
|
||||
let hydrated = hydrate_for_read(
|
||||
&st,
|
||||
&ctx,
|
||||
&owner,
|
||||
"void",
|
||||
scratch.path(),
|
||||
u64::MAX,
|
||||
u64::MAX,
|
||||
)
|
||||
.await
|
||||
.expect("hydrate")
|
||||
.expect("hydrate Some");
|
||||
|
||||
// HEAD points where the manifest said.
|
||||
let head_file = tokio::fs::read_to_string(hydrated.path().join("HEAD"))
|
||||
|
||||
@@ -581,6 +581,7 @@ async fn info_refs_subprocess(
|
||||
tenant,
|
||||
¶ms.owner,
|
||||
¶ms.repo,
|
||||
&state.config.git_repo_path,
|
||||
state.config.git_max_pack_bytes,
|
||||
state.config.git_max_repo_bytes,
|
||||
)
|
||||
@@ -596,11 +597,11 @@ async fn info_refs_subprocess(
|
||||
// "receive-pack" (without the "git-" prefix).
|
||||
let git_subcmd = service.strip_prefix("git-").unwrap_or(service);
|
||||
|
||||
let stdout_tmp = tempfile::NamedTempFile::new().map_err(|e| {
|
||||
let stdout_tmp = tempfile::NamedTempFile::new_in(&state.config.git_repo_path).map_err(|e| {
|
||||
error!(error = %e, "git info_refs stdout tempfile failed");
|
||||
(StatusCode::INTERNAL_SERVER_ERROR, "git error").into_response()
|
||||
})?;
|
||||
let stderr_tmp = tempfile::NamedTempFile::new().map_err(|e| {
|
||||
let stderr_tmp = tempfile::NamedTempFile::new_in(&state.config.git_repo_path).map_err(|e| {
|
||||
error!(error = %e, "git info_refs stderr tempfile failed");
|
||||
(StatusCode::INTERNAL_SERVER_ERROR, "git error").into_response()
|
||||
})?;
|
||||
@@ -712,6 +713,7 @@ pub async fn upload_pack(
|
||||
&auth.tenant,
|
||||
¶ms.owner,
|
||||
¶ms.repo,
|
||||
&state.config.git_repo_path,
|
||||
state.config.git_max_pack_bytes,
|
||||
state.config.git_max_repo_bytes,
|
||||
)
|
||||
@@ -799,6 +801,7 @@ pub async fn receive_pack(
|
||||
&auth.tenant,
|
||||
¶ms.owner,
|
||||
¶ms.repo,
|
||||
&state.config.git_repo_path,
|
||||
state.config.git_max_pack_bytes,
|
||||
state.config.git_max_repo_bytes,
|
||||
)
|
||||
@@ -848,6 +851,7 @@ pub async fn receive_pack(
|
||||
"receive-pack",
|
||||
body,
|
||||
&hook_env,
|
||||
&state.config.git_repo_path,
|
||||
RECEIVE_PACK_MAX_OUTPUT_BYTES,
|
||||
)
|
||||
.await?;
|
||||
@@ -900,13 +904,14 @@ async fn run_git_at(
|
||||
service: &str,
|
||||
body: Body,
|
||||
extra_env: &[(&str, String)],
|
||||
scratch_dir: &Path,
|
||||
max_output_bytes: u64,
|
||||
) -> Result<PackOutput, Response> {
|
||||
let stdout_tmp = tempfile::NamedTempFile::new().map_err(|e| {
|
||||
let stdout_tmp = tempfile::NamedTempFile::new_in(scratch_dir).map_err(|e| {
|
||||
error!(error = %e, service = %service, "git stdout tempfile failed");
|
||||
(StatusCode::INTERNAL_SERVER_ERROR, "git error").into_response()
|
||||
})?;
|
||||
let stderr_tmp = tempfile::NamedTempFile::new().map_err(|e| {
|
||||
let stderr_tmp = tempfile::NamedTempFile::new_in(scratch_dir).map_err(|e| {
|
||||
error!(error = %e, service = %service, "git stderr tempfile failed");
|
||||
(StatusCode::INTERNAL_SERVER_ERROR, "git error").into_response()
|
||||
})?;
|
||||
|
||||
@@ -169,12 +169,10 @@ pub struct Config {
|
||||
|
||||
/// Root directory for the relay's local git scratch. No per-repo bare repos
|
||||
/// or persistent git state live here — runtime reads/writes hydrate
|
||||
/// ephemeral repos from object storage per request, and repo-name
|
||||
/// uniqueness now lives in Postgres (`git_repo_names`), not on disk. Retained
|
||||
/// for ephemeral working space and env compatibility; the relay no longer
|
||||
/// depends on this path being persistent or shared across replicas, so it
|
||||
/// needs no ReadWriteMany volume. (Removing the field entirely is a
|
||||
/// follow-up cleanup once the deploy chart drops the git PVC mount.)
|
||||
/// ephemeral repos from object storage per request, and all temporary Git
|
||||
/// workspaces and buffered subprocess output are created beneath this path.
|
||||
/// Repo-name uniqueness lives in Postgres (`git_repo_names`), not on disk,
|
||||
/// so this directory need not be persistent or shared across replicas.
|
||||
pub git_repo_path: std::path::PathBuf,
|
||||
/// Maximum pack file size for git push (bytes). Default: 500 MB.
|
||||
pub git_max_pack_bytes: u64,
|
||||
|
||||
@@ -219,5 +219,6 @@ spec:
|
||||
persistentVolumeClaim:
|
||||
claimName: {{ default (printf "%s-git" (include "buzz.fullname" .)) .Values.persistence.git.existingClaim }}
|
||||
{{- else }}
|
||||
emptyDir: {}
|
||||
emptyDir:
|
||||
sizeLimit: {{ .Values.persistence.git.size | quote }}
|
||||
{{- end }}
|
||||
|
||||
@@ -130,7 +130,8 @@ tests:
|
||||
path: spec.template.spec.volumes
|
||||
content:
|
||||
name: git-repos
|
||||
emptyDir: {}
|
||||
emptyDir:
|
||||
sizeLimit: 10Gi
|
||||
template: templates/deployment.yaml
|
||||
# No PVC is rendered at all when persistence is disabled.
|
||||
- hasDocuments:
|
||||
|
||||
@@ -214,17 +214,18 @@ httproute:
|
||||
# enabled: true → mount a PVC at mountPath (durable across pod restarts, but a
|
||||
# single ReadWriteOnce PVC binds to one node, so it does NOT support multi-pod
|
||||
# scheduling across nodes on a Deployment).
|
||||
# enabled: false → mount a per-pod emptyDir at mountPath (pure scratch). This is
|
||||
# the correct choice for multi-replica HA: each pod gets its own local working
|
||||
# space, nothing is shared, and there is no volume to multi-attach. Safe
|
||||
# because the object store + Postgres are the sources of truth, not this disk.
|
||||
# enabled: false → mount a per-pod emptyDir at mountPath (pure scratch), bounded
|
||||
# by size. This is the correct choice for multi-replica HA: each pod gets its
|
||||
# own local working space, nothing is shared, and there is no volume to
|
||||
# multi-attach. Safe because the object store + Postgres are the sources of
|
||||
# truth, not this disk.
|
||||
persistence:
|
||||
git:
|
||||
enabled: true
|
||||
mountPath: /var/lib/buzz/git
|
||||
storageClass: ""
|
||||
accessMode: ReadWriteOnce
|
||||
size: 10Gi
|
||||
size: 10Gi # PVC capacity or emptyDir sizeLimit
|
||||
annotations: {}
|
||||
existingClaim: ""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user