Skip to content

Commit e870061

Browse files
iplay88keysEItanya
authored andcommitted
test: run upgrade tests through kagent install
The upgrade tests installed the previous release's charts with Helm and its bundled PostgreSQL, then upgraded with make helm-install-provider. That no longer works: the chart has no database, and the bundled database only accepts certificate logins. - install-previous-release downloads the previous release's kagent CLI from its GitHub release and runs kagent install, as a user would. It rejects releases whose tree lacks bundled_postgres.sql, and the CI setup action skips those legs; that sentinel replaces the "predates Goose" check. - The upgrade step runs make kagent-cli-deploy. - psql and pg_dump run as postgres over the pod's local socket, against the kagent schema. Migrations, the clean-install reference, and the schema rollback connect through a port-forward as kagent_user with role kagent_owner, using a client certificate the test signs with the active postgres-ca-pool CA. The clean-install reference lives in a scratch schema, because pg_hba admits application logins only to the kagent database. - The CI upgrade jobs drop KAGENT_HELM_EXTRA_ARGS=--cleanup-on-fail=false, which the CLI would pass to Helm as a junk --set value. No published release installs its database with kagent install yet, so both upgrade jobs skip until one does. Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
1 parent 930313e commit e870061

6 files changed

Lines changed: 174 additions & 131 deletions

File tree

‎.github/actions/upgrade-test-setup/action.yaml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ name: Upgrade Test Setup
22

33
description: >-
44
Shared prelude for the upgrade-tests and rolling-upgrade-tests jobs: resolve
5-
the upgrade-from version, skip redundant and pre-Goose legs, and bring up the
5+
the upgrade-from version, skip redundant legs and releases older than the CLI-managed database, and bring up the
66
build/cluster toolchain. Exposes the resolved version and skip flag so the
77
caller can gate its test step. The caller MUST run actions/checkout (with
88
fetch-depth: 0 + fetch-tags) before this action — a local action is loaded
@@ -15,7 +15,7 @@ inputs:
1515

1616
outputs:
1717
skip:
18-
description: '"true" when this leg is redundant or predates Goose and the caller should skip its test step.'
18+
description: '"true" when this leg is redundant or predates the CLI-managed database and the caller should skip its test step.'
1919
value: ${{ steps.resolve.outputs.skip }}
2020
version:
2121
description: The resolved upgrade-from version (empty when skip is true).
@@ -69,8 +69,9 @@ runs:
6969
fi
7070
V="$(./scripts/upgrade-from-version.sh)"
7171
fi
72-
if [[ "$V" == 0.10.* ]]; then
73-
echo "previous release v$V predates Goose; skipping upgrade tests."
72+
# Keep in sync with CLI_DATABASE_SENTINEL in the Makefile.
73+
if ! git cat-file -e "v$V:go/core/cli/internal/commands/bundled_postgres.sql" 2>/dev/null; then
74+
echo "previous release v$V predates the database kagent install manages; skipping upgrade tests."
7475
echo "skip=true" >> "$GITHUB_OUTPUT"
7576
exit 0
7677
fi

‎.github/workflows/ci.yaml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,6 @@ jobs:
323323
env:
324324
OPENAI_API_KEY: fake
325325
BUILDX_BUILDER_NAME: ${{ env.BUILDX_BUILDER_NAME }}
326-
KAGENT_HELM_EXTRA_ARGS: --cleanup-on-fail=false
327326
DOCKER_BUILD_ARGS: >-
328327
--cache-from=type=gha,scope=${{ needs.setup.outputs.cache-key }}-e2e
329328
--cache-from=type=gha,scope=${{ env.CACHE_KEY_PREFIX }}-main-e2e
@@ -371,7 +370,6 @@ jobs:
371370
env:
372371
OPENAI_API_KEY: fake
373372
BUILDX_BUILDER_NAME: ${{ env.BUILDX_BUILDER_NAME }}
374-
KAGENT_HELM_EXTRA_ARGS: --cleanup-on-fail=false
375373
DOCKER_BUILD_ARGS: >-
376374
--cache-from=type=gha,scope=${{ needs.setup.outputs.cache-key }}-e2e
377375
--cache-from=type=gha,scope=${{ env.CACHE_KEY_PREFIX }}-main-e2e

‎Makefile‎

Lines changed: 21 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ VERSION ?= $(shell git describe --tags --always 2>/dev/null | grep v || echo "v0
1414

1515
# Local architecture detection to build for the current platform
1616
LOCALARCH ?= $(shell uname -m | sed 's/x86_64/amd64/' | sed 's/aarch64/arm64/')
17+
LOCALOS ?= $(shell uname -s | tr '[:upper:]' '[:lower:]')
1718

1819
KUBECONFIG_PERM ?= $(shell \
1920
if [ "$$(uname -s | tr '[:upper:]' '[:lower:]')" = "darwin" ]; then \
@@ -55,6 +56,8 @@ KIND_KUBECONFIG ?= /tmp/kind-config
5556
# Substrate settings for Kind: workers pull localhost:5001 images through the
5657
# registry container, and templates reconcile at the minimum interval.
5758
KIND_SUBSTRATE_HELM_ARGS = --set atelet.extraArgs[0]=--localhost-registry-replacement=kind-registry:5000 --set ateApi.extraArgs[0]=--template-resync-interval=250ms
59+
# A file only releases whose `kagent install` deploys the database contain.
60+
CLI_DATABASE_SENTINEL = go/core/cli/internal/commands/bundled_postgres.sql
5861
KAGENT_POSTGRES_SECRET ?= kagent-postgres
5962

6063
CONTROLLER_IMAGE_NAME ?= controller
@@ -534,16 +537,11 @@ helm-uninstall: ## Uninstall kagent and kagent-crds Helm releases from the kind
534537
helm uninstall kagent --namespace kagent --kube-context kind-$(KIND_CLUSTER_NAME) --wait
535538
helm uninstall kagent-crds --namespace kagent --kube-context kind-$(KIND_CLUSTER_NAME) --wait
536539

537-
# Upgrade tests install a previous Kagent chart and upgrade it to the current build.
540+
# Upgrade tests install a previous release with that release's own `kagent install`,
541+
# exactly as a user would, then upgrade it to the current build with kagent-cli-deploy.
538542
# The tests use a separate cluster because they change the database and deployment.
539-
# The tests skip releases that do not use Goose.
540-
# UPGRADE_FROM_VERSION selects the previous release.
541-
# The previous install pins the bundled Postgres image to whatever the
542-
# upgrade-from release's own install target shipped (resolved inside
543-
# install-previous-release), so the baseline matches how that release actually
544-
# runs rather than a hardcoded guess; the upgrade then exercises the real
545-
# app/migration (and any DB image) change between that release and the current
546-
# build.
543+
# UPGRADE_FROM_VERSION selects the previous release; it must install its database with
544+
# `kagent install` (see CLI_DATABASE_SENTINEL); earlier releases are rejected.
547545
#
548546
# Prerequisite (provided by CI as a separate step; run it locally first): a kind
549547
# cluster (make create-kind-cluster).
@@ -552,48 +550,26 @@ helm-uninstall: ## Uninstall kagent and kagent-crds Helm releases from the kind
552550
# make invocations never run the resolver; CI passes UPGRADE_FROM_VERSION
553551
# explicitly (per matrix leg), which bypasses the script entirely.
554552
UPGRADE_FROM_VERSION ?= $(shell ./scripts/upgrade-from-version.sh)
553+
UPGRADE_FROM_CLI = /tmp/kagent-$(UPGRADE_FROM_VERSION)-$(LOCALOS)-$(LOCALARCH)
555554

556555
.PHONY: install-previous-release
557-
install-previous-release: ## Install the previous released kagent + kagent-crds charts from the public OCI registry
558-
# Abort early (rather than let helm fail confusingly) if the upgrade-from
559-
# version could not be resolved.
556+
install-previous-release: use-kind-cluster ## Install the previous release with its own kagent CLI
557+
# Abort early (rather than let the download fail confusingly) if the upgrade-from
558+
# version could not be resolved or predates the CLI-managed database.
560559
[ -n "$(UPGRADE_FROM_VERSION)" ] || { echo "UPGRADE_FROM_VERSION is empty; set it explicitly or ensure git tags are fetched." >&2; exit 1; }
560+
git cat-file -e "v$(UPGRADE_FROM_VERSION):$(CLI_DATABASE_SENTINEL)" 2>/dev/null || { echo "v$(UPGRADE_FROM_VERSION) predates the database kagent install manages; upgrade tests cannot start from it." >&2; exit 1; }
561561
@echo "=== Installing previous release: $(UPGRADE_FROM_VERSION) ==="
562-
helm upgrade --install kagent-crds $(HELM_REPO)/kagent/helm/kagent-crds \
563-
--version $(UPGRADE_FROM_VERSION) \
564-
--namespace kagent --create-namespace \
565-
--kube-context kind-$(KIND_CLUSTER_NAME) \
566-
--timeout 5m --wait
567-
# The bundled-Postgres image is selected by the install target's --set flags,
568-
# not by the chart defaults (the chart ships a non-vector image). So the
569-
# previous install must use the exact pins the upgrade-from release shipped —
570-
# otherwise the baseline DB would differ from how that release actually runs,
571-
# and the upgrade would conflate a DB swap with the migration change under
572-
# test. Read those flags straight from that release's own helm-install-provider
573-
# target (via its tagged Makefile) rather than hardcoding values that drift as
574-
# the bundled image changes. Resolved here in the recipe so the `git show` runs
575-
# only when this target runs, and so the flags can be validated before use
576-
# (they must be literal — a future release that parameterizes them with a
577-
# make/env variable would be rejected rather than passed to helm verbatim).
578-
@set -e; \
579-
db_flags="$$(git show v$(UPGRADE_FROM_VERSION):Makefile 2>/dev/null | grep -oE '\-\-set[[:space:]]+database\.postgres\.[^[:space:]\\]+' | tr '\n' ' ')"; \
580-
[ -n "$$db_flags" ] || { echo "Could not read bundled-Postgres --set flags from v$(UPGRADE_FROM_VERSION):Makefile; the upgrade-from release's install target may have moved or renamed them." >&2; exit 1; }; \
581-
case "$$db_flags" in *'$$'*|*'('*|*'{'*) echo "Bundled-Postgres --set flags from v$(UPGRADE_FROM_VERSION):Makefile contain an unexpanded variable and cannot be passed to helm verbatim: $$db_flags" >&2; exit 1;; esac; \
582-
echo " bundled-Postgres flags (from v$(UPGRADE_FROM_VERSION) install target): $$db_flags"; \
583-
helm upgrade --install kagent $(HELM_REPO)/kagent/helm/kagent \
584-
--version $(UPGRADE_FROM_VERSION) \
585-
--namespace kagent --create-namespace \
586-
--kube-context kind-$(KIND_CLUSTER_NAME) \
587-
--timeout 5m --wait \
588-
--set ui.service.type=LoadBalancer \
589-
--set controller.service.type=LoadBalancer \
590-
--set providers.default=openAI \
591-
--set providers.openAI.apiKey="$${OPENAI_API_KEY:-test}" \
592-
$$db_flags $(UPGRADE_PREV_EXTRA_ARGS)
562+
curl -fsSL -o $(UPGRADE_FROM_CLI) https://github.com/kagent-dev/kagent/releases/download/v$(UPGRADE_FROM_VERSION)/kagent-$(LOCALOS)-$(LOCALARCH)
563+
chmod +x $(UPGRADE_FROM_CLI)
564+
KUBECONFIG=$(KIND_KUBECONFIG) \
565+
KAGENT_DEFAULT_MODEL_PROVIDER=openAI \
566+
OPENAI_API_KEY="$${OPENAI_API_KEY:-test}" \
567+
KAGENT_SUBSTRATE_HELM_EXTRA_ARGS="$(KIND_SUBSTRATE_HELM_ARGS)" \
568+
KAGENT_HELM_EXTRA_ARGS="--set ui.service.type=LoadBalancer --set controller.service.type=LoadBalancer $(UPGRADE_PREV_EXTRA_ARGS)" \
569+
$(UPGRADE_FROM_CLI) install
593570

594571
# run-upgrade-tests installs the previous release and upgrades it to the current build.
595-
# The test skips releases that do not use Goose.
596-
# Later Goose releases test previous-release behavior after the target migrations,
572+
# It tests previous-release behavior after the target migrations,
597573
# data survival, schema equality, previous/current controller startup, and a
598574
# complete application and schema rollback to the previous release.
599575
# KAGENT_E2E_LOCAL_HOST lets the agent reach the local mock LLM.

‎go/core/test/upgrade/rolling_upgrade_test.go‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@ func TestRollingUpgradeCompatibility(t *testing.T) {
2929

3030
waitForReadyPods(t, env, postgresSelector, 3*time.Minute)
3131
waitForPostgresSchema(t, env, 3*time.Minute)
32-
if !hasGooseMigrationTable(t, env) {
33-
t.Skip("the baseline release does not use Goose")
34-
}
3532

3633
// Run even when there is no migration delta: a rolling upgrade rolls the new
3734
// image regardless of migrations, so the deploy can still break for
@@ -63,27 +60,27 @@ func TestRollingUpgradeCompatibility(t *testing.T) {
6360
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Minute)
6461
defer cancel()
6562
done := make(chan upgradeResult, 1)
66-
cmd := helmUpgradeCommand(ctx, env)
63+
cmd := upgradeCommand(ctx, env)
6764
go func() {
6865
out, err := cmd.CombinedOutput()
6966
done <- upgradeResult{out: string(out), err: err}
7067
}()
7168

7269
// Wait until the target schema has landed while at least one previous-release
7370
// controller pod is still ready. That is the rolling deploy hazard: old code
74-
// can keep serving briefly after a new pod has applied migrations. Surface a
75-
// helm upgrade failure immediately instead of letting it look like a
71+
// can keep serving briefly after a new pod has applied migrations. Surface an
72+
// upgrade failure immediately instead of letting it look like a
7673
// schema-observation timeout.
77-
var helmErr error
78-
var helmOut string
74+
var upgradeErr error
75+
var upgradeOut string
7976
require.Eventually(t, func() bool {
8077
select {
8178
case r := <-done:
8279
if r.err != nil {
83-
helmErr, helmOut = r.err, r.out
80+
upgradeErr, upgradeOut = r.err, r.out
8481
return true
8582
}
86-
// Helm finished successfully before we caught the window; put the
83+
// The upgrade finished successfully before we caught the window; put the
8784
// result back so the final read below still sees it, then keep polling.
8885
done <- r
8986
default:
@@ -94,7 +91,7 @@ func TestRollingUpgradeCompatibility(t *testing.T) {
9491
}
9592
return state.version == targetCoreVersion && anyPodsReady(t, env, oldPods)
9693
}, 6*time.Minute, 500*time.Millisecond, "target schema was not observed while old controller pods were still ready")
97-
require.NoError(t, helmErr, "helm upgrade failed before target schema was observed:\n%s", helmOut)
94+
require.NoError(t, upgradeErr, "upgrade failed before target schema was observed:\n%s", upgradeOut)
9895

9996
// These SQL canaries intentionally use the pre-upgrade column shape. They do
10097
// not prove every previous-release code path works, but they catch migrations
@@ -114,7 +111,7 @@ func TestRollingUpgradeCompatibility(t *testing.T) {
114111
)
115112

116113
result := <-done
117-
require.NoError(t, result.err, "helm upgrade failed:\n%s", result.out)
114+
require.NoError(t, result.err, "upgrade failed:\n%s", result.out)
118115

119116
kubectl(t, env, 3*time.Minute,
120117
"rollout", "status", "deployment/kagent-controller",

0 commit comments

Comments
 (0)