[Feature] Add the MLRun CE installer (uv + Python) by royischoss · Pull Request #315 · mlrun/ce

royischoss · 2026-09-22T07:54:36Z

📝 Description

Adds scripts/install.py, a uv + Python installer that wraps the whole CE setup — namespace creation, the registry secret, pre-install validation and the helm install itself — behind a single command, with an optional ce-config.yaml for repeatable installs.

It is purely additive to the chart: no template, values.yaml, requirements.yaml or NodePort changes, so installing by hand with helm install behaves exactly as it does today.

This supersedes #313, which proposed the same installer written in bash.

The two share a design and a CLI; only the implementation language changed, and the reasoning for that is below.

Why Python instead of bash

The bash version worked, but its prerequisites were larger than the installer itself.

It needed bash with 4.x semantics, which macOS does not ship — it still ships 3.2, where the bats assertions we relied on pass silently instead of failing.

It needed yq, without which the entire --config feature hard-exits.

It needed GNU-flavoured awk, readlink, tput and mktemp, which is why the bash version carried a hand-rolled symlink walker to stand in for readlink -f.

With uv all of that collapses to one prerequisite, because uv is a single static binary that downloads its own CPython — so Python is not a prerequisite either.

yq becomes a pyyaml import, and docker drops from required to optional, since it is only used for a best-effort registry-auth check.

The final list is uv, helm and kubectl.

The rewrite also fixed a real bug rather than just moving code.

The documented curl -sSL … | bash invocation makes the script itself bash's stdin, so every interactive read -r -p consumed script text instead of user input.

uv run <URL> downloads to a temp file before executing, which leaves stdin attached to the terminal and makes the interactive path work as intended.

Beyond that, typer replaced a hand-rolled argument parser and a usage heredoc that had to be kept in sync by hand — the --enable-otel bug found in review on #313 was exactly a hand-rolled-parser bug — while rich replaced roughly 75 lines of ANSI cursor arithmetic in the progress UI.

Behaviour was preserved by construction rather than by assertion, and then improved deliberately.

A differential harness ran both implementations against stubbed helm/kubectl/docker binaries and compared every recorded argv and exit code. Once they matched across 32 invocations, those recordings were committed as golden files and the bash script was deleted, so the old implementation's authority outlives its code.

The goldens have since moved on. Three rounds of review found roughly thirty places where a faithful port had faithfully carried a bash bug across, and fixing each one re-recorded the cases it touched. So the goldens are no longer a bash-parity claim — they are the specification for what the installer does to a cluster, reviewable as a diff without reading a line of Python.

🛠️ Changes Made

Installer (scripts/)

install.py — thin launcher with PEP 723 inline metadata; re-executes itself under uv when dependencies are missing
ce_installer/ — 11 modules: cli, settings, config, shell, cluster, registry, validators, helm_ops, ui, console, __init__
pyproject.toml — packages the installer as mlrun-ce-installer so uvx --from "git+https://github.com/mlrun/ce#subdirectory=scripts" works without a clone
hatch_build.py — reads charts/mlrun-ce/Chart.yaml at build time so an installed copy reports the real chart version instead of unknown; Chart.yaml stays the only version number in the repo
install.py.lock — committed lockfile pinning the four runtime dependencies (typer, rich, pyyaml, click) exactly for the ./scripts/install.py path; CI fails if it goes stale. The uvx --from git+... path resolves scripts/pyproject.toml instead, which the lock does not reach, so those four carry upper bounds at the next major.
docs/ — parameters.md, configuration.md, faq.md; AGENTS.md carries the design notes and the bug log

Tests (tests/installer/)

253 unit tests across test_cli, test_config, test_validators, test_cluster, test_registry, test_regressions, with shared fixtures in conftest.py
36 golden argv cases in tests/installer/golden/, driven by test_golden_argv.py against stub.py
tests/run.sh now installs under release name mlrun-ce to match the installer's default — previously mlrun, which left cluster-scoped resources (PriorityClass, ClusterRoles, CRDs) owned by a release the installer could not adopt
Chart
Chart.yaml — 0.12.0-rc.11 → 0.12.0-rc.15
charts/mlrun-ce/README.md — a short note pointing at the installer as a scripted alternative; the manual steps are unchanged and remain fully supported

CI (.github/workflows/installer-ci.yaml)

lint (ruff), unit tests on a Python 3.9 and 3.13 matrix, the golden suite, and a lockfile staleness check
a workflow_dispatch-only kind job for a real end-to-end install

Docs and developer guidance

README.md, CONTRIBUTING.md, AGENTS.md, scripts/README.md updated
five skills under .claude/skills/: deploy, run-tests, contributing, installer-feature, bump
Makefile — installer-test, installer-test-unit, installer-test-golden, installer-lint-python, installer-format, installer-link

✅ Checklist

The three values files are marked done because no values key was added or renamed — the installer only passes --set flags that the chart already accepts, so there is nothing for the install-mode files to carry.

🧪 Testing

make installer-test — 253 unit tests plus 36 golden argv cases, green on Python 3.9 and 3.13.

make installer-lint-python — ruff clean.

Golden parity against bash: every one of the original 32 invocations produced byte-identical helm/kubectl/docker argv and exit codes before the bash script was removed. The recordings have been re-taken since, as three rounds of review corrected behaviour the port had inherited; each re-recording was reviewed line by line, because every changed line is a change in what the installer does to a cluster.

Real cluster, rke2 (vmdev137): a --hard-clean uninstall followed by a full install, with all 28 pods reaching ready.

The reinstall was driven through --config rather than flags and produced helm values byte-identical to the previous flag-driven release.

Docker Desktop: --dry-run against a live cluster, exercising the server-side ownership check, the pre-install validators and the post-install access table.

Both cluster runs predate the third review round, which changed behaviour a reviewer who tried an earlier rc would notice — see the note below. The automated suites cover those changes; the cluster runs have not been repeated.

Not yet run: the workflow_dispatch kind end-to-end job.

🔗 References

Ticket link: CEML-733
External links:
- Supersedes [Feature] Adding CE installer #313 (same installer, bash implementation)
- uv — running scripts with dependencies
- PEP 723 — inline script metadata
Design docs links (Optional): scripts/AGENTS.md — architecture, known non-bugs, and the fixed-bug log

🚨 Breaking Changes?

Yes (explain below)
No

Nothing existing changes behaviour. The chart is untouched apart from the version bump and a README note, and development has no scripts/ directory today, so no downstream consumer depends on anything this PR modifies.

🔍️ Additional Notes

Two known issues are documented in scripts/AGENTS.md rather than fixed here, because both originate in the chart and its operators rather than the installer.

Uninstalling leaves Strimzi resources orphaned — the Kafka CRs are Helm hook resources, so the operator is deleted while its pods still run, and a PVC then hangs on pvc-protection. The manual cleanup is written up alongside it.

helm --wait returns before the operator-created OpenTelemetry collector is actually ready, so the access table can appear slightly ahead of that pod.

The Helm 4 warning Conflict: cannot merge map onto non-map for "registry" is informational, caused by the chart's global.registry YAML anchor; the value is applied correctly.

Two behaviour changes from the third review round are worth calling out for anyone who tried an earlier rc.

--local-registry no longer edits the cluster's CoreDNS ConfigMap. It prints the hosts entry to add and the two commands to apply it, and leaves the decision to the operator. The Corefile is cluster-wide configuration that nothing in the release namespace owns, the entry outlived --uninstall, and a bad rewrite takes DNS down for every workload on the cluster.

--dry-run is now honoured on the uninstall path. It previously applied only while building the install command, so --uninstall --hard-clean --dry-run really removed the release and really deleted every PVC and bound PV. The reads still run, so the dry run reports what would go.

Move the MLRun CE installer from its standalone repo into this one, so it ships alongside the chart it installs. The installer keeps installing the PUBLISHED chart by default. This repo's chart is used only when the caller passes --chart-path ./charts/mlrun-ce. The script is also served via curl | bash, where no repo exists around it, so the same invocation has to mean the same thing in both places. Repo integration: - make installer-test / installer-lint targets (bats, bash -n, shellcheck) - Installer CI workflow: lint and unit tests on PRs touching scripts/**, plus a workflow_dispatch-only kind end-to-end install - gitignore ce-config.yaml so filled-in registry details can't be committed - installer linked from the root and chart READMEs Align the pre-install version validators with the chart's own prerequisites rather than the product install docs: Helm >= 3.6 blocking, matching charts/mlrun-ce/README.md, and no Kubernetes floor at all, since the chart declares no kubeVersion and the README states no cluster version. The previous K8s >= 1.34 / Helm >= 4.1 floors rejected nearly every supported cluster and every Helm 3 user for a chart that renders fine on Helm 3. MIN_K8S_VERSION now only warns; MIN_HELM_VERSION remains the one hard floor and can be raised to tighten.

workflow_dispatch-only kind end-to-end install

@test

The resolve_external_host KUBE_CONTEXT test stubbed `command` but matched on the subcommand immediately after shifting off "kubectl", so it never saw the --context <ctx> that the kubectl wrapper injects ahead of the real arguments. The stub returned nothing, node_ip came back empty and the suggested host fell back to localhost. It passed locally anyway: bats aborts a test on the first failed assertion via set -e, and under macOS's system bash 3.2 that only holds for the last statement in a @test. The broken assertion was second-to-last, so it was swallowed and the test reported ok. CI runs bash 5, where it fails. Both AGENTS.md and the run-tests skill now warn about this, since any test whose stub drifts from the code can hide the same way. Also bump the chart to 0.12.0-rc.12: development is already at rc.11, so ct lint saw no version bump and failed. Co-authored-by: Cursor <cursoragent@cursor.com>

"load_config exits 1 when yq is not installed" hid yq by setting PATH=/usr/bin:/bin. That works on macOS, where yq lives in /opt/homebrew/bin, but the GitHub runners ship yq in /usr/bin — so on CI yq stayed on PATH, load_config parsed the file and returned 0, and the test failed. install.sh's yq guard was correct all along; only the test's isolation was wrong. Replace the hardcoded PATH with an _empty_bin helper pointing at a directory that provably holds no executables, so the assertion no longer depends on where the host installs yq. The neighbouring "no yq required when CONFIG_FILE is empty" test used the same hardcoded PATH and was therefore vacuous on CI; it now uses the helper too. Co-authored-by: Cursor <cursoragent@cursor.com>

It's a working design document rather than reference material for the chart, so it stays on disk and out of the PR via .git/info/exclude instead of the tracked .gitignore. Strip the four AGENTS.md pointers and the install.sh NodePort comment that referenced it, so nothing in the repo links to a file reviewers won't have. The NodePort note now points at REQUIRED_NODEPORTS, which is the actual source of truth for that list. Co-authored-by: Cursor <cursoragent@cursor.com>

Drop the Phase 1-6 status list and the Phase 3 pre-work notes: they record how the installer was built rather than how it works, which belongs in the local design doc, not the repo. Two genuinely reusable pieces are kept and restated without the phase framing — the value precedence rule (flag > env > config > --values > chart defaults, and why --config and -f compose) now sits under the install flow it describes, and the version floors read as current policy instead of a "realigned from X" note. De-specify the bug entries: they cited a named internal lab cluster, and one paragraph pinned live state on it, including an SSH command with an internal hostname and node IP. The findings hold for any remote cluster reached via --kube-context, so they now say that instead, and the teardown command is given with a placeholder context. Co-authored-by: Cursor <cursoragent@cursor.com>

The test hardcoded a named internal lab cluster and its node IP. Neither means anything to a reader of this repo, and the address is a real private one. Swap in a placeholder context name and 192.0.2.10, from RFC 5737's documentation range, so the fixture is self-evidently fake. Co-authored-by: Cursor <cursoragent@cursor.com>

The installer had no version at all, so a user couldn't say which script they ran and a run couldn't be reproduced. Add --version/-v, reading the version from charts/mlrun-ce/Chart.yaml beside the script rather than storing a copy: bumping the chart bumps the installer, with nothing to carry forward by hand. Run standalone (curl | bash, or copied to a bin directory) there's no chart to read and nothing recording where the script came from, so it reports unknown instead of inventing a number. Tying the version to the chart rather than giving the installer its own is deliberate. The script encodes chart internals — the fixed NodePort list, and the --set value paths it writes — so an installer and a chart from the same tag are the only pairing guaranteed to agree, and a renamed value path would otherwise fail silently as a --set that does nothing. The documented curl URLs pointed at .../development/scripts/install.sh, which is a 404 today (scripts/ isn't on development yet) and would be a moving target once it isn't. Use a pinned mlrun-ce-<version> tag as the primary form, since chart-releaser already tags every release and those tags contain this script; keep development documented as the rolling alternative. No new release workflow is needed as a result. Co-authored-by: Cursor <cursoragent@cursor.com>

…sion The modes were encoded as flags — --uninstall, and install as the unnamed default — which reads as a script rather than a CLI. Add a verb in front: install, uninstall, version, help, dispatched by parse_command so parse_args stays a pure flag parser. Nothing that worked before stops working. A leading flag, or no arguments at all, still means install, so every documented invocation and the curl | bash one-liner are unchanged, and `uninstall` and --uninstall are the same thing. A bare word that isn't a known command is an error rather than an install: `mlrun-ce-installer unistall` should not deploy a cluster on a typo. Two details worth knowing. main() expands COMMAND_ARGS through the ${a[@]+"${a[@]}"} guard because bash < 4.4 — including the macOS system bash this is developed on — treats an empty array as unset under set -u, so an argument-less run would otherwise abort; there's a test pinning that case. And the uninstall assignment is a full if rather than [[ ]] && x=y, which would return 1 and take errexit with it whenever the command wasn't uninstall. Also suppress color when stdout isn't a terminal or NO_COLOR is set. The log helpers previously emitted escape bytes unconditionally, which landed in CI logs and any redirected output. The installed command is now mlrun-ce-installer rather than mlrun-install. Co-authored-by: Cursor <cursoragent@cursor.com>

deploy_local_registry() had no DRY_RUN guard, so it ran kubectl apply unconditionally. On a cluster without the namespace the apply failed and errexit aborted the run, making --local-registry --dry-run unusable. On a cluster where the namespace existed the apply succeeded, so a run advertised as rendering-only really deployed a registry Deployment and Service and reported success. The guard returns early under DRY_RUN, placed after the LOCAL_REGISTRY_URL assignment so the URL still reaches the rendered --set flags. Verified live: the dry run renders it into nuclio's registry_url ConfigMap and mlrun's api chief/worker deployments while creating nothing. CI missed this because the kind-install job uses --local-registry for a real install, never with --dry-run. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

…ce-installer

scripts/install.py is a launcher carrying PEP 723 metadata; the implementation lives in the scripts/ce_installer package. install.sh is untouched and both ship during the overlap. Two deliberate behaviour changes: docker drops from a hard prerequisite to an optional one used only by the registry-auth validator, and --chart-path prefers `helm dependency build` (honouring requirements.lock) with a --skip-dependency-update escape hatch, so the installer can run from a pod or an air-gapped host. tests/installer runs both scripts over the same invocations with stubs on PATH and fails if their helm/kubectl/docker calls or exit codes differ. 28/28 cases match. Co-authored-by: Cursor <cursoragent@cursor.com>

install.py is presented as the installer; install.sh is no longer offered to users as an alternative, since only one of them will be released. It stays in the tree as the oracle the differential tests check the port against, which is how scripts/AGENTS.md and CONTRIBUTING.md now describe it. Requirements drop to uv, helm and kubectl: docker was only ever used by the best-effort registry-auth validator, and yq is gone entirely now that the config file is parsed by pyyaml. Invocation examples move from curl | bash to uvx --from git+..., which also fixes the prompts being unusable when the script is piped into bash as its own stdin. Documents --skip-dependency-update, and marks docs/design-proposal.md as a historical record of the bash design. Co-authored-by: Cursor <cursoragent@cursor.com>

print_notes_table assigned every non-empty line following a "X is available at:" header to the URL, so the last line won instead of the first. On a live install SeaweedFS showed "- S3 credentials: seaweed / seaweed123" as its address, and TimescaleDB — the final entry — swallowed the whole trailing OpenTelemetry section and displayed a sentence of prose. The first non-empty line now wins, and a blank line closes the entry so a service can no longer absorb the rest of the NOTES. A combined "- ... credentials: <user> / <pass>" line is read as credentials, and a service that names only a username renders it without a dangling separator. install.sh has the same bug and is left alone deliberately: it is being deleted, and the differential harness compares the helm/kubectl calls the two make rather than what they print, so changing it would only churn the oracle without buying coverage. Recorded in AGENTS.md alongside the rke2 verification run, the Helm 4 global.registry merge warning, and a sharper root cause for the orphaned Strimzi PVC (helm does not delete hook-created resources on uninstall). Co-authored-by: Cursor <cursoragent@cursor.com>

One named test per entry in scripts/AGENTS.md's "Fixed bugs" section, plus the output formatting the differential harness structurally cannot see: it compares the helm/kubectl calls two implementations make, not what they print, which is how the access-URL table shipped broken through 28 green matrix cases. Every test was verified by reintroducing the original bug and confirming it fails. That caught two that did not: the notes-parser fixes overlap, so reverting either one alone still left the real-world fixture rendering correctly. PLAIN_DETAIL_NOTES and LATER_SECTION_NOTES isolate them so each assertion depends on exactly one fix. Adds make installer-test-python, and extends installer-lint-python to cover tests/installer, which needs the package's ruff config passed explicitly since it lives outside scripts/ where ruff would otherwise fall back to its defaults. Records that bats test 87 fails and predates the port: install.sh and the bats file are unchanged since 6b445fa. It is not being fixed in a script scheduled for deletion, and the behaviour it guards is now covered on the Python side. Co-authored-by: Cursor <cursoragent@cursor.com>

scripts/install.sh and its 118-case bats suite are deleted. The Python port has been the shipping installer since 68bf8cc; keeping the bash script alive purely as a test oracle meant every installer change had to be reasoned about twice. Its behaviour is preserved rather than simply trusted. The differential harness that ran both implementations against stub binaries is replaced by a golden argv suite over the same 32 invocations: the calls were recorded while install.sh still existed and confirmed identical between the two on the same commit. What the installer does to a cluster is now pinned by files that can be read in a diff, so the oracle's authority outlives its code. The 118 bats cases are ported to pytest across test_cli, test_config, test_validators, test_cluster and test_registry, joining the existing regression suite: 194 unit tests plus the 32 golden cases. This also fixes the reported version on the installed path. The wheel packages ce_installer only, so `uvx --from git+...` had no chart to read and reported `unknown` — including for the pinned invocation the README recommends. A hardcoded version in pyproject.toml would have fixed that by introducing exactly the second number that reading the chart was meant to avoid, so scripts/hatch_build.py reads charts/mlrun-ce/Chart.yaml at build time instead: one hook sets the distribution version (converted to PEP 440, so 0.12.0-rc.12 becomes 0.12.0rc12), another bakes the literal string in for display. Chart.yaml stays the only version number in the repo. Smaller changes that fell out of the above: - scripts/install.py.lock is committed, so the clone path resolves its four dependencies reproducibly. CI fails on a stale lock. - The test harness takes its dependencies from scripts/pyproject.toml via --with-editable rather than a hand-written --with list, which had drifted and omitted click. It also runs --isolated on 3.9, so the declared floor is actually exercised instead of whichever interpreter happens to be active; CI covers 3.9 and 3.13. - shell.run captures stdout and stderr separately, so a warning on stderr can no longer corrupt parsed output. - New deploy and contributing skills; run-tests, installer-feature and bump updated for a world without install.sh. - tests/compare.py deleted, unreferenced since mlrun#82. Verified end to end against an rke2 cluster: a --hard-clean uninstall followed by a config-file-driven install produced helm values byte-identical to the previous flag-driven release, with all 28 pods ready. Co-authored-by: Cursor <cursoragent@cursor.com>

Removing it was unrelated to the installer port. The file is unreferenced and has been unrunnable since it landed in mlrun#82 — hardcoded absolute input paths and an undeclared ruamel.yaml import — but that is a separate cleanup and does not belong in a PR about the installer. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Copilot review overview

🟡 Changes recommended

Credential disclosure paths, unsafe CoreDNS handling, precedence errors, and pre-validation cluster mutations remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 5 · 6

Open (11)

Reject unknown installer options · New
Reject unsupported chartSource.kind values · New
Abort when CoreDNS ConfigMap retrieval fails · New
Avoid exporting file-based registry passwords · New
Redact passwords from failed command diagnostics · New
Treat dash-prefixed tokens as separate options · New
Validate values-file path before changing the cluster · New
Preserve explicit CLI disablement over configuration · New
Avoid hardcoded ingress Service name and namespace · New
Detect conflicting Services not owned by this release · New
Lock dependencies for the uvx package-entry path · New

What changed in this PR

Adds a uv-powered Python installer for MLRun CE, including configuration handling, validation, registry setup, Helm operations, documentation, tests, and CI automation.

Changes:

Introduces the modular ce_installer CLI and packaging/version infrastructure.
Adds comprehensive unit and golden-command tests plus installer CI.
Documents installer workflows and aligns the chart/test release name and version.

File	Description
`.gitignore`	Ignores local installer artifacts.
`.github/workflows/installer-ci.yaml`	Adds installer lint, test, lock, and kind jobs.
`.claude/skills/bump/SKILL.md`	Documents installer version coupling.
`.claude/skills/contributing/SKILL.md`	Adds installer contribution guidance.
`.claude/skills/deploy/SKILL.md`	Adds installer deployment guidance.
`.claude/skills/installer-feature/SKILL.md`	Adds installer feature workflow.
`.claude/skills/run-tests/SKILL.md`	Adds installer testing guidance.
`AGENTS.md`	Documents the installer architecture.
`CONTRIBUTING.md`	Adds installer prerequisites and workflow.
`Makefile`	Adds installer test, lint, format, and link targets.
`README.md`	Links the scripted installation path.
`charts/mlrun-ce/Chart.yaml`	Bumps the chart version.
`charts/mlrun-ce/README.md`	Documents the installer alternative.
`scripts/AGENTS.md`	Records installer design and behavior.
`scripts/README.md`	Provides installer usage documentation.
`scripts/ce-config.yaml.example`	Adds an example installer configuration.
`scripts/docs/configuration.md`	Documents configuration and precedence.
`scripts/docs/faq.md`	Documents installer limitations and gotchas.
`scripts/docs/parameters.md`	Documents flags and environment variables.
`scripts/hatch_build.py`	Derives package versions from the chart.
`scripts/install.py`	Adds the uv bootstrap launcher.
`scripts/install.py.lock`	Locks PEP 723 script dependencies.
`scripts/pyproject.toml`	Packages the installer command.
`scripts/ce_installer/__init__.py`	Exposes the installer entry point.
`scripts/ce_installer/cli.py`	Implements CLI parsing and orchestration.
`scripts/ce_installer/cluster.py`	Handles namespaces, hosts, and chart sources.
`scripts/ce_installer/config.py`	Parses `ce-config.yaml`.
`scripts/ce_installer/console.py`	Provides structured console output.
`scripts/ce_installer/helm_ops.py`	Implements Helm install and cleanup operations.
`scripts/ce_installer/registry.py`	Manages registry resources and credentials.
`scripts/ce_installer/settings.py`	Defines installer settings and defaults.
`scripts/ce_installer/shell.py`	Wraps external command execution.
`scripts/ce_installer/ui.py`	Renders progress and access tables.
`scripts/ce_installer/validators.py`	Implements pre-install validation.
`tests/__init__.py`	Initializes the test package.
`tests/compare.py`	Removes an obsolete comparison utility.
`tests/run.sh`	Aligns the test release name.
`tests/installer/__init__.py`	Initializes installer tests.
`tests/installer/conftest.py`	Adds shared fixtures and command recording.
`tests/installer/fixtures/ce-config.yaml`	Adds golden config input.
`tests/installer/fixtures/values.yaml`	Adds golden values input.
`tests/installer/stub.py`	Stubs external installer commands.
`tests/installer/test_cli.py`	Tests CLI behavior and precedence.
`tests/installer/test_cluster.py`	Tests cluster and chart resolution.
`tests/installer/test_config.py`	Tests configuration loading.
`tests/installer/test_golden_argv.py`	Drives end-to-end argv comparisons.
`tests/installer/test_registry.py`	Tests registry credential handling.
`tests/installer/test_regressions.py`	Covers previously identified regressions.
`tests/installer/test_validators.py`	Tests blocking and advisory validation.
`tests/installer/golden/badverb.txt`	Records invalid-command behavior.
`tests/installer/golden/chart-path.txt`	Records missing chart-path behavior.
`tests/installer/golden/dry-run-ce-version-0.11.0.txt`	Records chart-version selection.
`tests/installer/golden/dry-run-config-ce-config-f-values.txt`	Records combined config/value precedence.
`tests/installer/golden/dry-run-config-ce-config.txt`	Records config-driven installation.
`tests/installer/golden/dry-run-config-nonexistent.txt`	Records missing-config rejection.
`tests/installer/golden/dry-run-disable-model-monitoring.txt`	Records model-monitoring disabling.
`tests/installer/golden/dry-run-disable-mpi.txt`	Records MPI disabling.
`tests/installer/golden/dry-run-disable-spark-disable-mpi-disable-model-monitoring-disable-system-monitoring.txt`	Records combined component disabling.
`tests/installer/golden/dry-run-disable-spark.txt`	Records Spark disabling.
`tests/installer/golden/dry-run-disable-system-monitoring.txt`	Records monitoring disabling.
`tests/installer/golden/dry-run-enable-ingress-traefik.txt`	Records custom ingress-class behavior.
`tests/installer/golden/dry-run-enable-ingress.txt`	Records default ingress behavior.
`tests/installer/golden/dry-run-enable-otel-collector-enable-otel-instrumentation.txt`	Records ordered OTel toggles.
`tests/installer/golden/dry-run-enable-otel-collector.txt`	Records collector mode.
`tests/installer/golden/dry-run-enable-otel-full.txt`	Records full OTel mode.
`tests/installer/golden/dry-run-enable-otel-instrumentation-enable-otel-collector.txt`	Records reversed OTel ordering.
`tests/installer/golden/dry-run-enable-otel-off-enable-otel-operator.txt`	Records OTel off/operator ordering.
`tests/installer/golden/dry-run-enable-otel-off.txt`	Records disabled OTel mode.
`tests/installer/golden/dry-run-enable-otel-operator-enable-otel-collector.txt`	Records granular OTel enablement.
`tests/installer/golden/dry-run-enable-otel.txt`	Records default OTel enablement.
`tests/installer/golden/dry-run-f-values.txt`	Records values-only installation.
`tests/installer/golden/dry-run-local-registry-enable-ingress.txt`	Records ingress-backed local registry.
`tests/installer/golden/dry-run-local-registry.txt`	Records local registry behavior.
`tests/installer/golden/dry-run-skip-secret-skip-validators.txt`	Records combined skips.
`tests/installer/golden/dry-run-skip-secret.txt`	Records missing-secret rejection.
`tests/installer/golden/dry-run-skip-validators.txt`	Records validator bypass.
`tests/installer/golden/dry-run.txt`	Records baseline installation calls.
`tests/installer/golden/enable-otel-bogus.txt`	Records invalid OTel mode rejection.
`tests/installer/golden/hard-clean.txt`	Records invalid hard-clean usage.
`tests/installer/golden/uninstall-hard-clean.txt`	Records destructive uninstall flow.
`tests/installer/golden/uninstall.txt`	Records standard uninstall flow.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Eleven findings from the review of mlrun#315, plus a bug the fixes surfaced. Security-relevant: - The registry password no longer reaches argv. The pull secret is rendered as a dockerconfigjson manifest and piped to `kubectl apply -f -` rather than passed to `kubectl create secret docker-registry --docker-password`, so it is neither in the process table nor in the command text a failed call prints. shell.py gained a redact() helper covering the same ground defensively. - REGISTRY_PASSWORD_FILE contents are read into a local instead of exported into the environment every helm/kubectl/docker subprocess inherits. - The CoreDNS patch bails out on an unreadable or empty Corefile and confirms the rewrite changed something before applying, instead of risking an empty config written over cluster DNS. Correctness: - installer.chartSource.kind is validated against repo/path; a typo used to fall through to the default silently. - Passing any --enable-otel* flag makes the CLI the sole authority, so the config file cannot re-enable what the flag turned off. - The --values file existence check runs before anything touches the cluster. - Unknown options are fatal (exit 2) rather than warned-and-ignored, so `--dry-rnu` can no longer perform a real install. - Any dash-prefixed token is treated as an option, so a flag following a value-taking option is not swallowed as its value. - The ingress controller Service is looked up across the namespaces ingress-nginx actually installs into, with an INGRESS_CONTROLLER_SERVICE override, instead of assuming the release namespace. - The NodePort conflict check keys off the meta.helm.sh/release-name annotation, so a re-install no longer reports its own Services as conflicts. - The four dependencies carry upper bounds, which is what governs the uvx path; install.py.lock only pins `uv run --script`. The bug the above surfaced: newer typer vendors its own click, so the ClickException raised for an unknown option was not the class main() caught, and Python 3.10+ got a traceback and exit 1 where 3.9 got a clean exit 2. main() now catches the union of both. Every finding has a regression test. Goldens are re-recorded and gained two cases; 227 unit tests and 34 goldens pass on 3.9 and 3.13. Verified with a hard-clean uninstall and fresh install on vmdev137ig4. Chart bumped to 0.12.0-rc.13. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Copilot review overview

🟡 Changes recommended

Local-registry, repository-selection, secret-replacement, and CoreDNS handling contain unresolved reliability issues.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 · 3 · 2

Open (8)

Force configured Helm repository URL after alias collisions · New
Enable Kaniko insecure registry for all local registry modes · New
Validate CoreDNS patch and restart results · New
Use node-resolvable registry endpoint for launched images · New
Fall back to Helm output when progress UI is unavailable · New
Update pull secret atomically without deleting it first · New
Update installer examples to chart version 0.12.0-rc.13 · New
Clarify Service conflict description for release-owned Services · New

Resolved since last review (11)

Redact passwords from failed command diagnostics
Avoid exporting file-based registry passwords
Abort when CoreDNS ConfigMap retrieval fails
Reject unsupported chartSource.kind values
Reject unknown installer options
Lock dependencies for the uvx package-entry path
Detect conflicting Services not owned by this release
Avoid hardcoded ingress Service name and namespace
Preserve explicit CLI disablement over configuration
Validate values-file path before changing the cluster
Treat dash-prefixed tokens as separate options

Eight findings on fa19311. Three were bugs the port carried over faithfully from install.sh, which is noted against each. - `helm repo add` runs with `--force-update` and a checked result. Plain `repo add` errors when the `mlrun-ce` alias already points elsewhere, and bash swallowed that with `2>/dev/null || true`, so an alias left from an earlier run decided where the chart came from and `--helm-repo-url` was quietly ignored. (bash) - kaniko is told the local registry is insecure in both modes. The flag was gated on `--local-registry` *and* `--enable-ingress`, but registry:2 serves plain HTTP on its ClusterIP just as it does behind the ingress, so `--local-registry` alone had kaniko pushing to https:// and failing on TLS. (bash) - The CoreDNS patch reports only what landed. The render, apply and rollout were unchecked, so a user who could read kube-system but not write it got "CoreDNS patched" over an untouched ConfigMap. A failed restart is a warning rather than an abort: the ConfigMap is already updated by then and CoreDNS reloads it on its own. - The pull secret is updated in place. The delete-then-create was needed by bash's `kubectl create secret`, which refuses to overwrite; it survived the switch to `apply`, leaving a window with no credentials on the release. The delete remains only as a fallback for a Secret whose immutable `type` differs. - `--show-progress` takes over only on a terminal. Off a tty the progress UI returned immediately but helm was still run into a temp file that a successful run deleted, so `--show-progress` in CI produced no output at all. (bash) - The documented install commands pin the chart version. README.md, scripts/README.md and scripts/install.py still said rc.12 after the bump to rc.13, naming a tag that will never be cut. A test now ties them together so the next bump cannot forget them. - configuration.md described the NodePort check as excluding Services outside the namespace; since the previous round it excludes Services this release does not own, which is the opposite for a same-namespace conflict. Left as a documented limitation rather than fixed: with `--local-registry` and no ingress the registry is addressed as svc.cluster.local, which kaniko resolves but the node's container runtime generally cannot, so builds push and then fail at image pull. bash had the same shape, and a node-resolvable endpoint is a design change rather than a review fix. The installer now warns when it prints the registry URL, and there is an FAQ entry. 235 unit tests and 34 goldens pass on 3.9 and 3.13. The goldens move for --force-update across 24 cases and for the kaniko flag in the one local-registry-without-ingress case, which is the bug itself. Chart bumped to 0.12.0-rc.14. Co-authored-by: Cursor <cursoragent@cursor.com>

Eleven fixes, grouped by the plan they came from. The three merge blockers first: - kaniko was told the local registry was insecure at mlrun.api.kaniko.insecureRegistry, which no chart in the umbrella reads. The builder is nuclio's dashboard; the keys are nuclio.dashboard.kaniko.insecurePush/PullRegistry. helm accepts an unknown --set path silently, so the flag looked present and did nothing. The same search turned up mpi-operator.rbac.create, which also does not exist — so --disable-mpi still created the operator's ServiceAccount and RoleBinding. A new test resolves every non-global --set path against the umbrella's values merged with each vendored subchart's, so a third cannot be added quietly. - --dry-run was only read while building the install command, so --uninstall --hard-clean --dry-run really removed the release and really deleted every PVC and bound PV. - --non-interactive stopped on three separate things: the required-field check lived at the end of load_config, which returns early without --config; it also ran ahead of the uninstall branch, demanding registry credentials to tear a release down; and REGISTRY_EMAIL, which registries stopped requiring years ago, had no default and so counted as missing. The rest: - The newline in NODEPORT_JSONPATH sat inside the ports range, so a Service with no ports swallowed the next Service's line and hid real conflicts. - The CoreDNS ConfigMap is no longer rewritten. It is cluster-wide config nothing in the release namespace owns, the entry outlived --uninstall, and a bad rewrite takes DNS down for every workload. The entry and the two commands to apply it are reported instead. - --chart-path is validated before the namespace and local registry exist. - --dry-run falls back to a client-side dry run, with a warning, below Helm 3.13 — where --dry-run=server is an invalid boolean and the install never starts. - validate_registry_auth is skipped under --dry-run: a successful docker login rewrites the user's ~/.docker/config.json. - The Kubernetes version comes from the API server, not an arbitrary node's kubelet. - Every default StorageClass is reported, not just the first. - A hard clean leaves PVs that are still Bound; only Released and Failed go. - minikube ip is asked once, and helm repo add no longer rewrites the user's repositories.yaml. Goldens re-recorded; the uninstall cases now run against a stub that has a release and volumes to destroy, so the dry-run pair differs from the real one. Chart bumped to 0.12.0-rc.15. Co-authored-by: Cursor <cursoragent@cursor.com>

install.sh is gone, so the docs no longer measure the installer against it. scripts/AGENTS.md loses the "port from bash" section and the 30-entry "Deliberate divergences from the bash behaviour" list (718 -> 512 lines). What survives is the part that still applies without the comparison: the golden argv log is the contract, read the diff before re-recording, and the svc.cluster.local pull limitation under --local-registry without ingress. The golden suite's rationale is restated on its own terms. It used to derive its authority from a differential harness that no longer exists; it now says what it actually is — the specification for what the installer does to a cluster, readable as a diff without reading Python. Also fixed along the way: - The argv pre-parse section cited "divergence 9" and "divergence 6" by number. Both now state the reasoning directly. - A sentence about bash's ${a[@]+"${a[@]}"} empty-array guard under set -u had been left mid-thought in the install-flow list, describing a language this installer is not written in. - Stale counts: 34 -> 36 golden cases, 182 -> 253 unit tests. Root AGENTS.md and the installer-feature and run-tests skills drop their predecessor clauses. The user-facing docs needed no changes: every "bash" in them was a code fence or uv's own astral.sh/uv/install.sh. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Copilot review overview

🔵 Needs a closer look

Unresolved credential and registry lifecycle risks affect cluster state, and the latest behavior has not been rechecked on a live cluster.

Review effort: Balanced
Findings: 1 · 6

Open (7)

Do not delete pull secrets on non-immutable errors · New
Isolate Helm's repository cache · New
Require a usable Jupyter ingress hostname · New
Apply the selected ingress class to all ingresses · New
Persist images in the in-cluster registry · New
Track and clean up installer-created registry resources · New
Clear inherited STUB controls in subprocess tests · New

Resolved since last review (8)

Validate CoreDNS patch and restart results
Enable Kaniko insecure registry for all local registry modes
Force configured Helm repository URL after alias collisions
Update pull secret atomically without deleting it first
Fall back to Helm output when progress UI is unavailable
Use node-resolvable registry endpoint for launched images
Clarify Service conflict description for release-owned Services
Update installer examples to chart version 0.12.0-rc.13

+    log_warn(f"Could not update secret '{settings.registry_secret_name}' in place; replacing it.")
+    kubectl(


+    directory = tempfile.mkdtemp(prefix="mlrun-ce-helm-repos-")
+    os.environ["HELM_REPOSITORY_CONFIG"] = str(Path(directory) / "repositories.yaml")
+    atexit.register(shutil.rmtree, directory, True)


+        flags += ["--set", "timescaledb.enabled=false"]
+
+    if settings.enable_ingress:
+        flags += ["--set", "jupyterNotebook.ingress.enabled=true"]


+        flags += ["--set", "nuclio.dashboard.ingress.enabled=true"]
+        flags += ["--set", "mlrun.api.ingress.enabled=true"]
+        flags += ["--set", "mlrun.ui.ingress.enabled=true"]


+        - name: registry
+          image: registry:2
+          ports:
+            - containerPort: 5000


+    kubectl(
+        settings,
+        "apply",
+        "-f",
+        "-",
+        "--namespace",
+        settings.namespace,
+        input_data=LOCAL_REGISTRY_MANIFEST.format(namespace=settings.namespace),


+    log = tmp_path / "calls.log"
+    log.write_text("")
+
+    env = dict(os.environ)


royischoss and others added 23 commits August 20, 2026 17:19

same

4197b2f

- Installer CI workflow: lint and unit tests on every PR, plus a

95a959c

workflow_dispatch-only kind end-to-end install

adding helm timeout. refer to kafka upgrade crush loop

259d24b

Merge branch 'development' into feature/ce-installer

b096b69

update Usage: mlrun-ce-installer <command>

6ae103d

fix co-pilot review

ccca666

Enhance uninstall command timeout information

36364d0

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Merge remote-tracking branch 'roy/feature/ce-installer' into feature/…

30f2742

…ce-installer

allign namespace install installer and tests

88be81b

github-actions Bot added the feature label Sep 22, 2026

royischoss mentioned this pull request Sep 22, 2026

[Feature] Adding CE installer #313

Closed

4 of 13 tasks

royischoss requested a balanced review from Copilot September 22, 2026 08:04

Copilot started reviewing on behalf of royischoss September 22, 2026 08:05 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

royischoss requested a balanced review from Copilot September 22, 2026 11:51

Copilot started reviewing on behalf of royischoss September 22, 2026 11:52 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

royischoss and others added 3 commits September 22, 2026 16:21

royischoss requested a balanced review from Copilot September 30, 2026 07:44

Copilot started reviewing on behalf of royischoss September 30, 2026 07:45 View session

Copilot AI reviewed Sep 30, 2026

View reviewed changes

Navigation Menu

Repository navigation

[Feature] Add the MLRun CE installer (uv + Python) - #315

[Feature] Add the MLRun CE installer (uv + Python)#315
royischoss wants to merge 28 commits into
mlrun:developmentfrom
royischoss:feature/ce-installer-uv

royischoss commented Sep 22, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		log_warn(f"Could not update secret '{settings.registry_secret_name}' in place; replacing it.")
		kubectl(

Conversation

royischoss commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description

Why Python instead of bash

🛠️ Changes Made

✅ Checklist

🧪 Testing

🔗 References

🚨 Breaking Changes?

🔍️ Additional Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

royischoss commented Sep 22, 2026 •

edited

Loading