[User identity] Fix identity check

[Michaelvll]

Describe the changes in this PR:
(related to #1513) This PR fixes the identity check for the sky status -r as it does not go through refresh_cluster_status_handle.

Tested (run the relevant ones):

• sky launch -c test -t t3.micro; AWS_PROFILE=admin-user sky status -r

[concretevitamin]

I do not clearly remember the differences between _update_cluster_status() and refresh_cluster_status_handle(). After reading the docstrings, still having some troubles telling the differences.

The status -r code path calls the former:

skypilot/sky/backends/backend_utils.py

Lines 2046 to 2054 in c86b69d

	def _refresh_cluster(cluster_name):
	try:
	record = _update_cluster_status(
	cluster_name, acquire_per_cluster_status_lock=True)
	except (exceptions.ClusterStatusFetchingError,
	exceptions.ClusterOwnerIdentityMismatchError) as e:
	record = {'status': 'UNKNOWN', 'error': e}
	progress.update(task, advance=1)
	return record

Would this change be needed if it calls the latter? Which already had identity checks.

[Michaelvll]

The main difference is the former returns the entire record but the latter returns the (status, handle) tuple. If we let the status -r call the later, we may have to add a argument to the latter function to return the record. I would prefer the current implementation, since the refresh_cluster_status_handle is public and used a lot in other modules.

[Michaelvll]

I refactored it a bit, by adding a new function refresh_cluster_record, and all the callers will go through this function now. Does that look better @concretevitamin? : )

[concretevitamin]

LGTM. Mainly to clarify the two functions (I feel they do the same thing and differ on in return values).

[concretevitamin]

Suggested change

	"""Refresh the cluster record.
	"""Refresh the cluster, and return the possibly updated record.

Conceptually I feel the difference between the two functions is only what is returned?

[Michaelvll]

Yes, the only difference is the return value.