Skip to content
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

Routing errors propagation #385

Merged
merged 4 commits into from
Apr 16, 2021

Conversation

amisevsk
Copy link
Collaborator

@amisevsk amisevsk commented Apr 14, 2021

What does this PR do?

Adds a string field .status.message to DevWorkspaceRoutings, and uses this value in place of "preparing networking" in DevWorkspaces that are waiting for routing to be ready. This means that user-readable failure reasons can be set in DevWorkspaceRoutings and are automatically propagated to the DevWorkspace status.

Also adds printColumns to DevWorkspaceRoutings to improve readability for short output:

❯ kc get dwr
NAME                                DEVWORKSPACE ID             PHASE   INFO
routing-workspaceda77c562abd649a1   workspaceda77c562abd649a1   Ready   DevWorkspaceRouting prepared

Commit messages should be clear for what each commit adds. Note that I've also added propagating messages from dwr to dw status even when the dwr is not failed, but since a the dwr is ready so quickly in normal usage the message never reaches the DevWorkspace's status, so we never see Preparing [services|routes|ingresses]. This might still be useful if e.g. there is an issue applying resources, but might also make tracking progress harder (since the messages may or may not appear)

What issues does this PR fix or reference?

Closes eclipse-che/che#17455

Is it tested? How?

Easiest to test on Kubernetes as otherwise all routingClasses are valid and the default dwr reconciler cannot fail:

{ cat <<EOF | kubectl apply -f -
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: theia-next
spec:
  started: true
  routingClass: cluster-tls
  template:
    components:
      - name: theia
        plugin:
          uri: https://che-plugin-registry-main.surge.sh/v3/plugins/eclipse/che-theia/next/devfile.yaml
EOF
} && kubectl get dw -w

Output:

❯ kc apply -f samples/theia-next.yaml && kc get dw -w
devworkspace.workspace.devfile.io/theia-next created
NAME         DEVWORKSPACE ID             PHASE      INFO
theia-next   workspaceba44c7e060774cb3           
theia-next   workspaceba44c7e060774cb3   Starting   Preparing networking
theia-next   workspaceba44c7e060774cb3   Failed     Invalid routingClass for DevWorkspace: routing class cluster-tls only supported on OpenShift

Add string field .status.message to devworkspaceroutings to allow the
devworkspacerouting reconciler to propagate error messages up to the
owner DevWorkspace.

Also add a few printColumns to print useful information about the
DevWorkspaceRouting:
- The owner's DevWorkspaceId
- The current phase
- The current status.message

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
When a DevWorkspaceRouting enters the failed phase, propagate the
message in `routing.status.message` to the DevWorkspace as a failure
reason.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Propagate messages (where appropriate) from DevWorkspaceRouting to
DevWorkspaces (e.g. "preparing services/ingresses/routes").

Note that for a typical successful startup, these messages will never be
reconciled to the DevWorkspace (as the routing enters the ready state
too quickly) but could help diagnose issues if a particular step occurs
slowly.

Also change how status for DevWorkspaceRoutings is synced to ensure that
"Preparing" phase is entered while resources are being created.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
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.

I haven't tested but changes LGTM

@amisevsk
Copy link
Collaborator Author

/test v7-devworkspaces-operator-e2e

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, JPinkney, metlos, sleshchenko

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [JPinkney,amisevsk,sleshchenko]

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

@amisevsk amisevsk merged commit a752146 into devfile:main Apr 16, 2021
@amisevsk amisevsk deleted the routing-errors-propagation branch April 16, 2021 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[devworkspace-controller] Propagate errors from provisioning components and/or workspaceRoutings to condition
5 participants