diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index bfe24ca70..f0e77c427 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -80,14 +80,16 @@ jobs: run: | set -euo pipefail # id|path|tests_skip. tests_skip notes: - # umbrella-phase1: //src/libraries/go/lib test issues (NVCF-10337; - # icms-translate env-flaky, core_test goroutine leak) -- nvcf-go + # umbrella-phase1: //src/libraries/go/lib test issues + # (icms-translate env-flaky, core_test goroutine leak) -- nvcf-go # team owns the test-layer cleanup. - # nvca: tests run. TestReconcile_ModelCache (NVCF-10347) now - # passes under the bazel-ci envtest, verified locally against - # envtest 1.34.1 (5/5). The only flaky test, - # TestBackendK8sCacheQueryAPIs, is t.Skip'd at the source with a - # reference (informer/lister sync race, nvca-team fix). + # nvca: tests run. The failures tracked in NVIDIA/nvcf#238 were all + # test-layer and are fixed: the vendored dra-driver version stamp + # (x_defs on vendor/.../internal/info, mirroring the module + # Makefile's -ldflags -X), SPDX license headers in testdata and + # rendered template assets (empty-YAML-doc skip + stripSPDXHeaders + # in golden comparisons), and the version-adaptive completeJob for + # envtest Job completion. # nvsnap (src/compute-plane-services/nvsnap): intentionally NOT a # matrix row. It is a self-contained Bazel module whose OCI base is # its own private agent image (not publicly pullable), so it is not @@ -100,7 +102,7 @@ jobs: http-invocation|src/invocation-plane-services/http-invocation|false llm-api-gateway|src/invocation-plane-services/llm-api-gateway|false vanity-gateway|src/invocation-plane-services/vanity-gateway|false - nvca|src/compute-plane-services/nvca|true + nvca|src/compute-plane-services/nvca|false ess-agent|src/compute-plane-services/ess-agent|false image-credential-helper|src/compute-plane-services/image-credential-helper|false function-autoscaler|src/control-plane-services/function-autoscaler|false diff --git a/src/compute-plane-services/nvca/cmd/internal/util_test.go b/src/compute-plane-services/nvca/cmd/internal/util_test.go index 615440008..f8f4f7247 100644 --- a/src/compute-plane-services/nvca/cmd/internal/util_test.go +++ b/src/compute-plane-services/nvca/cmd/internal/util_test.go @@ -50,7 +50,12 @@ func TestNewK8sClient(t *testing.T) { kcPath: "", currContext: "", setupEnv: func() { - os.Setenv(clientcmd.RecommendedConfigPathEnvVar, "") + // Point at a path that cannot exist rather than "": an empty + // KUBECONFIG makes client-go fall back to ~/.kube/config, so on + // a machine with a real kubeconfig the client unexpectedly + // builds and the test fails. A nonexistent path keeps the + // intent (no usable kubeconfig) hermetic everywhere. + os.Setenv(clientcmd.RecommendedConfigPathEnvVar, "/nonexistent/kubeconfig") }, cleanupEnv: func() { os.Unsetenv(clientcmd.RecommendedConfigPathEnvVar) diff --git a/src/compute-plane-services/nvca/pkg/nvca/k8scomputebackend_miniservice_test.go b/src/compute-plane-services/nvca/pkg/nvca/k8scomputebackend_miniservice_test.go index 7090d14dc..febf11bd4 100644 --- a/src/compute-plane-services/nvca/pkg/nvca/k8scomputebackend_miniservice_test.go +++ b/src/compute-plane-services/nvca/pkg/nvca/k8scomputebackend_miniservice_test.go @@ -908,6 +908,12 @@ func decodeYAMLFile(t *testing.T, fp string) []*unstructured.Unstructured { err = sigsyaml.Unmarshal(doc, &obj.Object) require.NoError(t, err) + // Skip documents with no content, e.g. the license header comment + // block before the first separator or a trailing separator. + if len(obj.Object) == 0 { + continue + } + objs = append(objs, obj) } diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/miniservice_restrictions_test.go b/src/compute-plane-services/nvca/pkg/operator/reconcile/miniservice_restrictions_test.go index 239ca32d2..d43783805 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/miniservice_restrictions_test.go +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/miniservice_restrictions_test.go @@ -329,7 +329,7 @@ rules: } else if assert.NoError(t, err) { assert.NotNil(t, data) assert.Contains(t, data, MiniServicesPermissionsRoleName) - assert.Equal(t, tt.expRole, data[MiniServicesPermissionsRoleName]) + assert.Equal(t, tt.expRole, stripSPDXHeaders(data[MiniServicesPermissionsRoleName])) } }) } diff --git a/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go b/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go index 504d97df2..ce3f68ea7 100644 --- a/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go +++ b/src/compute-plane-services/nvca/pkg/operator/reconcile/nvcaagent_reconcile_test.go @@ -24,6 +24,7 @@ import ( "fmt" "io" "path/filepath" + "strings" "testing" "time" @@ -59,6 +60,28 @@ func readTestdataFile(t *testing.T, fileName string) string { return string(b) } +// stripSPDXHeaders removes the SPDX license header comment blocks that repo +// license stamping adds to rendered template assets and testdata goldens. The +// headers carry no semantic content (Kubernetes ignores YAML comments), appear +// a different number of times on each side of a golden comparison (once per +// stamped source file), and would otherwise churn every golden assertion each +// time stamping changes. +func stripSPDXHeaders(s string) string { + lines := strings.Split(s, "\n") + out := make([]string, 0, len(lines)) + for i := 0; i < len(lines); i++ { + if strings.HasPrefix(lines[i], "# SPDX-") { + // Swallow the blank line that closes a header block. + if i+1 < len(lines) && strings.TrimSpace(lines[i+1]) == "" { + i++ + } + continue + } + out = append(out, lines[i]) + } + return strings.Join(out, "\n") +} + func TestValidateNVCFBackendParams(t *testing.T) { tests := []struct { name string @@ -2460,7 +2483,7 @@ func TestGetNetworkPoliciesDataEmptyDDCSIPList(t *testing.T) { io.WriteString(b, "---\n") io.WriteString(b, got[k]) } - assert.Equal(t, readTestdataFile(t, filepath.Join("testdata", "netpols.yaml")), b.String()) + assert.Equal(t, stripSPDXHeaders(readTestdataFile(t, filepath.Join("testdata", "netpols.yaml"))), stripSPDXHeaders(b.String())) } func TestGetNetworkPoliciesDataWithDDCSIPList(t *testing.T) { @@ -2493,7 +2516,7 @@ func TestGetNetworkPoliciesDataWithDDCSIPList(t *testing.T) { io.WriteString(b, "---\n") io.WriteString(b, got[k]) } - assert.Equal(t, readTestdataFile(t, filepath.Join("testdata", "netpols_with_ddcs.yaml")), b.String()) + assert.Equal(t, stripSPDXHeaders(readTestdataFile(t, filepath.Join("testdata", "netpols_with_ddcs.yaml"))), stripSPDXHeaders(b.String())) } func assertNetworkPolicyAllowsTCPPort(t *testing.T, policyYAML, policyName string, port int32) { diff --git a/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go b/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go index 4fb77b426..01ce959d4 100644 --- a/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go +++ b/src/compute-plane-services/nvca/pkg/storage/modelcache_test.go @@ -242,20 +242,15 @@ func TestReconcile_ModelCache(t *testing.T) { }, 5*time.Second, 50*time.Millisecond) } - initJob.Status.Succeeded++ - initJob.Status.CompletionTime = &metav1.Time{Time: time.Now()} - initJob.Status.Conditions = append(initJob.Status.Conditions, - batchv1.JobCondition{ - Type: batchv1.JobSuccessCriteriaMet, - Status: corev1.ConditionTrue, - }, - batchv1.JobCondition{ - Type: batchv1.JobComplete, - Status: corev1.ConditionTrue, - }, - ) - err = c.Status().Update(ctx, initJob) - require.NoError(t, err) + // Drive the writer Job to completion. The reconciler keys off + // CompletionTime + Succeeded (see modelcache.go), not the conditions, but + // the apiserver validates the condition shape and its rules differ by + // version: k8s >= 1.34 requires SuccessCriteriaMet before Complete, while + // k8s 1.30-1.33 reject SuccessCriteriaMet on this NonIndexed Job (it has no + // SuccessPolicy). The Job's spec is reconciler-owned and immutable here, so + // apply the modern shape and fall back to the pre-1.34 shape only when the + // running apiserver returns an Invalid validation error. + completeJob(ctx, t, c, initJob) for _, st := range sts { assert.EventuallyWithT(t, func(ct *assert.CollectT) { @@ -705,6 +700,49 @@ func TestMapPodIssuesToFailureReason(t *testing.T) { } } +// completeJob marks a writer Job as succeeded in a way the running apiserver +// accepts. The reconciler detects completion via CompletionTime + Succeeded +// (see modelcache.go), not the Job conditions, but the apiserver still +// validates the condition shape and its rules changed across versions: +// - k8s >= 1.34 requires a SuccessCriteriaMet condition before Complete, and +// rejects CompletionTime without Complete. +// - k8s 1.30-1.33 reject SuccessCriteriaMet on a NonIndexed Job that has no +// SuccessPolicy, which is exactly this writer Job's shape. +// +// The Job's spec is reconciler-owned and immutable on update, so the test +// cannot reshape it to satisfy both. Apply the modern (>= 1.34) condition shape +// and fall back to the pre-1.34 shape only when the apiserver returns an +// Invalid validation error (apierrors.IsInvalid). +func completeJob(ctx context.Context, t *testing.T, c client.Client, job *batchv1.Job) { + t.Helper() + + job.Status.Succeeded = 1 + job.Status.CompletionTime = &metav1.Time{Time: time.Now()} + job.Status.Conditions = append(job.Status.Conditions, + batchv1.JobCondition{Type: batchv1.JobSuccessCriteriaMet, Status: corev1.ConditionTrue}, + batchv1.JobCondition{Type: batchv1.JobComplete, Status: corev1.ConditionTrue}, + ) + err := c.Status().Update(ctx, job) + if err == nil { + return + } + // Only a validation rejection means the modern condition shape is wrong for + // this apiserver version. Any other error (conflict, network, RBAC) is a + // real failure and must surface, not be masked by the fallback write. + require.True(t, apierrors.IsInvalid(err), "unexpected error updating Job status: %v", err) + + // Pre-1.34 apiserver rejected SuccessCriteriaMet. Re-fetch to reset the + // stale in-memory status and apply only Complete, which those versions + // accept alongside CompletionTime. + require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(job), job)) + job.Status.Succeeded = 1 + job.Status.CompletionTime = &metav1.Time{Time: time.Now()} + job.Status.Conditions = append(job.Status.Conditions, + batchv1.JobCondition{Type: batchv1.JobComplete, Status: corev1.ConditionTrue}, + ) + require.NoError(t, c.Status().Update(ctx, job)) +} + // newModelCacheLaunchEnvB64 builds the encoded launch env consumed by the // model cache envtests at runtime. The registry credentials and assertion // token are synthetic and assembled here instead of being committed as an @@ -918,13 +956,7 @@ func TestReconcile_ModelCacheSharedFS(t *testing.T) { require.NoError(t, c.Status().Update(ctx, rwPVC)) require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(initJob), initJob)) - initJob.Status.Succeeded++ - initJob.Status.CompletionTime = &metav1.Time{Time: time.Now()} - initJob.Status.Conditions = append(initJob.Status.Conditions, - batchv1.JobCondition{Type: batchv1.JobSuccessCriteriaMet, Status: corev1.ConditionTrue}, - batchv1.JobCondition{Type: batchv1.JobComplete, Status: corev1.ConditionTrue}, - ) - require.NoError(t, c.Status().Update(ctx, initJob)) + completeJob(ctx, t, c, initJob) // A read-only reader PVC is created in the workload namespace on the shared // class with the probed ROX access mode. @@ -1114,13 +1146,7 @@ func TestReconcile_ModelCacheSamba(t *testing.T) { require.NoError(t, c.Status().Update(ctx, rwPVC)) require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(initJob), initJob)) - initJob.Status.Succeeded++ - initJob.Status.CompletionTime = &metav1.Time{Time: time.Now()} - initJob.Status.Conditions = append(initJob.Status.Conditions, - batchv1.JobCondition{Type: batchv1.JobSuccessCriteriaMet, Status: corev1.ConditionTrue}, - batchv1.JobCondition{Type: batchv1.JobComplete, Status: corev1.ConditionTrue}, - ) - require.NoError(t, c.Status().Update(ctx, initJob)) + completeJob(ctx, t, c, initJob) // On success the per-handle backing PVC (samba-) is stamped with the // durable populated marker. That label, not an NVMesh primary PV, is the diff --git a/src/compute-plane-services/nvca/vendor/github.com/NVIDIA/k8s-dra-driver-gpu/internal/info/BUILD.bazel b/src/compute-plane-services/nvca/vendor/github.com/NVIDIA/k8s-dra-driver-gpu/internal/info/BUILD.bazel index 5001e0f0a..df2f9579c 100644 --- a/src/compute-plane-services/nvca/vendor/github.com/NVIDIA/k8s-dra-driver-gpu/internal/info/BUILD.bazel +++ b/src/compute-plane-services/nvca/vendor/github.com/NVIDIA/k8s-dra-driver-gpu/internal/info/BUILD.bazel @@ -6,6 +6,12 @@ go_library( importmap = "github.com/NVIDIA/nvcf/src/compute-plane-services/nvca/vendor/github.com/NVIDIA/k8s-dra-driver-gpu/internal/info", importpath = "github.com/NVIDIA/k8s-dra-driver-gpu/internal/info", visibility = ["//vendor/github.com/NVIDIA/k8s-dra-driver-gpu:__subpackages__"], + # The dra-driver's featuregates init() panics unless a real version is + # stamped over the "unknown" default. Upstream sets this with go build + # -ldflags -X (see this module's Makefile); mirror it here so every test + # and binary that transitively links this package starts. The relative key + # lets rules_go qualify it against importmap for vendored builds. + x_defs = {"version": "v25.8.0"}, ) alias(