ci(stack): check migrations against the proposed stack version bump

.github/workflows/build-test.yml

@@ -187,6 +187,43 @@ jobs:
       - name: Build, vet, and test the Go modules under tools/
         run: tools/ci/check-go-tools
 
+  stack-upgrade-policy:
+    name: stack upgrade policy
+    # Only a pull request has a baseline to measure a proposed bump against. On
+    # a push to main the candidate and the baseline are the same commit.
+    if: github.event_name == 'pull_request'
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        stack:
+          - nvcf-self-managed-stack
+          - nvcf-compute-plane-stack
+          - nvcf-observability-stack
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+          # The check diffs against the stack's last published release tag, so
+          # it needs the tags and the history behind them.
+          fetch-depth: 0
+          fetch-tags: true
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: tools/go-toolchain/go.mod
+
+      # Interpolations go through env, never into the script body: a ref name
+      # expanded inline is a shell injection.
+      - name: Check migrations against the proposed version bump
+        env:
+          BASE_REF: ${{ github.base_ref }}
+          STACK: ${{ matrix.stack }}
+        run: |
+          bump="$(tools/ci/release-bump-type for-branch "origin/${BASE_REF}")"
+          echo "proposed bump: ${bump}"
+          tools/ci/check-stack-upgrade-policy --stack "${STACK}" --bump "${bump}"
+
   github-release-helper:
     name: GitHub release helper
     runs-on: ubuntu-latest

tools/ci/check-stack-upgrade-policy

@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+# SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Stable CI entrypoint for the Go tool in tools/stack-upgrade-policy.
+#
+# The wrapper exists for the same two reasons tools/ci/chart-service-edge does.
+#
+# The repository root. `go run -C <dir>` leaves the process running with that
+# directory as its working directory, so the tool cannot find the release
+# metadata or the git history on its own. Resolving the root from this script's
+# own location means callers do not have to pass it.
+#
+# The exit code. `go run` does NOT propagate the program's status: it prints
+# "exit status N" and exits 1. This tool distinguishes 1 (policy violation or
+# error) from 2 (bad invocation), so collapsing them would be a trap.
+#
+# Run the tests with: go test -C tools/stack-upgrade-policy ./...
+set -euo pipefail
+
+repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
+bin_dir="$(mktemp -d)"
+trap 'rm -rf "${bin_dir}"' EXIT
+
+go build -C "${repo_root}/tools/stack-upgrade-policy" -o "${bin_dir}/stack-upgrade-policy" .
+
+# Not exec, so the trap above still runs, and not under errexit, so the exit
+# code reaches the caller rather than aborting the shell first.
+set +e
+"${bin_dir}/stack-upgrade-policy" --root "${repo_root}" "$@"
+status=$?
+set -e
+exit "${status}"

tools/ci/github-release-subprojects.json

@@ -42,6 +42,7 @@
     {
       "id": "nvcf-self-managed-stack",
       "path": "deploy/stacks/self-managed",
+      "migration_paths": ["migrations/cassandra", "migrations/openbao"],
       "service_name": "nvcf-self-managed-stack",
       "tag_format": "deploy/stacks/self-managed/v${version}",
       "version_file": "VERSION",

tools/stack-upgrade-policy/.gitignore

@@ -0,0 +1,2 @@
+# go build ./... drops the binary here; it must never be committed.
+/stack-upgrade-policy

tools/stack-upgrade-policy/classify.go

@@ -0,0 +1,57 @@
+// SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package main
+
+import (
+	"regexp"
+	"strings"
+)
+
+// Class is what a migration does to data that already exists.
+type Class int
+
+const (
+	// Additive migrations only add schema. A cluster that skipped every stack
+	// version between two points still arrives at the right schema, because
+	// golang-migrate applies the ordered set regardless of how far behind the
+	// cluster was.
+	Additive Class = iota
+	// Destructive migrations remove a table, type, or column, or delete rows.
+	// Skipping versions is still safe for the schema itself, but the drop is
+	// unrecoverable without a restore, and it constrains deployment order:
+	// the migration must land before the services that read what it removes.
+	Destructive
+)
+
+func (c Class) String() string {
+	if c == Destructive {
+		return "Destructive"
+	}
+	return "Additive"
+}
+
+var destructive = regexp.MustCompile(`(?i)\b(DROP|TRUNCATE|DELETE)\b`)
+
+// Classify reports what a CQL migration does. Comments are stripped first:
+// migrations/cassandra/keyspaces/nvcf_api/03_init_tables.up.sql documents a
+// "high-churn write/delete workload" above a CREATE TABLE, and a classifier
+// that reads raw text calls that destructive.
+func Classify(sql string) Class {
+	if destructive.MatchString(stripComments(sql)) {
+		return Destructive
+	}
+	return Additive
+}
+
+func stripComments(sql string) string {
+	var b strings.Builder
+	for line := range strings.SplitSeq(sql, "\n") {
+		if i := strings.Index(line, "--"); i >= 0 {
+			line = line[:i]
+		}
+		b.WriteString(line)
+		b.WriteByte('\n')
+	}
+	return b.String()
+}

tools/stack-upgrade-policy/classify_test.go

@@ -0,0 +1,46 @@
+// SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package main
+
+import "testing"
+
+func TestClassifyAddColumnIsAdditive(t *testing.T) {
+	sql := "ALTER TABLE nvcf_api.functions_v3 ADD llm_config frozen<llm_config_udt>;"
+	if got := Classify(sql); got != Additive {
+		t.Fatalf("Classify() = %v, want Additive", got)
+	}
+}
+
+func TestClassifyDropTableIsDestructive(t *testing.T) {
+	sql := "DROP TABLE IF EXISTS nvcf_autoscaler.recently_invoked_functions_history;"
+	if got := Classify(sql); got != Destructive {
+		t.Fatalf("Classify() = %v, want Destructive", got)
+	}
+}
+
+func TestClassifyAlterTableDropIsDestructive(t *testing.T) {
+	sql := "ALTER TABLE IF EXISTS nvcf_api.functions_deployment_v2 DROP IF EXISTS gpu_specs;"
+	if got := Classify(sql); got != Destructive {
+		t.Fatalf("Classify() = %v, want Destructive", got)
+	}
+}
+
+func TestClassifyDropTypeIsDestructive(t *testing.T) {
+	sql := "DROP TYPE IF EXISTS nvct_api.health_udt;"
+	if got := Classify(sql); got != Destructive {
+		t.Fatalf("Classify() = %v, want Destructive", got)
+	}
+}
+
+// migrations/cassandra/keyspaces/nvcf_api/03_init_tables.up.sql carries the
+// comment "Tuned for high-churn write/delete workload: UCS + short gc_grace."
+// A classifier that greps the raw text calls that table creation destructive.
+func TestClassifyIgnoresKeywordsInComments(t *testing.T) {
+	sql := `-- Tuned for high-churn write/delete workload: UCS + short gc_grace.
+-- We may DROP this table later.
+CREATE TABLE IF NOT EXISTS nvcf_api.functions_v3 (id uuid PRIMARY KEY);`
+	if got := Classify(sql); got != Additive {
+		t.Fatalf("Classify() = %v, want Additive", got)
+	}
+}

tools/stack-upgrade-policy/config.go

@@ -0,0 +1,54 @@
+// SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package main
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// MetadataPath is the release metadata every subproject is already registered
+// in. The migration paths live here rather than in a new per-stack file so
+// that a stack has one place describing how it releases, not two.
+const MetadataPath = "tools/ci/github-release-subprojects.json"
+
+const stackPrefix = "deploy/stacks/"
+
+// Stack is one released stack bundle and the paths whose migrations ship with
+// it. A stack that declares no migration paths is a stack that ships no
+// schema, and the check is a no-op for it.
+type Stack struct {
+	ID             string   `json:"id"`
+	Path           string   `json:"path"`
+	MigrationPaths []string `json:"migration_paths"`
+}
+
+type metadata struct {
+	Services []Stack `json:"services"`
+}
+
+// LoadStack finds a stack by its release-metadata id.
+func LoadStack(root, id string) (Stack, error) {
+	raw, err := os.ReadFile(filepath.Join(root, MetadataPath))
+	if err != nil {
+		return Stack{}, err
+	}
+	var meta metadata
+	if err := json.Unmarshal(raw, &meta); err != nil {
+		return Stack{}, fmt.Errorf("%s: %w", MetadataPath, err)
+	}
+	for _, s := range meta.Services {
+		if s.ID != id {
+			continue
+		}
+		if !strings.HasPrefix(s.Path, stackPrefix) {
+			return Stack{}, fmt.Errorf("%q is not a stack: its path %q is not under %s", id, s.Path, stackPrefix)
+		}
+		return s, nil
+	}
+	return Stack{}, fmt.Errorf("no subproject %q in %s", id, MetadataPath)
+}

tools/stack-upgrade-policy/config_test.go

@@ -0,0 +1,74 @@
+// SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+const metadataFixture = `{
+  "version": 1,
+  "services": [
+    {"id": "nvcf-cli", "path": "src/clis/nvcf-cli"},
+    {"id": "nvcf-self-managed-stack", "path": "deploy/stacks/self-managed",
+     "migration_paths": ["migrations/cassandra", "migrations/openbao"]},
+    {"id": "nvcf-observability-stack", "path": "deploy/stacks/observability"}
+  ]
+}`
+
+func metadataDir(t *testing.T) string {
+	t.Helper()
+	dir := t.TempDir()
+	p := filepath.Join(dir, MetadataPath)
+	if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(p, []byte(metadataFixture), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	return dir
+}
+
+func TestLoadStackReadsPathAndMigrationPaths(t *testing.T) {
+	s, err := LoadStack(metadataDir(t), "nvcf-self-managed-stack")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if s.Path != "deploy/stacks/self-managed" {
+		t.Errorf("Path = %q", s.Path)
+	}
+	if len(s.MigrationPaths) != 2 {
+		t.Errorf("MigrationPaths = %v, want two", s.MigrationPaths)
+	}
+}
+
+// A stack that declares no migration paths is not an error: the compute-plane
+// and observability stacks ship no schema, so the check is a no-op for them
+// until they need it.
+func TestLoadStackAllowsNoMigrationPaths(t *testing.T) {
+	s, err := LoadStack(metadataDir(t), "nvcf-observability-stack")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(s.MigrationPaths) != 0 {
+		t.Errorf("MigrationPaths = %v, want none", s.MigrationPaths)
+	}
+}
+
+func TestLoadStackRejectsUnknownID(t *testing.T) {
+	if _, err := LoadStack(metadataDir(t), "no-such-stack"); err == nil {
+		t.Fatal("err = nil, want an error naming the unknown stack")
+	}
+}
+
+// Guards against pointing the check at a service that is not a stack, which
+// would silently compare against a tag series that has nothing to do with a
+// published bundle.
+func TestLoadStackRejectsANonStackService(t *testing.T) {
+	if _, err := LoadStack(metadataDir(t), "nvcf-cli"); err == nil {
+		t.Fatal("err = nil, want an error for a non-stack service")
+	}
+}