From 2054c99b6f8f6af2822badf244dd03a19a38b052 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Sun, 29 Dec 2024 12:33:06 +0200 Subject: [PATCH 01/18] rw mutex --- internal/server/handlers/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/handlers/service.go b/internal/server/handlers/service.go index 3f8df245..9a27918c 100644 --- a/internal/server/handlers/service.go +++ b/internal/server/handlers/service.go @@ -12,7 +12,7 @@ import ( ) type Service struct { - sync.Mutex + sync.RWMutex config *model.Config configApplier service.ConfigApplier scheduler quartz.Scheduler From f8b18955ccd3da9ff12c711330399d87127502fa Mon Sep 17 00:00:00 2001 From: akorotkov Date: Sun, 29 Dec 2024 13:20:38 +0200 Subject: [PATCH 02/18] dont pass config to restore --- cmd/backup/main.go | 2 +- pkg/model/config.go | 65 +++++++++++++++++------------------ pkg/model/restore_request.go | 2 ++ pkg/service/restore_config.go | 1 + pkg/service/restore_data.go | 25 +++++--------- 5 files changed, 44 insertions(+), 51 deletions(-) diff --git a/cmd/backup/main.go b/cmd/backup/main.go index a9f96bee..3ce06e3b 100644 --- a/cmd/backup/main.go +++ b/cmd/backup/main.go @@ -98,7 +98,7 @@ func startService(configFile string, remote bool) error { service.NewMetricsCollector(backupHandlers, restoreJobs).Start(ctx, 1*time.Second) restoreMgr := service.NewRestoreManager( - backends, config, service.NewRestore(), clientManager, restoreJobs, nsValidator) + backends, service.NewRestore(), clientManager, restoreJobs, nsValidator) httpService := handlers.NewService( config, diff --git a/pkg/model/config.go b/pkg/model/config.go index ff475057..b8835ab3 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -7,7 +7,7 @@ import ( // Config represents the service configuration. type Config struct { - mu sync.Mutex + sync.Mutex ServiceConfig BackupServiceConfig AerospikeClusters map[string]*AerospikeCluster Storage map[string]Storage // Storage is an interface @@ -33,9 +33,6 @@ var ( ) func (c *Config) AddStorage(name string, s Storage) error { - c.mu.Lock() - defer c.mu.Unlock() - if _, exists := c.Storage[name]; exists { return fmt.Errorf("add storage %q: %w", name, ErrAlreadyExists) } @@ -44,8 +41,8 @@ func (c *Config) AddStorage(name string, s Storage) error { } func (c *Config) DeleteStorage(name string) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() s, exists := c.Storage[name] if !exists { @@ -59,8 +56,8 @@ func (c *Config) DeleteStorage(name string) error { } func (c *Config) UpdateStorage(name string, s Storage) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.Storage[name]; !exists { return fmt.Errorf("update storage %q: %w", name, ErrNotFound) @@ -88,8 +85,8 @@ func (c *Config) routineUsesStorage(s Storage) string { } func (c *Config) AddPolicy(name string, p *BackupPolicy) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.BackupPolicies[name]; exists { return fmt.Errorf("add backup policy %q: %w", name, ErrAlreadyExists) @@ -99,8 +96,8 @@ func (c *Config) AddPolicy(name string, p *BackupPolicy) error { } func (c *Config) DeletePolicy(name string) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() p, exists := c.BackupPolicies[name] if !exists { @@ -114,8 +111,8 @@ func (c *Config) DeletePolicy(name string) error { } func (c *Config) UpdatePolicy(name string, p *BackupPolicy) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.BackupPolicies[name]; !exists { return fmt.Errorf("update backup policy %q: %w", name, ErrNotFound) @@ -142,8 +139,8 @@ func (c *Config) routineUsesPolicy(p *BackupPolicy) string { } func (c *Config) AddRoutine(name string, r *BackupRoutine) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.BackupRoutines[name]; exists { return fmt.Errorf("add backup routine %q: %w", name, ErrAlreadyExists) @@ -153,8 +150,8 @@ func (c *Config) AddRoutine(name string, r *BackupRoutine) error { } func (c *Config) DeleteRoutine(name string) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.BackupRoutines[name]; !exists { return fmt.Errorf("delete backup routine %q: %w", name, ErrNotFound) @@ -164,8 +161,8 @@ func (c *Config) DeleteRoutine(name string) error { } func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.BackupRoutines[name]; !exists { return fmt.Errorf("update backup routine %q: %w", name, ErrNotFound) @@ -175,8 +172,8 @@ func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { } func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.AerospikeClusters[name]; exists { return fmt.Errorf("add Aerospike cluster %q: %w", name, ErrAlreadyExists) @@ -186,8 +183,8 @@ func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { } func (c *Config) DeleteCluster(name string) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() cluster, exists := c.AerospikeClusters[name] if !exists { @@ -201,8 +198,8 @@ func (c *Config) DeleteCluster(name string) error { } func (c *Config) UpdateCluster(name string, cluster *AerospikeCluster) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.AerospikeClusters[name]; !exists { return fmt.Errorf("update Aerospike cluster %q: %w", name, ErrNotFound) @@ -230,8 +227,8 @@ func (c *Config) routineUsesCluster(cluster *AerospikeCluster) string { } func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() if _, exists := c.SecretAgents[name]; exists { return fmt.Errorf("add Secret agent %q: %w", name, ErrAlreadyExists) @@ -241,10 +238,10 @@ func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { } func (c *Config) CopyFrom(other *Config) { - c.mu.Lock() - other.mu.Lock() - defer c.mu.Unlock() - defer other.mu.Unlock() + c.Lock() + other.Lock() + defer c.Unlock() + defer other.Unlock() c.AerospikeClusters = other.AerospikeClusters c.Storage = other.Storage @@ -268,8 +265,8 @@ func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*S // ToggleRoutineDisabled sets the Disabled field of the BackupRoutine based on the provided state. func (c *Config) ToggleRoutineDisabled(name string, isDisabled bool) error { - c.mu.Lock() - defer c.mu.Unlock() + c.Lock() + defer c.Unlock() _, exists := c.BackupRoutines[name] if !exists { diff --git a/pkg/model/restore_request.go b/pkg/model/restore_request.go index 8ba23237..e29c4aef 100644 --- a/pkg/model/restore_request.go +++ b/pkg/model/restore_request.go @@ -57,11 +57,13 @@ func NewRestoreRequest( policy *RestorePolicy, sourceStorage Storage, secretAgent *SecretAgent, + backupDataPath string, ) *RestoreRequest { return &RestoreRequest{ DestinationCluster: destinationCluster, Policy: policy, SourceStorage: sourceStorage, SecretAgent: secretAgent, + BackupDataPath: backupDataPath, } } diff --git a/pkg/service/restore_config.go b/pkg/service/restore_config.go index 4d62226d..2b59582a 100644 --- a/pkg/service/restore_config.go +++ b/pkg/service/restore_config.go @@ -8,6 +8,7 @@ import ( "github.com/aerospike/aerospike-backup-service/v3/pkg/util" ) +// configRetriever is used to read Aerospike configuration from backup. type configRetriever struct { backends BackendsHolder } diff --git a/pkg/service/restore_data.go b/pkg/service/restore_data.go index 017bf273..a333e7df 100644 --- a/pkg/service/restore_data.go +++ b/pkg/service/restore_data.go @@ -33,7 +33,6 @@ func NewErrJobNotFound(id model.RestoreJobID) *ErrJobNotFound { // Stores job information locally within a map. type dataRestorer struct { configRetriever - config *model.Config restoreJobs *RestoreJobsHolder restoreService Restore backends BackendsHolder @@ -45,7 +44,6 @@ var _ RestoreManager = (*dataRestorer)(nil) // NewRestoreManager returns a new dataRestorer instance. func NewRestoreManager(backends BackendsHolder, - config *model.Config, restoreService Restore, clientManager aerospike.ClientManager, restoreJobs *RestoreJobsHolder, @@ -58,7 +56,6 @@ func NewRestoreManager(backends BackendsHolder, restoreJobs: restoreJobs, restoreService: restoreService, backends: backends, - config: config, clientManager: clientManager, nsValidator: nsValidator, } @@ -205,7 +202,7 @@ func (r *dataRestorer) restoreNamespace( continue } - handler, err := r.restoreFromPath(ctx, client, request, b.Key) + handler, err := r.restoreFromPath(ctx, client, request, b.Key, fullBackup.Storage) if err != nil { return err } @@ -231,9 +228,15 @@ func (r *dataRestorer) restoreFromPath( client *backup.Client, request *model.RestoreTimestampRequest, backupPath string, + storage model.Storage, ) (RestoreHandler, error) { - restoreRequest := r.toRestoreRequest(request) - restoreRequest.BackupDataPath = backupPath + restoreRequest := model.NewRestoreRequest( + request.DestinationCluster, + request.Policy, + storage, + request.SecretAgent, + backupPath, + ) handler, err := r.restoreService.Run(ctx, client, restoreRequest) if err != nil { return nil, fmt.Errorf("could not start restore from backup at %s: %w", backupPath, err) @@ -242,16 +245,6 @@ func (r *dataRestorer) restoreFromPath( return handler, nil } -func (r *dataRestorer) toRestoreRequest(request *model.RestoreTimestampRequest) *model.RestoreRequest { - routine := r.config.BackupRoutines[request.RoutineName] - return model.NewRestoreRequest( - request.DestinationCluster, - request.Policy, - routine.Storage, - request.SecretAgent, - ) -} - // JobStatus returns the status of the job with the given id. func (r *dataRestorer) JobStatus(jobID model.RestoreJobID) (*model.RestoreJobStatus, error) { return r.restoreJobs.getStatus(jobID) From 177bfa441b98600a72c5a2e6800260a3d589243d Mon Sep 17 00:00:00 2001 From: akorotkov Date: Sun, 29 Dec 2024 13:23:43 +0200 Subject: [PATCH 03/18] update tests --- pkg/service/restore_data_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/pkg/service/restore_data_test.go b/pkg/service/restore_data_test.go index fe81b6e7..31e780d4 100644 --- a/pkg/service/restore_data_test.go +++ b/pkg/service/restore_data_test.go @@ -41,18 +41,6 @@ func (b *BackendHolderMock) Init(_ *model.Config) { } func makeTestRestoreService(wg *sync.WaitGroup) *dataRestorer { - storage := &model.LocalStorage{} - config := model.NewConfig() - _ = config.AddStorage("s", storage) - config.BackupRoutines = map[string]*model.BackupRoutine{ - "routine": { - Storage: storage, - }, - "routine_fail_restore": { - Storage: storage, - }, - } - return &dataRestorer{ configRetriever: configRetriever{ backends: &BackendHolderMock{}, @@ -60,7 +48,6 @@ func makeTestRestoreService(wg *sync.WaitGroup) *dataRestorer { restoreJobs: NewRestoreJobsHolder(), restoreService: NewRestoreMock(wg), backends: &BackendHolderMock{}, - config: config, clientManager: &MockClientManager{}, } } From d7511ea324f6d2064c8a9af4f6166bfb47c5a12c Mon Sep 17 00:00:00 2001 From: akorotkov Date: Sun, 29 Dec 2024 13:45:48 +0200 Subject: [PATCH 04/18] dont' pass config to makeHandler --- cmd/backup/main.go | 6 +++--- pkg/service/backup_routine_handler.go | 13 ++++++------- pkg/service/config_applier.go | 8 ++++---- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/cmd/backup/main.go b/cmd/backup/main.go index 3ce06e3b..60cec2c5 100644 --- a/cmd/backup/main.go +++ b/cmd/backup/main.go @@ -73,7 +73,7 @@ func startService(configFile string, remote bool) error { return fmt.Errorf("failed to load configuration: %w", err) } - appLogger := setDefaultLoggers(ctx, config) + appLogger := setDefaultLoggers(ctx, config.ServiceConfig.GetLoggerOrDefault()) slog.Info("Aerospike Backup Service", "commit", commit, "buildTime", buildTime) // schedule all configured backups @@ -121,9 +121,9 @@ func startService(configFile string, remote bool) error { return err } -func setDefaultLoggers(ctx context.Context, config *model.Config) *slog.Logger { +func setDefaultLoggers(ctx context.Context, loggerConfig *model.LoggerConfig) *slog.Logger { appLogger := slog.New( - util.LogHandler(config.ServiceConfig.GetLoggerOrDefault()), + util.LogHandler(loggerConfig), ) slog.SetDefault(appLogger) logger.SetDefault(util.NewQuartzLogger(ctx)) diff --git a/pkg/service/backup_routine_handler.go b/pkg/service/backup_routine_handler.go index fc27d168..73f606a9 100644 --- a/pkg/service/backup_routine_handler.go +++ b/pkg/service/backup_routine_handler.go @@ -97,28 +97,27 @@ type BackupHandlerHolder map[string]backupRunner // newBackupRoutineHandler returns a new BackupRoutineHandler instance. func newBackupRoutineHandler( - config *model.Config, clientManager aerospike.ClientManager, backupService Backup, routineName string, + routine *model.BackupRoutine, backupBackend *BackupBackend, lastRun *model.LastBackupRun, ) *BackupRoutineHandler { - backupRoutine := config.BackupRoutines[routineName] - backupPolicy := backupRoutine.BackupPolicy - backupStorage := backupRoutine.Storage + backupPolicy := routine.BackupPolicy + backupStorage := routine.Storage logger := slog.Default().With(slog.String("routine", routineName)) return &BackupRoutineHandler{ backupService: backupService, metadataWriter: backupBackend, - backupRoutine: backupRoutine, + backupRoutine: routine, backupFullPolicy: backupPolicy, backupIncrPolicy: backupPolicy.CopySMDDisabled(), // incremental backups should not contain metadata routineName: routineName, - namespaces: backupRoutine.Namespaces, + namespaces: routine.Namespaces, storage: backupStorage, - secretAgent: backupRoutine.SecretAgent, + secretAgent: routine.SecretAgent, lastRun: lastRun, retry: newRetryExecutor( backupPolicy.GetRetryPolicyOrDefault(), diff --git a/pkg/service/config_applier.go b/pkg/service/config_applier.go index 032cb296..882bca76 100644 --- a/pkg/service/config_applier.go +++ b/pkg/service/config_applier.go @@ -96,11 +96,11 @@ func makeHandlers( var wg sync.WaitGroup var mu sync.Mutex - for routineName := range config.BackupRoutines { + for routineName, routine := range config.BackupRoutines { wg.Add(1) go func() { defer wg.Done() - handler := makeHandler(ctx, clientManager, config, backends, oldHandlers, routineName) + handler := makeHandler(ctx, clientManager, backends, oldHandlers, routineName, routine) mu.Lock() handlers[routineName] = handler mu.Unlock() @@ -114,10 +114,10 @@ func makeHandlers( func makeHandler( ctx context.Context, clientManager aerospike.ClientManager, - config *model.Config, backends BackendsHolder, oldHandlers BackupHandlerHolder, routineName string, + routine *model.BackupRoutine, ) *BackupRoutineHandler { backupService := NewBackupGo() backend, _ := backends.Get(routineName) @@ -130,5 +130,5 @@ func makeHandler( lastRun = backend.findLastRun(ctx) // this scan can take some time. } - return newBackupRoutineHandler(config, clientManager, backupService, routineName, backend, lastRun) + return newBackupRoutineHandler(clientManager, backupService, routineName, routine, backend, lastRun) } From 14d871de24a0c59b45d0e816f70befeb2dea35eb Mon Sep 17 00:00:00 2001 From: akorotkov Date: Sun, 29 Dec 2024 14:53:52 +0200 Subject: [PATCH 05/18] move lock to service --- cmd/backup/main.go | 3 +- internal/server/handlers/backup.go | 15 +++++++ internal/server/handlers/config.go | 28 ++++++------- internal/server/handlers/config_cluster.go | 17 ++++++++ internal/server/handlers/config_policy.go | 19 +++++++++ internal/server/handlers/config_routine.go | 23 +++++++++++ internal/server/handlers/config_storage.go | 19 +++++++++ internal/server/handlers/handlers_test.go | 2 +- internal/server/handlers/restore.go | 9 +++++ internal/server/handlers/service.go | 2 +- pkg/model/config.go | 46 ---------------------- pkg/service/config_applier.go | 17 +++----- 12 files changed, 124 insertions(+), 76 deletions(-) diff --git a/cmd/backup/main.go b/cmd/backup/main.go index 60cec2c5..b794cb6a 100644 --- a/cmd/backup/main.go +++ b/cmd/backup/main.go @@ -83,13 +83,12 @@ func startService(configFile string, remote bool) error { configApplier := service.NewDefaultConfigApplier( scheduler, - config, backends, clientManager, backupHandlers, ) - err = configApplier.ApplyNewConfig(ctx) + err = configApplier.ApplyNewConfig(ctx, config) if err != nil { return fmt.Errorf("failed to apply new config: %w", err) } diff --git a/internal/server/handlers/backup.go b/internal/server/handlers/backup.go index d2a9ccfe..38531416 100644 --- a/internal/server/handlers/backup.go +++ b/internal/server/handlers/backup.go @@ -77,6 +77,9 @@ func (s *Service) GetIncrementalBackupsForRoutine(w http.ResponseWriter, r *http } func (s *Service) readAllBackups(w http.ResponseWriter, r *http.Request, isFullBackup bool) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "readAllBackups")) from := r.URL.Query().Get("from") @@ -125,6 +128,9 @@ func (s *Service) readAllBackups(w http.ResponseWriter, r *http.Request, isFullB //nolint:funlen // Function is long because of logging. func (s *Service) readBackupsForRoutine(w http.ResponseWriter, r *http.Request, isFullBackup bool) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "readBackupsForRoutine")) from := r.URL.Query().Get("from") @@ -231,6 +237,9 @@ func backupsReadFunction( // @Failure 404 {string} string // @Failure 500 {string} string func (s *Service) ScheduleFullBackup(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "ScheduleFullBackup")) routineName := mux.Vars(r)["name"] @@ -293,6 +302,9 @@ func (s *Service) ScheduleFullBackup(w http.ResponseWriter, r *http.Request) { // @Failure 400 {string} string // @Failure 500 {string} string func (s *Service) GetCurrentBackupInfo(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "GetCurrentBackupInfo")) routineName := mux.Vars(r)["name"] @@ -342,6 +354,9 @@ func (s *Service) GetCurrentBackupInfo(w http.ResponseWriter, r *http.Request) { // @Failure 404 {string} string // @Failure 500 {string} string func (s *Service) CancelCurrentBackup(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "CancelCurrentBackup")) routineName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/config.go b/internal/server/handlers/config.go index 3cc0e429..5b084b38 100644 --- a/internal/server/handlers/config.go +++ b/internal/server/handlers/config.go @@ -31,6 +31,9 @@ func (s *Service) ConfigActionHandler(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} dto.Config // @Failure 500 {string} string func (s *Service) readConfig(w http.ResponseWriter) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "readConfig")) configuration, err := dto.Serialize(dto.NewConfigFromModel(s.config), dto.JSON) @@ -60,6 +63,9 @@ func (s *Service) readConfig(w http.ResponseWriter) { // @Success 200 // @Failure 400 {string} string func (s *Service) updateConfig(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "updateConfig")) newConfig, err := dto.NewConfigFromReader(r.Body, dto.JSON) @@ -101,6 +107,9 @@ func (s *Service) updateConfig(w http.ResponseWriter, r *http.Request) { // @Success 200 // @Failure 400 {string} string func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "ApplyConfig")) config, err := s.configurationManager.Read(r.Context()) @@ -112,6 +121,7 @@ func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { return } + // validate static fields. newConfig := dto.NewConfigFromModel(s.config) oldConfig := dto.NewConfigFromModel(config) if err := validation.ValidateStaticFieldChanges(oldConfig, newConfig); err != nil { @@ -123,7 +133,9 @@ func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { return } - err = s.applyConfig(r.Context(), config) + s.config.CopyFrom(config) + err = s.configApplier.ApplyNewConfig(r.Context(), s.config) + if err != nil { hLogger.Error("failed to apply config", slog.Any("error", err), @@ -136,9 +148,6 @@ func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { } func (s *Service) changeConfig(ctx context.Context, updateFunc func(*model.Config) error) error { - s.Lock() - defer s.Unlock() - err := updateFunc(s.config) if err != nil { return fmt.Errorf("cannot update configuration: %w", err) @@ -149,19 +158,10 @@ func (s *Service) changeConfig(ctx context.Context, updateFunc func(*model.Confi return fmt.Errorf("failed to write configuration: %w", err) } - err = s.configApplier.ApplyNewConfig(ctx) + err = s.configApplier.ApplyNewConfig(ctx, s.config) if err != nil { return fmt.Errorf("failed to apply new configuration: %w", err) } return nil } - -func (s *Service) applyConfig(ctx context.Context, c *model.Config) error { - s.Lock() - defer s.Unlock() - - s.config.CopyFrom(c) - - return s.configApplier.ApplyNewConfig(ctx) -} diff --git a/internal/server/handlers/config_cluster.go b/internal/server/handlers/config_cluster.go index da498032..eee01b38 100644 --- a/internal/server/handlers/config_cluster.go +++ b/internal/server/handlers/config_cluster.go @@ -37,6 +37,9 @@ func (s *Service) ConfigClusterActionHandler(w http.ResponseWriter, r *http.Requ // @Failure 400 {string} string // @Failure 500 {string} string func (s *Service) addAerospikeCluster(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "addAerospikeCluster")) newCluster, err := dto.NewClusterFromReader(r.Body, dto.JSON) @@ -87,6 +90,9 @@ func (s *Service) addAerospikeCluster(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} map[string]dto.AerospikeCluster // @Failure 500 {string} string func (s *Service) ReadAerospikeClusters(w http.ResponseWriter, _ *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "ReadAerospikeClusters")) toDTO := dto.ConvertModelMapToDTO(s.config.AerospikeClusters, func(m *model.AerospikeCluster) *dto.AerospikeCluster { @@ -124,6 +130,9 @@ func (s *Service) ReadAerospikeClusters(w http.ResponseWriter, _ *http.Request) // @Failure 404 {string} string "The specified cluster could not be found" // @Failure 500 {string} string "The specified cluster could not be found" func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "readAerospikeCluster")) clusterName := mux.Vars(r)["name"] @@ -171,6 +180,9 @@ func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { // @Success 200 // @Failure 400 {string} string func (s *Service) updateAerospikeCluster(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "updateAerospikeCluster")) updatedCluster, err := dto.NewClusterFromReader(r.Body, dto.JSON) @@ -227,7 +239,12 @@ func (s *Service) updateAerospikeCluster(w http.ResponseWriter, r *http.Request) // @Param name path string true "Aerospike cluster name" // @Success 204 // @Failure 400 {string} string +// +//nolint:dupl func (s *Service) deleteAerospikeCluster(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "deleteAerospikeCluster")) clusterName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/config_policy.go b/internal/server/handlers/config_policy.go index be758b65..4970050e 100644 --- a/internal/server/handlers/config_policy.go +++ b/internal/server/handlers/config_policy.go @@ -38,6 +38,9 @@ func (s *Service) ConfigPolicyActionHandler(w http.ResponseWriter, r *http.Reque // //nolint:dupl func (s *Service) addPolicy(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "addPolicy")) newPolicy, err := dto.NewBackupPolicyFromReader(r.Body, dto.JSON) @@ -78,7 +81,12 @@ func (s *Service) addPolicy(w http.ResponseWriter, r *http.Request) { // @Produce json // @Success 200 {object} map[string]dto.BackupPolicy // @Failure 500 {string} string +// +//nolint:dupl func (s *Service) ReadPolicies(w http.ResponseWriter, _ *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "ReadPolicies")) policies := dto.ConvertModelMapToDTO(s.config.BackupPolicies, dto.NewBackupPolicyFromModel) @@ -113,6 +121,9 @@ func (s *Service) ReadPolicies(w http.ResponseWriter, _ *http.Request) { // @Failure 404 {string} string "The specified policy could not be found" // @Failure 500 {string} string "The specified policy could not be found" func (s *Service) readPolicy(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "readPolicy")) policyName := mux.Vars(r)["name"] @@ -159,6 +170,9 @@ func (s *Service) readPolicy(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) updatePolicy(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "updatePolicy")) updatedPolicy, err := dto.NewBackupPolicyFromReader(r.Body, dto.JSON) @@ -200,7 +214,12 @@ func (s *Service) updatePolicy(w http.ResponseWriter, r *http.Request) { // @Param name path string true "Backup policy name" // @Success 204 // @Failure 400 {string} string +// +//nolint:dupl func (s *Service) deletePolicy(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "deletePolicy")) policyName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/config_routine.go b/internal/server/handlers/config_routine.go index 72e130e5..07f67512 100644 --- a/internal/server/handlers/config_routine.go +++ b/internal/server/handlers/config_routine.go @@ -38,6 +38,9 @@ func (s *Service) ConfigRoutineActionHandler(w http.ResponseWriter, r *http.Requ // //nolint:dupl func (s *Service) addRoutine(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "addRoutine")) newRoutine, err := dto.NewRoutineFromReader(r.Body, dto.JSON) @@ -89,6 +92,9 @@ func (s *Service) addRoutine(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} map[string]dto.BackupRoutine // @Failure 400 {string} string func (s *Service) ReadRoutines(w http.ResponseWriter, _ *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "ReadRoutines")) toDTO := dto.ConvertModelMapToDTO(s.config.BackupRoutines, func(m *model.BackupRoutine) *dto.BackupRoutine { @@ -127,6 +133,9 @@ func (s *Service) ReadRoutines(w http.ResponseWriter, _ *http.Request) { // //nolint:dupl func (s *Service) readRoutine(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "readRoutine")) routineName := mux.Vars(r)["name"] @@ -172,6 +181,9 @@ func (s *Service) readRoutine(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) updateRoutine(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "updateRoutine")) updatedRoutine, err := dto.NewRoutineFromReader(r.Body, dto.JSON) @@ -223,7 +235,12 @@ func (s *Service) updateRoutine(w http.ResponseWriter, r *http.Request) { // @Param name path string true "Backup routine name" // @Success 204 // @Failure 400 {string} string +// +//nolint:dupl func (s *Service) deleteRoutine(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "deleteRoutine")) routineName := mux.Vars(r)["name"] @@ -257,6 +274,9 @@ func (s *Service) deleteRoutine(w http.ResponseWriter, r *http.Request) { // @Failure 404 {string} string // @Router /v1/config/routines/{name}/enable [put] func (s *Service) EnableRoutine(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "enableRoutine")) routineName := mux.Vars(r)["name"] @@ -297,6 +317,9 @@ func (s *Service) EnableRoutine(w http.ResponseWriter, r *http.Request) { // @Failure 500 {string} string "Unexpected error occurred." // @Router /v1/config/routines/{name}/disable [put] func (s *Service) DisableRoutine(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "disableRoutine")) routineName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/config_storage.go b/internal/server/handlers/config_storage.go index 9f055c04..77c75574 100644 --- a/internal/server/handlers/config_storage.go +++ b/internal/server/handlers/config_storage.go @@ -36,6 +36,9 @@ func (s *Service) ConfigStorageActionHandler(w http.ResponseWriter, r *http.Requ // @Success 201 // @Failure 400 {string} string func (s *Service) addStorage(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "addStorage")) newStorage, err := dto.NewStorageFromReader(r.Body, dto.JSON) @@ -79,7 +82,12 @@ func (s *Service) addStorage(w http.ResponseWriter, r *http.Request) { // @Produce json // @Success 200 {object} map[string]dto.Storage // @Failure 500 {string} string +// +//nolint:dupl func (s *Service) ReadAllStorage(w http.ResponseWriter, _ *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "ReadAllStorage")) toDTO := dto.ConvertStorageMapToDTO(s.config.Storage, s.config) @@ -116,6 +124,9 @@ func (s *Service) ReadAllStorage(w http.ResponseWriter, _ *http.Request) { // //nolint:dupl func (s *Service) readStorage(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "readStorage")) storageName := mux.Vars(r)["name"] @@ -160,6 +171,9 @@ func (s *Service) readStorage(w http.ResponseWriter, r *http.Request) { // @Success 200 // @Failure 400 {string} string func (s *Service) updateStorage(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "updateStorage")) updatedStorage, err := dto.NewStorageFromReader(r.Body, dto.JSON) @@ -203,7 +217,12 @@ func (s *Service) updateStorage(w http.ResponseWriter, r *http.Request) { // @Param name path string true "Backup storage name" // @Success 204 // @Failure 400 {string} string +// +//nolint:dupl func (s *Service) deleteStorage(w http.ResponseWriter, r *http.Request) { + s.Lock() + defer s.Unlock() + hLogger := s.logger.With(slog.String("handler", "deleteStorage")) storageName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/handlers_test.go b/internal/server/handlers/handlers_test.go index 24848004..5eba4c3f 100644 --- a/internal/server/handlers/handlers_test.go +++ b/internal/server/handlers/handlers_test.go @@ -209,7 +209,7 @@ func (mock configurationManagerMock) Write(_ context.Context, config *model.Conf type MockConfigApplier struct{} -func (a *MockConfigApplier) ApplyNewConfig(context.Context) error { +func (a *MockConfigApplier) ApplyNewConfig(_ context.Context, _ *model.Config) error { return nil } diff --git a/internal/server/handlers/restore.go b/internal/server/handlers/restore.go index 67790957..7ccda0bc 100644 --- a/internal/server/handlers/restore.go +++ b/internal/server/handlers/restore.go @@ -27,6 +27,9 @@ import ( // @Failure 400 {string} string // @Failure 405 {string} string func (s *Service) RestoreFullHandler(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "RestoreFullHandler")) var request dto.RestoreRequest @@ -80,6 +83,9 @@ func (s *Service) RestoreFullHandler(w http.ResponseWriter, r *http.Request) { // @Failure 400 {string} string // @Failure 405 {string} string func (s *Service) RestoreIncrementalHandler(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "RestoreIncrementalHandler")) var request dto.RestoreRequest @@ -135,6 +141,9 @@ func (s *Service) RestoreIncrementalHandler(w http.ResponseWriter, r *http.Reque // @Failure 400 {string} string // @Failure 405 {string} string func (s *Service) RestoreByTimeHandler(w http.ResponseWriter, r *http.Request) { + s.RLock() + defer s.RUnlock() + hLogger := s.logger.With(slog.String("handler", "RestoreByTimeHandler")) var request dto.RestoreTimestampRequest diff --git a/internal/server/handlers/service.go b/internal/server/handlers/service.go index 9a27918c..69960709 100644 --- a/internal/server/handlers/service.go +++ b/internal/server/handlers/service.go @@ -12,7 +12,7 @@ import ( ) type Service struct { - sync.RWMutex + sync.RWMutex // config should be used only under lock config *model.Config configApplier service.ConfigApplier scheduler quartz.Scheduler diff --git a/pkg/model/config.go b/pkg/model/config.go index b8835ab3..5209d3cf 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -2,12 +2,10 @@ package model import ( "fmt" - "sync" ) // Config represents the service configuration. type Config struct { - sync.Mutex ServiceConfig BackupServiceConfig AerospikeClusters map[string]*AerospikeCluster Storage map[string]Storage // Storage is an interface @@ -41,9 +39,6 @@ func (c *Config) AddStorage(name string, s Storage) error { } func (c *Config) DeleteStorage(name string) error { - c.Lock() - defer c.Unlock() - s, exists := c.Storage[name] if !exists { return fmt.Errorf("delete storage %q: %w", name, ErrNotFound) @@ -56,9 +51,6 @@ func (c *Config) DeleteStorage(name string) error { } func (c *Config) UpdateStorage(name string, s Storage) error { - c.Lock() - defer c.Unlock() - if _, exists := c.Storage[name]; !exists { return fmt.Errorf("update storage %q: %w", name, ErrNotFound) } @@ -85,9 +77,6 @@ func (c *Config) routineUsesStorage(s Storage) string { } func (c *Config) AddPolicy(name string, p *BackupPolicy) error { - c.Lock() - defer c.Unlock() - if _, exists := c.BackupPolicies[name]; exists { return fmt.Errorf("add backup policy %q: %w", name, ErrAlreadyExists) } @@ -96,9 +85,6 @@ func (c *Config) AddPolicy(name string, p *BackupPolicy) error { } func (c *Config) DeletePolicy(name string) error { - c.Lock() - defer c.Unlock() - p, exists := c.BackupPolicies[name] if !exists { return fmt.Errorf("delete backup policy %q: %w", name, ErrNotFound) @@ -111,9 +97,6 @@ func (c *Config) DeletePolicy(name string) error { } func (c *Config) UpdatePolicy(name string, p *BackupPolicy) error { - c.Lock() - defer c.Unlock() - if _, exists := c.BackupPolicies[name]; !exists { return fmt.Errorf("update backup policy %q: %w", name, ErrNotFound) } @@ -139,9 +122,6 @@ func (c *Config) routineUsesPolicy(p *BackupPolicy) string { } func (c *Config) AddRoutine(name string, r *BackupRoutine) error { - c.Lock() - defer c.Unlock() - if _, exists := c.BackupRoutines[name]; exists { return fmt.Errorf("add backup routine %q: %w", name, ErrAlreadyExists) } @@ -150,9 +130,6 @@ func (c *Config) AddRoutine(name string, r *BackupRoutine) error { } func (c *Config) DeleteRoutine(name string) error { - c.Lock() - defer c.Unlock() - if _, exists := c.BackupRoutines[name]; !exists { return fmt.Errorf("delete backup routine %q: %w", name, ErrNotFound) } @@ -161,9 +138,6 @@ func (c *Config) DeleteRoutine(name string) error { } func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { - c.Lock() - defer c.Unlock() - if _, exists := c.BackupRoutines[name]; !exists { return fmt.Errorf("update backup routine %q: %w", name, ErrNotFound) } @@ -172,9 +146,6 @@ func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { } func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { - c.Lock() - defer c.Unlock() - if _, exists := c.AerospikeClusters[name]; exists { return fmt.Errorf("add Aerospike cluster %q: %w", name, ErrAlreadyExists) } @@ -183,9 +154,6 @@ func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { } func (c *Config) DeleteCluster(name string) error { - c.Lock() - defer c.Unlock() - cluster, exists := c.AerospikeClusters[name] if !exists { return fmt.Errorf("delete Aerospike cluster %q: %w", name, ErrNotFound) @@ -198,9 +166,6 @@ func (c *Config) DeleteCluster(name string) error { } func (c *Config) UpdateCluster(name string, cluster *AerospikeCluster) error { - c.Lock() - defer c.Unlock() - if _, exists := c.AerospikeClusters[name]; !exists { return fmt.Errorf("update Aerospike cluster %q: %w", name, ErrNotFound) } @@ -227,9 +192,6 @@ func (c *Config) routineUsesCluster(cluster *AerospikeCluster) string { } func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { - c.Lock() - defer c.Unlock() - if _, exists := c.SecretAgents[name]; exists { return fmt.Errorf("add Secret agent %q: %w", name, ErrAlreadyExists) } @@ -238,11 +200,6 @@ func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { } func (c *Config) CopyFrom(other *Config) { - c.Lock() - other.Lock() - defer c.Unlock() - defer other.Unlock() - c.AerospikeClusters = other.AerospikeClusters c.Storage = other.Storage c.BackupPolicies = other.BackupPolicies @@ -265,9 +222,6 @@ func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*S // ToggleRoutineDisabled sets the Disabled field of the BackupRoutine based on the provided state. func (c *Config) ToggleRoutineDisabled(name string, isDisabled bool) error { - c.Lock() - defer c.Unlock() - _, exists := c.BackupRoutines[name] if !exists { return fmt.Errorf("toggle disable for backup routine %q: %w", name, ErrNotFound) diff --git a/pkg/service/config_applier.go b/pkg/service/config_applier.go index 882bca76..e4ea4460 100644 --- a/pkg/service/config_applier.go +++ b/pkg/service/config_applier.go @@ -14,13 +14,11 @@ import ( // ConfigApplier is responsible for applying new configuration to the service. type ConfigApplier interface { - ApplyNewConfig(context.Context) error + ApplyNewConfig(ctx context.Context, config *model.Config) error } type DefaultConfigApplier struct { - sync.Mutex scheduler quartz.Scheduler - config *model.Config backends BackendsHolder clientManager aerospike.ClientManager handlerHolder BackupHandlerHolder @@ -28,38 +26,33 @@ type DefaultConfigApplier struct { func NewDefaultConfigApplier( scheduler quartz.Scheduler, - config *model.Config, backends BackendsHolder, manager aerospike.ClientManager, handlerHolder BackupHandlerHolder, ) ConfigApplier { return &DefaultConfigApplier{ scheduler: scheduler, - config: config, backends: backends, clientManager: manager, handlerHolder: handlerHolder, } } -func (a *DefaultConfigApplier) ApplyNewConfig(ctx context.Context) error { - a.Lock() - defer a.Unlock() - +func (a *DefaultConfigApplier) ApplyNewConfig(ctx context.Context, config *model.Config) error { err := a.clearPeriodicSchedulerJobs() if err != nil { return fmt.Errorf("failed to clear periodic jobs: %w", err) } - a.backends.Init(a.config) + a.backends.Init(config) // Refill handlers - newHandlers := makeHandlers(ctx, a.clientManager, a.config, a.backends, a.handlerHolder) + newHandlers := makeHandlers(ctx, a.clientManager, config, a.backends, a.handlerHolder) clear(a.handlerHolder) for k, v := range newHandlers { (a.handlerHolder)[k] = v } - err = scheduleRoutines(a.scheduler, a.config.BackupRoutines, a.handlerHolder) + err = scheduleRoutines(a.scheduler, config.BackupRoutines, a.handlerHolder) if err != nil { return fmt.Errorf("failed to schedule periodic backups: %w", err) } From 0521d9229bab1d86c417b5300f5caf8d1f5b3d0e Mon Sep 17 00:00:00 2001 From: akorotkov Date: Sun, 29 Dec 2024 14:58:19 +0200 Subject: [PATCH 06/18] use rlock --- internal/server/handlers/config_policy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/handlers/config_policy.go b/internal/server/handlers/config_policy.go index 4970050e..6ca4e643 100644 --- a/internal/server/handlers/config_policy.go +++ b/internal/server/handlers/config_policy.go @@ -122,7 +122,7 @@ func (s *Service) ReadPolicies(w http.ResponseWriter, _ *http.Request) { // @Failure 500 {string} string "The specified policy could not be found" func (s *Service) readPolicy(w http.ResponseWriter, r *http.Request) { s.RLock() - defer s.Unlock() + defer s.RUnlock() hLogger := s.logger.With(slog.String("handler", "readPolicy")) From 1d314cb46dd7b25c8127cf5a0d61c46c837bf7da Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 09:31:50 +0200 Subject: [PATCH 07/18] ignore empty metadata files --- pkg/model/backup_details.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/model/backup_details.go b/pkg/model/backup_details.go index 388930da..2c27c000 100644 --- a/pkg/model/backup_details.go +++ b/pkg/model/backup_details.go @@ -40,6 +40,9 @@ type BackupMetadata struct { // NewMetadataFromBytes creates a new Metadata object from a byte slice func NewMetadataFromBytes(data []byte) (*BackupMetadata, error) { + if len(data) == 0 { + return nil, fmt.Errorf("empty metadata file") + } var metadata BackupMetadata err := yaml.Unmarshal(data, &metadata) if err != nil { From 99280628c769e2d3e98aafeace092c14442c5b1e Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 11:56:56 +0200 Subject: [PATCH 08/18] pass only routines to config applier --- cmd/backup/main.go | 2 +- internal/server/handlers/config.go | 4 ++-- internal/server/handlers/handlers_test.go | 4 ++-- pkg/service/backup_backends_holder.go | 5 ++--- pkg/service/config_applier.go | 14 +++++++------- pkg/service/restore_data_test.go | 2 +- 6 files changed, 15 insertions(+), 16 deletions(-) diff --git a/cmd/backup/main.go b/cmd/backup/main.go index b794cb6a..4eaf10a1 100644 --- a/cmd/backup/main.go +++ b/cmd/backup/main.go @@ -88,7 +88,7 @@ func startService(configFile string, remote bool) error { backupHandlers, ) - err = configApplier.ApplyNewConfig(ctx, config) + err = configApplier.ApplyNewRoutines(ctx, config.BackupRoutines) if err != nil { return fmt.Errorf("failed to apply new config: %w", err) } diff --git a/internal/server/handlers/config.go b/internal/server/handlers/config.go index 5b084b38..59199e70 100644 --- a/internal/server/handlers/config.go +++ b/internal/server/handlers/config.go @@ -134,7 +134,7 @@ func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { } s.config.CopyFrom(config) - err = s.configApplier.ApplyNewConfig(r.Context(), s.config) + err = s.configApplier.ApplyNewRoutines(r.Context(), s.config.BackupRoutines) if err != nil { hLogger.Error("failed to apply config", @@ -158,7 +158,7 @@ func (s *Service) changeConfig(ctx context.Context, updateFunc func(*model.Confi return fmt.Errorf("failed to write configuration: %w", err) } - err = s.configApplier.ApplyNewConfig(ctx, s.config) + err = s.configApplier.ApplyNewRoutines(ctx, s.config.BackupRoutines) if err != nil { return fmt.Errorf("failed to apply new configuration: %w", err) } diff --git a/internal/server/handlers/handlers_test.go b/internal/server/handlers/handlers_test.go index 5eba4c3f..04e1566e 100644 --- a/internal/server/handlers/handlers_test.go +++ b/internal/server/handlers/handlers_test.go @@ -183,7 +183,7 @@ func (mock backendsHolderMock) Get(routineName string) (*service.BackupBackend, return &service.BackupBackend{}, true } -func (mock backendsHolderMock) Init(_ *model.Config) { +func (mock backendsHolderMock) Init(_ map[string]*model.BackupRoutine) { } func (mock backendsHolderMock) GetAllReaders() map[string]service.BackupListReader { @@ -209,7 +209,7 @@ func (mock configurationManagerMock) Write(_ context.Context, config *model.Conf type MockConfigApplier struct{} -func (a *MockConfigApplier) ApplyNewConfig(_ context.Context, _ *model.Config) error { +func (a *MockConfigApplier) ApplyNewRoutines(_ context.Context, _ map[string]*model.BackupRoutine) error { return nil } diff --git a/pkg/service/backup_backends_holder.go b/pkg/service/backup_backends_holder.go index 50aa80ff..3af2d646 100644 --- a/pkg/service/backup_backends_holder.go +++ b/pkg/service/backup_backends_holder.go @@ -10,7 +10,7 @@ import ( // We need it because same backends are used in API handlers and backup jobs. type BackendsHolder interface { // Init creates new backends from config. - Init(config *model.Config) + Init(routines map[string]*model.BackupRoutine) // GetReader returns BackupBackend for routine as BackupListReader. GetReader(routineName string) (BackupListReader, bool) // Get returns BackupBackend for routine. @@ -24,11 +24,10 @@ type BackendHolderImpl struct { data map[string]*BackupBackend } -func (b *BackendHolderImpl) Init(config *model.Config) { +func (b *BackendHolderImpl) Init(routines map[string]*model.BackupRoutine) { b.Lock() defer b.Unlock() - routines := config.BackupRoutines b.data = make(map[string]*BackupBackend, len(routines)) for routineName, routine := range routines { b.data[routineName] = newBackend(routineName, routine.Storage) diff --git a/pkg/service/config_applier.go b/pkg/service/config_applier.go index e4ea4460..f3415d0d 100644 --- a/pkg/service/config_applier.go +++ b/pkg/service/config_applier.go @@ -14,7 +14,7 @@ import ( // ConfigApplier is responsible for applying new configuration to the service. type ConfigApplier interface { - ApplyNewConfig(ctx context.Context, config *model.Config) error + ApplyNewRoutines(ctx context.Context, routines map[string]*model.BackupRoutine) error } type DefaultConfigApplier struct { @@ -38,21 +38,21 @@ func NewDefaultConfigApplier( } } -func (a *DefaultConfigApplier) ApplyNewConfig(ctx context.Context, config *model.Config) error { +func (a *DefaultConfigApplier) ApplyNewRoutines(ctx context.Context, routines map[string]*model.BackupRoutine) error { err := a.clearPeriodicSchedulerJobs() if err != nil { return fmt.Errorf("failed to clear periodic jobs: %w", err) } - a.backends.Init(config) + a.backends.Init(routines) // Refill handlers - newHandlers := makeHandlers(ctx, a.clientManager, config, a.backends, a.handlerHolder) + newHandlers := makeHandlers(ctx, a.clientManager, routines, a.backends, a.handlerHolder) clear(a.handlerHolder) for k, v := range newHandlers { (a.handlerHolder)[k] = v } - err = scheduleRoutines(a.scheduler, config.BackupRoutines, a.handlerHolder) + err = scheduleRoutines(a.scheduler, routines, a.handlerHolder) if err != nil { return fmt.Errorf("failed to schedule periodic backups: %w", err) } @@ -81,7 +81,7 @@ func (a *DefaultConfigApplier) clearPeriodicSchedulerJobs() error { func makeHandlers( ctx context.Context, clientManager aerospike.ClientManager, - config *model.Config, + routines map[string]*model.BackupRoutine, backends BackendsHolder, oldHandlers BackupHandlerHolder, ) BackupHandlerHolder { @@ -89,7 +89,7 @@ func makeHandlers( var wg sync.WaitGroup var mu sync.Mutex - for routineName, routine := range config.BackupRoutines { + for routineName, routine := range routines { wg.Add(1) go func() { defer wg.Done() diff --git a/pkg/service/restore_data_test.go b/pkg/service/restore_data_test.go index 31e780d4..72cb5766 100644 --- a/pkg/service/restore_data_test.go +++ b/pkg/service/restore_data_test.go @@ -37,7 +37,7 @@ func (b *BackendHolderMock) GetAllReaders() map[string]BackupListReader { return nil } -func (b *BackendHolderMock) Init(_ *model.Config) { +func (b *BackendHolderMock) Init(_ map[string]*model.BackupRoutine) { } func makeTestRestoreService(wg *sync.WaitGroup) *dataRestorer { From 65f49c7ee7638fa981f2f9283e40d767b23b2590 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 12:00:29 +0200 Subject: [PATCH 09/18] remove all locks (temp) --- internal/server/handlers/backup.go | 15 --------------- internal/server/handlers/config.go | 9 --------- internal/server/handlers/config_cluster.go | 15 --------------- internal/server/handlers/config_policy.go | 15 --------------- internal/server/handlers/config_routine.go | 21 --------------------- internal/server/handlers/config_storage.go | 15 --------------- internal/server/handlers/restore.go | 9 --------- internal/server/handlers/service.go | 5 +---- pkg/model/config.go | 1 + 9 files changed, 2 insertions(+), 103 deletions(-) diff --git a/internal/server/handlers/backup.go b/internal/server/handlers/backup.go index 38531416..d2a9ccfe 100644 --- a/internal/server/handlers/backup.go +++ b/internal/server/handlers/backup.go @@ -77,9 +77,6 @@ func (s *Service) GetIncrementalBackupsForRoutine(w http.ResponseWriter, r *http } func (s *Service) readAllBackups(w http.ResponseWriter, r *http.Request, isFullBackup bool) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "readAllBackups")) from := r.URL.Query().Get("from") @@ -128,9 +125,6 @@ func (s *Service) readAllBackups(w http.ResponseWriter, r *http.Request, isFullB //nolint:funlen // Function is long because of logging. func (s *Service) readBackupsForRoutine(w http.ResponseWriter, r *http.Request, isFullBackup bool) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "readBackupsForRoutine")) from := r.URL.Query().Get("from") @@ -237,9 +231,6 @@ func backupsReadFunction( // @Failure 404 {string} string // @Failure 500 {string} string func (s *Service) ScheduleFullBackup(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "ScheduleFullBackup")) routineName := mux.Vars(r)["name"] @@ -302,9 +293,6 @@ func (s *Service) ScheduleFullBackup(w http.ResponseWriter, r *http.Request) { // @Failure 400 {string} string // @Failure 500 {string} string func (s *Service) GetCurrentBackupInfo(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "GetCurrentBackupInfo")) routineName := mux.Vars(r)["name"] @@ -354,9 +342,6 @@ func (s *Service) GetCurrentBackupInfo(w http.ResponseWriter, r *http.Request) { // @Failure 404 {string} string // @Failure 500 {string} string func (s *Service) CancelCurrentBackup(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "CancelCurrentBackup")) routineName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/config.go b/internal/server/handlers/config.go index 59199e70..dc727577 100644 --- a/internal/server/handlers/config.go +++ b/internal/server/handlers/config.go @@ -31,9 +31,6 @@ func (s *Service) ConfigActionHandler(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} dto.Config // @Failure 500 {string} string func (s *Service) readConfig(w http.ResponseWriter) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "readConfig")) configuration, err := dto.Serialize(dto.NewConfigFromModel(s.config), dto.JSON) @@ -63,9 +60,6 @@ func (s *Service) readConfig(w http.ResponseWriter) { // @Success 200 // @Failure 400 {string} string func (s *Service) updateConfig(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "updateConfig")) newConfig, err := dto.NewConfigFromReader(r.Body, dto.JSON) @@ -107,9 +101,6 @@ func (s *Service) updateConfig(w http.ResponseWriter, r *http.Request) { // @Success 200 // @Failure 400 {string} string func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "ApplyConfig")) config, err := s.configurationManager.Read(r.Context()) diff --git a/internal/server/handlers/config_cluster.go b/internal/server/handlers/config_cluster.go index eee01b38..0788c2d2 100644 --- a/internal/server/handlers/config_cluster.go +++ b/internal/server/handlers/config_cluster.go @@ -37,9 +37,6 @@ func (s *Service) ConfigClusterActionHandler(w http.ResponseWriter, r *http.Requ // @Failure 400 {string} string // @Failure 500 {string} string func (s *Service) addAerospikeCluster(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "addAerospikeCluster")) newCluster, err := dto.NewClusterFromReader(r.Body, dto.JSON) @@ -90,9 +87,6 @@ func (s *Service) addAerospikeCluster(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} map[string]dto.AerospikeCluster // @Failure 500 {string} string func (s *Service) ReadAerospikeClusters(w http.ResponseWriter, _ *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "ReadAerospikeClusters")) toDTO := dto.ConvertModelMapToDTO(s.config.AerospikeClusters, func(m *model.AerospikeCluster) *dto.AerospikeCluster { @@ -130,9 +124,6 @@ func (s *Service) ReadAerospikeClusters(w http.ResponseWriter, _ *http.Request) // @Failure 404 {string} string "The specified cluster could not be found" // @Failure 500 {string} string "The specified cluster could not be found" func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "readAerospikeCluster")) clusterName := mux.Vars(r)["name"] @@ -180,9 +171,6 @@ func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { // @Success 200 // @Failure 400 {string} string func (s *Service) updateAerospikeCluster(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "updateAerospikeCluster")) updatedCluster, err := dto.NewClusterFromReader(r.Body, dto.JSON) @@ -242,9 +230,6 @@ func (s *Service) updateAerospikeCluster(w http.ResponseWriter, r *http.Request) // //nolint:dupl func (s *Service) deleteAerospikeCluster(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "deleteAerospikeCluster")) clusterName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/config_policy.go b/internal/server/handlers/config_policy.go index 6ca4e643..70ab0376 100644 --- a/internal/server/handlers/config_policy.go +++ b/internal/server/handlers/config_policy.go @@ -38,9 +38,6 @@ func (s *Service) ConfigPolicyActionHandler(w http.ResponseWriter, r *http.Reque // //nolint:dupl func (s *Service) addPolicy(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "addPolicy")) newPolicy, err := dto.NewBackupPolicyFromReader(r.Body, dto.JSON) @@ -84,9 +81,6 @@ func (s *Service) addPolicy(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) ReadPolicies(w http.ResponseWriter, _ *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "ReadPolicies")) policies := dto.ConvertModelMapToDTO(s.config.BackupPolicies, dto.NewBackupPolicyFromModel) @@ -121,9 +115,6 @@ func (s *Service) ReadPolicies(w http.ResponseWriter, _ *http.Request) { // @Failure 404 {string} string "The specified policy could not be found" // @Failure 500 {string} string "The specified policy could not be found" func (s *Service) readPolicy(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "readPolicy")) policyName := mux.Vars(r)["name"] @@ -170,9 +161,6 @@ func (s *Service) readPolicy(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) updatePolicy(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "updatePolicy")) updatedPolicy, err := dto.NewBackupPolicyFromReader(r.Body, dto.JSON) @@ -217,9 +205,6 @@ func (s *Service) updatePolicy(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) deletePolicy(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "deletePolicy")) policyName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/config_routine.go b/internal/server/handlers/config_routine.go index 07f67512..6e432205 100644 --- a/internal/server/handlers/config_routine.go +++ b/internal/server/handlers/config_routine.go @@ -38,9 +38,6 @@ func (s *Service) ConfigRoutineActionHandler(w http.ResponseWriter, r *http.Requ // //nolint:dupl func (s *Service) addRoutine(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "addRoutine")) newRoutine, err := dto.NewRoutineFromReader(r.Body, dto.JSON) @@ -92,9 +89,6 @@ func (s *Service) addRoutine(w http.ResponseWriter, r *http.Request) { // @Success 200 {object} map[string]dto.BackupRoutine // @Failure 400 {string} string func (s *Service) ReadRoutines(w http.ResponseWriter, _ *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "ReadRoutines")) toDTO := dto.ConvertModelMapToDTO(s.config.BackupRoutines, func(m *model.BackupRoutine) *dto.BackupRoutine { @@ -133,9 +127,6 @@ func (s *Service) ReadRoutines(w http.ResponseWriter, _ *http.Request) { // //nolint:dupl func (s *Service) readRoutine(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "readRoutine")) routineName := mux.Vars(r)["name"] @@ -181,9 +172,6 @@ func (s *Service) readRoutine(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) updateRoutine(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "updateRoutine")) updatedRoutine, err := dto.NewRoutineFromReader(r.Body, dto.JSON) @@ -238,9 +226,6 @@ func (s *Service) updateRoutine(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) deleteRoutine(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "deleteRoutine")) routineName := mux.Vars(r)["name"] @@ -274,9 +259,6 @@ func (s *Service) deleteRoutine(w http.ResponseWriter, r *http.Request) { // @Failure 404 {string} string // @Router /v1/config/routines/{name}/enable [put] func (s *Service) EnableRoutine(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "enableRoutine")) routineName := mux.Vars(r)["name"] @@ -317,9 +299,6 @@ func (s *Service) EnableRoutine(w http.ResponseWriter, r *http.Request) { // @Failure 500 {string} string "Unexpected error occurred." // @Router /v1/config/routines/{name}/disable [put] func (s *Service) DisableRoutine(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "disableRoutine")) routineName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/config_storage.go b/internal/server/handlers/config_storage.go index 77c75574..bafa5820 100644 --- a/internal/server/handlers/config_storage.go +++ b/internal/server/handlers/config_storage.go @@ -36,9 +36,6 @@ func (s *Service) ConfigStorageActionHandler(w http.ResponseWriter, r *http.Requ // @Success 201 // @Failure 400 {string} string func (s *Service) addStorage(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "addStorage")) newStorage, err := dto.NewStorageFromReader(r.Body, dto.JSON) @@ -85,9 +82,6 @@ func (s *Service) addStorage(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) ReadAllStorage(w http.ResponseWriter, _ *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "ReadAllStorage")) toDTO := dto.ConvertStorageMapToDTO(s.config.Storage, s.config) @@ -124,9 +118,6 @@ func (s *Service) ReadAllStorage(w http.ResponseWriter, _ *http.Request) { // //nolint:dupl func (s *Service) readStorage(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "readStorage")) storageName := mux.Vars(r)["name"] @@ -171,9 +162,6 @@ func (s *Service) readStorage(w http.ResponseWriter, r *http.Request) { // @Success 200 // @Failure 400 {string} string func (s *Service) updateStorage(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "updateStorage")) updatedStorage, err := dto.NewStorageFromReader(r.Body, dto.JSON) @@ -220,9 +208,6 @@ func (s *Service) updateStorage(w http.ResponseWriter, r *http.Request) { // //nolint:dupl func (s *Service) deleteStorage(w http.ResponseWriter, r *http.Request) { - s.Lock() - defer s.Unlock() - hLogger := s.logger.With(slog.String("handler", "deleteStorage")) storageName := mux.Vars(r)["name"] diff --git a/internal/server/handlers/restore.go b/internal/server/handlers/restore.go index 7ccda0bc..67790957 100644 --- a/internal/server/handlers/restore.go +++ b/internal/server/handlers/restore.go @@ -27,9 +27,6 @@ import ( // @Failure 400 {string} string // @Failure 405 {string} string func (s *Service) RestoreFullHandler(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "RestoreFullHandler")) var request dto.RestoreRequest @@ -83,9 +80,6 @@ func (s *Service) RestoreFullHandler(w http.ResponseWriter, r *http.Request) { // @Failure 400 {string} string // @Failure 405 {string} string func (s *Service) RestoreIncrementalHandler(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "RestoreIncrementalHandler")) var request dto.RestoreRequest @@ -141,9 +135,6 @@ func (s *Service) RestoreIncrementalHandler(w http.ResponseWriter, r *http.Reque // @Failure 400 {string} string // @Failure 405 {string} string func (s *Service) RestoreByTimeHandler(w http.ResponseWriter, r *http.Request) { - s.RLock() - defer s.RUnlock() - hLogger := s.logger.With(slog.String("handler", "RestoreByTimeHandler")) var request dto.RestoreTimestampRequest diff --git a/internal/server/handlers/service.go b/internal/server/handlers/service.go index 69960709..b362df30 100644 --- a/internal/server/handlers/service.go +++ b/internal/server/handlers/service.go @@ -1,18 +1,15 @@ package handlers import ( - "log/slog" - "sync" - "github.com/aerospike/aerospike-backup-service/v3/internal/server/configuration" "github.com/aerospike/aerospike-backup-service/v3/pkg/model" "github.com/aerospike/aerospike-backup-service/v3/pkg/service" "github.com/aerospike/aerospike-backup-service/v3/pkg/service/aerospike" "github.com/reugn/go-quartz/quartz" + "log/slog" ) type Service struct { - sync.RWMutex // config should be used only under lock config *model.Config configApplier service.ConfigApplier scheduler quartz.Scheduler diff --git a/pkg/model/config.go b/pkg/model/config.go index 5209d3cf..9cda4b20 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -6,6 +6,7 @@ import ( // Config represents the service configuration. type Config struct { + //sync.RWMutex ServiceConfig BackupServiceConfig AerospikeClusters map[string]*AerospikeCluster Storage map[string]Storage // Storage is an interface From f070b06b07eda84b70866734d000eb00097247bf Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 12:02:17 +0200 Subject: [PATCH 10/18] reformat --- internal/server/handlers/config_cluster.go | 2 -- internal/server/handlers/config_policy.go | 4 ---- internal/server/handlers/config_routine.go | 2 -- internal/server/handlers/config_storage.go | 4 ---- internal/server/handlers/service.go | 3 ++- pkg/model/config.go | 2 +- 6 files changed, 3 insertions(+), 14 deletions(-) diff --git a/internal/server/handlers/config_cluster.go b/internal/server/handlers/config_cluster.go index 0788c2d2..da498032 100644 --- a/internal/server/handlers/config_cluster.go +++ b/internal/server/handlers/config_cluster.go @@ -227,8 +227,6 @@ func (s *Service) updateAerospikeCluster(w http.ResponseWriter, r *http.Request) // @Param name path string true "Aerospike cluster name" // @Success 204 // @Failure 400 {string} string -// -//nolint:dupl func (s *Service) deleteAerospikeCluster(w http.ResponseWriter, r *http.Request) { hLogger := s.logger.With(slog.String("handler", "deleteAerospikeCluster")) diff --git a/internal/server/handlers/config_policy.go b/internal/server/handlers/config_policy.go index 70ab0376..be758b65 100644 --- a/internal/server/handlers/config_policy.go +++ b/internal/server/handlers/config_policy.go @@ -78,8 +78,6 @@ func (s *Service) addPolicy(w http.ResponseWriter, r *http.Request) { // @Produce json // @Success 200 {object} map[string]dto.BackupPolicy // @Failure 500 {string} string -// -//nolint:dupl func (s *Service) ReadPolicies(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadPolicies")) @@ -202,8 +200,6 @@ func (s *Service) updatePolicy(w http.ResponseWriter, r *http.Request) { // @Param name path string true "Backup policy name" // @Success 204 // @Failure 400 {string} string -// -//nolint:dupl func (s *Service) deletePolicy(w http.ResponseWriter, r *http.Request) { hLogger := s.logger.With(slog.String("handler", "deletePolicy")) diff --git a/internal/server/handlers/config_routine.go b/internal/server/handlers/config_routine.go index 6e432205..72e130e5 100644 --- a/internal/server/handlers/config_routine.go +++ b/internal/server/handlers/config_routine.go @@ -223,8 +223,6 @@ func (s *Service) updateRoutine(w http.ResponseWriter, r *http.Request) { // @Param name path string true "Backup routine name" // @Success 204 // @Failure 400 {string} string -// -//nolint:dupl func (s *Service) deleteRoutine(w http.ResponseWriter, r *http.Request) { hLogger := s.logger.With(slog.String("handler", "deleteRoutine")) diff --git a/internal/server/handlers/config_storage.go b/internal/server/handlers/config_storage.go index bafa5820..9f055c04 100644 --- a/internal/server/handlers/config_storage.go +++ b/internal/server/handlers/config_storage.go @@ -79,8 +79,6 @@ func (s *Service) addStorage(w http.ResponseWriter, r *http.Request) { // @Produce json // @Success 200 {object} map[string]dto.Storage // @Failure 500 {string} string -// -//nolint:dupl func (s *Service) ReadAllStorage(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadAllStorage")) @@ -205,8 +203,6 @@ func (s *Service) updateStorage(w http.ResponseWriter, r *http.Request) { // @Param name path string true "Backup storage name" // @Success 204 // @Failure 400 {string} string -// -//nolint:dupl func (s *Service) deleteStorage(w http.ResponseWriter, r *http.Request) { hLogger := s.logger.With(slog.String("handler", "deleteStorage")) diff --git a/internal/server/handlers/service.go b/internal/server/handlers/service.go index b362df30..037673b8 100644 --- a/internal/server/handlers/service.go +++ b/internal/server/handlers/service.go @@ -1,12 +1,13 @@ package handlers import ( + "log/slog" + "github.com/aerospike/aerospike-backup-service/v3/internal/server/configuration" "github.com/aerospike/aerospike-backup-service/v3/pkg/model" "github.com/aerospike/aerospike-backup-service/v3/pkg/service" "github.com/aerospike/aerospike-backup-service/v3/pkg/service/aerospike" "github.com/reugn/go-quartz/quartz" - "log/slog" ) type Service struct { diff --git a/pkg/model/config.go b/pkg/model/config.go index 9cda4b20..046522b4 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -6,7 +6,7 @@ import ( // Config represents the service configuration. type Config struct { - //sync.RWMutex + // sync.RWMutex ServiceConfig BackupServiceConfig AerospikeClusters map[string]*AerospikeCluster Storage map[string]Storage // Storage is an interface From 286fe8e044e9e19e45dfd61823e824068befd110 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 12:15:14 +0200 Subject: [PATCH 11/18] add locks to config --- pkg/model/config.go | 53 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/pkg/model/config.go b/pkg/model/config.go index 046522b4..51cb2149 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -2,11 +2,12 @@ package model import ( "fmt" + "sync" ) // Config represents the service configuration. type Config struct { - // sync.RWMutex + sync.RWMutex ServiceConfig BackupServiceConfig AerospikeClusters map[string]*AerospikeCluster Storage map[string]Storage // Storage is an interface @@ -32,6 +33,9 @@ var ( ) func (c *Config) AddStorage(name string, s Storage) error { + c.Lock() + defer c.Unlock() + if _, exists := c.Storage[name]; exists { return fmt.Errorf("add storage %q: %w", name, ErrAlreadyExists) } @@ -40,6 +44,9 @@ func (c *Config) AddStorage(name string, s Storage) error { } func (c *Config) DeleteStorage(name string) error { + c.Lock() + defer c.Unlock() + s, exists := c.Storage[name] if !exists { return fmt.Errorf("delete storage %q: %w", name, ErrNotFound) @@ -52,6 +59,9 @@ func (c *Config) DeleteStorage(name string) error { } func (c *Config) UpdateStorage(name string, s Storage) error { + c.Lock() + defer c.Unlock() + if _, exists := c.Storage[name]; !exists { return fmt.Errorf("update storage %q: %w", name, ErrNotFound) } @@ -78,6 +88,9 @@ func (c *Config) routineUsesStorage(s Storage) string { } func (c *Config) AddPolicy(name string, p *BackupPolicy) error { + c.Lock() + defer c.Unlock() + if _, exists := c.BackupPolicies[name]; exists { return fmt.Errorf("add backup policy %q: %w", name, ErrAlreadyExists) } @@ -86,6 +99,9 @@ func (c *Config) AddPolicy(name string, p *BackupPolicy) error { } func (c *Config) DeletePolicy(name string) error { + c.Lock() + defer c.Unlock() + p, exists := c.BackupPolicies[name] if !exists { return fmt.Errorf("delete backup policy %q: %w", name, ErrNotFound) @@ -98,6 +114,9 @@ func (c *Config) DeletePolicy(name string) error { } func (c *Config) UpdatePolicy(name string, p *BackupPolicy) error { + c.Lock() + defer c.Unlock() + if _, exists := c.BackupPolicies[name]; !exists { return fmt.Errorf("update backup policy %q: %w", name, ErrNotFound) } @@ -123,6 +142,9 @@ func (c *Config) routineUsesPolicy(p *BackupPolicy) string { } func (c *Config) AddRoutine(name string, r *BackupRoutine) error { + c.Lock() + defer c.Unlock() + if _, exists := c.BackupRoutines[name]; exists { return fmt.Errorf("add backup routine %q: %w", name, ErrAlreadyExists) } @@ -131,6 +153,9 @@ func (c *Config) AddRoutine(name string, r *BackupRoutine) error { } func (c *Config) DeleteRoutine(name string) error { + c.Lock() + defer c.Unlock() + if _, exists := c.BackupRoutines[name]; !exists { return fmt.Errorf("delete backup routine %q: %w", name, ErrNotFound) } @@ -139,6 +164,9 @@ func (c *Config) DeleteRoutine(name string) error { } func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { + c.Lock() + defer c.Unlock() + if _, exists := c.BackupRoutines[name]; !exists { return fmt.Errorf("update backup routine %q: %w", name, ErrNotFound) } @@ -147,6 +175,9 @@ func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { } func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { + c.Lock() + defer c.Unlock() + if _, exists := c.AerospikeClusters[name]; exists { return fmt.Errorf("add Aerospike cluster %q: %w", name, ErrAlreadyExists) } @@ -155,6 +186,9 @@ func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { } func (c *Config) DeleteCluster(name string) error { + c.Lock() + defer c.Unlock() + cluster, exists := c.AerospikeClusters[name] if !exists { return fmt.Errorf("delete Aerospike cluster %q: %w", name, ErrNotFound) @@ -167,6 +201,9 @@ func (c *Config) DeleteCluster(name string) error { } func (c *Config) UpdateCluster(name string, cluster *AerospikeCluster) error { + c.Lock() + defer c.Unlock() + if _, exists := c.AerospikeClusters[name]; !exists { return fmt.Errorf("update Aerospike cluster %q: %w", name, ErrNotFound) } @@ -193,6 +230,9 @@ func (c *Config) routineUsesCluster(cluster *AerospikeCluster) string { } func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { + c.Lock() + defer c.Unlock() + if _, exists := c.SecretAgents[name]; exists { return fmt.Errorf("add Secret agent %q: %w", name, ErrAlreadyExists) } @@ -201,6 +241,11 @@ func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { } func (c *Config) CopyFrom(other *Config) { + c.Lock() + defer c.Unlock() + other.RLock() + defer other.RUnlock() + c.AerospikeClusters = other.AerospikeClusters c.Storage = other.Storage c.BackupPolicies = other.BackupPolicies @@ -209,6 +254,9 @@ func (c *Config) CopyFrom(other *Config) { } func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*SecretAgent, error) { + c.RLock() + defer c.RUnlock() + if name != nil { agent, ok := c.SecretAgents[*name] if !ok { @@ -223,6 +271,9 @@ func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*S // ToggleRoutineDisabled sets the Disabled field of the BackupRoutine based on the provided state. func (c *Config) ToggleRoutineDisabled(name string, isDisabled bool) error { + c.Lock() + defer c.Unlock() + _, exists := c.BackupRoutines[name] if !exists { return fmt.Errorf("toggle disable for backup routine %q: %w", name, ErrNotFound) From 663fc4fa54a2b487fcf7b074aea3a6c9ba532a28 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 13:26:10 +0200 Subject: [PATCH 12/18] add BackupConfig struct --- cmd/backup/main.go | 2 +- internal/server/handlers/backup.go | 5 +- internal/server/handlers/config.go | 4 +- internal/server/handlers/config_cluster.go | 12 +- internal/server/handlers/config_policy.go | 4 +- internal/server/handlers/config_routine.go | 11 +- internal/server/handlers/config_storage.go | 10 +- internal/server/handlers/handlers_test.go | 2 +- pkg/dto/aerospike_cluster.go | 6 +- pkg/dto/backup_details.go | 26 ++-- pkg/dto/backup_routine.go | 6 +- pkg/dto/base_dto.go | 2 +- pkg/dto/config.go | 18 +-- pkg/dto/config_test.go | 2 +- pkg/dto/restore_request.go | 2 +- pkg/dto/secret_agent.go | 2 +- pkg/dto/storage.go | 2 +- pkg/dto/storage_azure.go | 2 +- pkg/dto/storage_gcp.go | 2 +- pkg/dto/storage_s3.go | 2 +- pkg/model/config.go | 136 ++++++++++++------- pkg/service/aerospike/namespace_validator.go | 11 +- pkg/service/backup_scheduler_test.go | 11 +- 23 files changed, 162 insertions(+), 118 deletions(-) diff --git a/cmd/backup/main.go b/cmd/backup/main.go index 4eaf10a1..9184b180 100644 --- a/cmd/backup/main.go +++ b/cmd/backup/main.go @@ -88,7 +88,7 @@ func startService(configFile string, remote bool) error { backupHandlers, ) - err = configApplier.ApplyNewRoutines(ctx, config.BackupRoutines) + err = configApplier.ApplyNewRoutines(ctx, config.GetBackupConfig().BackupRoutines) if err != nil { return fmt.Errorf("failed to apply new config: %w", err) } diff --git a/internal/server/handlers/backup.go b/internal/server/handlers/backup.go index d2a9ccfe..0bd91e2a 100644 --- a/internal/server/handlers/backup.go +++ b/internal/server/handlers/backup.go @@ -103,7 +103,7 @@ func (s *Service) readAllBackups(w http.ResponseWriter, r *http.Request, isFullB return } - response, err := dto.Serialize(dto.ConvertBackupDetailsMap(backups, s.config), dto.JSON) + response, err := dto.Serialize(dto.ConvertBackupDetailsMap(backups, s.config.GetBackupConfig()), dto.JSON) if err != nil { hLogger.Error("failed to marshal backup list", slog.Any("error", err), @@ -169,8 +169,9 @@ func (s *Service) readBackupsForRoutine(w http.ResponseWriter, r *http.Request, http.Error(w, "failed to retrieve backup list: "+err.Error(), http.StatusInternalServerError) return } + backupConfig := s.config.GetBackupConfig() backupDetails := dto.ConvertModelsToDTO(backups, func(m *model.BackupDetails) *dto.BackupDetails { - return dto.NewBackupDetailsFromModel(m, s.config) + return dto.NewBackupDetailsFromModel(m, backupConfig) }) response, err := dto.Serialize(backupDetails, dto.JSON) if err != nil { diff --git a/internal/server/handlers/config.go b/internal/server/handlers/config.go index dc727577..f5ef5cac 100644 --- a/internal/server/handlers/config.go +++ b/internal/server/handlers/config.go @@ -125,7 +125,7 @@ func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { } s.config.CopyFrom(config) - err = s.configApplier.ApplyNewRoutines(r.Context(), s.config.BackupRoutines) + err = s.configApplier.ApplyNewRoutines(r.Context(), s.config.GetBackupConfig().BackupRoutines) if err != nil { hLogger.Error("failed to apply config", @@ -149,7 +149,7 @@ func (s *Service) changeConfig(ctx context.Context, updateFunc func(*model.Confi return fmt.Errorf("failed to write configuration: %w", err) } - err = s.configApplier.ApplyNewRoutines(ctx, s.config.BackupRoutines) + err = s.configApplier.ApplyNewRoutines(ctx, s.config.GetBackupConfig().BackupRoutines) if err != nil { return fmt.Errorf("failed to apply new configuration: %w", err) } diff --git a/internal/server/handlers/config_cluster.go b/internal/server/handlers/config_cluster.go index da498032..805c720e 100644 --- a/internal/server/handlers/config_cluster.go +++ b/internal/server/handlers/config_cluster.go @@ -89,8 +89,10 @@ func (s *Service) addAerospikeCluster(w http.ResponseWriter, r *http.Request) { func (s *Service) ReadAerospikeClusters(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadAerospikeClusters")) - toDTO := dto.ConvertModelMapToDTO(s.config.AerospikeClusters, func(m *model.AerospikeCluster) *dto.AerospikeCluster { - return dto.NewClusterFromModel(m, s.config) + backupConfig := s.config.GetBackupConfig() + clusters := backupConfig.AerospikeClusters + toDTO := dto.ConvertModelMapToDTO(clusters, func(m *model.AerospikeCluster) *dto.AerospikeCluster { + return dto.NewClusterFromModel(m, backupConfig) }) jsonResponse, err := dto.Serialize(toDTO, dto.JSON) if err != nil { @@ -132,7 +134,7 @@ func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { http.Error(w, clusterNameNotSpecifiedMsg, http.StatusBadRequest) return } - cluster, ok := s.config.AerospikeClusters[clusterName] + cluster, ok := s.config.GetBackupConfig().AerospikeClusters[clusterName] if !ok { hLogger.Error("cluster not found", slog.String("name", clusterName), @@ -140,7 +142,7 @@ func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { http.Error(w, fmt.Sprintf("cluster %s could not be found", clusterName), http.StatusNotFound) return } - jsonResponse, err := dto.Serialize(dto.NewClusterFromModel(cluster, s.config), dto.JSON) + jsonResponse, err := dto.Serialize(dto.NewClusterFromModel(cluster, s.config.GetBackupConfig()), dto.JSON) if err != nil { hLogger.Error("failed to marshal cluster", slog.Any("error", err), @@ -194,7 +196,7 @@ func (s *Service) updateAerospikeCluster(w http.ResponseWriter, r *http.Request) return } - err = s.nsValidator.ValidateRoutines(cluster, s.config) + err = s.nsValidator.ValidateRoutines(cluster, s.config.GetBackupConfig().BackupRoutines) if err != nil { hLogger.Error("cluster namespace validation failed", slog.String("name", clusterName), diff --git a/internal/server/handlers/config_policy.go b/internal/server/handlers/config_policy.go index be758b65..8a11c031 100644 --- a/internal/server/handlers/config_policy.go +++ b/internal/server/handlers/config_policy.go @@ -81,7 +81,7 @@ func (s *Service) addPolicy(w http.ResponseWriter, r *http.Request) { func (s *Service) ReadPolicies(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadPolicies")) - policies := dto.ConvertModelMapToDTO(s.config.BackupPolicies, dto.NewBackupPolicyFromModel) + policies := dto.ConvertModelMapToDTO(s.config.GetBackupConfig().BackupPolicies, dto.NewBackupPolicyFromModel) jsonResponse, err := dto.Serialize(policies, dto.JSON) if err != nil { hLogger.Error("failed to marshal backup policies", @@ -121,7 +121,7 @@ func (s *Service) readPolicy(w http.ResponseWriter, r *http.Request) { http.Error(w, policyNameNotSpecifiedMsg, http.StatusBadRequest) return } - policy, ok := s.config.BackupPolicies[policyName] + policy, ok := s.config.GetBackupConfig().BackupPolicies[policyName] if !ok { hLogger.Error("policy not found") http.Error(w, fmt.Sprintf("policy %s could not be found", policyName), http.StatusNotFound) diff --git a/internal/server/handlers/config_routine.go b/internal/server/handlers/config_routine.go index 72e130e5..49f621ff 100644 --- a/internal/server/handlers/config_routine.go +++ b/internal/server/handlers/config_routine.go @@ -55,7 +55,7 @@ func (s *Service) addRoutine(w http.ResponseWriter, r *http.Request) { http.Error(w, routineNameNotSpecifiedMsg, http.StatusBadRequest) return } - toModel, err := newRoutine.ToModel(s.config, s.nsValidator) + toModel, err := newRoutine.ToModel(s.config.GetBackupConfig(), s.nsValidator) if err != nil { hLogger.Error("failed to create routine", slog.String("name", name), @@ -91,7 +91,8 @@ func (s *Service) addRoutine(w http.ResponseWriter, r *http.Request) { func (s *Service) ReadRoutines(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadRoutines")) - toDTO := dto.ConvertModelMapToDTO(s.config.BackupRoutines, func(m *model.BackupRoutine) *dto.BackupRoutine { + routines := s.config.GetBackupConfig().BackupRoutines + toDTO := dto.ConvertModelMapToDTO(routines, func(m *model.BackupRoutine) *dto.BackupRoutine { return dto.NewRoutineFromModel(m, s.config) }) @@ -124,8 +125,6 @@ func (s *Service) ReadRoutines(w http.ResponseWriter, _ *http.Request) { // @Success 200 {object} dto.BackupRoutine // @Response 400 {string} string // @Failure 404 {string} string "The specified cluster could not be found" -// -//nolint:dupl func (s *Service) readRoutine(w http.ResponseWriter, r *http.Request) { hLogger := s.logger.With(slog.String("handler", "readRoutine")) @@ -135,7 +134,7 @@ func (s *Service) readRoutine(w http.ResponseWriter, r *http.Request) { http.Error(w, routineNameNotSpecifiedMsg, http.StatusBadRequest) return } - routine, ok := s.config.BackupRoutines[routineName] + routine, ok := s.config.GetBackupConfig().BackupRoutines[routineName] if !ok { http.Error(w, fmt.Sprintf("Routine %s could not be found", routineName), http.StatusNotFound) return @@ -190,7 +189,7 @@ func (s *Service) updateRoutine(w http.ResponseWriter, r *http.Request) { return } - toModel, err := updatedRoutine.ToModel(s.config, s.nsValidator) + toModel, err := updatedRoutine.ToModel(s.config.GetBackupConfig(), s.nsValidator) if err != nil { hLogger.Error("failed to create routine", slog.String("name", name), diff --git a/internal/server/handlers/config_storage.go b/internal/server/handlers/config_storage.go index 9f055c04..5e56aba5 100644 --- a/internal/server/handlers/config_storage.go +++ b/internal/server/handlers/config_storage.go @@ -82,7 +82,8 @@ func (s *Service) addStorage(w http.ResponseWriter, r *http.Request) { func (s *Service) ReadAllStorage(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadAllStorage")) - toDTO := dto.ConvertStorageMapToDTO(s.config.Storage, s.config) + backupConfig := s.config.GetBackupConfig() + toDTO := dto.ConvertStorageMapToDTO(backupConfig.Storage, backupConfig) jsonResponse, err := dto.Serialize(toDTO, dto.JSON) if err != nil { hLogger.Error("failed to marshal storage", @@ -113,8 +114,6 @@ func (s *Service) ReadAllStorage(w http.ResponseWriter, _ *http.Request) { // @Response 400 {string} string // @Failure 404 {string} string "The specified storage could not be found" // @Failure 500 {string} string -// -//nolint:dupl func (s *Service) readStorage(w http.ResponseWriter, r *http.Request) { hLogger := s.logger.With(slog.String("handler", "readStorage")) @@ -124,13 +123,14 @@ func (s *Service) readStorage(w http.ResponseWriter, r *http.Request) { http.Error(w, storageNameNotSpecifiedMsg, http.StatusBadRequest) return } - storage, ok := s.config.Storage[storageName] + backupConfig := s.config.GetBackupConfig() + storage, ok := backupConfig.Storage[storageName] if !ok { http.Error(w, fmt.Sprintf("Storage %s could not be found", storageName), http.StatusNotFound) return } - jsonResponse, err := dto.Serialize(dto.NewStorageFromModel(storage, s.config), dto.JSON) + jsonResponse, err := dto.Serialize(dto.NewStorageFromModel(storage, backupConfig), dto.JSON) if err != nil { hLogger.Error("failed to marshal storage", slog.Any("error", err), diff --git a/internal/server/handlers/handlers_test.go b/internal/server/handlers/handlers_test.go index 04e1566e..a69d84ba 100644 --- a/internal/server/handlers/handlers_test.go +++ b/internal/server/handlers/handlers_test.go @@ -219,7 +219,7 @@ func (m *MockNamespaceValidator) MissingNamespaces(_ *model.AerospikeCluster, _ return nil } -func (m *MockNamespaceValidator) ValidateRoutines(_ *model.AerospikeCluster, _ *model.Config) error { +func (m *MockNamespaceValidator) ValidateRoutines(_ *model.AerospikeCluster, _ map[string]*model.BackupRoutine) error { return nil } diff --git a/pkg/dto/aerospike_cluster.go b/pkg/dto/aerospike_cluster.go index bbe79ce6..14203ec0 100644 --- a/pkg/dto/aerospike_cluster.go +++ b/pkg/dto/aerospike_cluster.go @@ -63,7 +63,7 @@ func NewClusterFromReader(r io.Reader, format SerializationFormat) (*AerospikeCl return a, nil } -func NewClusterFromModel(m *model.AerospikeCluster, config *model.Config) *AerospikeCluster { +func NewClusterFromModel(m *model.AerospikeCluster, config *model.BackupConfig) *AerospikeCluster { if m == nil { return nil } @@ -73,7 +73,7 @@ func NewClusterFromModel(m *model.AerospikeCluster, config *model.Config) *Aeros return a } -func (a *AerospikeCluster) fromModel(m *model.AerospikeCluster, config *model.Config) { +func (a *AerospikeCluster) fromModel(m *model.AerospikeCluster, config *model.BackupConfig) { a.ClusterLabel = m.ClusterLabel a.SeedNodes = make([]SeedNode, len(m.SeedNodes)) for i, v := range m.SeedNodes { @@ -181,7 +181,7 @@ type Credentials struct { AuthMode *string `yaml:"auth-mode,omitempty" json:"auth-mode,omitempty" enums:"INTERNAL,EXTERNAL,PKI"` } -func (c *Credentials) fromModel(m *model.Credentials, config *model.Config) { +func (c *Credentials) fromModel(m *model.Credentials, config *model.BackupConfig) { c.User = m.User c.Password = m.Password c.PasswordPath = m.PasswordPath diff --git a/pkg/dto/backup_details.go b/pkg/dto/backup_details.go index d8010f25..c2019d99 100644 --- a/pkg/dto/backup_details.go +++ b/pkg/dto/backup_details.go @@ -35,7 +35,18 @@ type BackupMetadata struct { UDFCount uint64 `yaml:"udf-count" json:"udf-count" format:"int64" example:"2"` } -func (d *BackupDetails) fromModel(m *model.BackupDetails, config *model.Config) { +// NewBackupDetailsFromModel creates a new BackupDetails from a model.BackupDetails +func NewBackupDetailsFromModel(m *model.BackupDetails, config *model.BackupConfig) *BackupDetails { + if m == nil { + return nil + } + + var d BackupDetails + d.fromModel(m, config) + return &d +} + +func (d *BackupDetails) fromModel(m *model.BackupDetails, config *model.BackupConfig) { d.Key = m.Key d.Created = m.Created d.From = m.From @@ -48,19 +59,8 @@ func (d *BackupDetails) fromModel(m *model.BackupDetails, config *model.Config) d.Storage = NewStorageFromModel(m.Storage, config) } -// NewBackupDetailsFromModel creates a new BackupDetails from a model.BackupDetails -func NewBackupDetailsFromModel(m *model.BackupDetails, config *model.Config) *BackupDetails { - if m == nil { - return nil - } - - var d BackupDetails - d.fromModel(m, config) - return &d -} - func ConvertBackupDetailsMap( - modelMap map[string][]model.BackupDetails, config *model.Config, + modelMap map[string][]model.BackupDetails, config *model.BackupConfig, ) map[string][]BackupDetails { result := make(map[string][]BackupDetails, len(modelMap)) for key, modelSlice := range modelMap { diff --git a/pkg/dto/backup_routine.go b/pkg/dto/backup_routine.go index c90b7458..73bde8d1 100644 --- a/pkg/dto/backup_routine.go +++ b/pkg/dto/backup_routine.go @@ -84,7 +84,7 @@ func (r *BackupRoutine) Validate() error { } func (r *BackupRoutine) ToModel( - config *model.Config, + config *model.BackupConfig, nsValidator aerospike.NamespaceValidator, ) (*model.BackupRoutine, error) { policy, found := config.BackupPolicies[r.BackupPolicy] @@ -158,11 +158,11 @@ func NewRoutineFromModel(m *model.BackupRoutine, config *model.Config) *BackupRo } b := &BackupRoutine{} - b.fromModel(m, config) + b.fromModel(m, config.GetBackupConfig()) return b } -func (r *BackupRoutine) fromModel(m *model.BackupRoutine, config *model.Config) { +func (r *BackupRoutine) fromModel(m *model.BackupRoutine, config *model.BackupConfig) { r.BackupPolicy = findKeyByValue(config.BackupPolicies, m.BackupPolicy) r.SourceCluster = findKeyByValue(config.AerospikeClusters, m.SourceCluster) r.Storage = findStorageKey(config.Storage, m.Storage) diff --git a/pkg/dto/base_dto.go b/pkg/dto/base_dto.go index 128fd891..b92697d2 100644 --- a/pkg/dto/base_dto.go +++ b/pkg/dto/base_dto.go @@ -84,7 +84,7 @@ func ConvertModelMapToDTO[M any, D any](modelMap map[string]*M, dtoConstructor f } // ConvertStorageMapToDTO converts a map of models to a map of DTOs -func ConvertStorageMapToDTO(modelMap map[string]model.Storage, config *model.Config) map[string]*Storage { +func ConvertStorageMapToDTO(modelMap map[string]model.Storage, config *model.BackupConfig) map[string]*Storage { result := make(map[string]*Storage, len(modelMap)) for key, s := range modelMap { result[key] = NewStorageFromModel(s, config) diff --git a/pkg/dto/config.go b/pkg/dto/config.go index 81684896..d9ffdb97 100644 --- a/pkg/dto/config.go +++ b/pkg/dto/config.go @@ -33,29 +33,30 @@ func NewConfigFromModel(m *model.Config) *Config { func (c *Config) fromModel(m *model.Config) { c.ServiceConfig.fromModel(&m.ServiceConfig) + backupConfig := m.GetBackupConfig() c.AerospikeClusters = make(map[string]*AerospikeCluster) - for name, a := range m.AerospikeClusters { - c.AerospikeClusters[name] = NewClusterFromModel(a, m) + for name, a := range backupConfig.AerospikeClusters { + c.AerospikeClusters[name] = NewClusterFromModel(a, backupConfig) } c.Storage = make(map[string]*Storage) - for name, s := range m.Storage { - c.Storage[name] = NewStorageFromModel(s, m) + for name, s := range backupConfig.Storage { + c.Storage[name] = NewStorageFromModel(s, backupConfig) } c.BackupPolicies = make(map[string]*BackupPolicy) - for name, p := range m.BackupPolicies { + for name, p := range backupConfig.BackupPolicies { c.BackupPolicies[name] = NewBackupPolicyFromModel(p) } c.BackupRoutines = make(map[string]*BackupRoutine) - for name, r := range m.BackupRoutines { + for name, r := range backupConfig.BackupRoutines { c.BackupRoutines[name] = NewRoutineFromModel(r, m) } c.SecretAgents = make(map[string]*SecretAgent) - for name, s := range m.SecretAgents { + for name, s := range backupConfig.SecretAgents { c.SecretAgents[name] = newSecretAgentFromModel(s) } } @@ -172,9 +173,10 @@ func (c *Config) ToModel(nsValidator aerospike.NamespaceValidator) (*model.Confi } } + backupConfig := modelConfig.GetBackupConfig() // routines must be added after storage, secret agents and policies. for k, v := range c.BackupRoutines { - toModel, err := v.ToModel(modelConfig, nsValidator) + toModel, err := v.ToModel(backupConfig, nsValidator) if err != nil { return nil, fmt.Errorf("invalid backup routine %q: %w", k, err) } diff --git a/pkg/dto/config_test.go b/pkg/dto/config_test.go index 5f40a6c1..410f179b 100644 --- a/pkg/dto/config_test.go +++ b/pkg/dto/config_test.go @@ -64,7 +64,7 @@ func (m *MockNamespaceValidator) MissingNamespaces(_ *model.AerospikeCluster, _ return nil } -func (m *MockNamespaceValidator) ValidateRoutines(_ *model.AerospikeCluster, _ *model.Config) error { +func (m *MockNamespaceValidator) ValidateRoutines(_ *model.AerospikeCluster, _ map[string]*model.BackupRoutine) error { return nil } diff --git a/pkg/dto/restore_request.go b/pkg/dto/restore_request.go index 7d4baafc..dc78fa6b 100644 --- a/pkg/dto/restore_request.go +++ b/pkg/dto/restore_request.go @@ -76,7 +76,7 @@ func (r *RestoreTimestampRequest) ToModel(config *model.Config) (*model.RestoreT if err != nil { return nil, fmt.Errorf("invalid cluster: %w", err) } - if _, ok := config.BackupRoutines[r.Routine]; !ok { + if _, ok := config.GetBackupConfig().BackupRoutines[r.Routine]; !ok { return nil, notFoundValidationError("routine", r.Routine) } diff --git a/pkg/dto/secret_agent.go b/pkg/dto/secret_agent.go index a9d827aa..88ee3379 100644 --- a/pkg/dto/secret_agent.go +++ b/pkg/dto/secret_agent.go @@ -66,7 +66,7 @@ func (s *SecretAgent) ToModel() *model.SecretAgent { } } -func ResolveSecretAgentFromModel(s *model.SecretAgent, config *model.Config) SecretAgentConfig { +func ResolveSecretAgentFromModel(s *model.SecretAgent, config *model.BackupConfig) SecretAgentConfig { secretAgentName := findKeyByValue(config.SecretAgents, s) if secretAgentName != "" { return SecretAgentConfig{ diff --git a/pkg/dto/storage.go b/pkg/dto/storage.go index d3a4488d..1d687218 100644 --- a/pkg/dto/storage.go +++ b/pkg/dto/storage.go @@ -75,7 +75,7 @@ func (s *Storage) ToModel(c *model.Config) (model.Storage, error) { } // NewStorageFromModel creates a new Storage DTO from the model. -func NewStorageFromModel(m model.Storage, config *model.Config) *Storage { +func NewStorageFromModel(m model.Storage, config *model.BackupConfig) *Storage { switch s := m.(type) { case *model.LocalStorage: return &Storage{ diff --git a/pkg/dto/storage_azure.go b/pkg/dto/storage_azure.go index c3b35cf1..06f28a1c 100644 --- a/pkg/dto/storage_azure.go +++ b/pkg/dto/storage_azure.go @@ -84,7 +84,7 @@ func getAzureAuth(a *AzureStorage) model.AzureAuth { return nil } -func newAzureStorageFromModel(s *model.AzureStorage, config *model.Config) *AzureStorage { +func newAzureStorageFromModel(s *model.AzureStorage, config *model.BackupConfig) *AzureStorage { azureStorage := &AzureStorage{ Endpoint: s.Endpoint, ContainerName: s.ContainerName, diff --git a/pkg/dto/storage_gcp.go b/pkg/dto/storage_gcp.go index 92f900f7..01dd1de5 100644 --- a/pkg/dto/storage_gcp.go +++ b/pkg/dto/storage_gcp.go @@ -50,7 +50,7 @@ func (s *GcpStorage) toModel(config *model.Config) (model.Storage, error) { }, nil } -func newGcpStorageFromModel(s *model.GcpStorage, config *model.Config) *GcpStorage { +func newGcpStorageFromModel(s *model.GcpStorage, config *model.BackupConfig) *GcpStorage { return &GcpStorage{ KeyFile: s.KeyFile, BucketName: s.BucketName, diff --git a/pkg/dto/storage_s3.go b/pkg/dto/storage_s3.go index c50605b4..d685dc95 100644 --- a/pkg/dto/storage_s3.go +++ b/pkg/dto/storage_s3.go @@ -83,7 +83,7 @@ func (s *S3Storage) toModel(config *model.Config) (*model.S3Storage, error) { }, nil } -func newS3StorageFromModel(s *model.S3Storage, config *model.Config) *S3Storage { +func newS3StorageFromModel(s *model.S3Storage, config *model.BackupConfig) *S3Storage { result := &S3Storage{ Bucket: s.Bucket, Path: s.Path, diff --git a/pkg/model/config.go b/pkg/model/config.go index 51cb2149..39571d94 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -8,7 +8,11 @@ import ( // Config represents the service configuration. type Config struct { sync.RWMutex - ServiceConfig BackupServiceConfig + backupConfig BackupConfig + ServiceConfig BackupServiceConfig +} + +type BackupConfig struct { AerospikeClusters map[string]*AerospikeCluster Storage map[string]Storage // Storage is an interface BackupPolicies map[string]*BackupPolicy @@ -16,8 +20,40 @@ type Config struct { SecretAgents map[string]*SecretAgent } -func NewConfig() *Config { - return &Config{ +func (bc *BackupConfig) copy() *BackupConfig { + if bc == nil { + return nil + } + + newConfig := &BackupConfig{ + AerospikeClusters: make(map[string]*AerospikeCluster, len(bc.AerospikeClusters)), + Storage: make(map[string]Storage, len(bc.Storage)), + BackupPolicies: make(map[string]*BackupPolicy, len(bc.BackupPolicies)), + BackupRoutines: make(map[string]*BackupRoutine, len(bc.BackupRoutines)), + SecretAgents: make(map[string]*SecretAgent, len(bc.SecretAgents)), + } + + for k, v := range bc.AerospikeClusters { + newConfig.AerospikeClusters[k] = v + } + for k, v := range bc.Storage { + newConfig.Storage[k] = v + } + for k, v := range bc.BackupPolicies { + newConfig.BackupPolicies[k] = v + } + for k, v := range bc.BackupRoutines { + newConfig.BackupRoutines[k] = v + } + for k, v := range bc.SecretAgents { + newConfig.SecretAgents[k] = v + } + + return newConfig +} + +func newBackupConfig() *BackupConfig { + return &BackupConfig{ AerospikeClusters: make(map[string]*AerospikeCluster), Storage: make(map[string]Storage), BackupPolicies: make(map[string]*BackupPolicy), @@ -26,20 +62,32 @@ func NewConfig() *Config { } } +func NewConfig() *Config { + return &Config{ + backupConfig: *newBackupConfig(), + } +} + var ( ErrAlreadyExists = fmt.Errorf("item already exists") ErrNotFound = fmt.Errorf("item not found") ErrInUse = fmt.Errorf("item is in use") ) +func (c *Config) GetBackupConfig() *BackupConfig { + c.RLock() + defer c.RUnlock() + return c.backupConfig.copy() +} + func (c *Config) AddStorage(name string, s Storage) error { c.Lock() defer c.Unlock() - if _, exists := c.Storage[name]; exists { + if _, exists := c.backupConfig.Storage[name]; exists { return fmt.Errorf("add storage %q: %w", name, ErrAlreadyExists) } - c.Storage[name] = s + c.backupConfig.Storage[name] = s return nil } @@ -47,14 +95,14 @@ func (c *Config) DeleteStorage(name string) error { c.Lock() defer c.Unlock() - s, exists := c.Storage[name] + s, exists := c.backupConfig.Storage[name] if !exists { return fmt.Errorf("delete storage %q: %w", name, ErrNotFound) } if routine := c.routineUsesStorage(s); routine != "" { return fmt.Errorf("delete storage %q: %w: it is used in routine %q", name, ErrInUse, routine) } - delete(c.Storage, name) + delete(c.backupConfig.Storage, name) return nil } @@ -62,24 +110,24 @@ func (c *Config) UpdateStorage(name string, s Storage) error { c.Lock() defer c.Unlock() - if _, exists := c.Storage[name]; !exists { + if _, exists := c.backupConfig.Storage[name]; !exists { return fmt.Errorf("update storage %q: %w", name, ErrNotFound) } - oldStorage := c.Storage[name] - for _, r := range c.BackupRoutines { + oldStorage := c.backupConfig.Storage[name] + for _, r := range c.backupConfig.BackupRoutines { if r.Storage == oldStorage { r.Storage = s } } - c.Storage[name] = s + c.backupConfig.Storage[name] = s return nil } func (c *Config) routineUsesStorage(s Storage) string { - for name, r := range c.BackupRoutines { + for name, r := range c.backupConfig.BackupRoutines { if r.Storage == s { return name } @@ -91,10 +139,10 @@ func (c *Config) AddPolicy(name string, p *BackupPolicy) error { c.Lock() defer c.Unlock() - if _, exists := c.BackupPolicies[name]; exists { + if _, exists := c.backupConfig.BackupPolicies[name]; exists { return fmt.Errorf("add backup policy %q: %w", name, ErrAlreadyExists) } - c.BackupPolicies[name] = p + c.backupConfig.BackupPolicies[name] = p return nil } @@ -102,14 +150,14 @@ func (c *Config) DeletePolicy(name string) error { c.Lock() defer c.Unlock() - p, exists := c.BackupPolicies[name] + p, exists := c.backupConfig.BackupPolicies[name] if !exists { return fmt.Errorf("delete backup policy %q: %w", name, ErrNotFound) } if routine := c.routineUsesPolicy(p); routine != "" { return fmt.Errorf("delete backup policy %q: %w: it is used in routine %q", name, ErrInUse, routine) } - delete(c.BackupPolicies, name) + delete(c.backupConfig.BackupPolicies, name) return nil } @@ -117,23 +165,23 @@ func (c *Config) UpdatePolicy(name string, p *BackupPolicy) error { c.Lock() defer c.Unlock() - if _, exists := c.BackupPolicies[name]; !exists { + if _, exists := c.backupConfig.BackupPolicies[name]; !exists { return fmt.Errorf("update backup policy %q: %w", name, ErrNotFound) } - oldPolicy := c.BackupPolicies[name] - for _, r := range c.BackupRoutines { + oldPolicy := c.backupConfig.BackupPolicies[name] + for _, r := range c.backupConfig.BackupRoutines { if r.BackupPolicy == oldPolicy { r.BackupPolicy = p } } - c.BackupPolicies[name] = p + c.backupConfig.BackupPolicies[name] = p return nil } func (c *Config) routineUsesPolicy(p *BackupPolicy) string { - for name, r := range c.BackupRoutines { + for name, r := range c.backupConfig.BackupRoutines { if r.BackupPolicy == p { return name } @@ -145,10 +193,10 @@ func (c *Config) AddRoutine(name string, r *BackupRoutine) error { c.Lock() defer c.Unlock() - if _, exists := c.BackupRoutines[name]; exists { + if _, exists := c.backupConfig.BackupRoutines[name]; exists { return fmt.Errorf("add backup routine %q: %w", name, ErrAlreadyExists) } - c.BackupRoutines[name] = r + c.backupConfig.BackupRoutines[name] = r return nil } @@ -156,10 +204,10 @@ func (c *Config) DeleteRoutine(name string) error { c.Lock() defer c.Unlock() - if _, exists := c.BackupRoutines[name]; !exists { + if _, exists := c.backupConfig.BackupRoutines[name]; !exists { return fmt.Errorf("delete backup routine %q: %w", name, ErrNotFound) } - delete(c.BackupRoutines, name) + delete(c.backupConfig.BackupRoutines, name) return nil } @@ -167,10 +215,10 @@ func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { c.Lock() defer c.Unlock() - if _, exists := c.BackupRoutines[name]; !exists { + if _, exists := c.backupConfig.BackupRoutines[name]; !exists { return fmt.Errorf("update backup routine %q: %w", name, ErrNotFound) } - c.BackupRoutines[name] = r + c.backupConfig.BackupRoutines[name] = r return nil } @@ -178,10 +226,10 @@ func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { c.Lock() defer c.Unlock() - if _, exists := c.AerospikeClusters[name]; exists { + if _, exists := c.backupConfig.AerospikeClusters[name]; exists { return fmt.Errorf("add Aerospike cluster %q: %w", name, ErrAlreadyExists) } - c.AerospikeClusters[name] = cluster + c.backupConfig.AerospikeClusters[name] = cluster return nil } @@ -189,14 +237,14 @@ func (c *Config) DeleteCluster(name string) error { c.Lock() defer c.Unlock() - cluster, exists := c.AerospikeClusters[name] + cluster, exists := c.backupConfig.AerospikeClusters[name] if !exists { return fmt.Errorf("delete Aerospike cluster %q: %w", name, ErrNotFound) } if routine := c.routineUsesCluster(cluster); routine != "" { return fmt.Errorf("delete Aerospike cluster %q: %w: it is used in routine %q", name, ErrInUse, routine) } - delete(c.AerospikeClusters, name) + delete(c.backupConfig.AerospikeClusters, name) return nil } @@ -204,24 +252,24 @@ func (c *Config) UpdateCluster(name string, cluster *AerospikeCluster) error { c.Lock() defer c.Unlock() - if _, exists := c.AerospikeClusters[name]; !exists { + if _, exists := c.backupConfig.AerospikeClusters[name]; !exists { return fmt.Errorf("update Aerospike cluster %q: %w", name, ErrNotFound) } - oldCluster := c.AerospikeClusters[name] - for _, r := range c.BackupRoutines { + oldCluster := c.backupConfig.AerospikeClusters[name] + for _, r := range c.backupConfig.BackupRoutines { if r.SourceCluster == oldCluster { r.SourceCluster = cluster } } - c.AerospikeClusters[name] = cluster + c.backupConfig.AerospikeClusters[name] = cluster return nil } func (c *Config) routineUsesCluster(cluster *AerospikeCluster) string { - for name, r := range c.BackupRoutines { + for name, r := range c.backupConfig.BackupRoutines { if r.SourceCluster == cluster { return name } @@ -233,10 +281,10 @@ func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { c.Lock() defer c.Unlock() - if _, exists := c.SecretAgents[name]; exists { + if _, exists := c.backupConfig.SecretAgents[name]; exists { return fmt.Errorf("add Secret agent %q: %w", name, ErrAlreadyExists) } - c.SecretAgents[name] = agent + c.backupConfig.SecretAgents[name] = agent return nil } @@ -245,12 +293,6 @@ func (c *Config) CopyFrom(other *Config) { defer c.Unlock() other.RLock() defer other.RUnlock() - - c.AerospikeClusters = other.AerospikeClusters - c.Storage = other.Storage - c.BackupPolicies = other.BackupPolicies - c.BackupRoutines = other.BackupRoutines - c.SecretAgents = other.SecretAgents } func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*SecretAgent, error) { @@ -258,7 +300,7 @@ func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*S defer c.RUnlock() if name != nil { - agent, ok := c.SecretAgents[*name] + agent, ok := c.backupConfig.SecretAgents[*name] if !ok { return nil, fmt.Errorf("unknown secret agent %q", *name) } @@ -274,11 +316,11 @@ func (c *Config) ToggleRoutineDisabled(name string, isDisabled bool) error { c.Lock() defer c.Unlock() - _, exists := c.BackupRoutines[name] + _, exists := c.backupConfig.BackupRoutines[name] if !exists { return fmt.Errorf("toggle disable for backup routine %q: %w", name, ErrNotFound) } - c.BackupRoutines[name].Disabled = isDisabled + c.backupConfig.BackupRoutines[name].Disabled = isDisabled return nil } diff --git a/pkg/service/aerospike/namespace_validator.go b/pkg/service/aerospike/namespace_validator.go index 6eabee0f..be63b6ff 100644 --- a/pkg/service/aerospike/namespace_validator.go +++ b/pkg/service/aerospike/namespace_validator.go @@ -20,7 +20,7 @@ type NamespaceValidator interface { MissingNamespaces(cluster *model.AerospikeCluster, namespaces []string) []string // ValidateRoutines verifies that all namespaces referenced in backup routines // exist in their respective clusters. - ValidateRoutines(cluster *model.AerospikeCluster, config *model.Config) error + ValidateRoutines(cluster *model.AerospikeCluster, routines map[string]*model.BackupRoutine) error } type defaultNamespaceValidator struct { @@ -56,10 +56,11 @@ func (nv *defaultNamespaceValidator) MissingNamespaces( return util.MissingElements(namespaces, namespacesInCluster) } -func (nv *defaultNamespaceValidator) ValidateRoutines(cluster *model.AerospikeCluster, config *model.Config) error { +func (nv *defaultNamespaceValidator) ValidateRoutines( + cluster *model.AerospikeCluster, routines map[string]*model.BackupRoutine, +) error { var err error - routines := filterRoutinesByCluster(config.BackupRoutines, cluster) - for routineName, routine := range routines { + for routineName, routine := range filterRoutinesByCluster(routines, cluster) { missingNamespaces := nv.MissingNamespaces(cluster, routine.Namespaces) if len(missingNamespaces) > 0 { err = errors.Join(err, fmt.Errorf("cluster is missing namespaces %v that are used in routine %v", @@ -118,6 +119,6 @@ func (n *NoopNamespaceValidator) MissingNamespaces(_ *model.AerospikeCluster, _ } // ValidateRoutines returns nil, indicating no error in validation. -func (n *NoopNamespaceValidator) ValidateRoutines(_ *model.AerospikeCluster, _ *model.Config) error { +func (n *NoopNamespaceValidator) ValidateRoutines(_ *model.AerospikeCluster, _ map[string]*model.BackupRoutine) error { return nil } diff --git a/pkg/service/backup_scheduler_test.go b/pkg/service/backup_scheduler_test.go index a46cf50e..0f0edaa7 100644 --- a/pkg/service/backup_scheduler_test.go +++ b/pkg/service/backup_scheduler_test.go @@ -27,19 +27,16 @@ func TestDisabledRoutine(t *testing.T) { mockScheduler := new(MockScheduler) mockScheduler.On("ScheduleJob", mock.Anything, mock.Anything).Return(nil) - config := &model.Config{ - BackupRoutines: map[string]*model.BackupRoutine{ - "routine1": {Disabled: true}, - "routine2": {Disabled: false, IntervalCron: "@daily"}, - }, - } + config := model.NewConfig() + _ = config.AddRoutine("routine1", &model.BackupRoutine{Disabled: true}) + _ = config.AddRoutine("routine2", &model.BackupRoutine{IntervalCron: "@daily"}) handlers := BackupHandlerHolder{ "routine1": &BackupRoutineHandler{}, "routine2": &BackupRoutineHandler{lastRun: model.NewLastBackupRun(util.Ptr(time.Now()), nil)}, } - err := scheduleRoutines(mockScheduler, config.BackupRoutines, handlers) + err := scheduleRoutines(mockScheduler, config.GetBackupConfig().BackupRoutines, handlers) require.NoError(t, err) mockScheduler.AssertNumberOfCalls(t, "ScheduleJob", 1) From ef88367f4bfc5c401ba58b36b0bc55ab5c134e53 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 14:36:13 +0200 Subject: [PATCH 13/18] add setConfig method --- internal/server/handlers/config.go | 15 +++++++++++++-- pkg/model/config.go | 5 ++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/internal/server/handlers/config.go b/internal/server/handlers/config.go index f5ef5cac..a38028c7 100644 --- a/internal/server/handlers/config.go +++ b/internal/server/handlers/config.go @@ -71,13 +71,24 @@ func (s *Service) updateConfig(w http.ResponseWriter, r *http.Request) { return } + oldConfig := dto.NewConfigFromModel(s.config) + if err := validation.ValidateStaticFieldChanges(oldConfig, newConfig); err != nil { + hLogger.Error("static configuration has changed", + slog.Any("error", err), + ) + err := fmt.Errorf("static configuration has changed: %w", err) + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + newConfigModel, err := newConfig.ToModel(s.nsValidator) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return } + err = s.changeConfig(r.Context(), func(config *model.Config) error { - config.CopyFrom(newConfigModel) + config.SetBackupConfig(newConfigModel.GetBackupConfig()) return nil }) @@ -124,7 +135,7 @@ func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { return } - s.config.CopyFrom(config) + s.config.SetBackupConfig(config.GetBackupConfig()) err = s.configApplier.ApplyNewRoutines(r.Context(), s.config.GetBackupConfig().BackupRoutines) if err != nil { diff --git a/pkg/model/config.go b/pkg/model/config.go index 39571d94..066f8580 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -288,11 +288,10 @@ func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { return nil } -func (c *Config) CopyFrom(other *Config) { +func (c *Config) SetBackupConfig(other *BackupConfig) { c.Lock() defer c.Unlock() - other.RLock() - defer other.RUnlock() + c.backupConfig = *other } func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*SecretAgent, error) { From 43a97f7fcb83c4b822e1d1c103baec10e06cf438 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 14:49:27 +0200 Subject: [PATCH 14/18] use shortcut Routines() where possible --- cmd/backup/main.go | 2 +- internal/server/handlers/config.go | 7 ++++--- internal/server/handlers/config_cluster.go | 5 +++-- internal/server/handlers/config_routine.go | 5 ++--- pkg/dto/restore_request.go | 2 +- pkg/model/config.go | 12 ++++++++++++ pkg/service/backup_scheduler_test.go | 2 +- 7 files changed, 24 insertions(+), 11 deletions(-) diff --git a/cmd/backup/main.go b/cmd/backup/main.go index 9184b180..3b1da77e 100644 --- a/cmd/backup/main.go +++ b/cmd/backup/main.go @@ -88,7 +88,7 @@ func startService(configFile string, remote bool) error { backupHandlers, ) - err = configApplier.ApplyNewRoutines(ctx, config.GetBackupConfig().BackupRoutines) + err = configApplier.ApplyNewRoutines(ctx, config.Routines()) if err != nil { return fmt.Errorf("failed to apply new config: %w", err) } diff --git a/internal/server/handlers/config.go b/internal/server/handlers/config.go index a38028c7..2b18179f 100644 --- a/internal/server/handlers/config.go +++ b/internal/server/handlers/config.go @@ -135,8 +135,9 @@ func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { return } - s.config.SetBackupConfig(config.GetBackupConfig()) - err = s.configApplier.ApplyNewRoutines(r.Context(), s.config.GetBackupConfig().BackupRoutines) + backupConfig := config.GetBackupConfig() + s.config.SetBackupConfig(backupConfig) + err = s.configApplier.ApplyNewRoutines(r.Context(), backupConfig.BackupRoutines) if err != nil { hLogger.Error("failed to apply config", @@ -160,7 +161,7 @@ func (s *Service) changeConfig(ctx context.Context, updateFunc func(*model.Confi return fmt.Errorf("failed to write configuration: %w", err) } - err = s.configApplier.ApplyNewRoutines(ctx, s.config.GetBackupConfig().BackupRoutines) + err = s.configApplier.ApplyNewRoutines(ctx, s.config.Routines()) if err != nil { return fmt.Errorf("failed to apply new configuration: %w", err) } diff --git a/internal/server/handlers/config_cluster.go b/internal/server/handlers/config_cluster.go index 805c720e..16f9e95a 100644 --- a/internal/server/handlers/config_cluster.go +++ b/internal/server/handlers/config_cluster.go @@ -134,7 +134,8 @@ func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { http.Error(w, clusterNameNotSpecifiedMsg, http.StatusBadRequest) return } - cluster, ok := s.config.GetBackupConfig().AerospikeClusters[clusterName] + backupConfig := s.config.GetBackupConfig() + cluster, ok := backupConfig.AerospikeClusters[clusterName] if !ok { hLogger.Error("cluster not found", slog.String("name", clusterName), @@ -142,7 +143,7 @@ func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { http.Error(w, fmt.Sprintf("cluster %s could not be found", clusterName), http.StatusNotFound) return } - jsonResponse, err := dto.Serialize(dto.NewClusterFromModel(cluster, s.config.GetBackupConfig()), dto.JSON) + jsonResponse, err := dto.Serialize(dto.NewClusterFromModel(cluster, backupConfig), dto.JSON) if err != nil { hLogger.Error("failed to marshal cluster", slog.Any("error", err), diff --git a/internal/server/handlers/config_routine.go b/internal/server/handlers/config_routine.go index 49f621ff..6574f67b 100644 --- a/internal/server/handlers/config_routine.go +++ b/internal/server/handlers/config_routine.go @@ -91,8 +91,7 @@ func (s *Service) addRoutine(w http.ResponseWriter, r *http.Request) { func (s *Service) ReadRoutines(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadRoutines")) - routines := s.config.GetBackupConfig().BackupRoutines - toDTO := dto.ConvertModelMapToDTO(routines, func(m *model.BackupRoutine) *dto.BackupRoutine { + toDTO := dto.ConvertModelMapToDTO(s.config.Routines(), func(m *model.BackupRoutine) *dto.BackupRoutine { return dto.NewRoutineFromModel(m, s.config) }) @@ -134,7 +133,7 @@ func (s *Service) readRoutine(w http.ResponseWriter, r *http.Request) { http.Error(w, routineNameNotSpecifiedMsg, http.StatusBadRequest) return } - routine, ok := s.config.GetBackupConfig().BackupRoutines[routineName] + routine, ok := s.config.Routines()[routineName] if !ok { http.Error(w, fmt.Sprintf("Routine %s could not be found", routineName), http.StatusNotFound) return diff --git a/pkg/dto/restore_request.go b/pkg/dto/restore_request.go index dc78fa6b..b4ae6194 100644 --- a/pkg/dto/restore_request.go +++ b/pkg/dto/restore_request.go @@ -76,7 +76,7 @@ func (r *RestoreTimestampRequest) ToModel(config *model.Config) (*model.RestoreT if err != nil { return nil, fmt.Errorf("invalid cluster: %w", err) } - if _, ok := config.GetBackupConfig().BackupRoutines[r.Routine]; !ok { + if _, ok := config.Routines()[r.Routine]; !ok { return nil, notFoundValidationError("routine", r.Routine) } diff --git a/pkg/model/config.go b/pkg/model/config.go index 066f8580..22b93d5f 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -189,6 +189,18 @@ func (c *Config) routineUsesPolicy(p *BackupPolicy) string { return "" } +func (c *Config) Routines() map[string]*BackupRoutine { + c.RLock() + defer c.RUnlock() + + routines := make(map[string]*BackupRoutine, len(c.backupConfig.BackupRoutines)) + for key, value := range c.backupConfig.BackupRoutines { + routines[key] = value + } + + return routines +} + func (c *Config) AddRoutine(name string, r *BackupRoutine) error { c.Lock() defer c.Unlock() diff --git a/pkg/service/backup_scheduler_test.go b/pkg/service/backup_scheduler_test.go index 0f0edaa7..2b670eef 100644 --- a/pkg/service/backup_scheduler_test.go +++ b/pkg/service/backup_scheduler_test.go @@ -36,7 +36,7 @@ func TestDisabledRoutine(t *testing.T) { "routine2": &BackupRoutineHandler{lastRun: model.NewLastBackupRun(util.Ptr(time.Now()), nil)}, } - err := scheduleRoutines(mockScheduler, config.GetBackupConfig().BackupRoutines, handlers) + err := scheduleRoutines(mockScheduler, config.Routines(), handlers) require.NoError(t, err) mockScheduler.AssertNumberOfCalls(t, "ScheduleJob", 1) From d18fd2ffd58c787a57e3af78e2b976769e33f31c Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 14:55:37 +0200 Subject: [PATCH 15/18] rename to BackupConfigCopy() --- internal/server/handlers/backup.go | 4 ++-- internal/server/handlers/config.go | 4 ++-- internal/server/handlers/config_cluster.go | 6 +++--- internal/server/handlers/config_policy.go | 4 ++-- internal/server/handlers/config_routine.go | 4 ++-- internal/server/handlers/config_storage.go | 4 ++-- pkg/dto/backup_routine.go | 2 +- pkg/dto/config.go | 4 ++-- pkg/model/config.go | 2 +- 9 files changed, 17 insertions(+), 17 deletions(-) diff --git a/internal/server/handlers/backup.go b/internal/server/handlers/backup.go index 0bd91e2a..5f17e242 100644 --- a/internal/server/handlers/backup.go +++ b/internal/server/handlers/backup.go @@ -103,7 +103,7 @@ func (s *Service) readAllBackups(w http.ResponseWriter, r *http.Request, isFullB return } - response, err := dto.Serialize(dto.ConvertBackupDetailsMap(backups, s.config.GetBackupConfig()), dto.JSON) + response, err := dto.Serialize(dto.ConvertBackupDetailsMap(backups, s.config.BackupConfigCopy()), dto.JSON) if err != nil { hLogger.Error("failed to marshal backup list", slog.Any("error", err), @@ -169,7 +169,7 @@ func (s *Service) readBackupsForRoutine(w http.ResponseWriter, r *http.Request, http.Error(w, "failed to retrieve backup list: "+err.Error(), http.StatusInternalServerError) return } - backupConfig := s.config.GetBackupConfig() + backupConfig := s.config.BackupConfigCopy() backupDetails := dto.ConvertModelsToDTO(backups, func(m *model.BackupDetails) *dto.BackupDetails { return dto.NewBackupDetailsFromModel(m, backupConfig) }) diff --git a/internal/server/handlers/config.go b/internal/server/handlers/config.go index 2b18179f..9f251b44 100644 --- a/internal/server/handlers/config.go +++ b/internal/server/handlers/config.go @@ -88,7 +88,7 @@ func (s *Service) updateConfig(w http.ResponseWriter, r *http.Request) { } err = s.changeConfig(r.Context(), func(config *model.Config) error { - config.SetBackupConfig(newConfigModel.GetBackupConfig()) + config.SetBackupConfig(newConfigModel.BackupConfigCopy()) return nil }) @@ -135,7 +135,7 @@ func (s *Service) ApplyConfig(w http.ResponseWriter, r *http.Request) { return } - backupConfig := config.GetBackupConfig() + backupConfig := config.BackupConfigCopy() s.config.SetBackupConfig(backupConfig) err = s.configApplier.ApplyNewRoutines(r.Context(), backupConfig.BackupRoutines) diff --git a/internal/server/handlers/config_cluster.go b/internal/server/handlers/config_cluster.go index 16f9e95a..a9dc67b3 100644 --- a/internal/server/handlers/config_cluster.go +++ b/internal/server/handlers/config_cluster.go @@ -89,7 +89,7 @@ func (s *Service) addAerospikeCluster(w http.ResponseWriter, r *http.Request) { func (s *Service) ReadAerospikeClusters(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadAerospikeClusters")) - backupConfig := s.config.GetBackupConfig() + backupConfig := s.config.BackupConfigCopy() clusters := backupConfig.AerospikeClusters toDTO := dto.ConvertModelMapToDTO(clusters, func(m *model.AerospikeCluster) *dto.AerospikeCluster { return dto.NewClusterFromModel(m, backupConfig) @@ -134,7 +134,7 @@ func (s *Service) readAerospikeCluster(w http.ResponseWriter, r *http.Request) { http.Error(w, clusterNameNotSpecifiedMsg, http.StatusBadRequest) return } - backupConfig := s.config.GetBackupConfig() + backupConfig := s.config.BackupConfigCopy() cluster, ok := backupConfig.AerospikeClusters[clusterName] if !ok { hLogger.Error("cluster not found", @@ -197,7 +197,7 @@ func (s *Service) updateAerospikeCluster(w http.ResponseWriter, r *http.Request) return } - err = s.nsValidator.ValidateRoutines(cluster, s.config.GetBackupConfig().BackupRoutines) + err = s.nsValidator.ValidateRoutines(cluster, s.config.BackupConfigCopy().BackupRoutines) if err != nil { hLogger.Error("cluster namespace validation failed", slog.String("name", clusterName), diff --git a/internal/server/handlers/config_policy.go b/internal/server/handlers/config_policy.go index 8a11c031..b8a17264 100644 --- a/internal/server/handlers/config_policy.go +++ b/internal/server/handlers/config_policy.go @@ -81,7 +81,7 @@ func (s *Service) addPolicy(w http.ResponseWriter, r *http.Request) { func (s *Service) ReadPolicies(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadPolicies")) - policies := dto.ConvertModelMapToDTO(s.config.GetBackupConfig().BackupPolicies, dto.NewBackupPolicyFromModel) + policies := dto.ConvertModelMapToDTO(s.config.BackupConfigCopy().BackupPolicies, dto.NewBackupPolicyFromModel) jsonResponse, err := dto.Serialize(policies, dto.JSON) if err != nil { hLogger.Error("failed to marshal backup policies", @@ -121,7 +121,7 @@ func (s *Service) readPolicy(w http.ResponseWriter, r *http.Request) { http.Error(w, policyNameNotSpecifiedMsg, http.StatusBadRequest) return } - policy, ok := s.config.GetBackupConfig().BackupPolicies[policyName] + policy, ok := s.config.BackupConfigCopy().BackupPolicies[policyName] if !ok { hLogger.Error("policy not found") http.Error(w, fmt.Sprintf("policy %s could not be found", policyName), http.StatusNotFound) diff --git a/internal/server/handlers/config_routine.go b/internal/server/handlers/config_routine.go index 6574f67b..52722de3 100644 --- a/internal/server/handlers/config_routine.go +++ b/internal/server/handlers/config_routine.go @@ -55,7 +55,7 @@ func (s *Service) addRoutine(w http.ResponseWriter, r *http.Request) { http.Error(w, routineNameNotSpecifiedMsg, http.StatusBadRequest) return } - toModel, err := newRoutine.ToModel(s.config.GetBackupConfig(), s.nsValidator) + toModel, err := newRoutine.ToModel(s.config.BackupConfigCopy(), s.nsValidator) if err != nil { hLogger.Error("failed to create routine", slog.String("name", name), @@ -188,7 +188,7 @@ func (s *Service) updateRoutine(w http.ResponseWriter, r *http.Request) { return } - toModel, err := updatedRoutine.ToModel(s.config.GetBackupConfig(), s.nsValidator) + toModel, err := updatedRoutine.ToModel(s.config.BackupConfigCopy(), s.nsValidator) if err != nil { hLogger.Error("failed to create routine", slog.String("name", name), diff --git a/internal/server/handlers/config_storage.go b/internal/server/handlers/config_storage.go index 5e56aba5..a7ccb8df 100644 --- a/internal/server/handlers/config_storage.go +++ b/internal/server/handlers/config_storage.go @@ -82,7 +82,7 @@ func (s *Service) addStorage(w http.ResponseWriter, r *http.Request) { func (s *Service) ReadAllStorage(w http.ResponseWriter, _ *http.Request) { hLogger := s.logger.With(slog.String("handler", "ReadAllStorage")) - backupConfig := s.config.GetBackupConfig() + backupConfig := s.config.BackupConfigCopy() toDTO := dto.ConvertStorageMapToDTO(backupConfig.Storage, backupConfig) jsonResponse, err := dto.Serialize(toDTO, dto.JSON) if err != nil { @@ -123,7 +123,7 @@ func (s *Service) readStorage(w http.ResponseWriter, r *http.Request) { http.Error(w, storageNameNotSpecifiedMsg, http.StatusBadRequest) return } - backupConfig := s.config.GetBackupConfig() + backupConfig := s.config.BackupConfigCopy() storage, ok := backupConfig.Storage[storageName] if !ok { http.Error(w, fmt.Sprintf("Storage %s could not be found", storageName), http.StatusNotFound) diff --git a/pkg/dto/backup_routine.go b/pkg/dto/backup_routine.go index 73bde8d1..80e9099c 100644 --- a/pkg/dto/backup_routine.go +++ b/pkg/dto/backup_routine.go @@ -158,7 +158,7 @@ func NewRoutineFromModel(m *model.BackupRoutine, config *model.Config) *BackupRo } b := &BackupRoutine{} - b.fromModel(m, config.GetBackupConfig()) + b.fromModel(m, config.BackupConfigCopy()) return b } diff --git a/pkg/dto/config.go b/pkg/dto/config.go index d9ffdb97..570aaa85 100644 --- a/pkg/dto/config.go +++ b/pkg/dto/config.go @@ -33,7 +33,7 @@ func NewConfigFromModel(m *model.Config) *Config { func (c *Config) fromModel(m *model.Config) { c.ServiceConfig.fromModel(&m.ServiceConfig) - backupConfig := m.GetBackupConfig() + backupConfig := m.BackupConfigCopy() c.AerospikeClusters = make(map[string]*AerospikeCluster) for name, a := range backupConfig.AerospikeClusters { @@ -173,7 +173,7 @@ func (c *Config) ToModel(nsValidator aerospike.NamespaceValidator) (*model.Confi } } - backupConfig := modelConfig.GetBackupConfig() + backupConfig := modelConfig.BackupConfigCopy() // routines must be added after storage, secret agents and policies. for k, v := range c.BackupRoutines { toModel, err := v.ToModel(backupConfig, nsValidator) diff --git a/pkg/model/config.go b/pkg/model/config.go index 22b93d5f..78457d2a 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -74,7 +74,7 @@ var ( ErrInUse = fmt.Errorf("item is in use") ) -func (c *Config) GetBackupConfig() *BackupConfig { +func (c *Config) BackupConfigCopy() *BackupConfig { c.RLock() defer c.RUnlock() return c.backupConfig.copy() From d901c09bda7974afaba93085b97e18d449f0dbb4 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 15:15:28 +0200 Subject: [PATCH 16/18] private mutex --- internal/server/handlers/config_cluster.go | 2 +- pkg/model/config.go | 74 +++++++++++----------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/internal/server/handlers/config_cluster.go b/internal/server/handlers/config_cluster.go index a9dc67b3..e45bdf4d 100644 --- a/internal/server/handlers/config_cluster.go +++ b/internal/server/handlers/config_cluster.go @@ -197,7 +197,7 @@ func (s *Service) updateAerospikeCluster(w http.ResponseWriter, r *http.Request) return } - err = s.nsValidator.ValidateRoutines(cluster, s.config.BackupConfigCopy().BackupRoutines) + err = s.nsValidator.ValidateRoutines(cluster, s.config.Routines()) if err != nil { hLogger.Error("cluster namespace validation failed", slog.String("name", clusterName), diff --git a/pkg/model/config.go b/pkg/model/config.go index 78457d2a..d2293a9a 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -7,7 +7,7 @@ import ( // Config represents the service configuration. type Config struct { - sync.RWMutex + mu sync.RWMutex backupConfig BackupConfig ServiceConfig BackupServiceConfig } @@ -75,14 +75,14 @@ var ( ) func (c *Config) BackupConfigCopy() *BackupConfig { - c.RLock() - defer c.RUnlock() + c.mu.RLock() + defer c.mu.RUnlock() return c.backupConfig.copy() } func (c *Config) AddStorage(name string, s Storage) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.Storage[name]; exists { return fmt.Errorf("add storage %q: %w", name, ErrAlreadyExists) @@ -92,8 +92,8 @@ func (c *Config) AddStorage(name string, s Storage) error { } func (c *Config) DeleteStorage(name string) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() s, exists := c.backupConfig.Storage[name] if !exists { @@ -107,8 +107,8 @@ func (c *Config) DeleteStorage(name string) error { } func (c *Config) UpdateStorage(name string, s Storage) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.Storage[name]; !exists { return fmt.Errorf("update storage %q: %w", name, ErrNotFound) @@ -136,8 +136,8 @@ func (c *Config) routineUsesStorage(s Storage) string { } func (c *Config) AddPolicy(name string, p *BackupPolicy) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.BackupPolicies[name]; exists { return fmt.Errorf("add backup policy %q: %w", name, ErrAlreadyExists) @@ -147,8 +147,8 @@ func (c *Config) AddPolicy(name string, p *BackupPolicy) error { } func (c *Config) DeletePolicy(name string) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() p, exists := c.backupConfig.BackupPolicies[name] if !exists { @@ -162,8 +162,8 @@ func (c *Config) DeletePolicy(name string) error { } func (c *Config) UpdatePolicy(name string, p *BackupPolicy) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.BackupPolicies[name]; !exists { return fmt.Errorf("update backup policy %q: %w", name, ErrNotFound) @@ -190,8 +190,8 @@ func (c *Config) routineUsesPolicy(p *BackupPolicy) string { } func (c *Config) Routines() map[string]*BackupRoutine { - c.RLock() - defer c.RUnlock() + c.mu.RLock() + defer c.mu.RUnlock() routines := make(map[string]*BackupRoutine, len(c.backupConfig.BackupRoutines)) for key, value := range c.backupConfig.BackupRoutines { @@ -202,8 +202,8 @@ func (c *Config) Routines() map[string]*BackupRoutine { } func (c *Config) AddRoutine(name string, r *BackupRoutine) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.BackupRoutines[name]; exists { return fmt.Errorf("add backup routine %q: %w", name, ErrAlreadyExists) @@ -213,8 +213,8 @@ func (c *Config) AddRoutine(name string, r *BackupRoutine) error { } func (c *Config) DeleteRoutine(name string) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.BackupRoutines[name]; !exists { return fmt.Errorf("delete backup routine %q: %w", name, ErrNotFound) @@ -224,8 +224,8 @@ func (c *Config) DeleteRoutine(name string) error { } func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.BackupRoutines[name]; !exists { return fmt.Errorf("update backup routine %q: %w", name, ErrNotFound) @@ -235,8 +235,8 @@ func (c *Config) UpdateRoutine(name string, r *BackupRoutine) error { } func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.AerospikeClusters[name]; exists { return fmt.Errorf("add Aerospike cluster %q: %w", name, ErrAlreadyExists) @@ -246,8 +246,8 @@ func (c *Config) AddCluster(name string, cluster *AerospikeCluster) error { } func (c *Config) DeleteCluster(name string) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() cluster, exists := c.backupConfig.AerospikeClusters[name] if !exists { @@ -261,8 +261,8 @@ func (c *Config) DeleteCluster(name string) error { } func (c *Config) UpdateCluster(name string, cluster *AerospikeCluster) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.AerospikeClusters[name]; !exists { return fmt.Errorf("update Aerospike cluster %q: %w", name, ErrNotFound) @@ -290,8 +290,8 @@ func (c *Config) routineUsesCluster(cluster *AerospikeCluster) string { } func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() if _, exists := c.backupConfig.SecretAgents[name]; exists { return fmt.Errorf("add Secret agent %q: %w", name, ErrAlreadyExists) @@ -301,14 +301,14 @@ func (c *Config) AddSecretAgent(name string, agent *SecretAgent) error { } func (c *Config) SetBackupConfig(other *BackupConfig) { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() c.backupConfig = *other } func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*SecretAgent, error) { - c.RLock() - defer c.RUnlock() + c.mu.RLock() + defer c.mu.RUnlock() if name != nil { agent, ok := c.backupConfig.SecretAgents[*name] @@ -324,8 +324,8 @@ func (c *Config) ResolveSecretAgent(name *string, defaultAgent *SecretAgent) (*S // ToggleRoutineDisabled sets the Disabled field of the BackupRoutine based on the provided state. func (c *Config) ToggleRoutineDisabled(name string, isDisabled bool) error { - c.Lock() - defer c.Unlock() + c.mu.Lock() + defer c.mu.Unlock() _, exists := c.backupConfig.BackupRoutines[name] if !exists { From 8feec3f0462300a78c57f24976c49add73cbcc70 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Mon, 30 Dec 2024 15:19:13 +0200 Subject: [PATCH 17/18] use maps.Copy --- pkg/model/config.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/pkg/model/config.go b/pkg/model/config.go index d2293a9a..33c9ec12 100644 --- a/pkg/model/config.go +++ b/pkg/model/config.go @@ -2,6 +2,7 @@ package model import ( "fmt" + "maps" "sync" ) @@ -33,21 +34,11 @@ func (bc *BackupConfig) copy() *BackupConfig { SecretAgents: make(map[string]*SecretAgent, len(bc.SecretAgents)), } - for k, v := range bc.AerospikeClusters { - newConfig.AerospikeClusters[k] = v - } - for k, v := range bc.Storage { - newConfig.Storage[k] = v - } - for k, v := range bc.BackupPolicies { - newConfig.BackupPolicies[k] = v - } - for k, v := range bc.BackupRoutines { - newConfig.BackupRoutines[k] = v - } - for k, v := range bc.SecretAgents { - newConfig.SecretAgents[k] = v - } + maps.Copy(newConfig.AerospikeClusters, bc.AerospikeClusters) + maps.Copy(newConfig.Storage, bc.Storage) + maps.Copy(newConfig.BackupRoutines, bc.BackupRoutines) + maps.Copy(newConfig.BackupPolicies, bc.BackupPolicies) + maps.Copy(newConfig.SecretAgents, bc.SecretAgents) return newConfig } @@ -194,9 +185,7 @@ func (c *Config) Routines() map[string]*BackupRoutine { defer c.mu.RUnlock() routines := make(map[string]*BackupRoutine, len(c.backupConfig.BackupRoutines)) - for key, value := range c.backupConfig.BackupRoutines { - routines[key] = value - } + maps.Copy(routines, c.backupConfig.BackupRoutines) return routines } From 80da7c6fb1ddb9538dff172cf7008dfc64ea5c04 Mon Sep 17 00:00:00 2001 From: akorotkov Date: Tue, 31 Dec 2024 15:01:20 +0200 Subject: [PATCH 18/18] remove validation for old config --- internal/server/handlers/config.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/internal/server/handlers/config.go b/internal/server/handlers/config.go index 9f251b44..e0206d9c 100644 --- a/internal/server/handlers/config.go +++ b/internal/server/handlers/config.go @@ -71,16 +71,6 @@ func (s *Service) updateConfig(w http.ResponseWriter, r *http.Request) { return } - oldConfig := dto.NewConfigFromModel(s.config) - if err := validation.ValidateStaticFieldChanges(oldConfig, newConfig); err != nil { - hLogger.Error("static configuration has changed", - slog.Any("error", err), - ) - err := fmt.Errorf("static configuration has changed: %w", err) - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - newConfigModel, err := newConfig.ToModel(s.nsValidator) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest)