Skip to content

Commit

Permalink
chore: add checks before killing pods when updating istio annotations (
Browse files Browse the repository at this point in the history
…#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)
  • Loading branch information
YrrepNoj committed Jun 5, 2024
1 parent 7f058ec commit 3815a2a
Showing 1 changed file with 14 additions and 6 deletions.
20 changes: 14 additions & 6 deletions src/pepr/operator/controllers/istio/injection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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);
}
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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);
}
}

/**
Expand Down Expand Up @@ -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 || ""));
}

Expand Down

0 comments on commit 3815a2a

Please sign in to comment.