Skip to content

Commit 8efb067

Browse files
feat(hack): emit a disable list so a normal variant installs only what a run needs
select-install.sh has computed the forward dependency closure for a set of E2E suites since #3279, and nothing has ever consumed it. The reason it stalled was a wrong assumption, mine included until this was checked: that installing only the closure needs a new "minimal" platform variant, because the bundles emit their catalogue wholesale and enabledPackages is additive. It does not. The base package helper in packages/core/platform/templates/_helpers.tpl consults bundles.disabledPackages for EVERY package, not only the ones declared optional through package.optional, and nothing in the chart emits a Package by any other route. So an ordinary variant reduces to the closure by subtraction, with no new variant and no chart change at all. This adds `--disabled`, which emits the complement: every PackageSource the platform knows, minus the closure. For the postgres suite that is 21 kept and 78 disabled out of 99, of which 6 are PackageSources no bundle emits anyway, so the real reduction is 72. The closure now seeds a platform baseline alongside the engine: cozystack-basics, tenant-application, and the etcd, ingress, monitoring and seaweedfs applications. None of them carries a dependsOn edge from any app, so the graph walk cannot reach them, yet hack/e2e-install-cozystack.bats waits for the root Tenant and those HelmReleases before any suite runs, and the suites themselves live in tenant-root and reach seaweedfs-s3.tenant-root for object storage. Without the seed the reduced platform has nowhere to install anything and the install times out -- a graph-shaped answer that is nonetheless wrong, which is what the seed list exists for. Subtraction is safe by construction rather than by care. The closure is a forward walk over dependsOn, so anything a kept package needs is already inside it and cannot appear in the complement. A test asserts that over every suite in hack/e2e-chainsaw rather than on one named pair: an earlier version pinned backupstrategy-controller dependsOn velero, which no closure can reach because nothing declares an edge TO backupstrategy-controller, so it asserted nothing at all. An empty selection refuses instead of answering, since the complement of an empty closure is the entire platform -- the same class of silently-wrong answer #3392 removed from the suite selector. The refusal sits before the existing `[ -z "$seeds" ] && exit 0`, because in closure mode an empty answer is legitimate. Two limits are documented in the script rather than discovered later. Subtraction governs a fresh install only: both package defines stamp helm.sh/resource-policy: keep, so on a live cluster the Package CR stops being emitted but the installed package stays. And packages emitted through package.optional need bundles.enabledPackages as well, so keeping one in the closure does not install it. Proven at the rendering level: the new packages/core/platform/tests/selective_install_test.yaml renders the bundles with the real disable list and checks that postgres-application survives in the app catalogue, postgres-operator and tenant-application survive in the system bundle, mariadb-operator and kafka-operator are gone, and the five fail() guards across the iaas and naas bundles do not trip. The disable case is the one that goes red when the helper is mutated to ignore disabledPackages; the others are guards and say so. The list is held in one YAML anchor -- it was pasted five times before, 450 of 565 lines, with nothing keeping the copies in step. Not wired into the E2E lane here. That needs the list in the Package CR hack/e2e-install-cozystack.bats applies, plus a live run to confirm the reduced platform reaches Ready and to measure how much of the 27-minute install it saves, which is unmeasured -- the system bundle stays whole. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent dc2e427 commit 8efb067

3 files changed

Lines changed: 394 additions & 10 deletions

File tree

‎hack/select-install.sh‎

Lines changed: 108 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#
88
# Usage:
99
# hack/select-install.sh <suites> [<sources-dir>]
10+
# hack/select-install.sh --disabled <suites> [<sources-dir>]
1011
# hack/select-install.sh --validate [<sources-dir> [<suites-dir>]]
1112
#
1213
# Defaults: sources-dir = packages/core/platform/sources
@@ -16,8 +17,31 @@
1617
# (e.g. "postgres harbor"). "-" reads the list from stdin.
1718
#
1819
# Output (closure mode): space-separated PackageSource names (cozystack.*) that
19-
# the run must enable, seeds + the engine + all transitive dependsOn. Empty
20-
# when <suites> is empty. A suite with no known mapping is a HARD ERROR (fail
20+
# the run must enable, seeds + the platform baseline + all transitive
21+
# dependsOn. Empty when <suites> is empty.
22+
#
23+
# Output (--disabled): the COMPLEMENT of that closure, for bundles.disabledPackages
24+
# in the platform chart. The base package helper in
25+
# packages/core/platform/templates/_helpers.tpl consults disabledPackages for
26+
# every package rather than only the optional ones, so an ordinary variant
27+
# reduces to the closure by subtraction with no new variant. Refuses on an
28+
# empty selection, where the complement would be the whole platform.
29+
#
30+
# Two limits worth knowing before consuming it.
31+
#
32+
# Subtraction governs a FRESH install only. Both package defines stamp
33+
# `helm.sh/resource-policy: keep`, so adding a name on a live cluster stops
34+
# emitting the Package CR but does not remove the one already there; the
35+
# package stays installed. Fine for a per-run E2E install, wrong as a way to
36+
# uninstall.
37+
#
38+
# Packages emitted through package.optional (nfs-driver, telepresence,
39+
# external-dns, external-dns-application, and the others gated the same way)
40+
# need bundles.enabledPackages as well. Keeping one in the closure is
41+
# therefore not enough to install it, and a suite whose closure contains one
42+
# needs that list set separately. Subtraction is correct for them either way,
43+
# since an optional package absent from enabledPackages is not installed
44+
# regardless. A suite with no known mapping is a HARD ERROR (fail
2145
# closed) — a silently-skipped suite would install nothing and let the test
2246
# pass vacuously or fail later on a missing CRD.
2347
#
@@ -43,7 +67,17 @@ SUITES_DIR="hack/e2e-chainsaw"
4367
MODE="closure"
4468
SUITES=""
4569

46-
if [ "${1:-}" = "--validate" ]; then
70+
if [ "${1:-}" = "--disabled" ]; then
71+
MODE="disabled"
72+
shift
73+
suites_arg="${1?usage: select-install.sh --disabled <suites> [sources-dir]}"
74+
SOURCES_DIR="${2:-$SOURCES_DIR}"
75+
if [ "$suites_arg" = "-" ]; then
76+
SUITES="$(cat)"
77+
else
78+
SUITES="$suites_arg"
79+
fi
80+
elif [ "${1:-}" = "--validate" ]; then
4781
MODE="validate"
4882
SOURCES_DIR="${2:-$SOURCES_DIR}"
4983
SUITES_DIR="${3:-$SUITES_DIR}"
@@ -191,7 +225,17 @@ for suite in $SUITES; do
191225
done
192226
[ "$map_rc" = 0 ] || exit "$map_rc"
193227

194-
[ -z "$seeds" ] && exit 0
228+
# An empty seed set means no suite was selected. In closure mode that is a valid
229+
# answer -- install nothing extra. In disabled mode it is not: the complement of
230+
# an empty closure is the whole platform, which as an install instruction is a
231+
# platform with no packages at all, so refuse rather than emit it.
232+
if [ -z "$seeds" ]; then
233+
if [ "$MODE" = "disabled" ]; then
234+
echo "select-install: refusing to emit a disable list for an empty suite selection — the complement would be the entire platform" >&2
235+
exit 1
236+
fi
237+
exit 0
238+
fi
195239

196240
# Any non-empty install set needs the engine up: it registers the
197241
# ApplicationDefinition CRD every *-rd HelmRelease reconciles against, and its
@@ -200,11 +244,40 @@ done
200244
# a dependsOn edge to it; system-package-backed suites reach it via no edge, so
201245
# seed it explicitly here. Its absence from the graph is a hard error — nothing
202246
# can install without it.
203-
engine="cozystack.cozystack-engine"
204-
if echo "$NODES" | grep -Fxq "$engine"; then
205-
case " $seeds " in *" $engine "*) ;; *) seeds="$seeds $engine" ;; esac
206-
else
207-
echo "select-install: error: required engine PackageSource '$engine' not found in $SOURCES_DIR" >&2
247+
# The engine, plus the platform baseline every run needs whatever it selected.
248+
#
249+
# The engine registers the ApplicationDefinition CRD, so no *-rd HelmRelease can
250+
# reconcile without it. The rest carry no dependsOn edge to any app either, and
251+
# are just as unconditional: hack/e2e-install-cozystack.bats waits for the root
252+
# Tenant and then for the etcd, ingress, monitoring, seaweedfs and tenant-root
253+
# HelmReleases before any suite runs, and the app suites themselves live in the
254+
# tenant-root namespace and reach seaweedfs-s3.tenant-root for object storage.
255+
# Without these the reduced platform has nowhere to install anything and the
256+
# install step times out, which is a graph-shaped answer that is nonetheless
257+
# wrong -- exactly the case this seed list exists to cover.
258+
#
259+
# Seeded rather than given edges in the graph: they are a property of how the
260+
# platform is installed, not a dependency any single app declares, and inventing
261+
# edges for them would fan every app out to all of them in the reverse walk that
262+
# select-e2e.sh does.
263+
baseline="cozystack.cozystack-engine
264+
cozystack.cozystack-basics
265+
cozystack.tenant-application
266+
cozystack.etcd-application
267+
cozystack.ingress-application
268+
cozystack.monitoring-application
269+
cozystack.seaweedfs-application"
270+
271+
missing=""
272+
for b in $baseline; do
273+
if echo "$NODES" | grep -Fxq "$b"; then
274+
case " $seeds " in *" $b "*) ;; *) seeds="$seeds $b" ;; esac
275+
else
276+
missing="${missing:+$missing }$b"
277+
fi
278+
done
279+
if [ -n "$missing" ]; then
280+
echo "select-install: error: required baseline PackageSource(s) not found in $SOURCES_DIR: $missing" >&2
208281
exit 1
209282
fi
210283

@@ -220,4 +293,29 @@ while :; do
220293
all="$all $new"
221294
done
222295

223-
echo "$all" | tr ' ' '\n' | grep -v '^$' | sort -u | paste -sd ' ' -
296+
closure="$(echo "$all" | tr ' ' '\n' | grep -v '^$' | sort -u)"
297+
298+
if [ "$MODE" = "disabled" ]; then
299+
# The complement: every PackageSource the platform knows, minus the closure the
300+
# selected suites need. This is what goes into bundles.disabledPackages, and it
301+
# is why no new platform variant is required -- the base package helper
302+
# (packages/core/platform/templates/_helpers.tpl) consults disabledPackages for
303+
# EVERY package, not only the ones declared optional, so an ordinary variant can
304+
# be reduced to the closure by subtraction.
305+
#
306+
# Subtracting is safe by construction because the closure is a FORWARD walk over
307+
# dependsOn: anything the kept set needs is already in it, so the complement can
308+
# never contain a dependency of something kept. The hard pairs this protects are
309+
# real -- backupstrategy-controller depends on velero, and there is a unit test
310+
# in packages/core/platform guarding exactly that -- and they survive without
311+
# being special-cased here.
312+
#
313+
# An empty closure yields the full node list, which would disable everything.
314+
# That is a legitimate answer to "no suites selected" only if the caller then
315+
# installs nothing at all; as an INSTALL instruction it is a platform with no
316+
# packages, so refuse it rather than emit it.
317+
echo "$NODES" | grep -vxF "$closure" | paste -sd ' ' -
318+
exit 0
319+
fi
320+
321+
echo "$closure" | paste -sd ' ' -

‎hack/select-install_test.bats‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,3 +244,88 @@ YAML
244244
fi
245245
rm -rf "$tmp"
246246
}
247+
248+
# --disabled emits the complement of the closure: what bundles.disabledPackages
249+
# must hold so an ordinary platform variant installs only what the suites need.
250+
@test "disabled mode: closure and complement partition the source list" {
251+
# Unique metadata.name, not a file count: the script partitions names, and a
252+
# sources file holding two documents would make a file count disagree for a
253+
# reason that has nothing to do with the complement.
254+
total=$(yq -rN '.metadata.name | select(. != null and . != "")' \
255+
packages/core/platform/sources/*.yaml | sort -u | wc -l | tr -d ' ')
256+
keep=$(hack/select-install.sh "postgres" | wc -w | tr -d ' ')
257+
drop=$(hack/select-install.sh --disabled "postgres" | wc -w | tr -d ' ')
258+
sum=$((keep + drop))
259+
if [ "$sum" -ne "$total" ]; then
260+
echo "closure ($keep) + complement ($drop) = $sum, but there are $total PackageSources" >&2
261+
exit 1
262+
fi
263+
}
264+
265+
@test "disabled mode: never lists a package the closure keeps" {
266+
keep=$(hack/select-install.sh "postgres kafka")
267+
drop=$(hack/select-install.sh --disabled "postgres kafka")
268+
for k in $keep; do
269+
case " $drop " in
270+
*" $k "*) echo "package '$k' is both kept and disabled" >&2; exit 1 ;;
271+
esac
272+
done
273+
}
274+
275+
# The forward closure is what makes subtraction safe: a dependency of something
276+
# kept can never land in the complement. Asserted over every suite rather than
277+
# on one named pair -- an earlier version pinned backupstrategy-controller
278+
# dependsOn velero, which no closure can reach (nothing declares an edge TO
279+
# backupstrategy-controller), so the test asserted nothing at all.
280+
@test "disabled mode: no kept package has a dependency in the complement" {
281+
deps=$(mktemp)
282+
yq -rN '.metadata.name as $n | .spec.variants[]?.dependsOn[]? | select(. != null and . != "") | $n + " " + .' \
283+
packages/core/platform/sources/*.yaml > "$deps"
284+
285+
for suite in $(find hack/e2e-chainsaw -mindepth 2 -maxdepth 2 -name chainsaw-test.yaml \
286+
| sed -e 's,^hack/e2e-chainsaw/,,' -e 's,/chainsaw-test\.yaml$,,'); do
287+
keep=$(hack/select-install.sh "$suite")
288+
drop=$(hack/select-install.sh --disabled "$suite")
289+
for k in $keep; do
290+
for d in $(grep "^$k " "$deps" | cut -d' ' -f2); do
291+
case " $drop " in
292+
*" $d "*)
293+
echo "suite '$suite': kept '$k' depends on '$d', which is in the disable list" >&2
294+
rm -f "$deps"; exit 1 ;;
295+
esac
296+
done
297+
done
298+
done
299+
rm -f "$deps"
300+
}
301+
302+
# The platform baseline is seeded unconditionally: it carries no dependsOn edge
303+
# from any app, but hack/e2e-install-cozystack.bats waits for the root Tenant and
304+
# the etcd/ingress/monitoring/seaweedfs/tenant-root HelmReleases before any suite
305+
# runs, and the suites themselves live in tenant-root. Disabling those leaves the
306+
# reduced platform with nowhere to install anything.
307+
@test "disabled mode: never disables the platform baseline" {
308+
drop=$(hack/select-install.sh --disabled "postgres")
309+
for b in cozystack.cozystack-engine cozystack.cozystack-basics \
310+
cozystack.tenant-application cozystack.etcd-application \
311+
cozystack.ingress-application cozystack.monitoring-application \
312+
cozystack.seaweedfs-application; do
313+
case " $drop " in
314+
*" $b "*) echo "baseline package '$b' is in the disable list" >&2; exit 1 ;;
315+
esac
316+
done
317+
}
318+
319+
# An empty closure would make the complement the entire platform. As an install
320+
# instruction that is a platform with no packages, so it must refuse rather than
321+
# emit it.
322+
@test "disabled mode: refuses an empty selection" {
323+
out=$(mktemp); err=$(mktemp)
324+
if hack/select-install.sh --disabled "" >"$out" 2>"$err"; then
325+
echo "expected a non-zero exit for an empty selection, got: $(cat "$out")" >&2
326+
rm -f "$out" "$err"; exit 1
327+
fi
328+
grep -q "refusing to emit a disable list" "$err" \
329+
|| { echo "expected the refusal message, got: $(cat "$err")" >&2; rm -f "$out" "$err"; exit 1; }
330+
rm -f "$out" "$err"
331+
}

0 commit comments

Comments
 (0)