Skip to content

Add additional field to devworkspace status for showing info to users #221

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

Merged
merged 2 commits into from
Dec 22, 2020

Conversation

amisevsk
Copy link
Contributor

What does this PR do?

Adds a field to the status subresource of DevWorkspaces which can be used to provide additional information about e.g. workspace failures and is printed in the short-form output of commands like kuberctl get devworkspaces

What issues does this PR fix or reference?

Resolves #220, but should not be merged without discussion in the issue. I just wrote the PR since it's no effort to implement.

Is your PR tested? Consider putting some instruction how to test your changes

When applied to a cluster, output of kubectl get devworkspaces is something like

kc get devworkspace
NAME    WORKSPACE ID                PHASE    INFO   URL
theia   workspaceb8b430aa340343da   Failed

(propagating failure reason to the status is not yet implemented on the controller side)

Copy link
Collaborator

@davidfestal davidfestal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment on the issue: #220 (comment) about trying to be consistent with the corresponding fields in PodStatus for example ?

@amisevsk
Copy link
Contributor Author

@davidfestal Finally got around to updating this PR :)

@amisevsk
Copy link
Contributor Author

The conversion tests, as if to tell me they're actually useful, found a bug in conversion :). It is fixed now.

Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one question left

@amisevsk
Copy link
Contributor Author

The output for the current state of this PR is

NAME    WORKSPACE ID                PHASE      INFO   URL
theia   workspace210c0f0d88854521   Starting          http://workspace210c0f0d88854521-theia-3100.192.168.49.2.nip.io

Ideally we would be able to hide empty fields; I'm also considering moving INFO to be last in the list, but that may be confusing.

Failed workspaces still do not put anything there, as the changes aren't implemented in the controller.

@amisevsk
Copy link
Contributor Author

cc: @davidfestal waiting on your re-review

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: amisevsk, davidfestal, sleshchenko
To complete the pull request process, please assign after the PR has been reviewed.
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "message" field to DevWorkspace status and column for kubectl get
4 participants