Repository navigation
Move static runsc flags to configuration file - #2281
Benjamin Elder (BenTheElder) merged 1 commit into
Conversation
|
Once substrate goes GA and we have back-compat, we're probably going to need to conditionally set flags depending on the runsc version to match what was used when the snapshot was taken. (thockin is still drafting the policy) That seems incompatible with moving them out of the binary. Maybe we can keep the defaults inline but allow a config to append flags for debugging / development? |
Benjamin Elder (BenTheElder)
added
area/gvisor
kind/feature
|
The configuration file is part of the release and goes together with the binary. So it doesn't matter whether the flags are defined in binary or a config file on the side. For supporting multiple versions, with possibly different flags, the configuration file would have to be changed to support multiple versions too. But we'll cross that bridge when we need it. |
f7311ac to
4c95c2e
Compare
Yes ... but then we're introducing a breaking change itself [to the config format], while we're actively planning to cease all breaking changes soon until a possible 2.0. Tim Hockin (@thockin) has been working on a policy for this, I assume it includes all configuration. Do we need to expose this to users at all? For developers, is this really significantly faster than a cached go build + editing a go source code with these flags? I've found it pretty fast to iterate on ateom changes just with If we just wanted to move it to a seperate file on disk, either another go source file, or a https://pkg.go.dev/embed seems more appropriate than allowing runtime override we can't really support yet. (Zoe Zhao (@zoez7) is working on a sandboxconfig design that might have room to enable this in the future). Do we really need runtime override for these flags?
For something like "enable debug" we have a pattern to add one top-level debug-mode flag (which must be off by default #2274) that enables relevant underlying flags. |
|
This is not a public API and it's not intended to be controlled via sandbox config. It's a configuration file that is bundled and released together with the binary. Its reason to exist is to unify flag configuration into a single place that can be easily changed and audited. There are several advantages to making this a text file:
|
Benjamin Elder (BenTheElder)
left a comment
Benjamin Elder (BenTheElder)
left a comment
There was a problem hiding this comment.
What if we call this experimental-flags-do-not-edit.json and drop the CLI flag?
I just don't want an end user editing this and then complaining when things are broken, if it's meant for developers of substrate.
If we're building an end-user feature to add debug flags, we should probably give them a way to append debug flags instead of fully overriding the flagset.
4c95c2e to
a26ae40
Compare
|
As discussed, removed the flag and renamed the file to |
Move existing hardcoded runsc flags into kodata/internal_flags.json to make it easier to manage and add flags. Flags are reloaded automatically when file changes to allow for setting flags during runtime, e.g. enable debug logs. *AI disclosure: AI tooling assisted in writing and testing parts of this change.* - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR
a26ae40 to
eb5ba97
Compare
Move existing hardcoded runsc flags into kodata/flags.json to make it easier to manage and add flags. Flags are reloaded automatically when file changes to allow for setting flags during runtime, e.g. enable debug logs.
AI disclosure: AI tooling assisted in writing and testing parts of this change.