Skip to content

fix : execute preStart devfile events after project-clone initContainer #1461

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 1 commit into from
Aug 5, 2025

Conversation

rohanKanojia
Copy link
Member

@rohanKanojia rohanKanojia commented Jul 2, 2025

What does this PR do?

Change the ordering of initContainers to ensure that project-clone initContainer is executed before preStart initContainers

What issues does this PR fix or reference?

#1454

Is it tested? How?

  • Make sure Kubernetes cluster is running
  • Install DWO on cluster based on changes added in this PR. You'd need to create new container image and install operator
export DWO_IMG=docker.io/rohankanojia/devworkspace-controller:next
make docker
make install
  • Apply the following DevWorkspace object, which adds preStart events along with an existing git clone attribute
apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  name: simple-devfile-prestart
spec:
  started: true
  template:
    projects:
      - name: eclipse-jkube-demo-project
        clonePath: eclipse-jkube-demo-project
        git:
          remotes:
            origin: https://github.com/rohanKanojia/eclipse-jkube-demo-project.git
          checkoutFrom:
            revision: main
    components:
      - name: tools
        container:
          image: quay.io/devfile/universal-developer-image:ubi9-latest
          memoryLimit: 256Mi
          mountSources: true
          sourceMapping: /projects
      - name: readme
        container:
          image: quay.io/devfile/universal-developer-image:ubi9-latest
          mountSources: true
          sourceMapping: /projects
          command:
          - "cat"
          - "eclipse-jkube-demo-project/readme.md"
      - name: license
        container:
          image: quay.io/devfile/universal-developer-image:ubi9-latest
          mountSources: true
          sourceMapping: /projects
          command:
          - "cat"
          - "eclipse-jkube-demo-project/license.txt"
    commands:
      - id: project-license
        apply:
          component: license
      - id: project-readme
        apply:
          component: readme
    events:
      preStart:
        - project-license
        - project-readme
  • Check the pod created for workspace, espectially the order of initContainer execution:

Without these changes, it should be like this:

kubectl get deployments -l controller.devfile.io/creator -o json | jq -r '
  .items[] |
  "Deployment: \(.metadata.name)\n" +
  (
    .spec.template.spec.initContainers // [] |
    map("- \(.name)") | join("\n")
  ) + "\n"
'

Deployment: workspace91da279bda6b47fb
- readme
- license
- project-clone

With this PR project-clone initContainer is before preStart containers:

Deployment: workspace91da279bda6b47fb
- project-clone
- readme
- license

You'd notice that with changes in this PR DevWorkspace would come up in Running state. However, on main branch it would be in CrashLoopBackOff state due to failing initContainers

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

openshift-ci bot commented Jul 2, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@rohanKanojia
Copy link
Member Author

/ok-to-test

@rohanKanojia rohanKanojia marked this pull request as ready for review July 2, 2025 14:30
events:
preStart:
- go-mod
- go-build
Copy link
Contributor

Choose a reason for hiding this comment

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

no N&N

…tainer

Change ordering of initContainers to ensure that `project-clone`
initContainer is executed before preStart initContainers

Signed-off-by: Rohan Kumar <[email protected]>
@rohanKanojia
Copy link
Member Author

/ok-to-test

@rohanKanojia
Copy link
Member Author

/retest-required

@rohanKanojia
Copy link
Member Author

/retest

1 similar comment
@dkwon17
Copy link
Collaborator

dkwon17 commented Jul 28, 2025

/retest

Copy link

openshift-ci bot commented Jul 28, 2025

@rohanKanojia: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/v14-che-happy-path 9d9a082 link true /test v14-che-happy-path

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

@rohanKanojia
Copy link
Member Author

rohanKanojia commented Jul 29, 2025

It seems this ci/prow/v14-che-happy-path CI check has been added recently, I didn't see this check in older pull requests. It see a failure in logs:

+ chectl server:deploy -p openshift --batch --telemetry=off --installer=operator --workspace-engine=dev-workspace
Error: Nonexistent flag: --workspace-engine=dev-workspace
See more help with --help
    at validateArgs (/usr/local/lib/chectl/node_modules/@oclif/core/lib/parser/validate.js:10:19)
    at validate (/usr/local/lib/chectl/node_modules/@oclif/core/lib/parser/validate.js:173:5)
    at Object.parse (/usr/local/lib/chectl/node_modules/@oclif/core/lib/parser/index.js:19:35)
    at async Deploy.parse (/usr/local/lib/chectl/node_modules/@oclif/core/lib/command.js:246:25)
{"component":"entrypoint","error":"wrapped process failed: exit status 2","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go:84","func":"sigs.k8s.io/prow/pkg/entrypoint.Options.internalRun","level":"error","msg":"Error executing test process","severity":"error","time":"2025-07-28T21:48:34Z"}
error: failed to execute wrapped command: exit status 2

I see other pull requests failing for this check too #1440

@dkwon17
Copy link
Collaborator

dkwon17 commented Jul 29, 2025

@rohanKanojia good catch, I created a PR: #1474

Copy link

openshift-ci bot commented Aug 5, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akurinnoy, dkwon17, ibuziuk, rohanKanojia

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dkwon17
Copy link
Collaborator

dkwon17 commented Aug 5, 2025

I don't think the ci/prow/v14-che-happy-path check failing is due to this PR, it's due to eclipse-che/che#23514.

IMHO it should be ok to merge this PR without the check passing.

@dkwon17 dkwon17 merged commit 541a38a into devfile:main Aug 5, 2025
10 of 11 checks passed
@rohanKanojia rohanKanojia deleted the pr/issue1454 branch August 6, 2025 05:25
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.

4 participants