-
Notifications
You must be signed in to change notification settings - Fork 57
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
Add support for postStart events in DevWorkspaces #659
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM but it does not seem to work ideally for Che case, or for devfile case in general, but seems to may work in happy path )
Failure and edge cases seems to become more complex to investigate (instead of checking logs, you would need to build your process in a way it reports logs onto filesystem, and you should design your postStart events to start in not-blocking manner. In case it crashed - no one will take care of it and you would need to do exec and analyze processes and their logs on file system)
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amisevsk, 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:
Approvers can indicate their approval by writing |
FYI I tried with this DevWorkspace: https://gist.github.com/benoitf/541ffeac548fd33d91f91fbe2232de2d
when I do
but trying the route that can be grabbed using
I'm able to connect to the IDE |
My DevWorkspace has one init-container which is used as a preStart command: it copy IDE content to a volume The ide is running and I'm able to connect on the DevWorkspaceRoute but I still have the state |
The issue is that the editor is returning |
Waiting on editor to start error should be fixed by #662 Side note on usability: the editor's URL can always be grabbed via |
New changes are detected. LGTM label has been removed. |
/test v8-devworkspace-operator-e2e, v8-che-happy-path |
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Add support for devfile postStart events by converting the referenced command into a k8s postStart lifecycle hook. For now, only a subset of the fields in a devfile command are supported, and otherwise an error is returned. Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
Signed-off-by: Angel Misevski <amisevsk@redhat.com>
With the addition of lifecycle hooks to support postStart events, it's necessary to consider 'FailedPostStartHook' as a fatal event in workspace startup in order to show more info on workspace failures. Without this, a workspace will eventually fail with CrashLoopBackoff instead. Signed-off-by: Angel Misevski <amisevsk@redhat.com>
6ee719f
to
c50e980
Compare
/test v8-devworkspace-operator-e2e, v8-che-happy-path |
What does this PR do?
Adds basic support for postStart events in DevWorkspaces by utilizing Kubernetes pod lifecycle hooks. Hooks allow for running commands after a container starts, with the caveats:
This PR has some additional limitations:
workingDir
field is supported viacd <workingDir>
but other fields (e.g.env
) are currently notThe limitations above can be improved later if necessary.
What issues does this PR fix or reference?
Closes #629
Closes #655
Is it tested? How?
postStart-event.log
file is created:Hello from postStart!
PR Checklist
/test v8-devworkspace-operator-e2e, v8-che-happy-path
to trigger)v8-devworkspace-operator-e2e
: DevWorkspace e2e testv8-che-happy-path
: Happy path for verification integration with Che