Skip to content

Allow the coder agent to tell modules where they can place data, binaries, etc #11131

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

Closed
mafredri opened this issue Dec 11, 2023 · 8 comments · Fixed by #12205
Closed

Allow the coder agent to tell modules where they can place data, binaries, etc #11131

mafredri opened this issue Dec 11, 2023 · 8 comments · Fixed by #12205
Assignees

Comments

@mafredri
Copy link
Member

mafredri commented Dec 11, 2023

This is an alternate path to achieving PATH manipulations as discussed in: #10166

The premise is, let the agent decide where a module can store its files (binaries, data, settings, etc).

Let's say the agent discerns that $HOME is a good place to put files, it can create the appropriate folders/permissions and run module script(s) with the following env variables:

CODER_MODULE_DATA_PATH=~/.local/share/coder_module/${MODULE_ID}
CODER_MODULE_BIN_PATH=~/.local/share/coder_module/${MODULE_ID}/bin

Or let's say $HOME isn't writable, we can usually default to the agent directory often located in /tmp (granted, this is ephemeral storage, so a warning may be prudent).

CODER_MODULE_DATA_PATH=/tmp/coder.xxcxcv/module/${MODULE_ID}
CODER_MODULE_BIN_PATH=/tmp/coder.xxcxcv/module/${MODULE_ID}/bin

As long as we can create an association between coder module (name/id) and whatever coder_script it has introduced, we should be able to do this fairly easily in the agent.

We can either use a per-module /bin or a combine bin dir, e.g.

CODER_MODULE_BIN_PATH=~/.local/share/coder_module/bin

Lastly, the agent would make sure to prepend (or append) the module bin dir(s) to PATH for use in scripts, SSH sessions, etc.

@cdr-bot cdr-bot bot added the feature label Dec 11, 2023
@bpmct
Copy link
Member

bpmct commented Dec 12, 2023

We should avoid using $HOME for the bin but @kylecarbs, @stirby, and I are onboard!

@matifali
Copy link
Member

@bpmct why not using $HOME? We assume that most of the workspaces will have a persistent $HOME, and I think it is a good idea to store all binaries installed by modules in CODER_MODULE_BIN_PATH=~/.local/share/coder_module/bin. This will allow us to make modules intelligent by checking if the binary is already available.

@mafredri
Copy link
Member Author

@bpmct why not using $HOME? We assume that most of the workspaces will have a persistent $HOME, and I think it is a good idea to store all binaries installed by modules in CODER_MODULE_BIN_PATH=~/.local/share/coder_module/bin. This will allow us to make modules intelligent by checking if the binary is already available.

Agreed, storing data in persistent storage can help speed up workspace startup times and reduce flakes, e.g. waiting on curl https://some-awesome-tool, or network hiccups, etc.

I'm not a fan of too many options, but we could have both and a module can make its decision. Like CODER_MODULE_EPHEMERAL_* and CODER_MODULE_PERSISTENT_*.

@bpmct
Copy link
Member

bpmct commented Feb 5, 2024

My concern about $HOME is that a new version of a module may have another version of the script/binary and then it becomes up to the module to look at the checksum, compare, and copy the new one. Or a symlink, but then the binary/script is being re-downloaded

Plus, modules already have to download stuff every time into a non-home dir (e.g. code-server into bin). I agree I'd be good to have some form of caching for all of these utils but I worry about the trade-off "configuration/script drift"

Don't consider this blocking, just one thing to consider. I would be very happy to have a module bin and would happily write a few extra lines to compare a script in $HOME over a janky way to override PATH

@mafredri mafredri self-assigned this Feb 5, 2024
mafredri added a commit that referenced this issue Feb 19, 2024
The agent is extended with a `--script-data-dir` flag, defaulting to the
OS temp dir. This dir is used for storing `coder-script/bin` and
`coder-script/[script uuid]`. The former is a place for all scripts to
place executable binaries that will be available by other scripts, SSH
sessions, etc. The latter is a place for the script to store files.

Since we default to OS temp dir, files are ephemeral by default. In the
future, we may consider adding new env vars or changing the default
storage location. Workspace startup speed could potentially benefit from
scripts being able to skip steps that require downloading software. We
may also extend this with more env variables (e.g. persistent storage in
HOME).

Fixes #11131
mafredri added a commit that referenced this issue Feb 19, 2024
The agent is extended with a `--script-data-dir` flag, defaulting to the
OS temp dir. This dir is used for storing `coder-script/bin` and
`coder-script/[script uuid]`. The former is a place for all scripts to
place executable binaries that will be available by other scripts, SSH
sessions, etc. The latter is a place for the script to store files.

Since we default to OS temp dir, files are ephemeral by default. In the
future, we may consider adding new env vars or changing the default
storage location. Workspace startup speed could potentially benefit from
scripts being able to skip steps that require downloading software. We
may also extend this with more env variables (e.g. persistent storage in
HOME).

Fixes #11131
mafredri added a commit that referenced this issue Feb 19, 2024
The agent is extended with a `--script-data-dir` flag, defaulting to the
OS temp dir. This dir is used for storing `coder-script/bin` and
`coder-script/[script uuid]`. The former is a place for all scripts to
place executable binaries that will be available by other scripts, SSH
sessions, etc. The latter is a place for the script to store files.

Since we default to OS temp dir, files are ephemeral by default. In the
future, we may consider adding new env vars or changing the default
storage location. Workspace startup speed could potentially benefit from
scripts being able to skip steps that require downloading software. We
may also extend this with more env variables (e.g. persistent storage in
HOME).

Fixes #11131
mafredri added a commit that referenced this issue Feb 19, 2024
The agent is extended with a `--script-data-dir` flag, defaulting to the
OS temp dir. This dir is used for storing `coder-script/bin` and
`coder-script/[script uuid]`. The former is a place for all scripts to
place executable binaries that will be available by other scripts, SSH
sessions, etc. The latter is a place for the script to store files.

Since we default to OS temp dir, files are ephemeral by default. In the
future, we may consider adding new env vars or changing the default
storage location. Workspace startup speed could potentially benefit from
scripts being able to skip steps that require downloading software. We
may also extend this with more env variables (e.g. persistent storage in
HOME).

Fixes #11131
mafredri added a commit that referenced this issue Feb 20, 2024
The agent is extended with a `--script-data-dir` flag, defaulting to the
OS temp dir. This dir is used for storing `coder-script-data/bin` and
`coder-script/[script uuid]`. The former is a place for all scripts to
place executable binaries that will be available by other scripts, SSH
sessions, etc. The latter is a place for the script to store files.

Since we default to OS temp dir, files are ephemeral by default. In the
future, we may consider adding new env vars or changing the default
storage location. Workspace startup speed could potentially benefit from
scripts being able to skip steps that require downloading software. We
may also extend this with more env variables (e.g. persistent storage in
HOME).

Fixes #11131
@mafredri
Copy link
Member Author

mafredri commented Feb 20, 2024

@bpmct @matifali

For v1, I only implemented two env variables, $CODER_SCRIPT_DATA_DIR (/tmp/coder-script-data/8d5e8afb-86ec-4a01-ad07-8f53613782ac) and $CODER_SCRIPT_BIN_DIR (/tmp/coder-script-data/bin).

For now, these are ephemeral via /tmp, but it would be safest to consider that they may not be empty when writing modules.

I also decided that all modules share one bin path because otherwise we have 1-2 problems:

  1. Module priorities, one /bin dir will always have higher priority than the other, and if two modules install a command with the same name, that's a problem
  2. The $PATH variable grows unwieldy with, say, 10 modules or more

(The /tmp directory can be changed by setting coder agent --script-data-dir /some/other/path.)


Here's an example of a compatibility layer we can use in our scripts:

#!/bin/sh

BIN_DIR=/usr/local/bin
if [ -n "${CODER_SCRIPT_BIN_DIR}" ]; then
    BIN_DIR="${CODER_SCRIPT_BIN_DIR}"
fi

install_bin() {
	name=$(basename "${1}")
    echo "Installing ${name} to ${BIN_DIR}..."
    cp -f "${1}" "${BIN_DIR}/" || sudo cp -f "${1}" "${BIN_DIR}/"
    chmod +x "${BIN_DIR}/${name}"
}

install_bin vault-v1.0.0/bin/vault

@matifali
Copy link
Member

The next step would be refactor modules to make use of this new capabilities.
It's certainly a step for a better UX.

@bpmct
Copy link
Member

bpmct commented Apr 11, 2024

@mafredri Is there a way to programatically get the bin dir? For example, on Windows it may be different.

@bpmct
Copy link
Member

bpmct commented Apr 11, 2024

I guess all scripts would break on windows unless they are windows-specific

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

Successfully merging a pull request may close this issue.

3 participants