Skip to content

Commit 05a5394

Browse files
dhurleydhurley
committed
Move temp files instead of renaming them
1 parent bc8b1c2 commit 05a5394

File tree

9 files changed

+235
-33
lines changed

9 files changed

+235
-33
lines changed

‎Makefile‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ BINARY_NAME := nginx-agent
4545
PROJECT_DIR = cmd/agent
4646
PROJECT_FILE = main.go
4747
COLLECTOR_PATH ?= /etc/nginx-agent/opentelemetry-collector-agent.yaml
48-
MANIFEST_DIR ?= /var/lib/nginx-agent
48+
LIB_DIR ?= /var/lib/nginx-agent
4949
DIRS = $(BUILD_DIR) $(TEST_BUILD_DIR) $(BUILD_DIR)/$(DOCS_DIR) $(BUILD_DIR)/$(DOCS_DIR)/$(PROTO_DIR)
5050
$(shell mkdir -p $(DIRS))
5151

@@ -195,7 +195,7 @@ run: build ## Run code
195195

196196
dev: ## Run agent executable
197197
@echo "🚀 Running App"
198-
NGINX_AGENT_COLLECTOR_CONFIG_PATH=$(COLLECTOR_PATH) NGINX_AGENT_MANIFEST_DIR=$(MANIFEST_DIR) $(GORUN) -ldflags=$(DEBUG_LDFLAGS) $(PROJECT_DIR)/$(PROJECT_FILE)
198+
NGINX_AGENT_COLLECTOR_CONFIG_PATH=$(COLLECTOR_PATH) NGINX_AGENT_LIB_DIR=$(LIB_DIR) $(GORUN) -ldflags=$(DEBUG_LDFLAGS) $(PROJECT_DIR)/$(PROJECT_FILE)
199199

200200
race-condition-dev: ## Run agent executable with race condition detection
201201
@echo "🏎️ Running app with race condition detection enabled"

‎internal/config/config.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func ResolveConfig() (*Config, error) {
121121
Watchers: resolveWatchers(),
122122
Features: viperInstance.GetStringSlice(FeaturesKey),
123123
Labels: resolveLabels(),
124-
ManifestDir: viperInstance.GetString(ManifestDirPathKey),
124+
LibDir: viperInstance.GetString(LibDirPathKey),
125125
}
126126

127127
defaultCollector(collector, config)
@@ -380,9 +380,9 @@ func registerFlags() {
380380
"If the default path doesn't exist, log messages are output to stdout/stderr.",
381381
)
382382
fs.String(
383-
ManifestDirPathKey,
384-
DefManifestDir,
385-
"Specifies the path to the directory containing the manifest files",
383+
LibDirPathKey,
384+
DefLibDir,
385+
"Specifies the path to the nginx-agent lib directory",
386386
)
387387

388388
fs.StringSlice(AllowedDirectoriesKey,

‎internal/config/defaults.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ const (
110110
DefCollectorExtensionsHealthTLServerNameKey = ""
111111

112112
// File defaults
113-
DefManifestDir = "/var/lib/nginx-agent"
113+
DefLibDir = "/var/lib/nginx-agent"
114114
)
115115

116116
func DefaultFeatures() []string {

‎internal/config/flags.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const (
2424
InstanceWatcherMonitoringFrequencyKey = "watchers_instance_watcher_monitoring_frequency"
2525
InstanceHealthWatcherMonitoringFrequencyKey = "watchers_instance_health_watcher_monitoring_frequency"
2626
FileWatcherKey = "watchers_file_watcher"
27-
ManifestDirPathKey = "manifest_dir"
27+
LibDirPathKey = "lib_dir"
2828
)
2929

3030
var (

‎internal/config/types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type (
4848
Version string `yaml:"-"`
4949
Path string `yaml:"-"`
5050
UUID string `yaml:"-"`
51-
ManifestDir string `yaml:"-"`
51+
LibDir string `yaml:"-"`
5252
AllowedDirectories []string `yaml:"allowed_directories" mapstructure:"allowed_directories"`
5353
Features []string `yaml:"features" mapstructure:"features"`
5454
}

‎internal/file/file_manager_service.go‎

Lines changed: 102 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ import (
1111
"encoding/json"
1212
"errors"
1313
"fmt"
14+
"io"
1415
"log/slog"
1516
"os"
17+
"path"
1618
"path/filepath"
1719
"sync"
1820

@@ -117,7 +119,7 @@ func NewFileManagerService(fileServiceClient mpi.FileServiceClient, agentConfig
117119
rollbackFileContents: make(map[string][]byte),
118120
currentFilesOnDisk: make(map[string]*mpi.File),
119121
previousManifestFiles: make(map[string]*model.ManifestFile),
120-
manifestFilePath: agentConfig.ManifestDir + "/manifest.json",
122+
manifestFilePath: agentConfig.LibDir + "/manifest.json",
121123
manifestLock: manifestLock,
122124
}
123125
}
@@ -161,7 +163,12 @@ func (fms *FileManagerService) ConfigApply(ctx context.Context,
161163
fms.rollbackFileContents = fileContent
162164
fms.fileActions = diffFiles
163165

164-
fileErr := fms.executeFileActions(ctx)
166+
tempDir, tempDirError := fms.createTempConfigDirectory(ctx)
167+
if tempDirError != nil {
168+
return model.Error, tempDirError
169+
}
170+
171+
fileErr := fms.executeFileActions(ctx, tempDir)
165172
if fileErr != nil {
166173
return model.RollbackRequired, fileErr
167174
}
@@ -216,7 +223,7 @@ func (fms *FileManagerService) Rollback(ctx context.Context, instanceID string)
216223
}
217224

218225
manifestFileErr := fms.fileOperator.WriteManifestFile(fms.previousManifestFiles,
219-
fms.agentConfig.ManifestDir, fms.manifestFilePath)
226+
fms.agentConfig.LibDir, fms.manifestFilePath)
220227
if manifestFileErr != nil {
221228
return manifestFileErr
222229
}
@@ -419,7 +426,7 @@ func (fms *FileManagerService) UpdateManifestFile(currentFiles map[string]*mpi.F
419426
updatedFiles = manifestFiles
420427
}
421428

422-
return fms.fileOperator.WriteManifestFile(updatedFiles, fms.agentConfig.ManifestDir, fms.manifestFilePath)
429+
return fms.fileOperator.WriteManifestFile(updatedFiles, fms.agentConfig.LibDir, fms.manifestFilePath)
423430
}
424431

425432
func (fms *FileManagerService) manifestFile() (map[string]*model.ManifestFile, map[string]*mpi.File, error) {
@@ -450,9 +457,7 @@ func (fms *FileManagerService) manifestFile() (map[string]*model.ManifestFile, m
450457
return manifestFiles, fileMap, nil
451458
}
452459

453-
func (fms *FileManagerService) executeFileActions(ctx context.Context) error {
454-
tempDir := os.TempDir()
455-
460+
func (fms *FileManagerService) executeFileActions(ctx context.Context, tempDir string) (actionError error) {
456461
// Download files to temporary location
457462
downloadError := fms.downloadUpdatedFilesToTempLocation(ctx, tempDir)
458463
if downloadError != nil {
@@ -465,22 +470,31 @@ func (fms *FileManagerService) executeFileActions(ctx context.Context) error {
465470
case model.Delete:
466471
slog.DebugContext(ctx, "Deleting file", "file", fileAction.File.GetFileMeta().GetName())
467472
if err := os.Remove(fileAction.File.GetFileMeta().GetName()); err != nil && !os.IsNotExist(err) {
468-
return fmt.Errorf("error deleting file: %s error: %w",
473+
actionError = fmt.Errorf("error deleting file: %s error: %w",
469474
fileAction.File.GetFileMeta().GetName(), err)
475+
476+
break
470477
}
471478

472479
continue
473480
case model.Add, model.Update:
474481
err := fms.moveFilesFromTempDirectory(ctx, fileAction, tempDir)
475482
if err != nil {
476-
return err
483+
actionError = err
484+
485+
break
477486
}
478487
case model.Unchanged:
479488
slog.DebugContext(ctx, "File unchanged")
480489
}
481490
}
482491

483-
return nil
492+
// Remove temp files if there is a failure moving or deleting files
493+
if actionError != nil {
494+
fms.deleteTempFiles(ctx, tempDir)
495+
}
496+
497+
return actionError
484498
}
485499

486500
func (fms *FileManagerService) moveFilesFromTempDirectory(
@@ -495,9 +509,9 @@ func (fms *FileManagerService) moveFilesFromTempDirectory(
495509
return fmt.Errorf("failed to create directories for %s: %w", fileName, err)
496510
}
497511

498-
moveErr := os.Rename(tempFilePath, fileName)
512+
moveErr := moveFile(ctx, tempFilePath, fileName)
499513
if moveErr != nil {
500-
return fmt.Errorf("failed to rename file: %w", moveErr)
514+
return fmt.Errorf("failed to move file: %w", moveErr)
501515
}
502516

503517
if removeError := os.Remove(tempFilePath); removeError != nil && !os.IsNotExist(removeError) {
@@ -512,7 +526,24 @@ func (fms *FileManagerService) moveFilesFromTempDirectory(
512526
return fms.fileServiceOperator.ValidateFileHash(fileName, fileAction.File.GetFileMeta().GetHash())
513527
}
514528

515-
func (fms *FileManagerService) downloadUpdatedFilesToTempLocation(ctx context.Context, tempDir string) error {
529+
func (fms *FileManagerService) deleteTempFiles(ctx context.Context, tempDir string) {
530+
for _, fileAction := range fms.fileActions {
531+
if fileAction.Action == model.Add || fileAction.Action == model.Update {
532+
tempFile := path.Join(tempDir, fileAction.File.GetFileMeta().GetName())
533+
if err := os.Remove(tempFile); err != nil && !os.IsNotExist(err) {
534+
slog.ErrorContext(
535+
ctx, "Error deleting temp file",
536+
"file", fileAction.File.GetFileMeta().GetName(),
537+
"error", err,
538+
)
539+
}
540+
}
541+
}
542+
}
543+
544+
func (fms *FileManagerService) downloadUpdatedFilesToTempLocation(
545+
ctx context.Context, tempDir string,
546+
) (updateError error) {
516547
for _, fileAction := range fms.fileActions {
517548
if fileAction.Action == model.Add || fileAction.Action == model.Update {
518549
tempFilePath := filepath.Join(tempDir, fileAction.File.GetFileMeta().GetName())
@@ -525,12 +556,18 @@ func (fms *FileManagerService) downloadUpdatedFilesToTempLocation(ctx context.Co
525556

526557
updateErr := fms.fileUpdate(ctx, fileAction.File, tempFilePath)
527558
if updateErr != nil {
528-
return updateErr
559+
updateError = updateErr
560+
break
529561
}
530562
}
531563
}
532564

533-
return nil
565+
// Remove temp files if there is an error downloading any files
566+
if updateError != nil {
567+
fms.deleteTempFiles(ctx, tempDir)
568+
}
569+
570+
return updateError
534571
}
535572

536573
func (fms *FileManagerService) fileUpdate(ctx context.Context, file *mpi.File, tempFilePath string) error {
@@ -602,6 +639,21 @@ func (fms *FileManagerService) convertToFile(manifestFile *model.ManifestFile) *
602639
}
603640
}
604641

642+
func (fms *FileManagerService) createTempConfigDirectory(ctx context.Context) (string, error) {
643+
tempDir, tempDirError := os.MkdirTemp(fms.agentConfig.LibDir, "config")
644+
if tempDirError != nil {
645+
return "", fmt.Errorf("failed creating temp config directory: %w", tempDirError)
646+
}
647+
defer func(path string) {
648+
err := os.RemoveAll(path)
649+
if err != nil {
650+
slog.ErrorContext(ctx, "error removing temp config directory", "path", path, "err", err)
651+
}
652+
}(tempDir)
653+
654+
return tempDir, nil
655+
}
656+
605657
// ConvertToMapOfFiles converts a list of files to a map of file caches (file and action) with the file name as the key
606658
func ConvertToMapOfFileCache(convertFiles []*mpi.File) map[string]*model.FileCache {
607659
filesMap := make(map[string]*model.FileCache)
@@ -613,3 +665,38 @@ func ConvertToMapOfFileCache(convertFiles []*mpi.File) map[string]*model.FileCac
613665

614666
return filesMap
615667
}
668+
669+
func moveFile(ctx context.Context, sourcePath, destPath string) error {
670+
inputFile, err := os.Open(sourcePath)
671+
if err != nil {
672+
return err
673+
}
674+
675+
outputFile, err := os.Create(destPath)
676+
if err != nil {
677+
return err
678+
}
679+
defer closeFile(ctx, outputFile)
680+
681+
_, err = io.Copy(outputFile, inputFile)
682+
if err != nil {
683+
closeFile(ctx, inputFile)
684+
return err
685+
}
686+
687+
closeFile(ctx, inputFile)
688+
689+
err = os.Remove(sourcePath)
690+
if err != nil {
691+
return err
692+
}
693+
694+
return nil
695+
}
696+
697+
func closeFile(ctx context.Context, file *os.File) {
698+
err := file.Close()
699+
if err != nil {
700+
slog.ErrorContext(ctx, "Error closing file", "error", err, "file", file.Name())
701+
}
702+
}

0 commit comments

Comments
 (0)