Clean shutdown of shared_clients associated with a spawner can fail in the "ApiClient" circumstance

[athornton]

Bug description

I instantiated a shared_client in a spawn hook, and expected it to be quietly cleaned up when the spawner exited.

Instead I got a stack trace and a complaint about an unawaited Task.

Expected behaviour

I thought I would not get an error in my logs as my pod spawned.

Actual behaviour

I get:

ERROR:asyncio:Task exception was never retrieved
future: <Task finished name='Task-81' coro=<shared_client.<locals>.close_client_task() done, defined at /usr/local/lib/python3.8/dist-packages/kubespawner/clients.py:55> exception=AttributeError("'ApiClient' object has no attribute 'api_client'")>
Traceback (most recent call last):
  File "/usr/local/lib/python3.8/dist-packages/kubespawner/clients.py", line 57, in close_client_task
    async with client.api_client:
AttributeError: 'ApiClient' object has no attribute 'api_client'

How to reproduce

Add a pre-spawn hook that instantiates shared_client("ApiClient"); when the object goes out of scope, you get that error.

Your personal set up

GKE, zero-to-jupyterhub recent prerelease. The shared_client is used in the resourcemanager from https://github.com/lsst-sqre/nublado2 .

Conclusion

I can submit a PR for this. What's going on is that a shared_client invoked with "ApiClient" (rather than an actual K8s API such as "CoreV1API") doesn't get an api_client attribute, because it is the ApiClient attribute.

"Why on earth would you do that?" I hear you inquire. We want to cheat and lean on something else to do the serialization and camelCase to snake_case stuff as we're creating objects to throw around with our user pods.

Rubin Observatory is likely the only place in the world to have hit this, but, hey, may as well fix it upstream. I'll be back in a little while, maybe tomorrow, with a PR for you.

[athornton]

#603 should fix it, but I haven't actually tested exactly this on my own setup and won't be able to until tomorrow.

[rra]

I think the root problem here is that shared_client used to be able to retrieve a generic ApiClient, but no longer can because fo the cleanup task. The right fix may be to just retrieve a CoreV1Api object instead and then use its underlying api_client attribute.

Alternately, if the intent is to support this, the cleanup task should choose between client.api_client and using client directly based on whether isinstance(client, ApiClient) is true.

[athornton]

Well, now that our code is just grabbing the attribute from a CoreV1Api, there's probably no need to support shared_client("ApiClient"). I'll close this issue and withdraw the PR.