Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doc/man/client.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ LG_PLACE
~~~~~~~~
This variable can be used to specify a place without using the ``-p`` option, the ``-p`` option overrides it.

LG_TOKEN
~~~~~~~~
LG_RESERVATION (previously LG_TOKEN)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This variable can be used to specify a reservation for the ``wait`` command and
for the ``+`` place expansion.

Expand Down
29 changes: 20 additions & 9 deletions doc/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ tags).
$ labgrid-client reserve board=imx6-foo
Reservation 'SP37P5OQRU':
owner: rettich/jlu
token: SP37P5OQRU
id : SP37P5OQRU
state: waiting
filters:
main: board=imx6-foo
Expand All @@ -76,7 +76,7 @@ tags).

As soon as any matching place becomes free, the reservation state will change
from ``waiting`` to ``allocated``.
Then, you can use the reservation token prefixed by ``+`` to refer to the
Then, you can use the reservation id prefixed by ``+`` to refer to the
allocated place for locking and usage.
While a place is allocated for a reservation, only the owner of the reservation
can lock that place.
Expand All @@ -86,15 +86,15 @@ can lock that place.

$ labgrid-client wait SP37P5OQRU
owner: rettich/jlu
token: SP37P5OQRU
id: SP37P5OQRU
state: waiting
filters:
main: board=imx6-foo
created: 2019-08-06 12:56:49.779982
timeout: 2019-08-06 12:58:14.900621
owner: rettich/jlu
token: SP37P5OQRU
id: SP37P5OQRU
state: allocated
filters:
main: board=imx6-foo
Expand All @@ -107,7 +107,7 @@ can lock that place.
$ labgrid-client reservations
Reservation 'SP37P5OQRU':
owner: rettich/jlu
token: SP37P5OQRU
id: SP37P5OQRU
state: acquired
filters:
main: board=imx6-foo
Expand All @@ -117,28 +117,39 @@ can lock that place.
timeout: 2019-08-06 12:59:11.840780
$ labgrid-client -p +SP37P5OQRU console


.. note::

Reservation tokens are now called reservation IDs.
As part of this terminology change, ``LG_TOKEN`` has been renamed to
``LG_RESERVATION``, and ``labgrid-client reserve --shell`` now exports
``LG_RESERVATION``.
Command output now uses ``id`` instead of ``token``.
The legacy ``LG_TOKEN`` variable remains accepted as input for backwards
compatibility, but scripts should migrate to ``LG_RESERVATION``.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section should export why we are forcing people to change to LG_RESERVATION in all workflows/scripts/CLI usage. IMO this is needed because LG_TOKEN implies a security property (which is not present here) since the meaning of TOKEN has changed over the last few years. Additional explanation or links are welcome as well.


When using reservation in a CI job or to save some typing, the ``labgrid-client
reserve`` command supports a ``--shell`` command to print code for evaluating
in the shell.
This sets the ``LG_TOKEN`` environment variable, which is then automatically
This sets the ``LG_RESERVATION`` environment variable, which is then automatically
used by ``wait`` and expanded via ``-p +``.

.. code-block:: bash

$ eval `labgrid-client reserve --shell board=imx6-foo`
$ echo $LG_TOKEN
$ echo $LG_RESERVATION
ZDMZJZNLBF
$ labgrid-client wait
owner: rettich/jlu
token: ZDMZJZNLBF
id: ZDMZJZNLBF
state: waiting
filters:
main: board=imx6-foo
created: 2019-08-06 13:05:30.987072
timeout: 2019-08-06 13:06:44.629736
owner: rettich/jlu
token: ZDMZJZNLBF
id: ZDMZJZNLBF
state: allocated
filters:
main: board=imx6-foo
Expand Down
61 changes: 36 additions & 25 deletions labgrid/remote/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ def __str__(self):
return f"{self.message}:\n{errors_combined}"


def _get_reservation_id_from_env():
reservation_id = os.environ.get("LG_RESERVATION")
if reservation_id:
return reservation_id

reservation_id = os.environ.get("LG_TOKEN")
if reservation_id:
print("warning: LG_TOKEN is deprecated; use LG_RESERVATION instead", file=sys.stderr)
return reservation_id


@attr.s(eq=False)
class ClientSession:
"""The ClientSession encapsulates all the actions a Client can invoke on
Expand Down Expand Up @@ -438,19 +449,19 @@ def _match_places(self, pattern):
"""
result = set()

# reservation token lookup
token = None
# reservation id lookup
reservation_id = None
if pattern.startswith("+"):
token = pattern[1:]
if not token:
token = os.environ.get("LG_TOKEN", None)
if not token:
reservation_id = pattern[1:]
if not reservation_id:
reservation_id = _get_reservation_id_from_env()
if not reservation_id:
return []
for name, place in self.places.items():
if place.reservation == token:
if place.reservation == reservation_id:
result.add(name)
if not result:
raise UserError(f"reservation token {token} matches nothing")
raise UserError(f"reservation id {reservation_id} matches nothing")
return list(result)

# name and alias lookup
Expand Down Expand Up @@ -1572,28 +1583,28 @@ async def create_reservation(self):

res = Reservation.from_pb2(response.reservation)
if self.args.shell:
print(f"export LG_TOKEN={res.token}")
print(f"export LG_RESERVATION={res.id}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to export the old LG_TOKEN here as well at least for a release. Also please add a TODO to eventually remove the LG_TOKEN export print after that.

else:
print(f"Reservation '{res.token}':")
print(f"Reservation '{res.id}':")
res.show(level=1)
if self.args.wait:
if not self.args.shell:
print("Waiting for allocation...")
await self._wait_reservation(res.token, verbose=False)
await self._wait_reservation(res.id, verbose=False)

async def cancel_reservation(self):
token: str = self.args.token
reservation_id: str = getattr(self.args, "reservation-id")

request = labgrid_coordinator_pb2.CancelReservationRequest(token=token)
request = labgrid_coordinator_pb2.CancelReservationRequest(token=reservation_id)

try:
await self.stub.CancelReservation(request)
except grpc.aio.AioRpcError as e:
raise ServerError(e.details())

async def _wait_reservation(self, token: str, verbose=True):
async def _wait_reservation(self, reservation_id: str, verbose=True):
while True:
request = labgrid_coordinator_pb2.PollReservationRequest(token=token)
request = labgrid_coordinator_pb2.PollReservationRequest(token=reservation_id)

try:
response: labgrid_coordinator_pb2.PollReservationResponse = await self.stub.PollReservation(request)
Expand All @@ -1609,8 +1620,8 @@ async def _wait_reservation(self, token: str, verbose=True):
break

async def wait_reservation(self):
token = self.args.token
await self._wait_reservation(token)
reservation_id = getattr(self.args, "reservation-id")
await self._wait_reservation(reservation_id)

async def print_reservations(self):
request = labgrid_coordinator_pb2.GetReservationsRequest()
Expand All @@ -1622,7 +1633,7 @@ async def print_reservations(self):
raise ServerError(e.details())

for res in sorted(reservations, key=lambda x: (-x.prio, x.created)):
print(f"Reservation '{res.token}':")
print(f"Reservation '{res.id}':")
res.show(level=1)

async def export(self, place, target):
Expand Down Expand Up @@ -2210,11 +2221,11 @@ def get_parser(auto_doc_mode=False) -> "argparse.ArgumentParser | AutoProgramArg
subparser.set_defaults(func=ClientSession.create_reservation)

subparser = subparsers.add_parser("cancel-reservation", help="cancel a reservation")
subparser.add_argument("token", type=str, nargs="?")
subparser.add_argument("reservation-id", type=str, nargs="?", help="the reservation id (previously called token)")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
subparser.add_argument("reservation-id", type=str, nargs="?", help="the reservation id (previously called token)")
subparser.add_argument("reservation_id", type=str, nargs="?", help="the reservation id (previously called token)", metavar="reservation-id")

and we can replace the various getattr(args, "reservation-id") calls with args.reservation_id.

subparser.set_defaults(func=ClientSession.cancel_reservation)

subparser = subparsers.add_parser("wait", help="wait for a reservation to be allocated")
subparser.add_argument("token", type=str, nargs="?")
subparser.add_argument("reservation-id", type=str, nargs="?", help="the reservation id (previously called token)")
subparser.set_defaults(func=ClientSession.wait_reservation)

subparser = subparsers.add_parser("reservations", help="list current reservations")
Expand Down Expand Up @@ -2257,7 +2268,6 @@ def main():
state = os.environ.get("STATE", None)
state = os.environ.get("LG_STATE", state)
initial_state = os.environ.get("LG_INITIAL_STATE", None)
token = os.environ.get("LG_TOKEN", None)

parser = get_parser()

Expand All @@ -2281,11 +2291,12 @@ def main():
if args.initial_state is None:
args.initial_state = initial_state

if args.command in ["cancel-reservation", "wait"] and args.token is None:
if token:
args.token = token
if args.command in ["cancel-reservation", "wait"] and getattr(args, "reservation-id") is None:
reservation_id = _get_reservation_id_from_env()
if reservation_id:
setattr(args, "reservation-id", reservation_id)
else:
print("Please provide a token", file=sys.stderr)
print("Please provide a reservation id (previously called token)", file=sys.stderr)
exit(1)

if args.verbose:
Expand Down
8 changes: 4 additions & 4 deletions labgrid/remote/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ class ReservationState(enum.Enum):
@attr.s(eq=False)
class Reservation:
owner = attr.ib(validator=attr.validators.instance_of(str))
token = attr.ib(
id = attr.ib(
default=attr.Factory(lambda: "".join(random.choice(string.ascii_uppercase + string.digits) for i in range(10)))
)
state = attr.ib(
Expand Down Expand Up @@ -428,7 +428,7 @@ def expired(self):
def show(self, level=0):
indent = " " * level
print(indent + f"owner: {self.owner}")
print(indent + f"token: {self.token}")
print(indent + f"id: {self.id}")
print(indent + f"state: {self.state.name}")
if self.prio:
print(indent + f"prio: {self.prio}")
Expand All @@ -445,7 +445,7 @@ def show(self, level=0):
def as_pb2(self):
res = labgrid_coordinator_pb2.Reservation()
res.owner = self.owner
res.token = self.token
res.token = self.id
res.state = self.state.value
res.prio = self.prio
for name, fltr in self.filters.items():
Expand All @@ -471,7 +471,7 @@ def from_pb2(cls, pb2: labgrid_coordinator_pb2.Reservation):
allocations[fltr_name] = [place_name]
return cls(
owner=pb2.owner,
token=pb2.token,
id=pb2.token,
state=ReservationState(pb2.state),
prio=pb2.prio,
filters=filters,
Expand Down
44 changes: 22 additions & 22 deletions labgrid/remote/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -968,10 +968,10 @@ def schedule_reservations(self):
res.state = ReservationState.expired
res.allocations.clear()
res.refresh()
print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}")
print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}")
else:
del self.reservations[res.token]
print(f"removed {res.state.name} reservation ({res.owner}/{res.token})")
del self.reservations[res.id]
print(f"removed {res.state.name} reservation ({res.owner}/{res.id})")

# check which places are already allocated and handle state transitions
allocated_places = set()
Expand All @@ -985,7 +985,7 @@ def schedule_reservations(self):
res.state = ReservationState.invalid
res.allocations.clear()
res.refresh(300)
print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}")
print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}")
break
if place.acquired is not None:
acquired_places.add(name)
Expand All @@ -995,12 +995,12 @@ def schedule_reservations(self):
# an allocated place was acquired
res.state = ReservationState.acquired
res.refresh()
print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}")
print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}")
if not acquired_places and res.state is ReservationState.acquired:
# all allocated places were released
res.state = ReservationState.allocated
res.refresh()
print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}")
print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}")

# check which places are available for allocation
available_places = set()
Expand All @@ -1027,16 +1027,16 @@ def schedule_reservations(self):
place_tagsets.append(TagSet(name, tags))
filter_tagsets = []
for res in pending_reservations:
filter_tagsets.append(TagSet(res.token, set(res.filters["main"].items())))
filter_tagsets.append(TagSet(res.id, set(res.filters["main"].items())))
allocation = schedule(place_tagsets, filter_tagsets)

# apply allocations
for res_token, place_name in allocation.items():
res = self.reservations[res_token]
for res_id, place_name in allocation.items():
res = self.reservations[res_id]
res.allocations = {"main": [place_name]}
res.state = ReservationState.allocated
res.refresh()
print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}")
print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}")

# update reservation property of each place and notify
old_map = {}
Expand All @@ -1051,10 +1051,10 @@ def schedule_reservations(self):
for group in res.allocations.values():
for name in group:
assert name not in new_map, "conflicting allocation"
new_map[name] = res.token
new_map[name] = res.id
place = self.places.get(name)
assert place is not None, "invalid allocation"
place.reservation = res.token
place.reservation = res.id
for name in old_map.keys() | new_map.keys():
if old_map.get(name) != new_map.get(name):
self._publish_place(self.places[name])
Expand All @@ -1079,28 +1079,28 @@ async def CreateReservation(self, request: labgrid_coordinator_pb2.CreateReserva

owner = self.clients[peer].name
res = Reservation(owner=owner, prio=request.prio, filters=fltrs)
self.reservations[res.token] = res
self.reservations[res.id] = res
self.schedule_reservations()
return labgrid_coordinator_pb2.CreateReservationResponse(reservation=res.as_pb2())

@locked
async def CancelReservation(self, request: labgrid_coordinator_pb2.CancelReservationRequest, context):
token = request.token
if not isinstance(token, str) or not token:
await context.abort(grpc.StatusCode.INVALID_ARGUMENT, f"Invalid token {token}")
if token not in self.reservations:
await context.abort(grpc.StatusCode.FAILED_PRECONDITION, f"Reservation {token} does not exist")
del self.reservations[token]
reservation_id = request.token
if not isinstance(reservation_id, str) or not reservation_id:
await context.abort(grpc.StatusCode.INVALID_ARGUMENT, f"Invalid id {reservation_id}")
if reservation_id not in self.reservations:
await context.abort(grpc.StatusCode.FAILED_PRECONDITION, f"Reservation {reservation_id} does not exist")
del self.reservations[reservation_id]
self.schedule_reservations()
return labgrid_coordinator_pb2.CancelReservationResponse()

@locked
async def PollReservation(self, request: labgrid_coordinator_pb2.PollReservationRequest, context):
token = request.token
reservation_id = request.token
try:
res = self.reservations[token]
res = self.reservations[reservation_id]
except KeyError:
await context.abort(grpc.StatusCode.FAILED_PRECONDITION, f"Reservation {token} does not exist")
await context.abort(grpc.StatusCode.FAILED_PRECONDITION, f"Reservation {reservation_id} does not exist")
res.refresh()
return labgrid_coordinator_pb2.PollReservationResponse(reservation=res.as_pb2())

Expand Down
Loading
Loading