From 079a550a46939860655a268aeeb806c525dc3ccd Mon Sep 17 00:00:00 2001 From: Sylvain Niles Date: Mon, 22 Jun 2026 20:42:01 -0700 Subject: [PATCH 1/7] feat(database): enable PostgreSQL provider for all RPs via Helm chart Fixes the pre-existing gaps that prevented database.enabled=true from producing a working PostgreSQL-backed control plane: - UCP, Applications RP, and Dynamic RP configmaps/deployments were hardcoded to the apiserver provider with no database.enabled conditional; they now switch to the postgresql provider and inject POSTGRES_PASSWORD from the database secret when database.enabled=true. - Add init-db ConfigMap (mounted at /docker-entrypoint-initdb.d) that creates the per-RP databases, users, and tables on first start (option 3 from #8398). - Fix POSTGRES_DB secret value (was the literal string POSTGRES_DB). - Pin the postgres image tag to 16-alpine. - Fix the databaseProvider URL env-var substitution in factory.go, which replaced the entire URL with the first captured variable name instead of expanding env-var references. Adds helm-unittest coverage for the conditional rendering and a unit test for the env-var expansion helper. Also adds the Repo Radius state-storage technical design note. Relates to #8096, #8398 Signed-off-by: Sylvain Niles --- .../templates/database/configmap-initdb.yaml | 49 +++++ .../Chart/templates/database/configmaps.yaml | 2 +- .../Chart/templates/database/statefulset.yaml | 6 + .../templates/dynamic-rp/configmaps.yaml | 6 + .../templates/dynamic-rp/deployment.yaml | 7 + deploy/Chart/templates/rp/configmaps.yaml | 6 + deploy/Chart/templates/rp/deployment.yaml | 7 + deploy/Chart/templates/ucp/configmaps.yaml | 6 + deploy/Chart/templates/ucp/deployment.yaml | 7 + deploy/Chart/tests/database_test.yaml | 179 +++++++++++++++++ deploy/Chart/values.yaml | 2 +- .../2026-06-repo-radius-state-storage.md | 190 ++++++++++++++++++ .../database/databaseprovider/factory.go | 21 +- .../databaseprovider/storageprovider_test.go | 44 ++++ 14 files changed, 523 insertions(+), 9 deletions(-) create mode 100644 deploy/Chart/templates/database/configmap-initdb.yaml create mode 100644 deploy/Chart/tests/database_test.yaml create mode 100644 eng/design-notes/2026-06-repo-radius-state-storage.md diff --git a/deploy/Chart/templates/database/configmap-initdb.yaml b/deploy/Chart/templates/database/configmap-initdb.yaml new file mode 100644 index 00000000000..dd07fedd31b --- /dev/null +++ b/deploy/Chart/templates/database/configmap-initdb.yaml @@ -0,0 +1,49 @@ +{{- if .Values.database.enabled }} +apiVersion: v1 +kind: ConfigMap +metadata: + name: database-initdb + namespace: {{ .Release.Namespace }} + labels: + control-plane: database + app.kubernetes.io/name: database + app.kubernetes.io/part-of: radius +data: + init-db.sh: | + #!/bin/bash + set -e + + RESOURCE_PROVIDERS=("ucp" "applications_rp" "dynamic_rp") + + for RESOURCE_PROVIDER in "${RESOURCE_PROVIDERS[@]}"; do + echo "Creating database and user for $RESOURCE_PROVIDER" + psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL + DO \$\$ + BEGIN + IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = '$RESOURCE_PROVIDER') THEN + CREATE USER $RESOURCE_PROVIDER WITH PASSWORD '$POSTGRES_PASSWORD'; + END IF; + END + \$\$; + SELECT 'CREATE DATABASE $RESOURCE_PROVIDER OWNER $RESOURCE_PROVIDER' WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = '$RESOURCE_PROVIDER')\gexec + GRANT ALL PRIVILEGES ON DATABASE $RESOURCE_PROVIDER TO $RESOURCE_PROVIDER; + EOSQL + done + + for RESOURCE_PROVIDER in "${RESOURCE_PROVIDERS[@]}"; do + echo "Creating tables in database $RESOURCE_PROVIDER" + psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$RESOURCE_PROVIDER" <<-'EOSQL' + CREATE TABLE IF NOT EXISTS resources ( + id TEXT PRIMARY KEY NOT NULL, + original_id TEXT NOT NULL, + resource_type TEXT NOT NULL, + root_scope TEXT NOT NULL, + routing_scope TEXT NOT NULL, + etag TEXT NOT NULL, + created_at TIMESTAMP(6) WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, + resource_data JSONB NOT NULL + ); + CREATE INDEX IF NOT EXISTS idx_resource_query ON resources (resource_type, root_scope); + EOSQL + done +{{- end }} diff --git a/deploy/Chart/templates/database/configmaps.yaml b/deploy/Chart/templates/database/configmaps.yaml index 29d9b0edd3e..eafb146e122 100644 --- a/deploy/Chart/templates/database/configmaps.yaml +++ b/deploy/Chart/templates/database/configmaps.yaml @@ -11,5 +11,5 @@ metadata: stringData: POSTGRES_USER: "{{ .Values.database.postgres_user }}" POSTGRES_PASSWORD: "{{ randAlphaNum 16 }}" - POSTGRES_DB: POSTGRES_DB + POSTGRES_DB: "radius" {{- end }} diff --git a/deploy/Chart/templates/database/statefulset.yaml b/deploy/Chart/templates/database/statefulset.yaml index c8d0ce809ab..93af41a64cd 100644 --- a/deploy/Chart/templates/database/statefulset.yaml +++ b/deploy/Chart/templates/database/statefulset.yaml @@ -62,6 +62,12 @@ spec: - name: database mountPath: /var/lib/postgresql/data subPath: postgres + - name: initdb-scripts + mountPath: /docker-entrypoint-initdb.d + volumes: + - name: initdb-scripts + configMap: + name: database-initdb volumeClaimTemplates: - metadata: diff --git a/deploy/Chart/templates/dynamic-rp/configmaps.yaml b/deploy/Chart/templates/dynamic-rp/configmaps.yaml index 7f4c9e0d743..f815e85398e 100644 --- a/deploy/Chart/templates/dynamic-rp/configmaps.yaml +++ b/deploy/Chart/templates/dynamic-rp/configmaps.yaml @@ -14,10 +14,16 @@ data: name: self-hosted roleLocation: "global" databaseProvider: + {{- if .Values.database.enabled }} + provider: "postgresql" + postgresql: + url: "postgresql://dynamic_rp:${POSTGRES_PASSWORD}@database.{{ .Release.Namespace }}:5432/dynamic_rp?sslmode=disable" + {{- else }} provider: "apiserver" apiserver: context: "" namespace: "radius-system" + {{- end }} queueProvider: provider: "apiserver" name: "dynamic-rp" diff --git a/deploy/Chart/templates/dynamic-rp/deployment.yaml b/deploy/Chart/templates/dynamic-rp/deployment.yaml index 032d7d4539b..174f21ba6a6 100644 --- a/deploy/Chart/templates/dynamic-rp/deployment.yaml +++ b/deploy/Chart/templates/dynamic-rp/deployment.yaml @@ -213,6 +213,13 @@ spec: args: - --config-file=/etc/config/radius-self-host.yaml env: + {{- if .Values.database.enabled }} + - name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: database-secret + key: POSTGRES_PASSWORD + {{- end }} - name: SKIP_ARM value: 'false' - name: ARM_AUTH_METHOD diff --git a/deploy/Chart/templates/rp/configmaps.yaml b/deploy/Chart/templates/rp/configmaps.yaml index c8e2e5e3ebd..2fc782ea69e 100644 --- a/deploy/Chart/templates/rp/configmaps.yaml +++ b/deploy/Chart/templates/rp/configmaps.yaml @@ -14,10 +14,16 @@ data: name: self-hosted roleLocation: "global" databaseProvider: + {{- if .Values.database.enabled }} + provider: "postgresql" + postgresql: + url: "postgresql://applications_rp:${POSTGRES_PASSWORD}@database.{{ .Release.Namespace }}:5432/applications_rp?sslmode=disable" + {{- else }} provider: "apiserver" apiserver: context: "" namespace: "radius-system" + {{- end }} queueProvider: provider: "apiserver" name: "radius" diff --git a/deploy/Chart/templates/rp/deployment.yaml b/deploy/Chart/templates/rp/deployment.yaml index bae66529c13..7bd2c3cff63 100644 --- a/deploy/Chart/templates/rp/deployment.yaml +++ b/deploy/Chart/templates/rp/deployment.yaml @@ -162,6 +162,13 @@ spec: args: - --config-file=/etc/config/radius-self-host.yaml env: + {{- if .Values.database.enabled }} + - name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: database-secret + key: POSTGRES_PASSWORD + {{- end }} - name: SKIP_ARM value: 'false' - name: ARM_AUTH_METHOD diff --git a/deploy/Chart/templates/ucp/configmaps.yaml b/deploy/Chart/templates/ucp/configmaps.yaml index 64fbca32a40..9a830b88a67 100644 --- a/deploy/Chart/templates/ucp/configmaps.yaml +++ b/deploy/Chart/templates/ucp/configmaps.yaml @@ -18,10 +18,16 @@ data: pathBase: /apis/api.ucp.dev/v1alpha3 tlsCertificateDirectory: /var/tls/cert databaseProvider: + {{- if .Values.database.enabled }} + provider: "postgresql" + postgresql: + url: "postgresql://ucp:${POSTGRES_PASSWORD}@database.{{ .Release.Namespace }}:5432/ucp?sslmode=disable" + {{- else }} provider: "apiserver" apiserver: context: "" namespace: "radius-system" + {{- end }} secretProvider: provider: kubernetes diff --git a/deploy/Chart/templates/ucp/deployment.yaml b/deploy/Chart/templates/ucp/deployment.yaml index d90d87a62e8..20964f0447c 100644 --- a/deploy/Chart/templates/ucp/deployment.yaml +++ b/deploy/Chart/templates/ucp/deployment.yaml @@ -61,6 +61,13 @@ spec: value: '/var/tls/cert' - name: PORT value: '9443' + {{- if .Values.database.enabled }} + - name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: database-secret + key: POSTGRES_PASSWORD + {{- end }} {{- if .Values.global.rootCA.cert }} - name: {{ .Values.global.rootCA.sslCertDirEnvVar }} value: {{ .Values.global.rootCA.mountPath }} diff --git a/deploy/Chart/tests/database_test.yaml b/deploy/Chart/tests/database_test.yaml new file mode 100644 index 00000000000..a04499887e6 --- /dev/null +++ b/deploy/Chart/tests/database_test.yaml @@ -0,0 +1,179 @@ +suite: test database (postgresql) provider configuration +templates: + - database/configmaps.yaml + - database/configmap-initdb.yaml + - database/statefulset.yaml + - ucp/configmaps.yaml + - ucp/deployment.yaml + - rp/configmaps.yaml + - rp/deployment.yaml + - dynamic-rp/configmaps.yaml + - dynamic-rp/deployment.yaml +tests: + # --- database.enabled=false (default) --- + - it: should not render the database secret when database is disabled + set: + database.enabled: false + asserts: + - hasDocuments: + count: 0 + template: database/configmaps.yaml + + - it: should not render the initdb configmap when database is disabled + set: + database.enabled: false + asserts: + - hasDocuments: + count: 0 + template: database/configmap-initdb.yaml + + - it: should use the apiserver provider for UCP when database is disabled + set: + database.enabled: false + asserts: + - matchRegex: + path: data["ucp-config.yaml"] + pattern: 'provider: "apiserver"' + template: ucp/configmaps.yaml + + - it: should use the apiserver provider for applications-rp when database is disabled + set: + database.enabled: false + asserts: + - matchRegex: + path: data["radius-self-host.yaml"] + pattern: 'provider: "apiserver"' + template: rp/configmaps.yaml + + - it: should use the apiserver provider for dynamic-rp when database is disabled + set: + database.enabled: false + asserts: + - matchRegex: + path: data["radius-self-host.yaml"] + pattern: 'provider: "apiserver"' + template: dynamic-rp/configmaps.yaml + + # --- database.enabled=true --- + - it: should render the database secret with a literal radius database name + set: + database.enabled: true + asserts: + - isKind: + of: Secret + template: database/configmaps.yaml + - equal: + path: stringData.POSTGRES_DB + value: "radius" + template: database/configmaps.yaml + + - it: should render the initdb configmap when database is enabled + set: + database.enabled: true + asserts: + - isKind: + of: ConfigMap + template: database/configmap-initdb.yaml + - equal: + path: metadata.name + value: database-initdb + template: database/configmap-initdb.yaml + + - it: should mount the initdb scripts into the database statefulset + set: + database.enabled: true + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: initdb-scripts + configMap: + name: database-initdb + template: database/statefulset.yaml + + - it: should use the postgresql provider for UCP when database is enabled + set: + database.enabled: true + asserts: + - matchRegex: + path: data["ucp-config.yaml"] + pattern: 'provider: "postgresql"' + template: ucp/configmaps.yaml + - matchRegex: + path: data["ucp-config.yaml"] + pattern: 'postgresql://ucp:\$\{POSTGRES_PASSWORD\}@database\.' + template: ucp/configmaps.yaml + + - it: should use the postgresql provider for applications-rp when database is enabled + set: + database.enabled: true + asserts: + - matchRegex: + path: data["radius-self-host.yaml"] + pattern: 'postgresql://applications_rp:\$\{POSTGRES_PASSWORD\}@database\.' + template: rp/configmaps.yaml + + - it: should use the postgresql provider for dynamic-rp when database is enabled + set: + database.enabled: true + asserts: + - matchRegex: + path: data["radius-self-host.yaml"] + pattern: 'postgresql://dynamic_rp:\$\{POSTGRES_PASSWORD\}@database\.' + template: dynamic-rp/configmaps.yaml + + - it: should inject POSTGRES_PASSWORD into UCP when database is enabled + set: + database.enabled: true + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: database-secret + key: POSTGRES_PASSWORD + template: ucp/deployment.yaml + + - it: should inject POSTGRES_PASSWORD into applications-rp when database is enabled + set: + database.enabled: true + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: database-secret + key: POSTGRES_PASSWORD + template: rp/deployment.yaml + + - it: should inject POSTGRES_PASSWORD into dynamic-rp when database is enabled + set: + database.enabled: true + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: database-secret + key: POSTGRES_PASSWORD + template: dynamic-rp/deployment.yaml + + - it: should not inject POSTGRES_PASSWORD into dynamic-rp when database is disabled + set: + database.enabled: false + asserts: + - notContains: + path: spec.template.spec.containers[0].env + content: + name: POSTGRES_PASSWORD + valueFrom: + secretKeyRef: + name: database-secret + key: POSTGRES_PASSWORD + template: dynamic-rp/deployment.yaml diff --git a/deploy/Chart/values.yaml b/deploy/Chart/values.yaml index 987fa536385..1dd07158934 100644 --- a/deploy/Chart/values.yaml +++ b/deploy/Chart/values.yaml @@ -198,7 +198,7 @@ database: enabled: false # Enable the Postgres database install postgres_user: "radius" image: mirror/postgres - tag: latest + tag: 16-alpine storageClassName: "" # set to the storage class name if required, the empty string will pickup the default storage class. # Minimum resource requirements, may need to revisit and scale. storageSize: "1Gi" diff --git a/eng/design-notes/2026-06-repo-radius-state-storage.md b/eng/design-notes/2026-06-repo-radius-state-storage.md new file mode 100644 index 00000000000..927d97d3c87 --- /dev/null +++ b/eng/design-notes/2026-06-repo-radius-state-storage.md @@ -0,0 +1,190 @@ +# Repo Radius — State Storage (Technical Design) + +* **Author**: Sylvain Niles (@sylvainsf) +* **Status**: Draft +* **Feature spec**: Repo Radius (Zach Casper) — [PR #12078](https://github.com/radius-project/radius/pull/12078) +* **Related issues**: [#8096 External data store for Radius](https://github.com/radius-project/radius/issues/8096), [#8398 Postgres DB initialization](https://github.com/radius-project/radius/issues/8398) +* **Supersedes prototype**: [PR #11457](https://github.com/radius-project/radius/pull/11457) (closed unmerged) + +## Scope + +This document covers **Investment 2 of the Repo Radius feature spec: externalization of the +Radius data store** — and *only* the state-storage aspects of it. It addresses how Radius +control-plane state and Terraform recipe state survive across ephemeral GitHub Actions runs. + +Explicitly **out of scope** for this document: + +* Application graph storage / serialization (a separate workstream owns the serialized graph format). +* The Repo Radius workflow contract, OIDC/cloud credential integration, and external-cluster deployment (Investments 1, 3, 4). +* Replacing the persistent control-plane deployment model. + +## Problem + +Repo Radius runs the Radius control plane on an **ephemeral k3d cluster** inside a GitHub +Actions runner. The cluster — and everything stored in it — is destroyed when the workflow +ends. For Radius to be usable across runs, all durable state must be exported before teardown +and restored on the next run. + +"Durable state" in the current control plane is **two physically separate stores**: + +| State | Where it lives today | Survives teardown without action? | +|-------|----------------------|-----------------------------------| +| Control-plane resource data + deployment history | PostgreSQL — three logical databases (`ucp`, `applications_rp`, `dynamic_rp`) behind [`database.Client`](../../pkg/components/database/client.go) | No | +| Terraform recipe state | Kubernetes `Secret` objects named `tfstate-default-` in the `radius-system` namespace, written by the [`kubernetes` Terraform backend](../../pkg/recipes/terraform/config/backends/kubernetes.go) | No | + +## Gaps in the prototype / demo + +The prototype ([PR #11457](https://github.com/radius-project/radius/pull/11457), branch +`filesystem-state`) demonstrated state persistence by dumping the three PostgreSQL databases to +a git orphan branch and restoring them on startup. Reviewing it against the requirements above +surfaced the following gaps. This design addresses them. + +### Gap 1 — Terraform state is never persisted + +The prototype's backup covers only the three PostgreSQL databases. **Terraform state is stored +in Kubernetes Secrets, not in PostgreSQL**, so it is destroyed on every shutdown and never +restored. + +**Why the demo still worked:** the demo exercised paths that do not depend on Terraform state +surviving: + +* **Bicep recipes have no local state.** The Deployment Engine reconciles declaratively against + ARM/cloud, which is idempotent. Restoring PostgreSQL is sufficient. +* **First-time Terraform deploys** start from an empty backend and succeed; the cluster is alive + for the whole run, so state exists *within* a run. + +The gap only manifests on a **second deploy of the same Terraform-backed resource across two +runs** (feature-spec Scenario 4, "update a deployed application"). After a PostgreSQL restore, +Radius believes the resource exists, but the fresh cluster's Terraform backend is empty. The +next `terraform apply` plans from scratch, producing "already exists" errors for +stable-named resources or **orphaned duplicate** cloud resources for generated-named ones. + +This is the primary capability gap closed by this design. + +### Gap 2 — PostgreSQL was not actually usable for all RPs + +Independent of Repo Radius, enabling `database.enabled=true` in the Helm chart did not produce a +working PostgreSQL-backed control plane: + +* The UCP, Applications RP, and Dynamic RP configmaps/deployments were **hardcoded to the + `apiserver` provider** with no `database.enabled` conditional, so no resource provider used + PostgreSQL even when requested. +* The `POSTGRES_DB` secret value was the literal string `"POSTGRES_DB"`. +* No init-db scripts ran, so the per-RP databases, users, and tables were never created. +* The `databaseProvider` URL env-var substitution in + [`factory.go`](../../pkg/components/database/databaseprovider/factory.go) had a regex bug that + replaced the entire URL with the first captured variable name instead of expanding `${VAR}`. + +These are pre-existing defects on the path that [#8398](https://github.com/radius-project/radius/issues/8398) +("Postgres DB initialization") already chose to fix via an init-db configmap. This design +includes those fixes as a prerequisite. + +### Gap 3 — Backup push failures were silent + +The prototype's `Push` treated a failed `git push` as a non-fatal warning. That is acceptable +for advisory sentinel files but means a failed **state backup** push is silent data loss. The +durability requirement is that a backup either succeeds or fails the command loudly. + +### Gap 4 — The deploy lock was advisory, not a real mutex + +The prototype's `.deploy-lock` was read from the local worktree and then pushed, so two runners +could both observe "no lock" and both proceed. A correct lock must use an atomic +compare-and-swap. + +## Design decisions + +### Decision 1 — Storage backend: git orphan branch for v1, OCI/GHCR deferred to v2 + +State is persisted to a **git orphan branch** (`radius-state`) in the same repository, as in the +prototype. + +The feature spec raises OCI/GHCR as an alternative with better security and size properties. We +evaluated leading with OCI for v1 and rejected it: + +* **Speed**: backup payloads are small (PostgreSQL dumps of control-plane metadata are + hundreds of KB). At that size both `git push` and `oras push` are dominated by TLS + auth + round-trips, not payload, so there is no meaningful speed advantage. +* **Simplicity**: the orphan-branch implementation already exists and ran in the demo. OCI + requires new push/pull plumbing, a tag compare-and-swap lock, and an artifact media-type + decision — strictly more code to ship first. + +OCI/GHCR's genuine advantages are **security** (separate registry RBAC versus repository-read +exposure of the orphan branch) and **bounded storage growth** (content-addressed layers versus +unbounded git history). Both are real but neither is a v1 blocker. They are recorded as the +**v2 direction** below. + +### Decision 2 — Control-plane state: physical `pg_dump`, not logical export + +Control-plane state is captured with physical `pg_dump` / `psql` against the in-cluster +PostgreSQL, as in the prototype. + +A logical export through `database.Client` was considered (it would be storage-engine +independent and could feed an offline graph reader). It is rejected because: + +* Radius control-plane state is a graph of linked key/value records with ETags and + cross-references. A logical re-`Save` mints new ETags and forces a dependency-ordered restore — + strictly more fragile than a physical dump that preserves rows, ETags, and timestamps exactly. +* Its main upside, offline graph readability, is **out of scope** here and owned by a separate + serialized-format workstream. +* The control plane has a fixed, known set of resource providers; we are not adding new RPs, so + the fixed database list (`ucp`, `applications_rp`, `dynamic_rp`) is acceptable. + +### Decision 3 — Terraform state: back up the backend Secrets alongside the PostgreSQL dumps + +For v1, Terraform state is persisted by exporting the `tfstate-default-*` Kubernetes Secrets from +the `radius-system` namespace into the same state worktree as the PostgreSQL dumps, committed and +pushed in the same atomic operation. On startup, after the cluster is ready and **before any +deploy**, the Secrets are restored into the namespace. + +This mirrors the existing PostgreSQL backup flow exactly (same worktree, same commit/push, same +semaphore) and is the minimal change that closes Gap 1. + +A v2 alternative — switching the Terraform backend from `kubernetes` to the `pg` backend pointed +at the same PostgreSQL instance, so a single `pg_dump` captures both stores — is recorded below. +It is deferred because it requires backend credential injection and per-recipe state isolation +work beyond v1 scope. + +### Decision 4 — Durability and locking hardening + +* The **final state-backup push must fail the command** on error (Gap 3). Advisory sentinel + pushes remain best-effort. +* The deploy lock uses git's own compare-and-swap: acquisition commits the lock file and pushes; + a non-fast-forward **push rejection is a failed acquisition** (Gap 4). On rejection, fetch and + apply the existing `RunID`/`RunAttempt` takeover logic for same-run retries. The lock is keyed + by **GitHub Environment name** so deploys to different environments do not serialize against + each other. +* A small checksum manifest accompanies the dumps so a corrupt restore **fails closed** rather + than silently starting from an empty state. + +## v2 direction (not in this delivery) + +* **OCI/GHCR backend**: a pluggable storage backend that pushes an encrypted, content-addressed + state artifact to a private GHCR repo, with a tag compare-and-swap lock. Resolves the + orphan-branch security exposure and unbounded git-history growth. +* **Unified Terraform backend on PostgreSQL**: move the Terraform backend to `pg` so control-plane + and Terraform state collapse into a single dump and a single restore. +* **Client-side envelope encryption** (age/sops) of state artifacts before they leave the cluster, + since both PostgreSQL data and Terraform state can contain secrets. + +## Delivery plan + +| PR | Contents | Closes | +|----|----------|--------| +| PR 1 | PostgreSQL enablement fixes: Dynamic RP `database.enabled` conditional, init-db configmap, `POSTGRES_DB` value, `factory.go` env-var substitution, Helm chart tests | Gap 2 | +| PR 2 | Terraform-state Secret backup/restore wired into the existing state-worktree flow | Gap 1 | +| PR 3 | Durability hardening: loud backup-push failure, push-rejection CAS lock, checksum manifest | Gaps 3, 4 | + +## Test plan + +* **Unit**: Helm chart conditional rendering (PR 1); Terraform-state export/import round-trip with + a fake Kubernetes client (PR 2); push-failure and lock-contention paths (PR 3). +* **Functional**: an end-to-end lifecycle that deploys a **Terraform-backed** resource, shuts + down, restarts, and deploys an **update** to the same resource — the path that exposes Gap 1. + +## Security + +* State (PostgreSQL dumps and Terraform Secrets, which may contain secret values) is pushed to a + branch in the repository. For v1 the repository **must be private**; this constraint is removed + in v2 by the encrypted OCI backend. +* Git credentials use the GitHub Actions token; pushing the state branch requires + `contents: write`. diff --git a/pkg/components/database/databaseprovider/factory.go b/pkg/components/database/databaseprovider/factory.go index c0b3555014b..c3944dc3076 100644 --- a/pkg/components/database/databaseprovider/factory.go +++ b/pkg/components/database/databaseprovider/factory.go @@ -20,6 +20,7 @@ import ( context "context" "errors" "fmt" + "os" "regexp" "github.com/jackc/pgx/v5/pgxpool" @@ -84,19 +85,25 @@ func initInMemoryClient(ctx context.Context, opt Options) (store.Client, error) return inmemory.NewClient(), nil } +// envVarPattern matches ${VAR_NAME} references for expansion against environment variables. +var envVarPattern = regexp.MustCompile(`\$\{([a-zA-Z_][a-zA-Z0-9_]*)\}`) + +// expandEnvURL replaces ${VAR} references in the connection URL with the values of the +// corresponding environment variables. References to unset variables expand to an empty string. +func expandEnvURL(url string) string { + return envVarPattern.ReplaceAllStringFunc(url, func(match string) string { + varName := envVarPattern.FindStringSubmatch(match)[1] + return os.Getenv(varName) + }) +} + // initPostgreSQLClient creates a new PostgreSQL store client. func initPostgreSQLClient(ctx context.Context, opt Options) (store.Client, error) { if opt.PostgreSQL.URL == "" { return nil, errors.New("failed to initialize PostgreSQL client: URL is required") } - url := opt.PostgreSQL.URL - regex := regexp.MustCompile(`$\{([a-zA-Z_]+)\}`) - matches := regex.FindSubmatch([]byte(opt.PostgreSQL.URL)) - if len(matches) > 1 { - // Extract the captured expression. - url = string(matches[1]) - } + url := expandEnvURL(opt.PostgreSQL.URL) pool, err := pgxpool.New(ctx, url) if err != nil { diff --git a/pkg/components/database/databaseprovider/storageprovider_test.go b/pkg/components/database/databaseprovider/storageprovider_test.go index ef038e57dd7..8729c537f0c 100644 --- a/pkg/components/database/databaseprovider/storageprovider_test.go +++ b/pkg/components/database/databaseprovider/storageprovider_test.go @@ -118,6 +118,50 @@ func TestGetClient_UnsupportedProvider(t *testing.T) { require.Equal(t, "unsupported database provider: unsupported", err.Error()) } +func Test_expandEnvURL(t *testing.T) { + tests := []struct { + name string + url string + env map[string]string + expected string + }{ + { + name: "no substitution", + url: "postgresql://user:pass@host:5432/db", + expected: "postgresql://user:pass@host:5432/db", + }, + { + name: "single variable is expanded in place", + url: "postgresql://ucp:${POSTGRES_PASSWORD}@database:5432/ucp", + env: map[string]string{"POSTGRES_PASSWORD": "s3cret"}, + expected: "postgresql://ucp:s3cret@database:5432/ucp", + }, + { + name: "multiple variables are all expanded", + url: "postgresql://${PGUSER}:${PGPASS}@host:5432/db", + env: map[string]string{ + "PGUSER": "ucp", + "PGPASS": "p@ss", + }, + expected: "postgresql://ucp:p@ss@host:5432/db", + }, + { + name: "unset variable expands to empty string", + url: "postgresql://ucp:${MISSING}@host:5432/db", + expected: "postgresql://ucp:@host:5432/db", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for k, v := range tt.env { + t.Setenv(k, v) + } + require.Equal(t, tt.expected, expandEnvURL(tt.url)) + }) + } +} + func TestInitialize(t *testing.T) { options := Options{Provider: TypeInMemory} provider := FromOptions(options) From 25d2dd0bc99926126f7c1a707348f8a2feb5c591 Mon Sep 17 00:00:00 2001 From: Sylvain Niles Date: Mon, 22 Jun 2026 20:47:30 -0700 Subject: [PATCH 2/7] feat(cli): add Terraform recipe state backup and restore Terraform recipes store their state in Kubernetes Secrets (the Terraform "kubernetes" backend) in the radius-system namespace, not in the Radius PostgreSQL databases. On an ephemeral control plane those Secrets are destroyed on teardown, so a second deploy of the same Terraform-backed resource in a later run plans against an empty backend and either fails or orphans cloud resources. Add a pkg/cli/tfstate package that exports the Secrets labelled tfstate=true to a state directory and restores them into a fresh cluster before any deploy runs. The label selector also captures the chunked tfstate-{workspace}-{suffix}-{index} Secrets the backend creates for large state. Server-managed fields are stripped on backup, and restore is idempotent (create-or-update). Covered by unit tests using the client-go fake clientset. Relates to #8096 Signed-off-by: Sylvain Niles --- .../2026-06-repo-radius-state-storage.md | 6 + pkg/cli/tfstate/tfstate.go | 231 ++++++++++++++++++ pkg/cli/tfstate/tfstate_test.go | 157 ++++++++++++ 3 files changed, 394 insertions(+) create mode 100644 pkg/cli/tfstate/tfstate.go create mode 100644 pkg/cli/tfstate/tfstate_test.go diff --git a/eng/design-notes/2026-06-repo-radius-state-storage.md b/eng/design-notes/2026-06-repo-radius-state-storage.md index 927d97d3c87..ad1582775db 100644 --- a/eng/design-notes/2026-06-repo-radius-state-storage.md +++ b/eng/design-notes/2026-06-repo-radius-state-storage.md @@ -136,6 +136,12 @@ the `radius-system` namespace into the same state worktree as the PostgreSQL dum pushed in the same atomic operation. On startup, after the cluster is ready and **before any deploy**, the Secrets are restored into the namespace. +Secrets are selected by the `tfstate=true` label that the Terraform Kubernetes backend applies to +every state Secret, rather than by name. This automatically captures the additional +`tfstate-{workspace}-{suffix}-{index}` Secrets that the backend creates when chunking large +state. The Lease resources the backend uses for locking are intentionally **not** backed up; they +are ephemeral and irrelevant across runs. + This mirrors the existing PostgreSQL backup flow exactly (same worktree, same commit/push, same semaphore) and is the minimal change that closes Gap 1. diff --git a/pkg/cli/tfstate/tfstate.go b/pkg/cli/tfstate/tfstate.go new file mode 100644 index 00000000000..48cdef94121 --- /dev/null +++ b/pkg/cli/tfstate/tfstate.go @@ -0,0 +1,231 @@ +/* +Copyright 2023 The Radius Authors. + +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 tfstate backs up and restores Terraform recipe state across ephemeral Radius +// control planes. +// +// Terraform recipes store their state in Kubernetes Secrets (the Terraform "kubernetes" +// backend), not in the Radius PostgreSQL databases. Those Secrets live in the radius-system +// namespace and are labelled "tfstate=true" by the backend. When the Radius control plane runs +// on an ephemeral cluster (for example a k3d cluster inside a CI runner), those Secrets are +// destroyed on teardown. Without backing them up, a second deploy of the same Terraform-backed +// resource in a later run plans from an empty backend and either fails or orphans cloud +// resources. +// +// This package exports those Secrets to a state directory (the same directory used for the +// PostgreSQL dumps) and restores them into a fresh cluster before any deploy runs. +package tfstate + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/radius-project/radius/pkg/cli/kubernetes" + "github.com/radius-project/radius/pkg/ucp/ucplog" + corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + k8s "k8s.io/client-go/kubernetes" +) + +const ( + // DefaultNamespace is the Kubernetes namespace where the Radius control plane and its + // Terraform state Secrets are installed. + DefaultNamespace = "radius-system" + + // LabelSelector matches the Secrets that the Terraform Kubernetes backend creates for recipe + // state. The backend labels every state Secret with "tfstate=true". + // https://developer.hashicorp.com/terraform/language/settings/backends/kubernetes + LabelSelector = "tfstate=true" + + // SubDir is the directory, relative to the state directory, where Terraform state Secrets are + // written. It keeps the Terraform backups separate from the PostgreSQL dumps in the same tree. + SubDir = "tfstate" +) + +// Client backs up and restores Terraform recipe state stored as Kubernetes Secrets. +type Client struct { + clientset k8s.Interface + namespace string +} + +// NewClient creates a Client backed by the supplied Kubernetes clientset and namespace. +func NewClient(clientset k8s.Interface, namespace string) *Client { + return &Client{clientset: clientset, namespace: namespace} +} + +// NewClientForContext builds a Client from a kubeconfig context name, targeting the given +// namespace. +func NewClientForContext(kubeContext, namespace string) (*Client, error) { + clientset, _, err := kubernetes.NewClientset(kubeContext) + if err != nil { + return nil, fmt.Errorf("failed to create kubernetes client: %w", err) + } + + return NewClient(clientset, namespace), nil +} + +// Backup writes every Terraform state Secret in the namespace to /tfstate/.json. +// Existing files in the target directory are removed first so that a Secret deleted since the +// previous backup does not linger and get restored. +func (c *Client) Backup(ctx context.Context, stateDir string) error { + logger := ucplog.FromContextOrDiscard(ctx) + + secrets, err := c.clientset.CoreV1().Secrets(c.namespace).List(ctx, metav1.ListOptions{ + LabelSelector: LabelSelector, + }) + if err != nil { + return fmt.Errorf("failed to list Terraform state secrets: %w", err) + } + + outDir := filepath.Join(stateDir, SubDir) + // Start from a clean directory so removed secrets are not resurrected on restore. + if err := os.RemoveAll(outDir); err != nil { + return fmt.Errorf("failed to clear Terraform state directory %q: %w", outDir, err) + } + if err := os.MkdirAll(outDir, 0o755); err != nil { + return fmt.Errorf("failed to create Terraform state directory %q: %w", outDir, err) + } + + for i := range secrets.Items { + secret := sanitize(&secrets.Items[i]) + + data, err := json.MarshalIndent(secret, "", " ") + if err != nil { + return fmt.Errorf("failed to serialize Terraform state secret %q: %w", secret.Name, err) + } + + outPath := filepath.Join(outDir, secret.Name+".json") + if err := os.WriteFile(outPath, data, 0o644); err != nil { + return fmt.Errorf("failed to write Terraform state file %q: %w", outPath, err) + } + + logger.Info("Backed up Terraform state secret", "secret", secret.Name, "file", outPath) + } + + logger.Info("Terraform state backup complete", "count", len(secrets.Items), "stateDir", outDir) + return nil +} + +// HasBackup reports whether any Terraform state backup files exist in the state directory. +func HasBackup(stateDir string) bool { + entries, err := os.ReadDir(filepath.Join(stateDir, SubDir)) + if err != nil { + return false + } + + for _, entry := range entries { + if !entry.IsDir() && strings.HasSuffix(entry.Name(), ".json") { + return true + } + } + + return false +} + +// Restore re-creates the Terraform state Secrets from /tfstate into the namespace. +// It is idempotent: a Secret that already exists is updated in place. Restore must run before +// any deploy so that Terraform recipes plan against the restored backend. +func (c *Client) Restore(ctx context.Context, stateDir string) error { + logger := ucplog.FromContextOrDiscard(ctx) + + inDir := filepath.Join(stateDir, SubDir) + entries, err := os.ReadDir(inDir) + if err != nil { + if os.IsNotExist(err) { + logger.Info("No Terraform state backup found, skipping restore", "stateDir", inDir) + return nil + } + return fmt.Errorf("failed to read Terraform state directory %q: %w", inDir, err) + } + + restored := 0 + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".json") { + continue + } + + path := filepath.Join(inDir, entry.Name()) + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("failed to read Terraform state file %q: %w", path, err) + } + + var secret corev1.Secret + if err := json.Unmarshal(data, &secret); err != nil { + return fmt.Errorf("failed to parse Terraform state file %q: %w", path, err) + } + + // Force the namespace to the target in case the backup came from a differently-named one. + secret.Namespace = c.namespace + + if err := c.applySecret(ctx, &secret); err != nil { + return err + } + + logger.Info("Restored Terraform state secret", "secret", secret.Name) + restored++ + } + + logger.Info("Terraform state restore complete", "count", restored) + return nil +} + +// applySecret creates the Secret, or updates it in place if it already exists. +func (c *Client) applySecret(ctx context.Context, secret *corev1.Secret) error { + secrets := c.clientset.CoreV1().Secrets(c.namespace) + + _, err := secrets.Create(ctx, secret, metav1.CreateOptions{}) + if err == nil { + return nil + } + if !k8serrors.IsAlreadyExists(err) { + return fmt.Errorf("failed to create Terraform state secret %q: %w", secret.Name, err) + } + + // The Secret already exists; update it in place. Carry over the current resourceVersion, + // which the API server requires for updates. + existing, getErr := secrets.Get(ctx, secret.Name, metav1.GetOptions{}) + if getErr != nil { + return fmt.Errorf("failed to read existing Terraform state secret %q: %w", secret.Name, getErr) + } + + secret.ResourceVersion = existing.ResourceVersion + if _, err := secrets.Update(ctx, secret, metav1.UpdateOptions{}); err != nil { + return fmt.Errorf("failed to update Terraform state secret %q: %w", secret.Name, err) + } + + return nil +} + +// sanitize returns a copy of the Secret with server-managed fields cleared so that it can be +// re-created cleanly in a different cluster. +func sanitize(secret *corev1.Secret) *corev1.Secret { + cleaned := secret.DeepCopy() + cleaned.ResourceVersion = "" + cleaned.UID = "" + cleaned.Generation = 0 + cleaned.CreationTimestamp = metav1.Time{} + cleaned.DeletionTimestamp = nil + cleaned.ManagedFields = nil + cleaned.OwnerReferences = nil + cleaned.SelfLink = "" + return cleaned +} diff --git a/pkg/cli/tfstate/tfstate_test.go b/pkg/cli/tfstate/tfstate_test.go new file mode 100644 index 00000000000..f0601432bbe --- /dev/null +++ b/pkg/cli/tfstate/tfstate_test.go @@ -0,0 +1,157 @@ +/* +Copyright 2023 The Radius Authors. + +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 tfstate + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func tfstateSecret(name string, data map[string][]byte) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: DefaultNamespace, + Labels: map[string]string{"tfstate": "true"}, + ResourceVersion: "12345", + UID: "abcde-uid", + }, + Data: data, + } +} + +func Test_Backup_WritesLabelledSecretsOnly(t *testing.T) { + clientset := fake.NewSimpleClientset( + tfstateSecret("tfstate-default-aaa", map[string][]byte{"tfstate": []byte("state-a")}), + tfstateSecret("tfstate-default-bbb", map[string][]byte{"tfstate": []byte("state-b")}), + // A secret without the tfstate label must be ignored. + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "database-secret", Namespace: DefaultNamespace}, + Data: map[string][]byte{"POSTGRES_PASSWORD": []byte("nope")}, + }, + ) + + stateDir := t.TempDir() + client := NewClient(clientset, DefaultNamespace) + + err := client.Backup(context.Background(), stateDir) + require.NoError(t, err) + + entries, err := os.ReadDir(filepath.Join(stateDir, SubDir)) + require.NoError(t, err) + require.Len(t, entries, 2) + require.FileExists(t, filepath.Join(stateDir, SubDir, "tfstate-default-aaa.json")) + require.FileExists(t, filepath.Join(stateDir, SubDir, "tfstate-default-bbb.json")) + require.NoFileExists(t, filepath.Join(stateDir, SubDir, "database-secret.json")) +} + +func Test_Backup_ClearsStaleFiles(t *testing.T) { + stateDir := t.TempDir() + staleDir := filepath.Join(stateDir, SubDir) + require.NoError(t, os.MkdirAll(staleDir, 0o755)) + stalePath := filepath.Join(staleDir, "tfstate-default-deleted.json") + require.NoError(t, os.WriteFile(stalePath, []byte("{}"), 0o644)) + + clientset := fake.NewSimpleClientset( + tfstateSecret("tfstate-default-live", map[string][]byte{"tfstate": []byte("live")}), + ) + client := NewClient(clientset, DefaultNamespace) + + err := client.Backup(context.Background(), stateDir) + require.NoError(t, err) + + require.NoFileExists(t, stalePath) + require.FileExists(t, filepath.Join(staleDir, "tfstate-default-live.json")) +} + +func Test_Backup_NoSecrets(t *testing.T) { + clientset := fake.NewSimpleClientset() + stateDir := t.TempDir() + client := NewClient(clientset, DefaultNamespace) + + err := client.Backup(context.Background(), stateDir) + require.NoError(t, err) + require.False(t, HasBackup(stateDir)) +} + +func Test_HasBackup(t *testing.T) { + stateDir := t.TempDir() + require.False(t, HasBackup(stateDir)) + + dir := filepath.Join(stateDir, SubDir) + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.False(t, HasBackup(stateDir), "empty tfstate dir is not a backup") + + require.NoError(t, os.WriteFile(filepath.Join(dir, "tfstate-default-aaa.json"), []byte("{}"), 0o644)) + require.True(t, HasBackup(stateDir)) +} + +func Test_BackupRestore_RoundTrip(t *testing.T) { + original := tfstateSecret("tfstate-default-aaa", map[string][]byte{"tfstate": []byte("round-trip-state")}) + source := fake.NewSimpleClientset(original) + stateDir := t.TempDir() + + require.NoError(t, NewClient(source, DefaultNamespace).Backup(context.Background(), stateDir)) + + // Restore into a fresh, empty cluster. + target := fake.NewSimpleClientset() + require.NoError(t, NewClient(target, DefaultNamespace).Restore(context.Background(), stateDir)) + + restored, err := target.CoreV1().Secrets(DefaultNamespace).Get(context.Background(), "tfstate-default-aaa", metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, []byte("round-trip-state"), restored.Data["tfstate"]) + require.Equal(t, "true", restored.Labels["tfstate"]) + // Server-managed fields must not be carried over from the source cluster. + require.Empty(t, restored.UID) +} + +func Test_Restore_UpdatesExistingSecret(t *testing.T) { + stateDir := t.TempDir() + source := fake.NewSimpleClientset( + tfstateSecret("tfstate-default-aaa", map[string][]byte{"tfstate": []byte("new-state")}), + ) + require.NoError(t, NewClient(source, DefaultNamespace).Backup(context.Background(), stateDir)) + + // Target already has a secret of the same name with stale data. + target := fake.NewSimpleClientset( + tfstateSecret("tfstate-default-aaa", map[string][]byte{"tfstate": []byte("old-state")}), + ) + require.NoError(t, NewClient(target, DefaultNamespace).Restore(context.Background(), stateDir)) + + updated, err := target.CoreV1().Secrets(DefaultNamespace).Get(context.Background(), "tfstate-default-aaa", metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, []byte("new-state"), updated.Data["tfstate"]) +} + +func Test_Restore_NoBackupIsNoOp(t *testing.T) { + target := fake.NewSimpleClientset() + stateDir := t.TempDir() + + err := NewClient(target, DefaultNamespace).Restore(context.Background(), stateDir) + require.NoError(t, err) + + secrets, err := target.CoreV1().Secrets(DefaultNamespace).List(context.Background(), metav1.ListOptions{}) + require.NoError(t, err) + require.Empty(t, secrets.Items) +} From 288c23c81b901e7f2b14fd5a9a36a223412650be Mon Sep 17 00:00:00 2001 From: Sylvain Niles Date: Mon, 22 Jun 2026 21:12:06 -0700 Subject: [PATCH 3/7] feat(cli): add kind-agnostic rad startup and rad shutdown Adds the two commands that back up and restore all durable Radius state across an ephemeral control plane, plus the end-to-end lifecycle test. The commands operate on the current workspace's Kubernetes context like any other command. They do not create or delete clusters and do not install Radius; cluster lifecycle is the caller's responsibility. There is no dedicated workspace kind. - pkg/cli/pgbackup: control-plane PostgreSQL backup/restore via "kubectl exec pg_dump/psql". - pkg/cli/gitstate: persists the state directory to a git orphan branch (radius-state) in an isolated worktree. CommitAndPush fails loudly when a remote is configured (a failed backup push would otherwise be silent data loss) and tolerates the no-remote local/test case. - rad shutdown: backs up the control-plane databases and the Terraform state Secrets, then commits and pushes them. - rad startup: waits for the database, then restores the control-plane databases and the Terraform state Secrets. The lifecycle test (test/functional-portable/statestore) installs Radius with database.enabled=true, deploys a Terraform-backed resource, shuts down, uninstalls, reinstalls, starts up, and deploys an update to the same resource -- the cross-run path that fails when Terraform state is lost. It is destructive and requires a cluster, so it is skipped unless RADIUS_STATE_E2E is set. Relates to #8096 Signed-off-by: Sylvain Niles --- cmd/rad/cmd/root.go | 8 + .../2026-06-repo-radius-state-storage.md | 67 ++++-- pkg/cli/cmd/shutdown/shutdown.go | 159 +++++++++++++ pkg/cli/cmd/shutdown/shutdown_test.go | 127 ++++++++++ pkg/cli/cmd/shutdown/stateclient.go | 54 +++++ pkg/cli/cmd/startup/startup.go | 158 +++++++++++++ pkg/cli/cmd/startup/startup_test.go | 124 ++++++++++ pkg/cli/cmd/startup/stateclient.go | 61 +++++ pkg/cli/gitstate/gitstate.go | 218 ++++++++++++++++++ pkg/cli/gitstate/gitstate_test.go | 160 +++++++++++++ pkg/cli/pgbackup/pgbackup.go | 210 +++++++++++++++++ .../noncloud/statestore_lifecycle_test.go | 160 +++++++++++++ 12 files changed, 1483 insertions(+), 23 deletions(-) create mode 100644 pkg/cli/cmd/shutdown/shutdown.go create mode 100644 pkg/cli/cmd/shutdown/shutdown_test.go create mode 100644 pkg/cli/cmd/shutdown/stateclient.go create mode 100644 pkg/cli/cmd/startup/startup.go create mode 100644 pkg/cli/cmd/startup/startup_test.go create mode 100644 pkg/cli/cmd/startup/stateclient.go create mode 100644 pkg/cli/gitstate/gitstate.go create mode 100644 pkg/cli/gitstate/gitstate_test.go create mode 100644 pkg/cli/pgbackup/pgbackup.go create mode 100644 test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go diff --git a/cmd/rad/cmd/root.go b/cmd/rad/cmd/root.go index 40ad25c907c..ec14ef6d878 100644 --- a/cmd/rad/cmd/root.go +++ b/cmd/rad/cmd/root.go @@ -86,6 +86,8 @@ import ( "github.com/radius-project/radius/pkg/cli/cmd/rollback" rollback_kubernetes "github.com/radius-project/radius/pkg/cli/cmd/rollback/kubernetes" "github.com/radius-project/radius/pkg/cli/cmd/run" + cmd_shutdown "github.com/radius-project/radius/pkg/cli/cmd/shutdown" + cmd_startup "github.com/radius-project/radius/pkg/cli/cmd/startup" "github.com/radius-project/radius/pkg/cli/cmd/uninstall" uninstall_kubernetes "github.com/radius-project/radius/pkg/cli/cmd/uninstall/kubernetes" "github.com/radius-project/radius/pkg/cli/cmd/upgrade" @@ -361,6 +363,12 @@ func initSubCommands() { wirePreviewSubcommand(initCmd, previewInitCmd) RootCmd.AddCommand(initCmd) + startupCmd, _ := cmd_startup.NewCommand(framework) + RootCmd.AddCommand(startupCmd) + + shutdownCmd, _ := cmd_shutdown.NewCommand(framework) + RootCmd.AddCommand(shutdownCmd) + envCreateCmd, _ := env_create.NewCommand(framework) previewCreateCmd, _ := env_create_preview.NewCommand(framework) wirePreviewSubcommand(envCreateCmd, previewCreateCmd) diff --git a/eng/design-notes/2026-06-repo-radius-state-storage.md b/eng/design-notes/2026-06-repo-radius-state-storage.md index ad1582775db..3e53de2bc54 100644 --- a/eng/design-notes/2026-06-repo-radius-state-storage.md +++ b/eng/design-notes/2026-06-repo-radius-state-storage.md @@ -85,13 +85,25 @@ The prototype's `Push` treated a failed `git push` as a non-fatal warning. That for advisory sentinel files but means a failed **state backup** push is silent data loss. The durability requirement is that a backup either succeeds or fails the command loudly. -### Gap 4 — The deploy lock was advisory, not a real mutex +## Design decisions -The prototype's `.deploy-lock` was read from the local worktree and then pushed, so two runners -could both observe "no lock" and both proceed. A correct lock must use an atomic -compare-and-swap. +### Decision 0 — `rad startup` / `rad shutdown` are workspace-kind-agnostic -## Design decisions +The prototype introduced a dedicated `github` workspace kind and folded state restore into +`rad init --kind github`. **There is no `github` workspace kind in this design.** State backup and +restore are exposed as two ordinary commands — `rad shutdown` and `rad startup` — that operate on +the current workspace's Kubernetes context like any other command. They do not create or delete +clusters and do not install Radius; cluster lifecycle and `rad install` are orthogonal and remain +the caller's responsibility (the CI workflow, or a test harness). + +This keeps the commands usable in any context — local development, CI, or a self-hosted +install — and makes them directly testable: a test can install Radius, deploy, `rad shutdown`, +destroy the cluster, recreate and reinstall, `rad startup`, and deploy again. + +* `rad shutdown` — back up all durable Radius state (PostgreSQL databases + Terraform state + Secrets) for the workspace's context to the configured state location. +* `rad startup` — restore all durable Radius state from the configured state location into the + workspace's context, after the control plane is running. ### Decision 1 — Storage backend: git orphan branch for v1, OCI/GHCR deferred to v2 @@ -142,23 +154,19 @@ every state Secret, rather than by name. This automatically captures the additio state. The Lease resources the backend uses for locking are intentionally **not** backed up; they are ephemeral and irrelevant across runs. -This mirrors the existing PostgreSQL backup flow exactly (same worktree, same commit/push, same -semaphore) and is the minimal change that closes Gap 1. +This mirrors the PostgreSQL backup flow exactly (same state directory, same commit/push) and is +the minimal change that closes Gap 1. A v2 alternative — switching the Terraform backend from `kubernetes` to the `pg` backend pointed at the same PostgreSQL instance, so a single `pg_dump` captures both stores — is recorded below. It is deferred because it requires backend credential injection and per-recipe state isolation work beyond v1 scope. -### Decision 4 — Durability and locking hardening +### Decision 4 — Durability hardening -* The **final state-backup push must fail the command** on error (Gap 3). Advisory sentinel - pushes remain best-effort. -* The deploy lock uses git's own compare-and-swap: acquisition commits the lock file and pushes; - a non-fast-forward **push rejection is a failed acquisition** (Gap 4). On rejection, fetch and - apply the existing `RunID`/`RunAttempt` takeover logic for same-run retries. The lock is keyed - by **GitHub Environment name** so deploys to different environments do not serialize against - each other. +* The **state-backup push must fail the command** on error when a remote is configured (Gap 3). + When no remote is configured (local development, tests), the commit on the orphan branch is the + durable store and the absence of a remote is not an error. * A small checksum manifest accompanies the dumps so a corrupt restore **fails closed** rather than silently starting from an empty state. @@ -171,26 +179,39 @@ work beyond v1 scope. and Terraform state collapse into a single dump and a single restore. * **Client-side envelope encryption** (age/sops) of state artifacts before they leave the cluster, since both PostgreSQL data and Terraform state can contain secrets. +* **Concurrency control**: a real mutex (for example an OCI tag compare-and-swap, or a git + push-rejection lock) to serialize concurrent deploys to the same shared state. The product spec + lists this as an open question; it is not required for the single-writer lifecycle delivered + here. ## Delivery plan | PR | Contents | Closes | |----|----------|--------| -| PR 1 | PostgreSQL enablement fixes: Dynamic RP `database.enabled` conditional, init-db configmap, `POSTGRES_DB` value, `factory.go` env-var substitution, Helm chart tests | Gap 2 | -| PR 2 | Terraform-state Secret backup/restore wired into the existing state-worktree flow | Gap 1 | -| PR 3 | Durability hardening: loud backup-push failure, push-rejection CAS lock, checksum manifest | Gaps 3, 4 | +| PR 1 | PostgreSQL enablement fixes: RP `database.enabled` conditionals, init-db configmap, `POSTGRES_DB` value, `factory.go` env-var substitution, Helm chart tests | Gap 2 | +| PR 2 | Terraform-state Secret backup/restore (`pkg/cli/tfstate`) | Gap 1 | +| PR 3 | `pkg/cli/pgbackup` + `pkg/cli/gitstate` (orphan-branch state directory with loud push), the kind-agnostic `rad startup` / `rad shutdown` commands, and the end-to-end functional test | Gap 3, Decision 0 | ## Test plan * **Unit**: Helm chart conditional rendering (PR 1); Terraform-state export/import round-trip with - a fake Kubernetes client (PR 2); push-failure and lock-contention paths (PR 3). -* **Functional**: an end-to-end lifecycle that deploys a **Terraform-backed** resource, shuts - down, restarts, and deploys an **update** to the same resource — the path that exposes Gap 1. + a fake Kubernetes client (PR 2); state-directory commit/push behaviour (PR 3). +* **Functional**: an end-to-end lifecycle that exercises every state path: + 1. Install Radius with `database.enabled=true` on a cluster. + 2. `rad deploy` a **Terraform-backed** resource (creates control-plane state + a Terraform + state Secret). + 3. `rad shutdown` (back up both stores). + 4. Destroy the control-plane state (delete the cluster, or uninstall) to simulate ephemeral + teardown. + 5. Recreate the control plane and `rad startup` (restore both stores). + 6. `rad deploy` an **update** to the same resource — must plan from the restored Terraform + state and succeed without errors or orphaned cloud resources. This is the path that exposes + Gap 1. ## Security * State (PostgreSQL dumps and Terraform Secrets, which may contain secret values) is pushed to a branch in the repository. For v1 the repository **must be private**; this constraint is removed in v2 by the encrypted OCI backend. -* Git credentials use the GitHub Actions token; pushing the state branch requires - `contents: write`. +* Git credentials use the ambient git configuration of the checkout (in CI, the token provided by + the checkout step); pushing the state branch requires write access to the repository. diff --git a/pkg/cli/cmd/shutdown/shutdown.go b/pkg/cli/cmd/shutdown/shutdown.go new file mode 100644 index 00000000000..921363c84d3 --- /dev/null +++ b/pkg/cli/cmd/shutdown/shutdown.go @@ -0,0 +1,159 @@ +/* +Copyright 2023 The Radius Authors. + +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 shutdown implements the `rad shutdown` command, which backs up all durable Radius state +// (PostgreSQL control-plane databases and Terraform state Secrets) for the current workspace to a +// git orphan branch. It is the counterpart of `rad startup`. +// +// The command does not delete clusters or uninstall Radius; cluster lifecycle is the caller's +// responsibility. This keeps the command usable in any context and independent of any particular +// workspace kind. +package shutdown + +import ( + "context" + "fmt" + + "github.com/spf13/cobra" + + "github.com/radius-project/radius/pkg/cli" + "github.com/radius-project/radius/pkg/cli/clierrors" + "github.com/radius-project/radius/pkg/cli/cmd/commonflags" + "github.com/radius-project/radius/pkg/cli/framework" + "github.com/radius-project/radius/pkg/cli/gitstate" + "github.com/radius-project/radius/pkg/cli/output" + "github.com/radius-project/radius/pkg/cli/pgbackup" + "github.com/radius-project/radius/pkg/cli/workspaces" +) + +// NewCommand creates an instance of the `rad shutdown` command and runner. +func NewCommand(factory framework.Factory) (*cobra.Command, framework.Runner) { + runner := NewRunner(factory) + + cmd := &cobra.Command{ + Use: "shutdown", + Short: "Back up Radius state and prepare for shutdown", + Long: `Back up all durable Radius state for the current workspace. + +Dumps the control-plane PostgreSQL databases and exports the Terraform recipe state Secrets, +commits them to the radius-state git orphan branch, and pushes to the remote when one is +configured. The state can be restored into a fresh control plane with 'rad startup'. + +This command does not delete the cluster or uninstall Radius.`, + Example: ` +# Back up state for the current workspace +rad shutdown + +# Back up state for a specific workspace +rad shutdown --workspace my-workspace`, + Args: cobra.NoArgs, + RunE: framework.RunCommand(runner), + } + + commonflags.AddWorkspaceFlag(cmd) + + return cmd, runner +} + +// worktreeHandle decouples Run from the concrete gitstate worktree so the command can be tested +// without performing real git operations. +type worktreeHandle struct { + path string + commitAndPush func(ctx context.Context, message string) error + remove func(ctx context.Context) +} + +// Runner is the runner implementation for the `rad shutdown` command. +type Runner struct { + ConfigHolder *framework.ConfigHolder + Output output.Interface + Workspace *workspaces.Workspace + StateClient StateBackupClient + + // openWorktree opens (or creates) the state worktree. Overridable in tests. + openWorktree func(ctx context.Context) (worktreeHandle, error) +} + +// NewRunner creates a new Runner for the `rad shutdown` command. +func NewRunner(factory framework.Factory) *Runner { + r := &Runner{ + ConfigHolder: factory.GetConfigHolder(), + Output: factory.GetOutput(), + StateClient: NewStateBackupClient(), + } + r.openWorktree = defaultOpenWorktree + return r +} + +// defaultOpenWorktree opens the real gitstate worktree and adapts it to worktreeHandle. +func defaultOpenWorktree(ctx context.Context) (worktreeHandle, error) { + w, err := gitstate.OpenOrCreate(ctx, gitstate.BranchName()) + if err != nil { + return worktreeHandle{}, err + } + return worktreeHandle{ + path: w.Path, + commitAndPush: w.CommitAndPush, + remove: w.Remove, + }, nil +} + +// Validate resolves the workspace and ensures it targets a Kubernetes cluster. +func (r *Runner) Validate(cmd *cobra.Command, args []string) error { + workspace, err := cli.RequireWorkspace(cmd, r.ConfigHolder.Config) + if err != nil { + return err + } + + if _, ok := workspace.KubernetesContext(); !ok { + return clierrors.Message("The 'rad shutdown' command requires a workspace connected to a Kubernetes cluster. Workspace %q is not connected to a Kubernetes cluster.", workspace.Name) + } + + r.Workspace = workspace + return nil +} + +// Run backs up the control-plane and Terraform state, then commits and pushes it. +func (r *Runner) Run(ctx context.Context) error { + kubeContext, ok := r.Workspace.KubernetesContext() + if !ok { + return clierrors.Message("Could not determine the Kubernetes context for workspace %q.", r.Workspace.Name) + } + + wt, err := r.openWorktree(ctx) + if err != nil { + return fmt.Errorf("failed to open state worktree: %w", err) + } + defer wt.remove(ctx) + + r.Output.LogInfo("Backing up control-plane databases...") + if err := r.StateClient.BackupDatabases(ctx, kubeContext, pgbackup.DefaultNamespace, wt.path); err != nil { + return fmt.Errorf("failed to back up control-plane databases: %w", err) + } + + r.Output.LogInfo("Backing up Terraform recipe state...") + if err := r.StateClient.BackupTerraform(ctx, kubeContext, pgbackup.DefaultNamespace, wt.path); err != nil { + return fmt.Errorf("failed to back up Terraform state: %w", err) + } + + r.Output.LogInfo("Committing state to branch %q...", gitstate.BranchName()) + if err := wt.commitAndPush(ctx, "radius: shutdown backup"); err != nil { + return fmt.Errorf("failed to commit and push state: %w", err) + } + + r.Output.LogInfo("State backed up successfully.") + return nil +} diff --git a/pkg/cli/cmd/shutdown/shutdown_test.go b/pkg/cli/cmd/shutdown/shutdown_test.go new file mode 100644 index 00000000000..dc1858d11a7 --- /dev/null +++ b/pkg/cli/cmd/shutdown/shutdown_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2023 The Radius Authors. + +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 shutdown + +import ( + "context" + "errors" + "testing" + + "github.com/radius-project/radius/pkg/cli/output" + "github.com/radius-project/radius/pkg/cli/workspaces" + "github.com/stretchr/testify/require" +) + +// fakeStateBackupClient records calls and returns canned errors. +type fakeStateBackupClient struct { + backupDBErr error + backupTFErr error + dbCalled bool + tfCalled bool + stateDirSeen string +} + +func (f *fakeStateBackupClient) BackupDatabases(ctx context.Context, kubeContext, namespace, stateDir string) error { + f.dbCalled = true + f.stateDirSeen = stateDir + return f.backupDBErr +} + +func (f *fakeStateBackupClient) BackupTerraform(ctx context.Context, kubeContext, namespace, stateDir string) error { + f.tfCalled = true + return f.backupTFErr +} + +func kubernetesWorkspace() *workspaces.Workspace { + return &workspaces.Workspace{ + Name: "test", + Connection: map[string]any{ + "kind": workspaces.KindKubernetes, + "context": "k3d-test", + }, + } +} + +func newTestRunner(t *testing.T, ws *workspaces.Workspace, client *fakeStateBackupClient) (*Runner, *fakeWorktree) { + t.Helper() + wt := &fakeWorktree{path: t.TempDir()} + r := &Runner{ + Output: &output.MockOutput{}, + Workspace: ws, + StateClient: client, + openWorktree: func(ctx context.Context) (worktreeHandle, error) { + return worktreeHandle{ + path: wt.path, + commitAndPush: wt.commitAndPush, + remove: wt.remove, + }, nil + }, + } + return r, wt +} + +// fakeWorktree records commit/remove invocations. +type fakeWorktree struct { + path string + committed bool + removed bool + commitMessage string + commitErr error +} + +func (w *fakeWorktree) commitAndPush(ctx context.Context, message string) error { + w.committed = true + w.commitMessage = message + return w.commitErr +} + +func (w *fakeWorktree) remove(ctx context.Context) { w.removed = true } + +func Test_Run_BacksUpBothStoresAndCommits(t *testing.T) { + client := &fakeStateBackupClient{} + r, wt := newTestRunner(t, kubernetesWorkspace(), client) + + err := r.Run(context.Background()) + require.NoError(t, err) + + require.True(t, client.dbCalled, "databases should be backed up") + require.True(t, client.tfCalled, "terraform state should be backed up") + require.Equal(t, wt.path, client.stateDirSeen, "backup must target the worktree path") + require.True(t, wt.committed, "state should be committed and pushed") + require.True(t, wt.removed, "worktree should be removed") +} + +func Test_Run_DatabaseBackupFailureStopsAndStillRemovesWorktree(t *testing.T) { + client := &fakeStateBackupClient{backupDBErr: errors.New("pg_dump boom")} + r, wt := newTestRunner(t, kubernetesWorkspace(), client) + + err := r.Run(context.Background()) + require.ErrorContains(t, err, "pg_dump boom") + require.False(t, client.tfCalled, "terraform backup should not run after database failure") + require.False(t, wt.committed, "nothing should be committed on failure") + require.True(t, wt.removed, "worktree must still be removed via defer") +} + +func Test_Run_CommitFailureIsReturned(t *testing.T) { + client := &fakeStateBackupClient{} + r, wt := newTestRunner(t, kubernetesWorkspace(), client) + wt.commitErr = errors.New("push rejected") + + err := r.Run(context.Background()) + require.ErrorContains(t, err, "push rejected") + require.True(t, wt.removed) +} diff --git a/pkg/cli/cmd/shutdown/stateclient.go b/pkg/cli/cmd/shutdown/stateclient.go new file mode 100644 index 00000000000..b3b17911a68 --- /dev/null +++ b/pkg/cli/cmd/shutdown/stateclient.go @@ -0,0 +1,54 @@ +/* +Copyright 2023 The Radius Authors. + +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 shutdown + +import ( + "context" + + "github.com/radius-project/radius/pkg/cli/pgbackup" + "github.com/radius-project/radius/pkg/cli/tfstate" +) + +// StateBackupClient backs up the durable Radius state for a Kubernetes context. It wraps the +// pgbackup and tfstate packages so the command can be unit tested without a cluster. +type StateBackupClient interface { + // BackupDatabases dumps the control-plane PostgreSQL databases into stateDir. + BackupDatabases(ctx context.Context, kubeContext, namespace, stateDir string) error + + // BackupTerraform exports the Terraform state Secrets into stateDir. + BackupTerraform(ctx context.Context, kubeContext, namespace, stateDir string) error +} + +// defaultStateBackupClient is the production implementation. +type defaultStateBackupClient struct{} + +// NewStateBackupClient returns the production StateBackupClient. +func NewStateBackupClient() StateBackupClient { + return defaultStateBackupClient{} +} + +func (defaultStateBackupClient) BackupDatabases(ctx context.Context, kubeContext, namespace, stateDir string) error { + return pgbackup.Backup(ctx, kubeContext, namespace, stateDir) +} + +func (defaultStateBackupClient) BackupTerraform(ctx context.Context, kubeContext, namespace, stateDir string) error { + client, err := tfstate.NewClientForContext(kubeContext, namespace) + if err != nil { + return err + } + return client.Backup(ctx, stateDir) +} diff --git a/pkg/cli/cmd/startup/startup.go b/pkg/cli/cmd/startup/startup.go new file mode 100644 index 00000000000..36e4ca377ca --- /dev/null +++ b/pkg/cli/cmd/startup/startup.go @@ -0,0 +1,158 @@ +/* +Copyright 2023 The Radius Authors. + +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 startup implements the `rad startup` command, which restores durable Radius state +// (PostgreSQL control-plane databases and Terraform state Secrets) previously saved by +// `rad shutdown` into the current workspace's running control plane. +// +// The command assumes Radius is already installed and running on the target cluster; it does not +// create clusters or install Radius. This keeps it usable in any context and independent of any +// particular workspace kind. +package startup + +import ( + "context" + "fmt" + + "github.com/spf13/cobra" + + "github.com/radius-project/radius/pkg/cli" + "github.com/radius-project/radius/pkg/cli/clierrors" + "github.com/radius-project/radius/pkg/cli/cmd/commonflags" + "github.com/radius-project/radius/pkg/cli/framework" + "github.com/radius-project/radius/pkg/cli/gitstate" + "github.com/radius-project/radius/pkg/cli/output" + "github.com/radius-project/radius/pkg/cli/pgbackup" + "github.com/radius-project/radius/pkg/cli/workspaces" +) + +// NewCommand creates an instance of the `rad startup` command and runner. +func NewCommand(factory framework.Factory) (*cobra.Command, framework.Runner) { + runner := NewRunner(factory) + + cmd := &cobra.Command{ + Use: "startup", + Short: "Restore Radius state after startup", + Long: `Restore durable Radius state for the current workspace. + +Opens the radius-state git orphan branch, waits for the control-plane PostgreSQL instance to be +ready, restores the control-plane databases, and re-creates the Terraform recipe state Secrets. +Run this after Radius is installed on a fresh cluster to resume from the state saved by +'rad shutdown'. + +This command does not create the cluster or install Radius.`, + Example: ` +# Restore state for the current workspace +rad startup + +# Restore state for a specific workspace +rad startup --workspace my-workspace`, + Args: cobra.NoArgs, + RunE: framework.RunCommand(runner), + } + + commonflags.AddWorkspaceFlag(cmd) + + return cmd, runner +} + +// worktreeHandle decouples Run from the concrete gitstate worktree so the command can be tested +// without performing real git operations. +type worktreeHandle struct { + path string + remove func(ctx context.Context) +} + +// Runner is the runner implementation for the `rad startup` command. +type Runner struct { + ConfigHolder *framework.ConfigHolder + Output output.Interface + Workspace *workspaces.Workspace + StateClient StateRestoreClient + + // openWorktree opens (or creates) the state worktree. Overridable in tests. + openWorktree func(ctx context.Context) (worktreeHandle, error) +} + +// NewRunner creates a new Runner for the `rad startup` command. +func NewRunner(factory framework.Factory) *Runner { + r := &Runner{ + ConfigHolder: factory.GetConfigHolder(), + Output: factory.GetOutput(), + StateClient: NewStateRestoreClient(), + } + r.openWorktree = defaultOpenWorktree + return r +} + +// defaultOpenWorktree opens the real gitstate worktree and adapts it to worktreeHandle. +func defaultOpenWorktree(ctx context.Context) (worktreeHandle, error) { + w, err := gitstate.OpenOrCreate(ctx, gitstate.BranchName()) + if err != nil { + return worktreeHandle{}, err + } + return worktreeHandle{ + path: w.Path, + remove: w.Remove, + }, nil +} + +// Validate resolves the workspace and ensures it targets a Kubernetes cluster. +func (r *Runner) Validate(cmd *cobra.Command, args []string) error { + workspace, err := cli.RequireWorkspace(cmd, r.ConfigHolder.Config) + if err != nil { + return err + } + + if _, ok := workspace.KubernetesContext(); !ok { + return clierrors.Message("The 'rad startup' command requires a workspace connected to a Kubernetes cluster. Workspace %q is not connected to a Kubernetes cluster.", workspace.Name) + } + + r.Workspace = workspace + return nil +} + +// Run restores the control-plane and Terraform state from the state branch. +func (r *Runner) Run(ctx context.Context) error { + kubeContext, ok := r.Workspace.KubernetesContext() + if !ok { + return clierrors.Message("Could not determine the Kubernetes context for workspace %q.", r.Workspace.Name) + } + + wt, err := r.openWorktree(ctx) + if err != nil { + return fmt.Errorf("failed to open state worktree: %w", err) + } + defer wt.remove(ctx) + + r.Output.LogInfo("Waiting for control-plane database to be ready...") + if err := r.StateClient.WaitForDatabaseReady(ctx, kubeContext, pgbackup.DefaultNamespace); err != nil { + return fmt.Errorf("failed waiting for control-plane database: %w", err) + } + + r.Output.LogInfo("Restoring control-plane databases...") + if err := r.StateClient.RestoreDatabases(ctx, kubeContext, pgbackup.DefaultNamespace, wt.path); err != nil { + return fmt.Errorf("failed to restore control-plane databases: %w", err) + } + + r.Output.LogInfo("Restoring Terraform recipe state...") + if err := r.StateClient.RestoreTerraform(ctx, kubeContext, pgbackup.DefaultNamespace, wt.path); err != nil { + return fmt.Errorf("failed to restore Terraform state: %w", err) + } + + r.Output.LogInfo("State restored successfully.") + return nil +} diff --git a/pkg/cli/cmd/startup/startup_test.go b/pkg/cli/cmd/startup/startup_test.go new file mode 100644 index 00000000000..cf8b3573e87 --- /dev/null +++ b/pkg/cli/cmd/startup/startup_test.go @@ -0,0 +1,124 @@ +/* +Copyright 2023 The Radius Authors. + +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 startup + +import ( + "context" + "errors" + "testing" + + "github.com/radius-project/radius/pkg/cli/output" + "github.com/radius-project/radius/pkg/cli/workspaces" + "github.com/stretchr/testify/require" +) + +// fakeStateRestoreClient records calls and returns canned errors. +type fakeStateRestoreClient struct { + waitErr error + restoreDBErr error + restoreTFErr error + + waited bool + dbCalled bool + tfCalled bool + + order []string +} + +func (f *fakeStateRestoreClient) WaitForDatabaseReady(ctx context.Context, kubeContext, namespace string) error { + f.waited = true + f.order = append(f.order, "wait") + return f.waitErr +} + +func (f *fakeStateRestoreClient) RestoreDatabases(ctx context.Context, kubeContext, namespace, stateDir string) error { + f.dbCalled = true + f.order = append(f.order, "db") + return f.restoreDBErr +} + +func (f *fakeStateRestoreClient) RestoreTerraform(ctx context.Context, kubeContext, namespace, stateDir string) error { + f.tfCalled = true + f.order = append(f.order, "tf") + return f.restoreTFErr +} + +type fakeWorktree struct { + path string + removed bool +} + +func (w *fakeWorktree) remove(ctx context.Context) { w.removed = true } + +func kubernetesWorkspace() *workspaces.Workspace { + return &workspaces.Workspace{ + Name: "test", + Connection: map[string]any{ + "kind": workspaces.KindKubernetes, + "context": "k3d-test", + }, + } +} + +func newTestRunner(t *testing.T, client *fakeStateRestoreClient) (*Runner, *fakeWorktree) { + t.Helper() + wt := &fakeWorktree{path: t.TempDir()} + r := &Runner{ + Output: &output.MockOutput{}, + Workspace: kubernetesWorkspace(), + StateClient: client, + openWorktree: func(ctx context.Context) (worktreeHandle, error) { + return worktreeHandle{path: wt.path, remove: wt.remove}, nil + }, + } + return r, wt +} + +func Test_Run_RestoresInOrderWaitDatabaseTerraform(t *testing.T) { + client := &fakeStateRestoreClient{} + r, wt := newTestRunner(t, client) + + err := r.Run(context.Background()) + require.NoError(t, err) + + require.True(t, client.waited) + require.True(t, client.dbCalled) + require.True(t, client.tfCalled) + require.Equal(t, []string{"wait", "db", "tf"}, client.order, "must wait, then restore databases, then terraform") + require.True(t, wt.removed) +} + +func Test_Run_WaitFailureStopsBeforeRestore(t *testing.T) { + client := &fakeStateRestoreClient{waitErr: errors.New("db never ready")} + r, wt := newTestRunner(t, client) + + err := r.Run(context.Background()) + require.ErrorContains(t, err, "db never ready") + require.False(t, client.dbCalled) + require.False(t, client.tfCalled) + require.True(t, wt.removed) +} + +func Test_Run_DatabaseRestoreFailureStopsBeforeTerraform(t *testing.T) { + client := &fakeStateRestoreClient{restoreDBErr: errors.New("psql boom")} + r, wt := newTestRunner(t, client) + + err := r.Run(context.Background()) + require.ErrorContains(t, err, "psql boom") + require.False(t, client.tfCalled, "terraform restore must not run after database restore failure") + require.True(t, wt.removed) +} diff --git a/pkg/cli/cmd/startup/stateclient.go b/pkg/cli/cmd/startup/stateclient.go new file mode 100644 index 00000000000..6b813b7d1f0 --- /dev/null +++ b/pkg/cli/cmd/startup/stateclient.go @@ -0,0 +1,61 @@ +/* +Copyright 2023 The Radius Authors. + +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 startup + +import ( + "context" + + "github.com/radius-project/radius/pkg/cli/pgbackup" + "github.com/radius-project/radius/pkg/cli/tfstate" +) + +// StateRestoreClient restores the durable Radius state for a Kubernetes context. It wraps the +// pgbackup and tfstate packages so the command can be unit tested without a cluster. +type StateRestoreClient interface { + // WaitForDatabaseReady blocks until the control-plane PostgreSQL instance is ready. + WaitForDatabaseReady(ctx context.Context, kubeContext, namespace string) error + + // RestoreDatabases loads the control-plane PostgreSQL dumps from stateDir. + RestoreDatabases(ctx context.Context, kubeContext, namespace, stateDir string) error + + // RestoreTerraform re-creates the Terraform state Secrets from stateDir. + RestoreTerraform(ctx context.Context, kubeContext, namespace, stateDir string) error +} + +// defaultStateRestoreClient is the production implementation. +type defaultStateRestoreClient struct{} + +// NewStateRestoreClient returns the production StateRestoreClient. +func NewStateRestoreClient() StateRestoreClient { + return defaultStateRestoreClient{} +} + +func (defaultStateRestoreClient) WaitForDatabaseReady(ctx context.Context, kubeContext, namespace string) error { + return pgbackup.WaitForReady(ctx, kubeContext, namespace) +} + +func (defaultStateRestoreClient) RestoreDatabases(ctx context.Context, kubeContext, namespace, stateDir string) error { + return pgbackup.Restore(ctx, kubeContext, namespace, stateDir) +} + +func (defaultStateRestoreClient) RestoreTerraform(ctx context.Context, kubeContext, namespace, stateDir string) error { + client, err := tfstate.NewClientForContext(kubeContext, namespace) + if err != nil { + return err + } + return client.Restore(ctx, stateDir) +} diff --git a/pkg/cli/gitstate/gitstate.go b/pkg/cli/gitstate/gitstate.go new file mode 100644 index 00000000000..12d3dc722f9 --- /dev/null +++ b/pkg/cli/gitstate/gitstate.go @@ -0,0 +1,218 @@ +/* +Copyright 2023 The Radius Authors. + +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 gitstate persists Radius state to a git orphan branch. +// +// State files (PostgreSQL dumps and Terraform state) are stored on an orphan branch +// (default "radius-state") that shares no history with the application branches. The branch is +// checked out into a temporary git worktree, completely isolated from the application working +// tree, so state files never appear in the application checkout's "git status". +// +// Lifecycle: +// 1. OpenOrCreate returns a worktree; any files from the previous backup are present in Path. +// 2. Use Path as the state directory for backup/restore operations. +// 3. CommitAndPush commits everything in the worktree and pushes it (when a remote exists). +// 4. Always defer Remove to release the worktree. +package gitstate + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/radius-project/radius/pkg/ucp/ucplog" +) + +const ( + // DefaultBranch is the default orphan branch name for Radius state. + DefaultBranch = "radius-state" + + // branchEnvVar overrides the default branch name. It lets parallel tests use isolated + // branches without colliding. + branchEnvVar = "RADIUS_STATE_BRANCH" + + // remoteName is the git remote that state is pushed to when it is configured. + remoteName = "origin" + + // emptyTreeSHA is the well-known SHA of the empty tree, constant across all git repositories. + // It lets us create an orphan branch without touching the working tree. + emptyTreeSHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" +) + +// BranchName returns the state branch name, honoring the RADIUS_STATE_BRANCH environment +// variable and falling back to DefaultBranch. +func BranchName() string { + if v := os.Getenv(branchEnvVar); v != "" { + return v + } + return DefaultBranch +} + +// StateWorktree is a git worktree checked out to the state orphan branch in a temporary +// directory isolated from the application working tree. +type StateWorktree struct { + // Path is the absolute path of the worktree directory. Use it as the state directory for + // backup and restore operations. + Path string + branchName string + repoRoot string +} + +// OpenOrCreate opens a worktree for branchName, creating the orphan branch if it does not yet +// exist. When a remote holds the branch, its latest state is fetched first. Files from a previous +// backup are present in Path when this returns. The caller must defer Remove. +func OpenOrCreate(ctx context.Context, branchName string) (*StateWorktree, error) { + logger := ucplog.FromContextOrDiscard(ctx) + + root, err := repoRoot(ctx) + if err != nil { + return nil, fmt.Errorf("failed to determine git repo root: %w", err) + } + + // Pull the latest remote state if a remote with this branch exists, so the worktree reflects + // the most recent backup from another machine. + if hasRemote(ctx, root) { + _ = gitExecIn(ctx, root, "fetch", remoteName, branchName) + } + + if !branchExists(ctx, root, branchName) { + if remoteBranchExists(ctx, root, branchName) { + logger.Info("Creating local state branch from remote", "branch", branchName) + if err := gitExecIn(ctx, root, "branch", branchName, remoteName+"/"+branchName); err != nil { + return nil, fmt.Errorf("failed to create local branch %q from remote: %w", branchName, err) + } + } else { + logger.Info("Creating orphan state branch", "branch", branchName) + if err := createOrphanBranch(ctx, root, branchName); err != nil { + return nil, fmt.Errorf("failed to create orphan branch %q: %w", branchName, err) + } + } + } + + // git worktree add requires the target path to not exist; git creates it. + wtPath := filepath.Join(os.TempDir(), fmt.Sprintf("radius-state-%d", time.Now().UnixNano())) + logger.Info("Adding git worktree", "path", wtPath, "branch", branchName) + if err := gitExecIn(ctx, root, "worktree", "add", wtPath, branchName); err != nil { + return nil, fmt.Errorf("failed to add worktree: %w", err) + } + + return &StateWorktree{ + Path: wtPath, + branchName: branchName, + repoRoot: root, + }, nil +} + +// CommitAndPush stages everything in the worktree, commits it, and pushes it to the remote. +// +// The commit is the durable local store. The push is the durable remote store: when a remote is +// configured, a failed push fails the operation (state would otherwise be silently lost). When no +// remote is configured (local development, tests), the commit alone is sufficient and the missing +// remote is not an error. +func (w *StateWorktree) CommitAndPush(ctx context.Context, message string) error { + logger := ucplog.FromContextOrDiscard(ctx) + + if err := gitExecIn(ctx, w.Path, "add", "-A"); err != nil { + return fmt.Errorf("failed to stage state files: %w", err) + } + if err := gitExecIn(ctx, w.Path, "commit", "-m", message, "--allow-empty"); err != nil { + return fmt.Errorf("failed to commit state: %w", err) + } + + if !hasRemote(ctx, w.repoRoot) { + logger.Info("No git remote configured; state committed locally only", "branch", w.branchName) + return nil + } + + if err := gitExecIn(ctx, w.Path, "push", remoteName, w.branchName); err != nil { + return fmt.Errorf("failed to push state branch %q to %q: %w", w.branchName, remoteName, err) + } + + logger.Info("State committed and pushed", "branch", w.branchName) + return nil +} + +// Remove tears down the worktree entry. Always defer this after a successful OpenOrCreate. +func (w *StateWorktree) Remove(ctx context.Context) { + if err := gitExecIn(ctx, w.repoRoot, "worktree", "remove", "--force", w.Path); err != nil { + ucplog.FromContextOrDiscard(ctx).Info("Failed to remove git worktree", "path", w.Path, "error", err) + } +} + +// branchExists reports whether branchName exists locally in the repository at root. +func branchExists(ctx context.Context, root, branchName string) bool { + cmd := exec.CommandContext(ctx, "git", "rev-parse", "--verify", "refs/heads/"+branchName) + cmd.Dir = root + return cmd.Run() == nil +} + +// remoteBranchExists reports whether branchName exists on the remote. +func remoteBranchExists(ctx context.Context, root, branchName string) bool { + cmd := exec.CommandContext(ctx, "git", "rev-parse", "--verify", remoteName+"/"+branchName) + cmd.Dir = root + return cmd.Run() == nil +} + +// hasRemote reports whether a remote named origin is configured. +func hasRemote(ctx context.Context, root string) bool { + cmd := exec.CommandContext(ctx, "git", "remote", "get-url", remoteName) + cmd.Dir = root + return cmd.Run() == nil +} + +// createOrphanBranch creates an empty orphan branch using low-level git plumbing without touching +// the working tree or switching branches. +func createOrphanBranch(ctx context.Context, root, branchName string) error { + cmd := exec.CommandContext(ctx, "git", "commit-tree", emptyTreeSHA, "-m", "radius: init state branch") + cmd.Dir = root + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("git commit-tree: %w: %s", err, stderr.String()) + } + + commitSHA := strings.TrimSpace(stdout.String()) + return gitExecIn(ctx, root, "update-ref", "refs/heads/"+branchName, commitSHA) +} + +// repoRoot returns the absolute path to the root of the current git repository. +func repoRoot(ctx context.Context) (string, error) { + cmd := exec.CommandContext(ctx, "git", "rev-parse", "--show-toplevel") + var stdout bytes.Buffer + cmd.Stdout = &stdout + if err := cmd.Run(); err != nil { + return "", err + } + return strings.TrimSpace(stdout.String()), nil +} + +// gitExecIn runs a git command with its working directory set to dir. +func gitExecIn(ctx context.Context, dir string, args ...string) error { + cmd := exec.CommandContext(ctx, "git", args...) + cmd.Dir = dir + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, stderr.String()) + } + return nil +} diff --git a/pkg/cli/gitstate/gitstate_test.go b/pkg/cli/gitstate/gitstate_test.go new file mode 100644 index 00000000000..0279da852c9 --- /dev/null +++ b/pkg/cli/gitstate/gitstate_test.go @@ -0,0 +1,160 @@ +/* +Copyright 2023 The Radius Authors. + +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 gitstate + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// newTestRepo creates a throwaway git repository with one commit on the main branch and returns +// its root. All git operations in gitstate run relative to a checkout, so tests need a real repo. +func newTestRepo(t *testing.T) string { + t.Helper() + root := t.TempDir() + + run := func(args ...string) { + cmd := exec.Command("git", args...) + cmd.Dir = root + out, err := cmd.CombinedOutput() + require.NoErrorf(t, err, "git %v failed: %s", args, out) + } + + run("init", "-b", "main") + run("config", "user.email", "test@example.com") + run("config", "user.name", "test") + require.NoError(t, os.WriteFile(filepath.Join(root, "README.md"), []byte("test"), 0o644)) + run("add", "-A") + run("commit", "-m", "initial") + + return root +} + +// inDir runs fn with the process working directory temporarily set to dir. gitstate derives the +// repo root from the current directory, so tests must run from inside the repo. +func inDir(t *testing.T, dir string, fn func()) { + t.Helper() + orig, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) + defer func() { + require.NoError(t, os.Chdir(orig)) + }() + fn() +} + +func Test_BranchName_DefaultAndOverride(t *testing.T) { + require.Equal(t, DefaultBranch, BranchName()) + + t.Setenv(branchEnvVar, "radius-state-test-123") + require.Equal(t, "radius-state-test-123", BranchName()) +} + +func Test_OpenOrCreate_CreatesOrphanBranchAndIsolatesState(t *testing.T) { + root := newTestRepo(t) + + inDir(t, root, func() { + ctx := context.Background() + wt, err := OpenOrCreate(ctx, "radius-state-test") + require.NoError(t, err) + defer wt.Remove(ctx) + + // The worktree path is outside the application checkout. + require.NotEmpty(t, wt.Path) + require.NoDirExists(t, filepath.Join(root, "radius-state-test")) + + // Writing a state file and committing must not touch the application working tree. + require.NoError(t, os.WriteFile(filepath.Join(wt.Path, "ucp.sql"), []byte("dump"), 0o644)) + require.NoError(t, wt.CommitAndPush(ctx, "radius: backup")) + + status, err := exec.Command("git", "-C", root, "status", "--porcelain").CombinedOutput() + require.NoError(t, err) + require.Empty(t, string(status), "application working tree must stay clean") + }) +} + +func Test_OpenOrCreate_RestoresPreviousState(t *testing.T) { + root := newTestRepo(t) + + inDir(t, root, func() { + ctx := context.Background() + + // First session writes state and commits it. + wt1, err := OpenOrCreate(ctx, "radius-state-test") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(wt1.Path, "ucp.sql"), []byte("first-dump"), 0o644)) + require.NoError(t, wt1.CommitAndPush(ctx, "radius: backup 1")) + wt1.Remove(ctx) + + // Second session must see the committed state. + wt2, err := OpenOrCreate(ctx, "radius-state-test") + require.NoError(t, err) + defer wt2.Remove(ctx) + + data, err := os.ReadFile(filepath.Join(wt2.Path, "ucp.sql")) + require.NoError(t, err) + require.Equal(t, "first-dump", string(data)) + }) +} + +func Test_CommitAndPush_NoRemoteIsNotAnError(t *testing.T) { + root := newTestRepo(t) + + inDir(t, root, func() { + ctx := context.Background() + wt, err := OpenOrCreate(ctx, "radius-state-test") + require.NoError(t, err) + defer wt.Remove(ctx) + + require.NoError(t, os.WriteFile(filepath.Join(wt.Path, "ucp.sql"), []byte("dump"), 0o644)) + // No "origin" remote is configured; commit succeeds and the missing remote is tolerated. + require.NoError(t, wt.CommitAndPush(ctx, "radius: backup")) + }) +} + +func Test_CommitAndPush_PushesToRemote(t *testing.T) { + // Bare remote that the working repo pushes to. + remote := t.TempDir() + cmd := exec.Command("git", "init", "--bare", remote) + out, err := cmd.CombinedOutput() + require.NoErrorf(t, err, "git init --bare failed: %s", out) + + root := newTestRepo(t) + addRemote := exec.Command("git", "-C", root, "remote", "add", "origin", remote) + out, err = addRemote.CombinedOutput() + require.NoErrorf(t, err, "git remote add failed: %s", out) + + inDir(t, root, func() { + ctx := context.Background() + wt, err := OpenOrCreate(ctx, "radius-state-test") + require.NoError(t, err) + defer wt.Remove(ctx) + + require.NoError(t, os.WriteFile(filepath.Join(wt.Path, "ucp.sql"), []byte("dump"), 0o644)) + require.NoError(t, wt.CommitAndPush(ctx, "radius: backup")) + + // The branch must now exist on the remote. + lsRemote, err := exec.Command("git", "-C", root, "ls-remote", "origin", "radius-state-test").CombinedOutput() + require.NoError(t, err) + require.Contains(t, string(lsRemote), "radius-state-test") + }) +} diff --git a/pkg/cli/pgbackup/pgbackup.go b/pkg/cli/pgbackup/pgbackup.go new file mode 100644 index 00000000000..dc9056ea4ae --- /dev/null +++ b/pkg/cli/pgbackup/pgbackup.go @@ -0,0 +1,210 @@ +/* +Copyright 2023 The Radius Authors. + +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 pgbackup backs up and restores the Radius control-plane PostgreSQL databases. +// +// The Radius control plane stores resource data and deployment history in three logical +// PostgreSQL databases (ucp, applications_rp, dynamic_rp) served by a single in-cluster +// PostgreSQL instance. When the control plane runs on an ephemeral cluster, that data is lost on +// teardown. This package dumps each database to a plain SQL file (via "kubectl exec ... pg_dump") +// and restores them (via "kubectl exec ... psql") so state survives across runs. +package pgbackup + +import ( + "bytes" + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/radius-project/radius/pkg/ucp/ucplog" +) + +const ( + // DefaultNamespace is the Kubernetes namespace where Radius (and its PostgreSQL instance) + // is installed. + DefaultNamespace = "radius-system" + + // PodLabelSelector is the label selector for the PostgreSQL pod deployed by the Helm chart. + PodLabelSelector = "app.kubernetes.io/name=database" + + // PostgresUser is the superuser used for backup/restore operations. It can read and write + // every logical database regardless of which per-RP user owns the data. + PostgresUser = "radius" +) + +// Databases is the list of PostgreSQL databases that hold control-plane state. +var Databases = []string{"ucp", "applications_rp", "dynamic_rp"} + +// HasBackup reports whether a SQL dump exists for every database in the state directory. +func HasBackup(stateDir string) bool { + for _, db := range Databases { + path := filepath.Join(stateDir, db+".sql") + if _, err := os.Stat(path); err != nil { + return false + } + } + + return true +} + +// Backup dumps each PostgreSQL database to a plain SQL file in the state directory. +func Backup(ctx context.Context, kubeContext, namespace, stateDir string) error { + logger := ucplog.FromContextOrDiscard(ctx) + + if err := os.MkdirAll(stateDir, 0o755); err != nil { + return fmt.Errorf("failed to create state directory %q: %w", stateDir, err) + } + + podName, err := getPodName(ctx, kubeContext, namespace) + if err != nil { + return err + } + + for _, db := range Databases { + logger.Info("Backing up database", "database", db, "stateDir", stateDir) + + cmd := exec.CommandContext(ctx, "kubectl", + "--context", kubeContext, + "-n", namespace, + "exec", podName, "--", + "pg_dump", + "-U", PostgresUser, + "--format=plain", + "--clean", + "--if-exists", + db, + ) + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + return fmt.Errorf("failed to back up database %q: %w: %s", db, err, stderr.String()) + } + + outPath := filepath.Join(stateDir, db+".sql") + if err := os.WriteFile(outPath, stdout.Bytes(), 0o644); err != nil { + return fmt.Errorf("failed to write backup file %q: %w", outPath, err) + } + + logger.Info("Database backup complete", "database", db, "file", outPath) + } + + return nil +} + +// Restore loads each SQL dump from the state directory into its PostgreSQL database. The dumps +// are produced with --clean --if-exists, so restore is idempotent. +func Restore(ctx context.Context, kubeContext, namespace, stateDir string) error { + logger := ucplog.FromContextOrDiscard(ctx) + + if !HasBackup(stateDir) { + logger.Info("No database backup found, skipping restore", "stateDir", stateDir) + return nil + } + + podName, err := getPodName(ctx, kubeContext, namespace) + if err != nil { + return err + } + + for _, db := range Databases { + sqlPath := filepath.Join(stateDir, db+".sql") + logger.Info("Restoring database", "database", db, "file", sqlPath) + + sqlData, err := os.ReadFile(sqlPath) + if err != nil { + return fmt.Errorf("failed to read backup file %q: %w", sqlPath, err) + } + + cmd := exec.CommandContext(ctx, "kubectl", + "--context", kubeContext, + "-n", namespace, + "exec", "-i", podName, "--", + "psql", + "-U", PostgresUser, + "-d", db, + ) + + cmd.Stdin = bytes.NewReader(sqlData) + var stderr bytes.Buffer + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + return fmt.Errorf("failed to restore database %q: %w: %s", db, err, stderr.String()) + } + + logger.Info("Database restore complete", "database", db) + } + + return nil +} + +// WaitForReady blocks until the PostgreSQL pod reports ready, using "kubectl wait". +func WaitForReady(ctx context.Context, kubeContext, namespace string) error { + logger := ucplog.FromContextOrDiscard(ctx) + logger.Info("Waiting for PostgreSQL pod to be ready") + + cmd := exec.CommandContext(ctx, "kubectl", + "--context", kubeContext, + "-n", namespace, + "wait", + "--for=condition=ready", + "pod", + "-l", PodLabelSelector, + "--timeout=120s", + ) + + var stderr bytes.Buffer + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + return fmt.Errorf("timed out waiting for PostgreSQL pod: %w: %s", err, stderr.String()) + } + + logger.Info("PostgreSQL pod is ready") + return nil +} + +// getPodName resolves the name of the PostgreSQL pod via its label selector. +func getPodName(ctx context.Context, kubeContext, namespace string) (string, error) { + cmd := exec.CommandContext(ctx, "kubectl", + "--context", kubeContext, + "-n", namespace, + "get", "pods", + "-l", PodLabelSelector, + "-o", "jsonpath={.items[0].metadata.name}", + ) + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + return "", fmt.Errorf("failed to find PostgreSQL pod: %w: %s", err, stderr.String()) + } + + podName := strings.TrimSpace(stdout.String()) + if podName == "" { + return "", fmt.Errorf("no PostgreSQL pod found with selector %q in namespace %q", PodLabelSelector, namespace) + } + + return podName, nil +} diff --git a/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go b/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go new file mode 100644 index 00000000000..83a533d6bb6 --- /dev/null +++ b/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go @@ -0,0 +1,160 @@ +/* +Copyright 2023 The Radius Authors. + +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 statestore contains the end-to-end lifecycle test for `rad shutdown` / `rad startup`. +// +// Unlike the standard functional tests (which assume an already-running Radius install), this test +// installs Radius with a PostgreSQL state backend, deploys a Terraform-backed resource, backs up +// and restores all durable state across a full uninstall/reinstall cycle, and then deploys an +// UPDATE to the same resource. The update is the path that fails when Terraform state is lost: it +// proves that both the control-plane databases and the Terraform state Secrets survived the +// teardown. +// +// The test is destructive (it uninstalls and reinstalls Radius on the target cluster) and requires +// a real cluster plus the Terraform recipe module server, so it does not run as part of the normal +// functional suite. It is skipped unless RADIUS_STATE_E2E is set to a truthy value. +package statestore + +import ( + "context" + "os" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/radius-project/radius/test" + "github.com/radius-project/radius/test/functional-portable/corerp" + "github.com/radius-project/radius/test/radcli" + "github.com/radius-project/radius/test/testutil" +) + +const ( + stateNamespace = "radius-system" + secretPrefix = "tfstate-default-" + + // redisRecipeTemplate is the Terraform recipe fixture shared with the corerp recipe tests. + redisRecipeTemplate = "../../corerp/noncloud/resources/testdata/corerp-resources-terraform-redis.bicep" +) + +// shouldRun reports whether the destructive lifecycle test has been opted into. +func shouldRun(t *testing.T) { + t.Helper() + v, _ := strconv.ParseBool(os.Getenv("RADIUS_STATE_E2E")) + if !v { + t.Skip("set RADIUS_STATE_E2E=1 to run the destructive rad startup/shutdown lifecycle test") + } +} + +// installRadius installs Radius with the PostgreSQL state backend enabled. +func installRadius(ctx context.Context, t *testing.T, cli *radcli.CLI) { + t.Helper() + out, err := cli.RunCommand(ctx, []string{"install", "kubernetes", "--set", "database.enabled=true"}) + require.NoErrorf(t, err, "rad install failed: %s", out) +} + +// uninstallRadius removes Radius and its state so the next install starts from an empty control +// plane, simulating an ephemeral teardown. +func uninstallRadius(ctx context.Context, t *testing.T, cli *radcli.CLI) { + t.Helper() + out, err := cli.RunCommand(ctx, []string{"uninstall", "kubernetes", "--purge"}) + require.NoErrorf(t, err, "rad uninstall failed: %s", out) +} + +// Test_StateStore_ShutdownStartup_TerraformCrossDeploy exercises every state path: +// install, deploy a Terraform resource, shut down (backup), tear down, start up (restore), then +// deploy an update to the same resource. +func Test_StateStore_ShutdownStartup_TerraformCrossDeploy(t *testing.T) { + shouldRun(t) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) + defer cancel() + + cli := radcli.NewCLI(t, "") + + appName := "statestore-tf-redis-app" + envName := "statestore-tf-redis-env" + resourceName := "statestore-tf-redis" + redisCacheName := "statestore-redis" + + resourceID := "/planes/radius/local/resourcegroups/kind-radius/providers/Applications.Core/extenders/" + resourceName + secretSuffix, err := corerp.GetSecretSuffix(resourceID, envName, appName) + require.NoError(t, err) + secretName := secretPrefix + secretSuffix + + k8s := test.NewTestOptions(t).K8sClient + + deploy := func() { + out, derr := cli.RunCommand(ctx, []string{ + "deploy", redisRecipeTemplate, + "--parameters", testutil.GetTerraformRecipeModuleServerURL(), + "--parameters", "appName=" + appName, + "--parameters", "envName=" + envName, + "--parameters", "resourceName=" + resourceName, + "--parameters", "redisCacheName=" + redisCacheName, + }) + require.NoErrorf(t, derr, "rad deploy failed: %s", out) + } + + secretExists := func() bool { + _, getErr := k8s.CoreV1().Secrets(stateNamespace).Get(ctx, secretName, metav1.GetOptions{}) + return getErr == nil + } + + // 1. Fresh install with PostgreSQL state backend. + installRadius(ctx, t, cli) + t.Cleanup(func() { uninstallRadius(context.Background(), t, cli) }) + + // 2. Deploy the Terraform-backed resource. This creates control-plane state and a Terraform + // state Secret. + deploy() + require.True(t, secretExists(), "Terraform state secret should exist after the first deploy") + + // 3. Back up all durable state. + out, err := cli.RunCommand(ctx, []string{"shutdown"}) + require.NoErrorf(t, err, "rad shutdown failed: %s", out) + + // 4. Tear the control plane down completely (ephemeral teardown). + uninstallRadius(ctx, t, cli) + + // 5. Reinstall onto a fresh, empty control plane. + installRadius(ctx, t, cli) + require.False(t, secretExists(), "Terraform state secret must be gone after reinstall (teardown was real)") + + // 6. Restore the saved state. + out, err = cli.RunCommand(ctx, []string{"startup"}) + require.NoErrorf(t, err, "rad startup failed: %s", out) + + // 7. Both stores must be restored: the Terraform state Secret is back, and the control-plane + // resource is queryable again. + require.True(t, secretExists(), "Terraform state secret should be restored by rad startup") + resourceShow, err := cli.RunCommand(ctx, []string{"resource", "show", "Applications.Core/extenders", resourceName}) + require.NoErrorf(t, err, "resource should be restored into the control plane: %s", resourceShow) + + // 8. Cross-deploy: deploy an update to the same resource. With Terraform state restored this + // plans incrementally and succeeds; without it, Terraform would plan from an empty backend + // and either error or orphan cloud resources. + deploy() + + // The same single Terraform state Secret must still back the resource (no duplicate created). + secrets, err := k8s.CoreV1().Secrets(stateNamespace).List(ctx, metav1.ListOptions{ + LabelSelector: "tfstate=true", + }) + require.NoError(t, err) + require.Len(t, secrets.Items, 1, "exactly one Terraform state secret should exist after the cross-deploy") +} From 2548d77695efe0a669d8b8be6ae3fd88c76d3bfc Mon Sep 17 00:00:00 2001 From: Sylvain Niles Date: Mon, 22 Jun 2026 21:20:42 -0700 Subject: [PATCH 4/7] docs: note cluster-create/deploy workflow dependency for the state e2e test Signed-off-by: Sylvain Niles --- eng/design-notes/2026-06-repo-radius-state-storage.md | 10 ++++++++++ .../statestore/noncloud/statestore_lifecycle_test.go | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/eng/design-notes/2026-06-repo-radius-state-storage.md b/eng/design-notes/2026-06-repo-radius-state-storage.md index 3e53de2bc54..491e42da5da 100644 --- a/eng/design-notes/2026-06-repo-radius-state-storage.md +++ b/eng/design-notes/2026-06-repo-radius-state-storage.md @@ -208,6 +208,16 @@ work beyond v1 scope. state and succeed without errors or orphaned cloud resources. This is the path that exposes Gap 1. +> **Test dependency — cluster lifecycle and deploy.** `rad startup` / `rad shutdown` are +> deliberately kind-agnostic: they back up and restore state but do **not** create clusters or +> install Radius. The end-to-end lifecycle test therefore depends on the separate Repo Radius +> workflow code (in flight) that creates the ephemeral cluster, installs Radius, and runs the +> deploy. Until that lands, the functional test (`test/functional-portable/statestore`) drives the +> cluster install/uninstall itself and is gated behind the `RADIUS_STATE_E2E` environment +> variable so it does not run in the normal suite. Once the shared cluster-create + deploy +> workflow is merged, the test's install/uninstall helpers should be re-pointed at that code +> rather than duplicating it. + ## Security * State (PostgreSQL dumps and Terraform Secrets, which may contain secret values) is pushed to a diff --git a/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go b/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go index 83a533d6bb6..d2c5607059f 100644 --- a/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go +++ b/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go @@ -26,6 +26,12 @@ limitations under the License. // The test is destructive (it uninstalls and reinstalls Radius on the target cluster) and requires // a real cluster plus the Terraform recipe module server, so it does not run as part of the normal // functional suite. It is skipped unless RADIUS_STATE_E2E is set to a truthy value. +// +// Test dependency: rad startup/shutdown do not create clusters or install Radius. This test +// currently drives the cluster install/uninstall itself (installRadius/uninstallRadius below). It +// is expected to depend on the separate Repo Radius workflow code (in flight) that creates the +// ephemeral cluster, installs Radius, and runs the deploy. Once that lands, re-point the helpers +// at the shared workflow code instead of duplicating the install/uninstall steps here. package statestore import ( From afe3cbd6c689aae088000bec3725f70901021c2a Mon Sep 17 00:00:00 2001 From: Sylvain Niles Date: Mon, 22 Jun 2026 21:53:33 -0700 Subject: [PATCH 5/7] fix: address PR review feedback and CI failures - gitstate: treat a git fetch failure as fatal when the state branch exists on the remote, so a transient network/credential error cannot silently restore stale or empty state. - gitstate: inject a fallback git identity (Radius ) for the state commit when the repo has none configured, so rad shutdown works in fresh CI environments. - tfstate: drop the write to the deprecated Secret.SelfLink field (staticcheck SA1019), which was failing the lint check. - databaseprovider test: set MISSING explicitly so the env-var expansion test is not flaky on runners that happen to have it set. - statestore e2e: assert at least one tfstate Secret exists rather than exactly one, since the backend may shard large state across multiple Secrets. - design note: mark the checksum manifest as future work (not implemented in this delivery) and fix a spelling (behaviour -> behavior). - .cspellignore: add Sylvain. Signed-off-by: Sylvain Niles --- .cspellignore | 1 + .../2026-06-repo-radius-state-storage.md | 9 ++-- pkg/cli/gitstate/gitstate.go | 42 ++++++++++++++++--- pkg/cli/tfstate/tfstate.go | 1 - .../databaseprovider/storageprovider_test.go | 1 + .../noncloud/statestore_lifecycle_test.go | 5 ++- 6 files changed, 48 insertions(+), 11 deletions(-) diff --git a/.cspellignore b/.cspellignore index 2c2ac6234f7..64518f8a6ed 100644 --- a/.cspellignore +++ b/.cspellignore @@ -393,6 +393,7 @@ Statestores StaticCredential Subramanian Swappable +Sylvain Sylvainsf Symlinked SystemData diff --git a/eng/design-notes/2026-06-repo-radius-state-storage.md b/eng/design-notes/2026-06-repo-radius-state-storage.md index 491e42da5da..aff03e71270 100644 --- a/eng/design-notes/2026-06-repo-radius-state-storage.md +++ b/eng/design-notes/2026-06-repo-radius-state-storage.md @@ -167,8 +167,11 @@ work beyond v1 scope. * The **state-backup push must fail the command** on error when a remote is configured (Gap 3). When no remote is configured (local development, tests), the commit on the orphan branch is the durable store and the absence of a remote is not an error. -* A small checksum manifest accompanies the dumps so a corrupt restore **fails closed** rather - than silently starting from an empty state. +* When the state branch exists on the remote, opening the worktree fetches it and treats a fetch + failure as fatal, so a transient network or credential error cannot silently restore stale or + empty state. +* *(Future work)* A checksum manifest accompanying the dumps so a corrupt restore **fails closed** + rather than silently starting from an empty state. This is not implemented in this delivery. ## v2 direction (not in this delivery) @@ -195,7 +198,7 @@ work beyond v1 scope. ## Test plan * **Unit**: Helm chart conditional rendering (PR 1); Terraform-state export/import round-trip with - a fake Kubernetes client (PR 2); state-directory commit/push behaviour (PR 3). + a fake Kubernetes client (PR 2); state-directory commit/push behavior (PR 3). * **Functional**: an end-to-end lifecycle that exercises every state path: 1. Install Radius with `database.enabled=true` on a cluster. 2. `rad deploy` a **Terraform-backed** resource (creates control-plane state + a Terraform diff --git a/pkg/cli/gitstate/gitstate.go b/pkg/cli/gitstate/gitstate.go index 12d3dc722f9..f82f6cfa5bd 100644 --- a/pkg/cli/gitstate/gitstate.go +++ b/pkg/cli/gitstate/gitstate.go @@ -87,10 +87,13 @@ func OpenOrCreate(ctx context.Context, branchName string) (*StateWorktree, error return nil, fmt.Errorf("failed to determine git repo root: %w", err) } - // Pull the latest remote state if a remote with this branch exists, so the worktree reflects - // the most recent backup from another machine. - if hasRemote(ctx, root) { - _ = gitExecIn(ctx, root, "fetch", remoteName, branchName) + // If the state branch exists on the remote, fetching it must succeed. Silently falling back + // to an empty local branch here would make a later restore use stale or empty state, so a + // fetch failure (network, credentials) is fatal when the remote is known to hold the branch. + if hasRemote(ctx, root) && remoteHasBranch(ctx, root, branchName) { + if err := gitExecIn(ctx, root, "fetch", remoteName, branchName); err != nil { + return nil, fmt.Errorf("failed to fetch state branch %q from %q: %w", branchName, remoteName, err) + } } if !branchExists(ctx, root, branchName) { @@ -133,7 +136,7 @@ func (w *StateWorktree) CommitAndPush(ctx context.Context, message string) error if err := gitExecIn(ctx, w.Path, "add", "-A"); err != nil { return fmt.Errorf("failed to stage state files: %w", err) } - if err := gitExecIn(ctx, w.Path, "commit", "-m", message, "--allow-empty"); err != nil { + if err := gitExecIn(ctx, w.Path, commitArgs(ctx, w.Path, message)...); err != nil { return fmt.Errorf("failed to commit state: %w", err) } @@ -171,6 +174,35 @@ func remoteBranchExists(ctx context.Context, root, branchName string) bool { return cmd.Run() == nil } +// remoteHasBranch queries the remote directly (without relying on a prior fetch) to report +// whether branchName exists on it. This distinguishes "the branch exists remotely but we could +// not fetch it" (an error) from "the branch does not exist remotely yet" (a normal first run). +func remoteHasBranch(ctx context.Context, root, branchName string) bool { + cmd := exec.CommandContext(ctx, "git", "ls-remote", "--exit-code", "--heads", remoteName, branchName) + cmd.Dir = root + return cmd.Run() == nil +} + +// commitArgs builds the git arguments for committing the worktree, injecting a fallback identity +// when the repository has no user.name/user.email configured. Fresh CI environments frequently +// lack a git identity, which would otherwise make the commit (and therefore rad shutdown) fail +// even though the backup itself succeeded. +func commitArgs(ctx context.Context, dir, message string) []string { + var args []string + if !gitIdentityConfigured(ctx, dir) { + args = append(args, "-c", "user.name=Radius", "-c", "user.email=radius@radapp.io") + } + return append(args, "commit", "-m", message, "--allow-empty") +} + +// gitIdentityConfigured reports whether user.email is set for the repository at dir. +func gitIdentityConfigured(ctx context.Context, dir string) bool { + cmd := exec.CommandContext(ctx, "git", "config", "user.email") + cmd.Dir = dir + out, err := cmd.Output() + return err == nil && strings.TrimSpace(string(out)) != "" +} + // hasRemote reports whether a remote named origin is configured. func hasRemote(ctx context.Context, root string) bool { cmd := exec.CommandContext(ctx, "git", "remote", "get-url", remoteName) diff --git a/pkg/cli/tfstate/tfstate.go b/pkg/cli/tfstate/tfstate.go index 48cdef94121..286e911bee0 100644 --- a/pkg/cli/tfstate/tfstate.go +++ b/pkg/cli/tfstate/tfstate.go @@ -226,6 +226,5 @@ func sanitize(secret *corev1.Secret) *corev1.Secret { cleaned.DeletionTimestamp = nil cleaned.ManagedFields = nil cleaned.OwnerReferences = nil - cleaned.SelfLink = "" return cleaned } diff --git a/pkg/components/database/databaseprovider/storageprovider_test.go b/pkg/components/database/databaseprovider/storageprovider_test.go index 8729c537f0c..6e4db0a9b73 100644 --- a/pkg/components/database/databaseprovider/storageprovider_test.go +++ b/pkg/components/database/databaseprovider/storageprovider_test.go @@ -148,6 +148,7 @@ func Test_expandEnvURL(t *testing.T) { { name: "unset variable expands to empty string", url: "postgresql://ucp:${MISSING}@host:5432/db", + env: map[string]string{"MISSING": ""}, expected: "postgresql://ucp:@host:5432/db", }, } diff --git a/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go b/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go index d2c5607059f..a738b445ee2 100644 --- a/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go +++ b/test/functional-portable/statestore/noncloud/statestore_lifecycle_test.go @@ -157,10 +157,11 @@ func Test_StateStore_ShutdownStartup_TerraformCrossDeploy(t *testing.T) { // and either error or orphan cloud resources. deploy() - // The same single Terraform state Secret must still back the resource (no duplicate created). + // At least one Terraform state Secret must still back the resource. The backend may shard large + // state across multiple tfstate-* Secrets, so assert presence rather than an exact count. secrets, err := k8s.CoreV1().Secrets(stateNamespace).List(ctx, metav1.ListOptions{ LabelSelector: "tfstate=true", }) require.NoError(t, err) - require.Len(t, secrets.Items, 1, "exactly one Terraform state secret should exist after the cross-deploy") + require.GreaterOrEqual(t, len(secrets.Items), 1, "at least one Terraform state secret should exist after the cross-deploy") } From 4d2e0c6c8af4972086ba47fe72cd2d65fdc11c35 Mon Sep 17 00:00:00 2001 From: Sylvain Niles Date: Tue, 23 Jun 2026 10:00:00 -0700 Subject: [PATCH 6/7] feat(cli): quiesce the control plane around state restore rad startup runs after rad install, so the control-plane pods are already running and connected to PostgreSQL when state is restored. Restoring a pg_dump (DROP TABLE / CREATE TABLE) underneath those live connections can invalidate the providers' cached prepared statements (pgx default QueryExecModeCacheStatement; the resources table OID changes), and races the UCP initializer's boot-time writes. rad startup now scales the database-backed deployments (ucp, applications-rp, dynamic-rp) to zero before the restore and back to their previous replica counts afterward, via a new pkg/cli/controlplane package. This makes the restore atomic with respect to its consumers and ensures the providers establish fresh connection pools against the restored schema. The deployment engine and dashboard do not connect to PostgreSQL and are left running. The control plane is always scaled back up, including on a failed restore, and a deployment whose previous replica count was zero is restored to one. Adds unit tests for the scaler (fake clientset with a reconciling reactor that mirrors spec replicas to status) and updates the startup runner tests to assert the scale-down -> restore -> scale-up ordering. Records the decision in the state-storage design note. Signed-off-by: Sylvain Niles --- .../2026-06-repo-radius-state-storage.md | 27 +++ pkg/cli/cmd/startup/startup.go | 37 ++++ pkg/cli/cmd/startup/startup_test.go | 64 ++++++- pkg/cli/cmd/startup/stateclient.go | 19 ++ pkg/cli/controlplane/controlplane.go | 178 ++++++++++++++++++ pkg/cli/controlplane/controlplane_test.go | 141 ++++++++++++++ 6 files changed, 459 insertions(+), 7 deletions(-) create mode 100644 pkg/cli/controlplane/controlplane.go create mode 100644 pkg/cli/controlplane/controlplane_test.go diff --git a/eng/design-notes/2026-06-repo-radius-state-storage.md b/eng/design-notes/2026-06-repo-radius-state-storage.md index aff03e71270..bfe6b08533a 100644 --- a/eng/design-notes/2026-06-repo-radius-state-storage.md +++ b/eng/design-notes/2026-06-repo-radius-state-storage.md @@ -173,6 +173,33 @@ work beyond v1 scope. * *(Future work)* A checksum manifest accompanying the dumps so a corrupt restore **fails closed** rather than silently starting from an empty state. This is not implemented in this delivery. +### Decision 5 — Quiesce the control plane around the restore + +`rad startup` runs *after* `rad install`, so the control-plane pods (`ucp`, `applications-rp`, +`dynamic-rp`) are already running and connected to PostgreSQL when state is restored. Restoring a +`pg_dump` that uses `DROP TABLE` / `CREATE TABLE` underneath those live connections has two +problems: + +1. **Stale prepared statements.** The providers open `pgx` connection pools at startup, and pgx's + default `QueryExecModeCacheStatement` caches prepared statements per connection. Dropping and + recreating the `resources` table changes its OID, so an already-open pooled connection's cached + statement can fail with `cached plan must not change result type` (SQLSTATE `0A000`) or + `relation does not exist`. +2. **A write race.** The UCP initializer populates the fresh database with resource-provider + manifests at boot; restoring on top of a live system races those writes. + +`rad startup` therefore **scales the three database-backed deployments to zero, restores, then +scales them back to their previous replica counts** (`pkg/cli/controlplane`). This makes the +restore atomic with respect to its consumers and means the providers establish brand-new pools — +with no stale prepared statements — against the restored schema. The deployment engine and +dashboard do not connect to PostgreSQL directly and are left running. Components are always scaled +back up, including on a failed restore, and a deployment whose previous replica count was zero is +restored to one rather than left scaled down. + +A lighter alternative — restoring data-only with `TRUNCATE` instead of `--clean` to keep table +OIDs stable — was considered. It avoids the prepared-statement hazard but not the write race, so +the scale-to-zero approach was chosen as the one that addresses both. + ## v2 direction (not in this delivery) * **OCI/GHCR backend**: a pluggable storage backend that pushes an encrypted, content-addressed diff --git a/pkg/cli/cmd/startup/startup.go b/pkg/cli/cmd/startup/startup.go index 36e4ca377ca..b74320e920b 100644 --- a/pkg/cli/cmd/startup/startup.go +++ b/pkg/cli/cmd/startup/startup.go @@ -85,6 +85,9 @@ type Runner struct { // openWorktree opens (or creates) the state worktree. Overridable in tests. openWorktree func(ctx context.Context) (worktreeHandle, error) + + // newScaler builds the control-plane scaler for a context/namespace. Overridable in tests. + newScaler func(kubeContext, namespace string) (ControlPlaneScaler, error) } // NewRunner creates a new Runner for the `rad startup` command. @@ -95,6 +98,7 @@ func NewRunner(factory framework.Factory) *Runner { StateClient: NewStateRestoreClient(), } r.openWorktree = defaultOpenWorktree + r.newScaler = newScalerForContext return r } @@ -126,6 +130,11 @@ func (r *Runner) Validate(cmd *cobra.Command, args []string) error { } // Run restores the control-plane and Terraform state from the state branch. +// +// The database-backed control-plane deployments are scaled to zero before the restore and back up +// afterward. Restoring a pg_dump underneath live resource-provider connections would invalidate +// their cached prepared statements and race their writes; quiescing them makes the restore atomic +// with respect to its consumers without a separate restart step. func (r *Runner) Run(ctx context.Context) error { kubeContext, ok := r.Workspace.KubernetesContext() if !ok { @@ -138,6 +147,28 @@ func (r *Runner) Run(ctx context.Context) error { } defer wt.remove(ctx) + scaler, err := r.newScaler(kubeContext, pgbackup.DefaultNamespace) + if err != nil { + return fmt.Errorf("failed to initialise control-plane scaler: %w", err) + } + + r.Output.LogInfo("Scaling down control-plane components...") + saved, err := scaler.ScaleDown(ctx) + if err != nil { + return fmt.Errorf("failed to scale down the control plane: %w", err) + } + + // Ensure the control plane is brought back up even if a restore step fails. + scaledBackUp := false + defer func() { + if scaledBackUp { + return + } + if upErr := scaler.ScaleUp(ctx, saved); upErr != nil { + r.Output.LogInfo("Warning: failed to scale the control plane back up: %v", upErr) + } + }() + r.Output.LogInfo("Waiting for control-plane database to be ready...") if err := r.StateClient.WaitForDatabaseReady(ctx, kubeContext, pgbackup.DefaultNamespace); err != nil { return fmt.Errorf("failed waiting for control-plane database: %w", err) @@ -153,6 +184,12 @@ func (r *Runner) Run(ctx context.Context) error { return fmt.Errorf("failed to restore Terraform state: %w", err) } + r.Output.LogInfo("Scaling control-plane components back up...") + if err := scaler.ScaleUp(ctx, saved); err != nil { + return fmt.Errorf("failed to scale the control plane back up: %w", err) + } + scaledBackUp = true + r.Output.LogInfo("State restored successfully.") return nil } diff --git a/pkg/cli/cmd/startup/startup_test.go b/pkg/cli/cmd/startup/startup_test.go index cf8b3573e87..6e2eaf258ad 100644 --- a/pkg/cli/cmd/startup/startup_test.go +++ b/pkg/cli/cmd/startup/startup_test.go @@ -64,6 +64,35 @@ type fakeWorktree struct { func (w *fakeWorktree) remove(ctx context.Context) { w.removed = true } +// fakeScaler records scale operations and appends them to a shared order slice so tests can assert +// that the control plane is scaled down before any restore and back up afterward. +type fakeScaler struct { + scaleDownErr error + scaleUpErr error + downCalled bool + upCalled bool + order *[]string +} + +func (s *fakeScaler) ScaleDown(ctx context.Context) (map[string]int32, error) { + s.downCalled = true + if s.order != nil { + *s.order = append(*s.order, "scaledown") + } + if s.scaleDownErr != nil { + return nil, s.scaleDownErr + } + return map[string]int32{"ucp": 1}, nil +} + +func (s *fakeScaler) ScaleUp(ctx context.Context, saved map[string]int32) error { + s.upCalled = true + if s.order != nil { + *s.order = append(*s.order, "scaleup") + } + return s.scaleUpErr +} + func kubernetesWorkspace() *workspaces.Workspace { return &workspaces.Workspace{ Name: "test", @@ -74,9 +103,10 @@ func kubernetesWorkspace() *workspaces.Workspace { } } -func newTestRunner(t *testing.T, client *fakeStateRestoreClient) (*Runner, *fakeWorktree) { +func newTestRunner(t *testing.T, client *fakeStateRestoreClient) (*Runner, *fakeWorktree, *fakeScaler) { t.Helper() wt := &fakeWorktree{path: t.TempDir()} + scaler := &fakeScaler{order: &client.order} r := &Runner{ Output: &output.MockOutput{}, Workspace: kubernetesWorkspace(), @@ -84,13 +114,16 @@ func newTestRunner(t *testing.T, client *fakeStateRestoreClient) (*Runner, *fake openWorktree: func(ctx context.Context) (worktreeHandle, error) { return worktreeHandle{path: wt.path, remove: wt.remove}, nil }, + newScaler: func(kubeContext, namespace string) (ControlPlaneScaler, error) { + return scaler, nil + }, } - return r, wt + return r, wt, scaler } func Test_Run_RestoresInOrderWaitDatabaseTerraform(t *testing.T) { client := &fakeStateRestoreClient{} - r, wt := newTestRunner(t, client) + r, wt, scaler := newTestRunner(t, client) err := r.Run(context.Background()) require.NoError(t, err) @@ -98,27 +131,44 @@ func Test_Run_RestoresInOrderWaitDatabaseTerraform(t *testing.T) { require.True(t, client.waited) require.True(t, client.dbCalled) require.True(t, client.tfCalled) - require.Equal(t, []string{"wait", "db", "tf"}, client.order, "must wait, then restore databases, then terraform") + require.True(t, scaler.downCalled) + require.True(t, scaler.upCalled) + require.Equal(t, []string{"scaledown", "wait", "db", "tf", "scaleup"}, client.order, + "must scale down, wait, restore databases, restore terraform, then scale up") + require.True(t, wt.removed) +} + +func Test_Run_ScaleDownFailureStopsBeforeRestore(t *testing.T) { + client := &fakeStateRestoreClient{} + r, wt, scaler := newTestRunner(t, client) + scaler.scaleDownErr = errors.New("scale down boom") + + err := r.Run(context.Background()) + require.ErrorContains(t, err, "scale down boom") + require.False(t, client.waited, "no restore should run if scale down failed") + require.False(t, scaler.upCalled, "scale up should not run if scale down failed") require.True(t, wt.removed) } func Test_Run_WaitFailureStopsBeforeRestore(t *testing.T) { client := &fakeStateRestoreClient{waitErr: errors.New("db never ready")} - r, wt := newTestRunner(t, client) + r, wt, scaler := newTestRunner(t, client) err := r.Run(context.Background()) require.ErrorContains(t, err, "db never ready") require.False(t, client.dbCalled) require.False(t, client.tfCalled) + require.True(t, scaler.upCalled, "control plane must be scaled back up even when a restore step fails") require.True(t, wt.removed) } -func Test_Run_DatabaseRestoreFailureStopsBeforeTerraform(t *testing.T) { +func Test_Run_DatabaseRestoreFailureScalesBackUp(t *testing.T) { client := &fakeStateRestoreClient{restoreDBErr: errors.New("psql boom")} - r, wt := newTestRunner(t, client) + r, wt, scaler := newTestRunner(t, client) err := r.Run(context.Background()) require.ErrorContains(t, err, "psql boom") require.False(t, client.tfCalled, "terraform restore must not run after database restore failure") + require.True(t, scaler.upCalled, "control plane must be scaled back up after a failed restore") require.True(t, wt.removed) } diff --git a/pkg/cli/cmd/startup/stateclient.go b/pkg/cli/cmd/startup/stateclient.go index 6b813b7d1f0..3617ed28816 100644 --- a/pkg/cli/cmd/startup/stateclient.go +++ b/pkg/cli/cmd/startup/stateclient.go @@ -19,10 +19,29 @@ package startup import ( "context" + "github.com/radius-project/radius/pkg/cli/controlplane" "github.com/radius-project/radius/pkg/cli/pgbackup" "github.com/radius-project/radius/pkg/cli/tfstate" ) +// ControlPlaneScaler scales the database-backed control-plane deployments to zero and back, so +// state can be restored while no resource provider holds a live PostgreSQL connection. +type ControlPlaneScaler interface { + // ScaleDown scales the control-plane deployments to zero and returns their previous replica + // counts so they can be restored by ScaleUp. + ScaleDown(ctx context.Context) (map[string]int32, error) + + // ScaleUp restores the deployments to the replica counts captured by ScaleDown and waits until + // they are available again. + ScaleUp(ctx context.Context, saved map[string]int32) error +} + +// newScalerForContext is the production factory for a ControlPlaneScaler. It is a package variable +// so tests can replace it without a cluster. +var newScalerForContext = func(kubeContext, namespace string) (ControlPlaneScaler, error) { + return controlplane.NewScalerForContext(kubeContext, namespace) +} + // StateRestoreClient restores the durable Radius state for a Kubernetes context. It wraps the // pgbackup and tfstate packages so the command can be unit tested without a cluster. type StateRestoreClient interface { diff --git a/pkg/cli/controlplane/controlplane.go b/pkg/cli/controlplane/controlplane.go new file mode 100644 index 00000000000..a459387a826 --- /dev/null +++ b/pkg/cli/controlplane/controlplane.go @@ -0,0 +1,178 @@ +/* +Copyright 2023 The Radius Authors. + +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 controlplane scales the Radius control-plane deployments up and down. +// +// It exists to support restoring state into a running control plane. The PostgreSQL-backed +// resource providers open pgx connection pools at startup and cache prepared statements per +// connection. Restoring a pg_dump (which uses DROP TABLE / CREATE TABLE) underneath those live +// connections invalidates the cached statements and races the providers' own writes. Scaling the +// providers to zero before restore — and back up afterward — makes the restore atomic with +// respect to its consumers and avoids stale prepared-statement errors, without requiring a +// separate "restart" step. +package controlplane + +import ( + "context" + "fmt" + "time" + + appsv1 "k8s.io/api/apps/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" + k8s "k8s.io/client-go/kubernetes" + "k8s.io/client-go/util/retry" + + "github.com/radius-project/radius/pkg/cli/kubernetes" + "github.com/radius-project/radius/pkg/ucp/ucplog" +) + +// Deployments lists the control-plane deployments that hold PostgreSQL connection pools and must +// therefore be quiesced while state is restored. Other components (the deployment engine, the +// dashboard) do not connect to the database directly and are left running. +var Deployments = []string{"ucp", "applications-rp", "dynamic-rp"} + +const ( + // scaleTimeout bounds how long to wait for a scale operation to converge. + scaleTimeout = 2 * time.Minute + + // scalePollInterval is how often deployment status is polled while waiting. + scalePollInterval = 2 * time.Second +) + +// Scaler scales the control-plane deployments in a namespace. +type Scaler struct { + clientset k8s.Interface + namespace string +} + +// NewScaler creates a Scaler backed by the supplied Kubernetes clientset and namespace. +func NewScaler(clientset k8s.Interface, namespace string) *Scaler { + return &Scaler{clientset: clientset, namespace: namespace} +} + +// NewScalerForContext builds a Scaler from a kubeconfig context name, targeting the given +// namespace. +func NewScalerForContext(kubeContext, namespace string) (*Scaler, error) { + clientset, _, err := kubernetes.NewClientset(kubeContext) + if err != nil { + return nil, fmt.Errorf("failed to create kubernetes client: %w", err) + } + return NewScaler(clientset, namespace), nil +} + +// ScaleDown scales every control-plane deployment to zero replicas and waits until their pods are +// gone. It returns the previous replica counts keyed by deployment name so they can be restored by +// ScaleUp. Deployments that are not present are skipped (a partial install is not an error). +func (s *Scaler) ScaleDown(ctx context.Context) (map[string]int32, error) { + logger := ucplog.FromContextOrDiscard(ctx) + saved := make(map[string]int32, len(Deployments)) + + for _, name := range Deployments { + deployment, err := s.clientset.AppsV1().Deployments(s.namespace).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + logger.Info("Control-plane deployment not found, skipping", "deployment", name) + continue + } + return saved, fmt.Errorf("failed to read deployment %q: %w", name, err) + } + + saved[name] = replicasOf(deployment) + + logger.Info("Scaling down control-plane deployment", "deployment", name) + if err := s.setReplicas(ctx, name, 0); err != nil { + return saved, err + } + } + + for name := range saved { + if err := s.waitForReplicas(ctx, name, func(d *appsv1.Deployment) bool { + return d.Status.Replicas == 0 + }); err != nil { + return saved, fmt.Errorf("timed out waiting for deployment %q to scale down: %w", name, err) + } + } + + return saved, nil +} + +// ScaleUp restores each deployment to the replica count captured by ScaleDown and waits until the +// deployments report that many available replicas, so the control plane is serving again before +// the command returns. +func (s *Scaler) ScaleUp(ctx context.Context, saved map[string]int32) error { + logger := ucplog.FromContextOrDiscard(ctx) + + for name, replicas := range saved { + logger.Info("Scaling up control-plane deployment", "deployment", name, "replicas", replicas) + if err := s.setReplicas(ctx, name, replicas); err != nil { + return err + } + } + + for name, replicas := range saved { + want := replicas + if err := s.waitForReplicas(ctx, name, func(d *appsv1.Deployment) bool { + return d.Status.AvailableReplicas >= want + }); err != nil { + return fmt.Errorf("timed out waiting for deployment %q to scale up: %w", name, err) + } + } + + return nil +} + +// setReplicas sets the replica count of a deployment, retrying on optimistic-concurrency conflicts. +func (s *Scaler) setReplicas(ctx context.Context, name string, replicas int32) error { + deployments := s.clientset.AppsV1().Deployments(s.namespace) + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + deployment, getErr := deployments.Get(ctx, name, metav1.GetOptions{}) + if getErr != nil { + return getErr + } + deployment.Spec.Replicas = &replicas + _, updateErr := deployments.Update(ctx, deployment, metav1.UpdateOptions{}) + return updateErr + }) + if err != nil { + return fmt.Errorf("failed to scale deployment %q to %d: %w", name, replicas, err) + } + return nil +} + +// waitForReplicas polls the deployment until cond is satisfied or the timeout elapses. +func (s *Scaler) waitForReplicas(ctx context.Context, name string, cond func(*appsv1.Deployment) bool) error { + return wait.PollUntilContextTimeout(ctx, scalePollInterval, scaleTimeout, true, func(ctx context.Context) (bool, error) { + deployment, err := s.clientset.AppsV1().Deployments(s.namespace).Get(ctx, name, metav1.GetOptions{}) + if err != nil { + return false, err + } + return cond(deployment), nil + }) +} + +// replicasOf returns the configured replica count of a deployment, defaulting to 1 when unset +// (the Kubernetes default) so a deployment is never accidentally restored to zero replicas. +func replicasOf(deployment *appsv1.Deployment) int32 { + if deployment.Spec.Replicas == nil { + return 1 + } + if *deployment.Spec.Replicas == 0 { + return 1 + } + return *deployment.Spec.Replicas +} diff --git a/pkg/cli/controlplane/controlplane_test.go b/pkg/cli/controlplane/controlplane_test.go new file mode 100644 index 00000000000..e9f97079731 --- /dev/null +++ b/pkg/cli/controlplane/controlplane_test.go @@ -0,0 +1,141 @@ +/* +Copyright 2023 The Radius Authors. + +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 controlplane + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" + ktesting "k8s.io/client-go/testing" +) + +const testNamespace = "radius-system" + +func deployment(name string, replicas int32) *appsv1.Deployment { + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: testNamespace}, + Spec: appsv1.DeploymentSpec{Replicas: &replicas}, + // The fake clientset does not run controllers, so seed status to match spec so the + // scale-down/up waits converge immediately. + Status: appsv1.DeploymentStatus{Replicas: replicas, AvailableReplicas: replicas}, + } +} + +// newReconcilingClientset returns a fake clientset that mimics the deployment controller: on every +// create/update it copies Spec.Replicas into the status fields. Without this the fake never moves +// status to match a scale request and the scaler's status-based waits would never converge. +func newReconcilingClientset(objs ...runtime.Object) *fake.Clientset { + clientset := fake.NewSimpleClientset(objs...) + reconcile := func(action ktesting.Action) (bool, runtime.Object, error) { + obj := action.(interface{ GetObject() runtime.Object }).GetObject() + dep, ok := obj.(*appsv1.Deployment) + if !ok || dep.Spec.Replicas == nil { + return false, nil, nil + } + dep.Status.Replicas = *dep.Spec.Replicas + dep.Status.AvailableReplicas = *dep.Spec.Replicas + // Return handled=false so the default tracker still persists the (now-mutated) object. + return false, dep, nil + } + clientset.PrependReactor("create", "deployments", reconcile) + clientset.PrependReactor("update", "deployments", reconcile) + return clientset +} + +func Test_ScaleDown_RecordsReplicasAndZeroes(t *testing.T) { + clientset := newReconcilingClientset( + deployment("ucp", 1), + deployment("applications-rp", 2), + deployment("dynamic-rp", 1), + ) + scaler := NewScaler(clientset, testNamespace) + + saved, err := scaler.ScaleDown(context.Background()) + require.NoError(t, err) + require.Equal(t, map[string]int32{"ucp": 1, "applications-rp": 2, "dynamic-rp": 1}, saved) + + for _, name := range Deployments { + d, err := clientset.AppsV1().Deployments(testNamespace).Get(context.Background(), name, metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, int32(0), *d.Spec.Replicas, "deployment %q should be scaled to zero", name) + } +} + +func Test_ScaleDown_SkipsMissingDeployments(t *testing.T) { + // Only ucp exists; the other two are absent (partial install) and must be skipped. + clientset := newReconcilingClientset(deployment("ucp", 1)) + scaler := NewScaler(clientset, testNamespace) + + saved, err := scaler.ScaleDown(context.Background()) + require.NoError(t, err) + require.Equal(t, map[string]int32{"ucp": 1}, saved) +} + +func Test_ScaleUp_RestoresSavedReplicas(t *testing.T) { + clientset := newReconcilingClientset( + deployment("ucp", 0), + deployment("applications-rp", 0), + ) + scaler := NewScaler(clientset, testNamespace) + + err := scaler.ScaleUp(context.Background(), map[string]int32{"ucp": 1, "applications-rp": 3}) + require.NoError(t, err) + + ucp, err := clientset.AppsV1().Deployments(testNamespace).Get(context.Background(), "ucp", metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, int32(1), *ucp.Spec.Replicas) + + rp, err := clientset.AppsV1().Deployments(testNamespace).Get(context.Background(), "applications-rp", metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, int32(3), *rp.Spec.Replicas) +} + +func Test_ScaleDownThenUp_RoundTrip(t *testing.T) { + clientset := newReconcilingClientset( + deployment("ucp", 1), + deployment("applications-rp", 2), + deployment("dynamic-rp", 1), + ) + scaler := NewScaler(clientset, testNamespace) + ctx := context.Background() + + saved, err := scaler.ScaleDown(ctx) + require.NoError(t, err) + require.NoError(t, scaler.ScaleUp(ctx, saved)) + + for name, want := range map[string]int32{"ucp": 1, "applications-rp": 2, "dynamic-rp": 1} { + d, err := clientset.AppsV1().Deployments(testNamespace).Get(ctx, name, metav1.GetOptions{}) + require.NoError(t, err) + require.Equal(t, want, *d.Spec.Replicas, "deployment %q replicas should be restored", name) + } +} + +func Test_replicasOf_DefaultsToOne(t *testing.T) { + require.Equal(t, int32(1), replicasOf(&appsv1.Deployment{}), "nil replicas defaults to 1") + + zero := int32(0) + require.Equal(t, int32(1), replicasOf(&appsv1.Deployment{Spec: appsv1.DeploymentSpec{Replicas: &zero}}), + "already-zero replicas should restore to 1, never 0") + + three := int32(3) + require.Equal(t, int32(3), replicasOf(&appsv1.Deployment{Spec: appsv1.DeploymentSpec{Replicas: &three}})) +} From 9a157fd7892c52a8fe6533a049f8ac4a6282eb28 Mon Sep 17 00:00:00 2001 From: Sylvain Niles Date: Tue, 23 Jun 2026 19:15:59 -0700 Subject: [PATCH 7/7] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20fix?= =?UTF-8?q?=20database.enabled=3Dtrue=20install=20+=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two blocking bugs reproduced end-to-end by review, plus several non-blocking improvements and test-coverage gaps. Blocking: - The postgres image resolved to ghcr.io/radius-project/mirror/postgres:16-alpine, which the registry mirror does not publish (only :latest), causing ImagePullBackOff. Point the chart at docker.io/library/postgres:16-alpine, which is pullable and keeps the version pinned. - The init-db script created the resources table as the superuser but never granted the per-RP users (ucp, applications_rp, dynamic_rp) access, so UCP crashed on startup with "permission denied for table resources (42501)". Grant the per-RP user privileges on the table and sequences inside the table-creation loop (matches build/scripts/start-radius.sh). Non-blocking: - expandEnvURL now fails fast (os.LookupEnv) when a referenced env var is unset, instead of silently producing a malformed connection string. - controlplane.replicasOf preserves an explicit replicas: 0 rather than coercing it to 1, so ScaleUp faithfully restores the prior state. Test coverage: - Add pkg/cli/pgbackup/pgbackup_test.go covering HasBackup. - Add Validate()/command-shape tests for rad startup and rad shutdown via the shared radcli validation harness. - Add helm-unittest guards asserting the database image is the pullable reference and that the init-db script grants the per-RP users, so both blocking regressions are caught in CI without a cluster. Signed-off-by: Sylvain Niles --- .../templates/database/configmap-initdb.yaml | 6 +- deploy/Chart/tests/database_test.yaml | 25 +++++++++ deploy/Chart/values.yaml | 4 +- .../2026-06-repo-radius-state-storage.md | 4 +- pkg/cli/cmd/shutdown/shutdown_test.go | 42 ++++++++++++++ pkg/cli/cmd/startup/startup_test.go | 42 ++++++++++++++ pkg/cli/controlplane/controlplane.go | 8 +-- pkg/cli/controlplane/controlplane_test.go | 4 +- pkg/cli/pgbackup/pgbackup_test.go | 55 +++++++++++++++++++ .../database/databaseprovider/factory.go | 25 +++++++-- .../databaseprovider/storageprovider_test.go | 28 +++++++--- 11 files changed, 219 insertions(+), 24 deletions(-) create mode 100644 pkg/cli/pgbackup/pgbackup_test.go diff --git a/deploy/Chart/templates/database/configmap-initdb.yaml b/deploy/Chart/templates/database/configmap-initdb.yaml index dd07fedd31b..c95c009f369 100644 --- a/deploy/Chart/templates/database/configmap-initdb.yaml +++ b/deploy/Chart/templates/database/configmap-initdb.yaml @@ -32,7 +32,7 @@ data: for RESOURCE_PROVIDER in "${RESOURCE_PROVIDERS[@]}"; do echo "Creating tables in database $RESOURCE_PROVIDER" - psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$RESOURCE_PROVIDER" <<-'EOSQL' + psql -v ON_ERROR_STOP=1 --username "$POSTGRES_USER" --dbname "$RESOURCE_PROVIDER" <<-EOSQL CREATE TABLE IF NOT EXISTS resources ( id TEXT PRIMARY KEY NOT NULL, original_id TEXT NOT NULL, @@ -44,6 +44,10 @@ data: resource_data JSONB NOT NULL ); CREATE INDEX IF NOT EXISTS idx_resource_query ON resources (resource_type, root_scope); + -- The table is created by the superuser, so grant the per-RP user the privileges it + -- needs to read and write its own data (matches build/scripts/start-radius.sh). + GRANT ALL PRIVILEGES ON TABLE resources TO "$RESOURCE_PROVIDER"; + GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO "$RESOURCE_PROVIDER"; EOSQL done {{- end }} diff --git a/deploy/Chart/tests/database_test.yaml b/deploy/Chart/tests/database_test.yaml index a04499887e6..672fc8a2288 100644 --- a/deploy/Chart/tests/database_test.yaml +++ b/deploy/Chart/tests/database_test.yaml @@ -79,6 +79,31 @@ tests: value: database-initdb template: database/configmap-initdb.yaml + - it: should grant the per-RP users privileges on the resources table + set: + database.enabled: true + asserts: + # Guards against the "permission denied for table resources" startup crash: the table is + # created by the superuser, so the per-RP user must be granted access. + - matchRegex: + path: data["init-db.sh"] + pattern: 'GRANT ALL PRIVILEGES ON TABLE resources TO "\$RESOURCE_PROVIDER"' + template: database/configmap-initdb.yaml + - matchRegex: + path: data["init-db.sh"] + pattern: 'GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public TO "\$RESOURCE_PROVIDER"' + template: database/configmap-initdb.yaml + + - it: should use a pullable postgres image reference + set: + database.enabled: true + asserts: + # Guards against pointing at a registry-mirror tag that does not exist (ImagePullBackOff). + - equal: + path: spec.template.spec.containers[0].image + value: "docker.io/library/postgres:16-alpine" + template: database/statefulset.yaml + - it: should mount the initdb scripts into the database statefulset set: database.enabled: true diff --git a/deploy/Chart/values.yaml b/deploy/Chart/values.yaml index 1dd07158934..dfb5c0de919 100644 --- a/deploy/Chart/values.yaml +++ b/deploy/Chart/values.yaml @@ -197,7 +197,9 @@ dashboard: database: enabled: false # Enable the Postgres database install postgres_user: "radius" - image: mirror/postgres + # Pulled directly from Docker Hub: the registry mirror does not publish a pinned + # postgres tag, only ':latest'. Using the upstream image keeps the version deterministic. + image: docker.io/library/postgres tag: 16-alpine storageClassName: "" # set to the storage class name if required, the empty string will pickup the default storage class. # Minimum resource requirements, may need to revisit and scale. diff --git a/eng/design-notes/2026-06-repo-radius-state-storage.md b/eng/design-notes/2026-06-repo-radius-state-storage.md index bfe6b08533a..4e3399b2476 100644 --- a/eng/design-notes/2026-06-repo-radius-state-storage.md +++ b/eng/design-notes/2026-06-repo-radius-state-storage.md @@ -193,8 +193,8 @@ scales them back to their previous replica counts** (`pkg/cli/controlplane`). Th restore atomic with respect to its consumers and means the providers establish brand-new pools — with no stale prepared statements — against the restored schema. The deployment engine and dashboard do not connect to PostgreSQL directly and are left running. Components are always scaled -back up, including on a failed restore, and a deployment whose previous replica count was zero is -restored to one rather than left scaled down. +back up, including on a failed restore; the previous replica count is captured before scale-down +and restored faithfully (an unset count defaults to one, the Kubernetes default). A lighter alternative — restoring data-only with `TRUNCATE` instead of `--clean` to keep table OIDs stable — was considered. It avoids the prepared-statement hazard but not the write race, so diff --git a/pkg/cli/cmd/shutdown/shutdown_test.go b/pkg/cli/cmd/shutdown/shutdown_test.go index dc1858d11a7..13d35a79a13 100644 --- a/pkg/cli/cmd/shutdown/shutdown_test.go +++ b/pkg/cli/cmd/shutdown/shutdown_test.go @@ -21,11 +21,53 @@ import ( "errors" "testing" + "github.com/radius-project/radius/pkg/cli/framework" "github.com/radius-project/radius/pkg/cli/output" "github.com/radius-project/radius/pkg/cli/workspaces" + "github.com/radius-project/radius/test/radcli" "github.com/stretchr/testify/require" ) +func Test_CommandValidation(t *testing.T) { + radcli.SharedCommandValidation(t, NewCommand) +} + +func Test_Validate(t *testing.T) { + nonKubernetesConfig := radcli.LoadConfig(t, ` +workspaces: + default: github-workspace + items: + github-workspace: + connection: + kind: notkubernetes + environment: /planes/radius/local/resourceGroups/test/providers/Applications.Core/environments/test + scope: /planes/radius/local/resourceGroups/test +`) + + testcases := []radcli.ValidateInput{ + { + Name: "shutdown with a kubernetes workspace is valid", + Input: []string{}, + ExpectedValid: true, + ConfigHolder: framework.ConfigHolder{ConfigFilePath: "/weird/path", Config: radcli.LoadConfigWithWorkspace(t)}, + }, + { + Name: "shutdown with a non-kubernetes workspace is invalid", + Input: []string{}, + ExpectedValid: false, + ConfigHolder: framework.ConfigHolder{ConfigFilePath: "/weird/path", Config: nonKubernetesConfig}, + }, + { + Name: "shutdown does not accept positional args", + Input: []string{"unexpected"}, + ExpectedValid: false, + ConfigHolder: framework.ConfigHolder{ConfigFilePath: "/weird/path", Config: radcli.LoadConfigWithWorkspace(t)}, + }, + } + + radcli.SharedValidateValidation(t, NewCommand, testcases) +} + // fakeStateBackupClient records calls and returns canned errors. type fakeStateBackupClient struct { backupDBErr error diff --git a/pkg/cli/cmd/startup/startup_test.go b/pkg/cli/cmd/startup/startup_test.go index 6e2eaf258ad..d8c2b6bf251 100644 --- a/pkg/cli/cmd/startup/startup_test.go +++ b/pkg/cli/cmd/startup/startup_test.go @@ -21,11 +21,53 @@ import ( "errors" "testing" + "github.com/radius-project/radius/pkg/cli/framework" "github.com/radius-project/radius/pkg/cli/output" "github.com/radius-project/radius/pkg/cli/workspaces" + "github.com/radius-project/radius/test/radcli" "github.com/stretchr/testify/require" ) +func Test_CommandValidation(t *testing.T) { + radcli.SharedCommandValidation(t, NewCommand) +} + +func Test_Validate(t *testing.T) { + nonKubernetesConfig := radcli.LoadConfig(t, ` +workspaces: + default: github-workspace + items: + github-workspace: + connection: + kind: notkubernetes + environment: /planes/radius/local/resourceGroups/test/providers/Applications.Core/environments/test + scope: /planes/radius/local/resourceGroups/test +`) + + testcases := []radcli.ValidateInput{ + { + Name: "startup with a kubernetes workspace is valid", + Input: []string{}, + ExpectedValid: true, + ConfigHolder: framework.ConfigHolder{ConfigFilePath: "/weird/path", Config: radcli.LoadConfigWithWorkspace(t)}, + }, + { + Name: "startup with a non-kubernetes workspace is invalid", + Input: []string{}, + ExpectedValid: false, + ConfigHolder: framework.ConfigHolder{ConfigFilePath: "/weird/path", Config: nonKubernetesConfig}, + }, + { + Name: "startup does not accept positional args", + Input: []string{"unexpected"}, + ExpectedValid: false, + ConfigHolder: framework.ConfigHolder{ConfigFilePath: "/weird/path", Config: radcli.LoadConfigWithWorkspace(t)}, + }, + } + + radcli.SharedValidateValidation(t, NewCommand, testcases) +} + // fakeStateRestoreClient records calls and returns canned errors. type fakeStateRestoreClient struct { waitErr error diff --git a/pkg/cli/controlplane/controlplane.go b/pkg/cli/controlplane/controlplane.go index a459387a826..9b023836e53 100644 --- a/pkg/cli/controlplane/controlplane.go +++ b/pkg/cli/controlplane/controlplane.go @@ -165,14 +165,12 @@ func (s *Scaler) waitForReplicas(ctx context.Context, name string, cond func(*ap }) } -// replicasOf returns the configured replica count of a deployment, defaulting to 1 when unset -// (the Kubernetes default) so a deployment is never accidentally restored to zero replicas. +// replicasOf returns the configured replica count of a deployment, defaulting to 1 (the +// Kubernetes default) only when Spec.Replicas is unset. An explicit replica count, including an +// intentional 0, is preserved so ScaleUp faithfully restores the prior state. func replicasOf(deployment *appsv1.Deployment) int32 { if deployment.Spec.Replicas == nil { return 1 } - if *deployment.Spec.Replicas == 0 { - return 1 - } return *deployment.Spec.Replicas } diff --git a/pkg/cli/controlplane/controlplane_test.go b/pkg/cli/controlplane/controlplane_test.go index e9f97079731..3f045ce8861 100644 --- a/pkg/cli/controlplane/controlplane_test.go +++ b/pkg/cli/controlplane/controlplane_test.go @@ -133,8 +133,8 @@ func Test_replicasOf_DefaultsToOne(t *testing.T) { require.Equal(t, int32(1), replicasOf(&appsv1.Deployment{}), "nil replicas defaults to 1") zero := int32(0) - require.Equal(t, int32(1), replicasOf(&appsv1.Deployment{Spec: appsv1.DeploymentSpec{Replicas: &zero}}), - "already-zero replicas should restore to 1, never 0") + require.Equal(t, int32(0), replicasOf(&appsv1.Deployment{Spec: appsv1.DeploymentSpec{Replicas: &zero}}), + "an explicit zero is preserved, not coerced to 1") three := int32(3) require.Equal(t, int32(3), replicasOf(&appsv1.Deployment{Spec: appsv1.DeploymentSpec{Replicas: &three}})) diff --git a/pkg/cli/pgbackup/pgbackup_test.go b/pkg/cli/pgbackup/pgbackup_test.go new file mode 100644 index 00000000000..586499ad96b --- /dev/null +++ b/pkg/cli/pgbackup/pgbackup_test.go @@ -0,0 +1,55 @@ +/* +Copyright 2023 The Radius Authors. + +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 pgbackup + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func writeDumps(t *testing.T, dir string, dbs ...string) { + t.Helper() + for _, db := range dbs { + require.NoError(t, os.WriteFile(filepath.Join(dir, db+".sql"), []byte("-- dump"), 0o644)) + } +} + +func Test_HasBackup_AllDumpsPresent(t *testing.T) { + dir := t.TempDir() + writeDumps(t, dir, Databases...) + + require.True(t, HasBackup(dir), "HasBackup should be true when every database dump exists") +} + +func Test_HasBackup_EmptyDirectory(t *testing.T) { + require.False(t, HasBackup(t.TempDir()), "an empty directory is not a backup") +} + +func Test_HasBackup_PartialDumpsAreNotABackup(t *testing.T) { + dir := t.TempDir() + // Only the first database has a dump; the others are missing. + writeDumps(t, dir, Databases[0]) + + require.False(t, HasBackup(dir), "a partial set of dumps must not be treated as a complete backup") +} + +func Test_HasBackup_MissingDirectory(t *testing.T) { + require.False(t, HasBackup(filepath.Join(t.TempDir(), "does-not-exist"))) +} diff --git a/pkg/components/database/databaseprovider/factory.go b/pkg/components/database/databaseprovider/factory.go index c3944dc3076..d3c9fa681ba 100644 --- a/pkg/components/database/databaseprovider/factory.go +++ b/pkg/components/database/databaseprovider/factory.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "regexp" + "strings" "github.com/jackc/pgx/v5/pgxpool" store "github.com/radius-project/radius/pkg/components/database" @@ -89,12 +90,23 @@ func initInMemoryClient(ctx context.Context, opt Options) (store.Client, error) var envVarPattern = regexp.MustCompile(`\$\{([a-zA-Z_][a-zA-Z0-9_]*)\}`) // expandEnvURL replaces ${VAR} references in the connection URL with the values of the -// corresponding environment variables. References to unset variables expand to an empty string. -func expandEnvURL(url string) string { - return envVarPattern.ReplaceAllStringFunc(url, func(match string) string { +// corresponding environment variables. It returns an error if any referenced variable is unset, +// so a missing or mistyped variable fails fast with a clear message instead of silently producing +// a malformed connection string. +func expandEnvURL(url string) (string, error) { + var missing []string + expanded := envVarPattern.ReplaceAllStringFunc(url, func(match string) string { varName := envVarPattern.FindStringSubmatch(match)[1] - return os.Getenv(varName) + val, ok := os.LookupEnv(varName) + if !ok { + missing = append(missing, varName) + } + return val }) + if len(missing) > 0 { + return "", fmt.Errorf("required environment variable(s) not set: %s", strings.Join(missing, ", ")) + } + return expanded, nil } // initPostgreSQLClient creates a new PostgreSQL store client. @@ -103,7 +115,10 @@ func initPostgreSQLClient(ctx context.Context, opt Options) (store.Client, error return nil, errors.New("failed to initialize PostgreSQL client: URL is required") } - url := expandEnvURL(opt.PostgreSQL.URL) + url, err := expandEnvURL(opt.PostgreSQL.URL) + if err != nil { + return nil, fmt.Errorf("failed to initialize PostgreSQL client: %w", err) + } pool, err := pgxpool.New(ctx, url) if err != nil { diff --git a/pkg/components/database/databaseprovider/storageprovider_test.go b/pkg/components/database/databaseprovider/storageprovider_test.go index 6e4db0a9b73..2f399cb8744 100644 --- a/pkg/components/database/databaseprovider/storageprovider_test.go +++ b/pkg/components/database/databaseprovider/storageprovider_test.go @@ -120,10 +120,11 @@ func TestGetClient_UnsupportedProvider(t *testing.T) { func Test_expandEnvURL(t *testing.T) { tests := []struct { - name string - url string - env map[string]string - expected string + name string + url string + env map[string]string + expected string + expectErr bool }{ { name: "no substitution", @@ -146,11 +147,16 @@ func Test_expandEnvURL(t *testing.T) { expected: "postgresql://ucp:p@ss@host:5432/db", }, { - name: "unset variable expands to empty string", - url: "postgresql://ucp:${MISSING}@host:5432/db", - env: map[string]string{"MISSING": ""}, + name: "empty-but-set variable expands to empty string", + url: "postgresql://ucp:${EMPTY_VAR}@host:5432/db", + env: map[string]string{"EMPTY_VAR": ""}, expected: "postgresql://ucp:@host:5432/db", }, + { + name: "unset variable fails fast", + url: "postgresql://ucp:${DEFINITELY_NOT_SET_VAR}@host:5432/db", + expectErr: true, + }, } for _, tt := range tests { @@ -158,7 +164,13 @@ func Test_expandEnvURL(t *testing.T) { for k, v := range tt.env { t.Setenv(k, v) } - require.Equal(t, tt.expected, expandEnvURL(tt.url)) + got, err := expandEnvURL(tt.url) + if tt.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tt.expected, got) }) } }