Skip to content

Add "message" field to DevWorkspace status and column for kubectl get #220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
amisevsk opened this issue Nov 18, 2020 · 7 comments · Fixed by #221
Closed

Add "message" field to DevWorkspace status and column for kubectl get #220

amisevsk opened this issue Nov 18, 2020 · 7 comments · Fixed by #221
Labels
area/devworkspace Improvent or additions to the DevWorkspaces CRD kind/enhancement New feature or request

Comments

@amisevsk
Copy link
Contributor

Is your feature request related to a problem? Please describe.

There are many valid reasons for a DevWorkspace startup to fail and we represent status this via the Failed value for .status.phase. However, the only way to get insight into why a particular DevWorkspace failed to start is by inspecting the full yaml of the DevWorkspace and checking which Conditions are set.

Describe the solution you'd like

We should add a field, e.g. Message or StatusInfo to DevWorkspaceStatus and include it in the output for kubectl get. This might result in output such as

$ kc get devworkspaces
NAME    WORKSPACE ID                PHASE    URL   INFO
theia   workspacecd070100795e4809   Failed         Plugin X not found in registry

cc: @davidfestal

@amisevsk amisevsk added kind/enhancement New feature or request area/devworkspace Improvent or additions to the DevWorkspaces CRD labels Nov 18, 2020
@sleshchenko
Copy link
Member

Do we have any example when INFO may be not empty for not failed state?

@JPinkney
Copy link
Contributor

Personally, I like this idea. I dislike having to dig through the logs to find out why my devworkspace failed. I'd prefer it if we had some easy way to find out why it failed for users (and even ourselves)

@davidfestal
Copy link
Collaborator

@amisevsk Do we want to use the same field names are those used in PodStatus for consistency:

  • message: A human readable message indicating details about why the pod is in this condition.
  • reason: A brief CamelCase message indicating details about why the pod is in this state. e.g. Evicted

@amisevsk
Copy link
Contributor Author

Do we have any example when INFO may be not empty for not failed state?

In general I'd use the field to communicate info that requires user intervention, since it cannot be hidden and a normal running state is captured by phase = Running. Ideally we'd hide the field when empty in the get devworkspaces output

Do we want to use the same field names are those used in PodStatus for consistency:

That makes sense, I'm generally bad at naming things :D

@johnmcollier
Copy link
Member

Just chiming in here since I'm trying to do a similar thing for the registry operator - what about exposing the detailed status message as part of the kubectl describe devworkspace <resource> output on each state change?

Seems like that can be done with the manager's event recorder.

@amisevsk
Copy link
Contributor Author

Just chiming in here since I'm trying to do a similar thing for the registry operator - what about exposing the detailed status message as part of the kubectl describe devworkspace output on each state change?

Seems like that can be done with the manager's event recorder.

Sorry, getting back to this a little late. I don't know offhand how to do this -- could we create a follow-up issue to plan this? I think it's a good idea (I never think to use kubectl describe)

@johnmcollier
Copy link
Member

Yeah, I think that sounds reasonable 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devworkspace Improvent or additions to the DevWorkspaces CRD kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants