Move tools & configs from /home/user/ to /home/tooling/ #115
Conversation
|
Something to note about using stow: the stow command in the entrypoint adds a bit of delay to the startup of the UDI, which is unfortunate. I haven't yet measured the exact amount of time it takes to startup on my local machine. |
|
We should apply the same mechanism to the base image too so that also the tools installed in that Dockerfile assume that It's surprising that
What if we run it in background so that it doesn't block the execution? The side effect may be that commands that follow will not find conf files in |
We could either:
Something to note about changing the base image to install to /home/tooling/: the
# User specific environment
if ! [[ "$PATH" =~ "$HOME/.local/bin:$HOME/bin:" ]]
then
PATH="$HOME/.local/bin:$HOME/bin:$PATH"
fi
export PATH
...
export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
...
export PATH="${KREW_ROOT:-$HOME/.krew}/bin:$PATH"I'm not exactly sure yet why
This is a good point - there is no guarantee that the entrypoint is run before postStart commands (see postStart documentation here). This is very problematic IMO. We could change the stow step to be a default devworkspace postStart command, but that also comes with its own complexity such as verifying we only add the stow postStart command when the UDI is used (and the UDI would not be fully usable outside of Che). |
AObuchow
force-pushed
the
tooling_dir_new
branch
from
73c65bc
to
af40e3d
Compare
|
My latest round of changes involve running the stow command in the actual Dockerfile itself (to prevent unnecessary extra startup time when |
@l0rd after some consideration, I believe it's best I modify any additions to the
|
|
@AObuchow @l0rd I tested this PR on DevSpaces v3.8, while it works as advertised, i also ran an issue as mentioned above wherein not everything from /home/tooling would be end up /home/user running the following by manually after the devspace instance startup copies the missing items cd ${DEVWORKSPACE_NAME}
stow . -t /home/user/ -d /home/tooling/ --no-folding --adopt |
|
@spuranam Thank you for the testing & feedback Satish. Would you be able to provide a specific example of a file(s) that isn't being linked from /home/tooling to /home/user? This would help me figure out edge-cases that I am currently missing. Also, are you trying to reference/use these missing files in a postStart command (and potentially running into a race-condition with the entrypoint)? Lastly, is there a specific reason why you're using the |
|
@AObuchow The issue i am running into is that some times all the file /home/tooling would get linked sometime only few files would get linked to /home/user, for example i have seen many time /home/tooling/.sdkman and /home/tooling/.nvm will not get linked. My current workaround is to manually invoke I emailed all details needed for you to reproduce my results earlier today @l0rd , i am hoping it would be forward to you. |
|
@spuranam Thank you for the information, @l0rd forwarded me the necessary details, and I was able to reproduce (what I suspect) was the issue. Below are 3 observations I made:
$ cat /home/user/.bashrc
...
export NVM_DIR="/home/tooling/.nvm"
[ -s "${NVM_DIR}/nvm.sh" ] && source "${NVM_DIR}/nvm.sh"
export SDKMAN_DIR="/home/tooling/.sdkman"
[[ -s "${SDKMAN_DIR}/bin/sdkman-init.sh" ]] && source "${SDKMAN_DIR}/bin/sdkman-init.sh"sdkman's init script is not able to find the symbolic links in
A workaround is to add an additional postStart command that sources the events:
postStart:
- load-environment
- my-sdkman-command
- my-nvm-command
commands:
- id: load-environment
exec:
component: tools
commandLine: source /home/tooling/.bashrc
- id: my-sdkman-command
exec:
component: tools
commandLine: sdkman --help
- id: my-nvm-command
exec:
component: tools
commandLine: nvm --helpIn a devfile, the order of postStart commands in the devfile is preserved when executing the commands, so all commands that require PATH additions from the Additionally, the
In other words, the entrypoint should be: # /home/user/ will be mounted to by a PVC if persistUserHome is enabled
if mountpoint -q /home/user/; then
# Create symbolic links from /home/tooling/ -> /home/user/
- stow . -t /home/user/ -d /home/tooling/ --no-folding &
+ stow . -t /home/user/ -d /home/tooling/ --no-folding
fiI would be happy to submit the required changes as a PR to your reproducer repository if desired, so that you can easily check if my suggestions work for your needs. |
|
I realized it's actually problematic to have the I'll have to update this PR as soon as I can with changes that address all the issues I've found so far. |
AObuchow
force-pushed
the
tooling_dir_new
branch
from
af40e3d
to
ca65aa2
Compare
AObuchow
force-pushed
the
tooling_dir_new
branch
from
ca65aa2
to
c13de5f
Compare
|
I have updated the PR with my latest changes and rebased it onto the main branch:
There still are a few things that I need to address before this PR will no longer be in a draft state:
I've pushed to |
|
@AObuchow Thanks to you i can now confirm the issues i reported earlier have been resolved, with an exception of podman wrapper that you noted above |
AObuchow
force-pushed
the
tooling_dir_new
branch
2 times, most recently
from
4ce203b
to
b615cc0
Compare
|
@spuranam Glad to hear my changes resolved those issues. I am currently working on fixing the issue with the podman wrapper. @l0rd I've been trying to test my latest changes (pushed to I've made the following devfile to test out podman run with my latest changes. I also mounted a |
AObuchow
force-pushed
the
tooling_dir_new
branch
from
b615cc0
to
1cfed35
Compare
|
I fixed the issue I was having with the podman wrapper: I was able to do a @l0rd if you get a chance, could you please verify the podman wrapper is working as expected? My UDI image with the latest changes is at |
|
@AObuchow I have tested your image and the wrapper is working as expected. 👍 |
|
@l0rd I added two commits to address points 1. and 3. from my earlier comment. Point 4. wasn't necessary since we're already cleaning up the dnf cache after installing stow in the base image. The newest changes have been pushed to Something that came up while ensuring the I noticed in the base image's entrypoint, we're creating a However, this is also present in the upstream UDI, so maybe this code is still necessary? If it's not necessary, I could remove it as part of this PR or in a future PR. |
You probably mean downstream right? In any case you are right. In the entrypoints (both upstream and downstream) we should assume |
AObuchow
force-pushed
the
tooling_dir_new
branch
from
4b93564
to
7bab82c
Compare
Yes I meant the downstream UDI, my mistake. Sounds good, I added an extra commit to remove the |
AObuchow
force-pushed
the
tooling_dir_new
branch
from
ef9bb57
to
4d921eb
Compare
|
@l0rd I've squashed all the fixup commits from this PR for when we are ready to merge it Edit: I forgot to rebase this PR now that #132 has been merged. Should be done before this PR is merged. |
Part of eclipse-che/che#22411 Signed-off-by: Andrew Obuchowicz <[email protected]>
All bash-completions located in /usr/share/bash-completion/completions/ are now enabled by default. oc and git completions were already being stored in /usr/share/bash-completion/completions/ which is why they are no longer being explicitly sourced in the .bashrc. Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Fix eclipse-che/che#22412 When persistUserHome is enabled in the Che Cluster CR, the PVC will be mounted to /home/user/, overwriting all tools and configuration present in the UDI /home/user/ directory. To prevent this overwriting, all tools and configurations should be located in /home/tooling/. To ensure existing workflows still function correctly, symbolic links should be created to point from /home/tooling/ -> /home/user/. GNU stow is used to manage these symbolic links. with the --no-folding option enabled, to recreate the directory tree, ensuring all configuration directories exist in /home/user/ and can be written to. Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
The .viminfo cannot be a symbolic link for security reasons. Thus it is ignored by stow and manually copied from /home/tooling/ to /home/user/ instead. Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
Signed-off-by: Andrew Obuchowicz <[email protected]>
AObuchow
force-pushed
the
tooling_dir_new
branch
from
4d921eb
to
18b26a7
Compare
|
I rebased my changes and made the appropriate modification for the podman docker alias. This PR should be good to merge finally. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: AObuchow, l0rd The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This PR aims to resolve eclipse-che/che#22412, where enabling
devEnvironments.persistUserHomein the Che Cluster CR will cause the workspace's PVC to mount to/home/user/, overwriting any tools, configurations, etc. that are present in the UDI's/home/user/directory.In order to resolve this issue, we are now installing tools (and most config files) to
/home/tooling/and using GNU stow to create symbolic links to/home/user/for all files in/home/tooling/(with the exception of.viminfo, which cannot be a symbolic link by vim's design).Rather than trying to keep
$PATHto be exactly the same as prior to this change (i.e. tooling binaries are found in/home/user/) ,$PATHhas been modified to contain the appropriate directories in/home/tooling/. The reasoning for this change is due to the fact that the call to stow in the entrypoint is not guaranteed to run before any postStart commands. In order to ensure postStart commands can invoke all UDI binaries, they must be placed under/home/tooling/, which guarantees they will not be overwritten when a PVC is mounted to /home/user/.Below are some other details regarding the changes made in this PR:
--no-foldingoption to recreate the directory tree structure from/home/tooling/in/home/user/, to ensure any changes within a configuration folder will persist on the PVC (as directories in/home/tooling/are essentially ephemeral).--no-foldingoption is not used due to the lack of directories in the /home/tooling/ of the base image.--no-foldingenabled./home/user/has been mounted by a PVC (erasing the contents of/home/user/).viminfo, as it must be a regular file by vim's design. Consequently, the.viminfomust be manually copied from/home/tooling/->/home/user/./home/user/.local/bin/still on$PATHso that users could mount their own binaries to/home/user/.local/bin/and invoke them as VSCode tasks or postStart commands.$SDKMAN_DIRand$NVM_DIRare hard-coded to point to/home/tooling/.sdkman(rather than$HOME/.sdkman). sdkman will not function if$SDKMAN_DIRis set to/home/user/.sdkman/, as its init script won't detect symlinks.~/.bashrcto extend the bash environment. Instead, we now have 2 bash scripts located in/etc/profile.d/: one which modifies the bash environment, another to set the terminal prompt. These scripts are automatically loaded by bash. Now, users are free to modify the UDI's .bashrc (e.g. overwritting it with an automount configmap in Che) without breaking any modifications to$PATHthat we provide in the UDI (such as sdkman).~/.bashrc, which was obsolete.$KUBECONFIGrather than being hard-coded to/home/user/.kube/config.$KUBEDOCK_TIMEOUTThis PR is also based off of #110, so reading the changes introduced in that PR is also worthwhile.
Testing
I've pushed
quay.io/aobuchow/universal-developer-image:stowto test the UDI as a docker container. I've also created a sample devfile that uses the updated UDI, to test in Che withdevEnvironments.persistUserHome.enabled: true.Sample devfile notes
My sample devfile invokes some commands to ensure they are available on $PATH to run as preStart commands. Note: some of these commands generate stderr output, which can be viewed in
/tmp/poststart-stderr.txt.Additionally, I attempted to log the binaries available as postStart commands with
compgen -c | xargs which --all --skip-functions | sort | uniq > /projects/udi-tooling/binaries.txt ;Here's the output of that command.One final note about my devfile: since postStart commands in DWO are run as
/bin/sh -C { <commands> }any modifications to$PATHpresent in the bash environment are normally not usable (e.g. nvm cannot be invoked). To workaround this, I added a postStart command that runs source/home/user/.bashrcin the parent devfile. This workaround could potentially be used in CheCode when running VSCode tasks (relevant comment).Other testing notes
Properly ensuring this PR does not break any existing functionality or workflows may be tricky due to the nature of moving everything out of
/home/user/into/home/tooling/(and then "back" into/home/user/with stow).Since the podman wrapper was affected by this change, it's also worth ensuring the test cases (wrapped podman commands) from #107 work as well.
Below are some comparisons of the
/home/user/directory, the/home/user/.bashrcand$PATH, before and after this change.ls -lain/home/user/directoryBefore:
After:
/home/user/directory treeDirectory tree created using https://github.com/a8m/tree/tree/master: installed by running
go install github.com/a8m/tree/cmd/tree@latestthengo clean -cacheBefore:
After:
.bashrc
Before:
After:
Also, here are the 2 new files in
/etc/profile.d/:$PATH
Before:
After: