Recreate referenced files that were manually deleted on the data plane during a config apply (#1084)

Author: aphralG

internal/file/file_manager_service.go

@@ -195,7 +195,7 @@ func (fms *FileManagerService) UpdateOverview(
 	delta := files.ConvertToMapOfFiles(response.GetOverview().GetFiles())
 
 	if len(delta) != 0 {
-		return fms.updateFiles(ctx, delta, instanceID, iteration)
+		return fms.updateFiles(ctx, delta, request.GetOverview().GetFiles(), instanceID, iteration)
 	}
 
 	return err
@@ -219,6 +219,7 @@ func (fms *FileManagerService) setupIdentifiers(ctx context.Context, iteration i
 func (fms *FileManagerService) updateFiles(
 	ctx context.Context,
 	delta map[string]*mpi.File,
+	fileOverview []*mpi.File,
 	instanceID string,
 	iteration int,
 ) error {
@@ -234,7 +235,7 @@ func (fms *FileManagerService) updateFiles(
 	iteration++
 	slog.Debug("Updating file overview", "attempt_number", iteration)
 
-	return fms.UpdateOverview(ctx, instanceID, diffFiles, iteration)
+	return fms.UpdateOverview(ctx, instanceID, fileOverview, iteration)
 }
 
 func (fms *FileManagerService) UpdateFile(
@@ -728,7 +729,7 @@ func (fms *FileManagerService) DetermineFileActions(
 	fileDiff := make(map[string]*model.FileCache) // Files that have changed, key is file name
 	fileContents := make(map[string][]byte)       // contents of the file, key is file name
 
-	manifestFiles, filesMap, manifestFileErr := fms.manifestFile()
+	_, filesMap, manifestFileErr := fms.manifestFile()
 
 	if manifestFileErr != nil {
 		if errors.Is(manifestFileErr, os.ErrNotExist) {
@@ -769,14 +770,15 @@ func (fms *FileManagerService) DetermineFileActions(
 		}
 		// if file doesn't exist in the current files, file has been added
 		// set file action
-		if !ok {
+		if _, statErr := os.Stat(modifiedFile.File.GetFileMeta().GetName()); errors.Is(statErr, os.ErrNotExist) {
+			slog.Info("File is not present on disk", "file", modifiedFile.File.GetFileMeta().GetName())
 			modifiedFile.Action = model.Add
 			fileDiff[modifiedFile.File.GetFileMeta().GetName()] = modifiedFile
 
 			continue
 			// if file currently exists and file hash is different, file has been updated
 			// copy contents, set file action
-		} else if modifiedFile.File.GetFileMeta().GetHash() != currentFile.GetFileMeta().GetHash() {
+		} else if ok && modifiedFile.File.GetFileMeta().GetHash() != currentFile.GetFileMeta().GetHash() {
 			fileContent, readErr := os.ReadFile(fileName)
 			if readErr != nil {
 				return nil, nil, fmt.Errorf("error reading file %s, error: %w", fileName, readErr)
@@ -785,24 +787,6 @@ func (fms *FileManagerService) DetermineFileActions(
 			fileContents[fileName] = fileContent
 			fileDiff[modifiedFile.File.GetFileMeta().GetName()] = modifiedFile
 		}
-
-		// If the file is unreferenced we check if the file has been updated since the last time
-		// Also check if the unreferenced file still exists
-		if manifestFiles[modifiedFile.File.GetFileMeta().GetName()] != nil &&
-			!manifestFiles[modifiedFile.File.GetFileMeta().GetName()].ManifestFileMeta.Referenced {
-			if fileStats, err := os.Stat(modifiedFile.File.GetFileMeta().GetName()); errors.Is(err, os.ErrNotExist) {
-				modifiedFile.Action = model.Add
-				fileDiff[modifiedFile.File.GetFileMeta().GetName()] = modifiedFile
-			} else if timestamppb.New(fileStats.ModTime()) != modifiedFile.File.GetFileMeta().GetModifiedTime() {
-				fileContent, readErr := os.ReadFile(fileName)
-				if readErr != nil {
-					return nil, nil, fmt.Errorf("error reading file %s, error: %w", fileName, readErr)
-				}
-				modifiedFile.Action = model.Update
-				fileContents[fileName] = fileContent
-				fileDiff[modifiedFile.File.GetFileMeta().GetName()] = modifiedFile
-			}
-		}
 	}
 
 	return fileDiff, fileContents, nil

internal/file/file_manager_service_test.go

@@ -563,14 +563,21 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 	updateErr := os.WriteFile(updateTestFile.Name(), updatedFileContent, 0o600)
 	require.NoError(t, updateErr)
 
-	addTestFileName := tempDir + "/nginx_add.conf"
+	addTestFileName := tempDir + "nginx_add.conf"
 
 	unmanagedFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx_unmanaged.conf")
 	defer helpers.RemoveFileWithErrorCheck(t, unmanagedFile.Name())
 	unmanagedFileContent := []byte("test unmanaged file")
 	unmanagedErr := os.WriteFile(unmanagedFile.Name(), unmanagedFileContent, 0o600)
 	require.NoError(t, unmanagedErr)
 
+	addTestFile := helpers.CreateFileWithErrorCheck(t, tempDir, "nginx_add.conf")
+	defer helpers.RemoveFileWithErrorCheck(t, addTestFile.Name())
+	t.Logf("Adding file: %s", addTestFile.Name())
+	addFileContent := []byte("test add file")
+	addErr := os.WriteFile(addTestFile.Name(), addFileContent, 0o600)
+	require.NoError(t, addErr)
+
 	tests := []struct {
 		expectedError   error
 		modifiedFiles   map[string]*model.FileCache
@@ -645,9 +652,9 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 		{
 			name: "Test 2: Files same as on disk",
 			modifiedFiles: map[string]*model.FileCache{
-				addTestFileName: {
+				addTestFile.Name(): {
 					File: &mpi.File{
-						FileMeta: protos.FileMeta(addTestFileName, files.GenerateHash(fileContent)),
+						FileMeta: protos.FileMeta(addTestFile.Name(), files.GenerateHash(fileContent)),
 					},
 				},
 				updateTestFile.Name(): {
@@ -668,8 +675,8 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 				updateTestFile.Name(): {
 					FileMeta: protos.FileMeta(updateTestFile.Name(), files.GenerateHash(fileContent)),
 				},
-				addTestFileName: {
-					FileMeta: protos.FileMeta(addTestFileName, files.GenerateHash(fileContent)),
+				addTestFile.Name(): {
+					FileMeta: protos.FileMeta(addTestFile.Name(), files.GenerateHash(fileContent)),
 				},
 			},
 			expectedCache:   make(map[string]*model.FileCache),
@@ -685,10 +692,11 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
 			defer manifestFile.Close()
 			manifestDirPath = tempDir
 			manifestFilePath = manifestFile.Name()
-			t.Logf("path: %s", manifestFilePath)
+
 			fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
 			fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig())
 			require.NoError(tt, err)
+
 			diff, contents, fileActionErr := fileManagerService.DetermineFileActions(test.currentFiles,
 				test.modifiedFiles)
 			require.NoError(tt, fileActionErr)

internal/watcher/watcher_plugin.go

@@ -198,11 +198,9 @@ func (w *Watcher) handleConfigApplySuccess(ctx context.Context, msg *bus.Message
 
 	// If the config apply had no changes to any files, it is results in a ConfigApplySuccessfulTopic with an empty
 	// configContext being sent, there is no need to reparse the config as no change has occurred.
-	if successMessage.ConfigContext.InstanceID == "" {
-		slog.DebugContext(ctx, "NginxConfigContext is empty, no need to reparse config")
-		return
+	if successMessage.ConfigContext.InstanceID != "" {
+		w.instanceWatcherService.HandleNginxConfigContextUpdate(ctx, instanceID, successMessage.ConfigContext)
 	}
-	w.instanceWatcherService.HandleNginxConfigContextUpdate(ctx, instanceID, successMessage.ConfigContext)
 
 	w.watcherMutex.Lock()
 	w.instancesWithConfigApplyInProgress = slices.DeleteFunc(
@@ -213,8 +211,8 @@ func (w *Watcher) handleConfigApplySuccess(ctx context.Context, msg *bus.Message
 	)
 
 	w.fileWatcherService.SetEnabled(true)
-	w.watcherMutex.Unlock()
 	w.instanceWatcherService.SetEnabled(true)
+	w.watcherMutex.Unlock()
 }
 
 func (w *Watcher) handleHealthRequest(ctx context.Context) {