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

fix: Ensure $PROJECT_SOURCE and $PROJECT_ROOT are first environment variables in workspace containers #1282

Merged

Conversation

AObuchow
Copy link
Collaborator

What does this PR do?

Environment variables defined in a Kubernetes container can only reference other variables declared earlier in the list of container environment variables. See the second note from the Kubernetes docs on environment variables.

This change ensures that the $PROJECT_SOURCE and $PROJECT_ROOT devfile environment variables are the first environment variables defined in all workspace pod containers so that devfile environment variables can reference $PROJECT_SOURCE and $PROJECT_ROOT.

Instead of appending $PROJECT_SOURCE and $PROJECT_ROOT to all workspace containers, we are now prepending them.

Additionally, existing tests were modified to accommodate this change in the list of container environment variables ordering, and a new test was added for this change.

What issues does this PR fix or reference?

#1274

Is it tested? How?

  1. Install DWO with the changes made from this PR. I've pushed to quay.io/aobuchow/devworkspace-controller:project-source-first for ease of testing.
  2. Apply the following devworkspace:
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: plain-devworkspace-env-test
spec:
  started: true
  routingClass: 'basic'
  template:
    projects:
      - name: web-nodejs-sample
        git:
          remotes:
            origin: "https://github.com/che-samples/web-nodejs-sample.git"
    components:
      - name: web-terminal
        container:
          image: quay.io/wto/web-terminal-tooling:next
          memoryRequest: 256Mi
          memoryLimit: 512Mi
          mountSources: true
          command:
           - "tail"
           - "-f"
           - "/dev/null"
          env:
           - name: ENV_TEST
             value: $(PROJECT_SOURCE)/hey-there
  1. Once the workspace starts up, exec into the workspace pod and run echo $ENV_TEST. The output should be /projects/web-nodejs-sample/hey-there -- in other words, $(PROJECT_SOURCE) will get evaluated to /projects/web-nodejs-sample/.

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

Environment variables defined in a kubernetes container can only
reference other environment variables that are declared earlier in the
list of container environment variables.

This change ensures that the $PROJECT_SOURCE and $PROJECT_ROOT
devfile environment variables are the first environment variables
defined in all workspace pod containers so that devfile environment
variables can reference $PROJECT_SOURCE and $PROJECT_ROOT.

Fix devfile#1274

Signed-off-by: Andrew Obuchowicz <[email protected]>
Copy link

openshift-ci bot commented Jul 3, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AObuchow, dkwon17

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

@AObuchow
Copy link
Collaborator Author

AObuchow commented Jul 3, 2024

For some reason, the GH Actions aren't running. Will try and do a no-op commit to get it to run, then remove the no-op commit.

@AObuchow AObuchow force-pushed the ensure-project-source-first-env-var-backup branch from 89ad3a1 to 26b9f18 Compare July 3, 2024 18:34
@openshift-ci openshift-ci bot removed the lgtm label Jul 3, 2024
Copy link

openshift-ci bot commented Jul 3, 2024

New changes are detected. LGTM label has been removed.

@AObuchow
Copy link
Collaborator Author

AObuchow commented Jul 3, 2024

/retest

3 similar comments
@AObuchow
Copy link
Collaborator Author

AObuchow commented Jul 3, 2024

/retest

@AObuchow
Copy link
Collaborator Author

AObuchow commented Jul 3, 2024

/retest

@AObuchow
Copy link
Collaborator Author

AObuchow commented Jul 3, 2024

/retest

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

Successfully merging this pull request may close these issues.

2 participants