-
Notifications
You must be signed in to change notification settings - Fork 18
info: clarify APICapability and OptionalCapabilities docs #153
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
Merged
+155
−24
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| # Capabilities | ||
|
|
||
| A Device reports what it supports so a Controller can avoid sending | ||
| configuration the Device would ignore, and avoid waiting for messages the Device | ||
| will never send. There are **three independent mechanisms**, all in `ZInfoDevice` | ||
| (see [info.proto](./proto/info/info.proto)), with **different semantics**. | ||
| Conflating them is a correctness bug. | ||
|
|
||
| | mechanism | field | kind | | ||
| | --- | --- | --- | | ||
| | `APICapability` | `api_capability` | monotonic level | | ||
| | `OptionalCapabilities` | `optional_capabilities` | independent booleans | | ||
| | `Capabilities` | `capabilities` | independent booleans (hardware) | | ||
|
|
||
| ## APICapability — a level, not a set | ||
|
|
||
| `APICapability` covers two kinds of support: | ||
|
|
||
| 1. **`EdgeDevConfig` fields the Device parses.** Set one on a Device below the | ||
| level and it is silently ignored. | ||
| 2. **Messages or fields the Device sends.** A Controller expecting one from a | ||
| Device below the level waits for it indefinitely. | ||
|
|
||
| **A larger value implies every smaller one.** A Controller MUST test with `>=`: | ||
|
|
||
| ```text | ||
| if device.api_capability >= API_CAPABILITY_MTU { | ||
| // safe to set NetworkConfig.mtu and NetworkInstanceConfig.mtu | ||
| } | ||
| ``` | ||
|
|
||
| Equality and set-membership tests are wrong. A Device reporting | ||
| `API_CAPABILITY_SMART_REPORT` (20) also supports 1 through 19, and EVE-OS | ||
| reports a single top value rather than a set. | ||
|
|
||
| `API_CAPABILITY_UNSPECIFIED` (0) also covers Devices predating the field, so it | ||
| means "none of the below", not "unknown, try anyway". | ||
|
|
||
| ### What each value covers | ||
|
|
||
| `cfg` = an `EdgeDevConfig` field the Device parses. `rpt` = something the Device | ||
| sends. Rows marked **(?)** are inferred from the enum comment and the commit that | ||
| introduced the value, not stated anywhere authoritative — corrections welcome. | ||
|
|
||
| | value | | covers | | ||
| | --- | --- | --- | | ||
| | `RETRY_UPDATE` = 1 | cfg | `BaseOS.retry_update` | | ||
| | `SHUTDOWN` = 2 | cfg | `EdgeDevConfig.shutdown` | | ||
| | `START_DELAY_IN_SECONDS` = 3 | cfg | `AppInstanceConfig.start_delay_in_seconds` | | ||
| | `EDGEVIEW` = 4 | cfg | `EdgeDevConfig.edgeview`, `EdgeViewConfig.token` | | ||
| | `VOLUME_SNAPSHOTS` = 5 | cfg | `AppInstanceConfig.snapshot`, `SnapshotConfig`; snapshots taken during an app instance update (`SNAPSHOT_TYPE_APP_UPDATE`) **(?)** | | ||
| | `NETWORK_INSTANCE_ROUTING` = 6 | cfg | `NetworkInstanceConfig.static_routes`, `.propagate_connected_routes`, and `IPRoute` | | ||
| | `BOOT_MODE` = 7 | cfg | `VmConfig.boot_mode` | | ||
| | `MTU` = 8 | cfg | `NetworkConfig.mtu`, `NetworkInstanceConfig.mtu` | | ||
| | `ADAPTER_USER_LABELS` = 9 | cfg | `SystemAdapter.shared_labels` | | ||
| | `ENFORCED_NET_INTERFACE_ORDER` = 10 | cfg | `VmConfig.enforce_network_interface_order`, and hence `NetworkAdapter.interface_order` and `Adapter.interface_order` | | ||
| | `NTPS_FQDN` = 11 | cfg | NTP servers as FQDN, and more than one — `ipspec.ntp`, `ipspec.more_ntp` **(?)** | | ||
| | `WIN_LIC_PASSTHROUGH` = 12 | cfg | `VmConfig.enable_oem_win_license_key` | | ||
| | `VOLUME_SNAPSHOTS_IMMEDIATE` = 13 | cfg | `SnapshotType.SNAPSHOT_TYPE_IMMEDIATE`; snapshots on demand, which restart the app **(?)** | | ||
| | `ENCRYPTED_PATCH_ENVELOPE` = 14 | cfg | `EveBinaryArtifact.encrypted_inline`, `.encrypted_volumeref`, `.metadata_cipher_data` | | ||
| | `SINGLE_STACK_IP_NETWORK` = 15 | cfg | `NetworkType.V4Only`, `NetworkType.V6Only` | | ||
| | `CELLULAR_ATTACH_CONFIG` = 16 | cfg | `CellularAccessPoint.attach_apn`, `.attach_ip_type`, `.attach_auth_protocol` | | ||
| | `EDGEVIEW_AUTHENTICATION` = 17 | cfg | EdgeView command authentication; no single field — see [EDGEVIEW-CONTAINER-API.md](https://github.com/lf-edge/eve/blob/master/docs/EDGEVIEW-CONTAINER-API.md) **(?)** | | ||
| | `DISABLE_VTPM` = 18 | cfg | `VmConfig.disable_vtpm` | | ||
| | `LOC_REBOOT_COLLECT_INFO` = 19 | cfg | LOC-initiated reboot and collect-info; `LOCConfig.datastore_collect_info_id` **(?)** | | ||
| | `SMART_REPORT` = 20 | rpt | S.M.A.R.T. data in `ZHardwareHealth.disks`, superseding the deprecated `ZInfoHardware.disks` | | ||
| | `REPORT_TPM_EVENTLOG` = 21 | rpt | `ZAttestQuote.tpm_binary_event_log`, superseding the deprecated `ZAttestQuote.event_log` | | ||
|
|
||
| ### Adding a value | ||
|
|
||
| Appending value *N* asserts that a Device reporting it also supports everything | ||
| below. So: | ||
|
|
||
| 1. Append at the end. Never insert, never renumber. | ||
| 2. Name the field(s) or message(s) it covers, in the enum comment and in the | ||
| table above. | ||
| 3. If the capability depends on build flavor or hardware rather than on version, | ||
| it belongs in `OptionalCapabilities` instead. | ||
|
|
||
| ## OptionalCapabilities — independent booleans | ||
|
|
||
| Not monotonic; test each separately. These describe properties that vary by | ||
| EVE-OS build flavor rather than by version. | ||
|
|
||
| | field | meaning | matters because | | ||
| | --- | --- | --- | | ||
| | `hv_type_kubevirt` | Device runs the Kubevirt hypervisor | required before sending `EdgeNodeCluster`; the KVM flavor cannot join a cluster | | ||
| | `hw_inventory_support` | Device can produce `HardwareInventory` | distinguishes "found no hardware" from "cannot report" when `ZInfoHardware.inventory` is empty | | ||
| | `etcd_snapshot` | Device supports etcd snapshots | EVE-k cluster operations | | ||
|
|
||
| An absent boolean means "not supported or not reported". Do not infer support | ||
| from a Device predating the field. | ||
|
|
||
| ## Capabilities — hardware | ||
|
|
||
| Independent booleans describing the platform, not the software: | ||
|
|
||
| | field | meaning | | ||
| | --- | --- | | ||
| | `HWAssistedVirtualization` | VMX/SVM on amd64, virtualization extensions on arm64 | | ||
| | `IOVirtualization` | IOMMU / I/O virtualization support | | ||
|
|
||
| These bound what a Device can ever run: device passthrough | ||
| (`AppInstanceConfig.adapters`, SR-IOV VFs) will not work without | ||
| `IOVirtualization`, whatever `APICapability` says. | ||
|
|
||
| ## Checklist for Controller implementers | ||
|
|
||
| 1. Test `APICapability` with `>=`; treat `0` as "none". | ||
| 2. Test each `OptionalCapabilities` and `Capabilities` boolean individually. | ||
| 3. Check the gate before sending any field in the table above. | ||
| 4. A Device does not report being sent a gated field it does not understand — it | ||
| just ignores it. That silence is why these gates exist. |
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the same thing? If so, I prefer to have "Volume snapshots supported"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite the same thing — they gate two different
SnapshotTypevalues, so a controller does need to tell them apart.SNAPSHOT_TYPE_APP_UPDATEonly snapshots when the app instance is already being restarted or purged for some other reason, so it costs no extra downtime.SNAPSHOT_TYPE_IMMEDIATEmakes EVE stop and restart the app on its own purely to take the snapshot (handleModifysetsPurgeInprogresswithrestartReason = "Restart to create immediate snapshot"), and it takes precedence when both are requested.The history lines up with that: value 5 landed in 43f3a3a (2023-02) when
SNAPSHOT_TYPE_APP_UPDATEwas the only type;SNAPSHOT_TYPE_IMMEDIATEarrived in 8ebf050 (2025-02-06) and value 13 three weeks later in e841144, "proto: add capability for immediate snapshots". The comment on 13 was copied verbatim from 5. Since the enum is monotonic, a device at level 5..12 supports snapshots on app update but not immediate ones, and two identical comments give no way to see that.That said, your point about the wording stands — leading with a raw enum symbol reads worse, and it was asymmetric with value 5. Reworded both lines to keep the familiar phrasing and make the contrast explicit:
CAPABILITIES.md rows 5 and 13 updated to match. Let me know if you'd still rather have the bare "Volume snapshots supported" on both.