kn service describe should show # of instances

[duglin]

Today kn service describe will show this in it's output:

Revisions:  
  100%  @latest (echo-yyhmp-6) [6] (4d)

The [6] is interesting. I believe that the generation number - or in user's terms, my version # of my app. That's (sort of) interesting, but at first I thought it was the # of instances of this Revision that were running... that would be far more useful to me.

Proposal: show the # of instances of each revision instead of the generation #. I'm not really sure what the generation # tells the user, or what interesting thing they can do with it. It seems to me that seeing the revision name change in that list would be enough to indicate that there are multiple revisions out there.

This may be controversial because it means using a non-knative api (e.g. pods, or deployment) but I think we need to think about what's best (and most useful) for the user, and I think being able to see how their app is scaling (perhaps out of control!) would be very useful.

We could even make it conditional, and try to get the list number of instances and if it fails due to an known API then just not show it (and no error msg), but if the API is there then show the info.

[danielhelfand]

Would it perhaps be clearer to call this out by including a more explicit property under Revisions to denote that it signifies the generation? Mainly this suggestion is to keep this information available, but think additional information like you're suggesting on instances is valuable as well.

Revisions:  
  100%  @latest (echo-yyhmp-6) (4d)
  Image: ...
  Generation: 6

[rhuss]

I totally agree that the number of current replicas is useful to show (I hope this is part of the Revision resource, don't know this by heart). The generation is useful when compared against other revisions, just to easily spot which one is newer (also newer version) in which one is older. As absolute number, the generation is not of much use, agreed.

[rhuss]

Would it perhaps be clearer to call this out by including a more explicit property under Revisions to denote that it signifies the generation? Mainly this suggestion is to keep this information available, but think additional information like you're suggesting on instances is valuable as well.

There is always kind of a compromise between available screen estate and verbosity, especially when it comes to lists. The list of revisions might not be that long to maybe we overdid a bit with the conciseness. I would like to rework the presented information anyway, as I'm also not so happy how we visualize the "@latest" property and the images hashes (e.g. it could be that the same revision is listed twice in kn service describe, which imo is confusing).

[sixolet]

Knative revisoins don't in themselves include information about number of instances. This would work as a plugin though.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[duglin]

/remove-lifecycle stale
still would like to see this one

[rhuss]

If we would get to the number that would be quite cool but as mentioned it's really hard to impossible to get the number of running pods that belong to a revision as this required inspecting the corresponding Deploymentwhich is not accesible via Knative API. A plugin could help, but this would be a complete different thing as it would replace kn service describe (e.g. could be a kn-service-show.

Otherwise I agree that the presentation of kn service describe is not optimal, especially as the same revision can appear multiple time in the list of revisions that is associated to a service. This should be improved and we then also can question whether the generation number, which shows you the historical order for the revision is useful (for me it was useful to find out the latest revision deployed).

[duglin]

This touches on something we've discussed before... are we a K8s project or not? IMO it feels like we're doing a disservice to our users by avoiding something as useful as this. However, having said that, can we support this IF the API (data) is available and just return n/a if it's not (or remove the field entirely). This then gives the freedom for non-K8s backends to optionally support these other APIs over time. But at least then we have a common API that everyone can adhere to.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[duglin]

ping

[duglin]

/remove-lifecycle stale

[rhuss]

@duglin I still think that your ask is quite reasonable but I still don't see an easy solution to implement this. Technically we could implement it ...

• ... by adding an additional status field in the Revision that indicates the number of replicas configured in the backing Deployment. This would require changes in serving. @knative/serving-writers (btw is this the proper alias to use to ping serving folks ?) do you see an easy way to expose this information (nr replicas) within a revision's status ?
• ... by accessing the Deployment directly, but this is out of the scope of the Knative API contract. We could either work on that contract (as @duglin mentioned above in a comment to move close to the K8s APIs) or we implement kind of a 'describe plugin' system in kn that allows to plugin additional information when calling kn service describe. This would be much more effort, but would allow for showing off more, custom information like specific annotations that has been added by a user to their service (imagine company-wide conventions to label their services in a certain way)

[markusthoemmes]

We could update the Revision's status with the number of replicas surfaced by the Deployment no problem.

[rhuss]

Ah, that would be very cool and very helpful so that we can visualize this with kn service describe. Going to open an issue in https://github.com/knative/serving ...

Thx!

[duglin]

Having the revision.status show the desired size of the deployment would be nice. If I understood it properly, on yesterday's API call I think Matt mentioned the idea of introducing an abstraction between Revisions and Deployments so that someone could plug-in something other than a Deployment to manage the instances. While it would seem odd, it might be possible that the desired size of that yet-to-be-created scaling resource might not be exposed via it's API (meaning it could be a write-only field). And therefore having Knative's data (current desired scale) kept in Knative owned resources would be nice.

@markusthoemmes I'm assuming it's the desired count, not actual count - right?

[markusthoemmes]

@duglin up to you to decide. Both would work for deployments.

[duglin]

interesting, I would think it would be annoying to update the revision's status each time some pods died. But if it's trivial, then having a "desired" and "actual" instance count would be really cool.

[markusthoemmes]

Trivial to implement, non-trivial in terms of API churn, potentially :P

[vagababov]

Well, it's easy in terms of API as well (just take it off KPA) and populate status.
It's more of a KN work where it has to distinguish between 0 - scaled to 0, 0 - failed and 0 - server is older than client and does not set the field.

[dprotaso]

Can we start poking at this in the client given knative/serving#11038 has landed

[rhuss]

@dprotaso thanks for the heads, going to hunt for somebody to implement it ;-)

[rhuss]

Note to the implementer: When checking for the corresponding status fields (actualReplica, desiredReplicas) care should be taken that these fields might be missing when running against older Knative cluster. kn describe should then do not show anything related to the replicas.