connectivity: Introduce Cilium feature detection #998
Conversation
|
Commit 855bf47 does not contain "Signed-off-by". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
There was a problem hiding this comment.
Very nice work 🚀 Thanks for splitting this up into commits that are easy to review by themselves.
I like the approach of using per-node cilium status and falling back to ConfigMap in case information is not available there.
A few small nits inline. There's also a linter and an MLH warning about missing Signed-off-by. Other than that LGTM.
This fetches the Cilium pod information right after we initialized the client. This change should not have any functional changes, but allows future commits to access those Cilium pods early. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This commit adds Cilium feature detection to the Cilium connectivity test suite. Cilium features are things like L7 proxy status, chaining mode, flow aggregation mode etc. Because some features are dependent on run-time information like kernel versions, we prefer to collect the feature status from `cilium status` where available. Other features are currently detected by parsing the Cilium ConfigMap. The current code assumes that the detected feature set is consistent across all Cilium pods and clusters. If a discrepancy is detected, the current code simply emits a warning and assumes the feature set from the first Cilium pod probed. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This commit switches the flow aggregation detection to use the newly introduced Cilium feature set. This commit should introduce no functional changes. The flow aggregation log line is moved to a later stage ensure feature detection has been run before we print the status. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
This commit introduces a new function to the connectivity test framework to specify feature requirements for certain tests. All tests requiring the L7 proxy are updated to use the new requirements framework to skip those tests if the L7 proxy is not available. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
gandro
force-pushed
the
pr/gandro/cilium-feature-detection
branch
from
38605c4
to
96e106c
Compare
Testing (*FeatureSet).MatchRequirements for now. Adresses #998 (comment) Signed-off-by: Tobias Klauser <tobias@cilium.io>
This might be useful in CI to catch issues with feature detection. Signed-off-by: Tobias Klauser <tobias@cilium.io>
There was a problem hiding this comment.
LGTM, thank you!
FWIW, I've pushed two additional commits: one to introduce a basic unit test for (*FeatureSet).MatchRequirements, adressing #998 (comment) and a second to add a debug log with all detected features which IMO might be helpful for Cilium CI to catch issues with feature detection.
|
Sweet! Can I provide a feature list for the feature detection? 😅 |
Please do. FWIW, it should be fairly easy to add new features to be detected here: https://github.com/cilium/cilium-cli/pull/998/files#diff-30fe8f14d3d70ed606b3e8a9ed2305d14bc2f7630530af495df364d8d6609e74R113-R172 |
Testing (*FeatureSet).MatchRequirements for now. Adresses #998 (comment) Signed-off-by: Tobias Klauser <tobias@cilium.io>
Testing (*FeatureSet).MatchRequirements for now. Adresses cilium#998 (comment) Signed-off-by: Tobias Klauser <tobias@cilium.io>
Testing (*FeatureSet).MatchRequirements for now. Adresses cilium/cilium-cli#998 (comment) Signed-off-by: Tobias Klauser <tobias@cilium.io>
This PR adds Cilium feature detection to the Cilium connectivity test suite. Cilium features are things like L7 proxy status, chaining
mode, flow aggregation mode etc. Because some features are dependent on run-time information like kernel versions, we prefer to collect the feature status from
cilium statuswhere available. Other features are currently detected by parsing the Cilium ConfigMap.Some features are either simply turned on or off, while others additionally might include a
Modestring which provides more information about in what mode a particular feature is running (e.g. when running with CNI chaining,Enabledwill be true, and theModestring will additionally contain the name of the chained CNI).With these changes, tests can now require certain features to be enabled. This PR adds the
FeatureL7Proxyrequirement to all tests which depend on the L7 proxy being available. Subsequent PRs will also add more conditional connectivity test behavior based on the features introduced by this PR (e.g. host firewall and chaining mode).Review per commit