Skip to content
This repository has been archived by the owner on Aug 16, 2024. It is now read-only.

Update gradle mounted volume path to match dockerfile #14

Merged
merged 1 commit into from
Jun 20, 2019
Merged

Update gradle mounted volume path to match dockerfile #14

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions devfiles/java-gradle/devfile.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ components:
args: ['infinity']
env:
- name: GRADLE_USER_HOME
value: /home/user/.gradle
value: /home/gradle/.gradle
Copy link
Member

Choose a reason for hiding this comment

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

just a question: do we need two homes? or

      - name: HOME
        value: /home/user

Should be changed to

      - name: HOME
        value: /home/gradle

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point; it's probably better for consistency. The maybe confusing thing about this is that because of arbitrary UID on openshift, the terminal doesn't run as the gradle user, but this still probably makes more sense than /home/user.

In either case, since HOME is not mounted as a volume, any changes there would be wiped out. I wonder if it makes more sense to set HOME to /projects so that at least cd ~ takes users to a meaningful directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

I set to HOME to /home/user originally because that's how legacy Che workspaces worked and there were still some environment variables injected by wsmaster that assumed that. I don't know if that's still the case.

/home/gradle/.gradle is persisted as a volume right? data should not be wiped out.

I am afraid that setting /projects/ as HOME may have unexpected side effects. We may want to set the k8s container workingDir instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@l0rd That makes sense to me but I'm still not sure the best way to proceed here. While /home/gradle/.gradle is persisted, /home/gradle is not; are we okay with this (e.g. it would prevent using a persistent .bashrc, etc.). This issue is probably another, larger discussion, though.

I've updated $HOME to be /home/gradle. What is currently happening, AFAICT, is that whatever directory we're calling $HOME is being created at container start up, so I don't think this will cause issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are ok not to persist full $HOME. This is a cool idea (a user would find his dotfiles in every workspace) but I don't think there has ever been a requirement for that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyway I am +1 to merge this PR as it is

- name: JAVA_OPTS
value: "-XX:MaxRAMPercentage=50 -XX:+UseParallelGC -XX:MinHeapFreeRatio=10
-XX:MaxHeapFreeRatio=20 -XX:GCTimeRatio=4 -XX:AdaptiveSizePolicyWeight=90
Expand All @@ -32,14 +32,14 @@ components:
- name: PS1
value: "$(echo ${0})\\$ "
- name: HOME
value: /home/user
value: /home/gradle
memoryLimit: 512Mi
endpoints:
- name: '8080/tcp'
port: 8080
volumes:
- name: gradle
containerPath: /home/user/.gradle
containerPath: /home/gradle/.gradle
mountSources: true
commands:
-
Expand Down