"cml ls --all-users" displays wrong lab owners

[sgherdao]

Describe the bug

cml lab ls --all-users returns the wrong owner username:

❯ cml ls --all-users

Labs on Server
╒══════════════════════════════════════╤════════════════════════╤═════════════════════════════════════════╤══════════╤═════════════════╤═════════╤═════════╤══════════════╕
│ ID                                   │ Title                  │ Description                             │ Owner    │ Status          │   Nodes │   Links │   Interfaces │
╞══════════════════════════════════════╪════════════════════════╪═════════════════════════════════════════╪══════════╪═════════════════╪═════════╪═════════╪══════════════╡
├──────────────────────────────────────┼────────────────────────┼─────────────────────────────────────────┼──────────┼─────────────────┼─────────┼─────────┼──────────────┤
│ 931b20a1-f0be-4c9f-b7ef-4db96ee77135 │ Multi Platform Network │ A sample network built with IOS XE, NX- │ admin    │ STOPPED         │      14 │      32 │          101 │
│                                      │                        │ OS, IOS XR, and ASA devices.  Includes  │          │                 │         │         │              │
│                                      │                        │ Linux hosts.                            │          │                 │         │         │              │
├──────────────────────────────────────┼────────────────────────┼─────────────────────────────────────────┼──────────┼─────────────────┼─────────┼─────────┼──────────────┤
│ 4c916ee7-bef2-4545-9a92-a28d906c24e5 │ WONDERLAB              │                                         │ admin    │ DEFINED_ON_CORE │       0 │       0 │            0 │
╘══════════════════════════════════════╧════════════════════════╧═════════════════════════════════════════╧══════════╧═════════════════╧═════════╧═════════╧══════════════╛

WONDERLAB belongs to user alice, not admin

Change introduced by PR #149 and commit 0780c34

it changed from lab.owner to lab.username in the below file:

virlutils/virl/cli/views/labs/lab_views.py

Lines 16 to 27 in b3bc0b8

	def print_labs(labs):
	table = list()
	headers = ["ID", "Title", "Description", "Owner", "Status", "Nodes", "Links", "Interfaces"]
	for lab in labs:
	tr = list()
	tr.append(lab.id)
	tr.append(lab.title)
	wrapped_description = textwrap.fill(lab.description, width=40)
	tr.append(wrapped_description)
	tr.append(lab.username)
	status = lab.state()
	stats = lab.statistics

It would appear that lab.username is not the owner of the lab but the currently logged in user 😅.

I am checking with @tmikuska, he is checking upstream meanwhile I see a the following as potential solution:

We are currently doing this:

virlutils/virl/cli/ls/commands.py

Lines 25 to 37 in b3bc0b8

	def ls(all, all_users):
	"""
	lists running labs and optionally those in the cache
	"""
	server = VIRLServer()
	client = get_cml_client(server)
	labs = []
	cached_labs = None
	lab_ids = client.get_lab_list(all_users)
	for id in lab_ids:
	labs.append(client.join_existing_lab(id))

So an HTTP call to get all the lab IDs client.get_lab_list(all_users)
then another one for each lab another call, labs.append(client.join_existing_lab(id))

Then in the view, we make a few more calls (properties/methods... although most are cached):

virlutils/virl/cli/views/labs/lab_views.py

Lines 16 to 27 in b3bc0b8

	def print_labs(labs):
	table = list()
	headers = ["ID", "Title", "Description", "Owner", "Status", "Nodes", "Links", "Interfaces"]
	for lab in labs:
	tr = list()
	tr.append(lab.id)
	tr.append(lab.title)
	wrapped_description = textwrap.fill(lab.description, width=40)
	tr.append(wrapped_description)
	tr.append(lab.username)
	status = lab.state()
	stats = lab.statistics

Proposed Solution

Instead of making 2 calls to client.get_lab_list(all_users) and join_existing_lab(id), we could make 2 different calls:

1. client.all_labs(show_all=True)
2. Get a dict with a lab.details() to consume in the view

In [1]: all_labs = client.all_labs(show_all=True)

In [2]: all_labs
Out[2]:
[Lab('Multi-SA HSRP', '0786b045-aa39-4e98-af01-575b22566cf2', '/api/v0/', True, 1.0, True),
 Lab('WONDERLAB', '4c916ee7-bef2-4545-9a92-a28d906c24e5', '/api/v0/', True, 1.0, True)]

In [3]: all_labs[-1].username
Out[3]: 'admin'

In [4]: wonderlab = all_labs[-1].details()

In [5]: wonderlab
Out[5]:
{'state': 'DEFINED_ON_CORE',
 'created': '2024-06-26T16:11:08+00:00',
 'modified': '2024-06-26T16:11:15+00:00',
 'lab_title': 'WONDERLAB',
 'lab_description': '',
 'lab_notes': '',
 'owner': '336e0bb9-5b4d-418d-ae48-92544ef5c590',
 'owner_username': 'alice',
 'owner_fullname': '',
 'node_count': 2,
 'link_count': 1,
 'id': '4c916ee7-bef2-4545-9a92-a28d906c24e5',
 'groups': []}

The details returns actually what we need to pass to the view and more (maybe something we want to print in the future)
Small note, the dict doesn't contain interfaces info ... so we also need to keep the call lab.statistics but that should be cheap, I think

https://github.com/CiscoDevNet/virl2-client/blob/5bf320f62214c39a5615d19fda9a932f67d332ce/virl2_client/models/lab.py#L389C1-L402C10

@xorrkaz let me know your thoughts and I will prepare a PR

[tmikuska]

lab.username is indeed the username of the user that has opened the current client library session and loaded labs. The reason why we set this and lab.password is that it is being used for auto re-auth and pyats sync. But that's a completely different issue which we may need to look into in the client library.

I think that you should use lab.owner, but I'm afraid that it won't make difference because it seems that the client library sets owner to username when owner is not available. We are looking into this issue in the client library.

If it's not a big deal, you could make two 2 calls at least temporarily. lab.details calls a different endpoint than lab.join_existing_lab which provides more information about the owner.

[sgherdao]

Thanks @tmikuska for looking into this, I will prepare a PR to use client.all_labs() then lab.details()

[xorrkaz]

That sounds reasonable. We just need to understand the roundtrip cost. If it's comparable, this seems like a decent solution.

[sgherdao]

I submitted #153, there is an other potential solution, inside the view, we can make a call to client.users and get a mapping owner_id -> username.

In terms of performance, it might be slightly better but I think overall won't make a big difference, but would probably require less code modifications (e.g. CachedLab).

I thought about that option first but discarded it because I thoughtclient.all_labs() would be just one REST API call (in reality it is multiple as it calls join_existing_lab() under the hood).

[sgherdao]

made an alternate PR

[xorrkaz]

Fixed in 2.2.0. Thanks!