fix: remove Helm charts and standardize on operator-based deployment#448
Open
raballew wants to merge 18 commits intojumpstarter-dev:mainfrom
Open
fix: remove Helm charts and standardize on operator-based deployment#448raballew wants to merge 18 commits intojumpstarter-dev:mainfrom
raballew wants to merge 18 commits intojumpstarter-dev:mainfrom
Conversation
❌ Deploy Preview for jumpstarter-docs failed. Why did it fail? →
|
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR removes Helm-based deployment artifacts and support across the repository and consolidates deployments to operator-only flows using the Jumpstarter CR. Changes include deletion of Helm charts, Helm helpers, Helm-related Python APIs/CLI commands, CI lint/publish jobs, and e2e/Makefile Helm branching and flags. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
raballew
commented
Apr 9, 2026
controller/hack/utils
Outdated
| # $1: deployment method (e.g., "Helm", "operator") - optional | ||
| # $1: deployment method (e.g., "operator") - optional | ||
| print_deployment_success() { | ||
| local method=${1:-""} |
Member
Author
There was a problem hiding this comment.
what other methods exist?
e2e/compat/setup.sh
Outdated
| # Environment variables: | ||
| # COMPAT_SCENARIO - "old-controller" or "old-client" (required) | ||
| # COMPAT_CONTROLLER_TAG - Controller image tag for old-controller scenario (default: v0.7.1) | ||
| # COMPAT_CONTROLLER_TAG - Controller image tag for old-controller scenario (default: v0.7.0) |
Member
Author
There was a problem hiding this comment.
why is it 0.7.0 and not 0.7.1?
Member
There was a problem hiding this comment.
now it should be 0.8.1 to be right :)
e2e/compat/setup.sh
Outdated
|
|
||
| # Always use helm for compat tests (simpler, direct control) | ||
| export METHOD="helm" | ||
| export METHOD="operator" |
Member
Author
There was a problem hiding this comment.
is method really needed after all? dont we only have operator now or do other methods exist too?
e2e/setup-e2e.sh
Outdated
|
|
||
| # Deployment method: operator (default) or helm | ||
| # Deployment method | ||
| export METHOD="${METHOD:-operator}" |
Member
Author
There was a problem hiding this comment.
same question here - why is there still a method?
raballew
force-pushed
the
042-remove-helm-charts
branch
from
6884226 to
fa102b3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py (1)
220-228: Cover the non-zerokubectlfailure path too.
run_command()failures are often reported via a non-zero return code, not only by raising. This test only exercises the exception path, so the current regression incheck_jumpstarter_installation()can slip through unnoticed.💡 Suggested test addition
`@pytest.mark.asyncio` `@patch`("jumpstarter_kubernetes.cluster.kubectl.run_command") async def test_check_jumpstarter_installation_command_failure(self, mock_run_command): mock_run_command.side_effect = RuntimeError("kubectl not found") result = await check_jumpstarter_installation("test-context") assert result.installed is False assert result.has_crds is False assert result.error is not None assert "kubectl not found" in result.error + + `@pytest.mark.asyncio` + `@patch`("jumpstarter_kubernetes.cluster.kubectl.run_command") + async def test_check_jumpstarter_installation_nonzero_exit(self, mock_run_command): + mock_run_command.return_value = (1, "", "forbidden") + + result = await check_jumpstarter_installation("test-context") + + assert result.installed is False + assert result.has_crds is False + assert result.error is not None + assert "forbidden" in result.error🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py` around lines 220 - 228, The test test_check_jumpstarter_installation_command_failure only simulates run_command raising an exception, but check_jumpstarter_installation must also handle non-zero exit codes; add a test case that makes mock_run_command return a completed-result with a non-zero return code (and stderr containing "kubectl not found") instead of raising, then call check_jumpstarter_installation("test-context") and assert result.installed is False, result.has_crds is False, and the error message contains the stderr text; reference the same test function name and mock_run_command to locate where to add this non-exception path test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/Makefile`:
- Around line 67-68: The Makefile's manifests target now emits CRDs/RBAC to
deploy/operator/config/... (see output:crd:artifacts and output:rbac:artifacts),
but the consuming targets install, uninstall and build-installer still reference
config/crd and config/default; update the consuming targets to read from
deploy/operator/config/... (or revert manifests to write to the canonical
config/... tree) so generated artifacts and consumers are aligned. Specifically,
modify the install and uninstall targets (currently building config/crd) and the
build-installer target (currently building config/default) to point to
deploy/operator/config/crd and deploy/operator/config/default respectively, or
change the manifests output back to the original config/... paths so nothing
else needs changing.
In `@e2e/setup-e2e.sh`:
- Around line 296-299: The script currently hardcodes ports when building
ENDPOINT and LOGIN_ENDPOINT; instead query the Jumpstarter CR for the actual
endpoints/ports and use those values. Replace the kubectl JSONPath that sets
BASEDOMAIN/ENDPOINT/LOGIN_ENDPOINT with commands that read the controller and
login endpoint values from the CR spec (e.g., fields like
.spec.controllerEndpoint or .spec.endpoints.controller and .spec.loginEndpoint
or .spec.endpoints.login) and assign them to ENDPOINT and LOGIN_ENDPOINT (fall
back to the previous host:port construction only if those spec fields are
empty); update any references to BASEDOMAIN if you must derive host/port
separately so ENDPOINT and LOGIN_ENDPOINT reflect the CR configuration rather
than hardcoded ports.
In `@e2e/tests.bats`:
- Around line 65-69: The teardown log dump redirection is wrong: the kubectl log
commands using `kubectl -n "${JS_NAMESPACE}" logs -l component=controller
--tail=250 2>&1 >&2` and the similar router line send both streams to stdout
instead of stderr; update those two commands (`kubectl -n "${JS_NAMESPACE}" logs
-l component=controller --tail=250 ...` and `kubectl -n "${JS_NAMESPACE}" logs
-l component=router --tail=250 ...`) to redirect their output directly to stderr
(use `>&2`) so kubectl errors are shown in Bats failure output.
In
`@python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py`:
- Around line 92-117: The current logic calls run_command(crd_cmd) and if
returncode != 0 it silently leaves result_data without an error and reports
installed=False; change this so non-zero return codes set result_data["error"]
(include the returncode and stdout/stderr text) and do not flip "installed" to
True/False based on absent output; specifically update the code around
run_command/crd_cmd and the handling of returncode to populate
result_data["error"] on failures (and avoid treating lack of jumpstarter_crds as
a clean "not installed" success), using the existing result_data and
jumpstarter_crds variables so callers can distinguish real errors from a genuine
"no CRDs" state.
---
Nitpick comments:
In
`@python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py`:
- Around line 220-228: The test
test_check_jumpstarter_installation_command_failure only simulates run_command
raising an exception, but check_jumpstarter_installation must also handle
non-zero exit codes; add a test case that makes mock_run_command return a
completed-result with a non-zero return code (and stderr containing "kubectl not
found") instead of raising, then call
check_jumpstarter_installation("test-context") and assert result.installed is
False, result.has_crds is False, and the error message contains the stderr text;
reference the same test function name and mock_run_command to locate where to
add this non-exception path test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c194525-ebff-4562-9606-789010160bb2
📒 Files selected for processing (86)
.cursor/rules/working-with-operator.mdc.github/workflows/build-images.yaml.github/workflows/controller-kind.yaml.github/workflows/e2e.yaml.github/workflows/lint.yamlMakefilecontroller/Makefilecontroller/deploy/helm/jumpstarter/.helmignorecontroller/deploy/helm/jumpstarter/Chart.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/Chart.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/model.pycontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/_endpoints.tplcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-deployment.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-ingress.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-route.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/cms/controller-cm.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-deployment.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-ingress.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-route.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_clients.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_exporteraccesspolicies.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_exporters.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_leases.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-ingress.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-route.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/metrics/metrics_service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/metrics/monitor.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/client_editor_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/client_viewer_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/exporter_editor_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/exporter_viewer_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/leader_election_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/leader_election_role_binding.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/role_binding.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/service_account.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-deployment.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-ingress.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-route.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.schema.jsoncontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.yamlcontroller/deploy/helm/jumpstarter/model.pycontroller/deploy/helm/jumpstarter/values.kind.yamlcontroller/deploy/helm/jumpstarter/values.schema.jsoncontroller/deploy/helm/jumpstarter/values.yamlcontroller/deploy/operator/README.mdcontroller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.gocontroller/deploy/operator/test/e2e/e2e_test.gocontroller/hack/deploy_with_helm.shcontroller/hack/deploy_with_operator.shcontroller/hack/install_helm.shcontroller/hack/utilscontroller/internal/controller/suite_test.gocontroller/internal/service/suite_test.goe2e/compat/run.she2e/compat/setup.she2e/compat/tests-old-client.batse2e/compat/tests-old-controller.batse2e/setup-e2e.she2e/test/compat_old_controller_test.goe2e/tests.batspython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/__init__.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/__init__.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install_test.py
💤 Files with no reviewable changes (64)
- .cursor/rules/working-with-operator.mdc
- controller/deploy/helm/jumpstarter/.helmignore
- controller/deploy/helm/jumpstarter/values.kind.yaml
- .github/workflows/lint.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/exporter_editor_role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/leader_election_role_binding.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/metrics/monitor.yaml
- controller/deploy/helm/jumpstarter/Chart.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/Chart.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/exporter_viewer_role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/role_binding.yaml
- controller/deploy/helm/jumpstarter/values.schema.json
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/service_account.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-service.yaml
- .github/workflows/build-images.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-ingress.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/_endpoints.tpl
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-route.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-service.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_clients.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/client_editor_role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/leader_election_role.yaml
- controller/deploy/helm/jumpstarter/values.yaml
- controller/deploy/operator/README.md
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.schema.json
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-route.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-ingress.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yaml
- controller/hack/install_helm.sh
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/client_viewer_role.yaml
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-route.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/init.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/cms/controller-cm.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-deployment.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-ingress.yaml
- controller/deploy/helm/jumpstarter/model.py
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/init.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-deployment.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-service.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-route.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_exporters.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_exporteraccesspolicies.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-deployment.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-service.yaml
- controller/hack/deploy_with_helm.sh
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install_test.py
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/init.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-ingress.yaml
- e2e/test/compat_old_controller_test.go
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/model.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/metrics/metrics_service.yaml
- e2e/compat/tests-old-controller.bats
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_leases.yaml
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py
Comment on lines
+296
to
299
| BASEDOMAIN=$(kubectl get jumpstarter -n "${JS_NAMESPACE}" jumpstarter -o jsonpath='{.spec.baseDomain}') | ||
| export ENDPOINT="grpc.${BASEDOMAIN}:8082" | ||
| export LOGIN_ENDPOINT="login.${BASEDOMAIN}:8086" | ||
| log_info "Controller endpoint: $ENDPOINT" |
Contributor
There was a problem hiding this comment.
Avoid hardcoded endpoint ports; read actual endpoints from the Jumpstarter CR.
Line 297 and Line 298 hardcode 8082/8086, which can break when deployment mode or endpoint config changes (e.g., ingress). Derive both values directly from spec to keep setup consistent with deployed config.
Proposed fix
- local BASEDOMAIN
- BASEDOMAIN=$(kubectl get jumpstarter -n "${JS_NAMESPACE}" jumpstarter -o jsonpath='{.spec.baseDomain}')
- export ENDPOINT="grpc.${BASEDOMAIN}:8082"
- export LOGIN_ENDPOINT="login.${BASEDOMAIN}:8086"
+ local CONTROLLER_ENDPOINT LOGIN_ENDPOINT_VALUE
+ CONTROLLER_ENDPOINT=$(kubectl get jumpstarter -n "${JS_NAMESPACE}" jumpstarter -o jsonpath='{.spec.controller.grpc.endpoints[0].address}')
+ LOGIN_ENDPOINT_VALUE=$(kubectl get jumpstarter -n "${JS_NAMESPACE}" jumpstarter -o jsonpath='{.spec.login.endpoints[0].address}')
+ if [ -z "${CONTROLLER_ENDPOINT}" ] || [ -z "${LOGIN_ENDPOINT_VALUE}" ]; then
+ log_error "Failed to resolve controller/login endpoints from Jumpstarter CR"
+ exit 1
+ fi
+ export ENDPOINT="${CONTROLLER_ENDPOINT}"
+ export LOGIN_ENDPOINT="${LOGIN_ENDPOINT_VALUE}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/setup-e2e.sh` around lines 296 - 299, The script currently hardcodes
ports when building ENDPOINT and LOGIN_ENDPOINT; instead query the Jumpstarter
CR for the actual endpoints/ports and use those values. Replace the kubectl
JSONPath that sets BASEDOMAIN/ENDPOINT/LOGIN_ENDPOINT with commands that read
the controller and login endpoint values from the CR spec (e.g., fields like
.spec.controllerEndpoint or .spec.endpoints.controller and .spec.loginEndpoint
or .spec.endpoints.login) and assign them to ENDPOINT and LOGIN_ENDPOINT (fall
back to the previous host:port construction only if those spec fields are
empty); update any references to BASEDOMAIN if you must derive host/port
separately so ENDPOINT and LOGIN_ENDPOINT reflect the CR configuration rather
than hardcoded ports.
e2e/tests.bats
Outdated
Comment on lines
+65
to
+69
| kubectl -n "${JS_NAMESPACE}" logs -l component=controller --tail=250 2>&1 >&2 || true | ||
|
|
||
| echo "" >&2 | ||
| echo "--- Router logs (last 250 lines) ---" >&2 | ||
| # operator uses component=router, helm uses control-plane=controller-router | ||
| kubectl -n "${JS_NAMESPACE}" logs -l component=router --tail=250 2>&1 >&2 \ | ||
| || kubectl -n "${JS_NAMESPACE}" logs -l control-plane=controller-router --tail=250 2>&1 >&2 || true | ||
| kubectl -n "${JS_NAMESPACE}" logs -l component=router --tail=250 2>&1 >&2 || true |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
(
bash -lc 'printf "stdout\n"; printf "stderr\n" >&2' 2>&1 >&2
) >"$tmpdir/current.stdout" 2>"$tmpdir/current.stderr"
(
bash -lc 'printf "stdout\n"; printf "stderr\n" >&2' >&2
) >"$tmpdir/fixed.stdout" 2>"$tmpdir/fixed.stderr"
printf 'current.stdout:\n'
cat "$tmpdir/current.stdout"
printf '\ncurrent.stderr:\n'
cat "$tmpdir/current.stderr" || true
printf '\nfixed.stdout:\n'
cat "$tmpdir/fixed.stdout" || true
printf '\nfixed.stderr:\n'
cat "$tmpdir/fixed.stderr"Repository: jumpstarter-dev/jumpstarter
Length of output: 359
Fix the redirection order in teardown log dumps.
Lines 65 and 69 use 2>&1 >&2 which sends both stdout and stderr to stdout instead of stderr. This causes kubectl errors to be routed away from Bats failure output. Use >&2 to send the command output directly to stderr.
Suggested fix
- kubectl -n "${JS_NAMESPACE}" logs -l component=controller --tail=250 2>&1 >&2 || true
+ kubectl -n "${JS_NAMESPACE}" logs -l component=controller --tail=250 >&2 || true
...
- kubectl -n "${JS_NAMESPACE}" logs -l component=router --tail=250 2>&1 >&2 || true
+ kubectl -n "${JS_NAMESPACE}" logs -l component=router --tail=250 >&2 || true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| kubectl -n "${JS_NAMESPACE}" logs -l component=controller --tail=250 2>&1 >&2 || true | |
| echo "" >&2 | |
| echo "--- Router logs (last 250 lines) ---" >&2 | |
| # operator uses component=router, helm uses control-plane=controller-router | |
| kubectl -n "${JS_NAMESPACE}" logs -l component=router --tail=250 2>&1 >&2 \ | |
| || kubectl -n "${JS_NAMESPACE}" logs -l control-plane=controller-router --tail=250 2>&1 >&2 || true | |
| kubectl -n "${JS_NAMESPACE}" logs -l component=router --tail=250 2>&1 >&2 || true | |
| kubectl -n "${JS_NAMESPACE}" logs -l component=controller --tail=250 >&2 || true | |
| echo "" >&2 | |
| echo "--- Router logs (last 250 lines) ---" >&2 | |
| kubectl -n "${JS_NAMESPACE}" logs -l component=router --tail=250 >&2 || true |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/tests.bats` around lines 65 - 69, The teardown log dump redirection is
wrong: the kubectl log commands using `kubectl -n "${JS_NAMESPACE}" logs -l
component=controller --tail=250 2>&1 >&2` and the similar router line send both
streams to stdout instead of stderr; update those two commands (`kubectl -n
"${JS_NAMESPACE}" logs -l component=controller --tail=250 ...` and `kubectl -n
"${JS_NAMESPACE}" logs -l component=router --tail=250 ...`) to redirect their
output directly to stderr (use `>&2`) so kubectl errors are shown in Bats
failure output.
python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py
Show resolved
Hide resolved
mangelajo
reviewed
Apr 9, 2026
Comment on lines
-56
to
-58
| old-controller) | ||
| label_filter="old-controller" | ||
| ;; |
Member
There was a problem hiding this comment.
why are we removing the old controller compat test?
mangelajo
reviewed
Apr 9, 2026
| # | ||
| # Environment variables: | ||
| # COMPAT_SCENARIO - "old-controller" or "old-client" (required) | ||
| # COMPAT_CONTROLLER_TAG - Controller image tag for old-controller scenario (default: v0.7.1) |
Member
There was a problem hiding this comment.
ahh, no support in 0.7.1, I think we need to deploy the old controller (now 0.8.1) with the old operator ;)
Member
There was a problem hiding this comment.
we can just deploy it with https://github.com/jumpstarter-dev/jumpstarter/releases/download/v0.8.1/operator-installer.yaml
Member
Author
There was a problem hiding this comment.
Ah, I see. Let me fix this.
Member
There was a problem hiding this comment.
If you can handle that, let's kill it 🗡️
mangelajo
requested changes
Apr 9, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/compat/setup.sh`:
- Around line 205-208: After applying the released operator-installer.yaml via
kubectl apply -f "${INSTALLER_URL}", create a Jumpstarter custom resource
instance so setup_test_environment() can read .spec.baseDomain; mirror the CR
creation from controller/hack/deploy_with_operator.sh (lines ~173-208) by
applying a Jumpstarter manifest (name as in deploy_with_operator.sh) with
spec.baseDomain set to the computed BASEDOMAIN (use the same namespace used by
the operator), ensuring this step runs immediately after the kubectl apply and
before calling setup_test_environment().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fdb9e59c-c82e-4d79-ab26-f37ea140c28b
📒 Files selected for processing (3)
.github/workflows/e2e.yamlMakefilee2e/compat/setup.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/e2e.yaml
- Makefile
Remove all Helm-related code, configuration, CI steps, and documentation from the repository. The project has transitioned to OLM operator-based deployment, making Helm artifacts dead code. Changes: - Delete controller/deploy/helm/ directory (43 files) - Delete controller/hack/deploy_with_helm.sh and install_helm.sh - Update controller Makefile to generate CRDs/RBAC directly to operator config paths and remove Helm targets - Update Go test CRD paths to use operator config/crd/bases - Remove jumpstarter_kubernetes install.py, cluster/helm.py and their test files - Remove jumpstarter_cli_admin install.py and its test file - Update all __init__.py exports to remove Helm symbols - Update cluster/operations.py to always use operator install - Update cluster/kubectl.py to use CRD-only detection - Update CLI create.py and get.py to remove Helm options - Remove publish-helm-charts job from build-images.yaml - Remove lint-helm job from lint.yaml - Remove Helm from controller-kind.yaml CI matrix - Update e2e scripts to use operator-only deployment - Update documentation and rule files Closes jumpstarter-dev#445 Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… of swallowing Flatten nested try/except in check_jumpstarter_installation so that RuntimeError from run_command (e.g. kubectl not found, permission errors) propagates to the outer handler that properly reports the error. Previously the inner `except RuntimeError: pass` silently swallowed the error, making it impossible for callers to distinguish "not installed" from "failed to check". Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rter_installation After removing the nested Helm detection logic, the function complexity is well under the C901 threshold. The suppression comment is no longer needed. Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Helm deployment method has been removed. Update the Makefile comment and help text to no longer advertise helm as a valid option. Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The chart_name/chartName field was only populated by the Helm detection code path which has been removed. The field was permanently None, making it dead structure in the API model. Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The comment said COMPAT_CONTROLLER_TAG defaults to v0.7.1 but the actual default on line 31 is v0.7.0. Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… Helm chart repo The Helm chart publishing CI job is being removed, so the function querying quay.io/jumpstarter-dev/helm/jumpstarter for version tags will return stale results. Switch to querying the operator image repository (jumpstarter-dev/jumpstarter-operator) which continues to receive new version tags with each release. Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…setup Instead of silently falling back to a default domain when the Jumpstarter CR cannot be found, fail immediately with an actionable error message. This prevents misleading test failures caused by connecting to the wrong endpoint. Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Only one deployment method (operator) remains, so the METHOD variable is unnecessary indirection. Remove it from the root Makefile and the controller Makefile, and drop redundant METHOD=operator arguments from deploy-with-operator targets. Generated-By: Forge/20260409_114748_239149_5e586097 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Since Helm support was removed, "operator" is the only deployment method. Remove the METHOD variable, its branching logic, and the parameterized print_deployment_success function throughout the e2e scripts, CI workflows, and hack utilities. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ler) v0.7.0 predates the monorepo merge and was only deployable via OCI Helm chart. With Helm fully removed, the old-controller compat test cannot deploy a v0.7.0 controller. Remove the test, its CI job, and simplify the compat infrastructure to only support the old-client scenario. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r installer)" This reverts commit fa102b3.
v0.7.0 predated the operator installer and was only deployable via Helm. Replace it with v0.8.1 which ships an operator-installer.yaml release asset. Also bump old-client version from 0.7.1 to 0.8.1 so both compat scenarios test against the previous release. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The revert commit brought back tests-old-controller.bats, but upstream converted all bats tests to ginkgo. The ginkgo equivalent (e2e/test/compat_old_controller_test.go) is the canonical test file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
raballew
force-pushed
the
042-remove-helm-charts
branch
from
0630037 to
e15a5cc
Compare
- Create Jumpstarter CR after applying operator installer in compat setup, since the released installer only contains CRDs and operator deployment, not the CR instance - Align Makefile install/uninstall/build-installer/undeploy targets with the manifests output path (deploy/operator/config/ instead of config/) - Surface non-zero kubectl exit codes as errors in check_jumpstarter_installation instead of silently reporting installed=False - Add test for non-zero exit code path in check_jumpstarter_installation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (1)
93-113:⚠️ Potential issue | 🟠 MajorHandle non-zero CRD command exits explicitly.
Line 95 only handles success. If
run_commandreturns non-zero, this currently looks like a clean “not installed” result instead of an execution failure (e.g., RBAC/context errors).Proposed fix
- returncode, stdout, _ = await run_command(crd_cmd) - - if returncode == 0: - json_start = stdout.find("{") - if json_start >= 0: - json_output = stdout[json_start:] - crds = json.loads(json_output) - else: - crds = json.loads(stdout) - jumpstarter_crds = [] - for item in crds.get("items", []): - name = item.get("metadata", {}).get("name", "") - if "jumpstarter.dev" in name: - jumpstarter_crds.append(name) - - if jumpstarter_crds: - result_data["has_crds"] = True - result_data["installed"] = True - result_data["namespace"] = namespace or "unknown" - result_data["status"] = "installed" + returncode, stdout, stderr = await run_command(crd_cmd) + if returncode != 0: + result_data["error"] = f"Failed to query CRDs (exit {returncode}): {stderr or stdout}" + return V1Alpha1JumpstarterInstance(**result_data) + + json_start = stdout.find("{") + if json_start >= 0: + json_output = stdout[json_start:] + crds = json.loads(json_output) + else: + crds = json.loads(stdout) + jumpstarter_crds = [] + for item in crds.get("items", []): + name = item.get("metadata", {}).get("name", "") + if "jumpstarter.dev" in name: + jumpstarter_crds.append(name) + + if jumpstarter_crds: + result_data["has_crds"] = True + result_data["installed"] = True + result_data["namespace"] = namespace or "unknown" + result_data["status"] = "installed"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py` around lines 93 - 113, The code currently treats any non-zero return from run_command(crd_cmd) as a benign "not installed" path; change the logic in the block that calls run_command so that when returncode != 0 you explicitly handle the execution failure: read the captured stderr (the third value returned by run_command), set result_data["status"] = "error" (or raise an informative exception), include returncode and stderr (and namespace/context) in the error information, and avoid marking result_data["installed"] = True — update the branch around run_command/crd_cmd and the result_data assignments to reflect this explicit error handling.e2e/compat/setup.sh (1)
106-109:⚠️ Potential issue | 🔴 CriticalApply a
Jumpstarterresource after the released installer.The released
operator-installer.yamlinstalls the operator/CRDs, but not thejumpstartercustom resource the operator reconciles. In theold-controllerpath that leaves nothing for the operator to deploy, so the gRPC wait loop below times out and the later.spec.baseDomainlookup also fails.Please verify against the tagged release asset. Expected result: the installer contains no
kind: Jumpstarter, which confirms this script must create one itself immediately after thekubectl apply.#!/bin/bash set -euo pipefail tag="${COMPAT_CONTROLLER_TAG:-v0.8.1}" tmp="$(mktemp)" trap 'rm -f "$tmp"' EXIT curl -LsSf \ "https://github.com/jumpstarter-dev/jumpstarter/releases/download/${tag}/operator-installer.yaml" \ -o "$tmp" echo "== resource kinds in ${tag}/operator-installer.yaml ==" rg -n '^kind:\s*' "$tmp" echo echo "== Jumpstarter CRs in installer ==" rg -n '^kind:\s*Jumpstarter$' "$tmp" || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/compat/setup.sh` around lines 106 - 109, After applying the released operator-installer.yaml (the INSTALLER_URL built from COMPAT_CONTROLLER_TAG), immediately create and apply a minimal Jumpstarter custom resource so the old operator has something to reconcile; construct a Jumpstarter manifest (apiVersion jumpstarter.dev/v1alpha1, kind Jumpstarter, metadata name e.g. "jumpstarter") with the required spec (at least spec.baseDomain set from your cluster/base domain variable) and kubectl apply it right after the kubectl apply -f "${INSTALLER_URL}" line to avoid the gRPC wait loop timing out and the later .spec.baseDomain lookup failing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/setup-e2e.sh`:
- Around line 282-288: The script assigns BASEDOMAIN from kubectl get
jumpstarter but doesn't validate it; if empty it exports invalid
ENDPOINT/LOGIN_ENDPOINT. After BASEDOMAIN=$(kubectl get jumpstarter ...), check
if BASEDOMAIN is empty/blank and if so print an error (including the
JS_NAMESPACE and the kubectl command context) and exit non‑zero to fail fast;
otherwise export ENDPOINT="grpc.${BASEDOMAIN}:8082" and
LOGIN_ENDPOINT="login.${BASEDOMAIN}:8086". Ensure this validation runs under the
existing set -e behavior so failures propagate.
---
Duplicate comments:
In `@e2e/compat/setup.sh`:
- Around line 106-109: After applying the released operator-installer.yaml (the
INSTALLER_URL built from COMPAT_CONTROLLER_TAG), immediately create and apply a
minimal Jumpstarter custom resource so the old operator has something to
reconcile; construct a Jumpstarter manifest (apiVersion
jumpstarter.dev/v1alpha1, kind Jumpstarter, metadata name e.g. "jumpstarter")
with the required spec (at least spec.baseDomain set from your cluster/base
domain variable) and kubectl apply it right after the kubectl apply -f
"${INSTALLER_URL}" line to avoid the gRPC wait loop timing out and the later
.spec.baseDomain lookup failing.
In
`@python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py`:
- Around line 93-113: The code currently treats any non-zero return from
run_command(crd_cmd) as a benign "not installed" path; change the logic in the
block that calls run_command so that when returncode != 0 you explicitly handle
the execution failure: read the captured stderr (the third value returned by
run_command), set result_data["status"] = "error" (or raise an informative
exception), include returncode and stderr (and namespace/context) in the error
information, and avoid marking result_data["installed"] = True — update the
branch around run_command/crd_cmd and the result_data assignments to reflect
this explicit error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 43ff7ac7-0567-4906-a8b0-f68d4d33eba5
📒 Files selected for processing (80)
.cursor/rules/working-with-operator.mdc.github/workflows/build-images.yaml.github/workflows/controller-kind.yaml.github/workflows/e2e.yaml.github/workflows/lint.yamlMakefilecontroller/Makefilecontroller/deploy/helm/jumpstarter/.helmignorecontroller/deploy/helm/jumpstarter/Chart.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/Chart.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/model.pycontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/_endpoints.tplcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-deployment.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-ingress.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-route.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/cms/controller-cm.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-deployment.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-ingress.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-route.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_clients.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_exporteraccesspolicies.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_exporters.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_leases.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-ingress.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-route.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/metrics/metrics_service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/metrics/monitor.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/client_editor_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/client_viewer_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/exporter_editor_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/exporter_viewer_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/leader_election_role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/leader_election_role_binding.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/role.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/role_binding.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/service_account.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-deployment.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-ingress.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-route.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-service.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yamlcontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.schema.jsoncontroller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.yamlcontroller/deploy/helm/jumpstarter/model.pycontroller/deploy/helm/jumpstarter/values.kind.yamlcontroller/deploy/helm/jumpstarter/values.schema.jsoncontroller/deploy/helm/jumpstarter/values.yamlcontroller/deploy/operator/README.mdcontroller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.gocontroller/hack/deploy_with_helm.shcontroller/hack/deploy_with_operator.shcontroller/hack/install_helm.shcontroller/hack/utilscontroller/internal/controller/suite_test.gocontroller/internal/service/suite_test.goe2e/compat/setup.she2e/setup-e2e.shpython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/__init__.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.pypython/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/__init__.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install_test.py
💤 Files with no reviewable changes (62)
- .cursor/rules/working-with-operator.mdc
- controller/deploy/helm/jumpstarter/.helmignore
- controller/deploy/helm/jumpstarter/Chart.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/Chart.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py
- controller/deploy/operator/README.md
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/exporter_editor_role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/role_binding.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/client_editor_role.yaml
- controller/deploy/helm/jumpstarter/values.schema.json
- .github/workflows/lint.yaml
- controller/deploy/helm/jumpstarter/values.kind.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/leader_election_role_binding.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/init.py
- controller/deploy/helm/jumpstarter/values.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/service_account.yaml
- controller/hack/install_helm.sh
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/exporter_viewer_role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/leader_election_role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/metrics/monitor.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-route.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/_endpoints.tpl
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-service.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_clients.yaml
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-service.yaml
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/init.py
- controller/deploy/helm/jumpstarter/model.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-service.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-ingress.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/init.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/client_viewer_role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/rbac/role.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-route.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.yaml
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-route.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/cms/controller-cm.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-ingress.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/secrets-job.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_exporters.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-ingress.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-route.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install_test.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_exporteraccesspolicies.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/values.schema.json
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-deployment.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/controller-service.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py
- controller/hack/deploy_with_helm.sh
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/router-deployment.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/crds/jumpstarter.dev_leases.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/additional-router-deployment.yaml
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/login-ingress.yaml
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/model.py
- .github/workflows/build-images.yaml
- python/packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py
- controller/deploy/helm/jumpstarter/charts/jumpstarter-controller/templates/metrics/metrics_service.yaml
✅ Files skipped from review due to trivial changes (4)
- controller/internal/controller/suite_test.go
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py
- controller/deploy/operator/internal/controller/jumpstarter/jumpstarter_controller.go
🚧 Files skipped from review as they are similar to previous changes (8)
- .github/workflows/controller-kind.yaml
- controller/internal/service/suite_test.go
- controller/hack/utils
- .github/workflows/e2e.yaml
- Makefile
- controller/Makefile
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py
If the Jumpstarter CR is missing or baseDomain is not set, exit immediately instead of exporting invalid endpoints like grpc.:8082. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py (1)
371-376: This doesn't actually exercise the customminikubepath.Because Line 372 returns no contexts,
list_clusters()exits before any downstream call can observeminikube="custom-minikube". Consider returning one context and asserting the follow-up cluster-info call receives the custom binary path too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py` around lines 371 - 376, The test test_list_clusters_custom_parameters currently returns no contexts so list_clusters exits early; change the mock_get_contexts return to include a context (e.g., ["some-context"]) so the code proceeds, then assert that the subsequent call that invokes cluster info (the mock for whichever helper is used to run minikube/cluster-info in list_clusters) was called with minikube="custom-minikube" (and keep asserting mock_get_contexts.assert_called_once_with("custom-kubectl")); update references to the mocks used in this test (mock_get_contexts and the mock for cluster-info/runner used by list_clusters) to validate both custom kubectl and custom minikube are passed through.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py`:
- Around line 371-376: The test test_list_clusters_custom_parameters currently
returns no contexts so list_clusters exits early; change the mock_get_contexts
return to include a context (e.g., ["some-context"]) so the code proceeds, then
assert that the subsequent call that invokes cluster info (the mock for
whichever helper is used to run minikube/cluster-info in list_clusters) was
called with minikube="custom-minikube" (and keep asserting
mock_get_contexts.assert_called_once_with("custom-kubectl")); update references
to the mocks used in this test (mock_get_contexts and the mock for
cluster-info/runner used by list_clusters) to validate both custom kubectl and
custom minikube are passed through.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4b6a2325-119d-4569-a0dc-4bde621153da
📒 Files selected for processing (5)
controller/Makefilee2e/compat/setup.she2e/setup-e2e.shpython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.pypython/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py
🚧 Files skipped from review as they are similar to previous changes (1)
- python/packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py
The old-client compat test deploys the new controller via deploy_new_controller(), which needs the operator image loaded into kind. Download and load the pre-built operator image and manifest, matching how the main e2e-tests job handles this. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The jumpstarter base package was never published at 0.8.1 on PyPI, making the entire 0.8.1 client stack uninstallable. Fall back to 0.7.4 which is the latest version where all packages are available. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Member
Author
|
@mangelajo ptal again. i have applied your suggestion but i think branch protection rules stop CI from passing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #445
Test plan
make pkg-test-jumpstarter_cli_adminpassesmake pkg-test-jumpstarter_kubernetespasses🤖 Generated with Claude Code