Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions .github/workflows/bazel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 6 additions & 1 deletion src/compute-plane-services/nvca/cmd/internal/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
}
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"fmt"
"io"
"path/filepath"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
82 changes: 54 additions & 28 deletions src/compute-plane-services/nvca/pkg/storage/modelcache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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-<handle>) is stamped with the
// durable populated marker. That label, not an NVMesh primary PV, is the
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading