This repository was archived by the owner on Jan 23, 2026. It is now read-only.
Merged
Conversation
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
WalkthroughRefactors cluster management into a new jumpstarter_kubernetes.cluster package (split into submodules), re-exports cluster/controller APIs, adds CLI subcommands for create/get/delete cluster that delegate to the new library, updates install/uninstall to use controller version from jumpstarter_kubernetes, adjusts JSON dumping and deps, and adds extensive tests and docs updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CLI as jmp (admin CLI)
participant Ops as jumpstarter_kubernetes.cluster.operations
participant Provider as kind/minikube module
participant EP as endpoints
participant Helm as helm installer
U->>CLI: jmp admin create cluster [--kind|--minikube|auto ...]
CLI->>Ops: create_cluster_and_install(params)
Ops->>Ops: validate_cluster_type_selection(...)
alt kind
Ops->>Provider: create_kind_cluster_with_options(...)
else minikube
Ops->>Provider: create_minikube_cluster_with_options(...)
end
alt install enabled
Ops->>EP: configure_endpoints(...)
Ops->>Helm: install_jumpstarter_helm_chart(..., values_files?)
else skip install
Ops-->>CLI: cluster created (no install)
end
CLI-->>U: status/output
sequenceDiagram
autonumber
actor U as User
participant CLI as jmp (admin CLI)
participant Ops as jumpstarter_kubernetes.cluster.operations
U->>CLI: jmp admin delete cluster [--kind|--minikube|auto] [--force]
CLI->>Ops: delete_cluster_by_name(name, type?, force)
Ops->>Ops: validate/auto-detect & confirm
Ops->>Ops: delegate to delete_kind_cluster_with_feedback / delete_minikube_cluster_with_feedback
Ops-->>CLI: result/name
CLI-->>U: printed result
sequenceDiagram
autonumber
actor U as User
participant CLI as jmp (admin CLI)
participant K8s as jumpstarter_kubernetes.cluster.kubectl
U->>CLI: jmp admin get cluster [name?]
alt name provided
CLI->>K8s: get_cluster_info(name,...)
K8s-->>CLI: V1Alpha1ClusterInfo
else list
CLI->>K8s: list_clusters(...)
K8s-->>CLI: V1Alpha1ClusterList
end
CLI-->>U: printed model output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
![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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.py (1)
149-168: Same: raise on errors forget clustersMirror the error handling fix for consistent CLI behavior.
- except Exception as e: - click.echo(f"Error listing clusters: {e}", err=True) + except Exception as e: + raise click.ClickException(f"Error listing clusters: {e}") from epackages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (1)
162-171: Hard-coded cluster name breaks Minikube installsLocking
_configure_endpointsto"jumpstarter-lab"meansjmp admin install --minikubenow shells out tominikube ip -p jumpstarter-lab. On a stock Minikube setup (profile name"minikube"), this returns a non-zero exit code, soget_ip_genericraises and the install command fails unless the user manually supplies--ip. That’s a regression from the previous flow which accepted--cluster-name(defaulting to the actual Minikube profile). Please restore an argument so callers can pass the profile name and feed it through instead of the fixed literal.@click.option("-g", "--grpc-endpoint", type=str, help="The gRPC endpoint to use for the Jumpstarter API", default=None) @click.option("-r", "--router-endpoint", type=str, help="The gRPC endpoint to use for the router", default=None) +@click.option("--cluster-name", type=str, help="Name of the local cluster profile to use", default="jumpstarter-lab") @click.option("--nodeport", "mode", flag_value="nodeport", help="Use Nodeport routing (recommended)", default=True) @@ async def install( @@ - router_endpoint: Optional[str], + router_endpoint: Optional[str], + cluster_name: str, @@ - ip, basedomain, grpc_endpoint, router_endpoint = await _configure_endpoints( - cluster_type, minikube or "minikube", "jumpstarter-lab", ip, basedomain, grpc_endpoint, router_endpoint + ip, basedomain, grpc_endpoint, router_endpoint = await _configure_endpoints( + cluster_type, minikube or "minikube", cluster_name, ip, basedomain, grpc_endpoint, router_endpoint )
🧹 Nitpick comments (22)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common_test.py (3)
55-67: Empty-string inputs should not select a cluster typeTreating "" as a valid selection causes surprising outcomes (e.g., "" vs "minikube" returns "kind"). Prefer normalizing empty strings to None so only explicit selections count.
If you agree, update tests and implementation:
- result = validate_cluster_type("", "") - assert result == "kind" # First parameter is returned since "" is not None + result = validate_cluster_type("", "") + assert result is None # Empty strings treated as not provided - result = validate_cluster_type("", "minikube") - assert result == "kind" # Empty string is not None, so kind is returned + result = validate_cluster_type("", "minikube") + assert result == "minikube"Outside this file (common.py), normalize inputs before validating:
# common.py def validate_cluster_type(kind: Optional[str], minikube: Optional[str]) -> Optional[ClusterType]: kind = None if kind is not None and kind.strip() == "" else kind minikube = None if minikube is not None and minikube.strip() == "" else minikube ...
102-108: Consider early validation for custom certs pathReturning an absolute path even when the file doesn’t exist defers failure downstream. For a better UX, validate existence (isfile) and raise ClickException early when a bundle is missing. This will align with the new --custom-certs flow.
164-168: Type hint mismatch in validate_cluster_name(None) testRuntime behavior is fine, but the function signature is
validate_cluster_name(cluster_name: str). Either avoid passing None in tests or change the signature to accept Optional[str] and document None handling.packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
23-29: Handle pagination from Quay API to avoid missing newer tagsQuay’s tag API is paginated; only reading the first page risks selecting an outdated version. Consider following pagination (e.g., using
page/has_additional) or requesting a sufficiently largelimit.packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.py (2)
116-129: Avoid shadowing built-intype; use a clearer nameRename the parameter to
cluster_typeto improve readability and avoid shadowing.-@get.command("cluster") +@get.command("cluster") @@ -@click.option( - "--type", type=click.Choice(["kind", "minikube", "remote", "all"]), default="all", help="Filter clusters by type" -) +@click.option( + "--type", "cluster_type", type=click.Choice(["kind", "minikube", "remote", "all"]), default="all", + help="Filter clusters by type" +) @@ -async def get_cluster( - name: Optional[str], type: str, kubectl: str, helm: str, kind: str, minikube: str, output: OutputType -): +async def get_cluster( + name: Optional[str], cluster_type: str, kubectl: str, helm: str, kind: str, minikube: str, output: OutputType +): @@ - cluster_list = await list_clusters(type, kubectl, helm, kind, minikube, check_connectivity=False) + cluster_list = await list_clusters(cluster_type, kubectl, helm, kind, minikube, check_connectivity=False)
136-143: Fragile string match for “not found”Matching substrings in
cluster_info.erroris brittle. Prefer a structured error indicator (e.g., a booleannot_foundor error code) returned byget_cluster_info, then raise ClickException when it’s set. If changing the model is out of scope, at least match the full phraseContext "<name>" not foundexactly.packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py (2)
247-276: Mock cluster-type detection to keep tests hermetic
get_cluster_infocallsdetect_cluster_typewhich may shell out. Consider patching it in these tests to avoid environment coupling and flakiness.@patch("jumpstarter_kubernetes.cluster.kubectl.detect_cluster_type", return_value="kind")
359-366: Verify parameter pass‑through more thoroughlyYou assert custom kubectl is used; also assert
get_cluster_infowas called with custom helm and minikube to catch regressions in wiring.mock_get_cluster_info.assert_not_called() # when no contexts # Or, with contexts: # mock_get_cluster_info.assert_called_with(ANY, "custom-kubectl", "custom-helm", "custom-minikube", ANY)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints_test.py (1)
83-103: Optional: parametrize to reduce duplicationSeveral tests differ only by provided/None parameters. Parametrizing would shorten and clarify the suite without losing intent.
Also applies to: 126-167, 189-210, 212-231
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
27-31: Expand user home and normalize extra certs pathHandle "~" and return a normalized absolute path to reduce surprises when users pass tilde paths.
def get_extra_certs_path(extra_certs: Optional[str]) -> Optional[str]: """Get the absolute path to extra certificates file if provided.""" if extra_certs is None: return None - return os.path.abspath(extra_certs) + return os.path.abspath(os.path.expanduser(extra_certs))packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py (3)
128-143: Add coverage for --custom-certs pass-through in Kind pathWe’re not asserting that extra_certs propagates from _handle_cluster_creation to the wrapper. Add a test to lock this behavior.
@pytest.mark.asyncio -@patch("jumpstarter_kubernetes.cluster._create_kind_cluster") -async def test_handle_cluster_creation_kind(self, mock_create_kind): +@patch("jumpstarter_kubernetes.cluster._create_kind_cluster") +async def test_handle_cluster_creation_kind(self, mock_create_kind): await _handle_cluster_creation( create_cluster=True, cluster_type="kind", force_recreate_cluster=False, cluster_name="test-cluster", kind_extra_args="--verbosity=1", minikube_extra_args="", kind="kind", minikube="minikube", ) - mock_create_kind.assert_called_once_with("kind", "test-cluster", "--verbosity=1", False, None) + mock_create_kind.assert_called_once_with("kind", "test-cluster", "--verbosity=1", False, None) + +@pytest.mark.asyncio +@patch("jumpstarter_kubernetes.cluster._create_kind_cluster") +async def test_handle_cluster_creation_kind_with_custom_certs(self, mock_create_kind): + await _handle_cluster_creation( + create_cluster=True, + cluster_type="kind", + force_recreate_cluster=False, + cluster_name="test-cluster", + kind_extra_args="", + minikube_extra_args="", + kind="kind", + minikube="minikube", + extra_certs="/tmp/ca.crt", + ) + mock_create_kind.assert_called_once_with("kind", "test-cluster", "", False, "/tmp/ca.crt")
201-211: Assert cert preparation for Minikube and --embed-certs flag injectionAdd a focused test ensuring prepare_minikube_certs is called and --embed-certs is appended.
@pytest.mark.asyncio @patch("jumpstarter_kubernetes.cluster.minikube_installed") @patch("jumpstarter_kubernetes.cluster.create_minikube_cluster") +@patch("jumpstarter_kubernetes.cluster.operations.prepare_minikube_certs") -async def test_create_minikube_cluster_success(self, mock_create_minikube, mock_minikube_installed): +async def test_create_minikube_cluster_with_custom_certs(self, mock_prepare_certs, mock_create_minikube, mock_minikube_installed): mock_minikube_installed.return_value = True - mock_create_minikube.return_value = True + mock_create_minikube.return_value = True - await _create_minikube_cluster("minikube", "test-cluster", "--memory=4096", False) + await _create_minikube_cluster("minikube", "test-cluster", "--memory=4096", False, "/tmp/ca.crt") - mock_create_minikube.assert_called_once_with("minikube", "test-cluster", ["--memory=4096"], False) + args = mock_create_minikube.call_args[0] + assert args[:3] == ("minikube", "test-cluster", ["--memory=4096", "--embed-certs"]) + mock_prepare_certs.assert_called_once_with("/tmp/ca.crt")
347-373: Reduce brittleness of CLI output assertionsRely on presence checks rather than a strict single call to install_helm_chart and complete output shape, which can change with benign logging additions.
result = self.runner.invoke(install, ["--kind", "kind"]) assert result.exit_code == 0 -mock_install_helm.assert_called_once() +# Helm install should be invoked +assert mock_install_helm.called # Verify that kind cluster type was detected mock_validate_cluster.assert_called_once_with("kind", None)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py (2)
50-65: Avoid brittle call count checks for loggingKeeping assertions to “contains these lines” makes tests resilient to future, benign log additions.
-assert mock_click_echo.call_count == len(expected_calls) -for _, expected_call in enumerate(expected_calls): +for _, expected_call in enumerate(expected_calls): mock_click_echo.assert_any_call(expected_call)
179-185: Prefer explicit predicate over stringifying call objectsFilter success messages using call.args to avoid depending on the repr of mock calls.
-success_calls = [call for call in mock_click_echo.call_args_list if "Installed Helm release" in str(call)] +success_calls = [c for c in mock_click_echo.call_args_list if c.args and "Installed Helm release" in c.args[0]] assert len(success_calls) == 0packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.py (2)
139-142: Ensure --embed-certs is appended only once, accounting for “--embed-certs=true” formsBe robust to value forms by checking startswith to avoid duplicates.
-# Always add --embed-certs for container drivers -if "--embed-certs" not in extra_args_list: +# Always add --embed-certs for container drivers +if not any(a == "--embed-certs" or a.startswith("--embed-certs=") for a in extra_args_list): extra_args_list.append("--embed-certs")
188-245: Public surface looks consistent; minor note on dual validatorsBoth validate_cluster_type (optional) and validate_cluster_type_selection (auto-detect) are exported. Consider clarifying docstrings/names to prevent accidental misuse across CLI vs programmatic paths.
Would you like me to add short docstrings to both functions and a brief note in README to guide when to use each?
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (3)
54-63: Simplify force-recreate flow; unreachable False branch after delete
delete_kind_clusterreturns True or raises; theif not await ...: return Falsebranch never executes. Simplify and let exceptions bubble.- if cluster_exists: - if not force_recreate: - raise RuntimeError(f"Kind cluster '{cluster_name}' already exists.") - else: - if not await delete_kind_cluster(kind, cluster_name): - return False + if cluster_exists: + if not force_recreate: + raise RuntimeError(f"Kind cluster '{cluster_name}' already exists.") + else: + await delete_kind_cluster(kind, cluster_name)
21-25: Usekind get clusters -qto check existence (cheaper and more canonical)Querying clusters and matching by name avoids invoking kubeconfig generation and reduces noise.
- try: - returncode, _, _ = await run_command([kind, "get", "kubeconfig", "--name", cluster_name]) - return returncode == 0 + try: + returncode, stdout, _ = await run_command([kind, "get", "clusters", "-q"]) + return returncode == 0 and any(line.strip() == cluster_name for line in stdout.splitlines())
110-115: Moveimport osto top-levelMinor hygiene; avoid importing inside finally blocks.
import shutil import tempfile from typing import List, Optional +import os @@ - import os try: os.unlink(config_file) except OSError: passAlso applies to: 3-5
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/__init__.py (1)
80-98: Don’t export private underscore helpers in public__all__Re-exporting underscore-prefixed helpers as part of
__all__broadens the public API surface and invites external use. Keep them importable for tests, but omit from__all__."delete_kind_cluster", - "create_cluster_and_install", - "create_cluster_only", - "delete_cluster_by_name", - "get_cluster_info", - "list_clusters", - "get_ip_generic", - "list_kubectl_contexts", - "detect_cluster_type", - "check_jumpstarter_installation", - "_validate_cluster_type", - "_configure_endpoints", - "_create_kind_cluster", - "_create_minikube_cluster", - "_delete_kind_cluster", - "_delete_minikube_cluster", - "_handle_cluster_creation", - "_handle_cluster_deletion", + "create_cluster_and_install", + "create_cluster_only", + "delete_cluster_by_name", + "get_cluster_info", + "list_clusters", + "get_ip_generic", + "list_kubectl_contexts", + "detect_cluster_type", + "check_jumpstarter_installation", "get_latest_compatible_controller_version",packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py (1)
377-382: Add “get clusters” exit-code test (kubectl-compatible behavior)Add a test ensuring
jmp admin get clustersreturns exit code 0 when no clusters are found, matching kubectl semantics. Based on learnings.I can draft a small test (e.g., get_test.py) that patches the list function to return an empty list and asserts exit code 0. Want me to add it?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (34)
docs/source/conf.py(2 hunks)docs/source/getting-started/installation/service/service-local.md(3 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete_test.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py(18 hunks)packages/jumpstarter-cli-admin/pyproject.toml(1 hunks)packages/jumpstarter-cli-common/jumpstarter_cli_common/print.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/__init__.py(3 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster.py(0 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster_test.py(0 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py(2 hunks)packages/jumpstarter-kubernetes/pyproject.toml(1 hunks)
💤 Files with no reviewable changes (2)
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster.py
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster_test.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
📚 Learning: 2025-05-28T15:09:35.768Z
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
Applied to files:
docs/source/getting-started/installation/service/service-local.mdpackages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py
🧬 Code graph analysis (27)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (2)
get_cluster_info(195-279)list_clusters(282-320)packages/jumpstarter-cli-common/jumpstarter_cli_common/print.py (1)
model_print(9-75)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (6)
format_cluster_name(34-36)get_extra_certs_path(27-31)run_command(46-55)run_command_with_output(58-64)validate_cluster_name(39-43)validate_cluster_type(12-24)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
create_kind_cluster(44-115)delete_kind_cluster(28-41)kind_cluster_exists(16-25)kind_installed(10-12)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (5)
check_jumpstarter_installation(72-192)check_kubernetes_access(12-23)get_cluster_info(195-279)get_kubectl_contexts(26-69)list_clusters(282-320)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (2)
V1Alpha1ClusterInfo(26-76)V1Alpha1JumpstarterInstance(9-23)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (2)
configure_endpoints(28-47)get_ip_generic(11-25)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (1)
ip(187-196)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (4)
create_minikube_cluster(45-90)delete_minikube_cluster(29-42)minikube_cluster_exists(17-26)minikube_installed(10-12)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (3)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (3)
V1Alpha1ClusterInfo(26-76)V1Alpha1ClusterList(79-94)V1Alpha1JumpstarterInstance(9-23)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
run_command(46-55)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (1)
detect_cluster_type(105-140)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
packages/jumpstarter/jumpstarter/common/exceptions.py (1)
FileNotFoundError(66-69)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/json.py (1)
JsonBaseModel(5-14)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/list.py (1)
V1Alpha1List(10-15)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (2)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (2)
install(141-171)ip(187-196)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (1)
install_helm_chart(11-60)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (2)
run_command(46-55)run_command_with_output(58-64)packages/jumpstarter/jumpstarter/common/ipaddr.py (1)
get_minikube_ip(29-47)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py (2)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py (3)
TestClusterCreation(97-195)setup_method(291-292)setup_method(528-529)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py (1)
create(39-40)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (2)
run_command(46-55)run_command_with_output(58-64)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (9)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (1)
helm_installed(6-8)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (2)
run_command_with_output(58-64)validate_cluster_name(39-43)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (3)
auto_detect_cluster_type(91-102)detect_existing_cluster_type(59-88)detect_kind_provider(29-56)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (1)
configure_endpoints(28-47)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
install_jumpstarter_helm_chart(10-38)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
create_kind_cluster(44-115)delete_kind_cluster(28-41)kind_cluster_exists(16-25)kind_installed(10-12)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (4)
create_minikube_cluster(45-90)delete_minikube_cluster(29-42)minikube_cluster_exists(17-26)minikube_installed(10-12)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py (1)
create_cluster(236-303)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.py (2)
_create_kind_cluster(84-119)_create_minikube_cluster(122-157)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (3)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
run_command(46-55)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (2)
kind_cluster_exists(16-25)kind_installed(10-12)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (2)
minikube_cluster_exists(17-26)minikube_installed(10-12)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (5)
packages/jumpstarter-cli-common/jumpstarter_cli_common/version.py (2)
version(49-56)get_client_version(12-14)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
get_latest_compatible_controller_version(9-61)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (3)
helm_installed(6-8)install_helm_chart(11-60)uninstall_helm_chart(63-85)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (1)
minikube_installed(10-12)packages/jumpstarter/jumpstarter/common/ipaddr.py (2)
get_ip_address(7-26)get_minikube_ip(29-47)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (1)
minikube_installed(10-12)packages/jumpstarter/jumpstarter/common/ipaddr.py (2)
get_ip_address(7-26)get_minikube_ip(29-47)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py (4)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (2)
_validate_cluster_type(18-30)ip(187-196)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (1)
create_cluster_and_install(233-306)packages/jumpstarter-cli-common/jumpstarter_cli_common/version.py (2)
version(49-56)get_client_version(12-14)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
get_latest_compatible_controller_version(9-61)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete_test.py (2)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py (3)
TestClusterDeletion(463-522)setup_method(291-292)setup_method(528-529)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py (1)
delete(28-29)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.py (8)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (6)
format_cluster_name(34-36)get_extra_certs_path(27-31)run_command(46-55)run_command_with_output(58-64)validate_cluster_name(39-43)validate_cluster_type(12-24)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (3)
auto_detect_cluster_type(91-102)detect_cluster_type(105-140)detect_existing_cluster_type(59-88)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (2)
configure_endpoints(28-47)get_ip_generic(11-25)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
install_jumpstarter_helm_chart(10-38)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (5)
create_kind_cluster(44-115)delete_kind_cluster(28-41)kind_cluster_exists(16-25)kind_installed(10-12)list_kind_clusters(118-130)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (5)
check_jumpstarter_installation(72-192)check_kubernetes_access(12-23)get_cluster_info(195-279)get_kubectl_contexts(26-69)list_clusters(282-320)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (6)
create_minikube_cluster(45-90)delete_minikube_cluster(29-42)get_minikube_cluster_ip(110-112)list_minikube_clusters(93-107)minikube_cluster_exists(17-26)minikube_installed(10-12)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (7)
_handle_cluster_creation(333-374)create_cluster_and_install(233-306)create_cluster_only(309-330)delete_cluster_by_name(191-230)validate_cluster_type_selection(177-188)inject_certs_in_kind(18-48)prepare_minikube_certs(51-75)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py (3)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.py (4)
_create_kind_cluster(84-119)_create_minikube_cluster(122-157)_delete_kind_cluster(160-170)_delete_minikube_cluster(173-183)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (1)
_handle_cluster_creation(333-374)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (2)
install(141-171)_configure_endpoints(40-58)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (5)
auto_detect_cluster_type(91-102)detect_cluster_type(105-140)detect_container_runtime(15-26)detect_existing_cluster_type(59-88)detect_kind_provider(29-56)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (9)
create_cluster_and_install(233-306)create_cluster_only(309-330)create_kind_cluster_wrapper(78-109)create_minikube_cluster_wrapper(112-148)delete_cluster_by_name(191-230)delete_kind_cluster_wrapper(151-161)delete_minikube_cluster_wrapper(164-174)inject_certs_in_kind(18-48)prepare_minikube_certs(51-75)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/__init__.py (10)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (3)
_configure_endpoints(40-58)_validate_cluster_type(18-30)get_ip_generic(91-104)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.py (4)
_create_kind_cluster(84-119)_create_minikube_cluster(122-157)_delete_kind_cluster(160-170)_delete_minikube_cluster(173-183)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (5)
_handle_cluster_creation(333-374)_handle_cluster_deletion(377-383)create_cluster_and_install(233-306)create_cluster_only(309-330)delete_cluster_by_name(191-230)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (3)
check_jumpstarter_installation(72-192)get_cluster_info(195-279)list_clusters(282-320)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (3)
create_kind_cluster(44-115)delete_kind_cluster(28-41)kind_installed(10-12)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (3)
create_minikube_cluster(45-90)delete_minikube_cluster(29-42)minikube_installed(10-12)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (1)
detect_cluster_type(105-140)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (1)
get_ip_generic(11-25)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (3)
V1Alpha1ClusterInfo(26-76)V1Alpha1ClusterList(79-94)V1Alpha1JumpstarterInstance(9-23)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
get_latest_compatible_controller_version(9-61)
packages/jumpstarter-cli-common/jumpstarter_cli_common/print.py (1)
packages/jumpstarter/jumpstarter/client/grpc.py (1)
model_dump(238-249)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
install_jumpstarter_helm_chart(10-38)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (1)
delete_cluster_by_name(191-230)
🪛 markdownlint-cli2 (0.18.1)
docs/source/getting-started/installation/service/service-local.md
44-44: Dollar signs used before commands without showing output
(MD014, commands-show-output)
120-120: Dollar signs used before commands without showing output
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: e2e
🔇 Additional comments (8)
packages/jumpstarter-cli-admin/pyproject.toml (1)
9-13: All packaging/semver imports removed Verified no remainingpackagingorsemverimports inpackages/jumpstarter-cli-admin.packages/jumpstarter-kubernetes/pyproject.toml (1)
14-17: Dependency-groups need packaging 25+Good call bumping to
packaging>=25.0; tooling prior to 25.0 can’t parse PEP 751dependency_groupsmetadata, so this guards against resolver crashes when our project metadata is inspected. 👍packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common_test.py (1)
181-197: Nice coverage of async subprocess helpersMocks assert the right exec args and decoding/stripping behavior; not-found errors are surfaced cleanly as RuntimeError. LGTM.
Also applies to: 213-217, 219-229, 231-246
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.py (1)
18-20: Import usage looks correctRe-exported functions from jumpstarter_kubernetes are used directly. LGTM.
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py (1)
19-71: Comprehensive coverage; assertions read wellGood use of AsyncIO patches and explicit stderr messages. JSON parsing and error paths are exercised thoroughly. LGTM.
Also applies to: 73-165, 166-246, 303-366
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints_test.py (1)
11-33: Solid, scenario‑driven testsCovers success and failure paths for Minikube/Kind and all defaulting logic for endpoints. Mocks are precise; outputs validated clearly. LGTM.
Also applies to: 44-78, 80-115, 126-188, 190-210, 212-231, 233-251, 253-267
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/__init__.py (1)
23-25:list_kubectl_contextsexport verified
Defined viafrom .kubectl import get_kubectl_contexts as list_kubectl_contextsincluster/__init__.pyand included in the module’s__all__.packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (1)
64-92: Make NodePort range and host port mappings configurable
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (lines 64–92) hard-codes the NodePort range (3000–32767) and specific hostPort mappings (5080/5443/8082/8083); extract these into function parameters or CLI flags to avoid conflicts on developer machines.
- Unable to locate any Helm chart values in the repo—please verify that these NodePort assignments match your service chart defaults.
docs/source/getting-started/installation/service/service-local.md
Outdated
Show resolved
Hide resolved
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py
Show resolved
Hide resolved
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py
Outdated
Show resolved
Hide resolved
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py
Show resolved
Hide resolved
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py
Outdated
Show resolved
Hide resolved
mangelajo
reviewed
Oct 2, 2025
docs/source/getting-started/installation/service/service-local.md
Outdated
Show resolved
Hide resolved
kirkbrauer
force-pushed
the
improve-cluster-setup
branch
from
3f7f08b to
4855326
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (1)
docs/source/getting-started/installation/service/service-local.md (1)
18-21: Polish the new bullet descriptionTo stay consistent with the other entries (imperative verbs) and prior guidance, please tweak this bullet to read “Lists the local clusters detected from your kubeconfig contexts” (or similar). That keeps the grammar aligned with the surrounding list.
🧹 Nitpick comments (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
22-27: Tighten the callback type annotation
callbackdefaults toNone, so its type should beOptional[OutputCallback]; today’s signature violates the protocol contract and will upset type checkers. Update the annotation accordingly to keep static analysis quiet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
docs/source/getting-started/installation/service/service-local.md(3 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py(3 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py(3 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete_test.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.py(2 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py(2 hunks)packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/__init__.py(3 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/__init__.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/test_leases.py(0 hunks)
💤 Files with no reviewable changes (1)
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/test_leases.py
✅ Files skipped from review due to trivial changes (1)
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/init.py
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-28T15:09:35.768Z
Learnt from: NickCao
PR: jumpstarter-dev/jumpstarter#463
File: packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py:270-270
Timestamp: 2025-05-28T15:09:35.768Z
Learning: The jumpstarter CLI is designed to match kubectl's behavior, including returning exit code 0 (success) when no resources are found, rather than exit code 1 (failure).
Applied to files:
docs/source/getting-started/installation/service/service-local.md
🧬 Code graph analysis (22)
packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (20)
progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)warning(23-25)warning(51-53)warning(82-84)warning(114-116)error(27-29)error(55-57)error(86-88)error(118-120)confirm(31-33)confirm(59-61)confirm(90-93)confirm(122-124)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py (2)
ClickCallback(10-47)ForceClickCallback(50-59)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (1)
delete_cluster_by_name(46-93)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (1)
JumpstarterKubernetesError(8-11)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (2)
get_cluster_info(198-281)list_clusters(284-321)packages/jumpstarter-cli-common/jumpstarter_cli_common/print.py (1)
model_print(9-75)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (2)
ClusterTypeValidationError(79-82)ClusterNameValidationError(85-90)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (2)
ToolNotInstalledError(14-22)ClusterOperationError(48-60)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
run_command(52-76)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (2)
kind_cluster_exists(25-34)kind_installed(19-21)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (2)
minikube_cluster_exists(26-74)minikube_installed(21-23)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (10)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (1)
install_helm_chart(11-60)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py (5)
packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py (1)
ClickCallback(10-47)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (2)
create_cluster_and_install(96-184)validate_cluster_type_selection(32-43)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (1)
JumpstarterKubernetesError(8-11)packages/jumpstarter-cli-common/jumpstarter_cli_common/version.py (2)
version(49-56)get_client_version(12-14)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
get_latest_compatible_controller_version(9-74)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/__init__.py (6)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (3)
check_jumpstarter_installation(75-195)get_cluster_info(198-281)list_clusters(284-321)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (4)
create_cluster_and_install(96-184)create_cluster_only(187-210)delete_cluster_by_name(46-93)validate_cluster_type_selection(32-43)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (1)
detect_cluster_type(112-147)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (1)
get_ip_generic(10-24)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (3)
V1Alpha1ClusterInfo(26-76)V1Alpha1ClusterList(79-94)V1Alpha1JumpstarterInstance(9-23)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
get_latest_compatible_controller_version(9-74)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (4)
create_minikube_cluster(100-157)delete_minikube_cluster(77-97)minikube_cluster_exists(26-74)minikube_installed(21-23)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (2)
ToolNotInstalledError(14-22)ClusterOperationError(48-60)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (1)
packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py (6)
progress(21-24)success(26-29)warning(31-34)error(36-39)confirm(41-47)confirm(57-59)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (3)
V1Alpha1ClusterInfo(26-76)V1Alpha1ClusterList(79-94)V1Alpha1JumpstarterInstance(9-23)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
run_command(52-76)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (1)
KubeconfigError(71-76)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (1)
detect_cluster_type(112-147)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (9)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (18)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)confirm(31-33)confirm(59-61)confirm(90-93)confirm(122-124)success(19-21)success(47-49)success(78-80)success(110-112)warning(23-25)warning(51-53)warning(82-84)warning(114-116)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (5)
ClusterNameValidationError(85-90)ClusterNotFoundError(25-35)ClusterOperationError(48-60)ClusterTypeValidationError(79-82)ToolNotInstalledError(14-22)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (1)
helm_installed(6-8)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
validate_cluster_name(44-49)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (2)
auto_detect_cluster_type(97-109)detect_existing_cluster_type(59-94)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (1)
configure_endpoints(27-46)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
install_jumpstarter_helm_chart(9-41)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
create_kind_cluster_with_options(179-215)delete_kind_cluster_with_feedback(218-229)kind_cluster_exists(25-34)kind_installed(19-21)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (4)
create_minikube_cluster_with_options(211-247)delete_minikube_cluster_with_feedback(250-263)minikube_cluster_exists(26-74)minikube_installed(21-23)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py (3)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py (1)
TestClusterCreation(63-114)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete_test.py (1)
setup_method(257-258)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py (1)
create(41-42)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints_test.py (3)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (2)
configure_endpoints(27-46)get_ip_generic(10-24)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (2)
ToolNotInstalledError(14-22)EndpointConfigurationError(93-98)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (1)
ip(187-196)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete_test.py (2)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py (1)
setup_method(380-381)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py (1)
delete(30-31)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (5)
auto_detect_cluster_type(97-109)detect_cluster_type(112-147)detect_container_runtime(14-26)detect_existing_cluster_type(59-94)detect_kind_provider(29-56)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (2)
ToolNotInstalledError(14-22)ClusterOperationError(48-60)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
install_jumpstarter_helm_chart(9-41)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (8)
progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (2)
EndpointConfigurationError(93-98)ToolNotInstalledError(14-22)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (1)
minikube_installed(21-23)packages/jumpstarter/jumpstarter/common/ipaddr.py (2)
get_ip_address(7-26)get_minikube_ip(29-47)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (2)
get_ip_generic(91-104)ip(187-196)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install_test.py (5)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (1)
SilentCallback(36-61)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (1)
create_kind_cluster_with_options(179-215)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (1)
create_minikube_cluster_with_options(211-247)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (1)
validate_cluster_type_selection(32-43)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (2)
ClusterTypeValidationError(79-82)ToolNotInstalledError(14-22)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (10)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (4)
CertificateError(63-68)ClusterAlreadyExistsError(38-45)ClusterOperationError(48-60)ToolNotInstalledError(14-22)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (2)
run_command(52-76)run_command_with_output(79-95)packages/jumpstarter/jumpstarter/common/ipaddr.py (1)
get_minikube_ip(29-47)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (10)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (4)
CertificateError(63-68)ClusterAlreadyExistsError(38-45)ClusterOperationError(48-60)ToolNotInstalledError(14-22)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (2)
run_command(52-76)run_command_with_output(79-95)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (1)
detect_kind_provider(29-56)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (3)
create_cluster_and_install(96-184)create_cluster_only(187-210)delete_cluster_by_name(46-93)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (3)
ClusterNotFoundError(25-35)ToolNotInstalledError(14-22)ClusterOperationError(48-60)
🪛 GitHub Check: ruff
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py
[failure] 97-97: Ruff (E501)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py:97:121: E501 Line too long (121 > 120)
🪛 markdownlint-cli2 (0.18.1)
docs/source/getting-started/installation/service/service-local.md
45-45: Dollar signs used before commands without showing output
(MD014, commands-show-output)
121-121: Dollar signs used before commands without showing output
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (1)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py (1)
208-211: Allow~/env vars in--extra-certspaths
click.Path(resolve_path=True)doesn't expand the user’s home (~) or env vars, so a very common input like--extra-certs ~/corp.pemfails validation. Enable expansion so the new feature works with home-directory bundles.@click.option( "--extra-certs", - type=click.Path(exists=True, readable=True, dir_okay=False, resolve_path=True), + type=click.Path( + exists=True, + readable=True, + dir_okay=False, + resolve_path=True, + expand_user=True, + ),Likely an incorrect or invalid review comment.
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py
Show resolved
Hide resolved
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py
Show resolved
Hide resolved
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py
Show resolved
Hide resolved
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py (1)
80-82: Wrap long assertion to satisfy Ruff E501.Similar to the minikube test below (lines 91-102, which was correctly fixed), this assertion line exceeds the 120-character limit. Please split the call across multiple lines.
Apply this diff to fix the line length:
mock_create_and_install.assert_called_once_with( - "kind", False, "test-cluster", "", "", "kind", "minikube", None, install_jumpstarter=False, callback=None + "kind", + False, + "test-cluster", + "", + "", + "kind", + "minikube", + None, + install_jumpstarter=False, + callback=None, )
🧹 Nitpick comments (9)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py (2)
1234-1336: Move imports to module level for consistency.The imports of
V1Alpha1ClusterInfo,V1Alpha1ClusterList, andV1Alpha1JumpstarterInstanceat lines 1237, 1260, and 1292 are redundant since these types are already imported at the top of the file (lines 4-20). Remove the duplicate imports from inside the test functions.Apply this diff to clean up the imports:
@patch("jumpstarter_cli_admin.get.get_cluster_info") def test_get_cluster_by_name(get_cluster_info_mock: AsyncMock): - from jumpstarter_kubernetes import V1Alpha1ClusterInfo, V1Alpha1JumpstarterInstance - runner = CliRunner() cluster_info = V1Alpha1ClusterInfo(@patch("jumpstarter_cli_admin.get.get_cluster_info") def test_get_cluster_not_found(get_cluster_info_mock: AsyncMock): - from jumpstarter_kubernetes import V1Alpha1ClusterInfo, V1Alpha1JumpstarterInstance - runner = CliRunner() cluster_info = V1Alpha1ClusterInfo(@patch("jumpstarter_cli_admin.get.list_clusters") def test_get_clusters_list(list_clusters_mock: AsyncMock): - from jumpstarter_kubernetes import V1Alpha1ClusterInfo, V1Alpha1ClusterList, V1Alpha1JumpstarterInstance - runner = CliRunner() cluster_list = V1Alpha1ClusterList(
1234-1336: Consider adding output format tests for cluster commands.The cluster tests only verify the default (table) output format. For consistency with tests for other resources (clients, exporters, leases), consider adding tests for JSON, YAML, and name output formats. This would ensure the cluster commands properly support all output options.
Example test structure to add:
@patch("jumpstarter_cli_admin.get.get_cluster_info") def test_get_cluster_json_output(get_cluster_info_mock: AsyncMock): runner = CliRunner() cluster_info = V1Alpha1ClusterInfo(...) get_cluster_info_mock.return_value = cluster_info result = runner.invoke(get, ["cluster", "kind-test", "--output", "json"]) assert result.exit_code == 0 assert json_equal(result.output, EXPECTED_JSON)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exporters_test.py (2)
108-167: Consider adding edge case tests for rich table rendering.The existing tests for rich_add_columns and rich_add_rows are well-structured and use appropriate mocking. However, consider adding tests for edge cases such as:
- Exporters with empty device lists
- Exporters with None status
- Exporters with devices that have empty or None labels
These scenarios may occur in real usage and should be tested to ensure robust table rendering.
177-262: Consider adding test for empty exporter list.The list tests are comprehensive and properly validate delegation to individual exporter methods. However, consider adding a test for an empty V1Alpha1ExporterList to ensure the rich rendering methods handle this edge case gracefully.
Example test to add:
def test_exporter_list_empty(): """Test V1Alpha1ExporterList with no items""" from unittest.mock import MagicMock from jumpstarter_kubernetes import V1Alpha1ExporterList exporter_list = V1Alpha1ExporterList(items=[]) mock_table = MagicMock() exporter_list.rich_add_rows(mock_table, devices=False) assert mock_table.add_row.call_count == 0 names = [] exporter_list.rich_add_names(names) assert names == []packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clients_test.py (2)
126-236: Consider consolidating imports at module level.Several tests import
MagicMockandV1Alpha1ClientListinside the function body (lines 126, 137, 146, 168, 210, 221, 231), whileV1Alpha1Clientis imported at the top. For consistency and clarity, consider moving these to the module-level imports.Apply this diff to consolidate imports:
+from unittest.mock import MagicMock + from kubernetes_asyncio.client.models import V1ObjectMeta -from jumpstarter_kubernetes import V1Alpha1Client, V1Alpha1ClientStatus +from jumpstarter_kubernetes import V1Alpha1Client, V1Alpha1ClientList, V1Alpha1ClientStatusThen remove the inline imports from the test functions.
194-194: Invalid UUID format in test data.The UID contains non-hexadecimal characters:
"8b36fc92-7554-48fd-b73g-61276cggfef9"(note the 'g' characters). While this doesn't affect test functionality since it's just test data, using a valid UUID format would better represent production data.Apply this diff:
- "uid": "8b36fc92-7554-48fd-b73g-61276cggfef9", + "uid": "8b36fc92-7554-48fd-b73f-61276cffef9",packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters_test.py (3)
6-17: Verify column order withassert_has_calls.Using
assert_any_callchecks that each column was added but doesn't verify the order. Since column order matters for table display, consider usingassert_has_callswithordered=Trueto ensure columns are added in the correct sequence.Apply this diff:
def test_cluster_info_rich_add_columns(): """Test that ClusterInfo can add columns to a rich table""" mock_table = MagicMock() V1Alpha1ClusterInfo.rich_add_columns(mock_table) assert mock_table.add_column.call_count == 7 - mock_table.add_column.assert_any_call("CURRENT") - mock_table.add_column.assert_any_call("NAME") - mock_table.add_column.assert_any_call("TYPE") - mock_table.add_column.assert_any_call("STATUS") - mock_table.add_column.assert_any_call("JUMPSTARTER") - mock_table.add_column.assert_any_call("VERSION") - mock_table.add_column.assert_any_call("NAMESPACE") + from unittest.mock import call + expected_calls = [ + call("CURRENT"), + call("NAME"), + call("TYPE"), + call("STATUS"), + call("JUMPSTARTER"), + call("VERSION"), + call("NAMESPACE"), + ] + mock_table.add_column.assert_has_calls(expected_calls, any_order=False)
77-97: Add assertions for all row fields in error test.The test only verifies the
jumpstarterstatus field (args[4]) but doesn't check that other fields are still rendered correctly when an error occurs. Consider adding assertions for all seven row fields to ensure the error case is fully covered.Apply this diff:
cluster_info.rich_add_rows(mock_table) mock_table.add_row.assert_called_once() args = mock_table.add_row.call_args[0] + assert args[0] == "*" # current + assert args[1] == "test-cluster" # name + assert args[2] == "kind" # type + assert args[3] == "Running" # status assert args[4] == "Error" # jumpstarter status shows error + assert args[5] == "-" # version placeholder when error + assert args[6] == "-" # namespace placeholder when error
1-188: Consider using pytest fixtures to reduce test data duplication.Several tests create similar
V1Alpha1ClusterInfoinstances with only minor variations. Extracting common test data into pytest fixtures would reduce duplication and improve maintainability.Example approach:
import pytest @pytest.fixture def base_cluster_info(): """Base cluster info with common fields""" return { "name": "test-cluster", "cluster": "test-cluster", "server": "https://127.0.0.1:6443", "user": "test-user", "namespace": "default", } @pytest.fixture def running_cluster_with_jumpstarter(base_cluster_info): """Cluster that's current, running, with Jumpstarter installed""" return V1Alpha1ClusterInfo( **base_cluster_info, is_current=True, type="kind", accessible=True, jumpstarter=V1Alpha1JumpstarterInstance( installed=True, version="0.1.0", namespace="jumpstarter", ), ) def test_cluster_info_rich_add_rows_current_running_installed(running_cluster_with_jumpstarter): """Test rich table row for current, running cluster with Jumpstarter installed""" mock_table = MagicMock() running_cluster_with_jumpstarter.rich_add_rows(mock_table) # ... assertions
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py(2 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clients_test.py(2 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/datetime_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exporters_test.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (3)
ForceCallback(96-124)LoggingCallback(64-93)SilentCallback(36-61)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/datetime_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/datetime.py (1)
time_since(4-32)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clients_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clients.py (10)
V1Alpha1Client(27-69)from_dict(34-54)from_dict(76-77)rich_add_columns(57-60)rich_add_columns(80-81)rich_add_rows(62-66)rich_add_rows(83-85)rich_add_names(68-69)rich_add_names(87-89)V1Alpha1ClientList(72-89)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (3)
V1Alpha1ClusterInfo(26-76)V1Alpha1ClusterList(79-94)V1Alpha1JumpstarterInstance(9-23)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clients.py (1)
ClientsV1Alpha1Api(92-165)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exporters.py (1)
ExportersV1Alpha1Api(123-200)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/leases.py (1)
LeasesV1Alpha1Api(151-166)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (3)
V1Alpha1ClusterInfo(26-76)V1Alpha1JumpstarterInstance(9-23)V1Alpha1ClusterList(79-94)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (3)
create_cluster_and_install(96-184)create_cluster_only(187-210)delete_cluster_by_name(46-93)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (3)
ClusterNotFoundError(25-35)ToolNotInstalledError(14-22)ClusterOperationError(48-60)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exporters_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exporters.py (10)
V1Alpha1Exporter(31-100)from_dict(38-59)from_dict(107-108)rich_add_columns(62-73)rich_add_columns(111-112)rich_add_rows(75-97)rich_add_rows(114-116)rich_add_names(99-100)rich_add_names(118-120)V1Alpha1ExporterList(103-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (14)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py (3)
15-68: Well-structured deletion tests.The test class thoroughly covers the delete_cluster_by_name operation, including auto-detection paths for both kind and minikube, explicit type specification, and error handling for non-existent clusters. The use of
ANYfor callback arguments is appropriate.
84-102: LGTM! Proper line formatting.The minikube test correctly splits the assertion arguments across multiple lines, satisfying the line length requirement.
105-149: Comprehensive installation tests.The test class effectively covers the create_cluster_and_install operation, including the success path with proper mocking of helm, cluster creation, endpoint configuration, and chart installation. Error paths are also well-tested for missing helm and missing version scenarios.
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks_test.py (3)
7-34: LGTM! Comprehensive coverage of SilentCallback.The test suite thoroughly validates SilentCallback's no-op behavior for all message methods and confirms the auto-confirm behavior for prompts. The tests are clear, well-documented, and appropriate for the implementation.
36-85: LGTM! Excellent test coverage with proper mocking.The test suite effectively validates LoggingCallback's logging behavior:
- Verifies both default and custom logger initialization
- Confirms correct log levels for each message type
- Validates the exact confirmation message format
- Uses proper mocking techniques to isolate the logger
87-136: LGTM! Thorough validation of forwarding behavior.The test suite comprehensively validates ForceCallback's delegation pattern:
- Verifies default and custom output callback initialization
- Confirms all message methods correctly forward to the output callback
- Critically validates that
confirm()returns True without forwarding (line 136), which is the key behavior for force modeThe use of
assert_not_called()on line 136 is particularly important for verifying the intended non-forwarding behavior of confirmations.packages/jumpstarter-cli-admin/jumpstarter_cli_admin/get_test.py (2)
23-23: LGTM!The import of
ConfigExceptionis necessary for the new kubeconfig error handling tests.
1203-1231: LGTM!The ConfigException tests properly verify error handling when kubeconfig issues occur. The tests follow the existing pattern used for ApiException tests and correctly check for kubeconfig-related error messages.
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exporters_test.py (4)
1-21: LGTM! Well-structured test fixture.The imports are clean and the TEST_EXPORTER fixture provides comprehensive test data that's reused effectively across multiple tests.
24-77: LGTM! Comprehensive serialization tests.Both JSON and YAML serialization tests validate the complete output format, ensuring proper field naming and structure.
80-105: LGTM! Solid deserialization test.The test validates from_dict correctly reconstructs the object and verifies key fields including metadata, status, and nested device information.
170-174: LGTM! Clean test for name formatting.The test correctly validates the expected name format with the jumpstarter.dev namespace.
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clients_test.py (1)
62-236: Excellent test coverage!The new test suite comprehensively covers V1Alpha1Client and V1Alpha1ClientList functionality including:
- Deserialization scenarios (with/without credential, with/without status)
- Rich table rendering (columns, rows, names)
- Both single-item and list operations
All assertions correctly match the implementation from
clients.py.packages/jumpstarter-kubernetes/jumpstarter_kubernetes/datetime_test.py (1)
1-147: LGTM! Comprehensive test coverage with correct mocking approach.The test suite thoroughly covers all branches of the
time_sincefunction across different time ranges (seconds, minutes, hours, days, months, years). The mocking strategy is correct:
mock_datetime.strptime.return_value = past.replace(tzinfo=None)accurately simulates realstrptimebehavior (returns naive datetime)- The implementation correctly adds UTC timezone via
.replace(tzinfo=timezone.utc)mock_datetime.now.return_value = nowprovides deterministic test behaviorTest structure is clear with descriptive names and docstrings. The month/year approximations (30 days/month, 365 days/year) are appropriate for relative time display.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py(3 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete_test.py(2 hunks)packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py(1 hunks)packages/jumpstarter-kubernetes/pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl_test.py
🧰 Additional context used
🧬 Code graph analysis (9)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py (3)
packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py (3)
ClickCallback(10-47)ForceClickCallback(50-59)SilentWithConfirmCallback(62-75)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (1)
delete_cluster_by_name(46-100)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (1)
JumpstarterKubernetesError(8-11)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (1)
JumpstarterKubernetesError(8-11)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete_test.py (3)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create_test.py (1)
setup_method(380-381)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/delete.py (1)
delete(30-31)packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py (2)
SilentWithConfirmCallback(62-75)ForceClickCallback(50-59)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (10)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (4)
CertificateError(63-68)ClusterAlreadyExistsError(38-45)ClusterOperationError(48-60)ToolNotInstalledError(14-22)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (2)
run_command(52-76)run_command_with_output(79-95)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (1)
detect_kind_provider(29-56)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (9)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (18)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)confirm(31-33)confirm(59-61)confirm(90-93)confirm(122-124)success(19-21)success(47-49)success(78-80)success(110-112)warning(23-25)warning(51-53)warning(82-84)warning(114-116)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (5)
ClusterNameValidationError(89-94)ClusterNotFoundError(25-35)ClusterOperationError(48-60)ClusterTypeValidationError(79-86)ToolNotInstalledError(14-22)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (1)
helm_installed(6-8)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
validate_cluster_name(44-49)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (2)
auto_detect_cluster_type(97-109)detect_existing_cluster_type(59-94)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (1)
configure_endpoints(27-46)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
install_jumpstarter_helm_chart(9-41)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
create_kind_cluster_with_options(179-215)delete_kind_cluster_with_feedback(218-229)kind_cluster_exists(25-34)kind_installed(19-21)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (4)
create_minikube_cluster_with_options(211-247)delete_minikube_cluster_with_feedback(250-263)minikube_cluster_exists(26-74)minikube_installed(21-23)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kubectl.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/clusters.py (3)
V1Alpha1ClusterInfo(26-76)V1Alpha1ClusterList(79-94)V1Alpha1JumpstarterInstance(9-23)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
run_command(52-76)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (1)
KubeconfigError(71-76)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (1)
detect_cluster_type(112-147)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (3)
create_cluster_and_install(103-191)create_cluster_only(194-217)delete_cluster_by_name(46-100)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (4)
ClusterNotFoundError(25-35)ClusterTypeValidationError(79-86)ToolNotInstalledError(14-22)ClusterOperationError(48-60)
packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (20)
progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)warning(23-25)warning(51-53)warning(82-84)warning(114-116)error(27-29)error(55-57)error(86-88)error(118-120)confirm(31-33)confirm(59-61)confirm(90-93)confirm(122-124)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (2)
configure_endpoints(27-46)get_ip_generic(10-24)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (2)
EndpointConfigurationError(97-102)ToolNotInstalledError(14-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Redirect rules - jumpstarter-docs
- GitHub Check: Header rules - jumpstarter-docs
- GitHub Check: Pages changed - jumpstarter-docs
- GitHub Check: e2e
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (2)
140-156: Let--kind/--minikubebehave as flags again.
These options still require an argument because ofis_flag=False; folks runningjmp admin install --kindhit Click’s “option requires an argument” error. Removing the explicitis_flag=Falserestores the flag semantics without breaking existing usage.@click.option( - "--kind", is_flag=False, flag_value="kind", default=None, help="Use default settings for a local Kind cluster" + "--kind", flag_value="kind", default=None, help="Use default settings for a local Kind cluster" ) @click.option( "--minikube", - is_flag=False, flag_value="minikube", default=None, help="Use default settings for a local Minikube cluster", )
6-204: ConvertJumpstarterKubernetesErrorinto ClickExceptions ininstall.
get_latest_compatible_controller_version()now sits on the hot path and raisesJumpstarterKubernetesError(e.g., network hiccups). At the moment the exception bubbles straight to the user with a stack trace. Import the exception type and wrap the version lookup + helm install so the CLI reports a clean Click error.-from jumpstarter_kubernetes import ( - get_latest_compatible_controller_version, - helm_installed, - install_helm_chart, - minikube_installed, - uninstall_helm_chart, -) +from jumpstarter_kubernetes import ( + get_latest_compatible_controller_version, + helm_installed, + install_helm_chart, + minikube_installed, + uninstall_helm_chart, +) +from jumpstarter_kubernetes.exceptions import JumpstarterKubernetesError @@ - if version is None: - version = await get_latest_compatible_controller_version(get_client_version()) - - await _install_jumpstarter_helm_chart( - chart, - name, - namespace, - basedomain, - grpc_endpoint, - router_endpoint, - mode, - version, - kubeconfig, - context, - helm, - ip, - list(values_files) if values_files else None, - ) + try: + if version is None: + version = await get_latest_compatible_controller_version(get_client_version()) + + await _install_jumpstarter_helm_chart( + chart, + name, + namespace, + basedomain, + grpc_endpoint, + router_endpoint, + mode, + version, + kubeconfig, + context, + helm, + ip, + list(values_files) if values_files else None, + ) + except JumpstarterKubernetesError as e: + raise click.ClickException(str(e)) from e
♻️ Duplicate comments (1)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py (1)
192-199: Restore flag behavior for--kind/--minikube.
Withis_flag=False,jmp admin create cluster --kind(or--minikube) now errors because Click insists on a value. Please drop the explicitis_flag=Falseso these work as boolean flags again, matching the help text and previous UX.-@click.option("--kind", is_flag=False, flag_value="kind", default=None, help="Create a local Kind cluster") +@click.option("--kind", flag_value="kind", default=None, help="Create a local Kind cluster") @click.option( "--minikube", - is_flag=False, flag_value="minikube", default=None, help="Create a local Minikube cluster", )
🧹 Nitpick comments (3)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py (1)
3-3: ImportMagicMockat module level.
MagicMockis imported inside individual test methods (lines 16, 205). Move this import to the module level alongside the existingpatchimport for consistency and to avoid redundant imports.Apply this diff:
-from unittest.mock import patch +from unittest.mock import MagicMock, patchThen remove the redundant imports from lines 16 and 205.
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (2)
83-84: Remove redundant validation check.Lines 83-84 validate that
cluster_typeis in["kind", "minikube"], but this check is unreachable with a bad value:
- If
cluster_typewas explicitly provided (line 60), it was already validated on lines 71-73.- If
cluster_typewas auto-detected (lines 76-79),detect_existing_cluster_typeonly returns"kind","minikube", orNone.At this point in the flow,
cluster_typeis guaranteed to be valid.Apply this diff to remove the dead code:
- # Validate cluster type is supported for deletion - if cluster_type not in ["kind", "minikube"]: - raise ClusterTypeValidationError(cluster_type, ["kind", "minikube"]) - # Confirm deletion unless force is specified
198-221: Consider renamingcustom_certstoextra_certsfor consistency.Line 206 uses the parameter name
custom_certs, but it corresponds to theextra_certsparameter increate_cluster_and_install(line 111). While this works correctly due to positional argument passing, using consistent naming would improve readability.Apply this diff:
async def create_cluster_only( cluster_type: ClusterType, force_recreate_cluster: bool, cluster_name: str, kind_extra_args: str, minikube_extra_args: str, kind: str, minikube: str, - custom_certs: Optional[str] = None, + extra_certs: Optional[str] = None, callback: OutputCallback = None, ) -> None: """Create a cluster without installing Jumpstarter.""" await create_cluster_and_install( cluster_type, force_recreate_cluster, cluster_name, kind_extra_args, minikube_extra_args, kind, minikube, - custom_certs, + extra_certs, install_jumpstarter=False, callback=callback, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py(3 hunks)packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py(6 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py(1 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py(2 hunks)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install_test.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations_test.py
🧰 Additional context used
🧬 Code graph analysis (8)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install_test.py (1)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (2)
helm_installed(6-8)install_helm_chart(11-66)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (9)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (18)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)confirm(31-33)confirm(59-61)confirm(90-93)confirm(122-124)success(19-21)success(47-49)success(78-80)success(110-112)warning(23-25)warning(51-53)warning(82-84)warning(114-116)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (5)
ClusterNameValidationError(89-94)ClusterNotFoundError(25-35)ClusterOperationError(48-60)ClusterTypeValidationError(79-86)ToolNotInstalledError(14-22)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (1)
helm_installed(6-8)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (1)
validate_cluster_name(44-49)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (2)
auto_detect_cluster_type(97-109)detect_existing_cluster_type(59-94)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/endpoints.py (1)
configure_endpoints(27-46)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
install_jumpstarter_helm_chart(9-53)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
create_kind_cluster_with_options(181-217)delete_kind_cluster_with_feedback(220-231)kind_cluster_exists(24-33)kind_installed(19-21)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (4)
create_minikube_cluster_with_options(211-247)delete_minikube_cluster_with_feedback(250-263)minikube_cluster_exists(26-74)minikube_installed(21-23)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (10)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (4)
CertificateError(63-68)ClusterAlreadyExistsError(38-45)ClusterOperationError(48-60)ToolNotInstalledError(14-22)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/common.py (2)
run_command(52-76)run_command_with_output(79-95)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/detection.py (1)
detect_kind_provider(29-56)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
install_jumpstarter_helm_chart(9-53)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (8)
progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/install.py (5)
packages/jumpstarter-cli-common/jumpstarter_cli_common/version.py (2)
version(49-56)get_client_version(12-14)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
get_latest_compatible_controller_version(10-75)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (3)
helm_installed(6-8)install_helm_chart(11-66)uninstall_helm_chart(69-91)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/minikube.py (1)
minikube_installed(21-23)packages/jumpstarter/jumpstarter/common/ipaddr.py (2)
get_ip_address(7-26)get_minikube_ip(29-47)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/callbacks.py (10)
OutputCallback(12-33)SilentCallback(36-61)progress(15-17)progress(43-45)progress(74-76)progress(106-108)success(19-21)success(47-49)success(78-80)success(110-112)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/install.py (1)
install_helm_chart(11-66)
packages/jumpstarter-cli-admin/jumpstarter_cli_admin/create.py (5)
packages/jumpstarter-cli-common/jumpstarter_cli_common/callbacks.py (1)
ClickCallback(10-47)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (2)
create_cluster_and_install(103-195)validate_cluster_type_selection(32-43)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (1)
JumpstarterKubernetesError(8-11)packages/jumpstarter-cli-common/jumpstarter_cli_common/version.py (2)
version(49-56)get_client_version(12-14)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/controller.py (1)
get_latest_compatible_controller_version(10-75)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind_test.py (2)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (4)
create_kind_cluster(52-125)delete_kind_cluster(36-49)kind_cluster_exists(24-33)kind_installed(19-21)packages/jumpstarter-kubernetes/jumpstarter_kubernetes/exceptions.py (1)
ClusterAlreadyExistsError(38-45)
🔇 Additional comments (8)
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py (3)
13-56: LGTM!This test properly verifies that all parameters are correctly forwarded to
install_helm_chartand that the callback is invoked appropriately.
139-162: LGTM!This test correctly verifies that exceptions from
install_helm_chartpropagate to the caller.
201-243: LGTM!This test properly verifies that the
values_filesparameter is correctly passed through toinstall_helm_chart.packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm.py (1)
9-53: LGTM!The function correctly wraps the Helm installation workflow with progress logging and proper callback handling. The parameter passing to
install_helm_chartmatches its signature, and the callback defaulting pattern is appropriate.packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py (1)
103-196: LGTM!The orchestration logic correctly handles cluster creation, validation, endpoint configuration, and optional Jumpstarter installation. Past review comments about unsupported cluster types have been addressed with the explicit validation on lines 157-158.
packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/kind.py (3)
143-178: LGTM! Path expansion correctly handles~and environment variables.Lines 149-150 properly expand user home directories and environment variables before resolving the absolute path, which addresses the past review comment about
--custom-certspath handling. The certificate injection flow is robust with proper error handling.
181-217: LGTM! Past callback parameter issue has been addressed.The function correctly calls
create_kind_clusterwithout passing thecallbackparameter (line 201), and the exception handling properly distinguishes between creation and recreation failures. The past review comment about the stray callback argument has been resolved.
220-231: LGTM! Past callback parameter issue has been addressed.The function correctly calls
delete_kind_clusterwithout passing thecallbackparameter (line 229), which resolves the past review comment about the incorrect parameter passing.
Comment on lines
+271
to
+319
| cluster_type = validate_cluster_type_selection(kind, minikube) | ||
|
|
||
| if output is None: | ||
| if kind is None and minikube is None: | ||
| click.echo(f"Auto-detected {cluster_type} as the cluster type") | ||
| if skip_install: | ||
| click.echo(f'Creating {cluster_type} cluster "{name}"...') | ||
| else: | ||
| click.echo(f'Creating {cluster_type} cluster "{name}" and installing Jumpstarter...') | ||
|
|
||
| # Auto-detect version if not specified and installing Jumpstarter | ||
| if not skip_install and version is None: | ||
| from jumpstarter_cli_common.version import get_client_version | ||
| from jumpstarter_kubernetes import get_latest_compatible_controller_version | ||
|
|
||
| version = await get_latest_compatible_controller_version(get_client_version()) | ||
|
|
||
| # Create callback for library functions | ||
| # Use silent mode when JSON/YAML output is requested | ||
| callback = ClickCallback(silent=(output is not None)) | ||
|
|
||
| try: | ||
| await create_cluster_and_install( | ||
| cluster_type, | ||
| force_recreate, | ||
| name, | ||
| kind_extra_args, | ||
| minikube_extra_args, | ||
| kind or "kind", | ||
| minikube or "minikube", | ||
| extra_certs, | ||
| install_jumpstarter=not skip_install, | ||
| helm=helm, | ||
| chart=chart, | ||
| chart_name=chart_name, | ||
| namespace=namespace, | ||
| version=version, | ||
| kubeconfig=kubeconfig, | ||
| context=context, | ||
| ip=ip, | ||
| basedomain=basedomain, | ||
| grpc_endpoint=grpc_endpoint, | ||
| router_endpoint=router_endpoint, | ||
| callback=callback, | ||
| values_files=list(values_files) if values_files else None, | ||
| ) | ||
| except JumpstarterKubernetesError as e: | ||
| # Convert library exceptions to CLI exceptions | ||
| raise click.ClickException(str(e)) from e |
Contributor
There was a problem hiding this comment.
Catch K8s errors early and honor --nointeractive.
validate_cluster_type_selection and get_latest_compatible_controller_version can raise JumpstarterKubernetesError, but they currently sit outside the try, so the CLI surfaces a raw stack trace. In addition, --nointeractive keeps prompting because the callback always calls click.confirm. Move the pre-flight logic inside the try and bypass confirmation when non-interactive.
- cluster_type = validate_cluster_type_selection(kind, minikube)
-
- if output is None:
- if kind is None and minikube is None:
- click.echo(f"Auto-detected {cluster_type} as the cluster type")
- if skip_install:
- click.echo(f'Creating {cluster_type} cluster "{name}"...')
- else:
- click.echo(f'Creating {cluster_type} cluster "{name}" and installing Jumpstarter...')
-
- # Auto-detect version if not specified and installing Jumpstarter
- if not skip_install and version is None:
- from jumpstarter_cli_common.version import get_client_version
- from jumpstarter_kubernetes import get_latest_compatible_controller_version
-
- version = await get_latest_compatible_controller_version(get_client_version())
-
- # Create callback for library functions
- # Use silent mode when JSON/YAML output is requested
- callback = ClickCallback(silent=(output is not None))
-
- try:
- await create_cluster_and_install(
+ try:
+ cluster_type = validate_cluster_type_selection(kind, minikube)
+
+ if output is None:
+ if kind is None and minikube is None:
+ click.echo(f"Auto-detected {cluster_type} as the cluster type")
+ if skip_install:
+ click.echo(f'Creating {cluster_type} cluster "{name}"...')
+ else:
+ click.echo(f'Creating {cluster_type} cluster "{name}" and installing Jumpstarter...')
+
+ # Auto-detect version if not specified and installing Jumpstarter
+ if not skip_install and version is None:
+ from jumpstarter_cli_common.version import get_client_version
+ from jumpstarter_kubernetes import get_latest_compatible_controller_version
+
+ version = await get_latest_compatible_controller_version(get_client_version())
+
+ # Create callback for library functions
+ # Use silent mode when JSON/YAML output is requested
+ callback = ClickCallback(silent=(output is not None))
+ if nointeractive:
+ callback.confirm = lambda prompt: True
+
+ await create_cluster_and_install(
cluster_type,
force_recreate,
name,
@@
router_endpoint=router_endpoint,
callback=callback,
values_files=list(values_files) if values_files else None,
)
- except JumpstarterKubernetesError as e:
- # Convert library exceptions to CLI exceptions
- raise click.ClickException(str(e)) from e
+ except JumpstarterKubernetesError as e:
+ # Convert library exceptions to CLI exceptions
+ raise click.ClickException(str(e)) from e📝 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
| cluster_type = validate_cluster_type_selection(kind, minikube) | |
| if output is None: | |
| if kind is None and minikube is None: | |
| click.echo(f"Auto-detected {cluster_type} as the cluster type") | |
| if skip_install: | |
| click.echo(f'Creating {cluster_type} cluster "{name}"...') | |
| else: | |
| click.echo(f'Creating {cluster_type} cluster "{name}" and installing Jumpstarter...') | |
| # Auto-detect version if not specified and installing Jumpstarter | |
| if not skip_install and version is None: | |
| from jumpstarter_cli_common.version import get_client_version | |
| from jumpstarter_kubernetes import get_latest_compatible_controller_version | |
| version = await get_latest_compatible_controller_version(get_client_version()) | |
| # Create callback for library functions | |
| # Use silent mode when JSON/YAML output is requested | |
| callback = ClickCallback(silent=(output is not None)) | |
| try: | |
| await create_cluster_and_install( | |
| cluster_type, | |
| force_recreate, | |
| name, | |
| kind_extra_args, | |
| minikube_extra_args, | |
| kind or "kind", | |
| minikube or "minikube", | |
| extra_certs, | |
| install_jumpstarter=not skip_install, | |
| helm=helm, | |
| chart=chart, | |
| chart_name=chart_name, | |
| namespace=namespace, | |
| version=version, | |
| kubeconfig=kubeconfig, | |
| context=context, | |
| ip=ip, | |
| basedomain=basedomain, | |
| grpc_endpoint=grpc_endpoint, | |
| router_endpoint=router_endpoint, | |
| callback=callback, | |
| values_files=list(values_files) if values_files else None, | |
| ) | |
| except JumpstarterKubernetesError as e: | |
| # Convert library exceptions to CLI exceptions | |
| raise click.ClickException(str(e)) from e | |
| try: | |
| cluster_type = validate_cluster_type_selection(kind, minikube) | |
| if output is None: | |
| if kind is None and minikube is None: | |
| click.echo(f"Auto-detected {cluster_type} as the cluster type") | |
| if skip_install: | |
| click.echo(f'Creating {cluster_type} cluster "{name}"...') | |
| else: | |
| click.echo(f'Creating {cluster_type} cluster "{name}" and installing Jumpstarter...') | |
| # Auto-detect version if not specified and installing Jumpstarter | |
| if not skip_install and version is None: | |
| from jumpstarter_cli_common.version import get_client_version | |
| from jumpstarter_kubernetes import get_latest_compatible_controller_version | |
| version = await get_latest_compatible_controller_version(get_client_version()) | |
| # Create callback for library functions | |
| # Use silent mode when JSON/YAML output is requested | |
| callback = ClickCallback(silent=(output is not None)) | |
| if nointeractive: | |
| callback.confirm = lambda prompt: True | |
| await create_cluster_and_install( | |
| cluster_type, | |
| force_recreate, | |
| name, | |
| kind_extra_args, | |
| minikube_extra_args, | |
| kind or "kind", | |
| minikube or "minikube", | |
| extra_certs, | |
| install_jumpstarter=not skip_install, | |
| helm=helm, | |
| chart=chart, | |
| chart_name=chart_name, | |
| namespace=namespace, | |
| version=version, | |
| kubeconfig=kubeconfig, | |
| context=context, | |
| ip=ip, | |
| basedomain=basedomain, | |
| grpc_endpoint=grpc_endpoint, | |
| router_endpoint=router_endpoint, | |
| callback=callback, | |
| values_files=list(values_files) if values_files else None, | |
| ) | |
| except JumpstarterKubernetesError as e: | |
| # Convert library exceptions to CLI exceptions | |
| raise click.ClickException(str(e)) from e |
Comment on lines
+93
to
+94
| # Verify success message with correct values | ||
|
|
Contributor
There was a problem hiding this comment.
Remove or complete the incomplete assertion.
The comment suggests verification of success messages, but no assertion follows. Either remove this comment or add the corresponding callback verification.
If callback verification is intended, add assertions similar to the first test:
# Verify success message with correct values
assert mock_callback.success.call_count == 1Or remove the comment if no additional verification is needed.
🤖 Prompt for AI Agents
In packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py
around lines 93 to 94, there is an incomplete comment "Verify success message
with correct values" with no assertion; either remove the comment or add the
missing callback assertion(s). If callback verification is intended, add an
assertion like asserting mock_callback.success.call_count == 1 (and any other
expected calls/args) to mirror the first test; otherwise delete the comment to
avoid dead code.
Comment on lines
+115
to
+116
| # Verify gRPC mode is correctly displayed | ||
|
|
Contributor
There was a problem hiding this comment.
Remove or complete the incomplete assertion.
The comment suggests verification of gRPC mode display, but no assertion follows. Either remove this comment or add callback verification to ensure the mode is correctly logged.
🤖 Prompt for AI Agents
In packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py
around lines 115-116, there is an incomplete comment "Verify gRPC mode is
correctly displayed" with no assertion; either delete the stray comment or
replace it with an assertion that verifies the logging/callback was invoked with
the expected gRPC-mode message (e.g., assert the mocked callback/logger was
called_once_with or contains the expected substring), ensuring the test sets up
and inspects the mock correctly and fails if the mode is not logged.
Comment on lines
+137
to
+138
| # Verify custom endpoints are displayed correctly | ||
|
|
Contributor
There was a problem hiding this comment.
Remove or complete the incomplete assertion.
The comment suggests verification of custom endpoint display, but no assertion follows. Either remove this comment or add callback verification.
🤖 Prompt for AI Agents
In packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py
around lines 137 to 138, there's a trailing comment "Verify custom endpoints are
displayed correctly" with no assertion; remove this dangling comment or replace
it with an actual assertion/callback verification that checks the function
output or mock was called with the expected custom endpoints (e.g., assert
output contains the endpoint strings or assert
mock_display.called_with(expected_endpoints)). Ensure the test exercises the
code path that renders endpoints and validates the expected display values, or
delete the comment if no check is required.
Comment on lines
+199
to
+200
| # Verify appropriate echo calls were made | ||
|
|
Contributor
There was a problem hiding this comment.
Remove or complete the incomplete assertion.
The comment suggests verification of callback calls, but no assertion follows. Either remove this comment or add the corresponding callback verification.
🤖 Prompt for AI Agents
In packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/helm_test.py
around lines 199-200 there is a stray comment "Verify appropriate echo calls
were made" with no assertion; either remove that comment or add the missing
assertion(s) that verify the mocked echo/callbacks were invoked with the
expected arguments (e.g., use the existing mock variables in the test and call
assert_called_once_with / assert_has_calls or check call_count/args accordingly)
so the test actually verifies the expected echo behavior.
Comment on lines
+32
to
+44
| def validate_cluster_type_selection(kind: Optional[str], minikube: Optional[str]) -> ClusterType: | ||
| """Validate cluster type selection and return the cluster type.""" | ||
| if kind and minikube: | ||
| raise ClusterTypeValidationError('You can only select one local cluster type "kind" or "minikube"') | ||
|
|
||
| if kind is not None: | ||
| return "kind" | ||
| elif minikube is not None: | ||
| return "minikube" | ||
| else: | ||
| # Auto-detect cluster type when neither is specified | ||
| return auto_detect_cluster_type() | ||
|
|
Contributor
There was a problem hiding this comment.
Fix exception constructor call.
Line 35 passes a message string as the first argument to ClusterTypeValidationError, but the exception's __init__ expects cluster_type: str as the first parameter (see exceptions.py lines 78-85). This causes a confusing error message.
Apply this diff:
- if kind and minikube:
- raise ClusterTypeValidationError('You can only select one local cluster type "kind" or "minikube"')
+ if kind and minikube:
+ raise ClusterTypeValidationError("both", ["kind", "minikube"])📝 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
| def validate_cluster_type_selection(kind: Optional[str], minikube: Optional[str]) -> ClusterType: | |
| """Validate cluster type selection and return the cluster type.""" | |
| if kind and minikube: | |
| raise ClusterTypeValidationError('You can only select one local cluster type "kind" or "minikube"') | |
| if kind is not None: | |
| return "kind" | |
| elif minikube is not None: | |
| return "minikube" | |
| else: | |
| # Auto-detect cluster type when neither is specified | |
| return auto_detect_cluster_type() | |
| def validate_cluster_type_selection(kind: Optional[str], minikube: Optional[str]) -> ClusterType: | |
| """Validate cluster type selection and return the cluster type.""" | |
| if kind and minikube: | |
| raise ClusterTypeValidationError("both", ["kind", "minikube"]) | |
| if kind is not None: | |
| return "kind" | |
| elif minikube is not None: | |
| return "minikube" | |
| else: | |
| # Auto-detect cluster type when neither is specified | |
| return auto_detect_cluster_type() |
🤖 Prompt for AI Agents
In packages/jumpstarter-kubernetes/jumpstarter_kubernetes/cluster/operations.py
around lines 32 to 44, the code raises ClusterTypeValidationError with the
message string as the first argument but the exception expects cluster_type: str
as its first parameter; change the raise to pass a valid cluster_type value (for
the conflicting case use something like "both") as the first argument and move
the human-readable message to the second argument (e.g.
ClusterTypeValidationError("both", 'You can only select one local cluster type
"kind" or "minikube"')) so the exception constructor receives the expected
parameters.
mangelajo
reviewed
Oct 9, 2025
| import click | ||
|
|
||
|
|
||
| class ClickCallback: |
mangelajo
approved these changes
Oct 9, 2025
auto-merge was automatically disabled
October 9, 2025 15:32
Pull Request is not mergeable
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
jmp admin create clustersub-command to automatically create a local minikube or kind cluster.jmp admin get clusterssub-command to list available clusters based on user's local kubecontexts.--custom-certsparameter to pass a custom certificate bundle (injected into Kind/Minikube clusters).Summary by CodeRabbit
New Features
Refactor
Documentation
Tests
Chores