Fix handling of Forbidden errors returned from Kubernetes API when up…

…dating resources - should lead to requeue rather than be reported as error
otterize · Sep 22, 2024 · 7c7b239 · 7c7b239
1 parent cb66843
commit 7c7b239
Show file tree

Hide file tree

Showing 4 changed files with 6 additions and 10 deletions.
diff --git a/src/operator/controllers/iam/pods/pods_controller.go b/src/operator/controllers/iam/pods/pods_controller.go
@@ -88,7 +88,7 @@ func (r *PodReconciler) handlePodUpdate(ctx context.Context, pod corev1.Pod) (ct
 		controllerutil.AddFinalizer(updatedPod, r.agent.FinalizerName())
 		err := r.Patch(ctx, updatedPod, client.MergeFrom(&pod))
 		if err != nil {
-			if apierrors.IsConflict(err) {
+			if apierrors.IsConflict(err) || apierrors.IsNotFound(err) || apierrors.IsForbidden(err) {
 				return ctrl.Result{Requeue: true}, nil
 			}
 			return ctrl.Result{}, errors.Wrap(err)
@@ -97,7 +97,7 @@ func (r *PodReconciler) handlePodUpdate(ctx context.Context, pod corev1.Pod) (ct
 		apiutils.AddLabel(updatedServiceAccount, r.agent.ServiceAccountLabel(), metadata.OtterizeServiceAccountHasPodsValue)
 		err = r.Patch(ctx, updatedServiceAccount, client.MergeFrom(&serviceAccount))
 		if err != nil {
-			if apierrors.IsConflict(err) {
+			if apierrors.IsConflict(err) || apierrors.IsNotFound(err) || apierrors.IsForbidden(err) {
 				return ctrl.Result{Requeue: true}, nil
 			}
 			return ctrl.Result{}, errors.Wrap(err)
@@ -173,13 +173,9 @@ func (r *PodReconciler) handleLastPodWithThisSA(ctx context.Context, pod corev1.
 	apiutils.AddLabel(updatedServiceAccount, r.agent.ServiceAccountLabel(), metadata.OtterizeServiceAccountHasNoPodsValue)
 	err = r.Client.Patch(ctx, updatedServiceAccount, client.MergeFrom(&serviceAccount))
 	if err != nil {
-		if apierrors.IsConflict(err) {
+		if apierrors.IsConflict(err) || apierrors.IsNotFound(err) || apierrors.IsForbidden(err) {
 			return true, nil
 		}
-		// service account can be deleted before the pods go down, in which case cleanup has already occurred, so just let the pod terminate.
-		if apierrors.IsNotFound(err) {
-			return false, nil
-		}
 		return false, errors.Wrap(err)
 	}
 

diff --git a/src/operator/controllers/iam/serviceaccounts/serviceaccount_controller.go b/src/operator/controllers/iam/serviceaccounts/serviceaccount_controller.go
@@ -75,7 +75,7 @@ func (r *ServiceAccountReconciler) handleServiceAccountUpdate(ctx context.Contex
 	if updated {
 		err := r.Client.Patch(ctx, updatedServiceAccount, client.MergeFrom(&serviceAccount))
 		if err != nil {
-			if apierrors.IsConflict(err) {
+			if apierrors.IsConflict(err) || apierrors.IsNotFound(err) || apierrors.IsForbidden(err) {
 				return ctrl.Result{Requeue: true}, nil
 			}
 			return ctrl.Result{}, errors.Wrap(err)

diff --git a/src/operator/controllers/iam/webhooks/pod_webhook.go b/src/operator/controllers/iam/webhooks/pod_webhook.go
@@ -86,7 +86,7 @@ func (w *ServiceAccountAnnotatingPodWebhook) handleWithRetriesOnConflictOrNotFou
 		logger.Debugf("Handling pod (attempt %d out of %d)", attempt+1, maxRetries)
 		outputPod, patched, successMsg, err = w.handleOnce(ctx, *pod.DeepCopy(), dryRun)
 		if err != nil {
-			if k8serrors.IsConflict(err) || k8serrors.IsNotFound(err) {
+			if k8serrors.IsConflict(err) || k8serrors.IsNotFound(err) || k8serrors.IsForbidden(err) {
 				logger.WithError(err).Errorf("failed to handle pod due to conflict, retrying in 1 second (attempt %d out of %d)", attempt+1, 3)
 				time.Sleep(1 * time.Second)
 				continue

diff --git a/src/operator/controllers/tls_pod/tls_pod_reconciler.go b/src/operator/controllers/tls_pod/tls_pod_reconciler.go
@@ -101,7 +101,7 @@ func (r *PodReconciler) updatePodLabel(ctx context.Context, pod *corev1.Pod, lab
 	pod.Labels[labelKey] = labelValue
 
 	if err := r.Update(ctx, pod); err != nil {
-		if apierrors.IsConflict(err) {
+		if apierrors.IsConflict(err) || apierrors.IsNotFound(err) || apierrors.IsForbidden(err) {
 			// The Pod has been updated since we read it.
 			// Requeue the Pod to try to reconciliate again.
 			return ctrl.Result{Requeue: true}, nil