Improve correctness and efficiency in some of pods api helper functions (#2067)

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/renderer/api/__tests__/pods.test.ts

@@ -0,0 +1,303 @@
+import { Pod } from "../endpoints";
+
+interface GetDummyPodOptions {
+  running?: number;
+  dead?: number;
+  initRunning?: number;
+  initDead?: number;
+}
+
+function getDummyPodDefaultOptions(): Required<GetDummyPodOptions> {
+  return {
+    running: 0,
+    dead: 0,
+    initDead: 0,
+    initRunning: 0,
+  };
+}
+
+function getDummyPod(opts: GetDummyPodOptions = getDummyPodDefaultOptions()): Pod {
+  const pod = new Pod({
+    apiVersion: "v1",
+    kind: "Pod",
+    metadata: {
+      uid: "1",
+      name: "test",
+      resourceVersion: "v1",
+      selfLink: "http"
+    }
+  });
+
+  pod.spec = {
+    containers: [],
+    initContainers: [],
+    serviceAccount: "dummy",
+    serviceAccountName: "dummy",
+  };
+
+  pod.status = {
+    phase: "Running",
+    conditions: [],
+    hostIP: "10.0.0.1",
+    podIP: "10.0.0.1",
+    startTime: "now",
+    containerStatuses: [],
+    initContainerStatuses: [],
+  };
+
+  for (let i = 0; i < opts.running; i += 1) {
+    const name = `container_r_${i}`;
+
+    pod.spec.containers.push({
+      image: "dummy",
+      imagePullPolicy: "dummy",
+      name,
+    });
+    pod.status.containerStatuses.push({
+      image: "dummy",
+      imageID: "dummy",
+      name,
+      ready: true,
+      restartCount: i,
+      state: {
+        running: {
+          startedAt: "before"
+        },
+      }
+    });
+  }
+
+  for (let i = 0; i < opts.dead; i += 1) {
+    const name = `container_d_${i}`;
+
+    pod.spec.containers.push({
+      image: "dummy",
+      imagePullPolicy: "dummy",
+      name,
+    });
+    pod.status.containerStatuses.push({
+      image: "dummy",
+      imageID: "dummy",
+      name,
+      ready: false,
+      restartCount: i,
+      state: {
+        terminated: {
+          startedAt: "before",
+          exitCode: i+1,
+          finishedAt: "later",
+          reason: `reason_${i}`
+        }
+      }
+    });
+  }
+
+  for (let i = 0; i < opts.initRunning; i += 1) {
+    const name = `container_ir_${i}`;
+
+    pod.spec.initContainers.push({
+      image: "dummy",
+      imagePullPolicy: "dummy",
+      name,
+    });
+    pod.status.initContainerStatuses.push({
+      image: "dummy",
+      imageID: "dummy",
+      name,
+      ready: true,
+      restartCount: i,
+      state: {
+        running: {
+          startedAt: "before"
+        }
+      }
+    });
+  }
+
+  for (let i = 0; i < opts.initDead; i += 1) {
+    const name = `container_id_${i}`;
+
+    pod.spec.initContainers.push({
+      image: "dummy",
+      imagePullPolicy: "dummy",
+      name,
+    });
+    pod.status.initContainerStatuses.push({
+      image: "dummy",
+      imageID: "dummy",
+      name,
+      ready: false,
+      restartCount: i,
+      state: {
+        terminated: {
+          startedAt: "before",
+          exitCode: i+1,
+          finishedAt: "later",
+          reason: `reason_${i}`
+        }
+      }
+    });
+  }
+
+  return pod;
+}
+
+describe("Pods", () => {
+  const podTests = [];
+
+  for (let r = 0; r < 10; r += 1) {
+    for (let d = 0; d < 10; d += 1) {
+      for (let ir = 0; ir < 10; ir += 1) {
+        for (let id = 0; id < 10; id += 1) {
+          podTests.push([r, d, ir, id]);
+        }
+      }
+    }
+  }
+
+  describe.each(podTests)("for [%d running, %d dead] & initial [%d running, %d dead]", (running, dead, initRunning, initDead) => {
+    const pod = getDummyPod({ running, dead, initRunning, initDead });
+
+    function getNamedContainer(name: string) {
+      return {
+        image: "dummy",
+        imagePullPolicy: "dummy",
+        name
+      };
+    }
+
+    it("getRunningContainers should return only running and init running", () => {
+      const res = [
+        ...Array.from(new Array(running), (val, index) => getNamedContainer(`container_r_${index}`)),
+        ...Array.from(new Array(initRunning), (val, index) => getNamedContainer(`container_ir_${index}`)),
+      ];
+
+      expect(pod.getRunningContainers()).toStrictEqual(res);
+    });
+
+    it("getAllContainers should return all containers", () => {
+      const res = [
+        ...Array.from(new Array(running), (val, index) => getNamedContainer(`container_r_${index}`)),
+        ...Array.from(new Array(dead), (val, index) => getNamedContainer(`container_d_${index}`)),
+        ...Array.from(new Array(initRunning), (val, index) => getNamedContainer(`container_ir_${index}`)),
+        ...Array.from(new Array(initDead), (val, index) => getNamedContainer(`container_id_${index}`)),
+      ];
+
+      expect(pod.getAllContainers()).toStrictEqual(res);
+    });
+
+    it("getRestartsCount should return total restart counts", () => {
+      function sum(len: number): number {
+        let res = 0;
+
+        for (let i = 0; i < len; i += 1) {
+          res += i;
+        }
+
+        return res;
+      }
+
+      expect(pod.getRestartsCount()).toStrictEqual(sum(running) + sum(dead));
+    });
+
+    it("hasIssues should return false", () => {
+      expect(pod.hasIssues()).toStrictEqual(false);
+    });
+  });
+
+  describe("getSelectedNodeOs", () => {
+    it("should return stable", () => {
+      const pod = getDummyPod();
+
+      pod.spec.nodeSelector = {
+        "kubernetes.io/os": "foobar"
+      };
+
+      expect(pod.getSelectedNodeOs()).toStrictEqual("foobar");
+    });
+
+    it("should return beta", () => {
+      const pod = getDummyPod();
+
+      pod.spec.nodeSelector = {
+        "beta.kubernetes.io/os": "foobar1"
+      };
+
+      expect(pod.getSelectedNodeOs()).toStrictEqual("foobar1");
+    });
+
+    it("should return stable over beta", () => {
+      const pod = getDummyPod();
+
+      pod.spec.nodeSelector = {
+        "kubernetes.io/os": "foobar2",
+        "beta.kubernetes.io/os": "foobar3"
+      };
+
+      expect(pod.getSelectedNodeOs()).toStrictEqual("foobar2");
+    });
+
+    it("should return undefined if none set", () => {
+      const pod = getDummyPod();
+
+      expect(pod.getSelectedNodeOs()).toStrictEqual(undefined);
+    });
+  });
+
+  describe("hasIssues", () => {
+    it("should return true if a condition isn't ready", () => {
+      const pod = getDummyPod({ running: 1 });
+
+      pod.status.conditions.push({
+        type: "Ready",
+        status: "foobar",
+        lastProbeTime: 1,
+        lastTransitionTime: "longer ago"
+      });
+
+      expect(pod.hasIssues()).toStrictEqual(true);
+    });
+
+    it("should return false if a condition is non-ready", () => {
+      const pod = getDummyPod({ running: 1 });
+
+      pod.status.conditions.push({
+        type: "dummy",
+        status: "foobar",
+        lastProbeTime: 1,
+        lastTransitionTime: "longer ago"
+      });
+
+      expect(pod.hasIssues()).toStrictEqual(false);
+    });
+
+    it("should return true if a current container is in a crash loop back off", () => {
+      const pod = getDummyPod({ running: 1 });
+
+      pod.status.containerStatuses[0].state = {
+        waiting: {
+          reason: "CrashLookBackOff",
+          message: "too much foobar"
+        }
+      };
+
+      expect(pod.hasIssues()).toStrictEqual(true);
+    });
+
+    it("should return true if a current phase isn't running", () => {
+      const pod = getDummyPod({ running: 1 });
+
+      pod.status.phase = "not running";
+
+      expect(pod.hasIssues()).toStrictEqual(true);
+    });
+
+    it("should return false if a current phase is running", () => {
+      const pod = getDummyPod({ running: 1 });
+
+      pod.status.phase = "Running";
+
+      expect(pod.hasIssues()).toStrictEqual(false);
+    });
+  });
+});

src/renderer/api/endpoints/pods.api.ts

@@ -270,60 +270,55 @@ export class Pod extends WorkloadKubeObject {
   }
 
   getAllContainers() {
-    return this.getContainers().concat(this.getInitContainers());
+    return [...this.getContainers(), ...this.getInitContainers()];
   }
 
   getRunningContainers() {
-    const statuses = this.getContainerStatuses();
+    const runningContainerNames = new Set(
-
+      this.getContainerStatuses()
-    return this.getAllContainers().filter(container => {
+        .filter(({ state }) => state.running)
-      return statuses.find(status => status.name === container.name && !!status.state["running"]);
+        .map(({ name }) => name)
-    }
     );
+
+    return this.getAllContainers()
+      .filter(({ name }) => runningContainerNames.has(name));
   }
 
   getContainerStatuses(includeInitContainers = true) {
-    const statuses: IPodContainerStatus[] = [];
+    const { containerStatuses = [], initContainerStatuses = [] } = this.status ?? {};
-    const { containerStatuses, initContainerStatuses } = this.status;
 
-    if (containerStatuses) {
+    if (includeInitContainers) {
-      statuses.push(...containerStatuses);
+      return [...containerStatuses, ...initContainerStatuses];
     }
 
-    if (includeInitContainers && initContainerStatuses) {
+    return [...containerStatuses];
-      statuses.push(...initContainerStatuses);
-    }
-
-    return statuses;
   }
 
   getRestartsCount(): number {
-    const { containerStatuses } = this.status;
+    const { containerStatuses = [] } = this.status ?? {};
 
-    if (!containerStatuses) return 0;
+    return containerStatuses.reduce((totalCount, { restartCount }) => totalCount + restartCount, 0);
-
-    return containerStatuses.reduce((count, item) => count + item.restartCount, 0);
   }
 
   getQosClass() {
-    return this.status.qosClass || "";
+    return this.status?.qosClass || "";
   }
 
   getReason() {
-    return this.status.reason || "";
+    return this.status?.reason || "";
   }
 
   getPriorityClassName() {
     return this.spec.priorityClassName || "";
   }
 
-  // Returns one of 5 statuses: Running, Succeeded, Pending, Failed, Evicted
+  getStatus(): PodStatus {
-  getStatus() {
     const phase = this.getStatusPhase();
     const reason = this.getReason();
-    const goodConditions = ["Initialized", "Ready"].every(condition =>
+    const trueConditionTypes = new Set(this.getConditions()
-      !!this.getConditions().find(item => item.type === condition && item.status === "True")
+      .filter(({ status }) => status === "True")
-    );
+      .map(({ type }) => type));
+    const isInGoodCondition = ["Initialized", "Ready"].every(condition => trueConditionTypes.has(condition));
 
     if (reason === PodStatus.EVICTED) {
       return PodStatus.EVICTED;
@@ -337,7 +332,7 @@ export class Pod extends WorkloadKubeObject {
       return PodStatus.SUCCEEDED;
     }
 
-    if (phase === PodStatus.RUNNING && goodConditions) {
+    if (phase === PodStatus.RUNNING && isInGoodCondition) {
       return PodStatus.RUNNING;
     }
 
@@ -349,37 +344,27 @@ export class Pod extends WorkloadKubeObject {
     if (this.getReason() === PodStatus.EVICTED) return "Evicted";
     if (this.getStatus() === PodStatus.RUNNING && this.metadata.deletionTimestamp) return "Terminating";
 
-    let message = "";
     const statuses = this.getContainerStatuses(false); // not including initContainers
 
-    if (statuses.length) {
+    for (const { state } of statuses.reverse()) {
-      statuses.forEach(status => {
+      if (state.waiting) {
-        const { state } = status;
+        return state.waiting.reason || "Waiting";
+      }
 
-        if (state.waiting) {
+      if (state.terminated) {
-          const { reason } = state.waiting;
+        return state.terminated.reason || "Terminated";
-
+      }
-          message = reason ? reason : "Waiting";
-        }
-
-        if (state.terminated) {
-          const { reason } = state.terminated;
-
-          message = reason ? reason : "Terminated";
-        }
-      });
     }
-    if (message) return message;
 
     return this.getStatusPhase();
   }
 
   getStatusPhase() {
-    return this.status.phase;
+    return this.status?.phase;
   }
 
   getConditions() {
-    return this.status.conditions || [];
+    return this.status?.conditions || [];
   }
 
   getVolumes() {
@@ -393,9 +378,7 @@ export class Pod extends WorkloadKubeObject {
   }
 
   getNodeSelectors(): string[] {
-    const { nodeSelector } = this.spec;
+    const { nodeSelector = {} } = this.spec;
-
-    if (!nodeSelector) return [];
 
     return Object.entries(nodeSelector).map(values => values.join(": "));
   }
@@ -409,20 +392,19 @@ export class Pod extends WorkloadKubeObject {
   }
 
   hasIssues() {
-    const notReady = !!this.getConditions().find(condition => {
+    for (const { type, status } of this.getConditions()) {
-      return condition.type == "Ready" && condition.status !== "True";
+      if (type === "Ready" && status !== "True") {
-    });
+        return true;
-    const crashLoop = !!this.getContainerStatuses().find(condition => {
+      }
-      const waiting = condition.state.waiting;
+    }
 
-      return (waiting && waiting.reason == "CrashLoopBackOff");
+    for (const { state } of this.getContainerStatuses()) {
-    });
+      if (state?.waiting?.reason === "CrashLookBackOff") {
+        return true;
+      }
+    }
 
-    return (
+    return this.getStatusPhase() !== "Running";
-      notReady ||
-      crashLoop ||
-      this.getStatusPhase() !== "Running"
-    );
   }
 
   getLivenessProbe(container: IPodContainer) {
@@ -476,14 +458,11 @@ export class Pod extends WorkloadKubeObject {
   }
 
   getNodeName() {
-    return this.spec?.nodeName;
+    return this.spec.nodeName;
   }
 
-  getSelectedNodeOs() {
+  getSelectedNodeOs(): string | undefined {
-    if (!this.spec.nodeSelector) return;
+    return this.spec.nodeSelector?.["kubernetes.io/os"] || this.spec.nodeSelector?.["beta.kubernetes.io/os"];
-    if (!this.spec.nodeSelector["kubernetes.io/os"] && !this.spec.nodeSelector["beta.kubernetes.io/os"]) return;
-
-    return this.spec.nodeSelector["kubernetes.io/os"] || this.spec.nodeSelector["beta.kubernetes.io/os"];
   }
 }