Jupyter resource should indicate when notebook is running

[jlewi]

We want the jupyter web app to report when the notebook is ready so the user knows when to try to connect to it (#2588).

#2558 modified the jupyter notebook resource to add a condition "READY"

But the READY condition doesn't take into account whether the container is actually created and running.

We should update the conditions to indicate when the jupyter container is actually running.

We need this for #2417 so that we can surface to users when the notebook is ready to connect to.

Related to: #2417

[jlewi]

/cc @zabbasi

[vkoukis]

Two questions here, after the discussion in today's community meeting:

1. @jlewi Ca we make this part of 0.5., so that in combination with jupyter web app should provide status of notebook #2588 we have full e2e status reporting for notebooks? This way we will show significant UX improvements in 0.5, compared to 0.4.

2. If I understand correctly, the plan in this issue is to expose the full pod status as part of the status field of the Notebook instance in the K8s API, right? At the API level, I as a user know that the Notebook instance is backed by a StatefulSet -> pod, so I think we should surface as much information as we can, e.g., ContainerCreating, Running, etc. And then, the UI can translate all the low-level information into something user-friendly, as part of jupyter web app should provide status of notebook #2588.

[vkoukis]

I think I should also /cc @lluunn here

[lluunn]

@vkoukis
I think StatefulSet or Deployment only surfaces things like number of replicas that's ready.
Is that enough?

If we want to show the more low level detail, is it possible that the UI go query the pod directly (use labels)?

[zabbasi]

@vkoukis following @lluunn comment I'm working to surface ReadyReplicas.

[vkoukis]

@lluunn
Hello Lun-kai,

I think StatefulSet or Deployment only surfaces things like number of replicas that's ready.
Is that enough?
The controller currently runs a StatefulSet for a single replica. So if you implement just reporting of ready replicas, then we will essentially have a binary field: Either the Notebook is ready, or it's not. There is no way to expose to the user that the Notebook is being created, or that something has gone wrong.

For example, we have seen that one of the biggest user gripes -- right now -- is that once they create a notebook, they don't have any feedback on its progress. Think how great it will be if the UI can translate ImagePullBackOff or ErrImagePull or similar states into a yellow triangle / "Warning" state, and expose the fact that Kubeflow is trying to create the Notebook server again and again.

Surfacing ReadyReplicas would be an improvement, but I think the UX would absolutely rock if we can surface the state of each individual pod of the StatefulSet [well, there is only a single pod, so...]

If we want to show the more low level detail, is it possible that the UI go query the pod directly (use labels)?

I argue against this:

So far, the UI only know about Notebook instances. It submits Notebook instances, and queries them. It would be very strange to actually start using labels to query the underlying pods. It doesn't even know [and shouldn't know] that there is a StatefulSet underneath. The Controller knows that it creates a StatefulSet per notebook. It also knows that it sets its replicas to 1. So, I think it's best for the controller to surface per-pod information, as part of the Notebook's status.

Also, a lesser argument would be that the UI may only have access to Notebook objects in the K8s API, and it would be good from a security perspective to keep its rights as limited as possible, so querying other objects may be out of reach.

What do you think?

[jlewi]

@vkoukis What's the latest on this?

[lluunn]

@zabbasi 's #2743 added ReadyReplica to notebook status,
I am working on #2787 to relay container status to notebook.

[vkoukis]

@jlewi I'm watching #2787, looks great! Once this is merged [I'm seeing you're retesting it right now], we'll submit a PR for #2588 and close it before the end of the week.

[vkoukis]

/cc @kimwnasptd

[vkoukis]

/assign @kimwnasptd

[lluunn]

fixed bu #2787