Skip to content

Commit 93e00e6

Browse files
aphralGaphralG
committed
fix test and race
1 parent f035a05 commit 93e00e6

File tree

9 files changed

+48
-32
lines changed

9 files changed

+48
-32
lines changed

‎internal/file/file_manager_service.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ type (
8888
)
8989

9090
type FileManagerService struct {
91+
manifestLock *sync.RWMutex
9192
agentConfig *config.Config
9293
fileOperator fileOperator
9394
fileServiceOperator fileServiceOperatorInterface
@@ -100,10 +101,11 @@ type FileManagerService struct {
100101
previousManifestFiles map[string]*model.ManifestFile
101102
manifestFilePath string
102103
filesMutex sync.RWMutex
103-
manifestLock *sync.RWMutex
104104
}
105105

106-
func NewFileManagerService(fileServiceClient mpi.FileServiceClient, agentConfig *config.Config, manifestLock *sync.RWMutex) *FileManagerService {
106+
func NewFileManagerService(fileServiceClient mpi.FileServiceClient, agentConfig *config.Config,
107+
manifestLock *sync.RWMutex,
108+
) *FileManagerService {
107109
return &FileManagerService{
108110
agentConfig: agentConfig,
109111
fileOperator: NewFileOperator(manifestLock),
@@ -431,6 +433,7 @@ func (fms *FileManagerService) UpdateManifestFile(currentFiles map[string]*mpi.F
431433
if readError != nil && !errors.Is(readError, os.ErrNotExist) {
432434
slog.Debug("Error reading manifest file", "current_manifest_files",
433435
currentManifestFiles, "updated_files", currentFiles, "referenced", referenced)
436+
434437
return fmt.Errorf("unable to read manifest file: %w", readError)
435438
}
436439

@@ -476,6 +479,7 @@ func (fms *FileManagerService) manifestFile() (map[string]*model.ManifestFile, m
476479
if len(file) == 0 {
477480
return nil, nil, fmt.Errorf("manifest file is empty: %w", err)
478481
}
482+
479483
return nil, nil, fmt.Errorf("failed to parse manifest file: %w", err)
480484
}
481485

‎internal/file/file_manager_service_test.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"fmt"
1212
"os"
1313
"path/filepath"
14+
"sync"
1415
"testing"
1516

1617
"github.com/nginx/agent/v3/internal/model"
@@ -54,7 +55,7 @@ func TestFileManagerService_ConfigApply_Add(t *testing.T) {
5455
agentConfig := types.AgentConfig()
5556
agentConfig.AllowedDirectories = []string{tempDir}
5657

57-
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig)
58+
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig, &sync.RWMutex{})
5859
fileManagerService.agentConfig.ManifestDir = manifestDirPath
5960
fileManagerService.manifestFilePath = manifestFilePath
6061

@@ -101,7 +102,7 @@ func TestFileManagerService_ConfigApply_Add_LargeFile(t *testing.T) {
101102
fakeFileServiceClient.GetFileStreamReturns(fakeServerStreamingClient, nil)
102103
agentConfig := types.AgentConfig()
103104
agentConfig.AllowedDirectories = []string{tempDir}
104-
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig)
105+
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig, &sync.RWMutex{})
105106
fileManagerService.agentConfig.ManifestDir = manifestDirPath
106107
fileManagerService.manifestFilePath = manifestFilePath
107108

@@ -160,7 +161,7 @@ func TestFileManagerService_ConfigApply_Update(t *testing.T) {
160161
agentConfig := types.AgentConfig()
161162
agentConfig.AllowedDirectories = []string{tempDir}
162163

163-
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig)
164+
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig, &sync.RWMutex{})
164165
fileManagerService.agentConfig.ManifestDir = manifestDirPath
165166
fileManagerService.manifestFilePath = manifestFilePath
166167
err := fileManagerService.UpdateCurrentFilesOnDisk(ctx, filesOnDisk, false)
@@ -209,7 +210,7 @@ func TestFileManagerService_ConfigApply_Delete(t *testing.T) {
209210
agentConfig := types.AgentConfig()
210211
agentConfig.AllowedDirectories = []string{tempDir}
211212

212-
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig)
213+
fileManagerService := NewFileManagerService(fakeFileServiceClient, agentConfig, &sync.RWMutex{})
213214
fileManagerService.agentConfig.ManifestDir = manifestDirPath
214215
fileManagerService.manifestFilePath = manifestFilePath
215216
err := fileManagerService.UpdateCurrentFilesOnDisk(ctx, filesOnDisk, false)
@@ -247,7 +248,7 @@ func TestFileManagerService_ConfigApply_Delete(t *testing.T) {
247248

248249
func TestFileManagerService_checkAllowedDirectory(t *testing.T) {
249250
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
250-
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig())
251+
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})
251252

252253
allowedFiles := []*mpi.File{
253254
{
@@ -281,7 +282,7 @@ func TestFileManagerService_checkAllowedDirectory(t *testing.T) {
281282

282283
func TestFileManagerService_ClearCache(t *testing.T) {
283284
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
284-
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig())
285+
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})
285286

286287
filesCache := map[string]*model.FileCache{
287288
"file/path/test.conf": {
@@ -394,7 +395,7 @@ func TestFileManagerService_Rollback(t *testing.T) {
394395

395396
instanceID := protos.NginxOssInstance([]string{}).GetInstanceMeta().GetInstanceId()
396397
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
397-
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig())
398+
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})
398399
fileManagerService.rollbackFileContents = fileContentCache
399400
fileManagerService.fileActions = filesCache
400401
fileManagerService.agentConfig.ManifestDir = manifestDirPath
@@ -576,7 +577,7 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
576577
manifestFilePath := manifestFile.Name()
577578

578579
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
579-
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig())
580+
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})
580581
fileManagerService.agentConfig.ManifestDir = manifestDirPath
581582
fileManagerService.manifestFilePath = manifestFilePath
582583

@@ -597,7 +598,7 @@ func TestFileManagerService_DetermineFileActions(t *testing.T) {
597598
func CreateTestManifestFile(t testing.TB, tempDir string, currentFiles map[string]*mpi.File) *os.File {
598599
t.Helper()
599600
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
600-
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig())
601+
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})
601602
manifestFiles := fileManagerService.convertToManifestFileMap(currentFiles, true)
602603
manifestJSON, err := json.MarshalIndent(manifestFiles, "", " ")
603604
require.NoError(t, err)
@@ -685,7 +686,7 @@ func TestFileManagerService_fileActions(t *testing.T) {
685686
Contents: newFileContent,
686687
},
687688
}, nil)
688-
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig())
689+
fileManagerService := NewFileManagerService(fakeFileServiceClient, types.AgentConfig(), &sync.RWMutex{})
689690

690691
fileManagerService.fileActions = filesCache
691692

‎internal/file/file_operator_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"os"
1111
"path/filepath"
12+
"sync"
1213
"testing"
1314

1415
"github.com/nginx/agent/v3/pkg/files"
@@ -28,7 +29,7 @@ func TestFileOperator_Write(t *testing.T) {
2829
fileContent, err := os.ReadFile("../../test/config/nginx/nginx.conf")
2930
require.NoError(t, err)
3031
defer helpers.RemoveFileWithErrorCheck(t, filePath)
31-
fileOp := NewFileOperator()
32+
fileOp := NewFileOperator(&sync.RWMutex{})
3233

3334
fileMeta := protos.FileMeta(filePath, files.GenerateHash(fileContent))
3435

‎internal/file/file_plugin.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ var _ bus.Plugin = (*FilePlugin)(nil)
2828
// the file plugin does not care about the instance type
2929

3030
type FilePlugin struct {
31+
manifestLock *sync.RWMutex
3132
messagePipe bus.MessagePipeInterface
3233
config *config.Config
3334
conn grpc.GrpcConnectionInterface
3435
fileManagerService fileManagerServiceInterface
3536
serverType model.ServerType
36-
manifestLock *sync.RWMutex
3737
}
3838

3939
func NewFilePlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface,

‎internal/file/file_plugin_test.go‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"fmt"
1111
"os"
12+
"sync"
1213
"testing"
1314
"time"
1415

@@ -31,22 +32,24 @@ import (
3132
)
3233

3334
func TestFilePlugin_Info(t *testing.T) {
34-
filePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command)
35+
filePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{},
36+
model.Command, &sync.RWMutex{})
3537
assert.Equal(t, "file", filePlugin.Info().Name)
3638
}
3739

3840
func TestFilePlugin_Close(t *testing.T) {
3941
ctx := context.Background()
4042
fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{}
4143

42-
filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command)
44+
filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command, &sync.RWMutex{})
4345
filePlugin.Close(ctx)
4446

4547
assert.Equal(t, 1, fakeGrpcConnection.CloseCallCount())
4648
}
4749

4850
func TestFilePlugin_Subscriptions(t *testing.T) {
49-
filePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Command)
51+
filePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{},
52+
model.Command, &sync.RWMutex{})
5053
assert.Equal(
5154
t,
5255
[]string{
@@ -62,7 +65,8 @@ func TestFilePlugin_Subscriptions(t *testing.T) {
6265
filePlugin.Subscriptions(),
6366
)
6467

65-
readOnlyFilePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{}, model.Auxiliary)
68+
readOnlyFilePlugin := NewFilePlugin(types.AgentConfig(), &grpcfakes.FakeGrpcConnectionInterface{},
69+
model.Auxiliary, &sync.RWMutex{})
6670
assert.Equal(t, []string{
6771
bus.ConnectionResetTopic,
6872
bus.ConnectionCreatedTopic,
@@ -93,7 +97,7 @@ func TestFilePlugin_Process_NginxConfigUpdateTopic(t *testing.T) {
9397
fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient)
9498
messagePipe := busfakes.NewFakeMessagePipe()
9599

96-
filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command)
100+
filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command, &sync.RWMutex{})
97101
err := filePlugin.Init(ctx, messagePipe)
98102
require.NoError(t, err)
99103

@@ -168,7 +172,7 @@ func TestFilePlugin_Process_ConfigApplyRequestTopic(t *testing.T) {
168172
fakeFileManagerService := &filefakes.FakeFileManagerServiceInterface{}
169173
fakeFileManagerService.ConfigApplyReturns(test.configApplyStatus, test.configApplyReturnsErr)
170174
messagePipe := busfakes.NewFakeMessagePipe()
171-
filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command)
175+
filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command, &sync.RWMutex{})
172176
err := filePlugin.Init(ctx, messagePipe)
173177
filePlugin.fileManagerService = fakeFileManagerService
174178
require.NoError(t, err)
@@ -266,7 +270,7 @@ func TestFilePlugin_Process_ConfigUploadRequestTopic(t *testing.T) {
266270
fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient)
267271
messagePipe := busfakes.NewFakeMessagePipe()
268272

269-
filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command)
273+
filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command, &sync.RWMutex{})
270274
err := filePlugin.Init(ctx, messagePipe)
271275
require.NoError(t, err)
272276

@@ -321,7 +325,7 @@ func TestFilePlugin_Process_ConfigUploadRequestTopic_Failure(t *testing.T) {
321325
fakeGrpcConnection.FileServiceClientReturns(fakeFileServiceClient)
322326
messagePipe := busfakes.NewFakeMessagePipe()
323327

324-
filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command)
328+
filePlugin := NewFilePlugin(types.AgentConfig(), fakeGrpcConnection, model.Command, &sync.RWMutex{})
325329
err := filePlugin.Init(ctx, messagePipe)
326330
require.NoError(t, err)
327331

@@ -389,7 +393,7 @@ func TestFilePlugin_Process_ConfigApplyFailedTopic(t *testing.T) {
389393

390394
messagePipe := busfakes.NewFakeMessagePipe()
391395
agentConfig := types.AgentConfig()
392-
filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command)
396+
filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command, &sync.RWMutex{})
393397

394398
err := filePlugin.Init(ctx, messagePipe)
395399
require.NoError(t, err)
@@ -436,7 +440,7 @@ func TestFilePlugin_Process_ConfigApplyRollbackCompleteTopic(t *testing.T) {
436440
messagePipe := busfakes.NewFakeMessagePipe()
437441
agentConfig := types.AgentConfig()
438442
fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{}
439-
filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command)
443+
filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command, &sync.RWMutex{})
440444

441445
err := filePlugin.Init(ctx, messagePipe)
442446
require.NoError(t, err)
@@ -481,7 +485,7 @@ func TestFilePlugin_Process_ConfigApplyCompleteTopic(t *testing.T) {
481485
messagePipe := busfakes.NewFakeMessagePipe()
482486
agentConfig := types.AgentConfig()
483487
fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{}
484-
filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command)
488+
filePlugin := NewFilePlugin(agentConfig, fakeGrpcConnection, model.Command, &sync.RWMutex{})
485489

486490
err := filePlugin.Init(ctx, messagePipe)
487491
require.NoError(t, err)

‎internal/file/file_service_operator.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ type FileServiceOperator struct {
4242

4343
var _ fileServiceOperatorInterface = (*FileServiceOperator)(nil)
4444

45-
func NewFileServiceOperator(agentConfig *config.Config, fileServiceClient mpi.FileServiceClient, manifestLock *sync.RWMutex) *FileServiceOperator {
45+
func NewFileServiceOperator(agentConfig *config.Config, fileServiceClient mpi.FileServiceClient,
46+
manifestLock *sync.RWMutex,
47+
) *FileServiceOperator {
4648
isConnected := &atomic.Bool{}
4749
isConnected.Store(false)
4850

‎internal/file/file_service_operator_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"os"
1111
"path/filepath"
12+
"sync"
1213
"sync/atomic"
1314
"testing"
1415

@@ -45,7 +46,7 @@ func TestFileServiceOperator_UpdateOverview(t *testing.T) {
4546

4647
fakeFileServiceClient.UpdateFileReturns(&mpi.UpdateFileResponse{}, nil)
4748

48-
fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient)
49+
fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient, &sync.RWMutex{})
4950
fileServiceOperator.SetIsConnected(true)
5051

5152
err := fileServiceOperator.UpdateOverview(ctx, "123", []*mpi.File{
@@ -83,7 +84,7 @@ func TestFileServiceOperator_UpdateOverview_MaxIterations(t *testing.T) {
8384

8485
fakeFileServiceClient.UpdateFileReturns(&mpi.UpdateFileResponse{}, nil)
8586

86-
fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient)
87+
fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient, &sync.RWMutex{})
8788
fileServiceOperator.SetIsConnected(true)
8889

8990
err := fileServiceOperator.UpdateOverview(ctx, "123", []*mpi.File{
@@ -126,7 +127,7 @@ func TestFileManagerService_UpdateFile(t *testing.T) {
126127
}
127128

128129
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
129-
fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient)
130+
fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient, &sync.RWMutex{})
130131
fileServiceOperator.SetIsConnected(true)
131132

132133
err := fileServiceOperator.UpdateFile(ctx, "123", &mpi.File{FileMeta: fileMeta})
@@ -150,7 +151,7 @@ func TestFileManagerService_UpdateFile_LargeFile(t *testing.T) {
150151
fakeFileServiceClient := &v1fakes.FakeFileServiceClient{}
151152
fakeClientStreamingClient := &FakeClientStreamingClient{sendCount: atomic.Int32{}}
152153
fakeFileServiceClient.UpdateFileStreamReturns(fakeClientStreamingClient, nil)
153-
fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient)
154+
fileServiceOperator := NewFileServiceOperator(types.AgentConfig(), fakeFileServiceClient, &sync.RWMutex{})
154155

155156
fileServiceOperator.SetIsConnected(true)
156157
err := fileServiceOperator.UpdateFile(ctx, "123", &mpi.File{FileMeta: fileMeta})

‎internal/plugin/plugin_manager.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ func addResourcePlugin(plugins []bus.Plugin, agentConfig *config.Config) []bus.P
4646
return plugins
4747
}
4848

49-
func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentConfig *config.Config, manifestLock *sync.RWMutex) []bus.Plugin {
49+
func addCommandAndFilePlugins(ctx context.Context, plugins []bus.Plugin, agentConfig *config.Config,
50+
manifestLock *sync.RWMutex,
51+
) []bus.Plugin {
5052
if agentConfig.IsCommandGrpcClientConfigured() {
5153
grpcConnection, err := grpc.NewGrpcConnection(ctx, agentConfig, agentConfig.Command)
5254
if err != nil {

‎test/integration/auxiliarycommandserver/connection_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@ package auxiliarycommandserver
88
import (
99
"context"
1010
"fmt"
11-
"github.com/go-resty/resty/v2"
1211
"net"
1312
"net/http"
1413
"os"
1514
"sort"
1615
"testing"
1716
"time"
1817

18+
"github.com/go-resty/resty/v2"
19+
1920
mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
2021
"github.com/nginx/agent/v3/test/integration/utils"
2122
"github.com/stretchr/testify/suite"

0 commit comments

Comments
 (0)