docs: sync CONFIG-PROPERTIES.md with pkg/pillar/types/global.go#5960
Open
mvanhorn wants to merge 3 commits into
Open
docs: sync CONFIG-PROPERTIES.md with pkg/pillar/types/global.go#5960mvanhorn wants to merge 3 commits into
mvanhorn wants to merge 3 commits into
Conversation
Reconcile the runtime configuration properties doc with NewConfigItemSpecMap()
in pkg/pillar/types/global.go (the single source of truth).
Changes:
- Add a header pointing readers at the source file.
- Fix defaults that had drifted: app.allow.vnc (false), app.fml.resolution (""),
debug.enable.ssh (""), debug.default.loglevel (info), debug.default.remote.loglevel (info),
diag.probe.remote.http(s).endpoint (www.google.com unquoted), maintenance.mode (none),
airgap.mode (none).
- Add missing keys present in NewConfigItemSpecMap but not previously documented:
storage.dom0.disk.maxusagebytes, memory.eve.limit.bytes (marked deprecated),
memory.eve.limit.MiB, drain.skip.k8sapinotreachable.timeout, k3s.config.override,
k3s.version, debug.disable.dhcp.all-ones.netmask (marked deprecated).
- Remove timer.hardwareinfo.interval (no longer in NewConfigItemSpecMap).
- Move debug.tui.loglevel into the log-level table where it logically belongs.
- Tighten descriptions for network.fallback.any.eth, agent.*.debug.loglevel,
and agent.*.debug.remote.loglevel.
Closes lf-edge#5298
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
The www.google.com defaults for diag.probe.remote.http(s).endpoint were emitted as bare URLs after the sync, which yetus markdownlint flags as MD034 (no-bare-urls). Wrap both occurrences in backticks so the doc renders as inline code and the link auto-detection does not fire. Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
mvanhorn
force-pushed
the
osc/5298-sync-config-properties
branch
from
d5d7d54 to
f10327e
Compare
| | timer.metric.interval | integer in seconds | 60 (1 minute) | 5 | 3600 (1 hour) | how frequently device reports metrics | | ||
| | timer.hardwarehealth.interval | integer in seconds | 43200 (12 hours) | 21600 (6 hours) | 4294967295 (max uint32) | how frequently device reports hardware health information (ECC, SMART) to controller | | ||
| | timer.hardwareinfo.interval | integer in seconds | 10800 (3 hours) | 10800 (3 hours) | 4294967295 (max uint32) | how frequently device reports hardware information (SMART) to controller (deprecated) | | ||
| | timer.hardwareinfo.interval | integer in seconds | - | - | - | removed; no longer present in `NewConfigItemSpecMap()` | |
Contributor
There was a problem hiding this comment.
Shouldn't this line be removed from the doc?
| | debug.enable.console | boolean | true | - | - | allow console access to EVE, reboot required to disable (controller by default overrides to false) | | ||
| | debug.enable.vnc.shim.vm | boolean | false | - | - | allow VNC access to the container application shim VM (reboot required to disable) | | ||
| | storage.dom0.disk.minusage.percent | integer percent | 20 | 20 | 80 | min. percent of persist partition reserved for dom0 | | ||
| | storage.dom0.disk.maxusagebytes | integer bytes | 2147483648 | 104857600 | 4294967295 (max uint32) | max bytes of persist partition that can be used by dom0 | |
Contributor
There was a problem hiding this comment.
I think you can drop "(max uint32)" from this.
| | force.fallback.counter | integer | 0 | 0 | 4294967295 (max uint32) | forces fallback to other image if counter is changed | | ||
| | newlog.allow.fastupload | boolean | false | - | - | allow faster upload gzip logfiles to controller | | ||
| | memory.apps.ignore.check | boolean | false | - | - | Ignore memory usage check for Apps | | ||
| | memory.eve.limit.bytes | integer bytes | base.ClampToUint32(eveMemoryLimitInBytes) | base.ClampToUint32(eveMemoryLimitInBytes) | 4294967295 (max uint32) | deprecated; use memory.eve.limit.MiB instead. This legacy value is limited to 4GB and still has higher priority for backward compatibility | |
Contributor
There was a problem hiding this comment.
Can this be expressed using simple words and not code?
Contributor
There was a problem hiding this comment.
Also, "higher priority" than what? I assume the next item in the list but it makes sense to be explicit.
| | wwan.modem.recovery.restart.modemmanager | boolean | false | - | - | If a modem firmware crash occurs and ModemManager fails to properly recognize or manage the restarted modem, EVE will attempt to restart ModemManager as a recovery step. This occurs before the watchdog mechanism is triggered (if enabled) and can be combined with driver reload recovery mechanism. | | ||
| | diag.probe.remote.http.endpoint | string | `"http://www.google.com"` | - | - | Remote endpoint (URL, IP instead of hostname is accepted) queried over HTTP to assess the state of network connectivity whenever the controller is not reachable. Used only for diagnostics (no functional impact). Set to an empty string to disable. | | ||
| | diag.probe.remote.https.endpoint | string | `"https://www.google.com"` | - | - | Remote endpoint (URL, IP instead of hostname is NOT accepted) queried over HTTPS to assess the state of network connectivity whenever the controller is not reachable. Used only for diagnostics (no functional impact). Set to an empty string to disable. | | ||
| | diag.probe.remote.http.endpoint | string | `www.google.com` | - | - | Remote endpoint (URL, IP instead of hostname is accepted) queried over HTTP to assess the state of network connectivity whenever the controller is not reachable. Used only for diagnostics (no functional impact). Set to an empty string to disable. | |
Contributor
There was a problem hiding this comment.
The explanation says "URL" but the default is a hostname. Which one is correct per global.go?
| | diag.probe.remote.http.endpoint | string | `"http://www.google.com"` | - | - | Remote endpoint (URL, IP instead of hostname is accepted) queried over HTTP to assess the state of network connectivity whenever the controller is not reachable. Used only for diagnostics (no functional impact). Set to an empty string to disable. | | ||
| | diag.probe.remote.https.endpoint | string | `"https://www.google.com"` | - | - | Remote endpoint (URL, IP instead of hostname is NOT accepted) queried over HTTPS to assess the state of network connectivity whenever the controller is not reachable. Used only for diagnostics (no functional impact). Set to an empty string to disable. | | ||
| | diag.probe.remote.http.endpoint | string | `www.google.com` | - | - | Remote endpoint (URL, IP instead of hostname is accepted) queried over HTTP to assess the state of network connectivity whenever the controller is not reachable. Used only for diagnostics (no functional impact). Set to an empty string to disable. | | ||
| | diag.probe.remote.https.endpoint | string | `www.google.com` | - | - | Remote endpoint (URL, IP instead of hostname is NOT accepted) queried over HTTPS to assess the state of network connectivity whenever the controller is not reachable. Used only for diagnostics (no functional impact). Set to an empty string to disable. | |
Comment on lines
+163
to
+164
| | agent.*agentname*.debug.loglevel | string | info | if set overrides debug.default.loglevel for this particular agent (Legacy setting debug.*agentname*.loglevel still supported) | | ||
| | agent.*agentname*.debug.remote.loglevel | string | info | if set overrides debug.default.remote.loglevel for this particular agent (Legacy setting debug.*agentname*.remote.loglevel) | |
Contributor
There was a problem hiding this comment.
These two have a default of "" AFAIK, which matches the "if set" text.
Contributor
eriknordmark
left a comment
eriknordmark
left a comment
There was a problem hiding this comment.
Thanks for doing this.
A few comments and questions.
Also, the commitlint expects
<topic/summary>
- Remove stale 'timer.hardwareinfo.interval' (no longer in NewConfigItemSpecMap) - Drop '(max uint32)' parenthetical from timer.port.testbetterinterval max - Rewrite testbetterinterval description in plain words; clarify what 'higher priority' compares against - diag.probe.remote.http: accept hostname/IP, simpler wording - diag.probe.remote.https: hostname only, IP not accepted - agent.*.debug.loglevel and remote.loglevel defaults are "" (matches 'if set overrides' semantics) Refs: lf-edge#5298 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Contributor
Author
|
Thanks for the careful review @eriknordmark. Pushed 9afad7c addressing all seven inline comments:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reconciles
docs/CONFIG-PROPERTIES.mdagainst the canonical source inNewConfigItemSpecMap()(pkg/pillar/types/global.go), as the issue requests.Changes
Fixed defaults that had drifted from the code
app.allow.vnc:false(wasfalse (only local access)- dropped the editorial aside since the default is already documented in the description)app.fml.resolution:""(wasnotset;FmlResolutionUnset = "")debug.enable.ssh:""(wasempty string(ssh disabled); moved the explanation into the description)debug.default.loglevel:info(wasdebug;AddStringItem(DefaultLogLevel, "info", ...))debug.default.remote.loglevel:info(waswarning; same source)diag.probe.remote.http.endpoint/diag.probe.remote.https.endpoint: unquotedwww.google.commatches the AddStringItem defaultmaintenance.mode/airgap.mode:none(was"none"; matches theTS_NONETriState default)Added keys present in
NewConfigItemSpecMapbut missing from the docstorage.dom0.disk.maxusagebytes(Dom0DiskUsageMaxBytes)memory.eve.limit.bytes(EveMemoryLimitInBytes) - marked deprecated, points readers at the MiB variantmemory.eve.limit.MiB(EveMemoryLimitInMiB)drain.skip.k8sapinotreachable.timeout(DrainSkipK8sAPINotReachableTimeout)k3s.config.override(K3sConfigOverride)k3s.version(K3sVersionOverride)debug.disable.dhcp.all-ones.netmask(DisableDHCPAllOnesNetMask) - marked deprecated, matching the in-code commentRemoved
timer.hardwareinfo.interval- no longer inNewConfigItemSpecMap(). The previous entry described it asdeprecated; this PR removes it rather than keeping a deprecated row for a key that has fully been retired from the spec map.Reorganized
debug.tui.loglevelinto the log-level table, since it's a log-level setting and the surrounding entries (default/syslog/kernel) already live there.Header
NewConfigItemSpecMap()inpkg/pillar/types/global.goas the single source of truth for these properties.Verification
NewConfigItemSpecMap(), walkedgrep -n "AddIntItem\|AddBoolItem\|AddStringItem\|AddTriStateItem" pkg/pillar/types/global.goand confirmed each appears in the markdown table with the matching default.Add*Item(...)call.Closes #5298