Merge branch 'smh-backwards-compatible-hooks' into 'master'

Extract hooks also into their non-WAL location

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5842

Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Approved-by: John Cai <jcai@gitlab.com>

1 files changed, 45 insertions, 6 deletions

diff --git a/internal/gitaly/transaction_manager.go b/internal/gitaly/transaction_manager.go
index 7d7a22da8..0d1aaf6e6 100644
--- a/internal/gitaly/transaction_manager.go
+++ b/internal/gitaly/transaction_manager.go
@@ -1535,6 +1535,10 @@ func (mgr *TransactionManager) applyPackFile(ctx context.Context, packPrefix str
 // applyCustomHooks applies the custom hooks to the repository from the log entry. The custom hooks are stored
 // at `<repo>/wal/hooks/<log_index>`. The custom hooks are fsynced prior to returning so it is safe to delete
 // the log entry afterwards.
+//
+// The hooks are also extracted at `<repo>/custom_hooks`. This is done for backwards compatibility, as we want
+// the hooks to be present even if the WAL logic is disabled. This ensures we don't lose data if we have to
+// disable the WAL logic after rollout.
 func (mgr *TransactionManager) applyCustomHooks(ctx context.Context, logIndex LogIndex, update *gitalypb.LogEntry_CustomHooksUpdate) error {
 	if update == nil {
 		return nil
@@ -1550,14 +1554,22 @@ func (mgr *TransactionManager) applyCustomHooks(ctx context.Context, logIndex Lo
 		}
 	}
 
-	if err := repoutil.ExtractHooks(ctx, bytes.NewReader(update.CustomHooksTar), targetDirectory, true); err != nil {
-		return fmt.Errorf("extract hooks: %w", err)
+	syncer := safe.NewSyncer()
+	extractHooks := func(destinationDir string) error {
+		if err := repoutil.ExtractHooks(ctx, bytes.NewReader(update.CustomHooksTar), destinationDir, true); err != nil {
+			return fmt.Errorf("extract hooks: %w", err)
+		}
+
+		// TAR doesn't sync the extracted files so do it manually here.
+		if err := syncer.SyncRecursive(destinationDir); err != nil {
+			return fmt.Errorf("sync hooks: %w", err)
+		}
+
+		return nil
 	}
 
-	syncer := safe.NewSyncer()
-	// TAR doesn't sync the extracted files so do it manually here.
-	if err := syncer.SyncRecursive(targetDirectory); err != nil {
-		return fmt.Errorf("sync hooks: %w", err)
+	if err := extractHooks(targetDirectory); err != nil {
+		return fmt.Errorf("extract hooks: %w", err)
 	}
 
 	// Sync the parent directory as well.
@@ -1565,6 +1577,33 @@ func (mgr *TransactionManager) applyCustomHooks(ctx context.Context, logIndex Lo
 		return fmt.Errorf("sync hook directory: %w", err)
 	}
 
+	// Extract another copy that we can move to `<repo>/custom_hooks` where the hooks exist without the WAL enabled.
+	// We make a second copy as if we disable the WAL, we have to clear all of its state prior to re-enabling it.
+	// This would clear the hooks so symbolic linking the first copy is not enough.
+	tmpDir, err := os.MkdirTemp(mgr.stagingDirectory, "")
+	if err != nil {
+		return fmt.Errorf("create temporary directory: %w", err)
+	}
+
+	if err := extractHooks(tmpDir); err != nil {
+		return fmt.Errorf("extract legacy hooks: %w", err)
+	}
+
+	legacyHooksPath := filepath.Join(mgr.repositoryPath, repoutil.CustomHooksDir)
+	// The hooks are lost if we perform this removal but fail to perform the remaining operations and the
+	// WAL is disabled before succeeding. This is an existing issue already with SetCustomHooks RPC.
+	if err := os.RemoveAll(legacyHooksPath); err != nil {
+		return fmt.Errorf("remove existing legacy hooks: %w", err)
+	}
+
+	if err := os.Rename(tmpDir, legacyHooksPath); err != nil {
+		return fmt.Errorf("move legacy hooks in place: %w", err)
+	}
+
+	if err := syncer.SyncParent(legacyHooksPath); err != nil {
+		return fmt.Errorf("sync legacy hooks directory entry: %w", err)
+	}
+
 	return nil
 }