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

Suggestion: Support runtime-config in the spin runtime #2094

Open
Mossaka opened this issue Nov 14, 2023 · 8 comments
Open

Suggestion: Support runtime-config in the spin runtime #2094

Mossaka opened this issue Nov 14, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@Mossaka
Copy link
Contributor

Mossaka commented Nov 14, 2023

We may want to support detecting/finding the runtime config in Spin runtime instead of passing it as a flag to the Spin CLI

Recently, there was a PR to add runtime-config.toml file support to the spin shim (thanks to @kate-goldenring !) and I noticed a comment mentioning the need to support detecting runtime config file in spin runtime instead of the CLI. So I just want to open this issue for discussion here.

I am trying out adding runtime-config.toml file as a ConfigMap object in k8s and mount it as a read-only file volumn to the spin containers. Without the logic to detect this file, the mounting path has to be a fixed path ("/" for now). Hence there are some needs to make this happen at the spin runtime level.

Would it be reasonable to add a new field to spin.toml manifest file to point to where the runtime config file is expected in the working directory?

More generally, I wish all the spin up flags could be defined either in the manifest file, or as env vars that can be picked up by spin runtime.

@itowlson
Copy link
Contributor

I'm not sure what "the logic to detect this file" refers to. As you know, you can pass the path of the runtime config file to the trigger object. At the moment, https://github.com/deislabs/containerd-wasm-shims/blob/b2d695c739e2503deb4401ee86825cf2c77653e5/containerd-shim-spin/src/engine.rs#L226 hardwires that to a fixed location; but I don't know enough to understand why you couldn't pass a variable with the location of the ConfigMap volume.

I'm reasonably sure probing for runtime config files doesn't belong in the Spin runtime, because runtime configuration is entirely host dependent. Some hosts use TOML files; some hosts don't. Some hosts offer SQLite, Redis and Cosmos as K/V backing stores; some don't. Admittedly the current trigger abstraction makes it gnarly to use anything other than the TOML file and the CLI implementations. But I reckon the right strategy is to improve that abstraction rather than to bake the TOML file and CLI implementations even deeper into the runtime. Thoughts?

@Mossaka
Copy link
Contributor Author

Mossaka commented Nov 14, 2023

As you know, you can pass the path of the runtime config file to the trigger object

From the shim perspective, it doesn't know what path contains spin's runtime config file, it doesn't know if it exists. It only knows the arguments specified in the container image and the environment variables. So what we did is, as you've mentioned, hardwiring that to a fixed location (e.g. "/runtime-config.toml").

@kate-goldenring
Copy link
Contributor

When adding runtime config to the shim, there we some discussions where people assumed Spin was looking for the runtime config in the root directory, similar to how we resolve a default spin manifest path. It doesn't seem like a leap to want to resolve a default runtime config path.

However, my main consideration with this was that I was assuming that we will eventually want to start adding the runtime config to the Spin OCI artifact which means we would need to decide where in the OCI file structure the file should live. Once we have decided that, the shim should re-evaluate the default path to match that.

That being said, it sounds like it is not a priority and that using a fixed location is a fine practice for now.

@itowlson
Copy link
Contributor

@kate-goldenring Maybe we need to have a chat (and/or put together a document) to align our understandings. To me, the runtime config cannot be in the OCI artifact, because OCI artifact should be able to run in different execution environments, and the runtime config is specific to the user and where they are running the application. (E.g. my Vault instance is not available in your AKS account.) Indeed, to me, runtime configuration should be the business of the host (e.g. in k8s get variables from Secrets rather than from Vault), although we don't make that very practical in the current trigger API. But it may be that my vision for the division of responsibilities is out of whack with everybody else's!

@kate-goldenring
Copy link
Contributor

@itowlson good point on the portability of the OCI artifact. I was caught up on how we would transport the runtime config and landed in a way the removed it's ability to be dynamic.

To summarize, @Mossaka I think we are doing things right in the shim. That host has it's chosen location and format for the config. I think we can close this.

@jsturtevant
Copy link
Contributor

I would agree the runtime config shouldn't live as a an OCI layer, since it sounds more dynamic. From the shim perspective, I think we can handle the configuration via env variable and configmap.

Maybe there can be a helper function in spin to load it from either a given path or well known env to make it consistent across environments? Not sure if that makes sense after the comments in #2094 (comment)

@vdice vdice added the enhancement New feature or request label Mar 13, 2024
@vdice
Copy link
Member

vdice commented Mar 13, 2024

@Mossaka @kate-goldenring @jsturtevant what's the status on this one with the latest version of the shim? Are there still some enhancements that we need to track in Spin?

@jsturtevant
Copy link
Contributor

Maybe this should be an issue in the shim? Short term we could use config maps to bring in the config. I don't think that would require any changes to spin.

Longer term this might be a good fit for https://github.com/WebAssembly/wasi-runtime-config?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: 📋 Investigating / Open for Comment
Development

No branches or pull requests

5 participants