Skip to content

Commit ec2984e

Browse files
aphralGaphralG
committed
add tests
1 parent 9ecb92d commit ec2984e

File tree

2 files changed

+127
-16
lines changed

2 files changed

+127
-16
lines changed

‎internal/nginx/nginx_plugin.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,10 @@ func (n *NginxPlugin) failedConfigApply(ctx context.Context, correlationID, inst
430430

431431
err := n.fileManagerService.Rollback(ctx, instanceID)
432432
if err != nil {
433+
configErr := fmt.Errorf("config apply error: %w", applyErr)
434+
rbErr := fmt.Errorf("rollback error: %w", err)
435+
combinedErr := errors.Join(configErr, rbErr)
436+
433437
rollbackResponse := response.CreateDataPlaneResponse(
434438
correlationID,
435439
&mpi.CommandResponse{
@@ -446,7 +450,7 @@ func (n *NginxPlugin) failedConfigApply(ctx context.Context, correlationID, inst
446450
&mpi.CommandResponse{
447451
Status: mpi.CommandResponse_COMMAND_STATUS_FAILURE,
448452
Message: "Config apply failed, rollback failed",
449-
Error: err.Error(),
453+
Error: combinedErr.Error(),
450454
},
451455
mpi.DataPlaneResponse_CONFIG_APPLY_REQUEST,
452456
instanceID,

‎internal/nginx/nginx_plugin_test.go‎

Lines changed: 122 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import (
3939
"github.com/stretchr/testify/require"
4040
)
4141

42-
func TestResource_createPlusAPIError(t *testing.T) {
42+
func TestNginx_createPlusAPIError(t *testing.T) {
4343
s := "failed to get the HTTP servers of upstream nginx1: expected 200 response, got 404. error.status=404;" +
4444
" error.text=upstream not found; error.code=UpstreamNotFound; request_id=b534bdab5cb5e321e8b41b431828b270; " +
4545
"href=https://nginx.org/en/docs/http/ngx_http_api_module.html"
@@ -61,7 +61,7 @@ func TestResource_createPlusAPIError(t *testing.T) {
6161
assert.Equal(t, errors.New(string(expectedJSON)), result)
6262
}
6363

64-
func TestResource_Process_APIAction_GetHTTPServers(t *testing.T) {
64+
func TestNginx_Process_APIAction_GetHTTPServers(t *testing.T) {
6565
ctx := context.Background()
6666

6767
inValidInstance := protos.NginxPlusInstance([]string{})
@@ -136,14 +136,14 @@ func TestResource_Process_APIAction_GetHTTPServers(t *testing.T) {
136136
}
137137

138138
for _, test := range tests {
139-
runResourceTestHelper(t, ctx, test.name, func(fakeService *nginxfakes.FakeNginxServiceInterface) {
139+
runNginxTestHelper(t, ctx, test.name, func(fakeService *nginxfakes.FakeNginxServiceInterface) {
140140
fakeService.GetHTTPUpstreamServersReturns(test.upstreams, test.err)
141141
}, test.instance, test.message, test.topic, test.err)
142142
}
143143
}
144144

145145
//nolint:dupl // need to refactor so that redundant code can be removed
146-
func TestResource_Process_APIAction_UpdateHTTPUpstreams(t *testing.T) {
146+
func TestNginx_Process_APIAction_UpdateHTTPUpstreams(t *testing.T) {
147147
ctx := context.Background()
148148
tests := []struct {
149149
instance *mpi.Instance
@@ -247,7 +247,7 @@ func TestResource_Process_APIAction_UpdateHTTPUpstreams(t *testing.T) {
247247
}
248248

249249
//nolint:dupl // need to refactor so that redundant code can be removed
250-
func TestResource_Process_APIAction_UpdateStreamServers(t *testing.T) {
250+
func TestNginx_Process_APIAction_UpdateStreamServers(t *testing.T) {
251251
ctx := context.Background()
252252
tests := []struct {
253253
instance *mpi.Instance
@@ -350,7 +350,7 @@ func TestResource_Process_APIAction_UpdateStreamServers(t *testing.T) {
350350
}
351351
}
352352

353-
func TestResource_Process_APIAction_GetStreamUpstreams(t *testing.T) {
353+
func TestNginx_Process_APIAction_GetStreamUpstreams(t *testing.T) {
354354
ctx := context.Background()
355355

356356
inValidInstance := protos.NginxPlusInstance([]string{})
@@ -492,7 +492,7 @@ func TestResource_Process_APIAction_GetStreamUpstreams(t *testing.T) {
492492
}
493493
}
494494

495-
func TestResource_Process_APIAction_GetUpstreams(t *testing.T) {
495+
func TestNginx_Process_APIAction_GetUpstreams(t *testing.T) {
496496
ctx := context.Background()
497497

498498
inValidInstance := protos.NginxPlusInstance([]string{})
@@ -606,13 +606,13 @@ func TestResource_Process_APIAction_GetUpstreams(t *testing.T) {
606606
}
607607

608608
for _, test := range tests {
609-
runResourceTestHelper(t, ctx, test.name, func(fakeService *nginxfakes.FakeNginxServiceInterface) {
609+
runNginxTestHelper(t, ctx, test.name, func(fakeService *nginxfakes.FakeNginxServiceInterface) {
610610
fakeService.GetUpstreamsReturns(test.upstreams, test.err)
611611
}, test.instance, test.message, test.topic, test.err)
612612
}
613613
}
614614

615-
func TestResource_Subscriptions(t *testing.T) {
615+
func TestNginx_Subscriptions(t *testing.T) {
616616
fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{}
617617
nginxPlugin := NewNginx(types.AgentConfig(), fakeGrpcConnection, model.Command, &sync.RWMutex{})
618618
assert.Equal(t,
@@ -640,7 +640,7 @@ func TestResource_Subscriptions(t *testing.T) {
640640
readNginxPlugin.Subscriptions())
641641
}
642642

643-
func TestResource_Info(t *testing.T) {
643+
func TestNginx_Info(t *testing.T) {
644644
fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{}
645645
nginxPlugin := NewNginx(types.AgentConfig(), fakeGrpcConnection, model.Command, &sync.RWMutex{})
646646
assert.Equal(t, &bus.Info{Name: "nginx"}, nginxPlugin.Info())
@@ -649,7 +649,7 @@ func TestResource_Info(t *testing.T) {
649649
assert.Equal(t, &bus.Info{Name: "auxiliary-nginx"}, readNginxPlugin.Info())
650650
}
651651

652-
func TestResource_Init(t *testing.T) {
652+
func TestNginx_Init(t *testing.T) {
653653
ctx := context.Background()
654654
fakeNginxService := nginxfakes.FakeNginxServiceInterface{}
655655

@@ -667,7 +667,7 @@ func TestResource_Init(t *testing.T) {
667667
assert.Empty(t, messages)
668668
}
669669

670-
func TestResource_Process_handleConfigUploadRequest(t *testing.T) {
670+
func TestNginx_Process_handleConfigUploadRequest(t *testing.T) {
671671
ctx := context.Background()
672672

673673
tempDir := os.TempDir()
@@ -728,7 +728,7 @@ func TestResource_Process_handleConfigUploadRequest(t *testing.T) {
728728
)
729729
}
730730

731-
func TestResource_Process_handleConfigUploadRequest_Failure(t *testing.T) {
731+
func TestNginx_Process_handleConfigUploadRequest_Failure(t *testing.T) {
732732
ctx := context.Background()
733733

734734
fileMeta := protos.FileMeta("/unknown/file.conf", "")
@@ -786,7 +786,7 @@ func TestResource_Process_handleConfigUploadRequest_Failure(t *testing.T) {
786786
)
787787
}
788788

789-
func TestResource_Process_handleConfigApplyRequest(t *testing.T) {
789+
func TestNginx_Process_handleConfigApplyRequest(t *testing.T) {
790790
ctx := context.Background()
791791
tempDir := t.TempDir()
792792

@@ -932,6 +932,113 @@ func TestResource_Process_handleConfigApplyRequest(t *testing.T) {
932932
}
933933
}
934934

935+
func TestNginxPlugin_Failed_ConfigApply(t *testing.T) {
936+
ctx := context.Background()
937+
938+
fakeGrpcConnection := &grpcfakes.FakeGrpcConnectionInterface{}
939+
940+
tests := []struct {
941+
rollbackError error
942+
rollbackWriteError error
943+
message string
944+
name string
945+
}{
946+
{
947+
name: "Test 1 - Rollback Success",
948+
message: "",
949+
rollbackError: nil,
950+
rollbackWriteError: nil,
951+
},
952+
{
953+
name: "Test 2 - Rollback Failed",
954+
message: "config apply error: something went wrong\nrollback error: rollback failed",
955+
rollbackError: errors.New("rollback failed"),
956+
rollbackWriteError: nil,
957+
},
958+
{
959+
name: "Test 3 - Rollback Write Failed",
960+
message: "config apply error: something went wrong\nrollback error: rollback write failed",
961+
rollbackError: nil,
962+
rollbackWriteError: errors.New("rollback write failed"),
963+
},
964+
}
965+
966+
for _, tt := range tests {
967+
t.Run(tt.name, func(t *testing.T) {
968+
fakeNginxService := &nginxfakes.FakeNginxServiceInterface{}
969+
fakeNginxService.ApplyConfigReturnsOnCall(0, &model.NginxConfigContext{},
970+
errors.New("something went wrong"))
971+
fakeNginxService.ApplyConfigReturnsOnCall(1, &model.NginxConfigContext{}, tt.rollbackWriteError)
972+
973+
fakeFileManagerService := &filefakes.FakeFileManagerServiceInterface{}
974+
fakeFileManagerService.RollbackReturns(tt.rollbackError)
975+
976+
messagePipe := busfakes.NewFakeMessagePipe()
977+
978+
nginxPlugin := NewNginx(types.AgentConfig(), fakeGrpcConnection, model.Command, &sync.RWMutex{})
979+
980+
err := nginxPlugin.Init(ctx, messagePipe)
981+
nginxPlugin.fileManagerService = fakeFileManagerService
982+
nginxPlugin.nginxService = fakeNginxService
983+
require.NoError(t, err)
984+
985+
nginxPlugin.applyConfig(ctx, "dfsbhj6-bc92-30c1-a9c9-85591422068e", protos.
986+
NginxOssInstance([]string{}).GetInstanceMeta().GetInstanceId())
987+
988+
messages := messagePipe.Messages()
989+
990+
dataPlaneResponse, ok := messages[0].Data.(*mpi.DataPlaneResponse)
991+
assert.True(t, ok)
992+
assert.Equal(
993+
t,
994+
mpi.CommandResponse_COMMAND_STATUS_ERROR,
995+
dataPlaneResponse.GetCommandResponse().GetStatus(),
996+
)
997+
assert.Equal(t, "Config apply failed, rolling back config",
998+
dataPlaneResponse.GetCommandResponse().GetMessage())
999+
1000+
if tt.rollbackError == nil && tt.rollbackWriteError == nil {
1001+
assert.Len(t, messages, 3)
1002+
dataPlaneResponse, ok = messages[1].Data.(*mpi.DataPlaneResponse)
1003+
assert.True(t, ok)
1004+
assert.Equal(
1005+
t,
1006+
mpi.CommandResponse_COMMAND_STATUS_FAILURE,
1007+
dataPlaneResponse.GetCommandResponse().GetStatus(),
1008+
)
1009+
1010+
assert.Equal(t, "Config apply failed, rollback successful",
1011+
dataPlaneResponse.GetCommandResponse().GetMessage())
1012+
assert.Equal(t, bus.EnableWatchersTopic, messages[2].Topic)
1013+
} else {
1014+
assert.Len(t, messages, 4)
1015+
dataPlaneResponse, ok = messages[1].Data.(*mpi.DataPlaneResponse)
1016+
assert.True(t, ok)
1017+
assert.Equal(
1018+
t,
1019+
mpi.CommandResponse_COMMAND_STATUS_ERROR,
1020+
dataPlaneResponse.GetCommandResponse().GetStatus(),
1021+
)
1022+
1023+
assert.Equal(t, "Rollback failed", dataPlaneResponse.GetCommandResponse().GetMessage())
1024+
1025+
dataPlaneResponse, ok = messages[2].Data.(*mpi.DataPlaneResponse)
1026+
assert.True(t, ok)
1027+
assert.Equal(
1028+
t,
1029+
mpi.CommandResponse_COMMAND_STATUS_FAILURE,
1030+
dataPlaneResponse.GetCommandResponse().GetStatus(),
1031+
)
1032+
1033+
assert.Equal(t, "Config apply failed, rollback failed",
1034+
dataPlaneResponse.GetCommandResponse().GetMessage())
1035+
assert.Equal(t, tt.message, dataPlaneResponse.GetCommandResponse().GetError())
1036+
assert.Equal(t, bus.EnableWatchersTopic, messages[3].Topic)
1037+
}
1038+
})
1039+
}
1040+
}
1041+
9351042
func TestNginxPlugin_Process_NginxConfigUpdateTopic(t *testing.T) {
9361043
ctx := context.Background()
9371044

@@ -970,7 +1077,7 @@ func TestNginxPlugin_Process_NginxConfigUpdateTopic(t *testing.T) {
9701077
}
9711078

9721079
//nolint:revive,lll // maximum number of arguments exceed
973-
func runResourceTestHelper(t *testing.T, ctx context.Context, testName string,
1080+
func runNginxTestHelper(t *testing.T, ctx context.Context, testName string,
9741081
getUpstreamsFunc func(serviceInterface *nginxfakes.FakeNginxServiceInterface), instance *mpi.Instance,
9751082
message *bus.Message, topic []string, err error,
9761083
) {

0 commit comments

Comments
 (0)