Skip to content

Commit d6df15b

Browse files
authored
fix: dont kill container (#2780)
## Description This PR makes it so that on container failure, services are stopped instead of removed. This makes it possible for Kurtosis to display error logs from the failed container during kurtosis runs.
1 parent 2b3d704 commit d6df15b

File tree

3 files changed

+16
-6
lines changed

3 files changed

+16
-6
lines changed

container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/user_services_functions/stop_user_services.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package user_service_functions
22

33
import (
44
"context"
5+
"time"
6+
57
"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/docker_kurtosis_backend/shared_helpers"
68
"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/docker_manager"
79
"github.com/kurtosis-tech/kurtosis/container-engine-lib/lib/backend_impls/docker/docker_operation_parallelizer"
@@ -11,6 +13,10 @@ import (
1113
"github.com/kurtosis-tech/stacktrace"
1214
)
1315

16+
const (
17+
stopContainerTimeout = 10 * time.Second
18+
)
19+
1420
func StopUserServices(
1521
ctx context.Context,
1622
enclaveUuid enclave.EnclaveUUID,
@@ -43,8 +49,8 @@ func StopUserServices(
4349
dockerManager *docker_manager.DockerManager,
4450
dockerObjectId string,
4551
) error {
46-
if err := dockerManager.KillContainer(ctx, dockerObjectId); err != nil {
47-
return stacktrace.Propagate(err, "An error occurred killing user service container with ID '%v'", dockerObjectId)
52+
if err := dockerManager.StopContainer(ctx, dockerObjectId, stopContainerTimeout); err != nil {
53+
return stacktrace.Propagate(err, "An error occurred stopping user service container with ID '%v'", dockerObjectId)
4854
}
4955
return nil
5056
}

container-engine-lib/lib/backend_impls/docker/docker_manager/docker_manager.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,8 @@ const (
160160
//The Docker or Podman network name where all the containers in the engine and logs service context will be added
161161
NameOfNetworkToStartEngineAndLogServiceContainersInDocker = "bridge"
162162
NameOfNetworkToStartEngineAndLogServiceContainersInPodman = "podman"
163+
164+
defaultContainerStopTimeout = 1 * time.Second
163165
)
164166

165167
type RestartPolicy string
@@ -731,7 +733,8 @@ func (manager *DockerManager) CreateAndStartContainer(
731733
functionFinishedSuccessfully := false
732734
defer func() {
733735
if !functionFinishedSuccessfully {
734-
if err := manager.KillContainer(ctx, containerId); err != nil {
736+
// Instead of killing the container, stop it so that logs are still available
737+
if err := manager.StopContainer(ctx, containerId, defaultContainerStopTimeout); err != nil {
735738
logrus.Error("The container creation function didn't finish successfully, meaning we needed to kill the container we created. However, the killing threw an error:")
736739
fmt.Fprintln(logrus.StandardLogger().Out, err)
737740
logrus.Errorf("ACTION NEEDED: You'll need to manually kill this container with ID '%v'", containerId)
@@ -741,6 +744,7 @@ func (manager *DockerManager) CreateAndStartContainer(
741744

742745
//Check if the container dies because sometimes users starts containers with a wrong configuration and these quickly dies
743746
didContainerStartSuccessfully, err := manager.didContainerStartSuccessfully(ctx, containerId, dockerImage)
747+
logrus.Infof("didContainerStartSuccessfully: %v", didContainerStartSuccessfully)
744748
if err != nil {
745749
return "", nil, stacktrace.Propagate(err, "An error occurred checking if container '%v' is running", containerId)
746750
}

core/server/api_container/server/service_network/default_service_network.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -990,13 +990,13 @@ func (network *DefaultServiceNetwork) startRegisteredService(
990990
},
991991
Statuses: nil,
992992
}
993-
_, failedToDestroyUuids, err := network.kurtosisBackend.DestroyUserServices(context.Background(), network.enclaveUuid, userServiceFilters)
993+
_, failedToDestroyUuids, err := network.kurtosisBackend.StopUserServices(context.Background(), network.enclaveUuid, userServiceFilters)
994994
if err != nil {
995-
logrus.Errorf("Attempted to destroy the services with UUIDs '%v' but had no success. You must manually destroy the services! The following error had occurred:\n'%v'", serviceToDestroyUuid, err)
995+
logrus.Errorf("Attempted to stop the services with UUIDs '%v' but had no success. You must manually stop the services! The following error had occurred:\n'%v'", serviceToDestroyUuid, err)
996996
return
997997
}
998998
if failedToDestroyErr, found := failedToDestroyUuids[serviceToDestroyUuid]; found {
999-
logrus.Errorf("Attempted to destroy the services with UUIDs '%v' but had no success. You must manually destroy the services! The following error had occurred:\n'%v'", serviceToDestroyUuid, failedToDestroyErr)
999+
logrus.Errorf("Attempted to stop the services with UUIDs '%v' but had no success. You must manually stop the services! The following error had occurred:\n'%v'", serviceToDestroyUuid, failedToDestroyErr)
10001000
}
10011001
}()
10021002

0 commit comments

Comments
 (0)