Don't attempt re-injection after Disruption Duration has ended (#483)

* Dont attempt to reinject after duration has expired Add PreviouslyInjected to enum validation * Add a test * Update controllers/disruption_controller_test.go Co-authored-by: Joris Bonnefoy <[email protected]> Co-authored-by: Joris Bonnefoy <[email protected]>
DataDog · Jan 26, 2022 · 2873ca6 · 2873ca6
1 parent 4ff2255
commit 2873ca6
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 3 deletions.
diff --git a/api/v1beta1/disruption_types.go b/api/v1beta1/disruption_types.go
@@ -114,8 +114,8 @@ func (dd DisruptionDuration) Duration() time.Duration {
 type DisruptionStatus struct {
 	IsStuckOnRemoval bool `json:"isStuckOnRemoval,omitempty"`
 	IsInjected       bool `json:"isInjected,omitempty"`
-	// +kubebuilder:validation:Enum=NotInjected;PartiallyInjected;Injected
-	// +ddmark:validation:Enum=NotInjected;PartiallyInjected;Injected
+	// +kubebuilder:validation:Enum=NotInjected;PartiallyInjected;Injected;PreviouslyInjected
+	// +ddmark:validation:Enum=NotInjected;PartiallyInjected;Injected;PreviouslyInjected
 	InjectionStatus chaostypes.DisruptionInjectionStatus `json:"injectionStatus,omitempty"`
 	// +nullable
 	Targets []string `json:"targets,omitempty"`

diff --git a/chart/templates/crds/chaos.datadoghq.com_disruptions.yaml b/chart/templates/crds/chaos.datadoghq.com_disruptions.yaml
@@ -334,6 +334,7 @@ spec:
               - NotInjected
               - PartiallyInjected
               - Injected
+              - PreviouslyInjected
               type: string
             isInjected:
               type: boolean

diff --git a/controllers/disruption_controller.go b/controllers/disruption_controller.go
@@ -182,6 +182,37 @@ func (r *DisruptionReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			}
 
 			return ctrl.Result{Requeue: true}, err
+		} else if calculateRemainingDuration(*instance) <= 0 {
+			if _, err := r.updateInjectionStatus(instance); err != nil {
+				r.log.Errorw("error updating disruption injection status", "error", err)
+
+				return ctrl.Result{}, fmt.Errorf("error updating disruption injection status: %w", err)
+			}
+
+			isCleaned, err := r.cleanDisruption(instance)
+			if err != nil {
+				return ctrl.Result{}, err
+			}
+
+			if !isCleaned {
+				requeueAfter := time.Duration(rand.Intn(5)+5) * time.Second //nolint:gosec
+
+				r.log.Infow(fmt.Sprintf("disruption has not been fully cleaned yet, re-queuing in %v", requeueAfter))
+
+				return ctrl.Result{
+					Requeue:      true,
+					RequeueAfter: requeueAfter,
+				}, r.Update(context.Background(), instance)
+			}
+
+			requeueDelay := r.ExpiredDisruptionGCDelay
+
+			r.log.Infow("requeuing disruption to check for its expiration", "requeueDelay", requeueDelay.String())
+
+			return ctrl.Result{
+				Requeue:      true,
+				RequeueAfter: requeueDelay,
+			}, nil
 		}
 
 		// retrieve targets from label selector
@@ -218,7 +249,7 @@ func (r *DisruptionReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 				RequeueAfter: requeueAfter,
 			}, nil
 		}
-		requeueDelay := time.Duration(math.Max(calculateRemainingDuration(*instance).Seconds(), r.ExpiredDisruptionGCDelay.Seconds())) * time.Second
+		requeueDelay := calculateRemainingDuration(*instance)
 
 		r.log.Infow("requeuing disruption to check for its expiration", "requeueDelay", requeueDelay.String())
 

diff --git a/controllers/disruption_controller_test.go b/controllers/disruption_controller_test.go
@@ -287,6 +287,20 @@ var _ = Describe("Disruption Controller", func() {
 			By("Waiting for the disruption to expire naturally")
 			Eventually(func() error { return expectChaosPod(disruption, 0) }, timeout*2).Should(Succeed())
 
+			By("Waiting for the disruption to reach PreviouslyInjected")
+			Eventually(func() error {
+				if err := k8sClient.Get(context.Background(), instanceKey, disruption); err != nil {
+					return err
+				}
+
+				// check disruption injection status
+				if disruption.Status.InjectionStatus != chaostypes.DisruptionInjectionStatusPreviouslyInjected {
+					return fmt.Errorf("unexpected disruption status, current status is %s (expected PreviouslyInjected)", disruption.Status.InjectionStatus)
+				}
+
+				return nil
+			}, timeout*2).Should(Succeed())
+
 			By("Waiting for disruption to be removed")
 			Eventually(func() error { return k8sClient.Get(context.Background(), instanceKey, disruption) }, timeout).Should(MatchError("Disruption.chaos.datadoghq.com \"foo\" not found"))
 		})