diff --git a/tests/bdd/AGENTS.md b/tests/bdd/AGENTS.md index 30c44ef7b3..202deb25a4 100644 --- a/tests/bdd/AGENTS.md +++ b/tests/bdd/AGENTS.md @@ -66,10 +66,23 @@ logic into `dsl/`. namespace, and intended Gateway parent plus the shared context and timeout. The step requires `Accepted=True` and `ResolvedRefs=True` for that parent but does not allowlist route kinds or duplicate Gateway API validation. -- File-mutating steps (`I copy the file`, `I update yaml file`, - `I prepare self-managed secrets file`, `I substitute a block`) +- File-mutating steps (`I copy the file`, `I write yaml file`, + `I update yaml file`, `I prepare self-managed secrets file`, + `I substitute a block`) snapshot the destination through `Suite.Ledger` before the first write. Suite teardown restores every snapshotted path. +- `CommandRunner` tokenizes command text with shlex and execs the argv + directly. There is no shell, so heredocs, pipes, and redirections only + work inside an explicit `/bin/bash -c '...'` wrapper. Apply inline YAML + through `Kubernetes manifest ... is:` plus `I successfully apply + Kubernetes manifest ... using contexts:` instead of `kubectl apply -f -` + with a heredoc. +- `Kubernetes manifest ... is:` stores the raw docstring in + `ScenarioContext.Manifests`; the apply step interpolates `${VAR}` and + writes the rendered body under `out//` before each + explicit-context `kubectl apply`. Declare manifests with the scenario's + Givens even when they reference an env var exported later, and never + interpolate at declaration time. - `Given command has succeeded:` keys on the fully resolved command text. Two scenarios whose pre-interpolation text matches but whose env vars differ must miss the cache. The cache lives in diff --git a/tests/bdd/PLAN.md b/tests/bdd/PLAN.md index 781810aa6f..edf981a7c7 100644 --- a/tests/bdd/PLAN.md +++ b/tests/bdd/PLAN.md @@ -106,11 +106,23 @@ refactor in every consumer; that is a feature. | Step | Notes | |------|-------| | `And I copy the file {string} to {string}` | Both paths are repo-relative. | +| `And I write yaml file {string} with values:` (two-column table of dotted-path and value) | Creates a new YAML file from the visible table. The destination must not already exist; the step fails instead of overwriting so an authored file is never silently replaced. Parent directories are created. Path syntax and `${VAR}` expansion match `I update yaml file`. Boolean literals and collection literals such as `[]` are written as native YAML types, not quoted strings, because Helm treats the string `"false"` as truthy. Numbers stay as strings. The destination is ledger-backed and removed at teardown. | | `And I update yaml file {string} with keys:` (two-column table of dotted-path and value) | Path supports dotted notation and `[n]` indices (e.g. `global.imagePullSecrets[0].name`). Missing intermediate maps and missing list indices are upserted: writing `global.imagePullSecrets[0].name` against a file that has neither `global.imagePullSecrets` nor any list entry creates both. Existing scalars at intermediate positions cause the step to fail rather than silently overwrite a non-map. Value cells expand `${VAR}` from `os.Environ`. | | `And I prepare Helmfile environment {string} for stack {string} from fixture {string} with values:` (two-column table of dotted-path and value) | Validates the stack and environment names, derives `deploy/stacks//environments/.yaml` from the absolute repository root, copies the explicit fixture, and applies the visible values table with the same YAML update and `${VAR}` interpolation behavior. Supported stacks are `self-managed`, `observability`, and `nvcf-compute-plane`. The destination is ledger-backed. | | `And I prepare self-managed secrets file {string} from template {string} using the current NGC registry credential` | The destination and template are explicit repo-relative paths with `${VAR}` interpolation. Replaces the template's registry credential placeholder with base64 of the current `$oauthtoken:` credential and writes the destination with mode `0600`. The destination is ledger-backed, and secret material never enters Gherkin, command logs, or failure messages. | | `And I substitute a block in file {string}:` (docstring) | The docstring contains an old block and replacement block separated by exactly one `---` line. `${VAR}` interpolation applies before an exact, ledger-backed replacement. Missing or malformed old blocks fail. | +### Kubernetes manifests (Given / When) + +The Given keeps every manifest field visible in the feature file. The When +hides only the repeated `kubectl --context apply -f ` mechanics +and the target contexts stay visible as table rows. + +| Step | Notes | +|------|-------| +| `Given Kubernetes manifest {string} is:` (YAML docstring) | Stores the raw docstring under the visible name in scenario state. The name must be non-empty and declared at most once per scenario. No interpolation, validation, or I/O happens here, so the manifest may reference an env var that a later step exports. | +| `When I successfully apply Kubernetes manifest {string} using contexts:` (table) | Requires a `context` header and one or more contexts. Interpolates `${VAR}` in the named manifest at apply time, writes the rendered body once to a file under the run's `out//` directory, and runs one explicit-context `kubectl apply -f ` per row in order. Each apply must exit 0 and is recorded like any successful command. Failures name the row, manifest, and context. The rendered file is a run artifact, not a ledger-backed working-tree path. | + ### Command execution (When) | Step | Notes | @@ -195,6 +207,7 @@ original order. Repeated options and empty values are preserved. | `Then these Kubernetes resources should exist in namespace {string} using context {string}:` (table) | Requires `kind` and `name` headers. Gets each named resource with the explicit namespace and context, and reports the row whose resource is missing. | | `Then these Kubernetes resources should not exist in namespace {string} using context {string}:` (table) | Requires `kind` and `name` headers. Gets each named resource with `--ignore-not-found` and requires empty name output, so absence does not depend on human-readable error text. | | `Then deployment {string} in namespace {string} using context {string} should complete rollout within {string}` | Runs `kubectl rollout status` for the named deployment with the explicit namespace, context, and timeout. Failure messages name the deployment without printing command output. | +| `Then these Kubernetes workloads should complete rollout using context {string} within {string}:` (table) | Requires `kind`, `name`, and `namespace` headers. Runs one explicit-context `kubectl rollout status /` per row in order with the shared context and timeout. The kind is lowercased and passed through without an allowlist, so Deployment, StatefulSet, and DaemonSet rows all work. Failures name the row, kind, name, and namespace without printing command output. Prefer this over a raw `kubectl rollout status` command step. | | `Then NVCFBackend {string} in namespace {string} using context {string} should report agent status {string} within {string}` | Waits for the named backend's `status.agentStatus` to equal the visible value using the explicit namespace, context, and timeout. Failure messages name the backend without printing resource output. | | `Then these Gateway API routes should be accepted and resolved using context {string} within {string}:` (table) | Requires `kind`, `name`, `namespace`, and `parent` headers. Waits for every named route to report both `Accepted=True` and `ResolvedRefs=True` for the named Gateway parent using the explicit context and timeout. The route kind is passed through without an allowlist. Failures name the table row, route, namespace, parent, and unmet condition without printing resource output. | | `Then every Pylon for function {string} using container {string} and context {string} should report metrics within {string}:` (table) | Requires `metric`, `comparison`, and `count` headers. Polls every running pod selected by the visible `function-name` annotation and container name. Each pod must expose non-empty metrics, and each metric row counts connected series whose sample value is `1`; `comparison` is `exactly` or `at least`, and the expected non-negative count remains visible. Discovery, parsing, and scrape failures remain failures rather than zero metric counts. | @@ -289,7 +302,7 @@ contract verified in `src/clis/nvcf-cli/cmd/`): ## File restoration Every step that writes into a path under the repo working tree -(`I copy the file ... to ...`, `I update yaml file ...`, +(`I copy the file ... to ...`, `I write yaml file ...`, `I update yaml file ...`, `I prepare self-managed secrets file ...`, `I substitute a block ...`) registers that path with the runner's restoration ledger: diff --git a/tests/bdd/dsl/kubectl.go b/tests/bdd/dsl/kubectl.go index 686a2a2876..923f8c0e98 100644 --- a/tests/bdd/dsl/kubectl.go +++ b/tests/bdd/dsl/kubectl.go @@ -118,15 +118,38 @@ func KubernetesResourceAbsent(raw string, resource KubernetesResource) error { return nil } +// KubernetesWorkload identifies one rollout-capable workload by kind, name, +// and namespace. +type KubernetesWorkload struct { + Kind string + Name string + Namespace string +} + // KubernetesDeploymentRolloutCommand builds an explicit-context rollout wait // for one deployment. func KubernetesDeploymentRolloutCommand(name, namespace, kubeContext, timeout string) (string, error) { - target, err := resolveKubernetesWaitTarget("deployment", name, namespace, kubeContext, timeout) + return KubernetesWorkloadRolloutCommand( + KubernetesWorkload{Kind: "deployment", Name: name, Namespace: namespace}, + kubeContext, + timeout, + ) +} + +// KubernetesWorkloadRolloutCommand builds an explicit-context rollout wait +// for one workload. The kind is lowercased and passed through without an +// allowlist, so every kind kubectl rollout status accepts works. +func KubernetesWorkloadRolloutCommand(workload KubernetesWorkload, kubeContext, timeout string) (string, error) { + kind := strings.ToLower(strings.TrimSpace(Interpolate(workload.Kind))) + if kind == "" { + return "", fmt.Errorf("kubernetes workload kind is empty") + } + target, err := resolveKubernetesWaitTarget(kind, workload.Name, workload.Namespace, kubeContext, timeout) if err != nil { return "", err } return strings.Join([]string{ - "kubectl", "rollout", "status", quoteCommandArg("deployment/" + target.name), + "kubectl", "rollout", "status", quoteCommandArg(kind + "/" + target.name), "-n", quoteCommandArg(target.namespace), "--context", quoteCommandArg(target.kubeContext), quoteCommandArg("--timeout=" + target.timeout), diff --git a/tests/bdd/dsl/kubectl_test.go b/tests/bdd/dsl/kubectl_test.go index 7bccccd104..4c0f24662c 100644 --- a/tests/bdd/dsl/kubectl_test.go +++ b/tests/bdd/dsl/kubectl_test.go @@ -114,6 +114,44 @@ func TestKubernetesDeploymentRolloutCommandBuildsExplicitWait(t *testing.T) { } } +// TestKubernetesWorkloadRolloutCommandLowercasesKind verifies that a +// mixed-case kind is lowercased for kubectl and that ${VAR} in the +// workload name is interpolated before the argv is built. +func TestKubernetesWorkloadRolloutCommandLowercasesKind(t *testing.T) { + t.Setenv("BDD_ROUTER_NAME", "llm-request-router-region-b") + got, err := KubernetesWorkloadRolloutCommand( + KubernetesWorkload{Kind: "StatefulSet", Name: "${BDD_ROUTER_NAME}", Namespace: "nvcf"}, + "k3d-ncp-local-cp", + "10m", + ) + if err != nil { + t.Fatalf("build command: %v", err) + } + want := "kubectl rollout status statefulset/llm-request-router-region-b -n nvcf --context k3d-ncp-local-cp --timeout=10m" + if got != want { + t.Fatalf("command = %q, want %q", got, want) + } +} + +// TestKubernetesWorkloadRolloutCommandRejectsMissingInputs confirms that +// an empty kind, name, namespace, or kube context is rejected instead of +// producing a rollout wait against an implicit target. +func TestKubernetesWorkloadRolloutCommandRejectsMissingInputs(t *testing.T) { + cases := map[string]KubernetesWorkload{ + "kind": {Name: "router", Namespace: "nvcf"}, + "name": {Kind: "StatefulSet", Namespace: "nvcf"}, + "namespace": {Kind: "StatefulSet", Name: "router"}, + } + for missing, workload := range cases { + if _, err := KubernetesWorkloadRolloutCommand(workload, "k3d-ncp-local-cp", "10m"); err == nil { + t.Fatalf("expected empty %s error", missing) + } + } + if _, err := KubernetesWorkloadRolloutCommand(KubernetesWorkload{Kind: "Deployment", Name: "router", Namespace: "nvcf"}, "", "10m"); err == nil { + t.Fatal("expected empty context error") + } +} + func TestNVCFBackendAgentStatusCommandBuildsExplicitWait(t *testing.T) { t.Setenv("BDD_BACKEND_NAME", "ncp-local-compute-1") got, err := NVCFBackendAgentStatusCommand("${BDD_BACKEND_NAME}", "nvca-operator", "k3d-ncp-local-compute-1", "healthy", "10m") diff --git a/tests/bdd/dsl/yamledit.go b/tests/bdd/dsl/yamledit.go index 82d02cd3d9..021d4f2ef0 100644 --- a/tests/bdd/dsl/yamledit.go +++ b/tests/bdd/dsl/yamledit.go @@ -45,6 +45,32 @@ const ( MatchSubset ) +// RenderYAMLFromKeys builds a YAML document from the supplied +// dotted-path/value pairs and returns its serialized bytes. It is the +// pure counterpart of UpdateYAMLKeys for a file that does not exist +// yet: the caller owns the destination path, the existence check, and +// the write. Path syntax matches UpdateYAMLKeys. Value cells run +// through Interpolate and then decodeTypedValue so booleans and +// collection literals reach Helm as native YAML types rather than +// quoted strings. +func RenderYAMLFromKeys(keys [][2]string) ([]byte, error) { + rootMap := map[string]any{} + for _, kv := range keys { + segments, err := parsePath(kv[0]) + if err != nil { + return nil, fmt.Errorf("render yaml: %w", err) + } + if err := setNested(rootMap, segments, decodeTypedValue(Interpolate(kv[1]))); err != nil { + return nil, fmt.Errorf("render yaml: %w", err) + } + } + body, err := yaml.Marshal(rootMap) + if err != nil { + return nil, fmt.Errorf("render yaml: marshal: %w", err) + } + return body, nil +} + // UpdateYAMLKeys reads the YAML file at path, applies each (dotted-path, // value) pair as an upsert, and writes the file back. Path syntax uses // "." between segments and "[n]" for list indices; missing intermediate @@ -210,6 +236,30 @@ func SubstituteFileBlock(path, spec string) error { return SubstituteFile(path, oldBlock, newBlock) } +// decodeTypedValue converts the YAML-significant literals that Helm +// evaluates differently when quoted. Booleans are decoded because +// Go templates treat the string "false" as truthy. Collection +// literals like "[]" are decoded so Helm sees an empty list instead +// of a non-empty string. Numbers are left as strings: Helm coerces +// them in template expressions, and eagerly parsing "1.0" as a float +// would lose the trailing zero on round-trip. +func decodeTypedValue(s string) any { + switch s { + case "true": + return true + case "false": + return false + } + var decoded any + if err := yaml.Unmarshal([]byte(s), &decoded); err == nil { + switch decoded.(type) { + case []any, map[string]any: + return decoded + } + } + return s +} + // readYAMLAny reads path and unmarshals into a generic any value. // An empty document parses to nil. func readYAMLAny(path string) (any, error) { diff --git a/tests/bdd/dsl/yamledit_test.go b/tests/bdd/dsl/yamledit_test.go index 1b14576580..fb13376cf9 100644 --- a/tests/bdd/dsl/yamledit_test.go +++ b/tests/bdd/dsl/yamledit_test.go @@ -313,6 +313,107 @@ func TestSubstituteFileBlockRejectsMissingOldBlock(t *testing.T) { } } +// TestRenderYAMLFromKeysBuildsNestedDocument verifies that +// RenderYAMLFromKeys produces a nested YAML structure from dotted-path +// key/value pairs without touching the filesystem. +func TestRenderYAMLFromKeysBuildsNestedDocument(t *testing.T) { + keys := [][2]string{ + {"llmRequestRouter.fullnameOverride", "llm-request-router-region-b"}, + {"llmRequestRouter.replicaCount", "2"}, + {"llmRequestRouter.workload.kind", "StatefulSet"}, + } + body, err := RenderYAMLFromKeys(keys) + if err != nil { + t.Fatalf("render: %v", err) + } + + out := string(body) + for _, want := range []string{ + "fullnameOverride: llm-request-router-region-b", + "replicaCount: \"2\"", + "kind: StatefulSet", + } { + if !strings.Contains(out, want) { + t.Fatalf("output missing %q:\n%s", want, out) + } + } +} + +// TestRenderYAMLFromKeysPreservesBoolsAndCollections verifies that +// booleans and collection literals are decoded to native YAML types +// while numbers remain as quoted strings. +func TestRenderYAMLFromKeysPreservesBoolsAndCollections(t *testing.T) { + keys := [][2]string{ + {"router.enabled", "true"}, + {"router.pki.enabled", "false"}, + {"router.replicaCount", "2"}, + {"router.discovery.remoteWatchUrls", "[]"}, + {"router.name", "region-b"}, + } + body, err := RenderYAMLFromKeys(keys) + if err != nil { + t.Fatalf("render: %v", err) + } + out := string(body) + + for _, want := range []string{ + "enabled: true", + "enabled: false", + "remoteWatchUrls: []", + "name: region-b", + } { + if !strings.Contains(out, want) { + t.Fatalf("output missing %q:\n%s", want, out) + } + } + for _, unwanted := range []string{ + `enabled: "true"`, + `enabled: "false"`, + `remoteWatchUrls: "[]"`, + } { + if strings.Contains(out, unwanted) { + t.Fatalf("value emitted as quoted string %q:\n%s", unwanted, out) + } + } + // Numbers stay as quoted strings; Helm coerces them in templates. + if !strings.Contains(out, `replicaCount: "2"`) { + t.Fatalf("replicaCount should remain a quoted string:\n%s", out) + } +} + +// TestRenderYAMLFromKeysRejectsInvalidPath confirms that a malformed +// dotted path surfaces as an error instead of a partial document. +func TestRenderYAMLFromKeysRejectsInvalidPath(t *testing.T) { + keys := [][2]string{ + {"router.name", "region-b"}, + {"router..enabled", "true"}, + } + body, err := RenderYAMLFromKeys(keys) + if err == nil || !strings.Contains(err.Error(), "empty segment") { + t.Fatalf("err = %v, want invalid-path error", err) + } + if body != nil { + t.Fatalf("body should be nil on error, got:\n%s", body) + } +} + +// TestRenderYAMLFromKeysInterpolatesValues confirms that ${VAR} +// references in value cells are expanded before serialization. +func TestRenderYAMLFromKeysInterpolatesValues(t *testing.T) { + t.Setenv("BDD_TEST_HOST", "region-b.example.invalid") + + keys := [][2]string{ + {"service.host", "${BDD_TEST_HOST}"}, + } + body, err := RenderYAMLFromKeys(keys) + if err != nil { + t.Fatalf("render: %v", err) + } + if !strings.Contains(string(body), "host: region-b.example.invalid") { + t.Fatalf("interpolation failed:\n%s", body) + } +} + func TestParsePathInvalidShapes(t *testing.T) { bads := []string{ "a..b", diff --git a/tests/bdd/features/multi-cluster-helmfile-llm-registration-multiregion.feature b/tests/bdd/features/multi-cluster-helmfile-llm-registration-multiregion.feature index 9ce4e278b6..92d45c3043 100644 --- a/tests/bdd/features/multi-cluster-helmfile-llm-registration-multiregion.feature +++ b/tests/bdd/features/multi-cluster-helmfile-llm-registration-multiregion.feature @@ -52,28 +52,152 @@ Feature: Register an LLM worker securely with routers in two local regions @llm-registration-multiregion-install Scenario: Operator installs two secure regions with distinct router workload identities - When I run command "make -C deploy/stacks/self-managed template HELMFILE_ENV=local-bdd-registration-multiregion" - Then the command exit code should be 0 - And the rendered manifests in "deploy/stacks/self-managed/out" should contain: - | text | - | kind: Deployment | + # Region B: visible override values for a second LLM request router + # with a distinct StatefulSet identity. + Given I write yaml file "tests/bdd/out/region-b-values.yaml" with values: + | llmRequestRouter.fullnameOverride | llm-request-router-region-b | + | llmRequestRouter.replicaCount | 2 | + | llmRequestRouter.workload.kind | StatefulSet | + | llmRequestRouter.service.headlessName | llm-request-router-region-b-headless | + | llmRequestRouter.kubernetes.advertisedHostnameTemplate | {pod_name}.llm-request-router-region-b-headless.nvcf.svc.cluster.local | + | llmRequestRouter.discovery.remoteWatchUrls | [] | + | llmRequestRouter.backendRouter.enabled | true | + | llmRequestRouter.backendRouter.pylonGrpcDialAddress | https://region-b-watch.nvcf.svc.cluster.local:50071 | + | llmRequestRouter.backendRouter.pylonReverseTunnelDialAddress | region-b-watch.nvcf.svc.cluster.local:50072 | + | llmRequestRouter.serviceAccount.create | false | + | llmRequestRouter.serviceAccount.name | llm-request-router | + | llmRequestRouter.pki.enabled | false | + | llmRequestRouter.certificate.enabled | false | + | llmRequestRouter.tls.mode | existingSecret | + | llmRequestRouter.tls.secretName | stargate-quic-tls | + # Region B gateway resources: GRPCRoute, BackendTrafficPolicy, and + # cross-namespace ReferenceGrant. + And Kubernetes manifest "region-b-gateway" is: + """yaml + apiVersion: gateway.networking.k8s.io/v1 + kind: GRPCRoute + metadata: + name: llm-worker-region-b-grpc + namespace: envoy-gateway-system + spec: + parentRefs: + - name: grpc-gw + namespace: envoy-gateway-system + sectionName: llm-grpc + hostnames: + - "region-b-watch.nvcf.svc.cluster.local" + - "*.llm-request-router-region-b-headless.nvcf.svc.cluster.local" + rules: + - backendRefs: + - name: llm-request-router-region-b-backend-router + namespace: nvcf + port: 50071 + --- + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + name: llm-worker-region-b-grpc-streams + namespace: envoy-gateway-system + spec: + targetRefs: + - group: gateway.networking.k8s.io + kind: GRPCRoute + name: llm-worker-region-b-grpc + timeout: + http: + requestTimeout: 0s + --- + apiVersion: gateway.networking.k8s.io/v1beta1 + kind: ReferenceGrant + metadata: + name: allow-llm-worker-region-b-route + namespace: nvcf + spec: + from: + - group: gateway.networking.k8s.io + kind: GRPCRoute + namespace: envoy-gateway-system + to: + - group: "" + kind: Service + name: llm-request-router-region-b-backend-router + """ + # The region-b-watch Service and Endpoints alias lets each cluster + # reach the Region B backend-router by name. ${CONTROL_PLANE_IP} is + # exported later in the scenario and interpolated when the manifest + # is applied. + And Kubernetes manifest "region-b-watch" is: + """yaml + apiVersion: v1 + kind: Service + metadata: + name: region-b-watch + namespace: nvcf + spec: + ports: + - name: llm-grpc + port: 50071 + targetPort: llm-grpc + protocol: TCP + - name: llm-quic + port: 50072 + targetPort: llm-quic + protocol: UDP + --- + apiVersion: v1 + kind: Endpoints + metadata: + name: region-b-watch + namespace: nvcf + subsets: + - addresses: + - ip: ${CONTROL_PLANE_IP} + ports: + - name: llm-grpc + port: 50071 + protocol: TCP + - name: llm-quic + port: 50072 + protocol: UDP + """ + When I successfully run command "make -C deploy/stacks/self-managed template HELMFILE_ENV=local-bdd-registration-multiregion" + Then the rendered manifests in "deploy/stacks/self-managed/out" should contain: + | text | + | kind: Deployment | | --remote-stargate-url=https://region-b-watch.nvcf.svc.cluster.local:50071 | - When I run command "make -C deploy/stacks/self-managed install HELMFILE_ENV=local-bdd-registration-multiregion" - Then the command exit code should be 0 - When I run command "kubectl --context k3d-ncp-local-cp wait certificate llm-request-router-grpc-tls -n envoy-gateway-system --for=condition=Ready --timeout=5m" - Then the command exit code should be 0 - When I run command "kubectl --context k3d-ncp-local-cp get certificate llm-request-router-grpc-tls -n envoy-gateway-system -o jsonpath={.spec.dnsNames}" - Then the command exit code should be 0 - And the command output should contain "region-b-watch.nvcf.svc.cluster.local" - When I run command "kubectl --context k3d-ncp-local-cp rollout status deployment/llm-request-router -n nvcf --timeout=10m" - Then the command exit code should be 0 + When I successfully run command "make -C deploy/stacks/self-managed install HELMFILE_ENV=local-bdd-registration-multiregion" + And I successfully run command "kubectl --context k3d-ncp-local-cp wait certificate llm-request-router-grpc-tls -n envoy-gateway-system --for=condition=Ready --timeout=5m" + And I successfully run command "kubectl --context k3d-ncp-local-cp get certificate llm-request-router-grpc-tls -n envoy-gateway-system -o jsonpath={.spec.dnsNames}" + Then the command output should contain "region-b-watch.nvcf.svc.cluster.local" + And these Kubernetes workloads should complete rollout using context "k3d-ncp-local-cp" within "10m": + | kind | name | namespace | + | Deployment | llm-request-router | nvcf | - # This script still hides the Region B release values, routes, alias - # endpoints, and rollout waits. Replacing it with visible DSL steps is - # tracked in https://github.com/NVIDIA/nvcf/issues/1391. - When I run command "tests/bdd/scripts/install-llm-region-b.sh" - Then the command exit code should be 0 + # Region B: export the Region A base values so Region B inherits image + # tags and shared config, install Region B with the visible overrides, + # then wire the gateway route and the cross-cluster watch alias. + When I successfully run command: + """ + /bin/bash -c 'helm --kube-context k3d-ncp-local-cp get values llm-request-router --namespace nvcf --output json | jq "{llmRequestRouter: .llmRequestRouter}" > ${REPO_ROOT}/tests/bdd/out/region-a-base-values.json' + """ + And I successfully run command: + """ + helm --kube-context k3d-ncp-local-cp upgrade --install llm-request-router-region-b ${REPO_ROOT}/deploy/helm/llm-request-router/llm-request-router --namespace nvcf --values ${REPO_ROOT}/tests/bdd/out/region-a-base-values.json --values ${REPO_ROOT}/tests/bdd/out/region-b-values.yaml --wait --timeout 10m + """ + And I successfully apply Kubernetes manifest "region-b-gateway" using contexts: + | context | + | k3d-ncp-local-cp | + And I successfully run command "kubectl --context k3d-ncp-local-compute-1 get endpoints llm-request-router --namespace nvcf --output jsonpath={.subsets[0].addresses[0].ip}" + And I export command output to environment variable "CONTROL_PLANE_IP" + And I successfully apply Kubernetes manifest "region-b-watch" using contexts: + | context | + | k3d-ncp-local-cp | + | k3d-ncp-local-compute-1 | + Then these Kubernetes workloads should complete rollout using context "k3d-ncp-local-cp" within "10m": + | kind | name | namespace | + | StatefulSet | llm-request-router-region-b | nvcf | + | Deployment | llm-request-router-region-b-backend-router | nvcf | # The initial region advertises an explicit HTTPS recursive seed while # retaining every concrete Deployment pod identity. Three distinct @@ -94,37 +218,33 @@ Feature: Register an LLM worker securely with routers in two local regions And the command output should have exactly "2" distinct matches of "llm-request-router-region-b-[0-9]+" And the command output should not match "([0-9]{1,3}-){3}[0-9]{1,3}\." - When I run command: + When I successfully run command: """ ${NVCF_CLI} --config ${REPO_ROOT}/tests/bdd/fixtures/nvcf-cli-local.yaml self-hosted --control-plane-stack deploy/stacks/self-managed --env local-bdd-registration-multiregion --control-plane-context k3d-ncp-local-cp --compute-plane-context k3d-ncp-local-compute-1 control-plane profile export --cluster-name ncp-local-cp """ - Then the command exit code should be 0 - And file "deploy/stacks/self-managed/out/control-plane-profile.yaml" should exist + Then file "deploy/stacks/self-managed/out/control-plane-profile.yaml" should exist And yaml file "deploy/stacks/self-managed/out/control-plane-profile.yaml" should have non-empty keys: | key | | managementTls.caBundlePem | | transportTls.trustBundleFingerprint | | transportTls.trustBundlePem | - And command has succeeded: + When command has succeeded: """ /bin/sh -c '${NVCF_CLI} --config ${REPO_ROOT}/tests/bdd/fixtures/nvcf-cli-local.yaml init >/dev/null' """ - When I run command "kubectl config use-context k3d-ncp-local-compute-1" - Then the command exit code should be 0 - When I run command: + And I successfully run command "kubectl config use-context k3d-ncp-local-compute-1" + And I successfully run command: """ make -C deploy/stacks/nvcf-compute-plane register-cluster CLUSTER_NAME=ncp-local-compute-1 CONTROL_PLANE_PROFILE=${REPO_ROOT}/deploy/stacks/self-managed/out/control-plane-profile.yaml COMPUTE_KUBE_CONTEXT=k3d-ncp-local-compute-1 NVCF_CLI=${NVCF_CLI} NVCF_CLI_CONFIG=${REPO_ROOT}/tests/bdd/fixtures/nvcf-cli-local.yaml """ - Then the command exit code should be 0 And the "nvcr-pull-secret" image pull secret exists in namespaces: | nvca-operator | - When I run command: + And I successfully run command: """ make -C deploy/stacks/nvcf-compute-plane install CLUSTER_NAME=ncp-local-compute-1 HELMFILE_ENV=local-bdd-registration-multiregion COMPUTE_KUBE_CONTEXT=k3d-ncp-local-compute-1 NVCF_CLI=${NVCF_CLI} """ - Then the command exit code should be 0 - And NVCFBackend "ncp-local-compute-1" in namespace "nvca-operator" using context "k3d-ncp-local-compute-1" should report agent status "healthy" within "10m" + Then NVCFBackend "ncp-local-compute-1" in namespace "nvca-operator" using context "k3d-ncp-local-compute-1" should report agent status "healthy" within "10m" @llm-registration-multiregion-runtime Scenario: Pylon recursively registers with both regions and serves an authenticated request diff --git a/tests/bdd/godog_test.go b/tests/bdd/godog_test.go index 220e391c08..0460bdb671 100644 --- a/tests/bdd/godog_test.go +++ b/tests/bdd/godog_test.go @@ -1475,6 +1475,8 @@ func TestMultiClusterHelmfileLLMRegistrationMultiregionFeatureFileWiresToSteps(t " pylon_reverse_tunnel_connected 'at least' 3" grpcCertificateCommand = "kubectl --context k3d-ncp-local-cp get certificate llm-request-router-grpc-tls" + " -n envoy-gateway-system -o jsonpath={.spec.dnsNames}" + endpointDiscoveryCommand = "kubectl --context k3d-ncp-local-compute-1 get endpoints llm-request-router" + + " --namespace nvcf --output jsonpath={.subsets[0].addresses[0].ip}" invokeCommand = "/usr/bin/nvcf-cli --config /repo-root-placeholder/tests/bdd/fixtures/nvcf-cli-local.yaml function invoke" + " --inference-url /v1/chat/completions --model-name openai-compatible-sample" + " --request-body '{\"messages\":[{\"role\":\"user\",\"content\":\"bdd-registration-multiregion\"}]}' --timeout 120" @@ -1500,6 +1502,10 @@ func TestMultiClusterHelmfileLLMRegistrationMultiregionFeatureFileWiresToSteps(t ExitCode: 0, Stdout: "[llm-request-router.nvcf.svc.cluster.local region-b-watch.nvcf.svc.cluster.local]", }, + endpointDiscoveryCommand: { + ExitCode: 0, + Stdout: "192.0.2.10", + }, pylonMetricsCommand: { ExitCode: 0, Stdout: "pylon_registration_stream_connected=5\n" + @@ -1546,6 +1552,7 @@ func TestMultiClusterHelmfileLLMRegistrationMultiregionFeatureFileWiresToSteps(t } for _, command := range []string{ grpcCertificateCommand, + endpointDiscoveryCommand, regionAWatchCommand, regionBWatchCommand, pylonMetricsCommand, @@ -1554,8 +1561,103 @@ func TestMultiClusterHelmfileLLMRegistrationMultiregionFeatureFileWiresToSteps(t t.Fatalf("exact multi-region observation command was not invoked: %s", command) } } - if !commandRanThatContainsAll( - suite.Runner.(*fakeRunner).runs, + runs := suite.Runner.(*fakeRunner).runs + + // Region B base values export (the jq pipeline that captures Region A config). + if !commandRanThatContainsAll(runs, + "helm --kube-context k3d-ncp-local-cp get values llm-request-router", + "region-a-base-values.json", + ) { + t.Fatal("Region A base values export was not invoked") + } + + // Region B helm install references both the base values and the + // DSL-generated override file. + if !commandRanThatContainsAll(runs, + "upgrade --install llm-request-router-region-b", + "region-a-base-values.json", + "region-b-values.yaml", + ) { + t.Fatal("Region B helm install was not invoked with both base and override values") + } + + // The override file was written by the I write yaml file step and + // should contain the visible table values. + regionBValuesPath := filepath.Join(suite.Config.RepoRoot, "tests", "bdd", "out", "region-b-values.yaml") + regionBValues, err := os.ReadFile(regionBValuesPath) + if err != nil { + t.Fatalf("read region-b-values.yaml: %v", err) + } + for _, want := range []string{ + "fullnameOverride: llm-request-router-region-b", + "kind: StatefulSet", + "headlessName: llm-request-router-region-b-headless", + "pylonGrpcDialAddress: https://region-b-watch.nvcf.svc.cluster.local:50071", + "mode: existingSecret", + "secretName: stargate-quic-tls", + } { + if !strings.Contains(string(regionBValues), want) { + t.Fatalf("region-b-values.yaml missing %q:\n%s", want, regionBValues) + } + } + // Boolean and collection values must be emitted as native YAML + // types so Helm evaluates them correctly (string "false" is truthy + // in Go templates). MatchYAMLSubtree compares parsed values, so a + // missing key, a quoted "false", or a "[]" string all fail here. + for _, typed := range []struct { + key string + want string + }{ + {key: "llmRequestRouter.backendRouter.enabled", want: "true"}, + {key: "llmRequestRouter.serviceAccount.create", want: "false"}, + {key: "llmRequestRouter.pki.enabled", want: "false"}, + {key: "llmRequestRouter.certificate.enabled", want: "false"}, + {key: "llmRequestRouter.discovery.remoteWatchUrls", want: "[]"}, + } { + if err := dsl.MatchYAMLSubtree(regionBValuesPath, typed.key, typed.want, dsl.MatchExact); err != nil { + t.Fatalf("region-b-values.yaml %s is not the native value %s: %v\n%s", typed.key, typed.want, err, regionBValues) + } + } + + // Gateway resources (GRPCRoute, BackendTrafficPolicy, ReferenceGrant + // visible in the feature file docstring) are applied from a rendered + // manifest file under OutDir against the control-plane context. + if !commandRanThatContainsAll(appliedManifestBodies(t, runs, "k3d-ncp-local-cp"), + "kind: GRPCRoute", + "kind: BackendTrafficPolicy", + "kind: ReferenceGrant", + ) { + t.Fatal("Region B gateway resources were not applied to the control-plane cluster") + } + + // Watch alias applied to both clusters with the DSL-interpolated + // control-plane IP (no envsubst dependency). Each apply must carry + // both the Service and the Endpoints resource; a name and IP alone + // would also match an unrelated resource. + for _, cluster := range []struct { + context string + label string + }{ + {context: "k3d-ncp-local-cp", label: "control-plane"}, + {context: "k3d-ncp-local-compute-1", label: "compute"}, + } { + if !commandRanThatContainsAll(appliedManifestBodies(t, runs, cluster.context), + "kind: Service", + "kind: Endpoints", + "name: region-b-watch", + "ip: 192.0.2.10", + ) { + t.Fatalf("Region B watch alias Service and Endpoints were not applied to the %s cluster", cluster.label) + } + } + + if !commandRanThatContains(runs, "rollout status statefulset/llm-request-router-region-b") { + t.Fatal("Region B StatefulSet rollout wait was not invoked") + } + if !commandRanThatContains(runs, "rollout status deployment/llm-request-router-region-b-backend-router") { + t.Fatal("Region B backend-router Deployment rollout wait was not invoked") + } + if !commandRanThatContainsAll(runs, "function create --name bdd-registration-multiregion", "--function-type LLM", "--llm-model", @@ -2645,6 +2747,9 @@ func commandRanExactly(runs []string, want string) bool { return false } +// commandRanThatContainsAll reports whether a single recorded command +// contains every needle. Matching within one command, rather than across +// the whole recording, ties related arguments to the same invocation. func commandRanThatContainsAll(runs []string, needles ...string) bool { for _, run := range runs { matched := true @@ -2661,6 +2766,30 @@ func commandRanThatContainsAll(runs []string, needles ...string) bool { return false } +// appliedManifestBodies returns the contents of every manifest file that an +// explicit-context kubectl apply consumed against kubeContext. Manifest steps +// write their rendered body under OutDir and pass the path on argv, so the +// recorded command alone does not show the applied resources. +func appliedManifestBodies(t *testing.T, runs []string, kubeContext string) []string { + t.Helper() + prefix := "kubectl --context " + kubeContext + " apply -f " + var bodies []string + for _, run := range runs { + if !strings.HasPrefix(run, prefix) { + continue + } + path := strings.Trim(strings.TrimPrefix(run, prefix), "'") + body, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read applied manifest %s: %v", path, err) + } + bodies = append(bodies, string(body)) + } + return bodies +} + +// assertFileContains fails the test unless the file at path contains every +// needle. It is used to check handoff artifacts written by file steps. func assertFileContains(t *testing.T, path string, needles ...string) { t.Helper() content, err := os.ReadFile(path) diff --git a/tests/bdd/region_b_script_test.go b/tests/bdd/region_b_script_test.go deleted file mode 100644 index 23b6e30440..0000000000 --- a/tests/bdd/region_b_script_test.go +++ /dev/null @@ -1,101 +0,0 @@ -/* -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 bdd_tmp - -import ( - "os" - "os/exec" - "path/filepath" - "strings" - "testing" -) - -func TestInstallLLMRegionBCreatesWatchAliasInBothClusters(t *testing.T) { - binDir := t.TempDir() - applyDir := t.TempDir() - - helmScript := `#!/usr/bin/env bash -set -euo pipefail -case " $* " in - *" get values "*) printf '{"llmRequestRouter":{}}\n' ;; - *) cat >/dev/null ;; -esac -` - kubectlScript := `#!/usr/bin/env bash -set -euo pipefail -context="" -previous="" -for argument in "$@"; do - if [[ "${previous}" == "--context" ]]; then - context="${argument}" - fi - previous="${argument}" -done -case " $* " in - *" get endpoints llm-request-router "*) printf '192.0.2.10' ;; - *" apply -f - "*) - cat >>"${FAKE_APPLY_DIR}/${context}.yaml" - printf '\n---\n' >>"${FAKE_APPLY_DIR}/${context}.yaml" - ;; -esac -` - jqScript := `#!/usr/bin/env bash -set -euo pipefail -cat -` - for name, body := range map[string]string{ - "helm": helmScript, - "jq": jqScript, - "kubectl": kubectlScript, - } { - if err := os.WriteFile(filepath.Join(binDir, name), []byte(body), 0o755); err != nil { - t.Fatalf("write fake %s: %v", name, err) - } - } - - cmd := exec.Command("bash", "scripts/install-llm-region-b.sh") - cmd.Env = append(os.Environ(), - "CONTROL_CONTEXT=bdd-control", - "COMPUTE_CONTEXT=bdd-compute", - "FAKE_APPLY_DIR="+applyDir, - "PATH="+binDir+":"+os.Getenv("PATH"), - "REPO_ROOT="+t.TempDir(), - ) - if output, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("install region B: %v\n%s", err, output) - } - - for _, context := range []string{"bdd-control", "bdd-compute"} { - manifestPath := filepath.Join(applyDir, context+".yaml") - manifest, err := os.ReadFile(manifestPath) - if err != nil { - t.Fatalf("read %s aliases: %v", context, err) - } - for _, want := range []string{ - "kind: Service\nmetadata:\n name: region-b-watch", - "kind: Endpoints\nmetadata:\n name: region-b-watch", - "- ip: 192.0.2.10", - "name: llm-grpc", - "name: llm-quic", - } { - if !strings.Contains(string(manifest), want) { - t.Fatalf("%s aliases missing %q:\n%s", context, want, manifest) - } - } - } -} diff --git a/tests/bdd/scripts/install-llm-region-b.sh b/tests/bdd/scripts/install-llm-region-b.sh deleted file mode 100755 index 77fcd7e7b7..0000000000 --- a/tests/bdd/scripts/install-llm-region-b.sh +++ /dev/null @@ -1,138 +0,0 @@ -#!/usr/bin/env bash -# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -set -euo pipefail - -control_context="${CONTROL_CONTEXT:-k3d-ncp-local-cp}" -compute_context="${COMPUTE_CONTEXT:-k3d-ncp-local-compute-1}" -namespace="nvcf" -region_b_release="llm-request-router-region-b" -region_b_watch_host="region-b-watch.nvcf.svc.cluster.local" -region_b_headless_host="*.llm-request-router-region-b-headless.nvcf.svc.cluster.local" -chart="${REPO_ROOT:?REPO_ROOT is required}/deploy/helm/llm-request-router/llm-request-router" - -values_json="$(helm --kube-context "${control_context}" get values llm-request-router \ - --namespace "${namespace}" --output json)" - -printf '%s' "${values_json}" | jq --arg watch_host "${region_b_watch_host}" ' - { - llmRequestRouter: ( - .llmRequestRouter - | .fullnameOverride = "llm-request-router-region-b" - | .replicaCount = 2 - | .workload.kind = "StatefulSet" - | .service.headlessName = "llm-request-router-region-b-headless" - | .kubernetes.advertisedHostnameTemplate = "{pod_name}.llm-request-router-region-b-headless.nvcf.svc.cluster.local" - | .discovery.remoteWatchUrls = [] - | .backendRouter.enabled = true - | .backendRouter.pylonGrpcDialAddress = ("https://" + $watch_host + ":50071") - | .backendRouter.pylonReverseTunnelDialAddress = ($watch_host + ":50072") - | .serviceAccount.create = false - | .serviceAccount.name = "llm-request-router" - | .pki.enabled = false - | .certificate.enabled = false - | .tls.mode = "existingSecret" - | .tls.secretName = "stargate-quic-tls" - ) - } -' | helm --kube-context "${control_context}" upgrade --install "${region_b_release}" "${chart}" \ - --namespace "${namespace}" --values - --wait --timeout 10m - -kubectl --context "${control_context}" apply -f - <&2 - exit 1 -fi - -for alias_context in "${control_context}" "${compute_context}"; do - kubectl --context "${alias_context}" apply -f - <>> %s\n", st.Text) + if arg := st.Argument; arg != nil { + if dt := arg.DataTable; dt != nil { + for _, row := range dt.Rows { + cells := make([]string, len(row.Cells)) + for i, c := range row.Cells { + cells[i] = c.Value + } + fmt.Fprintf(os.Stderr, ">>> | %s |\n", strings.Join(cells, " | ")) + } + } + if ds := arg.DocString; ds != nil { + for _, line := range strings.Split(ds.Content, "\n") { + fmt.Fprintf(os.Stderr, ">>> %s\n", line) + } + } + } return c, nil }) registerFileSteps(ctx, sc) + registerManifestSteps(ctx, sc) registerCommandSteps(ctx, sc) registerNVCFCLISteps(ctx, sc) registerRegistrationSteps(ctx, sc) diff --git a/tests/bdd/steps/file_steps.go b/tests/bdd/steps/file_steps.go index 9903b43dfd..e9c64f59c3 100644 --- a/tests/bdd/steps/file_steps.go +++ b/tests/bdd/steps/file_steps.go @@ -33,6 +33,7 @@ import ( // to the Ledger before its first write so suite teardown can restore. func registerFileSteps(ctx *godog.ScenarioContext, sc *ScenarioContext) { ctx.Step(`^I copy the file "([^"]*)" to "([^"]*)"$`, sc.iCopyFile) + ctx.Step(`^I write yaml file "([^"]*)" with values:$`, sc.iWriteYAMLFile) ctx.Step(`^I update yaml file "([^"]*)" with keys:$`, sc.iUpdateYAMLFile) ctx.Step(`^I prepare Helmfile environment "([^"]*)" for stack "([^"]*)" from fixture "([^"]*)" with values:$`, sc.iPrepareHelmfileEnvironment) ctx.Step(`^I prepare self-managed secrets file "([^"]*)" from template "([^"]*)" using the current NGC registry credential$`, sc.iPrepareSelfManagedSecretsFile) @@ -81,6 +82,31 @@ func (sc *ScenarioContext) iCopyFile(src, dest string) error { return copyFile(resolvedSrc, resolvedDest) } +// iWriteYAMLFile creates a new YAML file from the supplied table of +// dotted-path/value rows. The step refuses to overwrite an existing +// file; use I update yaml file for that. The destination is recorded +// with the Ledger before the write so suite teardown removes it. YAML +// construction stays in dsl.RenderYAMLFromKeys; this handler owns only +// path resolution, the existence check, and the write. +func (sc *ScenarioContext) iWriteYAMLFile(path string, table *godog.Table) error { + resolved := sc.resolvePath(dsl.Interpolate(path)) + if _, err := os.Stat(resolved); err == nil { + return fmt.Errorf("write yaml %s: file already exists (use I update yaml file to modify)", resolved) + } + keys, err := tableToKeyValuePairs(table) + if err != nil { + return err + } + body, err := dsl.RenderYAMLFromKeys(keys) + if err != nil { + return fmt.Errorf("write yaml %s: %w", resolved, err) + } + if err := sc.Suite.Ledger.Snapshot(resolved); err != nil { + return err + } + return writeNewFile(resolved, body) +} + // iUpdateYAMLFile applies the supplied table of dotted-path/value rows // to path. Path keys do not interpolate; value cells do. func (sc *ScenarioContext) iUpdateYAMLFile(path string, table *godog.Table) error { @@ -212,6 +238,28 @@ func copyFile(src, dest string) error { return nil } +// writeNewFile creates dest with mode 0644, creating parent directories +// as needed. O_EXCL guarantees the write never clobbers a file that +// appeared between the caller's existence check and this call, and the +// Close error is checked so a flush failure surfaces. +func writeNewFile(dest string, body []byte) error { + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + return fmt.Errorf("mkdir %s: %w", filepath.Dir(dest), err) + } + out, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) + if err != nil { + return fmt.Errorf("create %s: %w", dest, err) + } + if _, err := out.Write(body); err != nil { + _ = out.Close() + return fmt.Errorf("write %s: %w", dest, err) + } + if err := out.Close(); err != nil { + return fmt.Errorf("close %s: %w", dest, err) + } + return nil +} + // tableToKeyValuePairs converts a two-column Godog table into the // (path, value) slice that dsl.UpdateYAMLKeys consumes. func tableToKeyValuePairs(table *godog.Table) ([][2]string, error) { diff --git a/tests/bdd/steps/infra_steps.go b/tests/bdd/steps/infra_steps.go index 69ca158b21..b3cf7092fa 100644 --- a/tests/bdd/steps/infra_steps.go +++ b/tests/bdd/steps/infra_steps.go @@ -191,29 +191,13 @@ func (sc *ScenarioContext) pullSecretInNamespacesAtContext( return nil } -// applyManifest writes body to a temp file inside the run's OutDir and -// runs kubectl apply against it. Routing through OutDir (rather than -// /tmp) means failed runs leave the artifacts in the run directory -// alongside command logs for post-mortem inspection. +// applyManifest writes body to a file inside the run's OutDir and runs +// kubectl apply against it. The pull-secret Given is cached and does not +// record its result, so it bypasses the scenario's LastResult bookkeeping. func (sc *ScenarioContext) applyManifest(ctx context.Context, body []byte, kubeContext string) error { - dir := sc.Suite.Config.OutDir - if dir == "" { - dir = os.TempDir() - } - if err := os.MkdirAll(dir, 0o755); err != nil { - return fmt.Errorf("ensure manifest dir: %w", err) - } - file, err := os.CreateTemp(dir, "manifest-*.yaml") + path, err := sc.writeManifestFile("pull-secret", body) if err != nil { - return fmt.Errorf("create manifest file: %w", err) - } - path := file.Name() - if _, err := file.Write(body); err != nil { - _ = file.Close() - return fmt.Errorf("write manifest: %w", err) - } - if err := file.Close(); err != nil { - return fmt.Errorf("close manifest: %w", err) + return err } command, err := dsl.KubectlApplyCommand(path, kubeContext) if err != nil { diff --git a/tests/bdd/steps/manifest_steps.go b/tests/bdd/steps/manifest_steps.go new file mode 100644 index 0000000000..5968d39452 --- /dev/null +++ b/tests/bdd/steps/manifest_steps.go @@ -0,0 +1,132 @@ +/* +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 steps + +import ( + "context" + "fmt" + "os" + "strings" + + "github.com/cucumber/godog" + + "nvcf-bdd/dsl" +) + +// registerManifestSteps hooks the named Kubernetes manifest steps. The +// Given stores the visible docstring under a scenario-scoped name; the When +// interpolates it, writes it under the run's OutDir, and applies it once per +// listed context. The pair hides only the repeated kubectl apply mechanics. +func registerManifestSteps(ctx *godog.ScenarioContext, sc *ScenarioContext) { + ctx.Step(`^Kubernetes manifest "([^"]*)" is:$`, sc.kubernetesManifestIs) + ctx.Step(`^I successfully apply Kubernetes manifest "([^"]*)" using contexts:$`, sc.iSuccessfullyApplyKubernetesManifest) +} + +// kubernetesManifestIs records the raw docstring. Interpolation is deferred +// to apply time so the manifest may reference an env var exported by a +// later step. Redeclaring a name in the same scenario is an error so two +// docstrings cannot silently compete for one apply. +func (sc *ScenarioContext) kubernetesManifestIs(name string, doc *godog.DocString) error { + name = strings.TrimSpace(name) + if name == "" { + return fmt.Errorf("kubernetes manifest name is empty") + } + if doc == nil || strings.TrimSpace(doc.Content) == "" { + return fmt.Errorf("kubernetes manifest %q has an empty body", name) + } + if _, exists := sc.Manifests[name]; exists { + return fmt.Errorf("kubernetes manifest %q is already declared in this scenario", name) + } + if sc.Manifests == nil { + sc.Manifests = map[string]string{} + } + sc.Manifests[name] = doc.Content + return nil +} + +// iSuccessfullyApplyKubernetesManifest writes the interpolated manifest once +// and runs one explicit-context kubectl apply per context row. Each apply +// must exit 0; the failure names the row, manifest, and context. +func (sc *ScenarioContext) iSuccessfullyApplyKubernetesManifest(ctx context.Context, name string, table *godog.Table) error { + name = strings.TrimSpace(name) + body, declared := sc.Manifests[name] + if !declared { + return fmt.Errorf("kubernetes manifest %q is not declared in this scenario", name) + } + contexts, err := tableToSingleColumn(table, "context") + if err != nil { + return err + } + path, err := sc.writeManifestFile(name, []byte(dsl.Interpolate(body))) + if err != nil { + return fmt.Errorf("kubernetes manifest %q: %w", name, err) + } + for index, kubeContext := range contexts { + command, err := dsl.KubectlApplyCommand(path, kubeContext) + if err != nil { + return fmt.Errorf("row %d: kubernetes manifest %q: %w", index+1, name, err) + } + if err := sc.runResolvedSuccessfully(ctx, command); err != nil { + return fmt.Errorf( + "row %d: kubernetes manifest %q was not applied using context %q: %w", + index+1, name, strings.TrimSpace(dsl.Interpolate(kubeContext)), err, + ) + } + } + return nil +} + +// writeManifestFile writes body to a new file inside the run's OutDir and +// returns its path. Routing through OutDir (rather than /tmp) means failed +// runs leave the artifacts in the run directory alongside command logs for +// post-mortem inspection. The label becomes part of the file name so an +// operator can match a manifest file to the step that produced it. +func (sc *ScenarioContext) writeManifestFile(label string, body []byte) (string, error) { + dir := sc.Suite.Config.OutDir + if dir == "" { + dir = os.TempDir() + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return "", fmt.Errorf("ensure manifest dir: %w", err) + } + file, err := os.CreateTemp(dir, "manifest-"+manifestFileLabel(label)+"-*.yaml") + if err != nil { + return "", fmt.Errorf("create manifest file: %w", err) + } + path := file.Name() + if _, err := file.Write(body); err != nil { + _ = file.Close() + return "", fmt.Errorf("write manifest: %w", err) + } + if err := file.Close(); err != nil { + return "", fmt.Errorf("close manifest: %w", err) + } + return path, nil +} + +// manifestFileLabel keeps only characters that are safe in a file name. +func manifestFileLabel(label string) string { + return strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '.', r == '_', r == '-': + return r + default: + return '-' + } + }, label) +} diff --git a/tests/bdd/steps/steps_test.go b/tests/bdd/steps/steps_test.go index 6e53686480..9db11374fc 100644 --- a/tests/bdd/steps/steps_test.go +++ b/tests/bdd/steps/steps_test.go @@ -365,6 +365,161 @@ func TestIUpdateYAMLFileWritesKeys(t *testing.T) { } } +// TestIWriteYAMLFileCreatesAndRestores verifies that the write-yaml +// step creates the file and that Suite.Teardown removes it. +func TestIWriteYAMLFileCreatesAndRestores(t *testing.T) { + sc, _ := newScenarioContext(t) + rel := "out/region-b-values.yaml" + abs := filepath.Join(sc.Suite.Config.RepoRoot, rel) + + table := docTable(t, [][]string{ + {"llmRequestRouter.fullnameOverride", "llm-request-router-region-b"}, + {"llmRequestRouter.replicaCount", "2"}, + }) + if err := sc.iWriteYAMLFile(rel, table); err != nil { + t.Fatalf("write: %v", err) + } + got, err := os.ReadFile(abs) + if err != nil { + t.Fatalf("read: %v", err) + } + if !strings.Contains(string(got), "fullnameOverride: llm-request-router-region-b") { + t.Fatalf("missing key:\n%s", got) + } + + if err := sc.Suite.Teardown(); err != nil { + t.Fatalf("teardown: %v", err) + } + if _, err := os.Stat(abs); err == nil { + t.Fatal("file should be removed after restore") + } +} + +// TestIWriteYAMLFileRejectsExistingFile confirms that the write-yaml +// step refuses to overwrite an existing file. +func TestIWriteYAMLFileRejectsExistingFile(t *testing.T) { + sc, _ := newScenarioContext(t) + rel := "existing.yaml" + abs := filepath.Join(sc.Suite.Config.RepoRoot, rel) + if err := os.WriteFile(abs, []byte("key: value\n"), 0o644); err != nil { + t.Fatalf("seed: %v", err) + } + + table := docTable(t, [][]string{{"key", "new"}}) + err := sc.iWriteYAMLFile(rel, table) + if err == nil || !strings.Contains(err.Error(), "already exists") { + t.Fatalf("err = %v, want already-exists error", err) + } +} + +// TestKubernetesManifestAppliesInterpolatedBodyPerContext verifies that the +// Given stores the raw docstring, that ${VAR} expands at apply time rather +// than declaration time, and that one explicit-context apply runs per row +// against the same rendered file under OutDir. +func TestKubernetesManifestAppliesInterpolatedBodyPerContext(t *testing.T) { + sc, fake := newScenarioContext(t) + sc.Suite.Config.OutDir = filepath.Join(sc.Suite.Config.RepoRoot, "out", "run") + fake.result = harness.Result{ExitCode: 0} + doc := &godog.DocString{Content: "apiVersion: v1\nkind: Endpoints\nsubsets:\n - addresses:\n - ip: ${BDD_ALIAS_IP}\n"} + if err := sc.kubernetesManifestIs("region-b-watch", doc); err != nil { + t.Fatalf("declare: %v", err) + } + // Exported after the declaration, as the feature does with CONTROL_PLANE_IP. + t.Setenv("BDD_ALIAS_IP", "192.0.2.10") + + table := docTable(t, [][]string{{"context"}, {"k3d-ncp-local-cp"}, {"k3d-ncp-local-compute-1"}}) + if err := sc.iSuccessfullyApplyKubernetesManifest(context.Background(), "region-b-watch", table); err != nil { + t.Fatalf("apply: %v", err) + } + if len(fake.runs) != 2 { + t.Fatalf("runs = %d, want 2", len(fake.runs)) + } + var path string + for index, wantContext := range []string{"k3d-ncp-local-cp", "k3d-ncp-local-compute-1"} { + prefix := "kubectl --context " + wantContext + " apply -f " + if !strings.HasPrefix(fake.runs[index].command, prefix) { + t.Fatalf("command %d = %q, want prefix %q", index+1, fake.runs[index].command, prefix) + } + got := strings.TrimPrefix(fake.runs[index].command, prefix) + if path == "" { + path = got + } else if got != path { + t.Fatalf("second apply used %q, want the same file %q", got, path) + } + } + if !strings.HasPrefix(path, sc.Suite.Config.OutDir) { + t.Fatalf("manifest %q was not written under OutDir %q", path, sc.Suite.Config.OutDir) + } + body, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read manifest: %v", err) + } + if !strings.Contains(string(body), "ip: 192.0.2.10") { + t.Fatalf("manifest body was not interpolated at apply time:\n%s", body) + } + if sc.LastResult.ExitCode != 0 || sc.LastCommand != fake.runs[1].command { + t.Fatalf("last result not recorded: %+v %q", sc.LastResult, sc.LastCommand) + } +} + +// TestKubernetesManifestIsRejectsEmptyAndDuplicateDeclarations confirms +// that the manifest Given rejects an empty name, a blank body, and a +// second declaration of the same name within one scenario. +func TestKubernetesManifestIsRejectsEmptyAndDuplicateDeclarations(t *testing.T) { + sc, _ := newScenarioContext(t) + if err := sc.kubernetesManifestIs("", &godog.DocString{Content: "kind: Service\n"}); err == nil { + t.Fatal("expected empty name error") + } + if err := sc.kubernetesManifestIs("alias", &godog.DocString{Content: " \n"}); err == nil { + t.Fatal("expected empty body error") + } + if err := sc.kubernetesManifestIs("alias", &godog.DocString{Content: "kind: Service\n"}); err != nil { + t.Fatalf("declare: %v", err) + } + err := sc.kubernetesManifestIs("alias", &godog.DocString{Content: "kind: Endpoints\n"}) + if err == nil || !strings.Contains(err.Error(), "already declared") { + t.Fatalf("err = %v, want already-declared error", err) + } +} + +// TestISuccessfullyApplyKubernetesManifestRejectsUndeclaredNameAndBadTable +// confirms that applying an undeclared manifest or passing a table without +// the context header fails before any kubectl command runs. +func TestISuccessfullyApplyKubernetesManifestRejectsUndeclaredNameAndBadTable(t *testing.T) { + sc, fake := newScenarioContext(t) + table := docTable(t, [][]string{{"context"}, {"k3d-ncp-local-cp"}}) + err := sc.iSuccessfullyApplyKubernetesManifest(context.Background(), "missing", table) + if err == nil || !strings.Contains(err.Error(), "not declared") { + t.Fatalf("err = %v, want not-declared error", err) + } + if err := sc.kubernetesManifestIs("alias", &godog.DocString{Content: "kind: Service\n"}); err != nil { + t.Fatalf("declare: %v", err) + } + badHeader := docTable(t, [][]string{{"cluster"}, {"k3d-ncp-local-cp"}}) + if err := sc.iSuccessfullyApplyKubernetesManifest(context.Background(), "alias", badHeader); err == nil { + t.Fatal("expected header error") + } + if len(fake.runs) != 0 { + t.Fatalf("runs = %d, want 0 before validation passes", len(fake.runs)) + } +} + +// TestISuccessfullyApplyKubernetesManifestNamesFailingContext verifies +// that when a later context row fails, the error names that row number +// and kube context so the operator knows which cluster rejected the apply. +func TestISuccessfullyApplyKubernetesManifestNamesFailingContext(t *testing.T) { + sc, fake := newScenarioContext(t) + fake.runResults = []harness.Result{{ExitCode: 0}, {ExitCode: 1}} + if err := sc.kubernetesManifestIs("alias", &godog.DocString{Content: "kind: Service\n"}); err != nil { + t.Fatalf("declare: %v", err) + } + table := docTable(t, [][]string{{"context"}, {"k3d-ncp-local-cp"}, {"k3d-ncp-local-compute-1"}}) + err := sc.iSuccessfullyApplyKubernetesManifest(context.Background(), "alias", table) + if err == nil || !strings.Contains(err.Error(), "row 2") || !strings.Contains(err.Error(), "k3d-ncp-local-compute-1") { + t.Fatalf("err = %v, want row 2 compute context failure", err) + } +} + func TestISubstituteBlockReplacesAndRestoresFile(t *testing.T) { sc, _ := newScenarioContext(t) rel := "global.yaml.gotmpl" @@ -1070,6 +1225,70 @@ func TestDNSNameShouldResolveFailureNamesTargetWithoutResolverOutput(t *testing. } } +// TestKubernetesWorkloadsShouldCompleteRolloutRunsExplicitWaits verifies +// that each table row produces exactly one explicit-context rollout status +// command, in row order, with the kind lowercased for kubectl. +func TestKubernetesWorkloadsShouldCompleteRolloutRunsExplicitWaits(t *testing.T) { + sc, fake := newScenarioContext(t) + fake.result = harness.Result{ExitCode: 0} + table := docTable(t, [][]string{ + {"kind", "name", "namespace"}, + {"StatefulSet", "llm-request-router-region-b", "nvcf"}, + {"Deployment", "llm-request-router-region-b-backend-router", "nvcf"}, + }) + + if err := sc.kubernetesWorkloadsShouldCompleteRollout(context.Background(), "k3d-ncp-local-cp", "10m", table); err != nil { + t.Fatalf("wait for workload rollouts: %v", err) + } + want := []string{ + "kubectl rollout status statefulset/llm-request-router-region-b -n nvcf --context k3d-ncp-local-cp --timeout=10m", + "kubectl rollout status deployment/llm-request-router-region-b-backend-router -n nvcf --context k3d-ncp-local-cp --timeout=10m", + } + if len(fake.runs) != len(want) { + t.Fatalf("runs = %d, want %d", len(fake.runs), len(want)) + } + for index, run := range fake.runs { + if run.command != want[index] { + t.Fatalf("command %d = %q, want %q", index+1, run.command, want[index]) + } + } +} + +// TestKubernetesWorkloadsShouldCompleteRolloutNamesFailingRow confirms +// that a non-zero rollout exit names the row, kind, and workload name in +// the error without echoing kubectl output. +func TestKubernetesWorkloadsShouldCompleteRolloutNamesFailingRow(t *testing.T) { + sc, fake := newScenarioContext(t) + fake.result = harness.Result{ExitCode: 1} + table := docTable(t, [][]string{ + {"kind", "name", "namespace"}, + {"StatefulSet", "llm-request-router-region-b", "nvcf"}, + }) + + err := sc.kubernetesWorkloadsShouldCompleteRollout(context.Background(), "k3d-ncp-local-cp", "10m", table) + if err == nil || !strings.Contains(err.Error(), "row 1") || !strings.Contains(err.Error(), `StatefulSet "llm-request-router-region-b"`) { + t.Fatalf("error = %v", err) + } +} + +// TestKubernetesWorkloadsShouldCompleteRolloutRejectsWrongHeaders confirms +// that a table whose headers are not kind, name, namespace in that order is +// rejected before any rollout command runs. +func TestKubernetesWorkloadsShouldCompleteRolloutRejectsWrongHeaders(t *testing.T) { + sc, fake := newScenarioContext(t) + table := docTable(t, [][]string{ + {"kind", "namespace", "name"}, + {"StatefulSet", "nvcf", "llm-request-router-region-b"}, + }) + + if err := sc.kubernetesWorkloadsShouldCompleteRollout(context.Background(), "k3d-ncp-local-cp", "10m", table); err == nil { + t.Fatal("expected header order error") + } + if len(fake.runs) != 0 { + t.Fatalf("runs = %d, want 0", len(fake.runs)) + } +} + func TestKubernetesResourceShouldContainFailureDoesNotExposeResourceValues(t *testing.T) { sc, fake := newScenarioContext(t) fake.result = harness.Result{ExitCode: 0, Stdout: `data: