Strip verbose fields from VLAN interfaces in large NodeNetworkState - #1505
Strip verbose fields from VLAN interfaces in large NodeNetworkState#1505mkowalski wants to merge 2 commits into
Conversation
When a node has more than 500 interfaces, strip non-essential fields from VLAN interfaces in the NodeNetworkState to prevent the object from exceeding the etcd request size limit (1.5 MB). For VLAN interfaces, only the following fields are preserved: name, type, state, ipv4, ipv6, vlan (id + base-iface) Fields like mac-address, mtu, lldp, ethtool, etc. are stripped. Non-VLAN interfaces (ethernet, bridge, bond, etc.) are not affected. When the interface count is below the threshold, all fields are preserved as before. Fixes: https://redhat.atlassian.net/browse/OCPBUGS-62470 Signed-off-by: Mateusz Kowalski <mko@redhat.com> Generated-by: OpenClaw OpenClaw 2026.4.15 (041266a) AI-model: claude-opus-4.6
kubevirt-bot
added
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to strip non-essential fields from VLAN interfaces when the total number of interfaces exceeds a threshold of 500. This optimization aims to reduce the size of the NodeNetworkState object, preventing it from exceeding the etcd request size limit (1.5 MB) on nodes with many VLANs. Comprehensive tests have been added to verify that fields are correctly stripped when the threshold is met and preserved otherwise. I have no feedback to provide.
Add an E2E test suite that validates the VLAN verbose field stripping feature end-to-end. The test exercises three scenarios: 1. Below-threshold behavior: Creates a small number of VLANs and verifies all fields (including verbose ones like mtu, mac-address) are preserved in the NodeNetworkState. 2. Above-threshold behavior: Lowers the threshold via the new VLAN_FILTER_INTERFACE_COUNT_THRESHOLD environment variable on the handler DaemonSet, creates VLANs exceeding the threshold, and verifies that: - Essential fields (name, type, state, vlan, ipv4, ipv6) are preserved - Verbose fields (mtu, mac-address, lldp, ethtool) are stripped - Non-VLAN interfaces (ethernet) retain all fields - NNS object remains valid and within etcd size limits 3. NNCP integration: Creates VLANs via NodeNetworkConfigurationPolicy and verifies correct VLAN metadata in NNS across all worker nodes. To support E2E testing without creating 500+ real interfaces, the interfaceCountThreshold constant is replaced with a configurable function that reads from the VLAN_FILTER_INTERFACE_COUNT_THRESHOLD environment variable (defaulting to 500). Also adds unit tests for: - Environment variable override behavior - Invalid/zero/negative env var values falling back to default Signed-off-by: Mateusz Kowalski <mko@redhat.com> Generated-by: OpenClaw AI-model: claude-opus-4 Signed-off-by: Mateusz Kowalski <mko@redhat.com>
|
@mkowalski: The following test failed, say
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/retest |
|
@mkowalski: The following test failed, say
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@mkowalski can we fix CI first before review it ? I think it's related. |
|
I think no because it failed in the test case with small amount of VLANs Anyway will need to look someday because I want this in |
I think is just the e2e tests there since it expect "state: up" for the vlans but since the tests is creating htem with ip link NNS has "state: ignore" for the vlans. |
3 participants
Is this a BUG FIX or a FEATURE?:
/kind bug
What this PR does / why we need it:
When a node has a large number of interfaces (e.g., 1500+ VLANs), the NodeNetworkState object exceeds the etcd request size limit (1.5 MB), causing
etcdserver: request is too largeerrors. This breaks NNS updates entirely, which cascades into empty NNCP status/reason fields.This PR extends the existing
filterOut()function to strip verbose fields from VLAN interfaces when the total interface count exceeds 500. For each VLAN interface, only essential fields are preserved:namemac-addresstypemtustatelldpipv4ethtoolipv6accept-all-mac-addressesvlan(id + base-iface)wait-ip,description, etc.This reduces per-VLAN size from ~300 bytes to ~100 bytes. For 1500 VLANs, that is ~450 KB → ~150 KB, safely under the etcd limit.
Key design decisions:
Fixes: https://redhat.atlassian.net/browse/OCPBUGS-62470
Special notes for your reviewer:
The etcd limit is 1.5 MB per request. A VLAN interface with full nmstate output is ~250-300 bytes of YAML. At 1500 VLANs, the NNS easily exceeds the limit. The upstream nmstate library also has
MAX_SUPPORTED_INTERFACES = 1000but that is a separate issue (desired state limit, not observed state).Release note: