feat: use generateName for leases to allow alias reuse after deletion#727
Open
bennyz wants to merge 2 commits into
Open
feat: use generateName for leases to allow alias reuse after deletion#727bennyz wants to merge 2 commits into
bennyz wants to merge 2 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds lease alias support across API contract (label key + proto field), controller ToProtobuf propagation and tests, alias-aware ClientService create/get/update/delete with per-alias creation locks, regenerated protobuf/Python stubs, Python client/CLI/MCP surfaces, and related tests and docstring updates. ChangesLease alias support
Sequence DiagramsequenceDiagram
participant Client
participant ClientService
participant Resolver as resolveLeaseByNameOrAlias
participant KubeAPI as Kubernetes API
Client->>ClientService: Create/Get/Update/Delete (name or lease_id)
ClientService->>Resolver: resolve name or alias
Resolver->>KubeAPI: Get(name)
alt not found
Resolver->>KubeAPI: List(labelSelector=jumpstarter.dev/lease-name, limit=2)
end
KubeAPI-->>Resolver: lease(s) or none
Resolver-->>ClientService: resolved lease or error
ClientService-->>Client: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py`:
- Around line 33-34: The generated protobufs in client_pb2.py import nonexistent
top-level package 'jumpstarter.v1' (see the import lines that map to
jumpstarter_dot_v1_dot_kubernetes__pb2 and jumpstarter_dot_v1_dot_common__pb2);
fix by regenerating the jumpstarter-protocol protos with the repo protobuf-gen
target so the Makefile's relative-import rewrite is applied, or manually replace
the absolute imports with the correct relative imports used elsewhere (e.g.,
change the import that produces jumpstarter_dot_v1_dot_kubernetes__pb2 to use
the relative package path like ...v1.kubernetes_pb2 and adjust the common_pb2
import to a relative import) so client_pb2.py no longer tries to import
jumpstarter.v1.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 455a9c3c-dabb-4175-81e0-a064f673c020
⛔ Files ignored due to path filters (9)
controller/internal/protocol/jumpstarter/client/v1/client.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/client/v1/client_grpc.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/common.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/jumpstarter.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/jumpstarter_grpc.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/kubernetes.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/router.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/router_grpc.pb.gois excluded by!**/*.pb.gopython/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
controller/api/v1alpha1/lease_helpers.gocontroller/api/v1alpha1/lease_helpers_test.gocontroller/api/v1alpha1/lease_types.gocontroller/internal/service/client/v1/client_service.goprotocol/proto/jumpstarter/client/v1/client.protopython/packages/jumpstarter-cli/jumpstarter_cli/shell.pypython/packages/jumpstarter-mcp/jumpstarter_mcp/tools/leases.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/common_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/kubernetes_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pyipython/packages/jumpstarter/jumpstarter/client/grpc.py
bennyz
force-pushed
the
worktree-lease-generate-name
branch
from
f16cd14 to
4088449
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.py`:
- Around line 79-81: Update the RPC doc comments in the source .proto for the
lease service to state that GetLease and DeleteLease resolve leases by name or
alias (not name-only), then re-run the protobuf/grpc codegen to regenerate the
Python stubs so client_pb2_grpc.py reflects the new docs; specifically edit the
proto comments for the RPCs corresponding to the GetLease and DeleteLease
methods and regenerate to update the docstrings in the GetLease and DeleteLease
functions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 88a8252c-f456-4f34-98c1-189bc967cd18
⛔ Files ignored due to path filters (9)
controller/internal/protocol/jumpstarter/client/v1/client.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/client/v1/client_grpc.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/common.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/jumpstarter.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/jumpstarter_grpc.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/kubernetes.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/router.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/router_grpc.pb.gois excluded by!**/*.pb.gopython/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
controller/api/v1alpha1/lease_helpers.gocontroller/api/v1alpha1/lease_helpers_test.gocontroller/api/v1alpha1/lease_types.gocontroller/internal/service/client/v1/client_service.goprotocol/proto/jumpstarter/client/v1/client.protopython/packages/jumpstarter-cli/jumpstarter_cli/shell.pypython/packages/jumpstarter-mcp/jumpstarter_mcp/tools/leases.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/common_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/kubernetes_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pyipython/packages/jumpstarter/jumpstarter/client/grpc.py
✅ Files skipped from review due to trivial changes (7)
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/kubernetes_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/common_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2.pyi
🚧 Files skipped from review as they are similar to previous changes (9)
- controller/api/v1alpha1/lease_helpers_test.go
- protocol/proto/jumpstarter/client/v1/client.proto
- controller/api/v1alpha1/lease_helpers.go
- python/packages/jumpstarter/jumpstarter/client/grpc.py
- python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/leases.py
- python/packages/jumpstarter-cli/jumpstarter_cli/shell.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.py
- controller/internal/service/client/v1/client_service.go
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pyi
Comment on lines
79
to
+81
| def GetLease(self, request, context): | ||
| """Missing associated documentation comment in .proto file.""" | ||
| """Retrieve a single lease by resource name. | ||
| """ |
Contributor
There was a problem hiding this comment.
Update the generated RPC docs to mention alias lookup.
These docstrings still describe name-only lookup, but this PR’s contract says GetLease and DeleteLease resolve a lease by name or alias. Please fix the source .proto comments and regenerate so the Python stubs do not advertise narrower behavior than the service actually supports.
Also applies to: 107-109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.py`
around lines 79 - 81, Update the RPC doc comments in the source .proto for the
lease service to state that GetLease and DeleteLease resolve leases by name or
alias (not name-only), then re-run the protobuf/grpc codegen to regenerate the
Python stubs so client_pb2_grpc.py reflects the new docs; specifically edit the
proto comments for the RPCs corresponding to the GetLease and DeleteLease
methods and regenerate to update the docstrings in the GetLease and DeleteLease
functions.
d33a6bd to
cddd7dd
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/packages/jumpstarter-mcp/jumpstarter_mcp/server_test.py`:
- Line 287: The test defines duration with the wrong type annotation—change the
field declaration in server_test.py from `duration: None = None` to an optional
timedelta type to match the Lease model (e.g., `duration: datetime.timedelta |
None = None`) and ensure `datetime.timedelta` (or `timedelta`) is imported;
alternatively remove the unused `duration` field if tests don't need it. Refer
to the Lease-related test variables where `duration` is used to update the
annotation or remove the field accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a59445c5-0568-46e4-b8d6-3ba6db0d5a95
⛔ Files ignored due to path filters (9)
controller/internal/protocol/jumpstarter/client/v1/client.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/client/v1/client_grpc.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/common.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/jumpstarter.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/jumpstarter_grpc.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/kubernetes.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/router.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/router_grpc.pb.gois excluded by!**/*.pb.gopython/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
controller/api/v1alpha1/lease_helpers.gocontroller/api/v1alpha1/lease_helpers_test.gocontroller/api/v1alpha1/lease_types.gocontroller/internal/service/client/v1/client_service.goprotocol/proto/jumpstarter/client/v1/client.protopython/packages/jumpstarter-cli/jumpstarter_cli/shell.pypython/packages/jumpstarter-cli/jumpstarter_cli/shell_test.pypython/packages/jumpstarter-mcp/jumpstarter_mcp/server_test.pypython/packages/jumpstarter-mcp/jumpstarter_mcp/tools/leases.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/common_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/kubernetes_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pyipython/packages/jumpstarter/jumpstarter/client/grpc.py
✅ Files skipped from review due to trivial changes (10)
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/kubernetes_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/common_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.py
🚧 Files skipped from review as they are similar to previous changes (7)
- controller/api/v1alpha1/lease_types.go
- protocol/proto/jumpstarter/client/v1/client.proto
- controller/api/v1alpha1/lease_helpers.go
- python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/leases.py
- controller/api/v1alpha1/lease_helpers_test.go
- controller/internal/service/client/v1/client_service.go
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pyi
| conditions: list | ||
| effective_begin_time: datetime | None = None | ||
| effective_end_time: datetime | None = None | ||
| duration: None = None |
Contributor
There was a problem hiding this comment.
Incorrect type annotation for duration field.
The type annotation duration: None = None declares the field type as None itself, not as an optional value. This should likely be duration: timedelta | None = None to match the actual Lease model, or the field could be omitted if it's unused in these tests.
📝 Proposed fix
- duration: None = None
+ duration: timedelta | None = None📝 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
| duration: None = None | |
| duration: timedelta | None = None |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/packages/jumpstarter-mcp/jumpstarter_mcp/server_test.py` at line 287,
The test defines duration with the wrong type annotation—change the field
declaration in server_test.py from `duration: None = None` to an optional
timedelta type to match the Lease model (e.g., `duration: datetime.timedelta |
None = None`) and ensure `datetime.timedelta` (or `timedelta`) is imported;
alternatively remove the unused `duration` field if tests don't need it. Refer
to the Lease-related test variables where `duration` is used to update the
annotation or remove the field accordingly.
cddd7dd to
876af2e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/internal/service/client/v1/client_service.go`:
- Around line 291-303: The duplicate-alias check calls
s.resolveLeaseByAlias(ctx, namespace, req.LeaseId) but only handles err == nil
and ignores other errors, allowing duplicate aliases on transient failures;
update the block in CreateLease (or wherever jlease is prepared) so that after
calling resolveLeaseByAlias you treat a NotFound error as the only acceptable
error and for any other non-nil error return that error (wrap it with an
appropriate gRPC code, e.g., status.Errorf(codes.Internal, "failed to resolve
lease by alias %q: %v", req.LeaseId, err)); keep the existing AlreadyExists
return when err == nil and still set jlease.GenerateName and jlease.Labels when
the alias is acceptable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfdae748-8c96-4b32-ab14-5d5a5f5faf87
⛔ Files ignored due to path filters (9)
controller/internal/protocol/jumpstarter/client/v1/client.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/client/v1/client_grpc.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/common.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/jumpstarter.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/jumpstarter_grpc.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/kubernetes.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/router.pb.gois excluded by!**/*.pb.gocontroller/internal/protocol/jumpstarter/v1/router_grpc.pb.gois excluded by!**/*.pb.gopython/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
controller/api/v1alpha1/lease_helpers.gocontroller/api/v1alpha1/lease_helpers_test.gocontroller/api/v1alpha1/lease_types.gocontroller/internal/service/client/v1/client_service.goe2e/test/hooks_test.goprotocol/proto/jumpstarter/client/v1/client.protopython/packages/jumpstarter-cli/jumpstarter_cli/shell.pypython/packages/jumpstarter-cli/jumpstarter_cli/shell_test.pypython/packages/jumpstarter-mcp/jumpstarter_mcp/server_test.pypython/packages/jumpstarter-mcp/jumpstarter_mcp/tools/leases.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/common_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/kubernetes_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2.pyipython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pypython/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pyipython/packages/jumpstarter/jumpstarter/client/grpc.py
✅ Files skipped from review due to trivial changes (10)
- e2e/test/hooks_test.go
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2_grpc.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/common_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2_grpc.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/jumpstarter_pb2.pyi
🚧 Files skipped from review as they are similar to previous changes (11)
- controller/api/v1alpha1/lease_types.go
- controller/api/v1alpha1/lease_helpers.go
- controller/api/v1alpha1/lease_helpers_test.go
- python/packages/jumpstarter-cli/jumpstarter_cli/shell_test.py
- python/packages/jumpstarter/jumpstarter/client/grpc.py
- python/packages/jumpstarter-cli/jumpstarter_cli/shell.py
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/router_pb2_grpc.py
- python/packages/jumpstarter-mcp/jumpstarter_mcp/tools/leases.py
- protocol/proto/jumpstarter/client/v1/client.proto
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/kubernetes_pb2.pyi
- python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1/client_pb2.pyi
Member
|
I am not sure about the extra layer of complexity, what if we just trigger a generate name on the lease name if there is a conflict? The current implementation has some level of race condition (2 leases could end up with the same label) ... |
Member
I was about to comment the same thing. I think this just moves the problem one layer of complexity up. maybe we run generated name with the prefix of the user provided input if it conflicts, or we could also simply raise an error and clean up leases properly - the latter is what i prefer @mangelajo |
Regenerated Go and Python protobuf stubs pick up doc comments from proto definitions that the previous generator version omitted. No functional changes. Assited-by: claude-opus-4.6 Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
bennyz
force-pushed
the
worktree-lease-generate-name
branch
from
876af2e to
aac6e20
Compare
Member
Author
the problem with this is that it's essentially the same behavior we already have and the user won't get the name they request |
Lease names now use K8s generateName instead of fixed names, allowing custom lease IDs (aliases) to be reused after deletion. The user-provided alias is stored as a label and resolved transparently in Get/Delete/Update. Assisted-by: claude-opus-4.6 Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
bennyz
force-pushed
the
worktree-lease-generate-name
branch
from
aac6e20 to
d93af9d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Leases created with --lease-id now use K8s generateName instead of a fixed Name. The user-provided alias is stored as a label and resolved transparently in Get/Delete/Update via a name-or-alias lookup. This prevents the "already exists" error when re-creating a lease with the same --lease-id after deletion (since deletion is a soft-delete that leaves the K8s object in etcd).