Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions __test__/mirror-cleanup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe('cleanup commit decision', () => {
})
expect(commands().some(c => c.includes('repack'))).toBe(false)
expect(mockCommitStickyDisk).toHaveBeenCalledWith(
expect.objectContaining({shouldCommit: false, vmHydratedGitMirror: false})
expect.objectContaining({shouldCommit: false, vmHydratedGitMirror: true})
)
})

Expand All @@ -197,7 +197,21 @@ describe('cleanup commit decision', () => {

expect(commands().some(c => c.includes('repack'))).toBe(false)
expect(mockCommitStickyDisk).toHaveBeenCalledWith(
expect.objectContaining({shouldCommit: false, vmHydratedGitMirror: false})
expect.objectContaining({shouldCommit: false, vmHydratedGitMirror: true})
)
})

it('reports a performed hydration as-is when the job is not committing', async () => {
await blacksmithCache.cleanup({
...base,
mirrorPath,
shouldCommit: false,
vmHydratedGitMirror: true
})

expect(commands().some(c => c.includes('repack'))).toBe(false)
expect(mockCommitStickyDisk).toHaveBeenCalledWith(
expect.objectContaining({shouldCommit: false, vmHydratedGitMirror: true})
)
})

Expand Down
17 changes: 6 additions & 11 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1924,13 +1924,8 @@ function flushBlockDevice(devicePath) {
function cleanup(options) {
return __awaiter(this, void 0, void 0, function* () {
var _a;
const { exposeId, stickyDiskKey, repoName, mountPoint, mirrorPath, mirrorSyncFailed, mirrorSyncTimedOut } = options;
const { exposeId, stickyDiskKey, repoName, mountPoint, mirrorPath, mirrorSyncFailed, mirrorSyncTimedOut, vmHydratedGitMirror } = options;
let { shouldCommit } = options;
// vmHydratedGitMirror must track shouldCommit: if we decide not to commit
// (due to sync failure), we must not tell the backend that hydration
// completed, otherwise it marks the entry as ready despite no valid disk
// being persisted.
let vmHydratedGitMirror = options.vmHydratedGitMirror;
const result = {
// skipped until maintenance actually runs, so a mirror that is not
// maintained (no mirrorPath, or not committing) never reports a
Expand All @@ -1943,7 +1938,6 @@ function cleanup(options) {
const reason = mirrorSyncTimedOut ? 'timed out' : 'failed';
core.warning(`[git-mirror] Mirror sync ${reason}, will not commit sticky disk`);
shouldCommit = false;
vmHydratedGitMirror = false;
}
// Maintenance only pays off if the result is persisted.
if (mirrorPath && shouldCommit) {
Expand Down Expand Up @@ -2043,7 +2037,6 @@ function cleanup(options) {
if (!unmountSuccess) {
core.warning(`[git-mirror] Failed to unmount ${mountPoint} after ${UMOUNT_MAX_RETRIES} attempts, will not commit sticky disk`);
shouldCommit = false;
vmHydratedGitMirror = false;
}
}
// Flush block device buffers after unmount to ensure data durability
Expand Down Expand Up @@ -4792,16 +4785,18 @@ function cleanup() {
shouldCommit = false;
core.info('[git-mirror] Mirror unchanged since last commit, releasing sticky disk without commit');
}
// Only set vmHydratedGitMirror to true if we're committing AND we performed hydration
const vmHydratedGitMirror = shouldCommit && performedHydration;
// vmHydratedGitMirror reports whether this job performed the initial
// clone, independent of shouldCommit: the host combines the two to
// decide whether the hydration was persisted, and classifies a clone
// that is not committed separately from a clone that failed.
cleanupResult = yield blacksmithCache.cleanup({
exposeId,
stickyDiskKey,
repoName: repoName || undefined,
mountPoint: mountPoint || undefined,
mirrorPath: mirrorChanged ? mirrorPath || undefined : undefined,
shouldCommit,
vmHydratedGitMirror,
vmHydratedGitMirror: performedHydration,
mirrorSyncFailed,
mirrorSyncTimedOut
});
Expand Down
16 changes: 6 additions & 10 deletions src/blacksmith-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2304,8 +2304,10 @@ export interface CleanupOptions {
// shouldCommit indicates whether changes should be persisted.
// Set to false if the job failed/was cancelled to avoid committing bad state.
shouldCommit: boolean
// vmHydratedGitMirror indicates this job performed initial git mirror clone.
// Used by backend to mark hydration as complete.
// vmHydratedGitMirror indicates this job performed the initial git mirror
// clone. It is reported as-is, independent of shouldCommit: the host only
// marks hydration complete when the disk is also committed, and uses the
// pair to tell a clone that was not persisted apart from a failed clone.
vmHydratedGitMirror: boolean
// Mirror sync outcome from the main step.
mirrorSyncFailed?: boolean
Expand All @@ -2327,14 +2329,10 @@ export async function cleanup(options: CleanupOptions): Promise<CleanupResult> {
mountPoint,
mirrorPath,
mirrorSyncFailed,
mirrorSyncTimedOut
mirrorSyncTimedOut,
vmHydratedGitMirror
} = options
let {shouldCommit} = options
// vmHydratedGitMirror must track shouldCommit: if we decide not to commit
// (due to sync failure), we must not tell the backend that hydration
// completed, otherwise it marks the entry as ready despite no valid disk
// being persisted.
let vmHydratedGitMirror = options.vmHydratedGitMirror

const result: CleanupResult = {
// skipped until maintenance actually runs, so a mirror that is not
Expand All @@ -2354,7 +2352,6 @@ export async function cleanup(options: CleanupOptions): Promise<CleanupResult> {
`[git-mirror] Mirror sync ${reason}, will not commit sticky disk`
)
shouldCommit = false
vmHydratedGitMirror = false
}

// Maintenance only pays off if the result is persisted.
Expand Down Expand Up @@ -2484,7 +2481,6 @@ export async function cleanup(options: CleanupOptions): Promise<CleanupResult> {
`[git-mirror] Failed to unmount ${mountPoint} after ${UMOUNT_MAX_RETRIES} attempts, will not commit sticky disk`
)
shouldCommit = false
vmHydratedGitMirror = false
}
}

Expand Down
9 changes: 5 additions & 4 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,17 +141,18 @@ async function cleanup(): Promise<void> {
)
}

// Only set vmHydratedGitMirror to true if we're committing AND we performed hydration
const vmHydratedGitMirror = shouldCommit && performedHydration

// vmHydratedGitMirror reports whether this job performed the initial
// clone, independent of shouldCommit: the host combines the two to
// decide whether the hydration was persisted, and classifies a clone
// that is not committed separately from a clone that failed.
cleanupResult = await blacksmithCache.cleanup({
exposeId,
stickyDiskKey,
repoName: repoName || undefined,
mountPoint: mountPoint || undefined,
mirrorPath: mirrorChanged ? mirrorPath || undefined : undefined,
shouldCommit,
vmHydratedGitMirror,
vmHydratedGitMirror: performedHydration,
mirrorSyncFailed,
mirrorSyncTimedOut
})
Expand Down
Loading