Kubernetes-based Runtime Config #122
Conversation
endocrimes
force-pushed
the
dani/runtimeconfig
branch
from
9c2fa74 to
1e12f1e
Compare
endocrimes
changed the title
endocrimes
force-pushed
the
dani/runtimeconfig
branch
4 times, most recently
from
da6ac51 to
b5c4686
Compare
Signed-off-by: Danielle Lancashire <[email protected]>
Signed-off-by: Danielle Lancashire <[email protected]>
This code is... not pleasant. Runtime Config translation from kubernetes-native api through to spin-native api is rather complex when accounting for the number of places secrets and config maps can be referenced. This commit attempts to at least shove all of the nastier bits into a single package that can be _mostly_ ignored. Signed-off-by: Danielle Lancashire <[email protected]>
…ntal exposure harder Signed-off-by: Danielle Lancashire <[email protected]>
Signed-off-by: Danielle Lancashire <[email protected]>
Signed-off-by: Danielle Lancashire <[email protected]>
Signed-off-by: Danielle Lancashire <[email protected]>
Signed-off-by: Danielle Lancashire <[email protected]>
endocrimes
force-pushed
the
dani/runtimeconfig
branch
from
b5c4686 to
ea2a8ac
Compare
|
|
||
| type SqliteDatabaseConfig struct { | ||
| Name string `json:"name"` | ||
| Type string `json:"type"` |
There was a problem hiding this comment.
what are valid values for type here? should we validate input? should we omitempty and default to on-disk?
There was a problem hiding this comment.
The operator technically doesn't know - because we have no way to discover them from spin. I'd like to avoid defaulting mostly because of that.
|
|
||
| type KeyValueStoreConfig struct { | ||
| Name string `json:"name"` | ||
| Type string `json:"type"` |
There was a problem hiding this comment.
same question as above for type
| // e.g on-disk or turso. | ||
| SqliteDatabases []SqliteDatabaseConfig `json:"sqliteDatabases,omitempty"` | ||
|
|
||
| KeyValueStores []KeyValueStoreConfig `json:"keyValueStores,omitempty"` |
There was a problem hiding this comment.
This could use a doc comment
There was a problem hiding this comment.
@endocrimes please followup here in a subsequent PR
|
|
||
| KeyValueStores []KeyValueStoreConfig `json:"keyValueStores,omitempty"` | ||
|
|
||
| LLMCompute *LLMComputeConfig `json:"llmCompute,omitempty"` |
There was a problem hiding this comment.
This could use a doc comment
There was a problem hiding this comment.
@endocrimes please followup here in a subsequent PR
| key: "turso-token" | ||
|
|
||
| llmCompute: | ||
| type: "remote_http" |
There was a problem hiding this comment.
Will all this runtime config just work out of the box if someone applied this sample? If not I think we should add some comments to explain how someone could apply this sample and use it. I also think we should still keep around a sample that shows loading the runtime config from a secret.
There was a problem hiding this comment.
@endocrimes please followup here in a subsequent PR
| // path = "/mnt/store/redis.db" | ||
| // | ||
| // To maximize compatibility with different spin options + custom builds, we do | ||
| // very little validation of runtime config options in the operator. |
There was a problem hiding this comment.
Thank you for this very thorough comment. This is amazing!
|
|
||
| runtimeConfig := app.Spec.RuntimeConfig | ||
| if runtimeConfig.LoadFromSecret != "" { | ||
| // TODO: Should we block on the runtime config secret for consistency? |
There was a problem hiding this comment.
I don't understand this question
There was a problem hiding this comment.
If you provide a wholesale runtime config (LoadFromSecret) should we block on that secret existing, like we do if we render runtime config for you
There was a problem hiding this comment.
Yeah probably, that seems like a good pattern to follow.
There was a problem hiding this comment.
Cool i'll include that in follow up tomorrow (that was also my preference 😅 )
closes: #10