From a62f9a0e04bb538a8018a3f866c88e8b93c59826 Mon Sep 17 00:00:00 2001
From: Jonathan Perry <YrrepNoj@gmail.com>
Date: Wed, 5 Jun 2024 13:38:44 -0400
Subject: [PATCH] chore: add checks before killing pods when updating istio
 annotations (#457)

## Description

As I was deploying multiple Package CRDs to a namespace I noticed that
my Pods were cycling whenever I deployed another Package CR to my
namespace. When looking through the code (with the help of engineers far
more familiar and smarter than me) we realized we might have been too
excited about when we were killing the pods.

This PR updates the operator so that it only kills the pods in the
namespace if it has actually changed the value of the istio-injection
label.


## Type of change
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Other (security config, docs update, etc)
---
 .../operator/controllers/istio/injection.ts   | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/pepr/operator/controllers/istio/injection.ts b/src/pepr/operator/controllers/istio/injection.ts
index b1562164e..36102cff3 100644
--- a/src/pepr/operator/controllers/istio/injection.ts
+++ b/src/pepr/operator/controllers/istio/injection.ts
@@ -17,16 +17,17 @@ export async function enableInjection(pkg: UDSPackage) {
 
   const sourceNS = await K8s(kind.Namespace).Get(pkg.metadata.namespace);
   const labels = sourceNS.metadata?.labels || {};
+  const originalInjectionLabel = labels[injectionLabel];
   const annotations = sourceNS.metadata?.annotations || {};
   const pkgKey = `uds.dev/pkg-${pkg.metadata.name}`;
 
   // Mark the original namespace injection setting for if all packages are removed
   if (!annotations[injectionAnnotation]) {
-    annotations[injectionAnnotation] = labels[injectionLabel] || "non-existent";
+    annotations[injectionAnnotation] = originalInjectionLabel || "non-existent";
   }
 
   // Ensure the namespace is configured
-  if (!annotations[pkgKey] || labels[injectionLabel] !== "enabled") {
+  if (!annotations[pkgKey] || originalInjectionLabel !== "enabled") {
     // Ensure Istio injection is enabled
     labels[injectionLabel] = "enabled";
 
@@ -45,7 +46,10 @@ export async function enableInjection(pkg: UDSPackage) {
       { force: true },
     );
 
-    await killPods(pkg.metadata.namespace, true);
+    // Kill the pods if we changed the value of the istio-injection label
+    if (originalInjectionLabel !== labels[injectionLabel]) {
+      await killPods(pkg.metadata.namespace, true);
+    }
   }
 }
 
@@ -61,6 +65,7 @@ export async function cleanupNamespace(pkg: UDSPackage) {
 
   const sourceNS = await K8s(kind.Namespace).Get(pkg.metadata.namespace);
   const labels = sourceNS.metadata?.labels || {};
+  const originalInjectionLabel = labels[injectionLabel];
   const annotations = sourceNS.metadata?.annotations || {};
 
   // Remove the package annotation
@@ -88,7 +93,10 @@ export async function cleanupNamespace(pkg: UDSPackage) {
     { force: true },
   );
 
-  await killPods(pkg.metadata.namespace, false);
+  // Kill the pods if we changed the value of the istio-injection label
+  if (originalInjectionLabel !== labels[injectionLabel]) {
+    await killPods(pkg.metadata.namespace, false);
+  }
 }
 
 /**
@@ -129,8 +137,8 @@ async function killPods(ns: string, enableInjection: boolean) {
 
   // Delete each group of pods
   for (const group of Object.values(groups)) {
-    // If this is a daemonset, delete the pods in reverse name order
-    if (group[0].metadata?.ownerReferences?.find(ref => ref.kind === "DaemonSet")) {
+    // If this is a statefulset, delete the pods in reverse name order
+    if (group[0].metadata?.ownerReferences?.find(ref => ref.kind === "StatefulSet")) {
       group.sort((a, b) => (b.metadata?.name || "").localeCompare(a.metadata?.name || ""));
     }