Skip to content

Commit

Permalink
Safely remove injector pods that never started (#928)
Browse files Browse the repository at this point in the history
* Safely remove injector pods that never started

* pointers can be nil, apparently

* Add a unit test
  • Loading branch information
ptnapoleon authored Oct 9, 2024
1 parent bd7682e commit 5b4d7f3
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
6 changes: 6 additions & 0 deletions services/chaospod.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ func (m *chaosPodService) HandleChaosPodTermination(ctx context.Context, disrupt
continue
}

// if the injector container definitively did not start, we can remove the finalizer
if cs.Started != nil && !(*cs.Started) {
shouldRemoveFinalizer = true
}

// if the injector container was terminated due to a start error, it can't have injected, we can remove the finalizer
if cs.State.Terminated != nil && cs.State.Terminated.Reason == "StartError" {
shouldRemoveFinalizer = true
}
Expand Down
24 changes: 24 additions & 0 deletions services/chaospod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/DataDog/chaos-controller/services"
"github.com/DataDog/chaos-controller/targetselector"
chaostypes "github.com/DataDog/chaos-controller/types"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/stretchr/testify/mock"
Expand Down Expand Up @@ -243,13 +244,15 @@ var _ = Describe("Chaos Pod Service", func() {

var (
isStuckOnRemoval bool
falseStarted bool
cpBuilder *builderstest.ChaosPodBuilder
)

BeforeEach(func() {
// Arrange
cpBuilder = builderstest.NewPodBuilder("test-1", DefaultChaosNamespace)
targetSelectorMock.EXPECT().TargetIsHealthy(mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
falseStarted = false
})

JustBeforeEach(func() {
Expand Down Expand Up @@ -304,6 +307,27 @@ var _ = Describe("Chaos Pod Service", func() {
"test",
DefaultNamespace,
).WithStatusPhase(v1.PodFailed)),
Entry(
"with a failed pod that never started",
builderstest.NewPodBuilder(
"test",
DefaultNamespace,
).WithStatus(v1.PodStatus{
Phase: v1.PodFailed,
Reason: "PodFailed",
ContainerStatuses: []v1.ContainerStatus{
{
Name: "injector",
State: v1.ContainerState{
Terminated: &v1.ContainerStateTerminated{
Reason: "ContainerStatusUnknown",
ExitCode: 137,
},
},
Started: &falseStarted,
},
},
})),
Entry(
"with failed a pod exceeding its activeDeadlineSeconds",
builderstest.NewPodBuilder(
Expand Down

0 comments on commit 5b4d7f3

Please sign in to comment.