From 1c4863f4ec112e3c8ca6f095b3551883cbc8a213 Mon Sep 17 00:00:00 2001 From: Gyanendra Mishra Date: Fri, 15 Dec 2023 21:31:18 +0530 Subject: [PATCH] fix: ignore the current status of the service during a start/stop (#1965) --- .../default_service_network.go | 6 ---- .../default_service_network_test.go | 33 +++++++++++++------ .../startosis_start_service_test.go | 5 --- .../startosis_stop_service_test.go | 7 ++-- 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/core/server/api_container/server/service_network/default_service_network.go b/core/server/api_container/server/service_network/default_service_network.go index 63c2b78a7d..b20426bbfd 100644 --- a/core/server/api_container/server/service_network/default_service_network.go +++ b/core/server/api_container/server/service_network/default_service_network.go @@ -463,9 +463,6 @@ func (network *DefaultServiceNetwork) StartServices( if err != nil { return nil, nil, stacktrace.Propagate(err, "An error occurred while getting service registration for identifier '%v'", serviceIdentifier) } - if serviceRegistration.GetStatus() == service.ServiceStatus_Started { - return nil, nil, stacktrace.NewError("Service '%v' is already started", serviceRegistration.GetName()) - } serviceRegistrations[serviceRegistration.GetUUID()] = serviceRegistration } @@ -531,9 +528,6 @@ func (network *DefaultServiceNetwork) StopServices( if err != nil { return nil, nil, stacktrace.Propagate(err, "An error occurred while getting service registration for identifier '%v'", serviceIdentifier) } - if serviceRegistration.GetStatus() == service.ServiceStatus_Stopped { - return nil, nil, stacktrace.NewError("Service '%v' is already stopped", serviceRegistration.GetName()) - } serviceUuids[serviceRegistration.GetUUID()] = true serviceNamesByUuid[serviceRegistration.GetUUID()] = serviceRegistration.GetName() } diff --git a/core/server/api_container/server/service_network/default_service_network_test.go b/core/server/api_container/server/service_network/default_service_network_test.go index 7c7e1e4fe6..45c5ccf5cd 100644 --- a/core/server/api_container/server/service_network/default_service_network_test.go +++ b/core/server/api_container/server/service_network/default_service_network_test.go @@ -685,13 +685,19 @@ func TestStopService_ServiceAlreadyStopped(t *testing.T) { }, Statuses: nil, }, - ).Maybe().Times(0) + ).Times(1).Return( + map[service.ServiceUUID]bool{ + serviceUuid: true, + }, + map[service.ServiceUUID]error{}, + nil, + ) err = network.StopService(ctx, string(serviceName)) - require.NotNil(t, err) - expectedErrorMsg := fmt.Sprintf("Service '%s' is already stopped", string(serviceName)) - require.Contains(t, err.Error(), expectedErrorMsg) - require.Equal(t, service.ServiceStatus_Stopped, serviceRegistration.GetStatus()) + require.Nil(t, err) + serviceRegistrationAfterBeingStopped, err := network.serviceRegistrationRepository.Get(serviceName) + require.NoError(t, err) + require.Equal(t, serviceRegistrationAfterBeingStopped.GetStatus(), service.ServiceStatus_Stopped) } func TestStartService_Successful(t *testing.T) { @@ -814,6 +820,7 @@ func TestStartService_ServiceAlreadyStarted(t *testing.T) { serviceRegistration.SetStatus(service.ServiceStatus_Started) serviceConfig := testServiceConfig(t, testContainerImageName) serviceRegistration.SetConfig(serviceConfig) + serviceObj := service.NewService(serviceRegistration, map[string]*port_spec.PortSpec{}, successfulServiceIp, map[string]*port_spec.PortSpec{}, container.NewContainer(container.ContainerStatus_Running, testContainerImageName, nil, nil, nil)) file, err := os.CreateTemp("/tmp", "*.db") defer os.Remove(file.Name()) @@ -841,13 +848,19 @@ func TestStartService_ServiceAlreadyStarted(t *testing.T) { map[service.ServiceUUID]*service.ServiceConfig{ serviceUuid: serviceConfig, }, - ).Maybe().Times(0) + ).Times(1).Return( + map[service.ServiceUUID]*service.Service{ + serviceUuid: serviceObj, + }, + map[service.ServiceUUID]error{}, + nil, + ) err = network.StartService(ctx, string(serviceName)) - require.NotNil(t, err) - expectedErrorMsg := fmt.Sprintf("Service '%s' is already started", string(serviceName)) - require.Contains(t, err.Error(), expectedErrorMsg) - require.Equal(t, serviceRegistration.GetStatus(), service.ServiceStatus_Started) + require.Nil(t, err) + serviceRegistrationAfterBeingStarted, err := network.serviceRegistrationRepository.Get(serviceName) + require.NoError(t, err) + require.Equal(t, serviceRegistrationAfterBeingStarted.GetStatus(), service.ServiceStatus_Started) } func TestUpdateService(t *testing.T) { diff --git a/internal_testsuites/golang/testsuite/startosis_start_service_test/startosis_start_service_test.go b/internal_testsuites/golang/testsuite/startosis_start_service_test/startosis_start_service_test.go index 12c8b28720..20f7dcbc9a 100644 --- a/internal_testsuites/golang/testsuite/startosis_start_service_test/startosis_start_service_test.go +++ b/internal_testsuites/golang/testsuite/startosis_start_service_test/startosis_start_service_test.go @@ -2,7 +2,6 @@ package startosis_stop_service_test import ( "context" - "fmt" "testing" "github.com/kurtosis-tech/kurtosis-cli/golang_internal_testsuite/test_helpers" @@ -108,8 +107,6 @@ Service ` + serviceName + ` deployed successfully. require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error") require.Empty(t, runResult.ValidationErrors, "Unexpected validation error") require.NotEmpty(t, runResult.ExecutionError, "Expected execution error coming from already started service") - expectedErrorStr := fmt.Sprintf("Service '%v' is already started", serviceName) - require.Contains(t, runResult.ExecutionError.ErrorMessage, expectedErrorStr) // we run the stop and restart script and validate that the service is unreachable. runResult, err = test_helpers.RunScriptWithDefaultConfig(ctx, enclaveCtx, stopAndStartScript) @@ -136,6 +133,4 @@ Service ` + serviceName + ` deployed successfully. require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error") require.Empty(t, runResult.ValidationErrors, "Unexpected validation error") require.NotEmpty(t, runResult.ExecutionError, "Expected execution error coming from already started service") - expectedErrorStr = fmt.Sprintf("Service '%v' is already started", serviceName) - require.Contains(t, runResult.ExecutionError.ErrorMessage, expectedErrorStr) } diff --git a/internal_testsuites/golang/testsuite/startosis_stop_service_test/startosis_stop_service_test.go b/internal_testsuites/golang/testsuite/startosis_stop_service_test/startosis_stop_service_test.go index 60419da0e0..31b37bf6ba 100644 --- a/internal_testsuites/golang/testsuite/startosis_stop_service_test/startosis_stop_service_test.go +++ b/internal_testsuites/golang/testsuite/startosis_stop_service_test/startosis_stop_service_test.go @@ -2,7 +2,6 @@ package startosis_stop_service_test import ( "context" - "fmt" "testing" "github.com/kurtosis-tech/kurtosis-cli/golang_internal_testsuite/test_helpers" @@ -116,11 +115,9 @@ Service ` + serviceName + ` deployed successfully. logrus.Infof("Validated that the service is stopped") - // we run the stop script one more time and validate that an error is returned since the service is already stopped. + // already stopped service should be able to stop just fine runResult, _ = test_helpers.RunScriptWithDefaultConfig(ctx, enclaveCtx, stopScript2) require.Nil(t, runResult.InterpretationError, "Unexpected interpretation error") require.Empty(t, runResult.ValidationErrors, "Unexpected validation error") - require.NotEmpty(t, runResult.ExecutionError, "Expected execution error coming from already stopped service") - expectedErrorStr := fmt.Sprintf("Service '%v' is already stopped", serviceName) - require.Contains(t, runResult.ExecutionError.ErrorMessage, expectedErrorStr) + require.Nil(t, runResult.ExecutionError) }