Skip to content

Commit 3c93362

Browse files
aphralGaphralG
committed
WIP to rollback using temp files
1 parent 8fc60c6 commit 3c93362

File tree

9 files changed

+197
-110
lines changed

9 files changed

+197
-110
lines changed

‎internal/file/file_manager_service.go‎

Lines changed: 104 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ type (
7171
fileToUpdate *mpi.File,
7272
) error
7373
SetIsConnected(isConnected bool)
74-
MoveFilesFromTempDirectory(ctx context.Context, fileAction *model.FileCache, tempDir string) error
74+
MoveFileFromTempDirectory(ctx context.Context, fileAction *model.FileCache, tempDir string) error
7575
UpdateClient(ctx context.Context, fileServiceClient mpi.FileServiceClient)
7676
}
7777

@@ -80,14 +80,15 @@ type (
8080
err error)
8181
Rollback(ctx context.Context, instanceID string) error
8282
ClearCache()
83+
SetConfigPath(configPath string)
8384
ConfigUpload(ctx context.Context, configUploadRequest *mpi.ConfigUploadRequest) error
8485
ConfigUpdate(ctx context.Context, nginxConfigContext *model.NginxConfigContext)
8586
UpdateCurrentFilesOnDisk(ctx context.Context, updateFiles map[string]*mpi.File, referenced bool) error
8687
DetermineFileActions(
8788
ctx context.Context,
8889
currentFiles map[string]*mpi.File,
8990
modifiedFiles map[string]*model.FileCache,
90-
) (map[string]*model.FileCache, map[string][]byte, error)
91+
) (map[string]*model.FileCache, error)
9192
IsConnected() bool
9293
SetIsConnected(isConnected bool)
9394
ResetClient(ctx context.Context, fileServiceClient mpi.FileServiceClient)
@@ -107,6 +108,9 @@ type FileManagerService struct {
107108
currentFilesOnDisk map[string]*mpi.File // key is file path
108109
previousManifestFiles map[string]*model.ManifestFile
109110
manifestFilePath string
111+
tempConfigDir string
112+
tempRollbackDir string
113+
configPath string
110114
rollbackManifest bool
111115
filesMutex sync.RWMutex
112116
}
@@ -124,10 +128,15 @@ func NewFileManagerService(fileServiceClient mpi.FileServiceClient, agentConfig
124128
previousManifestFiles: make(map[string]*model.ManifestFile),
125129
rollbackManifest: true,
126130
manifestFilePath: agentConfig.LibDir + "/manifest.json",
131+
configPath: "/etc/nginx/",
127132
manifestLock: manifestLock,
128133
}
129134
}
130135

136+
func (fms *FileManagerService) SetConfigPath(configPath string) {
137+
fms.configPath = filepath.Dir(configPath)
138+
}
139+
131140
func (fms *FileManagerService) ResetClient(ctx context.Context, fileServiceClient mpi.FileServiceClient) {
132141
fms.fileServiceOperator.UpdateClient(ctx, fileServiceClient)
133142
slog.DebugContext(ctx, "File manager service reset client successfully")
@@ -144,6 +153,9 @@ func (fms *FileManagerService) SetIsConnected(isConnected bool) {
144153
func (fms *FileManagerService) ConfigApply(ctx context.Context,
145154
configApplyRequest *mpi.ConfigApplyRequest,
146155
) (status model.WriteStatus, err error) {
156+
var configTempErr error
157+
var rollbackTempErr error
158+
147159
fms.rollbackManifest = true
148160
fileOverview := configApplyRequest.GetOverview()
149161

@@ -156,7 +168,7 @@ func (fms *FileManagerService) ConfigApply(ctx context.Context,
156168
return model.Error, allowedErr
157169
}
158170

159-
diffFiles, fileContent, compareErr := fms.DetermineFileActions(
171+
diffFiles, compareErr := fms.DetermineFileActions(
160172
ctx,
161173
fms.currentFilesOnDisk,
162174
ConvertToMapOfFileCache(fileOverview.GetFiles()),
@@ -170,15 +182,25 @@ func (fms *FileManagerService) ConfigApply(ctx context.Context,
170182
return model.NoChange, nil
171183
}
172184

173-
fms.rollbackFileContents = fileContent
185+
// fms.rollbackFileContents = fileContent
174186
fms.fileActions = diffFiles
175187

176-
tempDir, tempDirError := fms.createTempConfigDirectory(ctx)
177-
if tempDirError != nil {
178-
return model.Error, tempDirError
188+
fms.tempConfigDir, configTempErr = fms.createTempConfigDirectory(ctx, "config")
189+
if configTempErr != nil {
190+
return model.Error, configTempErr
191+
}
192+
193+
fms.tempRollbackDir, rollbackTempErr = fms.createTempConfigDirectory(ctx, "rollback")
194+
if rollbackTempErr != nil {
195+
return model.Error, rollbackTempErr
179196
}
180197

181-
fileErr := fms.executeFileActions(ctx, tempDir)
198+
rollbackTempFilesErr := fms.RollbackTempFiles(ctx)
199+
if rollbackTempFilesErr != nil {
200+
return model.Error, rollbackTempFilesErr
201+
}
202+
203+
fileErr := fms.executeFileActions(ctx)
182204
if fileErr != nil {
183205
fms.rollbackManifest = false
184206
return model.RollbackRequired, fileErr
@@ -193,13 +215,54 @@ func (fms *FileManagerService) ConfigApply(ctx context.Context,
193215
return model.OK, nil
194216
}
195217

218+
func (fms *FileManagerService) RollbackTempFiles(ctx context.Context) error {
219+
for _, file := range fms.fileActions {
220+
if file.Action == model.Add || file.Action == model.Unchanged {
221+
continue
222+
}
223+
224+
filePath := file.File.GetFileMeta().GetName()
225+
226+
if _, err := os.Stat(filePath); os.IsNotExist(err) {
227+
slog.DebugContext(ctx, "Unable to backup file content since file does not exist",
228+
"file", filePath)
229+
230+
continue
231+
}
232+
233+
tempFilePath := filepath.Join(fms.tempRollbackDir, filePath)
234+
slog.DebugContext(ctx, "Attempting to backup file content since file exists", "temp_path", tempFilePath)
235+
236+
moveErr := fms.fileOperator.MoveFile(ctx, filePath, tempFilePath)
237+
238+
if moveErr != nil {
239+
return moveErr
240+
}
241+
}
242+
243+
return nil
244+
}
245+
196246
func (fms *FileManagerService) ClearCache() {
247+
slog.Debug("Clearing cache and temp files after config apply")
197248
clear(fms.rollbackFileContents)
198249
clear(fms.fileActions)
199250
clear(fms.previousManifestFiles)
251+
252+
configErr := os.RemoveAll(fms.tempConfigDir)
253+
if configErr != nil {
254+
slog.Error("error removing temp config directory", "path", fms.tempConfigDir, "err", configErr)
255+
}
256+
257+
rollbackErr := os.RemoveAll(fms.tempRollbackDir)
258+
if rollbackErr != nil {
259+
slog.Error("error removing temp rollback directory", "path", fms.tempRollbackDir, "err", rollbackErr)
260+
}
261+
262+
slog.Info("Cleaned up temp files")
200263
}
201264

202-
//nolint:revive // cognitive-complexity of 13 max is 12, loop is needed cant be broken up
265+
//nolint:revive,cyclop // cognitive-complexity of 13 max is 12, loop is needed cant be broken up
203266
func (fms *FileManagerService) Rollback(ctx context.Context, instanceID string) error {
204267
slog.InfoContext(ctx, "Rolling back config for instance", "instance_id", instanceID)
205268

@@ -218,10 +281,25 @@ func (fms *FileManagerService) Rollback(ctx context.Context, instanceID string)
218281
continue
219282
case model.Delete, model.Update:
220283
fileMeta := fileAction.File.GetFileMeta()
221-
content := fms.rollbackFileContents[fileMeta.GetName()]
222-
err := fms.fileOperator.Write(ctx, content, fileMeta.GetName(), fileMeta.GetPermissions())
223-
if err != nil {
224-
return err
284+
fileName := fileMeta.GetName()
285+
286+
tempFilePath := filepath.Join(fms.tempRollbackDir, fileName)
287+
288+
// Create parent directories for the target file if they don't exist
289+
if err := os.MkdirAll(filepath.Dir(fileName), dirPerm); err != nil {
290+
return fmt.Errorf("failed to create directories for %s: %w", fileName, err)
291+
}
292+
293+
slog.InfoContext(ctx, "Moving files during rollback")
294+
moveErr := os.Rename(tempFilePath, fileName)
295+
if moveErr != nil {
296+
return fmt.Errorf("failed to rename file, %s to %s: %w", tempFilePath, fileName, moveErr)
297+
}
298+
299+
content, readErr := os.ReadFile(fileMeta.GetName())
300+
if readErr != nil {
301+
return fmt.Errorf("error reading file, unable to generate hash: %s error: %w",
302+
fileMeta.GetName(), readErr)
225303
}
226304

227305
// currentFilesOnDisk needs to be updated after rollback action is performed
@@ -308,20 +386,18 @@ func (fms *FileManagerService) DetermineFileActions(
308386
modifiedFiles map[string]*model.FileCache,
309387
) (
310388
map[string]*model.FileCache,
311-
map[string][]byte,
312389
error,
313390
) {
314391
fms.filesMutex.Lock()
315392
defer fms.filesMutex.Unlock()
316393

317394
fileDiff := make(map[string]*model.FileCache) // Files that have changed, key is file name
318-
fileContents := make(map[string][]byte) // contents of the file, key is file name
319395

320396
_, filesMap, manifestFileErr := fms.manifestFile()
321397

322398
if manifestFileErr != nil {
323399
if !errors.Is(manifestFileErr, os.ErrNotExist) {
324-
return nil, nil, manifestFileErr
400+
return nil, manifestFileErr
325401
}
326402
filesMap = currentFiles
327403
}
@@ -332,18 +408,6 @@ func (fms *FileManagerService) DetermineFileActions(
332408
_, exists := modifiedFiles[fileName]
333409

334410
if !exists {
335-
// Read file contents before marking it deleted
336-
fileContent, readErr := os.ReadFile(fileName)
337-
if readErr != nil {
338-
if errors.Is(readErr, os.ErrNotExist) {
339-
slog.DebugContext(ctx, "Unable to backup file contents since file does not exist", "file", fileName)
340-
continue
341-
}
342-
343-
return nil, nil, fmt.Errorf("error reading file %s: %w", fileName, readErr)
344-
}
345-
fileContents[fileName] = fileContent
346-
347411
fileDiff[fileName] = &model.FileCache{
348412
File: manifestFile,
349413
Action: model.Delete,
@@ -353,7 +417,7 @@ func (fms *FileManagerService) DetermineFileActions(
353417

354418
for _, modifiedFile := range modifiedFiles {
355419
fileName := modifiedFile.File.GetFileMeta().GetName()
356-
currentFile, ok := filesMap[modifiedFile.File.GetFileMeta().GetName()]
420+
currentFile, ok := filesMap[fileName]
357421
// default to unchanged action
358422
modifiedFile.Action = model.Unchanged
359423

@@ -363,25 +427,20 @@ func (fms *FileManagerService) DetermineFileActions(
363427
}
364428
// if file doesn't exist in the current files, file has been added
365429
// set file action
366-
if _, statErr := os.Stat(modifiedFile.File.GetFileMeta().GetName()); errors.Is(statErr, os.ErrNotExist) {
430+
if _, statErr := os.Stat(fileName); errors.Is(statErr, os.ErrNotExist) {
367431
modifiedFile.Action = model.Add
368-
fileDiff[modifiedFile.File.GetFileMeta().GetName()] = modifiedFile
432+
fileDiff[fileName] = modifiedFile
369433

370434
continue
371435
// if file currently exists and file hash is different, file has been updated
372436
// copy contents, set file action
373437
} else if ok && modifiedFile.File.GetFileMeta().GetHash() != currentFile.GetFileMeta().GetHash() {
374-
fileContent, readErr := os.ReadFile(fileName)
375-
if readErr != nil {
376-
return nil, nil, fmt.Errorf("error reading file %s, error: %w", fileName, readErr)
377-
}
378438
modifiedFile.Action = model.Update
379-
fileContents[fileName] = fileContent
380-
fileDiff[modifiedFile.File.GetFileMeta().GetName()] = modifiedFile
439+
fileDiff[fileName] = modifiedFile
381440
}
382441
}
383442

384-
return fileDiff, fileContents, nil
443+
return fileDiff, nil
385444
}
386445

387446
// UpdateCurrentFilesOnDisk updates the FileManagerService currentFilesOnDisk slice which contains the files
@@ -485,17 +544,17 @@ func (fms *FileManagerService) manifestFile() (map[string]*model.ManifestFile, m
485544
return manifestFiles, fileMap, nil
486545
}
487546

488-
func (fms *FileManagerService) executeFileActions(ctx context.Context, tempDir string) (actionError error) {
547+
func (fms *FileManagerService) executeFileActions(ctx context.Context) (actionError error) {
489548
// Download files to temporary location
490-
downloadError := fms.downloadUpdatedFilesToTempLocation(ctx, tempDir)
549+
downloadError := fms.downloadUpdatedFilesToTempLocation(ctx, fms.tempConfigDir)
491550
if downloadError != nil {
492551
return downloadError
493552
}
494553

495554
// Remove temp files if there is a failure moving or deleting files
496-
actionError = fms.moveOrDeleteFiles(ctx, tempDir, actionError)
555+
actionError = fms.moveOrDeleteFiles(ctx, fms.tempConfigDir, actionError)
497556
if actionError != nil {
498-
fms.deleteTempFiles(ctx, tempDir)
557+
fms.deleteTempFiles(ctx, fms.tempConfigDir)
499558
}
500559

501560
return actionError
@@ -522,11 +581,6 @@ func (fms *FileManagerService) downloadUpdatedFilesToTempLocation(
522581
}
523582
}
524583

525-
// Remove temp files if there is an error downloading any files
526-
if updateError != nil {
527-
fms.deleteTempFiles(ctx, tempDir)
528-
}
529-
530584
return updateError
531585
}
532586

@@ -545,7 +599,7 @@ actionsLoop:
545599

546600
continue
547601
case model.Add, model.Update:
548-
err := fms.fileServiceOperator.MoveFilesFromTempDirectory(ctx, fileAction, tempDir)
602+
err := fms.fileServiceOperator.MoveFileFromTempDirectory(ctx, fileAction, tempDir)
549603
if err != nil {
550604
actionError = err
551605

@@ -643,8 +697,8 @@ func (fms *FileManagerService) convertToFile(manifestFile *model.ManifestFile) *
643697
}
644698
}
645699

646-
func (fms *FileManagerService) createTempConfigDirectory(ctx context.Context) (string, error) {
647-
tempDir, tempDirError := os.MkdirTemp(fms.agentConfig.LibDir, "config")
700+
func (fms *FileManagerService) createTempConfigDirectory(ctx context.Context, pattern string) (string, error) {
701+
tempDir, tempDirError := os.MkdirTemp(fms.configPath, pattern)
648702
if tempDirError != nil {
649703
return "", fmt.Errorf("failed creating temp config directory: %w", tempDirError)
650704
}

‎internal/file/file_manager_service_test.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -625,13 +625,12 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
625625

626626
require.NoError(tt, err)
627627

628-
diff, contents, fileActionErr := fileManagerService.DetermineFileActions(
628+
diff, fileActionErr := fileManagerService.DetermineFileActions(
629629
ctx,
630630
test.currentFiles,
631631
test.modifiedFiles,
632632
)
633633
require.NoError(tt, fileActionErr)
634-
assert.Equal(tt, test.expectedContent, contents)
635634
assert.Equal(tt, test.expectedCache, diff)
636635
})
637636
}
@@ -892,7 +891,7 @@ func TestFileManagerService_fileActions(t *testing.T) {
892891

893892
fileManagerService.fileActions = filesCache
894893

895-
actionErr := fileManagerService.executeFileActions(ctx, os.TempDir())
894+
actionErr := fileManagerService.executeFileActions(ctx)
896895
require.NoError(t, actionErr)
897896

898897
assert.FileExists(t, addFilePath)
@@ -1020,14 +1019,14 @@ func TestFileManagerService_createTempConfigDirectory(t *testing.T) {
10201019
agentConfig: agentConfig,
10211020
}
10221021

1023-
dir, err := fileManagerService.createTempConfigDirectory(t.Context())
1022+
dir, err := fileManagerService.createTempConfigDirectory(t.Context(), "config")
10241023
assert.NotEmpty(t, dir)
10251024
require.NoError(t, err)
10261025

10271026
// Test for unknown directory path
10281027
agentConfig.LibDir = "/unknown/"
10291028

1030-
dir, err = fileManagerService.createTempConfigDirectory(t.Context())
1029+
dir, err = fileManagerService.createTempConfigDirectory(t.Context(), "config")
10311030
assert.Empty(t, dir)
10321031
require.Error(t, err)
10331032
}

0 commit comments

Comments
 (0)