Skip to content

Commit 95d0916

Browse files
aphralGaphralG
authored
Add multi management plane integration test (#1157)
1 parent 852abe3 commit 95d0916

24 files changed

+528
-115
lines changed

‎Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,13 +161,13 @@ integration-test: $(SELECTED_PACKAGE) build-mock-management-plane-grpc
161161
TEST_ENV="Container" CONTAINER_OS_TYPE=$(CONTAINER_OS_TYPE) BUILD_TARGET="install-agent-local" CONTAINER_NGINX_IMAGE_REGISTRY=${CONTAINER_NGINX_IMAGE_REGISTRY} \
162162
PACKAGES_REPO=$(OSS_PACKAGES_REPO) PACKAGE_NAME=$(PACKAGE_NAME) BASE_IMAGE=$(BASE_IMAGE) DOCKERFILE_PATH=$(DOCKERFILE_PATH) IMAGE_PATH=$(IMAGE_PATH) TAG=${IMAGE_TAG} \
163163
OS_VERSION=$(OS_VERSION) OS_RELEASE=$(OS_RELEASE) \
164-
go test -v ./test/integration/installuninstall ./test/integration/managementplane ./test/integration/nginxless
164+
go test -v ./test/integration/installuninstall ./test/integration/managementplane ./test/integration/auxiliarycommandserver ./test/integration/nginxless
165165

166166
official-image-integration-test: $(SELECTED_PACKAGE) build-mock-management-plane-grpc
167167
TEST_ENV="Container" CONTAINER_OS_TYPE=$(CONTAINER_OS_TYPE) CONTAINER_NGINX_IMAGE_REGISTRY=${CONTAINER_NGINX_IMAGE_REGISTRY} BUILD_TARGET="install" \
168168
PACKAGES_REPO=$(OSS_PACKAGES_REPO) TAG=${TAG} PACKAGE_NAME=$(PACKAGE_NAME) BASE_IMAGE=$(BASE_IMAGE) DOCKERFILE_PATH=$(OFFICIAL_IMAGE_DOCKERFILE_PATH) \
169169
OS_VERSION=$(OS_VERSION) OS_RELEASE=$(OS_RELEASE) IMAGE_PATH=$(IMAGE_PATH) \
170-
go test -v ./test/integration/managementplane
170+
go test -v ./test/integration/managementplane ./test/integration/auxiliarycommandserver
171171

172172
performance-test:
173173
@mkdir -p $(TEST_BUILD_DIR)

‎internal/file/file_manager_service.go

Lines changed: 17 additions & 3 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
@@ -102,16 +103,19 @@ type FileManagerService struct {
102103
filesMutex sync.RWMutex
103104
}
104105

105-
func NewFileManagerService(fileServiceClient mpi.FileServiceClient, agentConfig *config.Config) *FileManagerService {
106+
func NewFileManagerService(fileServiceClient mpi.FileServiceClient, agentConfig *config.Config,
107+
manifestLock *sync.RWMutex,
108+
) *FileManagerService {
106109
return &FileManagerService{
107110
agentConfig: agentConfig,
108-
fileOperator: NewFileOperator(),
109-
fileServiceOperator: NewFileServiceOperator(agentConfig, fileServiceClient),
111+
fileOperator: NewFileOperator(manifestLock),
112+
fileServiceOperator: NewFileServiceOperator(agentConfig, fileServiceClient, manifestLock),
110113
fileActions: make(map[string]*model.FileCache),
111114
rollbackFileContents: make(map[string][]byte),
112115
currentFilesOnDisk: make(map[string]*mpi.File),
113116
previousManifestFiles: make(map[string]*model.ManifestFile),
114117
manifestFilePath: agentConfig.ManifestDir + "/manifest.json",
118+
manifestLock: manifestLock,
115119
}
116120
}
117121

@@ -423,9 +427,13 @@ func (fms *FileManagerService) UpdateCurrentFilesOnDisk(
423427
// seems to be a control flag, avoid control coupling
424428
// nolint: revive
425429
func (fms *FileManagerService) UpdateManifestFile(currentFiles map[string]*mpi.File, referenced bool) (err error) {
430+
slog.Debug("Updating manifest file", "current_files", currentFiles, "referenced", referenced)
426431
currentManifestFiles, _, readError := fms.manifestFile()
427432
fms.previousManifestFiles = currentManifestFiles
428433
if readError != nil && !errors.Is(readError, os.ErrNotExist) {
434+
slog.Debug("Error reading manifest file", "current_manifest_files",
435+
currentManifestFiles, "updated_files", currentFiles, "referenced", referenced)
436+
429437
return fmt.Errorf("unable to read manifest file: %w", readError)
430438
}
431439

@@ -457,6 +465,8 @@ func (fms *FileManagerService) manifestFile() (map[string]*model.ManifestFile, m
457465
return nil, nil, err
458466
}
459467

468+
fms.manifestLock.Lock()
469+
defer fms.manifestLock.Unlock()
460470
file, err := os.ReadFile(fms.manifestFilePath)
461471
if err != nil {
462472
return nil, nil, fmt.Errorf("failed to read manifest file: %w", err)
@@ -466,6 +476,10 @@ func (fms *FileManagerService) manifestFile() (map[string]*model.ManifestFile, m
466476

467477
err = json.Unmarshal(file, &manifestFiles)
468478
if err != nil {
479+
if len(file) == 0 {
480+
return nil, nil, fmt.Errorf("manifest file is empty: %w", err)
481+
}
482+
469483
return nil, nil, fmt.Errorf("failed to parse manifest file: %w", err)
470484
}
471485

‎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.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"log/slog"
1515
"os"
1616
"path"
17+
"sync"
1718

1819
"github.com/nginx/agent/v3/internal/model"
1920

@@ -24,14 +25,18 @@ import (
2425
mpi "github.com/nginx/agent/v3/api/grpc/mpi/v1"
2526
)
2627

27-
type FileOperator struct{}
28+
type FileOperator struct {
29+
manifestLock *sync.RWMutex
30+
}
2831

2932
var _ fileOperator = (*FileOperator)(nil)
3033

3134
// FileOperator only purpose is to write files,
3235

33-
func NewFileOperator() *FileOperator {
34-
return &FileOperator{}
36+
func NewFileOperator(manifestLock *sync.RWMutex) *FileOperator {
37+
return &FileOperator{
38+
manifestLock: manifestLock,
39+
}
3540
}
3641

3742
func (fo *FileOperator) Write(ctx context.Context, fileContent []byte, file *mpi.FileMeta) error {
@@ -152,6 +157,8 @@ func (fo *FileOperator) WriteManifestFile(updatedFiles map[string]*model.Manifes
152157
return fmt.Errorf("unable to marshal manifest file json: %w", err)
153158
}
154159

160+
fo.manifestLock.Lock()
161+
defer fo.manifestLock.Unlock()
155162
// 0755 allows read/execute for all, write for owner
156163
if err = os.MkdirAll(manifestDir, dirPerm); err != nil {
157164
return fmt.Errorf("unable to create directory %s: %w", manifestDir, err)

‎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: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package file
88
import (
99
"context"
1010
"log/slog"
11+
"sync"
1112

1213
"github.com/nginx/agent/v3/pkg/files"
1314
"github.com/nginx/agent/v3/pkg/id"
@@ -27,6 +28,7 @@ var _ bus.Plugin = (*FilePlugin)(nil)
2728
// the file plugin does not care about the instance type
2829

2930
type FilePlugin struct {
31+
manifestLock *sync.RWMutex
3032
messagePipe bus.MessagePipeInterface
3133
config *config.Config
3234
conn grpc.GrpcConnectionInterface
@@ -35,12 +37,13 @@ type FilePlugin struct {
3537
}
3638

3739
func NewFilePlugin(agentConfig *config.Config, grpcConnection grpc.GrpcConnectionInterface,
38-
serverType model.ServerType,
40+
serverType model.ServerType, manifestLock *sync.RWMutex,
3941
) *FilePlugin {
4042
return &FilePlugin{
41-
config: agentConfig,
42-
conn: grpcConnection,
43-
serverType: serverType,
43+
config: agentConfig,
44+
conn: grpcConnection,
45+
serverType: serverType,
46+
manifestLock: manifestLock,
4447
}
4548
}
4649

@@ -52,7 +55,7 @@ func (fp *FilePlugin) Init(ctx context.Context, messagePipe bus.MessagePipeInter
5255
slog.DebugContext(ctx, "Starting file plugin")
5356

5457
fp.messagePipe = messagePipe
55-
fp.fileManagerService = NewFileManagerService(fp.conn.FileServiceClient(), fp.config)
58+
fp.fileManagerService = NewFileManagerService(fp.conn.FileServiceClient(), fp.config, fp.manifestLock)
5659

5760
return nil
5861
}
@@ -145,7 +148,7 @@ func (fp *FilePlugin) handleConnectionReset(ctx context.Context, msg *bus.Messag
145148
fp.conn = newConnection
146149

147150
reconnect = fp.fileManagerService.IsConnected()
148-
fp.fileManagerService = NewFileManagerService(fp.conn.FileServiceClient(), fp.config)
151+
fp.fileManagerService = NewFileManagerService(fp.conn.FileServiceClient(), fp.config, fp.manifestLock)
149152
fp.fileManagerService.SetIsConnected(reconnect)
150153

151154
slog.DebugContext(ctx, "File manager service client reset successfully")

‎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)

0 commit comments

Comments
 (0)