Merge pull request #720 from jumpstarter-dev/backport-704-to-release-0.7

[Backport release-0.7] client lease improvements

Author: mangelajo

Makefile

@@ -8,7 +8,7 @@ help:
 	@echo ""
 	@echo "Build targets:"
 	@echo "  build              - Build all packages"
-	@echo "  generate           - Generate code from protobuf definitions"
+	@echo "  protobuf-gen       - Generate code from protobuf definitions"
 	@echo "  sync               - Sync all packages and extras"
 	@echo ""
 	@echo "Documentation targets:"
@@ -75,8 +75,18 @@ pkg-ty-all: $(addprefix pkg-ty-,$(PKG_TARGETS))
 build:
 	uv build --all --out-dir dist
 
-generate:
-	buf generate
+protobuf-gen:
+	podman run --volume "$(shell pwd):/workspace" --workdir /workspace docker.io/bufbuild/buf:latest generate
+	# Fix Python imports: convert absolute imports to relative imports
+	# In jumpstarter/client/v1: from jumpstarter.v1 import -> from ...v1 import
+	find packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/client/v1 -name "*_pb2*.py" -type f -exec sed -i.bak \
+		-e 's|^from jumpstarter\.v1 import|from ...v1 import|g' \
+		-e 's|^from jumpstarter\.client\.v1 import|from . import|g' \
+		{} \; -exec rm {}.bak \;
+	# In jumpstarter/v1: from jumpstarter.v1 import -> from . import
+	find packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1 -name "*_pb2*.py" -type f -exec sed -i.bak \
+		-e 's|^from jumpstarter\.v1 import|from . import|g' \
+		{} \; -exec rm {}.bak \;
 
 sync:
 	uv sync --all-packages --all-extras

buf.gen.yaml

@@ -8,4 +8,5 @@ plugins:
     out: ./packages/jumpstarter-protocol/jumpstarter_protocol
 inputs:
   - git_repo: https://github.com/jumpstarter-dev/jumpstarter-protocol.git
+    branch: main
     subdir: proto

packages/jumpstarter-cli/jumpstarter_cli/common.py

@@ -1,8 +1,8 @@
-from datetime import timedelta
+from datetime import datetime, timedelta
 from functools import partial
 
 import click
-from pydantic import TypeAdapter
+from pydantic import TypeAdapter, ValidationError
 
 opt_selector = click.option(
     "-l",
@@ -21,7 +21,7 @@ def convert(self, value, param, ctx):
 
         try:
             return TypeAdapter(timedelta).validate_python(value)
-        except ValueError:
+        except (ValueError, ValidationError):
             self.fail(f"{value!r} is not a valid duration", param, ctx)
 
 
@@ -44,3 +44,37 @@ def convert(self, value, param, ctx):
 See https://docs.rs/speedate/latest/speedate/ for details
 """,
 )
+
+
+class DateTimeParamType(click.ParamType):
+    name = "datetime"
+
+    def convert(self, value, param, ctx):
+        if isinstance(value, datetime):
+            dt = value
+        else:
+            try:
+                dt = TypeAdapter(datetime).validate_python(value)
+            except (ValueError, ValidationError):
+                self.fail(f"{value!r} is not a valid datetime", param, ctx)
+
+        # Normalize naive datetimes to local timezone
+        if dt.tzinfo is None:
+            dt = dt.astimezone()
+
+        return dt
+
+
+DATETIME = DateTimeParamType()
+
+opt_begin_time = click.option(
+    "--begin-time",
+    "begin_time",
+    type=DATETIME,
+    default=None,
+    help="""
+Begin time for the lease in ISO 8601 format (e.g., 2024-01-01T12:00:00 or 2024-01-01T12:00:00Z).
+If not specified, the lease tries to be acquired immediately. The lease duration always starts
+at the actual time of acquisition.
+""",
+)

packages/jumpstarter-cli/jumpstarter_cli/common_test.py

@@ -0,0 +1,90 @@
+from datetime import datetime, timedelta, timezone
+
+import click
+import pytest
+
+from jumpstarter_cli.common import DATETIME, DURATION, DateTimeParamType, DurationParamType
+
+
+class TestDateTimeParamType:
+    """Test DateTimeParamType parameter parsing and normalization."""
+
+    def test_parse_iso8601_with_timezone(self):
+        """Test parsing ISO 8601 datetime with timezone."""
+        dt = DATETIME.convert("2024-01-01T12:00:00Z", None, None)
+        assert dt.year == 2024
+        assert dt.month == 1
+        assert dt.day == 1
+        assert dt.hour == 12
+        assert dt.minute == 0
+        assert dt.second == 0
+        assert dt.tzinfo is not None
+        assert dt.tzinfo == timezone.utc
+
+    def test_parse_iso8601_naive_gets_normalized(self):
+        """Test that naive datetime gets normalized to local timezone."""
+        dt = DATETIME.convert("2024-01-01T12:00:00", None, None)
+        assert dt.year == 2024
+        assert dt.month == 1
+        assert dt.day == 1
+        assert dt.hour == 12
+        assert dt.minute == 0
+        assert dt.second == 0
+        # Should have been normalized to local timezone
+        assert dt.tzinfo is not None
+
+    def test_pass_through_datetime_object_with_timezone(self):
+        """Test that datetime object with timezone passes through."""
+        input_dt = datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc)
+        dt = DATETIME.convert(input_dt, None, None)
+        assert dt == input_dt
+        assert dt.tzinfo == timezone.utc
+
+    def test_pass_through_datetime_object_naive_gets_normalized(self):
+        """Test that naive datetime object gets normalized."""
+        input_dt = datetime(2024, 1, 1, 12, 0, 0)  # Naive
+        dt = DATETIME.convert(input_dt, None, None)
+        assert dt.year == 2024
+        assert dt.month == 1
+        assert dt.day == 1
+        assert dt.hour == 12
+        # Should have been normalized to local timezone
+        assert dt.tzinfo is not None
+
+    def test_invalid_datetime_raises_click_exception(self):
+        """Test that invalid datetime string raises click exception."""
+        param_type = DateTimeParamType()
+        with pytest.raises(click.BadParameter, match="is not a valid datetime"):
+            param_type.convert("not-a-datetime", None, None)
+
+
+class TestDurationParamType:
+    """Test DurationParamType parameter parsing."""
+
+    def test_parse_iso8601_duration(self):
+        """Test parsing ISO 8601 duration."""
+        td = DURATION.convert("PT1H30M", None, None)
+        assert td == timedelta(hours=1, minutes=30)
+
+    def test_parse_time_format(self):
+        """Test parsing HH:MM:SS format."""
+        td = DURATION.convert("01:30:00", None, None)
+        assert td == timedelta(hours=1, minutes=30)
+
+    def test_parse_days_and_time(self):
+        """Test parsing 'D days, HH:MM:SS' format."""
+        td = DURATION.convert("2 days, 01:30:00", None, None)
+        assert td == timedelta(days=2, hours=1, minutes=30)
+
+    def test_pass_through_timedelta_object(self):
+        """Test that timedelta object passes through."""
+        input_td = timedelta(hours=1, minutes=30)
+        td = DURATION.convert(input_td, None, None)
+        assert td == input_td
+
+    def test_invalid_duration_raises_click_exception(self):
+        """Test that invalid duration string raises click exception."""
+        param_type = DurationParamType()
+        with pytest.raises(click.BadParameter, match="is not a valid duration"):
+            param_type.convert("not-a-duration", None, None)
+

packages/jumpstarter-cli/jumpstarter_cli/create.py

@@ -1,12 +1,12 @@
-from datetime import timedelta
+from datetime import datetime, timedelta
 
 import click
 from jumpstarter_cli_common.config import opt_config
 from jumpstarter_cli_common.exceptions import handle_exceptions_with_reauthentication
 from jumpstarter_cli_common.opt import OutputType, opt_output_all
 from jumpstarter_cli_common.print import model_print
 
-from .common import opt_duration_partial, opt_selector
+from .common import opt_begin_time, opt_duration_partial, opt_selector
 from .login import relogin_client
 
 
@@ -21,9 +21,10 @@ def create():
 @opt_config(exporter=False)
 @opt_selector
 @opt_duration_partial(required=True)
+@opt_begin_time
 @opt_output_all
 @handle_exceptions_with_reauthentication(relogin_client)
-def create_lease(config, selector: str, duration: timedelta, output: OutputType):
+def create_lease(config, selector: str, duration: timedelta, begin_time: datetime | None, output: OutputType):
     """
     Create a lease
 
@@ -49,6 +50,6 @@ def create_lease(config, selector: str, duration: timedelta, output: OutputType)
 
     """
 
-    lease = config.create_lease(selector=selector, duration=duration)
+    lease = config.create_lease(selector=selector, duration=duration, begin_time=begin_time)
 
     model_print(lease, output)

packages/jumpstarter-cli/jumpstarter_cli/get.py

@@ -41,12 +41,19 @@ def get_exporters(config, selector: str | None, output: OutputType, with_options
 @opt_config(exporter=False)
 @opt_selector
 @opt_output_all
+@click.option(
+    "--all",
+    "show_all",
+    is_flag=True,
+    default=False,
+    help="Show all leases including expired ones"
+)
 @handle_exceptions_with_reauthentication(relogin_client)
-def get_leases(config, selector: str | None, output: OutputType):
+def get_leases(config, selector: str | None, output: OutputType, show_all: bool):
     """
     Display one or many leases
     """
 
-    leases = config.list_leases(filter=selector)
+    leases = config.list_leases(filter=selector, only_active=not show_all)
 
     model_print(leases, output)

packages/jumpstarter-cli/jumpstarter_cli/get_test.py

@@ -1,10 +1,11 @@
+from datetime import datetime, timedelta
 from unittest.mock import Mock
 
 import click
 import pytest
 from jumpstarter_cli_common.opt import parse_comma_separated
 
-from jumpstarter.client.grpc import Exporter, ExporterList, Lease
+from jumpstarter.client.grpc import Exporter, ExporterList, Lease, LeaseList
 from jumpstarter.config.client import ClientConfigV1Alpha1
 
 
@@ -239,3 +240,109 @@ def test_exporter_to_exporter_list_flow(self):
         assert exporter_list.exporters[1].name == "server-001"
         assert exporter_list.include_online is True
         assert exporter_list.include_leases is False
+
+
+class TestGetLeasesLogic:
+    """Tests for get leases command logic (simulating server-side filtering)"""
+
+    def create_test_lease(self, namespace="default", name="lease-1", status="In-Use",
+                          effective_begin_time=None, effective_end_time=None,
+                          duration=timedelta(hours=1)):
+        """Create a mock lease for testing"""
+        lease = Mock(spec=Lease)
+        lease.namespace = namespace
+        lease.name = name
+        lease.client = "test-client"
+        lease.exporter = "test-exporter"
+        lease.get_status.return_value = status
+        lease.effective_begin_time = effective_begin_time
+        lease.effective_end_time = effective_end_time
+        lease.duration = duration
+        lease.effective_duration = timedelta(minutes=30) if effective_begin_time else None
+        lease.begin_time = None
+        return lease
+
+    def test_only_active_excludes_expired_leases(self):
+        """Test that server returns only active leases when only_active=True"""
+        # When only_active=True, server returns only active lease
+        active_lease = self.create_test_lease(
+            name="active-lease",
+            status="In-Use",
+            effective_begin_time=datetime(2023, 1, 1, 10, 0, 0)
+        )
+
+        leases_from_server = LeaseList(leases=[active_lease], next_page_token=None)
+
+        assert len(leases_from_server.leases) == 1
+        assert leases_from_server.leases[0].name == "active-lease"
+        assert leases_from_server.leases[0].get_status() == "In-Use"
+
+    def test_show_all_includes_expired_leases(self):
+        """Test that server returns all leases including expired when only_active=False"""
+        # When only_active=False, server returns both active and expired
+        active_lease = self.create_test_lease(
+            name="active-lease",
+            status="In-Use",
+            effective_begin_time=datetime(2023, 1, 1, 10, 0, 0)
+        )
+        expired_lease = self.create_test_lease(
+            name="expired-lease",
+            status="Expired",
+            effective_begin_time=datetime(2023, 1, 1, 8, 0, 0),
+            effective_end_time=datetime(2023, 1, 1, 9, 0, 0)
+        )
+
+        leases_from_server = LeaseList(leases=[active_lease, expired_lease], next_page_token=None)
+
+        assert len(leases_from_server.leases) == 2
+        assert leases_from_server.leases[0].name == "active-lease"
+        assert leases_from_server.leases[1].name == "expired-lease"
+
+    def test_multiple_active_leases_returned(self):
+        """Test that server returns all active leases when only_active=True"""
+        # Server returns multiple active leases (different statuses but all non-expired)
+        lease1 = self.create_test_lease(
+            name="lease-1",
+            status="In-Use",
+            effective_begin_time=datetime(2023, 1, 1, 10, 0, 0)
+        )
+        lease2 = self.create_test_lease(
+            name="lease-2",
+            status="Waiting",
+            effective_begin_time=datetime(2023, 1, 1, 11, 0, 0)
+        )
+        lease3 = self.create_test_lease(
+            name="lease-3",
+            status="In-Use",
+            effective_begin_time=datetime(2023, 1, 1, 12, 0, 0)
+        )
+
+        leases_from_server = LeaseList(leases=[lease1, lease2, lease3], next_page_token=None)
+
+        assert len(leases_from_server.leases) == 3
+        assert all(lease.get_status() != "Expired" for lease in leases_from_server.leases)
+
+    def test_all_expired_when_show_all(self):
+        """Test that server can return only expired leases when only_active=False"""
+        # When only_active=False and all leases happen to be expired
+        expired1 = self.create_test_lease(
+            name="expired-1",
+            status="Expired",
+            effective_end_time=datetime(2023, 1, 1, 8, 0, 0)
+        )
+        expired2 = self.create_test_lease(
+            name="expired-2",
+            status="Expired",
+            effective_end_time=datetime(2023, 1, 1, 9, 0, 0)
+        )
+
+        leases_from_server = LeaseList(leases=[expired1, expired2], next_page_token=None)
+
+        assert len(leases_from_server.leases) == 2
+        assert all(lease.get_status() == "Expired" for lease in leases_from_server.leases)
+
+    def test_empty_lease_list(self):
+        """Test that server can return empty lease list"""
+        leases_from_server = LeaseList(leases=[], next_page_token=None)
+
+        assert len(leases_from_server.leases) == 0