From 411259c9db2515694492f8aa883cf1d65ddf3f3d Mon Sep 17 00:00:00 2001 From: Michi Mutsuzaki Date: Wed, 11 Oct 2023 14:38:26 -0700 Subject: [PATCH] sysdump feature detection: Don't depend on Cilium version Detecting Cilium version can get arbitrary complicated. Let's not even try during sysdump feature detection since sysdump doesn't depend on any version-specific features yet. This commit separates ExtractFromConfigMap into two functions: - ExtractFromConfigMap to detect version-independent features. - ExtractFromVersionedConfigMap to detect version-specific features. Then sysdump uses ExtractFromConfigMap to extract enabled features it needs to know about without having to detect Cilium version. Problem solved, at least for now... Fixes: #2029 Signed-off-by: Michi Mutsuzaki --- connectivity/check/features.go | 3 ++- sysdump/sysdump.go | 8 ++----- utils/features/features.go | 40 ++++++++++++++++++--------------- utils/features/features_test.go | 23 +++++++++++++------ 4 files changed, 42 insertions(+), 32 deletions(-) diff --git a/connectivity/check/features.go b/connectivity/check/features.go index 2a6b24ebde..f7638b2668 100644 --- a/connectivity/check/features.go +++ b/connectivity/check/features.go @@ -278,7 +278,8 @@ func (ct *ConnectivityTest) detectFeatures(ctx context.Context) error { // If unsure from which source to retrieve the information from, // prefer "CiliumStatus" over "ConfigMap" over "RuntimeConfig". // See the corresponding functions for more information. - features.ExtractFromConfigMap(ct.CiliumVersion, cm) + features.ExtractFromVersionedConfigMap(ct.CiliumVersion, cm) + features.ExtractFromConfigMap(cm) err = ct.extractFeaturesFromRuntimeConfig(ctx, ciliumPod, features) if err != nil { return err diff --git a/sysdump/sysdump.go b/sysdump/sysdump.go index 009d0206eb..ef9f49df7b 100644 --- a/sysdump/sysdump.go +++ b/sysdump/sysdump.go @@ -267,12 +267,8 @@ func NewCollector(k KubernetesClient, o Options, startTime time.Time, cliVersion } c.log("ℹ️ %s ConfigMap not found in %s namespace", ciliumConfigMapName, c.Options.CiliumNamespace) } - if c.CiliumConfigMap != nil && len(c.CiliumPods) > 0 { - ciliumVersion, err := c.Client.GetCiliumVersion(context.Background(), c.CiliumPods[0]) - if err != nil { - return nil, fmt.Errorf("failed to get Cilium version from %s/%s: %w", c.CiliumPods[0].Namespace, c.CiliumPods[0].Name, err) - } - c.FeatureSet.ExtractFromConfigMap(*ciliumVersion, c.CiliumConfigMap) + if c.CiliumConfigMap != nil { + c.FeatureSet.ExtractFromConfigMap(c.CiliumConfigMap) c.log("🔮 Detected Cilium features: %v", c.FeatureSet) } } diff --git a/utils/features/features.go b/utils/features/features.go index 0f1ff3b47c..27e50051d7 100644 --- a/utils/features/features.go +++ b/utils/features/features.go @@ -177,24 +177,11 @@ func RequireMode(feature Feature, mode string) Requirement { } } -// extractFeaturesFromConfigMap extracts features from the Cilium ConfigMap. -// Note that there is no rule regarding if the default value is reflected -// in the ConfigMap or not. -func (fs Set) ExtractFromConfigMap(ciliumVersion semver.Version, cm *v1.ConfigMap) { - // CNI chaining. - // Note: This value might be overwritten by extractFeaturesFromCiliumStatus - // if this information is present in `cilium status` - mode := "none" - if v, ok := cm.Data["cni-chaining-mode"]; ok { - mode = v - } - fs[CNIChaining] = Status{ - Enabled: mode != "none", - Mode: mode, - } - +// ExtractFromVersionedConfigMap extracts features based on Cilium version and cilium-config +// ConfigMap. +func (fs Set) ExtractFromVersionedConfigMap(ciliumVersion semver.Version, cm *v1.ConfigMap) { if versioncheck.MustCompile("<1.14.0")(ciliumVersion) { - mode = "vxlan" + mode := "vxlan" if v, ok := cm.Data["tunnel"]; ok { mode = v } @@ -203,7 +190,7 @@ func (fs Set) ExtractFromConfigMap(ciliumVersion semver.Version, cm *v1.ConfigMa Mode: mode, } } else { - mode = "tunnel" + mode := "tunnel" if v, ok := cm.Data["routing-mode"]; ok { mode = v } @@ -220,6 +207,23 @@ func (fs Set) ExtractFromConfigMap(ciliumVersion semver.Version, cm *v1.ConfigMa Mode: tunnelProto, } } +} + +// ExtractFromConfigMap extracts features from the Cilium ConfigMap. +// Note that there is no rule regarding if the default value is reflected +// in the ConfigMap or not. +func (fs Set) ExtractFromConfigMap(cm *v1.ConfigMap) { + // CNI chaining. + // Note: This value might be overwritten by extractFeaturesFromCiliumStatus + // if this information is present in `cilium status` + mode := "none" + if v, ok := cm.Data["cni-chaining-mode"]; ok { + mode = v + } + fs[CNIChaining] = Status{ + Enabled: mode != "none", + Mode: mode, + } fs[IPv4] = Status{ Enabled: cm.Data["enable-ipv4"] == "true", diff --git a/utils/features/features_test.go b/utils/features/features_test.go index 6e2745df2b..6d251207cd 100644 --- a/utils/features/features_test.go +++ b/utils/features/features_test.go @@ -58,24 +58,33 @@ func TestFeatureSetMatchRequirements(t *testing.T) { } } -func TestFeatureSet_extractFeaturesFromConfigMap(t *testing.T) { +func TestFeatureSet_extractFromVersionedConfigMap(t *testing.T) { fs := Set{} ciliumVersion := semver.Version{Major: 1, Minor: 14, Patch: 0} cm := corev1.ConfigMap{} - fs.ExtractFromConfigMap(ciliumVersion, &cm) + fs.ExtractFromVersionedConfigMap(ciliumVersion, &cm) + cm.Data = map[string]string{ + "routing-mode": "tunnel", + "tunnel-protocol": "geneve", + } + fs.ExtractFromVersionedConfigMap(ciliumVersion, &cm) + assert.True(t, fs[Tunnel].Enabled) + assert.Equal(t, "geneve", fs[Tunnel].Mode) +} + +func TestFeatureSet_extractFromConfigMap(t *testing.T) { + fs := Set{} + cm := corev1.ConfigMap{} + fs.ExtractFromConfigMap(&cm) cm.Data = map[string]string{ "enable-ipv4": "true", "enable-ipv6": "true", - "routing-mode": "tunnel", - "tunnel-protocol": "geneve", "mesh-auth-mutual-enabled": "true", "enable-ipv4-egress-gateway": "true", } - fs.ExtractFromConfigMap(ciliumVersion, &cm) + fs.ExtractFromConfigMap(&cm) assert.True(t, fs[IPv4].Enabled) assert.True(t, fs[IPv6].Enabled) assert.True(t, fs[AuthSpiffe].Enabled) assert.True(t, fs[EgressGateway].Enabled) - assert.True(t, fs[Tunnel].Enabled) - assert.Equal(t, "geneve", fs[Tunnel].Mode) }