Update devfile/api dependency and populate devworkspace.status.message with info about failures

[amisevsk]

What does this PR do?

1. Update dependency to latest commit in devfile/api
2. Update samples to follow updated devfile/api spec (restrictions on command.id formatting)
3. Update webhooks to specify that they have no side-effects
   • This probably could be a separate PR, but it's a minor change I realized we needed while testing our samples. Note that v1 webhooks support only None and NoneOnDryRun.
4. Propagate the message from any "FailedStart" or "Error" condition to devworkspace.status.message. Error overrides FailedStart if both happen to be present (they should never be)

What issues does this PR fix or reference?

Closes #232

Is it tested? How?

Before testing, it's necessary to run make update_devworkspace_crds to the latest devfile/api CRDs (and make install to deploy them).

Easiest to test on minikube:

$ kubectl apply -f samples/theia-next.yaml
$ kubectl apply -f samples/web-terminal.yaml
# wait a bit...
$ kubectl get dw

NAME           WORKSPACE ID                PHASE     INFO                                                          URL
theia          workspace8af91dfdf1f1417f   Running                                                                 http://workspace8af91dfdf1f1417f-theia-3100.192.168.49.2.nip.io
web-terminal   workspace32d4b43e65e64e64   Failed    Failed to install network objects required for devworkspace   

We should probably move INFO to be the last column

Additional info

Kubernetes docs on webhooks with side effects: https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#side-effects

[sleshchenko]

/test v5-devworkspaces-operator-e2e

[sleshchenko]

I haven't tested but changes LGTM

[JPinkney]

I think ideally INFO would be in the last column but LGTM

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, JPinkney, 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]

I think ideally INFO would be in the last column but LGTM

I agree, but it'll look ugly no matter how we do it -- WDYT about potentially combining URL and INFO? If a workspace starts correctly, it's populated by the URL where you can access it, otherwise it's populated with an info message.

cc: @davidfestal

[JPinkney]

WDYT about potentially combining URL and INFO?

Personally, I like the idea. It would keep the output a lot shorter and easier to manage on a terminal. Aren't showing a URL and INFO technically mutually exclusive anyway? Or is there a case that a URL will show when INFO is set as well?

[davidfestal]

WDYT about potentially combining URL and INFO?

Even now you don't show in the column the same name as in the status field: INFO v Message

So if kubebuilder:printcolumn would allow combining 2 status fields, I'm not against combining both ideURL and message fields.

[amisevsk]

@davidfestal I was thinking of removing ideUrl from printColumns and repurposing message to show the URL to the user; ideUrl would still be around for API uses. That way we still have all the functionality and a cleaner interface for users.

[openshift-ci-robot]

@amisevsk: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[sleshchenko]

@davidfestal I was thinking of removing ideUrl from printColumns and repurposing message to show the URL to the user; ideUrl would still be around for API uses. That way we still have all the functionality and a cleaner interface for users.

I'm OK with the proposed change.
And it should not be an issue if we hide URL for stopped or failed DevWorkspace. It may be even better since this URL gives 503 and user may reconfigure routing before start.

[amisevsk]

Putting this one on hold as I'd like to include devfile/api#335, at which point we should bring in at least devfile/api#324 as well.

[sleshchenko]

Seems it's a time to update the PR or close it )

[amisevsk]

Seems it's a time to update the PR or close it )

I never thought an 80 line change could conflict with so many files :D

I'll create a new PR since we need to update the devfile/api dependency again