diff --git a/src/pepr/operator/controllers/network/generators/kubeNodes.spec.ts b/src/pepr/operator/controllers/network/generators/kubeNodes.spec.ts index 2b933bba9..9ddd137df 100644 --- a/src/pepr/operator/controllers/network/generators/kubeNodes.spec.ts +++ b/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(); }); }); diff --git a/src/pepr/operator/controllers/network/generators/kubeNodes.ts b/src/pepr/operator/controllers/network/generators/kubeNodes.ts index e2cdb8ef8..97a8f96db 100644 --- a/src/pepr/operator/controllers/network/generators/kubeNodes.ts +++ b/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); } } }