From c68325ec36ffe7ce8c5c9735643746660984c597 Mon Sep 17 00:00:00 2001 From: rohithb Date: Wed, 12 Aug 2026 02:12:15 +0530 Subject: [PATCH 01/12] feat(nvca): add control-plane cluster validator role, gateway and storage checks --- .../nvca/cmd/cluster-validator/BUILD.bazel | 4 + .../nvca/cmd/cluster-validator/main.go | 34 +- .../nvca/cmd/cluster-validator/main_test.go | 30 +- .../internal/clustervalidator/BUILD.bazel | 3 + .../nvca/internal/clustervalidator/checks.go | 468 ++++++++++++++++-- .../checks_controlplane_test.go | 278 +++++++++++ .../internal/clustervalidator/validator.go | 124 +++-- .../clustervalidator/validator_test.go | 116 ++++- 8 files changed, 985 insertions(+), 72 deletions(-) create mode 100644 src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go diff --git a/src/compute-plane-services/nvca/cmd/cluster-validator/BUILD.bazel b/src/compute-plane-services/nvca/cmd/cluster-validator/BUILD.bazel index ca27c79dc..78dd4c13c 100644 --- a/src/compute-plane-services/nvca/cmd/cluster-validator/BUILD.bazel +++ b/src/compute-plane-services/nvca/cmd/cluster-validator/BUILD.bazel @@ -13,6 +13,7 @@ go_library( "//src/compute-plane-services/nvca/cmd/internal", "//src/compute-plane-services/nvca/internal/clustervalidator", "//src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/core", + "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/dynamic", ], ) @@ -42,4 +43,7 @@ go_test( name = "cluster-validator_test", srcs = ["main_test.go"], embed = [":cluster-validator_lib"], + deps = [ + "//src/compute-plane-services/nvca/internal/clustervalidator", + ], ) diff --git a/src/compute-plane-services/nvca/cmd/cluster-validator/main.go b/src/compute-plane-services/nvca/cmd/cluster-validator/main.go index c479dfc76..b786241d4 100644 --- a/src/compute-plane-services/nvca/cmd/cluster-validator/main.go +++ b/src/compute-plane-services/nvca/cmd/cluster-validator/main.go @@ -23,6 +23,7 @@ import ( "strings" "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/core" + "k8s.io/client-go/dynamic" internalutil "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/cmd/internal" "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/internal/clustervalidator" @@ -39,11 +40,21 @@ func main() { log := core.GetLogger(ctx) log.Logger.SetFormatter(&clustervalidator.CLIFormatter{}) - client, _, err := internalutil.NewK8sClient(ctx, "") + client, restCfg, err := internalutil.NewK8sClient(ctx, "") if err != nil { log.WithError(err).Fatal("Failed to create Kubernetes client") } + // Build the dynamic client from the same REST config. Used for listing + // Gateway API custom resources (HTTPRoutes, etc.) which are not in the + // typed k8s.io/client-go clientset. Failure is non-fatal: checkGatewayRoutes + // skips gracefully when dynClient is nil. + dynClient, err := dynamic.NewForConfig(restCfg) + if err != nil { + log.WithError(err).Warn("Could not create dynamic client; gateway route check will be skipped") + dynClient = nil + } + configNS := os.Getenv("VALIDATOR_CONFIG_NAMESPACE") if configNS == "" { configNS = podNamespace() @@ -72,11 +83,30 @@ func main() { clustervalidator.SummaryConfigMapNamespaceEnv) } - if err := clustervalidator.Run(ctx, client, configNS, configName, summaryNS, emitMetrics); err != nil { + // VALIDATOR_ROLE selects which check set runs: "control-plane" enables + // gateway and StorageClass checks and skips GPU/SMB; anything else (including + // unset) runs the compute-plane check set (backward-compatible default). + role := parseRole(os.Getenv("VALIDATOR_ROLE")) + + if err := clustervalidator.Run(ctx, client, dynClient, configNS, configName, summaryNS, emitMetrics, role); err != nil { log.WithError(err).Fatal("Cluster validation failed") } } +// parseRole normalizes the VALIDATOR_ROLE env value. Returns the matching +// clustervalidator constant for "control-plane" or "compute-plane"; returns "" +// (compute-plane default) for any other value so unknown inputs are safe. +func parseRole(v string) string { + switch strings.ToLower(strings.TrimSpace(v)) { + case clustervalidator.RoleControlPlane: + return clustervalidator.RoleControlPlane + case clustervalidator.RoleComputePlane: + return clustervalidator.RoleComputePlane + default: + return "" + } +} + // preflightMode reports whether this is a one-shot preflight run (e.g. nvcf-cli, // before NVCA is installed), which skips the summary write. Read from an env // (not a flag) so an unknown value is ignored rather than crashing arg parsing. diff --git a/src/compute-plane-services/nvca/cmd/cluster-validator/main_test.go b/src/compute-plane-services/nvca/cmd/cluster-validator/main_test.go index 29c5a8213..4052f778e 100644 --- a/src/compute-plane-services/nvca/cmd/cluster-validator/main_test.go +++ b/src/compute-plane-services/nvca/cmd/cluster-validator/main_test.go @@ -17,7 +17,35 @@ limitations under the License. package main -import "testing" +import ( + "testing" + + "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/internal/clustervalidator" +) + +func TestParseRole(t *testing.T) { + tests := []struct { + in string + want string + }{ + // Known roles are normalized. + {"control-plane", clustervalidator.RoleControlPlane}, + {"CONTROL-PLANE", clustervalidator.RoleControlPlane}, + {" control-plane ", clustervalidator.RoleControlPlane}, + {"compute-plane", clustervalidator.RoleComputePlane}, + {"COMPUTE-PLANE", clustervalidator.RoleComputePlane}, + // Unknown values (including unset) fall back to "" = compute-plane default. + {"", ""}, + {"gpu", ""}, + {"both", ""}, + {"control_plane", ""}, // underscore, not hyphen + } + for _, tt := range tests { + if got := parseRole(tt.in); got != tt.want { + t.Errorf("parseRole(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} func TestPreflightMode(t *testing.T) { tests := []struct { diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel b/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel index 92465815c..a24647e2c 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel +++ b/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel @@ -25,8 +25,10 @@ go_library( "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/api/errors", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/api/resource", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/apis/meta/v1:meta", + "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/runtime/schema", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/util/intstr", "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/discovery", + "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/dynamic", "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/kubernetes", "//src/compute-plane-services/nvca/vendor/sigs.k8s.io/yaml", ], @@ -41,6 +43,7 @@ alias( go_test( name = "clustervalidator_test", srcs = [ + "checks_controlplane_test.go", "checks_test.go", "config_test.go", "enforcement_test.go", diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index 75c2cc031..82987fe66 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -25,11 +25,14 @@ import ( "sort" "strconv" "strings" + "time" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/discovery" + "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" ) @@ -112,18 +115,8 @@ func summarizeContainerRuntimes(nodes []corev1.Node) string { return strings.Join(parts, ", ") } -// checkControlPlaneHealth verifies cluster health using three signals: -// 1. /readyz — canonical API-server health (works on every distribution). -// 2. Data-plane capabilities — DNS resolution of kubernetes.default.svc -// and HTTPS routing to kubernetes.default.svc/readyz via the in-cluster -// ClusterIP. Both must succeed; pod-presence detection (CoreDNS vs -// kube-dns, kube-proxy vs Cilium vs OVN-Kubernetes vs k3s-embedded) -// is diagnostic only and does not affect the verdict. -// 3. Control-plane pods (kube-apiserver, etcd, scheduler, controller-manager) -// — informational only. Visible on self-hosted, hidden on managed K8s -// (EKS, GKE, AKS) where the cloud provider runs them. /readyz already -// covers their health. -// +// checkControlPlaneHealth verifies /readyz, in-cluster DNS, and service routing. +// Control-plane pod presence is informational only; /readyz is authoritative. // NotReady worker nodes are Warning only (non-blocking). func checkControlPlaneHealth(ctx context.Context, client kubernetes.Interface, state *ValidationState) { log := state.Log @@ -310,16 +303,9 @@ var ( probeAPIServiceIPFn = probeKubernetesAPIServiceIP ) -// detectDNSProvider inspects kube-system pods and returns a short name for -// the cluster's DNS provider when recognised. Diagnostic only — the -// authoritative DNS health signal comes from probeInClusterDNS. -// -// Known providers: -// - CoreDNS: pod prefix "coredns" (vanilla, kubeadm, EKS, AKS, k3s) -// - kube-dns: pod prefix "kube-dns" (GKE's managed default) -// - OpenShift DNS: namespace openshift-dns hosts dns-default-*; this -// function only sees kube-system pods, so OpenShift returns "" here -// and the capability probe is authoritative. +// detectDNSProvider inspects kube-system pods and returns a short provider +// name (CoreDNS, kube-dns) when recognised. Diagnostic only; the authoritative +// DNS health signal comes from probeInClusterDNS. func detectDNSProvider(pods []corev1.Pod) string { switch { case countRunningPods(pods, "coredns") > 0: @@ -330,16 +316,9 @@ func detectDNSProvider(pods []corev1.Pod) string { return "" } -// detectServiceRoutingImpl inspects the K8s version and kube-system pods -// to identify the cluster's kube-proxy implementation. Diagnostic only — -// the authoritative routing health signal comes from -// probeKubernetesAPIServiceIP. -// -// Recognised implementations: -// - kube-proxy DaemonSet (vanilla / kubeadm / EKS / AKS / GKE classic) -// - kube-proxy embedded in the server binary (k3s / rke2) -// - Cilium with kubeProxyReplacement (GKE Dataplane V2, custom Cilium) -// - OVN-Kubernetes (OpenShift 4.x default) +// detectServiceRoutingImpl inspects K8s version and kube-system pods to +// identify the kube-proxy implementation (DaemonSet, k3s/rke2 embedded, +// Cilium, OVN-Kubernetes). Diagnostic only; probeKubernetesAPIServiceIP is authoritative. func detectServiceRoutingImpl(k8sVersion string, pods []corev1.Pod) string { switch { case isEmbeddedKubeProxyDistro(k8sVersion): @@ -833,6 +812,431 @@ func checkGPUOperator(ctx context.Context, client kubernetes.Interface, state *V } } +// checkStorageClass verifies that a default StorageClass is present. NVCF +// workloads use PersistentVolumeClaims; without a default StorageClass those +// claims remain unbound and workloads fail to start. Critical for both +// control-plane (operator chart) and compute-plane (model cache), but surfaced +// here for the control-plane validator role. +func checkStorageClass(ctx context.Context, client kubernetes.Interface, state *ValidationState) { + log := state.Log + printHeader(log, "Default StorageClass") + + classes, err := client.StorageV1().StorageClasses().List(ctx, metav1.ListOptions{}) + if err != nil { + printWarning(log, fmt.Sprintf("Could not list StorageClasses: %v", err)) + ok := false + state.DefaultStorageClassOK = &ok + return + } + + var defaultClass string + for _, sc := range classes.Items { + if sc.Annotations["storageclass.kubernetes.io/is-default-class"] == "true" || + sc.Annotations["storageclass.beta.kubernetes.io/is-default-class"] == "true" { + defaultClass = sc.Name + break + } + } + + if defaultClass == "" { + printError(log, fmt.Sprintf("No default StorageClass found (%d classes present, none marked as default)", len(classes.Items))) + state.Recommendations = append(state.Recommendations, + "Mark a StorageClass as default with: "+ + "kubectl patch storageclass -p '{\"metadata\":{\"annotations\":{\"storageclass.kubernetes.io/is-default-class\":\"true\"}}}'") + ok := false + state.DefaultStorageClassOK = &ok + return + } + + printSuccess(log, fmt.Sprintf("Default StorageClass: %s", defaultClass)) + ok := true + state.DefaultStorageClassOK = &ok +} + +const ( + gatewayAPIGroup = "gateway.networking.k8s.io" + gatewayAPIVersion = "v1" + // envoyGatewayNamespace is the namespace created by the Envoy Gateway Helm chart. + envoyGatewayNamespace = "envoy-gateway-system" +) + +var requiredGatewayResources = []string{"gatewayclasses", "gateways", "httproutes", "grpcroutes"} + +// checkGatewayAPICRDs verifies that the Gateway API CRD set is installed and +// registers all four required resource types. Without these CRDs neither the +// Gateway controller nor nvcf-cli can create routing objects. +func checkGatewayAPICRDs(ctx context.Context, client kubernetes.Interface, state *ValidationState) { + log := state.Log + printHeader(log, "Gateway API CRDs") + + gv := gatewayAPIGroup + "/" + gatewayAPIVersion + resources, err := client.Discovery().ServerResourcesForGroupVersion(gv) + if err != nil { + printError(log, fmt.Sprintf("Gateway API CRDs not installed (%s not registered): %v", gv, err)) + state.Recommendations = append(state.Recommendations, + "Install Gateway API CRDs: kubectl apply -f https://github.com/kubernetes-sigs/gateway-api/releases/latest/download/standard-install.yaml") + ok := false + state.GatewayAPICRDsOK = &ok + return + } + + found := make(map[string]bool, len(resources.APIResources)) + for _, r := range resources.APIResources { + found[r.Name] = true + } + var missing []string + for _, r := range requiredGatewayResources { + if !found[r] { + missing = append(missing, r) + } + } + if len(missing) > 0 { + printError(log, fmt.Sprintf("Gateway API CRDs missing resources: %s", strings.Join(missing, ", "))) + ok := false + state.GatewayAPICRDsOK = &ok + return + } + + printSuccess(log, fmt.Sprintf("Gateway API CRDs installed (%s): %s", gv, strings.Join(requiredGatewayResources, ", "))) + ok := true + state.GatewayAPICRDsOK = &ok +} + +// checkEnvoyGateway verifies the Envoy Gateway controller is installed and has +// at least one running pod in the envoy-gateway-system namespace. Without a +// running gateway controller, Gateway and HTTPRoute objects are never reconciled +// and no traffic reaches NVCF services. +func checkEnvoyGateway(ctx context.Context, client kubernetes.Interface, state *ValidationState) { + log := state.Log + printHeader(log, "Envoy Gateway") + + _, err := client.CoreV1().Namespaces().Get(ctx, envoyGatewayNamespace, metav1.GetOptions{}) + if err != nil { + if apierrors.IsNotFound(err) { + printError(log, fmt.Sprintf("Envoy Gateway namespace %s not found", envoyGatewayNamespace)) + } else { + printError(log, fmt.Sprintf("Could not check Envoy Gateway namespace: %v", err)) + } + state.Recommendations = append(state.Recommendations, + "Install Envoy Gateway via the NVCF self-managed stack (nvcf-cli up) or "+ + "helm install eg oci://docker.io/envoyproxy/gateway-helm -n envoy-gateway-system --create-namespace") + ok := false + state.EnvoyGatewayOK = &ok + return + } + + pods, err := client.CoreV1().Pods(envoyGatewayNamespace).List(ctx, metav1.ListOptions{}) + if err != nil { + printError(log, fmt.Sprintf("Could not list Envoy Gateway pods: %v", err)) + ok := false + state.EnvoyGatewayOK = &ok + return + } + + running := 0 + for i := range pods.Items { + if pods.Items[i].Status.Phase == corev1.PodRunning { + running++ + } + } + log.Infof(" Pods in %s: %d total, %d running", envoyGatewayNamespace, len(pods.Items), running) + + if running == 0 { + printError(log, fmt.Sprintf("No running pods found in %s", envoyGatewayNamespace)) + ok := false + state.EnvoyGatewayOK = &ok + return + } + + printSuccess(log, fmt.Sprintf("Envoy Gateway: %d pod(s) running in %s", running, envoyGatewayNamespace)) + ok := true + state.EnvoyGatewayOK = &ok +} + +// checkGatewayRoutes lists HTTPRoutes across all namespaces using the dynamic +// client. At least one HTTPRoute must exist for traffic to reach NVCF +// services. When dynClient is nil the check is silently skipped (used in tests +// or early preflight before Gateway API CRDs are installed). +// +// Non-critical: routes may be deployed after the gateway infrastructure, and +// their absence does not block the cluster verdict. +func checkGatewayRoutes(ctx context.Context, dynClient dynamic.Interface, state *ValidationState) { + log := state.Log + printHeader(log, "Gateway Routes") + + if dynClient == nil { + printInfo(log, " Gateway route check skipped (no dynamic client configured)") + return + } + + gvr := schema.GroupVersionResource{ + Group: gatewayAPIGroup, + Version: gatewayAPIVersion, + Resource: "httproutes", + } + list, err := dynClient.Resource(gvr).Namespace("").List(ctx, metav1.ListOptions{}) + if err != nil { + printWarning(log, fmt.Sprintf("Could not list HTTPRoutes: %v", err)) + state.Warnings = append(state.Warnings, + "Gateway Routes: could not list HTTPRoutes — verify Gateway API CRDs are installed") + ok := false + state.GatewayRoutesOK = &ok + return + } + + count := len(list.Items) + if count == 0 { + printWarning(log, "No HTTPRoutes found in any namespace") + state.Warnings = append(state.Warnings, + "Gateway Routes: no HTTPRoutes found — routes may not yet be deployed by nvcf-cli") + ok := false + state.GatewayRoutesOK = &ok + return + } + + printSuccess(log, fmt.Sprintf("HTTPRoutes present: %d", count)) + for i := range list.Items { + printInfo(log, fmt.Sprintf(" %s/%s", list.Items[i].GetNamespace(), list.Items[i].GetName())) + } + ok := true + state.GatewayRoutesOK = &ok +} + +// checkExternalLoadBalancer performs a passive check: it lists all Services of +// type LoadBalancer across all namespaces and looks for one with a populated +// .status.loadBalancer.ingress. A populated ingress means a load balancer +// controller (cloud LB, MetalLB, etc.) is active and assigned an IP or hostname. +// +// Non-critical: the passive form only detects an existing LB service; it does +// not create a probe service, so absence means either no LB service exists yet +// or no LB controller is installed. +func checkExternalLoadBalancer(ctx context.Context, client kubernetes.Interface, state *ValidationState) { + log := state.Log + printHeader(log, "External Load Balancer") + + services, err := client.CoreV1().Services("").List(ctx, metav1.ListOptions{}) + if err != nil { + printWarning(log, fmt.Sprintf("Could not list services: %v", err)) + ok := false + state.ExternalLBOK = &ok + return + } + + type lbResult struct { + name string + namespace string + addr string + } + var found []lbResult + for i := range services.Items { + svc := &services.Items[i] + if svc.Spec.Type != corev1.ServiceTypeLoadBalancer { + continue + } + for _, ing := range svc.Status.LoadBalancer.Ingress { + addr := ing.IP + if addr == "" { + addr = ing.Hostname + } + if addr != "" { + found = append(found, lbResult{svc.Name, svc.Namespace, addr}) + break + } + } + } + + if len(found) == 0 { + printWarning(log, "No LoadBalancer Services with an assigned external address found") + printInfo(log, " This may indicate: no LB controller is installed (MetalLB, cloud LB), "+ + "or no LoadBalancer Service exists yet (normal before nvcf-cli up)") + state.Warnings = append(state.Warnings, + "External Load Balancer: no Service of type LoadBalancer has an assigned external IP or hostname. "+ + "Verify a load balancer controller is installed.") + ok := false + state.ExternalLBOK = &ok + return + } + + printSuccess(log, fmt.Sprintf("%d LoadBalancer Service(s) with external address:", len(found))) + for _, svc := range found { + printInfo(log, fmt.Sprintf(" %s/%s → %s", svc.namespace, svc.name, svc.addr)) + } + ok := true + state.ExternalLBOK = &ok +} + +const ( + nodeToNodeTestPort = 19999 + nodeToNodeImage = enforcementDefaultImg // busybox:1.36 + nodeToNodeNamespace = "default" + nodeToNodeServerName = "nvcf-n2n-server" + nodeToNodeClientName = "nvcf-n2n-client" + // 90 s per pod matches enforcementPodTimeout — image should already be + // cached from the enforcement check that ran earlier in the same run. + nodeToNodePodTimeout = 90 * time.Second +) + +// checkNodeToNode verifies raw overlay-network connectivity between two +// schedulable nodes. It pins a TCP server pod (busybox nc) to node A and a +// client pod (nc -z) to node B, then checks whether the TCP connect succeeds. +// +// Single-node clusters are skipped with a passing warning: inter-node +// connectivity is not applicable when there is only one node. +// +// Critical: broken overlay means NVCF services on different nodes cannot +// communicate, causing cascade failures across every API call. +func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *ValidationState) { + log := state.Log + printHeader(log, "Node-to-Node Communication") + + nodes, err := client.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) + if err != nil { + printWarning(log, fmt.Sprintf("Could not list nodes: %v", err)) + ok := false + state.NodeToNodeOK = &ok + return + } + + var schedulable []string + for i := range nodes.Items { + if !nodes.Items[i].Spec.Unschedulable { + schedulable = append(schedulable, nodes.Items[i].Name) + } + } + + if len(schedulable) < 2 { + printInfo(log, fmt.Sprintf(" %d schedulable node(s) — node-to-node check skipped (not applicable for single-node clusters)", len(schedulable))) + state.Warnings = append(state.Warnings, + "Node-to-Node: skipped — fewer than 2 schedulable nodes; not applicable for single-node clusters") + ok := true + state.NodeToNodeOK = &ok + return + } + + nodeA, nodeB := schedulable[0], schedulable[1] + log.Infof(" Probing overlay connectivity: %s → %s", nodeA, nodeB) + + suffix := fmt.Sprintf("%d", time.Now().UnixNano()%1000000) + serverName := nodeToNodeServerName + "-" + suffix + clientName := nodeToNodeClientName + "-" + suffix + + // Deferred cleanup uses a fresh context so it runs even when ctx is expired. + defer func() { + grace := int64(0) + opts := metav1.DeleteOptions{GracePeriodSeconds: &grace} + _ = client.CoreV1().Pods(nodeToNodeNamespace).Delete(context.Background(), serverName, opts) + _ = client.CoreV1().Pods(nodeToNodeNamespace).Delete(context.Background(), clientName, opts) + }() + + if _, err := client.CoreV1().Pods(nodeToNodeNamespace).Create( + ctx, buildNodeToNodeServerPod(serverName, nodeA), metav1.CreateOptions{}, + ); err != nil { + printError(log, fmt.Sprintf("Failed to create server pod on %s: %v", nodeA, err)) + ok := false + state.NodeToNodeOK = &ok + return + } + + if err := waitForPodReady(ctx, client, nodeToNodeNamespace, serverName, nodeToNodePodTimeout); err != nil { + printError(log, fmt.Sprintf("Server pod on %s not ready: %v", nodeA, err)) + ok := false + state.NodeToNodeOK = &ok + return + } + + serverIP, err := getPodIP(ctx, client, nodeToNodeNamespace, serverName) + if err != nil { + printError(log, fmt.Sprintf("Could not get server pod IP: %v", err)) + ok := false + state.NodeToNodeOK = &ok + return + } + log.Infof(" Server pod on %s has IP %s", nodeA, serverIP) + + if _, err := client.CoreV1().Pods(nodeToNodeNamespace).Create( + ctx, buildNodeToNodeClientPod(clientName, nodeB, serverIP), metav1.CreateOptions{}, + ); err != nil { + printError(log, fmt.Sprintf("Failed to create client pod on %s: %v", nodeB, err)) + ok := false + state.NodeToNodeOK = &ok + return + } + + succeeded, err := waitForPodDone(ctx, client, nodeToNodeNamespace, clientName, nodeToNodePodTimeout) + if err != nil { + printError(log, fmt.Sprintf("Client pod probe error: %v", err)) + ok := false + state.NodeToNodeOK = &ok + return + } + + if succeeded { + printSuccess(log, fmt.Sprintf("Node-to-node overlay connectivity verified: %s → %s (%s:%d)", + nodeB, nodeA, serverIP, nodeToNodeTestPort)) + ok := true + state.NodeToNodeOK = &ok + } else { + printError(log, fmt.Sprintf("Client on %s could not reach server on %s at %s:%d", + nodeB, nodeA, serverIP, nodeToNodeTestPort)) + printInfo(log, " Possible causes: CNI overlay misconfiguration, host firewall rules, "+ + "or cloud security group rules blocking inter-node pod traffic") + state.Recommendations = append(state.Recommendations, + fmt.Sprintf("Check host firewall and security groups between nodes %s and %s. "+ + "Verify the CNI overlay (VXLAN, Geneve, etc.) is not blocked.", nodeA, nodeB)) + ok := false + state.NodeToNodeOK = &ok + } +} + +func buildNodeToNodeServerPod(name, nodeName string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: nodeToNodeNamespace, + Labels: map[string]string{ + "app.kubernetes.io/managed-by": "nvcf-cli", + "app.kubernetes.io/component": "n2n-probe", + }, + }, + Spec: corev1.PodSpec{ + NodeName: nodeName, + RestartPolicy: corev1.RestartPolicyNever, + Containers: []corev1.Container{{ + Name: "server", + Image: nodeToNodeImage, + // Loop keeps the pod Running while we resolve its IP and + // start the client. The pod is cleaned up via a deferred + // background-context delete, not by natural exit. + Command: []string{"sh", "-c", fmt.Sprintf("while true; do nc -l -p %d; done", nodeToNodeTestPort)}, + Resources: enforcementResources(), + }}, + }, + } +} + +func buildNodeToNodeClientPod(name, nodeName, serverIP string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: nodeToNodeNamespace, + Labels: map[string]string{ + "app.kubernetes.io/managed-by": "nvcf-cli", + "app.kubernetes.io/component": "n2n-probe", + }, + }, + Spec: corev1.PodSpec{ + NodeName: nodeName, + RestartPolicy: corev1.RestartPolicyNever, + Containers: []corev1.Container{{ + Name: "client", + Image: nodeToNodeImage, + Command: []string{"sh", "-c", fmt.Sprintf("nc -z -w 5 %s %d", serverIP, nodeToNodeTestPort)}, + Resources: enforcementResources(), + }}, + }, + } +} + // checkConfigurableReachability probes user-defined endpoints loaded from the // cluster-validator ConfigMap. func checkConfigurableReachability(state *ValidationState, cfg *ReachabilityConfig) { diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go new file mode 100644 index 000000000..d030f1bb1 --- /dev/null +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go @@ -0,0 +1,278 @@ +/* +SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package clustervalidator + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + storagev1 "k8s.io/api/storage/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + ktesting "k8s.io/client-go/testing" +) + +// -- checkStorageClass -- + +func TestCheckStorageClass_DefaultPresent(t *testing.T) { + client := fake.NewSimpleClientset(&storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "standard", + Annotations: map[string]string{ + "storageclass.kubernetes.io/is-default-class": "true", + }, + }, + }) + state := &ValidationState{Log: testLog()} + checkStorageClass(context.Background(), client, state) + + require.NotNil(t, state.DefaultStorageClassOK) + assert.True(t, *state.DefaultStorageClassOK, "a StorageClass with the default annotation must set DefaultStorageClassOK=true") + assert.Empty(t, state.Recommendations) +} + +func TestCheckStorageClass_BetaAnnotationAlsoAccepted(t *testing.T) { + client := fake.NewSimpleClientset(&storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "local-path", + Annotations: map[string]string{ + "storageclass.beta.kubernetes.io/is-default-class": "true", + }, + }, + }) + state := &ValidationState{Log: testLog()} + checkStorageClass(context.Background(), client, state) + + require.NotNil(t, state.DefaultStorageClassOK) + assert.True(t, *state.DefaultStorageClassOK) +} + +func TestCheckStorageClass_NoDefault(t *testing.T) { + client := fake.NewSimpleClientset(&storagev1.StorageClass{ + ObjectMeta: metav1.ObjectMeta{Name: "no-annotation-class"}, + }) + state := &ValidationState{Log: testLog()} + checkStorageClass(context.Background(), client, state) + + require.NotNil(t, state.DefaultStorageClassOK) + assert.False(t, *state.DefaultStorageClassOK, "StorageClass without default annotation must set DefaultStorageClassOK=false") + assert.NotEmpty(t, state.Recommendations, "missing default StorageClass must add a recommendation") +} + +func TestCheckStorageClass_NoStorageClasses(t *testing.T) { + client := fake.NewSimpleClientset() + state := &ValidationState{Log: testLog()} + checkStorageClass(context.Background(), client, state) + + require.NotNil(t, state.DefaultStorageClassOK) + assert.False(t, *state.DefaultStorageClassOK) +} + +// -- checkGatewayAPICRDs -- +// The fake discovery client does not populate ServerResourcesForGroupVersion, +// so checkGatewayAPICRDs will always see the group as absent. +// We test that it runs without panic and sets GatewayAPICRDsOK=false. + +func TestCheckGatewayAPICRDs_AbsentOnFakeClient(t *testing.T) { + client := fake.NewSimpleClientset() + state := &ValidationState{Log: testLog()} + checkGatewayAPICRDs(context.Background(), client, state) + + require.NotNil(t, state.GatewayAPICRDsOK, + "GatewayAPICRDsOK must be set even when discovery returns an error") + assert.False(t, *state.GatewayAPICRDsOK, + "absent Gateway API CRDs must set GatewayAPICRDsOK=false") + assert.NotEmpty(t, state.Recommendations) +} + +// -- checkEnvoyGateway -- + +func TestCheckEnvoyGateway_RunningPods(t *testing.T) { + client := fake.NewSimpleClientset( + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: envoyGatewayNamespace}}, + makePod("envoy-gateway-abc", envoyGatewayNamespace, corev1.PodRunning), + ) + state := &ValidationState{Log: testLog()} + checkEnvoyGateway(context.Background(), client, state) + + require.NotNil(t, state.EnvoyGatewayOK) + assert.True(t, *state.EnvoyGatewayOK, "running Envoy Gateway pods must set EnvoyGatewayOK=true") +} + +func TestCheckEnvoyGateway_NamespaceAbsent(t *testing.T) { + client := fake.NewSimpleClientset() + state := &ValidationState{Log: testLog()} + checkEnvoyGateway(context.Background(), client, state) + + require.NotNil(t, state.EnvoyGatewayOK) + assert.False(t, *state.EnvoyGatewayOK, "absent namespace must set EnvoyGatewayOK=false") + assert.NotEmpty(t, state.Recommendations) +} + +func TestCheckEnvoyGateway_NamespacePresentNoRunningPods(t *testing.T) { + client := fake.NewSimpleClientset( + &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: envoyGatewayNamespace}}, + makePod("envoy-gateway-abc", envoyGatewayNamespace, corev1.PodPending), + ) + state := &ValidationState{Log: testLog()} + checkEnvoyGateway(context.Background(), client, state) + + require.NotNil(t, state.EnvoyGatewayOK) + assert.False(t, *state.EnvoyGatewayOK, "no running pods must set EnvoyGatewayOK=false") +} + +// -- checkGatewayRoutes -- + +func TestCheckGatewayRoutes_NilClientSkips(t *testing.T) { + state := &ValidationState{Log: testLog()} + // Should not panic or set GatewayRoutesOK. + checkGatewayRoutes(context.Background(), nil, state) + assert.Nil(t, state.GatewayRoutesOK, "nil dynClient must leave GatewayRoutesOK unset") +} + +// -- checkExternalLoadBalancer -- + +func TestCheckExternalLoadBalancer_ServiceWithIP(t *testing.T) { + client := fake.NewSimpleClientset(&corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "envoy-gateway", Namespace: envoyGatewayNamespace}, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeLoadBalancer}, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{{IP: "203.0.113.1"}}, + }, + }, + }) + state := &ValidationState{Log: testLog()} + checkExternalLoadBalancer(context.Background(), client, state) + + require.NotNil(t, state.ExternalLBOK) + assert.True(t, *state.ExternalLBOK, "a LB service with an assigned IP must set ExternalLBOK=true") +} + +func TestCheckExternalLoadBalancer_ServiceWithHostname(t *testing.T) { + client := fake.NewSimpleClientset(&corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "envoy-gateway", Namespace: envoyGatewayNamespace}, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeLoadBalancer}, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{{Hostname: "lb.example.com"}}, + }, + }, + }) + state := &ValidationState{Log: testLog()} + checkExternalLoadBalancer(context.Background(), client, state) + + require.NotNil(t, state.ExternalLBOK) + assert.True(t, *state.ExternalLBOK, "a LB service with a hostname must set ExternalLBOK=true") +} + +func TestCheckExternalLoadBalancer_NoLBServices(t *testing.T) { + client := fake.NewSimpleClientset(&corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-ip-svc", Namespace: "default"}, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeClusterIP}, + }) + state := &ValidationState{Log: testLog()} + checkExternalLoadBalancer(context.Background(), client, state) + + require.NotNil(t, state.ExternalLBOK) + assert.False(t, *state.ExternalLBOK, "no LB service must set ExternalLBOK=false") + assert.NotEmpty(t, state.Warnings) +} + +func TestCheckExternalLoadBalancer_LBServicePendingNoIP(t *testing.T) { + // LB type but .status.loadBalancer.ingress is empty → no IP assigned yet. + client := fake.NewSimpleClientset(&corev1.Service{ + ObjectMeta: metav1.ObjectMeta{Name: "pending-lb", Namespace: "default"}, + Spec: corev1.ServiceSpec{Type: corev1.ServiceTypeLoadBalancer}, + // No Status.LoadBalancer.Ingress + }) + state := &ValidationState{Log: testLog()} + checkExternalLoadBalancer(context.Background(), client, state) + + require.NotNil(t, state.ExternalLBOK) + assert.False(t, *state.ExternalLBOK, "LB service with no assigned IP must set ExternalLBOK=false") +} + +// -- checkNodeToNode -- + +func TestCheckNodeToNode_NoNodes(t *testing.T) { + client := fake.NewSimpleClientset() + state := &ValidationState{Log: testLog()} + checkNodeToNode(context.Background(), client, state) + + require.NotNil(t, state.NodeToNodeOK) + assert.True(t, *state.NodeToNodeOK, "zero schedulable nodes must skip with pass, not fail") + assert.NotEmpty(t, state.Warnings, "skip must add a warning") +} + +func TestCheckNodeToNode_SingleNode_Skip(t *testing.T) { + client := fake.NewSimpleClientset(makeNode("node-1", true, 0)) + state := &ValidationState{Log: testLog()} + checkNodeToNode(context.Background(), client, state) + + require.NotNil(t, state.NodeToNodeOK) + assert.True(t, *state.NodeToNodeOK, "single-node cluster must skip with pass, not fail") + assert.NotEmpty(t, state.Warnings) +} + +func TestCheckNodeToNode_UnschedulableNodesSkipped(t *testing.T) { + // Two nodes but both unschedulable — should also skip. + n1 := makeNode("node-1", true, 0) + n1.Spec.Unschedulable = true + n2 := makeNode("node-2", true, 0) + n2.Spec.Unschedulable = true + + client := fake.NewSimpleClientset(n1, n2) + state := &ValidationState{Log: testLog()} + checkNodeToNode(context.Background(), client, state) + + require.NotNil(t, state.NodeToNodeOK) + assert.True(t, *state.NodeToNodeOK, "no schedulable nodes must skip, not fail") +} + +func TestCheckNodeToNode_ServerPodCreateFailure(t *testing.T) { + // Two schedulable nodes, but pod creation fails. + client := fake.NewSimpleClientset( + makeNode("node-1", true, 0), + makeNode("node-2", true, 0), + ) + client.PrependReactor("create", "pods", func(_ ktesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("pod quota exceeded") + }) + + state := &ValidationState{Log: testLog()} + checkNodeToNode(context.Background(), client, state) + + require.NotNil(t, state.NodeToNodeOK) + assert.False(t, *state.NodeToNodeOK, "server pod create failure must set NodeToNodeOK=false") +} + +// init is required to register types with the fake client's object tracker. +func init() { + _ = []runtime.Object{ + &storagev1.StorageClass{}, + &corev1.Namespace{}, + &corev1.Pod{}, + &corev1.Service{}, + } +} diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go index 0f41e2fdc..4a0ff41d3 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go @@ -24,12 +24,22 @@ import ( "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/core" "github.com/sirupsen/logrus" + "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" ) +// Role values for VALIDATOR_ROLE. +const ( + RoleComputePlane = "compute-plane" + RoleControlPlane = "control-plane" +) + // ValidationState captures the results of every validation check. type ValidationState struct { - Log *logrus.Entry + Log *logrus.Entry + // Role is "control-plane" or "compute-plane" (empty = compute-plane default). + // printSummary uses it to include only the checks relevant to the role. + Role string ControlPlaneHealthy bool // NodesAllReady tracks whether all worker nodes are Ready. False means at // least one NotReady node. Warning only — does not flip cluster readiness. @@ -67,6 +77,18 @@ type ValidationState struct { // critical: true, meaning enforcement failure blocks readiness. EnforcementCritical bool + // Control-plane-specific check outcomes. Nil means the check was not run + // (compute-plane role). Non-nil means the check ran and the bool holds + // the pass/fail result. + DefaultStorageClassOK *bool + GatewayAPICRDsOK *bool + EnvoyGatewayOK *bool + GatewayRoutesOK *bool + ExternalLBOK *bool + // NodeToNodeOK is nil when the check was skipped (single-node cluster or + // compute-plane role). true = overlay verified, false = failed. + NodeToNodeOK *bool + // EndpointResults captures per-endpoint reachability outcomes for the // summary ConfigMap / metrics pipeline. Keyed by the user-supplied // endpoint name (the same string Prometheus will use as the label @@ -96,25 +118,16 @@ type NetpolPairResult struct { Directions map[string]DirectionStatus } -// Run executes all cluster validation checks and prints a summary. -// It returns a non-nil error if the cluster is not ready, which the caller -// should use to set the process exit code. -// -// configNamespace and configName identify an optional ConfigMap that holds -// user-defined reachability and network-policy checks. When the ConfigMap -// does not exist the configurable checks are silently skipped. -// -// summaryNamespace is where the summary ConfigMap is written for the agent to -// read — kept separate from configNamespace so a config-namespace override -// can't redirect the summary away from the namespace the agent watches. -// -// emitMetrics gates that write. In-cluster runs emit by default; callers pass -// false for preflight (no agent to read it, no RBAC to write it). +// Run executes all cluster validation checks and returns a non-nil error when +// the cluster is not ready. role selects the check set; configNamespace/configName +// identify the optional ConfigMap; emitMetrics gates the summary write. func Run( ctx context.Context, client kubernetes.Interface, + dynClient dynamic.Interface, configNamespace, configName, summaryNamespace string, emitMetrics bool, + role string, ) error { startedAt := time.Now() log := core.GetLogger(ctx) @@ -127,6 +140,7 @@ func Run( state := &ValidationState{ Log: log, + Role: role, ControlPlaneHealthy: true, NodesAllReady: true, } @@ -145,7 +159,6 @@ func Run( checkControlPlaneHealth(ctx, client, state) checkWebhookSupport(ctx, client, state) checkNetworkPolicies(ctx, client, state) - checkSMBCSIDriver(ctx, client, state) var netCfg *NetworkCheckConfig if configNamespace != "" && configName != "" { @@ -161,8 +174,22 @@ func Run( checkConfigurableReachability(state, netCfg.Reachability) } - checkGPUResources(ctx, client, state) - checkGPUOperator(ctx, client, state) + if role == RoleControlPlane { + // Control-plane cluster: check gateway infrastructure, storage, and + // inter-node overlay connectivity. GPU operator and SMB CSI are + // compute-plane concerns and are skipped. + checkStorageClass(ctx, client, state) + checkGatewayAPICRDs(ctx, client, state) + checkEnvoyGateway(ctx, client, state) + checkGatewayRoutes(ctx, dynClient, state) + checkExternalLoadBalancer(ctx, client, state) + checkNodeToNode(ctx, client, state) + } else { + // Compute-plane cluster (default): GPU operator, SMB CSI driver. + checkSMBCSIDriver(ctx, client, state) + checkGPUResources(ctx, client, state) + checkGPUOperator(ctx, client, state) + } if netCfg != nil { if netCfg.NetworkPolicies != nil && len(netCfg.NetworkPolicies.Pairs) > 0 { @@ -226,13 +253,6 @@ func printSummary(state *ValidationState) error { false}, {state.WebhooksSupported, "Admission Webhooks: Mutating & Validating Supported", "Admission Webhooks: Not Supported", true}, {state.NetworkPoliciesSupported, "Network Policies: Supported", "Network Policies: Not Confirmed", false}, - // SMB CSI Driver missing is non-blocking: it is required only when - // the HelmSharedStorage feature flag is enabled (NVCA model-cache). - // pkg/storage/smbcsidriver.go's runtime health check itself flags - // this at StatusLevelWarn, not StatusLevelError — block install - // only when the customer has explicitly opted in to a feature that - // needs SMB CSI, not for every operator install. - {state.SMBCSIDriverOK, "SMB CSI Driver: v1.16.0+ Installed", "SMB CSI Driver: Not Installed or Below v1.16.0", false}, } if state.ReachabilityOK != nil { @@ -246,15 +266,51 @@ func printSummary(state *ValidationState) error { }) } - checks = append(checks, - check{state.GPUAvailable, "GPU Resources: Available", "GPU Resources: Not Available", true}, - // GPU Operator missing is non-blocking: clusters registered with - // Manual Instance Configuration expose GPUs via an alternative - // mechanism (pre-labeled nodes, DaemonSet, etc.) and do not require - // GPU Operator. GPU Resources above is the load-bearing signal — - // if GPUs aren't usable that fails Critical separately. - check{state.GPUOperatorInstalled, "GPU Operator: Installed", "GPU Operator: Not Installed", false}, - ) + if state.Role == RoleControlPlane { + // Control-plane checks: gateway infrastructure and storage. GPU and + // SMB checks are compute-plane concerns and are excluded here. + if state.DefaultStorageClassOK != nil { + checks = append(checks, check{*state.DefaultStorageClassOK, + "Default StorageClass: Present", "Default StorageClass: Not Found", true}) + } + if state.GatewayAPICRDsOK != nil { + checks = append(checks, check{*state.GatewayAPICRDsOK, + "Gateway API CRDs: Installed", "Gateway API CRDs: Not Installed", true}) + } + if state.EnvoyGatewayOK != nil { + // Non-critical: Envoy Gateway is installed by nvcf-cli up, so it is + // expected to be absent on a fresh cluster before the first install. + // A missing Envoy is informative (tells the operator the stack is not + // yet deployed) but must not block a pre-install readiness check. + checks = append(checks, check{*state.EnvoyGatewayOK, + "Envoy Gateway: Installed and Running", "Envoy Gateway: Not Found or Not Running", false}) + } + if state.GatewayRoutesOK != nil { + checks = append(checks, check{*state.GatewayRoutesOK, + "Gateway Routes: Present", "Gateway Routes: None Found", false}) + } + if state.ExternalLBOK != nil { + checks = append(checks, check{*state.ExternalLBOK, + "External Load Balancer: IP Assigned", "External Load Balancer: No IP Assigned", false}) + } + if state.NodeToNodeOK != nil { + checks = append(checks, check{*state.NodeToNodeOK, + "Node-to-Node Communication: Verified", "Node-to-Node Communication: Failed", true}) + } + } else { + // Compute-plane checks: GPU resources, GPU operator, SMB CSI driver. + // SMB CSI Driver missing is non-blocking: it is required only when + // the HelmSharedStorage feature flag is enabled (NVCA model-cache). + checks = append(checks, + check{state.SMBCSIDriverOK, "SMB CSI Driver: v1.16.0+ Installed", "SMB CSI Driver: Not Installed or Below v1.16.0", false}, + check{state.GPUAvailable, "GPU Resources: Available", "GPU Resources: Not Available", true}, + // GPU Operator missing is non-blocking: clusters registered with + // Manual Instance Configuration expose GPUs via an alternative + // mechanism (pre-labeled nodes, DaemonSet, etc.) and do not require + // GPU Operator. GPU Resources above is the load-bearing signal. + check{state.GPUOperatorInstalled, "GPU Operator: Installed", "GPU Operator: Not Installed", false}, + ) + } if state.ConfigurableNetPolOK != nil { isCritical := state.ConfigurableNetPolCriticalOK != nil && diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go index 1c102e81c..478a2e99c 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go @@ -64,7 +64,7 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { t.Run("preflight (emitMetrics=false) does not write the summary", func(t *testing.T) { client := fake.NewSimpleClientset() - _ = Run(context.Background(), client, ns, "cluster-validator-network-checks", ns, false) + _ = Run(context.Background(), client, nil, ns, "cluster-validator-network-checks", ns, false, "") _, err := client.CoreV1().ConfigMaps(ns).Get( context.Background(), SummaryConfigMapName, metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err), @@ -73,7 +73,7 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { t.Run("post-install (emitMetrics=true) writes the summary", func(t *testing.T) { client := fake.NewSimpleClientset() - _ = Run(context.Background(), client, ns, "cluster-validator-network-checks", ns, true) + _ = Run(context.Background(), client, nil, ns, "cluster-validator-network-checks", ns, true, "") cm, err := client.CoreV1().ConfigMaps(ns).Get( context.Background(), SummaryConfigMapName, metav1.GetOptions{}) require.NoError(t, err, "summary ConfigMap must be written when emitMetrics=true") @@ -85,7 +85,7 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { // Guards the decoupling: a non-operator config namespace must NOT // redirect the summary away from the namespace the agent watches. client := fake.NewSimpleClientset() - _ = Run(context.Background(), client, "some-config-ns", "cluster-validator-network-checks", ns, true) + _ = Run(context.Background(), client, nil, "some-config-ns", "cluster-validator-network-checks", ns, true, "") _, err := client.CoreV1().ConfigMaps(ns).Get( context.Background(), SummaryConfigMapName, metav1.GetOptions{}) @@ -98,6 +98,116 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { }) } +// TestRun_ControlPlaneRoleSkipsGPUChecks verifies that with role="control-plane" +// the GPU and SMB checks do not run, so a control-plane cluster without GPU +// nodes is not falsely reported as not-ready. +func TestRun_ControlPlaneRoleSkipsGPUChecks(t *testing.T) { + // A cluster with no GPU nodes and no GPU Operator. Under the compute-plane + // role (default) this would be NVCF-Not-Ready because GPUAvailable=false + // is a critical check. Under the control-plane role it must pass (no GPU + // row in the summary). + client := fake.NewSimpleClientset( + makeNode("node-1", true, 0), // no GPUs + ) + // Run must not return an error on a control-plane role even when there are + // no GPU nodes. The control-plane checks (StorageClass, Gateway) will also + // fail on this bare cluster, but that's fine for this assertion — we only + // care that the GPU row absence means the call doesn't immediately return + // "not ready" due to GPUAvailable. + // + // Use emitMetrics=false so we don't need the summary write RBAC. + err := Run(context.Background(), client, nil, "ns", "cfg", "ns", false, RoleControlPlane) + // The control-plane checks (StorageClass missing, gateway CRDs missing) + // will fail, so the cluster IS not-ready. But the failure must be due to + // control-plane checks, NOT GPU checks. We verify by inspecting the state + // indirectly: if the GPU check ran and caused the failure, the error would + // mention GPU; the control-plane checks produce different messages. + // We can't easily inspect internal state here, so we settle for a simpler + // invariant: the call must complete without panicking, and the error (if any) + // must not be nil only for GPU-related reasons. + // The true correctness guard is TestPrintSummary_ControlPlaneRole below. + _ = err // return value is checked in the summary test +} + +// TestPrintSummary_ControlPlaneRole verifies that with Role=RoleControlPlane +// the summary omits GPU rows and includes control-plane check rows. +func TestPrintSummary_ControlPlaneRole(t *testing.T) { + t.Run("control-plane role excludes GPU rows", func(t *testing.T) { + ok := true + buf := &bytes.Buffer{} + l := logrus.New() + l.SetOutput(buf) + state := &ValidationState{ + Log: logrus.NewEntry(l), + Role: RoleControlPlane, + ControlPlaneHealthy: true, + NodesAllReady: true, + WebhooksSupported: true, + NetworkPoliciesSupported: true, + // Control-plane checks all pass + DefaultStorageClassOK: &ok, + GatewayAPICRDsOK: &ok, + EnvoyGatewayOK: &ok, + GatewayRoutesOK: &ok, + ExternalLBOK: &ok, + K8sVersion: "v1.30.0", + TotalNodes: "2", + } + err := printSummary(state) + assert.NoError(t, err, "all control-plane checks passing must yield NVCF-Ready") + out := buf.String() + assert.NotContains(t, out, "GPU Resources", "GPU row must not appear for control-plane role") + assert.NotContains(t, out, "GPU Operator", "GPU Operator row must not appear for control-plane role") + assert.Contains(t, out, "Default StorageClass", "StorageClass row must appear for control-plane role") + assert.Contains(t, out, "Gateway API CRDs", "Gateway CRD row must appear for control-plane role") + assert.Contains(t, out, "Envoy Gateway", "Envoy Gateway row must appear for control-plane role") + }) + + t.Run("control-plane role critical failure blocks readiness", func(t *testing.T) { + fail := false + ok := true + state := &ValidationState{ + Log: testLog(), + Role: RoleControlPlane, + ControlPlaneHealthy: true, + NodesAllReady: true, + WebhooksSupported: true, + NetworkPoliciesSupported: true, + DefaultStorageClassOK: &fail, // critical: no default StorageClass + GatewayAPICRDsOK: &ok, + EnvoyGatewayOK: &ok, + K8sVersion: "v1.30.0", + TotalNodes: "2", + } + err := printSummary(state) + assert.Error(t, err, "missing default StorageClass must block control-plane readiness") + }) + + t.Run("compute-plane role (default) still includes GPU rows", func(t *testing.T) { + buf := &bytes.Buffer{} + l := logrus.New() + l.SetOutput(buf) + state := &ValidationState{ + Log: logrus.NewEntry(l), + Role: "", + ControlPlaneHealthy: true, + NodesAllReady: true, + WebhooksSupported: true, + NetworkPoliciesSupported: true, + SMBCSIDriverOK: true, + GPUAvailable: true, + GPUOperatorInstalled: true, + K8sVersion: "v1.30.0", + TotalNodes: "2", + } + err := printSummary(state) + assert.NoError(t, err) + out := buf.String() + assert.Contains(t, out, "GPU Resources", "GPU row must appear for compute-plane role") + assert.NotContains(t, out, "Default StorageClass", "StorageClass row must not appear for compute-plane role") + }) +} + func TestVersionGTE(t *testing.T) { tests := []struct { name string From 1b05986b8668972ba7f9bad41fa4e730e8811f5b Mon Sep 17 00:00:00 2001 From: rohithb Date: Wed, 12 Aug 2026 11:46:46 +0530 Subject: [PATCH 02/12] fix(nvca): address code-review findings in control-plane validator --- .../nvca/cmd/cluster-validator/main.go | 22 +++++--- .../internal/clustervalidator/BUILD.bazel | 1 + .../nvca/internal/clustervalidator/checks.go | 55 +++++++++++-------- .../internal/clustervalidator/validator.go | 7 ++- 4 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/compute-plane-services/nvca/cmd/cluster-validator/main.go b/src/compute-plane-services/nvca/cmd/cluster-validator/main.go index b786241d4..9b8edf281 100644 --- a/src/compute-plane-services/nvca/cmd/cluster-validator/main.go +++ b/src/compute-plane-services/nvca/cmd/cluster-validator/main.go @@ -45,14 +45,14 @@ func main() { log.WithError(err).Fatal("Failed to create Kubernetes client") } - // Build the dynamic client from the same REST config. Used for listing - // Gateway API custom resources (HTTPRoutes, etc.) which are not in the - // typed k8s.io/client-go clientset. Failure is non-fatal: checkGatewayRoutes - // skips gracefully when dynClient is nil. - dynClient, err := dynamic.NewForConfig(restCfg) - if err != nil { - log.WithError(err).Warn("Could not create dynamic client; gateway route check will be skipped") - dynClient = nil + // Build the dynamic client from the same REST config. Declared as + // dynamic.Interface so the nil guard in checkGatewayRoutes works: assigning + // a typed *DynamicClient nil to an interface creates a non-nil interface. + var dynClient dynamic.Interface + if dc, dcErr := dynamic.NewForConfig(restCfg); dcErr != nil { + log.WithError(dcErr).Warn("Could not create dynamic client; gateway route check will be skipped") + } else { + dynClient = dc } configNS := os.Getenv("VALIDATOR_CONFIG_NAMESPACE") @@ -86,7 +86,11 @@ func main() { // VALIDATOR_ROLE selects which check set runs: "control-plane" enables // gateway and StorageClass checks and skips GPU/SMB; anything else (including // unset) runs the compute-plane check set (backward-compatible default). - role := parseRole(os.Getenv("VALIDATOR_ROLE")) + roleEnv := os.Getenv("VALIDATOR_ROLE") + role := parseRole(roleEnv) + if roleEnv != "" && role == "" { + log.Warnf("VALIDATOR_ROLE=%q is not recognized; defaulting to compute-plane", roleEnv) + } if err := clustervalidator.Run(ctx, client, dynClient, configNS, configName, summaryNS, emitMetrics, role); err != nil { log.WithError(err).Fatal("Cluster validation failed") diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel b/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel index a24647e2c..707da8687 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel +++ b/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel @@ -26,6 +26,7 @@ go_library( "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/api/resource", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/apis/meta/v1:meta", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/runtime/schema", + "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/util/rand", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/util/intstr", "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/discovery", "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/dynamic", diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index 82987fe66..3384613cf 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -31,6 +31,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/rand" "k8s.io/client-go/discovery" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" @@ -823,9 +824,11 @@ func checkStorageClass(ctx context.Context, client kubernetes.Interface, state * classes, err := client.StorageV1().StorageClasses().List(ctx, metav1.ListOptions{}) if err != nil { + // Leave DefaultStorageClassOK nil (unknown) so the summary row is + // omitted rather than reported as "Not Found" — an API error is not + // confirmation that no default StorageClass exists. printWarning(log, fmt.Sprintf("Could not list StorageClasses: %v", err)) - ok := false - state.DefaultStorageClassOK = &ok + state.Warnings = append(state.Warnings, "Default StorageClass: status unknown (listing failed)") return } @@ -1066,11 +1069,12 @@ func checkExternalLoadBalancer(ctx context.Context, client kubernetes.Interface, } const ( - nodeToNodeTestPort = 19999 - nodeToNodeImage = enforcementDefaultImg // busybox:1.36 - nodeToNodeNamespace = "default" - nodeToNodeServerName = "nvcf-n2n-server" - nodeToNodeClientName = "nvcf-n2n-client" + nodeToNodeTestPort = 19999 + nodeToNodeImage = enforcementDefaultImg // busybox:1.36 + nodeToNodeNamespace = "default" + nodeToNodeServerName = "nvcf-n2n-server" + nodeToNodeClientName = "nvcf-n2n-client" + nodeToNodeActiveDeadline = int64(120) // API server terminates pods if deferred cleanup never runs // 90 s per pod matches enforcementPodTimeout — image should already be // cached from the enforcement check that ran earlier in the same run. nodeToNodePodTimeout = 90 * time.Second @@ -1091,9 +1095,11 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va nodes, err := client.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) if err != nil { + // Leave NodeToNodeOK nil (unknown) so the summary row is omitted rather + // than reported as "Failed" — an RBAC or API error is not confirmation + // of a broken overlay network. printWarning(log, fmt.Sprintf("Could not list nodes: %v", err)) - ok := false - state.NodeToNodeOK = &ok + state.Warnings = append(state.Warnings, "Node-to-Node: status unknown (node listing failed)") return } @@ -1116,7 +1122,7 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va nodeA, nodeB := schedulable[0], schedulable[1] log.Infof(" Probing overlay connectivity: %s → %s", nodeA, nodeB) - suffix := fmt.Sprintf("%d", time.Now().UnixNano()%1000000) + suffix := rand.String(6) serverName := nodeToNodeServerName + "-" + suffix clientName := nodeToNodeClientName + "-" + suffix @@ -1189,24 +1195,23 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va } func buildNodeToNodeServerPod(name, nodeName string) *corev1.Pod { + deadline := nodeToNodeActiveDeadline return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: nodeToNodeNamespace, Labels: map[string]string{ - "app.kubernetes.io/managed-by": "nvcf-cli", + "app.kubernetes.io/managed-by": "nvcf-cluster-validator", "app.kubernetes.io/component": "n2n-probe", }, }, Spec: corev1.PodSpec{ - NodeName: nodeName, - RestartPolicy: corev1.RestartPolicyNever, + NodeName: nodeName, + RestartPolicy: corev1.RestartPolicyNever, + ActiveDeadlineSeconds: &deadline, Containers: []corev1.Container{{ - Name: "server", - Image: nodeToNodeImage, - // Loop keeps the pod Running while we resolve its IP and - // start the client. The pod is cleaned up via a deferred - // background-context delete, not by natural exit. + Name: "server", + Image: nodeToNodeImage, Command: []string{"sh", "-c", fmt.Sprintf("while true; do nc -l -p %d; done", nodeToNodeTestPort)}, Resources: enforcementResources(), }}, @@ -1215,22 +1220,24 @@ func buildNodeToNodeServerPod(name, nodeName string) *corev1.Pod { } func buildNodeToNodeClientPod(name, nodeName, serverIP string) *corev1.Pod { + deadline := nodeToNodeActiveDeadline return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: nodeToNodeNamespace, Labels: map[string]string{ - "app.kubernetes.io/managed-by": "nvcf-cli", + "app.kubernetes.io/managed-by": "nvcf-cluster-validator", "app.kubernetes.io/component": "n2n-probe", }, }, Spec: corev1.PodSpec{ - NodeName: nodeName, - RestartPolicy: corev1.RestartPolicyNever, + NodeName: nodeName, + RestartPolicy: corev1.RestartPolicyNever, + ActiveDeadlineSeconds: &deadline, Containers: []corev1.Container{{ - Name: "client", - Image: nodeToNodeImage, - Command: []string{"sh", "-c", fmt.Sprintf("nc -z -w 5 %s %d", serverIP, nodeToNodeTestPort)}, + Name: "client", + Image: nodeToNodeImage, + Command: []string{"sh", "-c", fmt.Sprintf("nc -z -w 5 %s %d", serverIP, nodeToNodeTestPort)}, Resources: enforcementResources(), }}, }, diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go index 4a0ff41d3..ed08e41b1 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go @@ -183,7 +183,12 @@ func Run( checkEnvoyGateway(ctx, client, state) checkGatewayRoutes(ctx, dynClient, state) checkExternalLoadBalancer(ctx, client, state) - checkNodeToNode(ctx, client, state) + // Node-to-node creates pods and requires pod-create RBAC. Skip during + // preflight (emitMetrics=false) where the SA may not hold that permission; + // run only for in-cluster scheduled checks where the SA is fully provisioned. + if emitMetrics { + checkNodeToNode(ctx, client, state) + } } else { // Compute-plane cluster (default): GPU operator, SMB CSI driver. checkSMBCSIDriver(ctx, client, state) From cc062af870405cbcebd422453f7803c8b3454ec3 Mon Sep 17 00:00:00 2001 From: rohithb Date: Wed, 12 Aug 2026 12:14:47 +0530 Subject: [PATCH 03/12] fix(nvca): add security context, summary schema entries, and test assertions --- .../nvca/internal/clustervalidator/checks.go | 31 +++++++++--- .../nvca/internal/clustervalidator/summary.go | 36 ++++++++++++++ .../clustervalidator/validator_test.go | 49 +++++++++---------- 3 files changed, 82 insertions(+), 34 deletions(-) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index 3384613cf..a2b670de8 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -1194,6 +1194,19 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va } } +// nodeToNodeSecurityContext returns a restricted Pod Security Standards compliant +// context. Port 19999 is above 1024 so busybox nc runs fine as non-root. +func nodeToNodeSecurityContext() *corev1.SecurityContext { + runAsNonRoot := true + allowPrivEsc := false + return &corev1.SecurityContext{ + RunAsNonRoot: &runAsNonRoot, + AllowPrivilegeEscalation: &allowPrivEsc, + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + } +} + func buildNodeToNodeServerPod(name, nodeName string) *corev1.Pod { deadline := nodeToNodeActiveDeadline return &corev1.Pod{ @@ -1210,10 +1223,11 @@ func buildNodeToNodeServerPod(name, nodeName string) *corev1.Pod { RestartPolicy: corev1.RestartPolicyNever, ActiveDeadlineSeconds: &deadline, Containers: []corev1.Container{{ - Name: "server", - Image: nodeToNodeImage, - Command: []string{"sh", "-c", fmt.Sprintf("while true; do nc -l -p %d; done", nodeToNodeTestPort)}, - Resources: enforcementResources(), + Name: "server", + Image: nodeToNodeImage, + Command: []string{"sh", "-c", fmt.Sprintf("while true; do nc -l -p %d; done", nodeToNodeTestPort)}, + Resources: enforcementResources(), + SecurityContext: nodeToNodeSecurityContext(), }}, }, } @@ -1235,10 +1249,11 @@ func buildNodeToNodeClientPod(name, nodeName, serverIP string) *corev1.Pod { RestartPolicy: corev1.RestartPolicyNever, ActiveDeadlineSeconds: &deadline, Containers: []corev1.Container{{ - Name: "client", - Image: nodeToNodeImage, - Command: []string{"sh", "-c", fmt.Sprintf("nc -z -w 5 %s %d", serverIP, nodeToNodeTestPort)}, - Resources: enforcementResources(), + Name: "client", + Image: nodeToNodeImage, + Command: []string{"sh", "-c", fmt.Sprintf("nc -z -w 5 %s %d", serverIP, nodeToNodeTestPort)}, + Resources: enforcementResources(), + SecurityContext: nodeToNodeSecurityContext(), }}, }, } diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/summary.go b/src/compute-plane-services/nvca/internal/clustervalidator/summary.go index 27bd11470..26e015221 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/summary.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/summary.go @@ -151,6 +151,14 @@ const ( CheckKeyGPUOperator = "gpu_operator" CheckKeyConfigurableNetpol = "configurable_netpol" CheckKeyNetpolEnforcement = "netpol_enforcement" + // Control-plane-specific check keys. Only written to the summary when the + // check ran (nil pointer = check was skipped for this role). + CheckKeyDefaultStorageClass = "default_storage_class" + CheckKeyGatewayAPICRDs = "gateway_api_crds" + CheckKeyEnvoyGateway = "envoy_gateway" + CheckKeyGatewayRoutes = "gateway_routes" + CheckKeyExternalLB = "external_lb" + CheckKeyNodeToNode = "node_to_node" ) // AllCheckKeys is the canonical ordering used for documentation and @@ -160,12 +168,20 @@ var AllCheckKeys = []string{ CheckKeyWorkerNodesAllReady, CheckKeyWebhooks, CheckKeyNetworkPoliciesSupport, + // Compute-plane checks. CheckKeySMBCSI, CheckKeyEndpointReachability, CheckKeyGPUResources, CheckKeyGPUOperator, CheckKeyConfigurableNetpol, CheckKeyNetpolEnforcement, + // Control-plane checks (only present in summary when the role ran them). + CheckKeyDefaultStorageClass, + CheckKeyGatewayAPICRDs, + CheckKeyEnvoyGateway, + CheckKeyGatewayRoutes, + CheckKeyExternalLB, + CheckKeyNodeToNode, } // buildSummary projects a ValidationState into the wire format. Checks @@ -204,6 +220,26 @@ func buildSummary(state *ValidationState, startedAt time.Time, verdictReady bool if state.EnforcementOK != nil { s.Checks[CheckKeyNetpolEnforcement] = *state.EnforcementOK } + // Control-plane checks are only written when the check ran (non-nil pointer). + // A nil pointer means the check was skipped because the role was compute-plane. + if state.DefaultStorageClassOK != nil { + s.Checks[CheckKeyDefaultStorageClass] = *state.DefaultStorageClassOK + } + if state.GatewayAPICRDsOK != nil { + s.Checks[CheckKeyGatewayAPICRDs] = *state.GatewayAPICRDsOK + } + if state.EnvoyGatewayOK != nil { + s.Checks[CheckKeyEnvoyGateway] = *state.EnvoyGatewayOK + } + if state.GatewayRoutesOK != nil { + s.Checks[CheckKeyGatewayRoutes] = *state.GatewayRoutesOK + } + if state.ExternalLBOK != nil { + s.Checks[CheckKeyExternalLB] = *state.ExternalLBOK + } + if state.NodeToNodeOK != nil { + s.Checks[CheckKeyNodeToNode] = *state.NodeToNodeOK + } if len(state.EndpointResults) > 0 { s.Endpoints = make(map[string]EndpointStatus, len(state.EndpointResults)) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go index 478a2e99c..d7e3e3016 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go @@ -99,34 +99,31 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { } // TestRun_ControlPlaneRoleSkipsGPUChecks verifies that with role="control-plane" -// the GPU and SMB checks do not run, so a control-plane cluster without GPU -// nodes is not falsely reported as not-ready. +// the GPU and SMB checks do not run. A bare cluster with no GPUs should fail +// because of missing StorageClass or Gateway CRDs, not because of GPUAvailable. func TestRun_ControlPlaneRoleSkipsGPUChecks(t *testing.T) { - // A cluster with no GPU nodes and no GPU Operator. Under the compute-plane - // role (default) this would be NVCF-Not-Ready because GPUAvailable=false - // is a critical check. Under the control-plane role it must pass (no GPU - // row in the summary). - client := fake.NewSimpleClientset( - makeNode("node-1", true, 0), // no GPUs - ) - // Run must not return an error on a control-plane role even when there are - // no GPU nodes. The control-plane checks (StorageClass, Gateway) will also - // fail on this bare cluster, but that's fine for this assertion — we only - // care that the GPU row absence means the call doesn't immediately return - // "not ready" due to GPUAvailable. - // - // Use emitMetrics=false so we don't need the summary write RBAC. + client := fake.NewSimpleClientset(makeNode("node-1", true, 0)) err := Run(context.Background(), client, nil, "ns", "cfg", "ns", false, RoleControlPlane) - // The control-plane checks (StorageClass missing, gateway CRDs missing) - // will fail, so the cluster IS not-ready. But the failure must be due to - // control-plane checks, NOT GPU checks. We verify by inspecting the state - // indirectly: if the GPU check ran and caused the failure, the error would - // mention GPU; the control-plane checks produce different messages. - // We can't easily inspect internal state here, so we settle for a simpler - // invariant: the call must complete without panicking, and the error (if any) - // must not be nil only for GPU-related reasons. - // The true correctness guard is TestPrintSummary_ControlPlaneRole below. - _ = err // return value is checked in the summary test + // A bare fake cluster fails control-plane checks (no StorageClass, no Gateway CRDs). + require.Error(t, err) + assert.Contains(t, err.Error(), "NVCF-Not-Ready", + "error must name the verdict, not a GPU-specific failure") + assert.NotContains(t, err.Error(), "GPU", + "GPU checks must not run under the control-plane role") +} + +// TestRun_ControlPlaneRoleRunsControlPlaneChecks verifies the role dispatch: +// StorageClass check runs and GPU state is not populated. +func TestRun_ControlPlaneRoleRunsControlPlaneChecks(t *testing.T) { + state := &ValidationState{Log: testLog(), Role: RoleControlPlane} + client := fake.NewSimpleClientset(makeNode("node-1", true, 0)) + + checkStorageClass(context.Background(), client, state) + + require.NotNil(t, state.DefaultStorageClassOK, + "control-plane role must set DefaultStorageClassOK after running the StorageClass check") + assert.False(t, state.GPUAvailable, + "GPUAvailable must remain false — GPU check must not have run") } // TestPrintSummary_ControlPlaneRole verifies that with Role=RoleControlPlane From 793bb3240c085928e9a94e4583c2548c283c7756 Mon Sep 17 00:00:00 2001 From: rohithb Date: Wed, 12 Aug 2026 12:39:10 +0530 Subject: [PATCH 04/12] fix(nvca): sync AllCheckKeys count and clusterValidatorCheckKeys with new control-plane entries --- .../nvca/internal/clustervalidator/summary_test.go | 9 ++++++++- .../nvca/internal/metrics/metrics.go | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/summary_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/summary_test.go index 926d6856e..6e91a5e66 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/summary_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/summary_test.go @@ -282,8 +282,15 @@ func TestAllCheckKeysCoversEveryCheckKeyConst(t *testing.T) { CheckKeyGPUOperator, CheckKeyConfigurableNetpol, CheckKeyNetpolEnforcement, + // Control-plane-specific keys added with the role-aware validator. + CheckKeyDefaultStorageClass, + CheckKeyGatewayAPICRDs, + CheckKeyEnvoyGateway, + CheckKeyGatewayRoutes, + CheckKeyExternalLB, + CheckKeyNodeToNode, } { assert.True(t, known[k], "%q is a CheckKey constant but missing from AllCheckKeys", k) } - assert.Len(t, AllCheckKeys, 10, "if you added a new CheckKey, also add it to AllCheckKeys AND to clusterValidatorCheckKeys() in internal/metrics/metrics.go") + assert.Len(t, AllCheckKeys, 16, "if you added a new CheckKey, also add it to AllCheckKeys AND to clusterValidatorCheckKeys() in internal/metrics/metrics.go") } diff --git a/src/compute-plane-services/nvca/internal/metrics/metrics.go b/src/compute-plane-services/nvca/internal/metrics/metrics.go index 1271e7834..558d6a203 100644 --- a/src/compute-plane-services/nvca/internal/metrics/metrics.go +++ b/src/compute-plane-services/nvca/internal/metrics/metrics.go @@ -1310,6 +1310,13 @@ func clusterValidatorCheckKeys() []string { "gpu_operator", "configurable_netpol", "netpol_enforcement", + // Control-plane-specific keys (only populated when VALIDATOR_ROLE=control-plane). + "default_storage_class", + "gateway_api_crds", + "envoy_gateway", + "gateway_routes", + "external_lb", + "node_to_node", } } From 21b85e88f36754a0f21df01d1a30b9c2e101ad7e Mon Sep 17 00:00:00 2001 From: rohithb Date: Wed, 12 Aug 2026 12:54:40 +0530 Subject: [PATCH 05/12] fix(nvca): set RunAsUser on node-to-node probe security context --- .../nvca/internal/clustervalidator/checks.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index a2b670de8..2f1a25082 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -1196,11 +1196,16 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va // nodeToNodeSecurityContext returns a restricted Pod Security Standards compliant // context. Port 19999 is above 1024 so busybox nc runs fine as non-root. +// RunAsUser must be set explicitly: busybox:1.36 declares no USER in its image +// config, so kubelet rejects the container at admission when RunAsNonRoot is +// true but RunAsUser is absent. func nodeToNodeSecurityContext() *corev1.SecurityContext { runAsNonRoot := true allowPrivEsc := false + runAsUser := int64(65534) // nobody — the conventional non-root UID for scratch/busybox images return &corev1.SecurityContext{ RunAsNonRoot: &runAsNonRoot, + RunAsUser: &runAsUser, AllowPrivilegeEscalation: &allowPrivEsc, Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, From eeb2c306dc19351fcabbe4191d495d3ce938d6ed Mon Sep 17 00:00:00 2001 From: rohithb Date: Wed, 12 Aug 2026 13:01:57 +0530 Subject: [PATCH 06/12] style(nvca): replace em dash with semicolon in security context comment --- .../nvca/internal/clustervalidator/checks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index 2f1a25082..7c4c2ad5e 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -1202,7 +1202,7 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va func nodeToNodeSecurityContext() *corev1.SecurityContext { runAsNonRoot := true allowPrivEsc := false - runAsUser := int64(65534) // nobody — the conventional non-root UID for scratch/busybox images + runAsUser := int64(65534) // nobody; conventional non-root UID for scratch/busybox images return &corev1.SecurityContext{ RunAsNonRoot: &runAsNonRoot, RunAsUser: &runAsUser, From 257ac5ee2d0f220fb25e89f7aa14ca820e86b43a Mon Sep 17 00:00:00 2001 From: rohithb Date: Mon, 17 Aug 2026 22:57:30 +0530 Subject: [PATCH 07/12] feat(nvca): extend control-plane validator with DaemonSet n2n, HA checks, route CR check - Replace two-node pinning in checkNodeToNode with a DaemonSet approach: a server pod is scheduled on every schedulable node and a checker pod on node[0] verifies reachability to all cross-node server IPs. This catches per-node CNI issues that the two-node probe missed. - Remove the emitMetrics gate on checkNodeToNode. The CLI RBAC bootstrap (Req 3) grants the validator SA DaemonSet create/delete before Job submission so no separate permission gate is needed. - Replace checkGatewayRoutes dynamic-client list with a discovery API check: verifies httproute, tcproute, grpcroute, udproute CR types are registered across all gateway.networking.k8s.io versions. No dependency on actual route object names or counts. - Remove dynClient dynamic.Interface parameter from Run() and main.go since no check requires it after the routes check was reworked. - Add checkTier1Deployments: lists all Deployments in control-plane namespaces and fails if any have readyReplicas < spec.replicas. - Add checkTier2StatefulSets: lists StatefulSets with spec.replicas==3 and fails if readyReplicas < 3 or any two pods share a node. Covers NATS, OpenBao, Cassandra without hardcoding names. - Add CheckKeyTier1Deployments and CheckKeyTier2StatefulSets to summary.go and metrics.go so the gauges appear pre-zeroed on the first Prometheus scrape. Closes NVIDIA/nvcf#583 --- .../nvca/cmd/cluster-validator/main.go | 15 +- .../nvca/internal/clustervalidator/checks.go | 416 +++++++++++++----- .../checks_controlplane_test.go | 22 +- .../nvca/internal/clustervalidator/summary.go | 11 + .../internal/clustervalidator/summary_test.go | 5 +- .../internal/clustervalidator/validator.go | 27 +- .../clustervalidator/validator_test.go | 8 +- .../nvca/internal/metrics/metrics.go | 3 + 8 files changed, 364 insertions(+), 143 deletions(-) diff --git a/src/compute-plane-services/nvca/cmd/cluster-validator/main.go b/src/compute-plane-services/nvca/cmd/cluster-validator/main.go index 9b8edf281..b849bf85f 100644 --- a/src/compute-plane-services/nvca/cmd/cluster-validator/main.go +++ b/src/compute-plane-services/nvca/cmd/cluster-validator/main.go @@ -23,7 +23,6 @@ import ( "strings" "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/core" - "k8s.io/client-go/dynamic" internalutil "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/cmd/internal" "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/internal/clustervalidator" @@ -40,21 +39,11 @@ func main() { log := core.GetLogger(ctx) log.Logger.SetFormatter(&clustervalidator.CLIFormatter{}) - client, restCfg, err := internalutil.NewK8sClient(ctx, "") + client, _, err := internalutil.NewK8sClient(ctx, "") if err != nil { log.WithError(err).Fatal("Failed to create Kubernetes client") } - // Build the dynamic client from the same REST config. Declared as - // dynamic.Interface so the nil guard in checkGatewayRoutes works: assigning - // a typed *DynamicClient nil to an interface creates a non-nil interface. - var dynClient dynamic.Interface - if dc, dcErr := dynamic.NewForConfig(restCfg); dcErr != nil { - log.WithError(dcErr).Warn("Could not create dynamic client; gateway route check will be skipped") - } else { - dynClient = dc - } - configNS := os.Getenv("VALIDATOR_CONFIG_NAMESPACE") if configNS == "" { configNS = podNamespace() @@ -92,7 +81,7 @@ func main() { log.Warnf("VALIDATOR_ROLE=%q is not recognized; defaulting to compute-plane", roleEnv) } - if err := clustervalidator.Run(ctx, client, dynClient, configNS, configName, summaryNS, emitMetrics, role); err != nil { + if err := clustervalidator.Run(ctx, client, configNS, configName, summaryNS, emitMetrics, role); err != nil { log.WithError(err).Fatal("Cluster validation failed") } } diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index 7c4c2ad5e..41561e0da 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -27,13 +27,12 @@ import ( "strings" "time" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/rand" "k8s.io/client-go/discovery" - "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" ) @@ -958,49 +957,58 @@ func checkEnvoyGateway(ctx context.Context, client kubernetes.Interface, state * // checkGatewayRoutes lists HTTPRoutes across all namespaces using the dynamic // client. At least one HTTPRoute must exist for traffic to reach NVCF -// services. When dynClient is nil the check is silently skipped (used in tests -// or early preflight before Gateway API CRDs are installed). -// -// Non-critical: routes may be deployed after the gateway infrastructure, and -// their absence does not block the cluster verdict. -func checkGatewayRoutes(ctx context.Context, dynClient dynamic.Interface, state *ValidationState) { +// Non-critical: route CR types are installed by nvcf up and are expected to +// be absent on a fresh cluster before install. +func checkGatewayRoutes(ctx context.Context, client kubernetes.Interface, state *ValidationState) { log := state.Log - printHeader(log, "Gateway Routes") - - if dynClient == nil { - printInfo(log, " Gateway route check skipped (no dynamic client configured)") - return - } + printHeader(log, "Gateway Route CR Types") - gvr := schema.GroupVersionResource{ - Group: gatewayAPIGroup, - Version: gatewayAPIVersion, - Resource: "httproutes", - } - list, err := dynClient.Resource(gvr).Namespace("").List(ctx, metav1.ListOptions{}) + groups, err := client.Discovery().ServerGroups() if err != nil { - printWarning(log, fmt.Sprintf("Could not list HTTPRoutes: %v", err)) + printWarning(log, fmt.Sprintf("Could not list API server groups: %v", err)) state.Warnings = append(state.Warnings, - "Gateway Routes: could not list HTTPRoutes — verify Gateway API CRDs are installed") + "Gateway Routes: status unknown (API group discovery failed)") ok := false state.GatewayRoutesOK = &ok return } - count := len(list.Items) - if count == 0 { - printWarning(log, "No HTTPRoutes found in any namespace") + // Collect all resource names registered under gateway.networking.k8s.io + // across all versions (httproutes is v1, tcproutes/udproutes are v1alpha2). + found := make(map[string]bool) + for _, g := range groups.Groups { + if g.Name != gatewayAPIGroup { + continue + } + for _, v := range g.Versions { + resources, err := client.Discovery().ServerResourcesForGroupVersion(v.GroupVersion) + if err != nil { + continue + } + for _, r := range resources.APIResources { + found[r.Name] = true + } + } + } + + required := []string{"httproutes", "tcproutes", "grpcroutes", "udproutes"} + var missing []string + for _, rt := range required { + if !found[rt] { + missing = append(missing, rt) + } + } + + if len(missing) > 0 { + printWarning(log, fmt.Sprintf("Route CR types not registered: %s", strings.Join(missing, ", "))) state.Warnings = append(state.Warnings, - "Gateway Routes: no HTTPRoutes found — routes may not yet be deployed by nvcf-cli") + "Gateway Routes: route CR types missing — install Gateway API CRDs via nvcf up") ok := false state.GatewayRoutesOK = &ok return } - printSuccess(log, fmt.Sprintf("HTTPRoutes present: %d", count)) - for i := range list.Items { - printInfo(log, fmt.Sprintf(" %s/%s", list.Items[i].GetNamespace(), list.Items[i].GetName())) - } + printSuccess(log, "Route CR types registered: httproutes, tcproutes, grpcroutes, udproutes") ok := true state.GatewayRoutesOK = &ok } @@ -1069,23 +1077,23 @@ func checkExternalLoadBalancer(ctx context.Context, client kubernetes.Interface, } const ( - nodeToNodeTestPort = 19999 - nodeToNodeImage = enforcementDefaultImg // busybox:1.36 - nodeToNodeNamespace = "default" - nodeToNodeServerName = "nvcf-n2n-server" - nodeToNodeClientName = "nvcf-n2n-client" - nodeToNodeActiveDeadline = int64(120) // API server terminates pods if deferred cleanup never runs - // 90 s per pod matches enforcementPodTimeout — image should already be - // cached from the enforcement check that ran earlier in the same run. - nodeToNodePodTimeout = 90 * time.Second + nodeToNodeTestPort = 19999 + nodeToNodeImage = enforcementDefaultImg // busybox:1.36 + nodeToNodeNamespace = "default" + nodeToNodeDSName = "nvcf-n2n-server" + nodeToNodeCheckerName = "nvcf-n2n-checker" + nodeToNodeActiveDeadline = int64(180) + nodeToNodeDSTimeout = 2 * time.Minute + nodeToNodeCheckerTimeout = 90 * time.Second ) -// checkNodeToNode verifies raw overlay-network connectivity between two -// schedulable nodes. It pins a TCP server pod (busybox nc) to node A and a -// client pod (nc -z) to node B, then checks whether the TCP connect succeeds. +// checkNodeToNode verifies overlay-network connectivity across all schedulable +// nodes using a DaemonSet-based probe. A server DaemonSet is deployed on every +// schedulable node; a checker pod on node[0] connects to each server pod IP on +// nodes[1..N-1]. This validates full-mesh connectivity, not just a single pair. // -// Single-node clusters are skipped with a passing warning: inter-node -// connectivity is not applicable when there is only one node. +// The CLI RBAC bootstrap (Req 3) grants the validator SA DaemonSet create/delete +// and pod-create before Job submission, so no separate permission gate is needed. // // Critical: broken overlay means NVCF services on different nodes cannot // communicate, causing cascade failures across every API call. @@ -1095,9 +1103,6 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va nodes, err := client.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) if err != nil { - // Leave NodeToNodeOK nil (unknown) so the summary row is omitted rather - // than reported as "Failed" — an RBAC or API error is not confirmation - // of a broken overlay network. printWarning(log, fmt.Sprintf("Could not list nodes: %v", err)) state.Warnings = append(state.Warnings, "Node-to-Node: status unknown (node listing failed)") return @@ -1111,98 +1116,131 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va } if len(schedulable) < 2 { - printInfo(log, fmt.Sprintf(" %d schedulable node(s) — node-to-node check skipped (not applicable for single-node clusters)", len(schedulable))) + printInfo(log, fmt.Sprintf(" %d schedulable node(s) — node-to-node check skipped", len(schedulable))) state.Warnings = append(state.Warnings, - "Node-to-Node: skipped — fewer than 2 schedulable nodes; not applicable for single-node clusters") + "Node-to-Node: skipped — fewer than 2 schedulable nodes") ok := true state.NodeToNodeOK = &ok return } - nodeA, nodeB := schedulable[0], schedulable[1] - log.Infof(" Probing overlay connectivity: %s → %s", nodeA, nodeB) - suffix := rand.String(6) - serverName := nodeToNodeServerName + "-" + suffix - clientName := nodeToNodeClientName + "-" + suffix + dsName := nodeToNodeDSName + "-" + suffix + checkerName := nodeToNodeCheckerName + "-" + suffix + dsLabels := map[string]string{ + "app.kubernetes.io/managed-by": "nvcf-cluster-validator", + "app.kubernetes.io/component": "n2n-server", + "app.kubernetes.io/instance": suffix, + } - // Deferred cleanup uses a fresh context so it runs even when ctx is expired. defer func() { grace := int64(0) opts := metav1.DeleteOptions{GracePeriodSeconds: &grace} - _ = client.CoreV1().Pods(nodeToNodeNamespace).Delete(context.Background(), serverName, opts) - _ = client.CoreV1().Pods(nodeToNodeNamespace).Delete(context.Background(), clientName, opts) + _ = client.AppsV1().DaemonSets(nodeToNodeNamespace).Delete(context.Background(), dsName, opts) + _ = client.CoreV1().Pods(nodeToNodeNamespace).Delete(context.Background(), checkerName, opts) }() - if _, err := client.CoreV1().Pods(nodeToNodeNamespace).Create( - ctx, buildNodeToNodeServerPod(serverName, nodeA), metav1.CreateOptions{}, + if _, err := client.AppsV1().DaemonSets(nodeToNodeNamespace).Create( + ctx, buildNodeToNodeDaemonSet(dsName, dsLabels), metav1.CreateOptions{}, ); err != nil { - printError(log, fmt.Sprintf("Failed to create server pod on %s: %v", nodeA, err)) + printError(log, fmt.Sprintf("Failed to create server DaemonSet: %v", err)) ok := false state.NodeToNodeOK = &ok return } - if err := waitForPodReady(ctx, client, nodeToNodeNamespace, serverName, nodeToNodePodTimeout); err != nil { - printError(log, fmt.Sprintf("Server pod on %s not ready: %v", nodeA, err)) + log.Infof(" Waiting for server DaemonSet pods on %d nodes...", len(schedulable)) + selector := metav1.FormatLabelSelector(&metav1.LabelSelector{MatchLabels: dsLabels}) + serverPods, err := waitForDaemonSetPods(ctx, client, nodeToNodeNamespace, selector, len(schedulable), nodeToNodeDSTimeout) + if err != nil { + printError(log, fmt.Sprintf("Server DaemonSet pods did not become ready: %v", err)) ok := false state.NodeToNodeOK = &ok return } - serverIP, err := getPodIP(ctx, client, nodeToNodeNamespace, serverName) - if err != nil { - printError(log, fmt.Sprintf("Could not get server pod IP: %v", err)) - ok := false + checkerNode := schedulable[0] + var targetIPs []string + for i := range serverPods { + if serverPods[i].Spec.NodeName != checkerNode && serverPods[i].Status.PodIP != "" { + targetIPs = append(targetIPs, serverPods[i].Status.PodIP) + log.Infof(" Server pod on %s: %s", serverPods[i].Spec.NodeName, serverPods[i].Status.PodIP) + } + } + + if len(targetIPs) == 0 { + printWarning(log, "No cross-node server pod IPs available") + ok := true state.NodeToNodeOK = &ok return } - log.Infof(" Server pod on %s has IP %s", nodeA, serverIP) if _, err := client.CoreV1().Pods(nodeToNodeNamespace).Create( - ctx, buildNodeToNodeClientPod(clientName, nodeB, serverIP), metav1.CreateOptions{}, + ctx, buildNodeToNodeCheckerPod(checkerName, checkerNode, targetIPs), metav1.CreateOptions{}, ); err != nil { - printError(log, fmt.Sprintf("Failed to create client pod on %s: %v", nodeB, err)) + printError(log, fmt.Sprintf("Failed to create checker pod: %v", err)) ok := false state.NodeToNodeOK = &ok return } - succeeded, err := waitForPodDone(ctx, client, nodeToNodeNamespace, clientName, nodeToNodePodTimeout) + succeeded, err := waitForPodDone(ctx, client, nodeToNodeNamespace, checkerName, nodeToNodeCheckerTimeout) if err != nil { - printError(log, fmt.Sprintf("Client pod probe error: %v", err)) + printError(log, fmt.Sprintf("Checker pod error: %v", err)) ok := false state.NodeToNodeOK = &ok return } if succeeded { - printSuccess(log, fmt.Sprintf("Node-to-node overlay connectivity verified: %s → %s (%s:%d)", - nodeB, nodeA, serverIP, nodeToNodeTestPort)) + printSuccess(log, fmt.Sprintf("Node-to-node overlay verified: %s → %d node(s) reachable on port %d", + checkerNode, len(targetIPs), nodeToNodeTestPort)) ok := true state.NodeToNodeOK = &ok } else { - printError(log, fmt.Sprintf("Client on %s could not reach server on %s at %s:%d", - nodeB, nodeA, serverIP, nodeToNodeTestPort)) + printError(log, fmt.Sprintf("Checker on %s could not reach one or more server pods (port %d)", + checkerNode, nodeToNodeTestPort)) printInfo(log, " Possible causes: CNI overlay misconfiguration, host firewall rules, "+ "or cloud security group rules blocking inter-node pod traffic") state.Recommendations = append(state.Recommendations, - fmt.Sprintf("Check host firewall and security groups between nodes %s and %s. "+ - "Verify the CNI overlay (VXLAN, Geneve, etc.) is not blocked.", nodeA, nodeB)) + "Check host firewall and security groups between nodes. "+ + "Verify the CNI overlay (VXLAN, Geneve, etc.) is not blocked across all nodes.") ok := false state.NodeToNodeOK = &ok } } -// nodeToNodeSecurityContext returns a restricted Pod Security Standards compliant -// context. Port 19999 is above 1024 so busybox nc runs fine as non-root. -// RunAsUser must be set explicitly: busybox:1.36 declares no USER in its image -// config, so kubelet rejects the container at admission when RunAsNonRoot is -// true but RunAsUser is absent. +func waitForDaemonSetPods(ctx context.Context, client kubernetes.Interface, ns, selector string, wantCount int, timeout time.Duration) ([]corev1.Pod, error) { + deadline := time.Now().Add(timeout) + for { + pods, err := client.CoreV1().Pods(ns).List(ctx, metav1.ListOptions{LabelSelector: selector}) + if err != nil { + return nil, err + } + var running []corev1.Pod + for i := range pods.Items { + if pods.Items[i].Status.Phase == corev1.PodRunning && pods.Items[i].Status.PodIP != "" { + running = append(running, pods.Items[i]) + } + } + if len(running) >= wantCount { + return running, nil + } + if time.Now().After(deadline) { + return nil, fmt.Errorf("timed out waiting for %d Running pods (got %d)", wantCount, len(running)) + } + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(3 * time.Second): + } + } +} + func nodeToNodeSecurityContext() *corev1.SecurityContext { runAsNonRoot := true allowPrivEsc := false - runAsUser := int64(65534) // nobody; conventional non-root UID for scratch/busybox images + runAsUser := int64(65534) return &corev1.SecurityContext{ RunAsNonRoot: &runAsNonRoot, RunAsUser: &runAsUser, @@ -1212,41 +1250,43 @@ func nodeToNodeSecurityContext() *corev1.SecurityContext { } } -func buildNodeToNodeServerPod(name, nodeName string) *corev1.Pod { +func buildNodeToNodeDaemonSet(name string, labels map[string]string) *appsv1.DaemonSet { deadline := nodeToNodeActiveDeadline - return &corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: nodeToNodeNamespace, - Labels: map[string]string{ - "app.kubernetes.io/managed-by": "nvcf-cluster-validator", - "app.kubernetes.io/component": "n2n-probe", + return &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: nodeToNodeNamespace, Labels: labels}, + Spec: appsv1.DaemonSetSpec{ + Selector: &metav1.LabelSelector{MatchLabels: labels}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: labels}, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyAlways, + ActiveDeadlineSeconds: &deadline, + Containers: []corev1.Container{{ + Name: "server", + Image: nodeToNodeImage, + Command: []string{"sh", "-c", fmt.Sprintf("while true; do nc -l -p %d; done", nodeToNodeTestPort)}, + Resources: enforcementResources(), + SecurityContext: nodeToNodeSecurityContext(), + }}, + }, }, }, - Spec: corev1.PodSpec{ - NodeName: nodeName, - RestartPolicy: corev1.RestartPolicyNever, - ActiveDeadlineSeconds: &deadline, - Containers: []corev1.Container{{ - Name: "server", - Image: nodeToNodeImage, - Command: []string{"sh", "-c", fmt.Sprintf("while true; do nc -l -p %d; done", nodeToNodeTestPort)}, - Resources: enforcementResources(), - SecurityContext: nodeToNodeSecurityContext(), - }}, - }, } } -func buildNodeToNodeClientPod(name, nodeName, serverIP string) *corev1.Pod { +func buildNodeToNodeCheckerPod(name, nodeName string, targetIPs []string) *corev1.Pod { deadline := nodeToNodeActiveDeadline + var cmds []string + for _, ip := range targetIPs { + cmds = append(cmds, fmt.Sprintf("nc -z -w 5 %s %d || exit 1", ip, nodeToNodeTestPort)) + } return &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: nodeToNodeNamespace, Labels: map[string]string{ "app.kubernetes.io/managed-by": "nvcf-cluster-validator", - "app.kubernetes.io/component": "n2n-probe", + "app.kubernetes.io/component": "n2n-checker", }, }, Spec: corev1.PodSpec{ @@ -1254,9 +1294,9 @@ func buildNodeToNodeClientPod(name, nodeName, serverIP string) *corev1.Pod { RestartPolicy: corev1.RestartPolicyNever, ActiveDeadlineSeconds: &deadline, Containers: []corev1.Container{{ - Name: "client", + Name: "checker", Image: nodeToNodeImage, - Command: []string{"sh", "-c", fmt.Sprintf("nc -z -w 5 %s %d", serverIP, nodeToNodeTestPort)}, + Command: []string{"sh", "-c", strings.Join(cmds, " && ")}, Resources: enforcementResources(), SecurityContext: nodeToNodeSecurityContext(), }}, @@ -1264,6 +1304,168 @@ func buildNodeToNodeClientPod(name, nodeName, serverIP string) *corev1.Pod { } } +// controlPlaneNamespaces is the set of namespaces scanned by Tier-1 and +// Tier-2 HA checks on the control-plane cluster. +var controlPlaneNamespaces = []string{ + "nvcf", "sis", "api-keys", "ess", "ncp", + "nats-system", "vault-system", "cassandra-system", "envoy-gateway-system", +} + +// checkTier1Deployments verifies that every Deployment in the control-plane +// namespaces has readyReplicas >= spec.replicas. Any under-replicated Deployment +// means HA headroom is gone and a second failure causes a full outage. +// +// The check is generic — no hardcoded Deployment names. New services added to +// those namespaces are automatically covered. +// +// Critical: under-replication means a single additional failure causes a full +// service outage. +func checkTier1Deployments(ctx context.Context, client kubernetes.Interface, state *ValidationState) { + log := state.Log + printHeader(log, "Tier-1 Deployment Readiness") + + var underReplicated []string + checkedCount := 0 + + for _, ns := range controlPlaneNamespaces { + deploys, err := client.AppsV1().Deployments(ns).List(ctx, metav1.ListOptions{}) + if err != nil { + if apierrors.IsNotFound(err) || apierrors.IsForbidden(err) { + continue + } + printWarning(log, fmt.Sprintf("Could not list Deployments in %s: %v", ns, err)) + return // leave nil on API error + } + for i := range deploys.Items { + d := &deploys.Items[i] + checkedCount++ + want := int32(1) + if d.Spec.Replicas != nil { + want = *d.Spec.Replicas + } + if d.Status.ReadyReplicas < want { + underReplicated = append(underReplicated, + fmt.Sprintf("%s/%s (ready: %d, want: %d)", ns, d.Name, d.Status.ReadyReplicas, want)) + } + } + } + + if checkedCount == 0 { + printInfo(log, " No Deployments found in control-plane namespaces (pre-install state)") + ok := true + state.Tier1DeploymentsOK = &ok + return + } + + if len(underReplicated) > 0 { + printError(log, fmt.Sprintf("Under-replicated Deployments (%d):", len(underReplicated))) + for _, name := range underReplicated { + printInfo(log, " "+name) + } + state.Recommendations = append(state.Recommendations, + "Apply the Helmfile resilience profile (resilience.enabled=true) to bring Tier-1 services to >= 2 replicas.") + ok := false + state.Tier1DeploymentsOK = &ok + return + } + + printSuccess(log, fmt.Sprintf("All %d Deployments in control-plane namespaces are fully ready", checkedCount)) + ok := true + state.Tier1DeploymentsOK = &ok +} + +// checkTier2StatefulSets verifies quorum membership and node placement for +// Tier-2 stateful components (NATS JetStream, OpenBao Raft, Cassandra). +// Any StatefulSet with spec.replicas == 3 is treated as a quorum component +// and checked for: +// 1. readyReplicas == 3 +// 2. all 3 pods on distinct nodes +// +// The check is generic — no hardcoded StatefulSet names. +// +// Critical: broken quorum or co-located peers leave the stack one failure +// away from a total control-plane outage. +func checkTier2StatefulSets(ctx context.Context, client kubernetes.Interface, state *ValidationState) { + log := state.Log + printHeader(log, "Tier-2 StatefulSet Quorum and Placement") + + const quorumSize = int32(3) + var failures []string + checkedCount := 0 + + for _, ns := range controlPlaneNamespaces { + stsList, err := client.AppsV1().StatefulSets(ns).List(ctx, metav1.ListOptions{}) + if err != nil { + if apierrors.IsNotFound(err) || apierrors.IsForbidden(err) { + continue + } + printWarning(log, fmt.Sprintf("Could not list StatefulSets in %s: %v", ns, err)) + return // leave nil on API error + } + + for i := range stsList.Items { + sts := &stsList.Items[i] + if sts.Spec.Replicas == nil || *sts.Spec.Replicas != quorumSize { + continue + } + checkedCount++ + + if sts.Status.ReadyReplicas < quorumSize { + failures = append(failures, + fmt.Sprintf("%s/%s: readyReplicas=%d (need %d)", + ns, sts.Name, sts.Status.ReadyReplicas, quorumSize)) + continue + } + + selector := metav1.FormatLabelSelector(sts.Spec.Selector) + pods, err := client.CoreV1().Pods(ns).List(ctx, metav1.ListOptions{LabelSelector: selector}) + if err != nil { + failures = append(failures, + fmt.Sprintf("%s/%s: could not list pods: %v", ns, sts.Name, err)) + continue + } + + nodeOwner := make(map[string]string) + for j := range pods.Items { + p := &pods.Items[j] + if p.Status.Phase != corev1.PodRunning { + continue + } + if first, dup := nodeOwner[p.Spec.NodeName]; dup { + failures = append(failures, + fmt.Sprintf("%s/%s: pods %s and %s are co-located on node %s", + ns, sts.Name, first, p.Name, p.Spec.NodeName)) + } else { + nodeOwner[p.Spec.NodeName] = p.Name + } + } + } + } + + if checkedCount == 0 { + printInfo(log, " No quorum StatefulSets (spec.replicas==3) found (pre-install or non-HA install)") + ok := true + state.Tier2StatefulSetsOK = &ok + return + } + + if len(failures) > 0 { + printError(log, fmt.Sprintf("Tier-2 quorum/placement failures (%d):", len(failures))) + for _, f := range failures { + printInfo(log, " "+f) + } + state.Recommendations = append(state.Recommendations, + "Ensure Tier-2 StatefulSets (NATS, OpenBao, Cassandra) have 3 Ready pods each on distinct nodes.") + ok := false + state.Tier2StatefulSetsOK = &ok + return + } + + printSuccess(log, fmt.Sprintf("All %d quorum StatefulSet(s): 3 Ready pods on distinct nodes", checkedCount)) + ok := true + state.Tier2StatefulSetsOK = &ok +} + // checkConfigurableReachability probes user-defined endpoints loaded from the // cluster-validator ConfigMap. func checkConfigurableReachability(state *ValidationState, cfg *ReachabilityConfig) { diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go index d030f1bb1..bf07db2a1 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -143,11 +144,13 @@ func TestCheckEnvoyGateway_NamespacePresentNoRunningPods(t *testing.T) { // -- checkGatewayRoutes -- -func TestCheckGatewayRoutes_NilClientSkips(t *testing.T) { +func TestCheckGatewayRoutes_MissingCRDs(t *testing.T) { + // Fake client with no gateway.networking.k8s.io group registered. + client := fake.NewSimpleClientset() state := &ValidationState{Log: testLog()} - // Should not panic or set GatewayRoutesOK. - checkGatewayRoutes(context.Background(), nil, state) - assert.Nil(t, state.GatewayRoutesOK, "nil dynClient must leave GatewayRoutesOK unset") + checkGatewayRoutes(context.Background(), client, state) + require.NotNil(t, state.GatewayRoutesOK) + assert.False(t, *state.GatewayRoutesOK, "missing route CR types must set GatewayRoutesOK=false") } // -- checkExternalLoadBalancer -- @@ -250,26 +253,27 @@ func TestCheckNodeToNode_UnschedulableNodesSkipped(t *testing.T) { assert.True(t, *state.NodeToNodeOK, "no schedulable nodes must skip, not fail") } -func TestCheckNodeToNode_ServerPodCreateFailure(t *testing.T) { - // Two schedulable nodes, but pod creation fails. +func TestCheckNodeToNode_DaemonSetCreateFailure(t *testing.T) { + // Two schedulable nodes, but DaemonSet creation fails. client := fake.NewSimpleClientset( makeNode("node-1", true, 0), makeNode("node-2", true, 0), ) - client.PrependReactor("create", "pods", func(_ ktesting.Action) (bool, runtime.Object, error) { - return true, nil, fmt.Errorf("pod quota exceeded") + client.PrependReactor("create", "daemonsets", func(_ ktesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("quota exceeded") }) state := &ValidationState{Log: testLog()} checkNodeToNode(context.Background(), client, state) require.NotNil(t, state.NodeToNodeOK) - assert.False(t, *state.NodeToNodeOK, "server pod create failure must set NodeToNodeOK=false") + assert.False(t, *state.NodeToNodeOK, "DaemonSet create failure must set NodeToNodeOK=false") } // init is required to register types with the fake client's object tracker. func init() { _ = []runtime.Object{ + &appsv1.DaemonSet{}, &storagev1.StorageClass{}, &corev1.Namespace{}, &corev1.Pod{}, diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/summary.go b/src/compute-plane-services/nvca/internal/clustervalidator/summary.go index 26e015221..4f99e8791 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/summary.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/summary.go @@ -159,6 +159,9 @@ const ( CheckKeyGatewayRoutes = "gateway_routes" CheckKeyExternalLB = "external_lb" CheckKeyNodeToNode = "node_to_node" + // HA readiness checks (CP Resilience SDD). + CheckKeyTier1Deployments = "tier1_deployments" + CheckKeyTier2StatefulSets = "tier2_statefulsets" ) // AllCheckKeys is the canonical ordering used for documentation and @@ -182,6 +185,8 @@ var AllCheckKeys = []string{ CheckKeyGatewayRoutes, CheckKeyExternalLB, CheckKeyNodeToNode, + CheckKeyTier1Deployments, + CheckKeyTier2StatefulSets, } // buildSummary projects a ValidationState into the wire format. Checks @@ -240,6 +245,12 @@ func buildSummary(state *ValidationState, startedAt time.Time, verdictReady bool if state.NodeToNodeOK != nil { s.Checks[CheckKeyNodeToNode] = *state.NodeToNodeOK } + if state.Tier1DeploymentsOK != nil { + s.Checks[CheckKeyTier1Deployments] = *state.Tier1DeploymentsOK + } + if state.Tier2StatefulSetsOK != nil { + s.Checks[CheckKeyTier2StatefulSets] = *state.Tier2StatefulSetsOK + } if len(state.EndpointResults) > 0 { s.Endpoints = make(map[string]EndpointStatus, len(state.EndpointResults)) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/summary_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/summary_test.go index 6e91a5e66..8ad316222 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/summary_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/summary_test.go @@ -289,8 +289,11 @@ func TestAllCheckKeysCoversEveryCheckKeyConst(t *testing.T) { CheckKeyGatewayRoutes, CheckKeyExternalLB, CheckKeyNodeToNode, + // HA readiness keys (CP Resilience SDD). + CheckKeyTier1Deployments, + CheckKeyTier2StatefulSets, } { assert.True(t, known[k], "%q is a CheckKey constant but missing from AllCheckKeys", k) } - assert.Len(t, AllCheckKeys, 16, "if you added a new CheckKey, also add it to AllCheckKeys AND to clusterValidatorCheckKeys() in internal/metrics/metrics.go") + assert.Len(t, AllCheckKeys, 18, "if you added a new CheckKey, also add it to AllCheckKeys AND to clusterValidatorCheckKeys() in internal/metrics/metrics.go") } diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go index ed08e41b1..ff2adda06 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go @@ -24,7 +24,6 @@ import ( "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/core" "github.com/sirupsen/logrus" - "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" ) @@ -88,6 +87,10 @@ type ValidationState struct { // NodeToNodeOK is nil when the check was skipped (single-node cluster or // compute-plane role). true = overlay verified, false = failed. NodeToNodeOK *bool + // Tier1DeploymentsOK is nil when no Deployments were found (pre-install). + Tier1DeploymentsOK *bool + // Tier2StatefulSetsOK is nil when no quorum StatefulSets (spec.replicas==3) were found. + Tier2StatefulSetsOK *bool // EndpointResults captures per-endpoint reachability outcomes for the // summary ConfigMap / metrics pipeline. Keyed by the user-supplied @@ -124,7 +127,6 @@ type NetpolPairResult struct { func Run( ctx context.Context, client kubernetes.Interface, - dynClient dynamic.Interface, configNamespace, configName, summaryNamespace string, emitMetrics bool, role string, @@ -181,14 +183,13 @@ func Run( checkStorageClass(ctx, client, state) checkGatewayAPICRDs(ctx, client, state) checkEnvoyGateway(ctx, client, state) - checkGatewayRoutes(ctx, dynClient, state) + checkGatewayRoutes(ctx, client, state) checkExternalLoadBalancer(ctx, client, state) - // Node-to-node creates pods and requires pod-create RBAC. Skip during - // preflight (emitMetrics=false) where the SA may not hold that permission; - // run only for in-cluster scheduled checks where the SA is fully provisioned. - if emitMetrics { - checkNodeToNode(ctx, client, state) - } + // CLI RBAC bootstrap (Req 3) grants DaemonSet create/delete and + // pod-create before Job submission — no emitMetrics gate needed. + checkNodeToNode(ctx, client, state) + checkTier1Deployments(ctx, client, state) + checkTier2StatefulSets(ctx, client, state) } else { // Compute-plane cluster (default): GPU operator, SMB CSI driver. checkSMBCSIDriver(ctx, client, state) @@ -302,6 +303,14 @@ func printSummary(state *ValidationState) error { checks = append(checks, check{*state.NodeToNodeOK, "Node-to-Node Communication: Verified", "Node-to-Node Communication: Failed", true}) } + if state.Tier1DeploymentsOK != nil { + checks = append(checks, check{*state.Tier1DeploymentsOK, + "Tier-1 Deployments: All Ready", "Tier-1 Deployments: Under-replicated", true}) + } + if state.Tier2StatefulSetsOK != nil { + checks = append(checks, check{*state.Tier2StatefulSetsOK, + "Tier-2 StatefulSets: Quorum and Placement OK", "Tier-2 StatefulSets: Quorum or Placement Failed", true}) + } } else { // Compute-plane checks: GPU resources, GPU operator, SMB CSI driver. // SMB CSI Driver missing is non-blocking: it is required only when diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go index d7e3e3016..a483b9c74 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator_test.go @@ -64,7 +64,7 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { t.Run("preflight (emitMetrics=false) does not write the summary", func(t *testing.T) { client := fake.NewSimpleClientset() - _ = Run(context.Background(), client, nil, ns, "cluster-validator-network-checks", ns, false, "") + _ = Run(context.Background(), client, ns, "cluster-validator-network-checks", ns, false, "") _, err := client.CoreV1().ConfigMaps(ns).Get( context.Background(), SummaryConfigMapName, metav1.GetOptions{}) assert.True(t, apierrors.IsNotFound(err), @@ -73,7 +73,7 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { t.Run("post-install (emitMetrics=true) writes the summary", func(t *testing.T) { client := fake.NewSimpleClientset() - _ = Run(context.Background(), client, nil, ns, "cluster-validator-network-checks", ns, true, "") + _ = Run(context.Background(), client, ns, "cluster-validator-network-checks", ns, true, "") cm, err := client.CoreV1().ConfigMaps(ns).Get( context.Background(), SummaryConfigMapName, metav1.GetOptions{}) require.NoError(t, err, "summary ConfigMap must be written when emitMetrics=true") @@ -85,7 +85,7 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { // Guards the decoupling: a non-operator config namespace must NOT // redirect the summary away from the namespace the agent watches. client := fake.NewSimpleClientset() - _ = Run(context.Background(), client, nil, "some-config-ns", "cluster-validator-network-checks", ns, true, "") + _ = Run(context.Background(), client, "some-config-ns", "cluster-validator-network-checks", ns, true, "") _, err := client.CoreV1().ConfigMaps(ns).Get( context.Background(), SummaryConfigMapName, metav1.GetOptions{}) @@ -103,7 +103,7 @@ func TestRun_EmitMetricsGatesSummaryWrite(t *testing.T) { // because of missing StorageClass or Gateway CRDs, not because of GPUAvailable. func TestRun_ControlPlaneRoleSkipsGPUChecks(t *testing.T) { client := fake.NewSimpleClientset(makeNode("node-1", true, 0)) - err := Run(context.Background(), client, nil, "ns", "cfg", "ns", false, RoleControlPlane) + err := Run(context.Background(), client, "ns", "cfg", "ns", false, RoleControlPlane) // A bare fake cluster fails control-plane checks (no StorageClass, no Gateway CRDs). require.Error(t, err) assert.Contains(t, err.Error(), "NVCF-Not-Ready", diff --git a/src/compute-plane-services/nvca/internal/metrics/metrics.go b/src/compute-plane-services/nvca/internal/metrics/metrics.go index 558d6a203..724ae055e 100644 --- a/src/compute-plane-services/nvca/internal/metrics/metrics.go +++ b/src/compute-plane-services/nvca/internal/metrics/metrics.go @@ -1317,6 +1317,9 @@ func clusterValidatorCheckKeys() []string { "gateway_routes", "external_lb", "node_to_node", + // HA readiness keys (CP Resilience SDD). + "tier1_deployments", + "tier2_statefulsets", } } From 29180fb666595f3d647fdfae1ee14c62c3ac1f95 Mon Sep 17 00:00:00 2001 From: rohithb Date: Mon, 17 Aug 2026 23:41:36 +0530 Subject: [PATCH 08/12] fix(nvca): remove activeDeadlineSeconds from DaemonSet pod template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kubernetes rejects DaemonSets with activeDeadlineSeconds in the pod template spec — it is only valid on Pods and Jobs. Cleanup is handled by the deferred DaemonSet delete in checkNodeToNode. --- .../nvca/internal/clustervalidator/checks.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index 41561e0da..75e42f172 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -1251,7 +1251,6 @@ func nodeToNodeSecurityContext() *corev1.SecurityContext { } func buildNodeToNodeDaemonSet(name string, labels map[string]string) *appsv1.DaemonSet { - deadline := nodeToNodeActiveDeadline return &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: nodeToNodeNamespace, Labels: labels}, Spec: appsv1.DaemonSetSpec{ @@ -1259,8 +1258,9 @@ func buildNodeToNodeDaemonSet(name string, labels map[string]string) *appsv1.Dae Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{Labels: labels}, Spec: corev1.PodSpec{ - RestartPolicy: corev1.RestartPolicyAlways, - ActiveDeadlineSeconds: &deadline, + // ActiveDeadlineSeconds is forbidden on DaemonSet pod templates. + // Cleanup is handled by deleting the DaemonSet in the deferred sweep. + RestartPolicy: corev1.RestartPolicyAlways, Containers: []corev1.Container{{ Name: "server", Image: nodeToNodeImage, From 603233df5d78521c751dbee9beb421643afc9742 Mon Sep 17 00:00:00 2001 From: rohithb Date: Tue, 18 Aug 2026 00:17:00 +0530 Subject: [PATCH 09/12] fix(nvca): sweep orphan n2n DaemonSets left by SIGKILL'd validator runs Add sweepOrphanN2NDaemonSets to delete nvcf-n2n-server-* DaemonSets older than 10 minutes at the start of every validator run. DaemonSets do not support activeDeadlineSeconds so a SIGKILL before defer fires leaves server pods running on every node indefinitely. The 10-minute TTL avoids racing with concurrent runs (checker timeout is 90s). --- .../nvca/internal/clustervalidator/checks.go | 44 +++++++++++++++++++ .../internal/clustervalidator/validator.go | 1 + 2 files changed, 45 insertions(+) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index 75e42f172..ea2876a6c 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -27,6 +27,7 @@ import ( "strings" "time" + "github.com/sirupsen/logrus" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -1087,6 +1088,44 @@ const ( nodeToNodeCheckerTimeout = 90 * time.Second ) +// sweepOrphanN2NDaemonSets deletes any nvcf-n2n-server-* DaemonSets older +// than ttl. These are left behind when the validator process is killed with +// SIGKILL (OOM, force-delete, node failure) before the deferred cleanup fires. +// DaemonSets younger than ttl are skipped in case they belong to a concurrent run. +func sweepOrphanN2NDaemonSets(ctx context.Context, log *logrus.Entry, client kubernetes.Interface, ttl time.Duration) { + listCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + + dsList, err := client.AppsV1().DaemonSets(nodeToNodeNamespace).List(listCtx, metav1.ListOptions{ + LabelSelector: "app.kubernetes.io/managed-by=nvcf-cluster-validator,app.kubernetes.io/component=n2n-server", + }) + if err != nil || len(dsList.Items) == 0 { + return + } + + cutoff := time.Now().Add(-ttl) + grace := int64(0) + deleted := 0 + for i := range dsList.Items { + ds := &dsList.Items[i] + if ds.CreationTimestamp.After(cutoff) { + continue // still within TTL — might be a concurrent run + } + delCtx, delCancel := context.WithTimeout(ctx, 30*time.Second) + err := client.AppsV1().DaemonSets(nodeToNodeNamespace).Delete(delCtx, ds.Name, + metav1.DeleteOptions{GracePeriodSeconds: &grace}) + delCancel() + if err != nil && !apierrors.IsNotFound(err) { + log.Warnf("N2N orphan sweep: failed to delete DaemonSet %s: %v", ds.Name, err) + continue + } + deleted++ + } + if deleted > 0 { + printInfo(log, fmt.Sprintf("N2N orphan sweep: deleted %d stale server DaemonSet(s) older than %s", deleted, ttl)) + } +} + // checkNodeToNode verifies overlay-network connectivity across all schedulable // nodes using a DaemonSet-based probe. A server DaemonSet is deployed on every // schedulable node; a checker pod on node[0] connects to each server pod IP on @@ -1101,6 +1140,11 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va log := state.Log printHeader(log, "Node-to-Node Communication") + // Reclaim DaemonSets orphaned by prior runs killed before their deferred + // cleanup fired (SIGKILL, OOM, node failure). TTL of 10 minutes is long + // enough to avoid racing with concurrent runs (checker timeout is 90s). + sweepOrphanN2NDaemonSets(ctx, log, client, 10*time.Minute) + nodes, err := client.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) if err != nil { printWarning(log, fmt.Sprintf("Could not list nodes: %v", err)) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go index ff2adda06..9a1b2b9ff 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go @@ -157,6 +157,7 @@ func Run( // flow). Runs unconditionally so orphans get reclaimed even if enforcement // is currently disabled. sweepOrphanTestNamespaces(ctx, log, client, orphanNamespaceTTL) + sweepOrphanN2NDaemonSets(ctx, log, client, 10*time.Minute) checkControlPlaneHealth(ctx, client, state) checkWebhookSupport(ctx, client, state) From 8c5bafe5ced770de3619c4a460d34bde137910ad Mon Sep 17 00:00:00 2001 From: rohithb Date: Tue, 18 Aug 2026 15:23:48 +0530 Subject: [PATCH 10/12] fix(nvca): fix Bazel dep, DaemonSet taint handling, orphan sweep cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - BUILD.bazel: add k8s.io/api/apps/v1 dep (CI failure), remove k8s.io/client-go/dynamic and apimachinery/pkg/runtime/schema (no longer used after removing dynClient and reworking route check) - checkNodeToNode: use DaemonSet.Status.DesiredNumberScheduled as the waitForDaemonSetPods target instead of len(schedulable). The DaemonSet scheduler respects taints and tolerations, so nodes with NoSchedule taints that the DaemonSet has no toleration for are excluded from DesiredNumberScheduled. Waiting on len(schedulable) would block on pods that can never be scheduled. Fall back to len(schedulable) when the status field is not populated immediately after creation. - checkNodeToNode: select checkerNode from a Running server pod instead of schedulable[0], so the checker is guaranteed to be on a node where the DaemonSet actually scheduled. - Remove duplicate sweepOrphanN2NDaemonSets call from Run() — the sweep is already called inside checkNodeToNode which is the only place that creates n2n DaemonSets. Add orphanN2NDaemonSetTTL named constant. - sweepOrphanN2NDaemonSets: log a warning when the DaemonSet list call fails instead of silently discarding the error. --- .../internal/clustervalidator/BUILD.bazel | 3 +- .../nvca/internal/clustervalidator/checks.go | 39 ++++++++++++++----- .../internal/clustervalidator/validator.go | 1 - 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel b/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel index 707da8687..609581d40 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel +++ b/src/compute-plane-services/nvca/internal/clustervalidator/BUILD.bazel @@ -20,16 +20,15 @@ go_library( deps = [ "//src/compute-plane-services/nvca/vendor/github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/core", "//src/compute-plane-services/nvca/vendor/github.com/sirupsen/logrus", + "//src/compute-plane-services/nvca/vendor/k8s.io/api/apps/v1:apps", "//src/compute-plane-services/nvca/vendor/k8s.io/api/core/v1:core", "//src/compute-plane-services/nvca/vendor/k8s.io/api/networking/v1:networking", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/api/errors", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/api/resource", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/apis/meta/v1:meta", - "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/runtime/schema", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/util/rand", "//src/compute-plane-services/nvca/vendor/k8s.io/apimachinery/pkg/util/intstr", "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/discovery", - "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/dynamic", "//src/compute-plane-services/nvca/vendor/k8s.io/client-go/kubernetes", "//src/compute-plane-services/nvca/vendor/sigs.k8s.io/yaml", ], diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index ea2876a6c..b9197945c 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -1086,6 +1086,10 @@ const ( nodeToNodeActiveDeadline = int64(180) nodeToNodeDSTimeout = 2 * time.Minute nodeToNodeCheckerTimeout = 90 * time.Second + // orphanN2NDaemonSetTTL is the minimum age before a leftover nvcf-n2n-server-* + // DaemonSet is swept. Must exceed nodeToNodeCheckerTimeout to avoid racing + // with a concurrent run. + orphanN2NDaemonSetTTL = 10 * time.Minute ) // sweepOrphanN2NDaemonSets deletes any nvcf-n2n-server-* DaemonSets older @@ -1099,7 +1103,11 @@ func sweepOrphanN2NDaemonSets(ctx context.Context, log *logrus.Entry, client kub dsList, err := client.AppsV1().DaemonSets(nodeToNodeNamespace).List(listCtx, metav1.ListOptions{ LabelSelector: "app.kubernetes.io/managed-by=nvcf-cluster-validator,app.kubernetes.io/component=n2n-server", }) - if err != nil || len(dsList.Items) == 0 { + if err != nil { + log.Warnf("N2N orphan sweep: failed to list DaemonSets in %s: %v", nodeToNodeNamespace, err) + return + } + if len(dsList.Items) == 0 { return } @@ -1141,9 +1149,8 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va printHeader(log, "Node-to-Node Communication") // Reclaim DaemonSets orphaned by prior runs killed before their deferred - // cleanup fired (SIGKILL, OOM, node failure). TTL of 10 minutes is long - // enough to avoid racing with concurrent runs (checker timeout is 90s). - sweepOrphanN2NDaemonSets(ctx, log, client, 10*time.Minute) + // cleanup fired (SIGKILL, OOM, node failure). + sweepOrphanN2NDaemonSets(ctx, log, client, orphanN2NDaemonSetTTL) nodes, err := client.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) if err != nil { @@ -1184,18 +1191,30 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va _ = client.CoreV1().Pods(nodeToNodeNamespace).Delete(context.Background(), checkerName, opts) }() - if _, err := client.AppsV1().DaemonSets(nodeToNodeNamespace).Create( + ds, err := client.AppsV1().DaemonSets(nodeToNodeNamespace).Create( ctx, buildNodeToNodeDaemonSet(dsName, dsLabels), metav1.CreateOptions{}, - ); err != nil { + ) + if err != nil { printError(log, fmt.Sprintf("Failed to create server DaemonSet: %v", err)) ok := false state.NodeToNodeOK = &ok return } - log.Infof(" Waiting for server DaemonSet pods on %d nodes...", len(schedulable)) + // Use DesiredNumberScheduled from the DaemonSet status rather than + // len(schedulable): the scheduler respects taints and tolerations, so nodes + // with NoSchedule taints the DaemonSet has no toleration for are excluded. + // Waiting for len(schedulable) would block on pods that can never be scheduled. + wantPods := int(ds.Status.DesiredNumberScheduled) + if wantPods == 0 { + // Status may not be populated immediately after creation; fall back to + // the schedulable count and let the timeout surface any real problems. + wantPods = len(schedulable) + } + + log.Infof(" Waiting for server DaemonSet pods on %d nodes...", wantPods) selector := metav1.FormatLabelSelector(&metav1.LabelSelector{MatchLabels: dsLabels}) - serverPods, err := waitForDaemonSetPods(ctx, client, nodeToNodeNamespace, selector, len(schedulable), nodeToNodeDSTimeout) + serverPods, err := waitForDaemonSetPods(ctx, client, nodeToNodeNamespace, selector, wantPods, nodeToNodeDSTimeout) if err != nil { printError(log, fmt.Sprintf("Server DaemonSet pods did not become ready: %v", err)) ok := false @@ -1203,7 +1222,9 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va return } - checkerNode := schedulable[0] + // Select checkerNode from a Running server pod so it is guaranteed to be + // a node where the DaemonSet actually scheduled. + checkerNode := serverPods[0].Spec.NodeName var targetIPs []string for i := range serverPods { if serverPods[i].Spec.NodeName != checkerNode && serverPods[i].Status.PodIP != "" { diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go index 9a1b2b9ff..ff2adda06 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go @@ -157,7 +157,6 @@ func Run( // flow). Runs unconditionally so orphans get reclaimed even if enforcement // is currently disabled. sweepOrphanTestNamespaces(ctx, log, client, orphanNamespaceTTL) - sweepOrphanN2NDaemonSets(ctx, log, client, 10*time.Minute) checkControlPlaneHealth(ctx, client, state) checkWebhookSupport(ctx, client, state) From f810b2222caf192f0b433e2d89c50b7ab1d6509a Mon Sep 17 00:00:00 2001 From: rohithb Date: Tue, 18 Aug 2026 16:02:41 +0530 Subject: [PATCH 11/12] fix(nvca): address CodeRabbit review comments Em dashes: replace U+2014 with ASCII punctuation in all new strings, comments, and godoc added in this branch (checks.go, validator.go). Tier-1 rolling update false positive: skip Deployments where a rolling update is in progress (ObservedGeneration < Generation or UpdatedReplicas < spec.replicas) to avoid flagging transient readiness drops during normal rollouts as under-replication failures. Fix recommendation text to not reference a specific replica count. Nil comments: correct Tier1DeploymentsOK and Tier2StatefulSetsOK godoc to state they are nil only when the check did not run or a list call failed; pre-install (no resources found) yields true, not nil. Tainted node regression test: add TestCheckNodeToNode_TaintedNodeExcluded covering a 3-node cluster with one NoSchedule taint. The test captures the DaemonSet's label set (including the random instance suffix) so the pod-list reactor returns pods that survive FakePods.List label filtering. The test proves waitForDaemonSetPods converges on DesiredNumberScheduled=2 rather than hanging on len(schedulable)=3. --- .../nvca/internal/clustervalidator/checks.go | 25 +++++--- .../checks_controlplane_test.go | 61 +++++++++++++++++++ .../internal/clustervalidator/validator.go | 10 ++- 3 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index b9197945c..37baffb32 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -1003,7 +1003,7 @@ func checkGatewayRoutes(ctx context.Context, client kubernetes.Interface, state if len(missing) > 0 { printWarning(log, fmt.Sprintf("Route CR types not registered: %s", strings.Join(missing, ", "))) state.Warnings = append(state.Warnings, - "Gateway Routes: route CR types missing — install Gateway API CRDs via nvcf up") + "Gateway Routes: route CR types missing; install Gateway API CRDs via nvcf up") ok := false state.GatewayRoutesOK = &ok return @@ -1117,7 +1117,7 @@ func sweepOrphanN2NDaemonSets(ctx context.Context, log *logrus.Entry, client kub for i := range dsList.Items { ds := &dsList.Items[i] if ds.CreationTimestamp.After(cutoff) { - continue // still within TTL — might be a concurrent run + continue // still within TTL; might be a concurrent run } delCtx, delCancel := context.WithTimeout(ctx, 30*time.Second) err := client.AppsV1().DaemonSets(nodeToNodeNamespace).Delete(delCtx, ds.Name, @@ -1167,9 +1167,9 @@ func checkNodeToNode(ctx context.Context, client kubernetes.Interface, state *Va } if len(schedulable) < 2 { - printInfo(log, fmt.Sprintf(" %d schedulable node(s) — node-to-node check skipped", len(schedulable))) + printInfo(log, fmt.Sprintf(" %d schedulable node(s); node-to-node check skipped", len(schedulable))) state.Warnings = append(state.Warnings, - "Node-to-Node: skipped — fewer than 2 schedulable nodes") + "Node-to-Node: skipped (fewer than 2 schedulable nodes)") ok := true state.NodeToNodeOK = &ok return @@ -1380,7 +1380,7 @@ var controlPlaneNamespaces = []string{ // namespaces has readyReplicas >= spec.replicas. Any under-replicated Deployment // means HA headroom is gone and a second failure causes a full outage. // -// The check is generic — no hardcoded Deployment names. New services added to +// The check is generic; no hardcoded Deployment names. New services added to // those namespaces are automatically covered. // // Critical: under-replication means a single additional failure causes a full @@ -1408,6 +1408,17 @@ func checkTier1Deployments(ctx context.Context, client kubernetes.Interface, sta if d.Spec.Replicas != nil { want = *d.Spec.Replicas } + // Skip Deployments where a rolling update is in progress. + // During a rollout, readyReplicas transiently drops below + // spec.replicas even on healthy clusters. A rollout is in + // progress when the controller has not yet reconciled the + // generation (ObservedGeneration < Generation) or when not + // all pods have been updated (UpdatedReplicas < spec.replicas). + rollingOut := d.Status.ObservedGeneration < d.Generation || + d.Status.UpdatedReplicas < want + if rollingOut { + continue + } if d.Status.ReadyReplicas < want { underReplicated = append(underReplicated, fmt.Sprintf("%s/%s (ready: %d, want: %d)", ns, d.Name, d.Status.ReadyReplicas, want)) @@ -1428,7 +1439,7 @@ func checkTier1Deployments(ctx context.Context, client kubernetes.Interface, sta printInfo(log, " "+name) } state.Recommendations = append(state.Recommendations, - "Apply the Helmfile resilience profile (resilience.enabled=true) to bring Tier-1 services to >= 2 replicas.") + "Check for crashed or evicted pods in control-plane namespaces. If the resilience profile is not yet applied, enable it (resilience.enabled=true) to ensure Tier-1 services run with multiple replicas.") ok := false state.Tier1DeploymentsOK = &ok return @@ -1446,7 +1457,7 @@ func checkTier1Deployments(ctx context.Context, client kubernetes.Interface, sta // 1. readyReplicas == 3 // 2. all 3 pods on distinct nodes // -// The check is generic — no hardcoded StatefulSet names. +// The check is generic; no hardcoded StatefulSet names. // // Critical: broken quorum or co-located peers leave the stack one failure // away from a total control-plane outage. diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go index bf07db2a1..4766b3b96 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go @@ -253,6 +253,66 @@ func TestCheckNodeToNode_UnschedulableNodesSkipped(t *testing.T) { assert.True(t, *state.NodeToNodeOK, "no schedulable nodes must skip, not fail") } +func TestCheckNodeToNode_TaintedNodeExcluded(t *testing.T) { + // Three nodes: two schedulable, one with a NoSchedule taint. + // DesiredNumberScheduled=2 (tainted node excluded by scheduler), so + // waitForDaemonSetPods must converge on 2 pods, not 3. If the old + // len(schedulable)=3 path were used the test would block until deadline. + n1 := makeNode("node-1", true, 0) + n2 := makeNode("node-2", true, 0) + n3 := makeNode("node-3", true, 0) + n3.Spec.Taints = []corev1.Taint{{ + Key: "dedicated", Value: "gpu", Effect: corev1.TaintEffectNoSchedule, + }} + + client := fake.NewSimpleClientset(n1, n2, n3) + + // Capture DaemonSet labels (which include a random suffix) so the pod-list + // reactor can return pods that survive FakePods.List label filtering. + // capturedLabels is set synchronously by the daemonset create reactor + // before any list call, so no synchronisation is needed. + var capturedLabels map[string]string + client.PrependReactor("create", "daemonsets", func(action ktesting.Action) (bool, runtime.Object, error) { + ds := action.(ktesting.CreateAction).GetObject().(*appsv1.DaemonSet) + capturedLabels = ds.Labels + ds.Status.DesiredNumberScheduled = 2 + return true, ds, nil + }) + + // Return 2 Running pods whose labels match the DaemonSet selector. + // FakePods.List filters by label after the reactor returns, so pods must + // carry the full label set including the random instance suffix. + client.PrependReactor("list", "pods", func(_ ktesting.Action) (bool, runtime.Object, error) { + lbl := capturedLabels + return true, &corev1.PodList{Items: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{Name: "s-1", Namespace: nodeToNodeNamespace, Labels: lbl}, + Spec: corev1.PodSpec{NodeName: "node-1"}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, PodIP: "10.0.0.1"}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "s-2", Namespace: nodeToNodeNamespace, Labels: lbl}, + Spec: corev1.PodSpec{NodeName: "node-2"}, + Status: corev1.PodStatus{Phase: corev1.PodRunning, PodIP: "10.0.0.2"}, + }, + }}, nil + }) + + // Fail checker pod creation so the test exits quickly without needing to + // simulate full pod lifecycle (no Get/poll needed). + client.PrependReactor("create", "pods", func(_ ktesting.Action) (bool, runtime.Object, error) { + return true, nil, fmt.Errorf("no pods scheduled") + }) + + state := &ValidationState{Log: testLog()} + checkNodeToNode(context.Background(), client, state) + + // NodeToNodeOK must be non-nil: the check reached the checker-pod step, + // proving waitForDaemonSetPods did not block waiting for the tainted node. + require.NotNil(t, state.NodeToNodeOK, "check must not hang waiting for tainted node") + assert.False(t, *state.NodeToNodeOK, "NodeToNodeOK false because checker pod creation failed") +} + func TestCheckNodeToNode_DaemonSetCreateFailure(t *testing.T) { // Two schedulable nodes, but DaemonSet creation fails. client := fake.NewSimpleClientset( @@ -280,3 +340,4 @@ func init() { &corev1.Service{}, } } + diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go index ff2adda06..e4b02aae6 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/validator.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/validator.go @@ -87,9 +87,13 @@ type ValidationState struct { // NodeToNodeOK is nil when the check was skipped (single-node cluster or // compute-plane role). true = overlay verified, false = failed. NodeToNodeOK *bool - // Tier1DeploymentsOK is nil when no Deployments were found (pre-install). + // Tier1DeploymentsOK is nil when the check did not run (compute-plane role) + // or when a Deployment list call fails. Pre-install (no Deployments found) + // sets this to true, not nil. Tier1DeploymentsOK *bool - // Tier2StatefulSetsOK is nil when no quorum StatefulSets (spec.replicas==3) were found. + // Tier2StatefulSetsOK is nil when the check did not run (compute-plane role) + // or when a StatefulSet list call fails. No quorum StatefulSets found + // (pre-install or non-HA install) sets this to true, not nil. Tier2StatefulSetsOK *bool // EndpointResults captures per-endpoint reachability outcomes for the @@ -186,7 +190,7 @@ func Run( checkGatewayRoutes(ctx, client, state) checkExternalLoadBalancer(ctx, client, state) // CLI RBAC bootstrap (Req 3) grants DaemonSet create/delete and - // pod-create before Job submission — no emitMetrics gate needed. + // pod-create before Job submission; no emitMetrics gate needed. checkNodeToNode(ctx, client, state) checkTier1Deployments(ctx, client, state) checkTier2StatefulSets(ctx, client, state) From 8e332e667764cbab4be27efd4ed38e77bde5464e Mon Sep 17 00:00:00 2001 From: rohithb Date: Tue, 18 Aug 2026 16:28:51 +0530 Subject: [PATCH 12/12] feat(nvca): warn on in-progress Tier-1 rollouts and strengthen tainted-node regression test --- .../nvca/internal/clustervalidator/checks.go | 4 + .../checks_controlplane_test.go | 85 ++++++++++++++++++- 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go index 37baffb32..847ff7e4c 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks.go @@ -1417,6 +1417,10 @@ func checkTier1Deployments(ctx context.Context, client kubernetes.Interface, sta rollingOut := d.Status.ObservedGeneration < d.Generation || d.Status.UpdatedReplicas < want if rollingOut { + msg := fmt.Sprintf("%s/%s: rollout in progress (updated: %d/%d); re-run check after rollout completes", + ns, d.Name, d.Status.UpdatedReplicas, want) + printWarning(log, msg) + state.Warnings = append(state.Warnings, "Tier-1 Deployments: "+msg) continue } if d.Status.ReadyReplicas < want { diff --git a/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go b/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go index 4766b3b96..ef11d740a 100644 --- a/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go +++ b/src/compute-plane-services/nvca/internal/clustervalidator/checks_controlplane_test.go @@ -300,16 +300,21 @@ func TestCheckNodeToNode_TaintedNodeExcluded(t *testing.T) { // Fail checker pod creation so the test exits quickly without needing to // simulate full pod lifecycle (no Get/poll needed). + var checkerPodCreateCalled bool client.PrependReactor("create", "pods", func(_ ktesting.Action) (bool, runtime.Object, error) { + checkerPodCreateCalled = true return true, nil, fmt.Errorf("no pods scheduled") }) state := &ValidationState{Log: testLog()} checkNodeToNode(context.Background(), client, state) - // NodeToNodeOK must be non-nil: the check reached the checker-pod step, - // proving waitForDaemonSetPods did not block waiting for the tainted node. - require.NotNil(t, state.NodeToNodeOK, "check must not hang waiting for tainted node") + // checkerPodCreateCalled must be true: if waitForDaemonSetPods had + // waited for 3 pods (len(schedulable)) instead of 2 (DesiredNumberScheduled), + // it would have timed out before reaching pod creation and this flag + // would stay false, catching the regression. + require.True(t, checkerPodCreateCalled, "check must reach checker pod creation step") + require.NotNil(t, state.NodeToNodeOK) assert.False(t, *state.NodeToNodeOK, "NodeToNodeOK false because checker pod creation failed") } @@ -330,6 +335,80 @@ func TestCheckNodeToNode_DaemonSetCreateFailure(t *testing.T) { assert.False(t, *state.NodeToNodeOK, "DaemonSet create failure must set NodeToNodeOK=false") } +// -- checkTier1Deployments -- + +func TestCheckTier1Deployments_AllReady(t *testing.T) { + replicas := int32(2) + client := fake.NewSimpleClientset(&appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "nvcf-api", Namespace: "nvcf"}, + Spec: appsv1.DeploymentSpec{Replicas: &replicas}, + Status: appsv1.DeploymentStatus{ + ObservedGeneration: 1, + UpdatedReplicas: 2, + ReadyReplicas: 2, + }, + }) + state := &ValidationState{Log: testLog()} + checkTier1Deployments(context.Background(), client, state) + + require.NotNil(t, state.Tier1DeploymentsOK) + assert.True(t, *state.Tier1DeploymentsOK) + assert.Empty(t, state.Warnings) +} + +func TestCheckTier1Deployments_UnderReplicated(t *testing.T) { + replicas := int32(2) + client := fake.NewSimpleClientset(&appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nvcf-api", Namespace: "nvcf", + Generation: 1, + }, + Spec: appsv1.DeploymentSpec{Replicas: &replicas}, + Status: appsv1.DeploymentStatus{ + ObservedGeneration: 1, + UpdatedReplicas: 2, + ReadyReplicas: 1, // one pod crashed + }, + }) + state := &ValidationState{Log: testLog()} + checkTier1Deployments(context.Background(), client, state) + + require.NotNil(t, state.Tier1DeploymentsOK) + assert.False(t, *state.Tier1DeploymentsOK, "crashed pod must set Tier1DeploymentsOK=false") +} + +func TestCheckTier1Deployments_RollingOutEmitsWarningNotFailure(t *testing.T) { + replicas := int32(2) + client := fake.NewSimpleClientset(&appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nvcf-api", Namespace: "nvcf", + Generation: 3, // new spec written + }, + Spec: appsv1.DeploymentSpec{Replicas: &replicas}, + Status: appsv1.DeploymentStatus{ + ObservedGeneration: 2, // controller hasn't caught up yet + UpdatedReplicas: 1, // only 1 of 2 pods updated + ReadyReplicas: 2, // old pods still serving (maxUnavailable=0) + }, + }) + state := &ValidationState{Log: testLog()} + checkTier1Deployments(context.Background(), client, state) + + require.NotNil(t, state.Tier1DeploymentsOK) + assert.True(t, *state.Tier1DeploymentsOK, "in-progress rollout must not set Tier1DeploymentsOK=false") + assert.NotEmpty(t, state.Warnings, "rollout in progress must emit a warning") + assert.Contains(t, state.Warnings[0], "rollout in progress") +} + +func TestCheckTier1Deployments_PreInstallPassesTrivially(t *testing.T) { + client := fake.NewSimpleClientset() // no namespaces, no deployments + state := &ValidationState{Log: testLog()} + checkTier1Deployments(context.Background(), client, state) + + require.NotNil(t, state.Tier1DeploymentsOK) + assert.True(t, *state.Tier1DeploymentsOK, "pre-install (no deployments) must pass trivially") +} + // init is required to register types with the fake client's object tracker. func init() { _ = []runtime.Object{