update to no longer care about ready status, and to not make extra ne…

…tpol modifications

Signed-off-by: catsby <[email protected]>

src/pepr/operator/controllers/network/generators/kubeNodes.spec.ts

@@ -137,22 +137,6 @@ describe("kubeNodes module", () => {
       expect(mockApply).toHaveBeenCalled();
     });
 
-    it("should not add a node IP if node not ready", async () => {
-      const notReadyNode = {
-        metadata: { name: "node3" },
-        status: {
-          addresses: [{ type: "InternalIP", address: "10.0.0.3" }],
-          conditions: [{ type: "Ready", status: "False" }],
-        },
-      };
-      mockK8sGetNodes.mockResolvedValueOnce({ items: [] });
-      await initAllNodesTarget(); // start empty
-      await updateKubeNodesFromCreateUpdate(notReadyNode);
-      const cidrs = kubeNodes();
-      expect(cidrs).toEqual([anywhere]);
-      expect(mockApply).toHaveBeenCalled(); // Still called to update polices even if empty
-    });
-
     it("should not remove a node that's no longer ready", async () => {
       mockK8sGetNodes.mockResolvedValue(mockNodeList);
       await initAllNodesTarget();
@@ -182,7 +166,38 @@ describe("kubeNodes module", () => {
           { ipBlock: { cidr: "10.0.0.2/32" } },
         ]),
       );
-      expect(mockApply).toHaveBeenCalled(); // Still called to update polices even if empty
+    });
+
+    it("should not apply netpol policy changes if a node is already included", async () => {
+      // setup 1 node in the set and expect 1 application to a policy
+      mockK8sGetNodes.mockResolvedValueOnce({ items: [] });
+      mockGetNetworkPolicies.mockResolvedValue(mockNetworkPolicyList);
+      await initAllNodesTarget(); // start empty
+      // add a node even if it's not ready
+      const initialNode = {
+        metadata: { name: "node1" },
+        status: {
+          addresses: [{ type: "InternalIP", address: "10.0.0.9" }],
+          conditions: [{ type: "Ready", status: "False" }],
+        },
+      };
+      await updateKubeNodesFromCreateUpdate(initialNode);
+      let cidrs = kubeNodes();
+      expect(cidrs).toHaveLength(1);
+      expect(cidrs[0].ipBlock?.cidr).toBe("10.0.0.9/32");
+      expect(mockApply).toHaveBeenCalled();
+
+      // clear out the apply from the setup
+      mockApply.mockClear();
+      // change initialNode to set the status to ready
+      initialNode.status.conditions[0].status = "True";
+      await updateKubeNodesFromCreateUpdate(initialNode);
+      cidrs = kubeNodes();
+      expect(cidrs).toHaveLength(1);
+      expect(cidrs[0].ipBlock?.cidr).toBe("10.0.0.9/32");
+
+      // the apply should not have been called
+      expect(mockApply).not.toHaveBeenCalled();
     });
   });
 

src/pepr/operator/controllers/network/generators/kubeNodes.ts

@@ -4,8 +4,9 @@
  */
 
 import { KubernetesListObject } from "@kubernetes/client-node";
-import { V1NetworkPolicyPeer, V1NodeCondition, V1NodeAddress } from "@kubernetes/client-node";
-import { K8s, kind } from "pepr";
+import { V1NetworkPolicyPeer, V1NodeAddress } from "@kubernetes/client-node";
+import { K8s, kind, R } from "pepr";
+
 import { Component, setupLogger } from "../../../../logger";
 import { RemoteGenerated } from "../../../crd";
 import { anywhere } from "./anywhere";
@@ -61,18 +62,8 @@ export function kubeNodes(): V1NetworkPolicyPeer[] {
  * rebuild the policies, and update the NetworkPolicies.
  */
 export async function updateKubeNodesFromCreateUpdate(node: kind.Node) {
-  const isReady = node.status?.conditions?.some(
-    (condition: V1NodeCondition) => condition.type === "Ready" && condition.status === "True",
-  );
-
   const ip = getNodeInternalIP(node);
-  // in order to be added to the node set, the node must be ready. Additional
-  // status conditions will not cause the node to be removed however, e.g. disck
-  // pressure, memory pressure, etc. Nodes are only removed from the node set
-  // when they are deleted, and done so in the updateKubeNodesFromDelete function.
-  if (isReady) {
-    if (ip) nodeSet.add(ip);
-  }
+  if (ip) nodeSet.add(ip);
 
   await updateKubeNodesNetworkPolicies();
 }
@@ -113,32 +104,44 @@ export async function updateKubeNodesNetworkPolicies() {
       continue;
     }
 
+    let updateRequired = false;
     if (netPol.spec.egress) {
       netPol.spec.egress[0] = netPol.spec.egress[0] || { to: [] };
-      netPol.spec.egress[0].to = newNodes;
+      const oldNodes = netPol.spec.egress[0].to;
+      if (!R.equals(oldNodes, newNodes)) {
+        updateRequired = true;
+        netPol.spec.egress[0].to = newNodes;
+      }
     } else if (netPol.spec.ingress) {
       netPol.spec.ingress[0] = netPol.spec.ingress[0] || { from: [] };
-      netPol.spec.ingress[0].from = newNodes;
+      const oldNodes = netPol.spec.ingress[0].from;
+      if (!R.equals(oldNodes, newNodes)) {
+        updateRequired = true;
+        netPol.spec.ingress[0].from = newNodes;
+      }
     }
 
-    if (netPol.metadata) {
-      // Remove managed fields to prevent server-side apply errors
-      netPol.metadata.managedFields = undefined;
-    }
+    // If the policy required a change, apply the new policy
+    if (updateRequired) {
+      if (netPol.metadata) {
+        // Remove managed fields to prevent server-side apply errors
+        netPol.metadata.managedFields = undefined;
+      }
+
+      log.debug(
+        `Updating KubeNodes NetworkPolicy ${netPol.metadata?.namespace}/${netPol.metadata?.name} with new CIDRs.`,
+      );
 
-    log.debug(
-      `Updating KubeNodes NetworkPolicy ${netPol.metadata?.namespace}/${netPol.metadata?.name} with new CIDRs.`,
-    );
-
-    try {
-      await K8s(kind.NetworkPolicy).Apply(netPol, { force: true });
-    } catch (err) {
-      let message = err.data?.message || "Unknown error while applying KubeNode network policies";
-      if (UDSConfig.kubeNodeCidrs) {
-        message +=
-          ", ensure that the KUBENODE_CIDRS override configured for the operator is correct.";
+      try {
+        await K8s(kind.NetworkPolicy).Apply(netPol, { force: true });
+      } catch (err) {
+        let message = err.data?.message || "Unknown error while applying KubeNode network policies";
+        if (UDSConfig.kubeNodeCidrs) {
+          message +=
+            ", ensure that the KUBENODE_CIDRS override configured for the operator is correct.";
+        }
+        throw new Error(message);
       }
-      throw new Error(message);
     }
   }
 }