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

Add support for postStart events in DevWorkspaces #659

Merged
merged 5 commits into from
Nov 2, 2021

Conversation

amisevsk
Copy link
Collaborator

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:

  • There's no guarantee the container's entrypoint has run
  • The command is re-run on every container start (i.e. if a container crashes the hook executes again)
  • If the hook returns a non-zero exit code, container startup fails

This PR has some additional limitations:

  • Only exec-type commands are supported (perhaps we could also support composite commands if they're all exec...)
  • The workingDir field is supported via cd <workingDir> but other fields (e.g. env) are currently not
  • Only one postStart event can be tied to each component, we don't join commands

The limitations above can be improved later if necessary.

What issues does this PR fix or reference?

Closes #629
Closes #655

Is it tested? How?

  1. Create a DevWorkspace that specifies a postStart event:
    cat <<EOF | kubectl apply -f -
    kind: DevWorkspace
    apiVersion: workspace.devfile.io/v1alpha2
    metadata:
      name: test-poststart
    spec:
      started: true
      template:
        projects:
          - name: web-nodejs-sample
            git:
              remotes:
                origin: "https://github.com/che-samples/web-nodejs-sample.git"
        components:
          - name: theia
            plugin:
              uri: https://che-plugin-registry-main.surge.sh/v3/plugins/eclipse/che-theia/next/devfile.yaml
              components:
                - name: theia-ide
                  container:
                    env:
                      - name: THEIA_HOST
                        value: 0.0.0.0
        commands:
          - id: say-hello
            exec:
              component: theia-ide
              commandLine: echo "Hello from postStart!" > postStart-event.log
              workingDir: ${PROJECTS_ROOT}
        events:
          postStart:
            - say-hello
    EOF
  2. Wait for workspace to enter running state
  3. Check that postStart-event.log file is created:
    WKSP_ID=$(kubectl get dw test-poststart -o json | jq -r '.status.devworkspaceId')
    kubectl exec deploy/${WKSP_ID} -c theia-ide -- cat '/projects/postStart-event.log'
    should output Hello from postStart!

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

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.

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)

@openshift-ci
Copy link

openshift-ci bot commented Oct 28, 2021

[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:
  • OWNERS [amisevsk,sleshchenko]

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

@benoitf
Copy link
Collaborator

benoitf commented Oct 28, 2021

FYI I tried with this DevWorkspace: https://gist.github.com/benoitf/541ffeac548fd33d91f91fbe2232de2d

$ kubectl apply -f https://gist.githubusercontent.com/benoitf/541ffeac548fd33d91f91fbe2232de2d/raw/f8f006a4f8617aedfeb9e6a94e94901ce40472ef/devworkspace-post-start.yaml

when I do oc get dw I've the following output:

NAME               DEVWORKSPACE ID             PHASE      INFO
my-dummy-project   workspace10131a939b7f4b05   Starting   Waiting for editor to start

but trying the route that can be grabbed using

$ oc get dwr -o yaml | grep 'url:'
        url: https://workspace10131a939b7f4b05.apps.cluster-4ad3.4ad3.sandbox1894.opentlc.com/che-code/

I'm able to connect to the IDE

@benoitf
Copy link
Collaborator

benoitf commented Oct 28, 2021

My DevWorkspace has one init-container which is used as a preStart command: it copy IDE content to a volume
Then postStart event is used to start the IDE from the mounted volume

The ide is running and I'm able to connect on the DevWorkspaceRoute but I still have the state Waiting for editor to start

@amisevsk
Copy link
Collaborator Author

The issue is that the editor is returning 400 Bad Request for the /healthz endpoint, breaking the health check that marks a workspace as running. I can add it to ignored reasons (our health checks are generally pretty fragile currently), but that failure does appear to be unrelated to this PR.

@amisevsk
Copy link
Collaborator Author

Waiting on editor to start error should be fixed by #662

Side note on usability: the editor's URL can always be grabbed via oc get dw -o json | jq '.status.mainUrl', even if the workspace is not running.

@openshift-ci
Copy link

openshift-ci bot commented Oct 28, 2021

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm label Oct 28, 2021
@sleshchenko
Copy link
Member

/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>
@amisevsk
Copy link
Collaborator Author

Rebased to include #662, which should fix @benoitf's use case.

@amisevsk
Copy link
Collaborator Author

/test v8-devworkspace-operator-e2e, v8-che-happy-path

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