search

coder / modules

A collection of Terraform Modules to extend Coder templates.
https://registry.coder.com
Apache License 2.0
33 stars 33 forks source link

fix(kasmvnc): optimize KasmVNC deployment script #329

Closed djarbz closed 3 weeks ago

djarbz commented 4 weeks ago

I took the opportunity to majorly refactor the run.sh script.

Basically a rewrite from the ground up, and I found a few things that I don't think were tested for non-Ubuntu distros.

I updated the Kasm Version reference to avoid confusion in the script. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/main.tf#L42-L46

Removed the extra shebang and added an early exit signal in case of failure. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/run.sh#L1-L4

Optimize the download function to set the command and then one block of logic to perform the download and handle a failure. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/run.sh#L16-L37

Created a cross platform function to add a user to a group based on which command is available. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/run.sh#L39-L52

Refactored the install_deb function to dynamically update the (stale) package cache and wait for a lock. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/run.sh#L54-L69

Check and fail early if sudo or /etc/os-release are not available.

Source /etc/os-release since it is formatted as a .env file. Also, adjust the distro and version for Oracle. Note we are using distro_version to make it more readable vs just version. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/run.sh#L100-L108

Cleaned up the ARCH mappings to make it more readable. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/run.sh#L115-L127

For installation, set a base URL that is the same for all distros. Cleaned up the cases and merged ones that could be merged. Reduced checking for specific distro versions, we want this to be compatible with future versions without modifying the script each time. If KASM doesn't support the version, then it will fail during the attempt to download. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/run.sh#L129-L154

Finally, added a check to verify that the VNC server was actually running. https://github.com/coder/modules/blob/a8cc861e831d796914f965ec5c6c5b1b59aa9118/kasmvnc/run.sh#L177-L182

Also closes #327

djarbz commented 4 weeks ago

@matifali I have pushed a few changes and just tested locally. We now only need sudo if we are installing kasm. We are running KasmVNC as the coder user.

Still need to test with non-Ubuntu images, but I don't have any readily available. Will also need to test if this works for @bpmct and his issue with official KasmWeb images.

matifali commented 4 weeks ago

@djarbz I just tested with kasmweb/postman:1.16.0,, and VNC starts, although I do not see a postman icon on the Desktop.

djarbz commented 4 weeks ago

@djarbz I just tested with kasmweb/postman:1.16.0,, and VNC starts, although I do not see a postman icon on the Desktop.

Is it available in the application menu? The $HOME directory might be overridden or somehow masked, or maybe it never existed on the desktop?

djarbz commented 4 weeks ago

@matifali Can you share your template? I just tried with a minimal template and I get stuck on the dashboard loading.

mafredri commented 3 weeks ago

Looks like package-lock was accidentally included in this PR, could you remove it?