helm: add default tunnel values in configmap

[3u13r]

This PR fixes the feature discovery for tunneling in cilium-cli.

Background:
With the new routingMode and tunnelProtocol helm values, the default is not filled in as a value in values.yaml anymore (in contrast to the old tunnel value, see: https://github.com/cilium/cilium/pull/24561/files). Therefore, those default values don't show up in the config-map. The cilium-cli tries to get the enabled features by (among other things) reading out the config-map. Since it does not find any value, it sets the feature to Enabled: false (see: cilium-cli/check/features.go:209 or https://github.com/cilium/cilium-cli/pull/1655/files)

Of course there are multiple places to fix this behavior. I opted for this solution as I think that the configuration should always be reflected in the configmap for visibility reasons. Especially if the default is tunneling + vxlan not disabled tunneling.

Add the tunnel values to the config map even when the default values are used.

[kaworu]

Thanks for the patch and clear description @3u13r, LGTM. Since this compatibility issue with the cilium-cli affects v1.14 I added the needs-backport/1.14 label. Also pulling @pchaigno for review.

[…] Of course there are multiple places to fix this behavior. I opted for this solution as I think that the configuration should always be reflected in the configmap for visibility reasons.

That make sense to me. However I still think the cilium-cli should default to the same values as Helm / cilium-agent. Let me know if you're willing to open a PR at https://github.com/cilium/cilium-cli 🙏

[pchaigno]

Why should default values be visible in the ConfigMap? We have many many flags that are not visible in the ConfigMap when set to their defaults. If I understand correctly the rationale for this change, it sounds like a bug that needs to be fixed in the CLI.

[kaworu]

Why should default values be visible in the ConfigMap? We have many many flags that are not visible in the ConfigMap when set to their defaults. If I understand correctly the rationale for this change, it sounds like a bug that needs to be fixed in the CLI.

Yes, it's a bug that needs to be fixed in the CLI. On the Cilium side, this PR is case in point of why default values in the ConfigMap are useful: so that we don't have to duplicate and maintain them in the Cilium CLI. Why not do both?

[joamaki]

Hmm, tricky. I see the point of having it in helm values, but on the flip side having defaults picked there would mean we'd have two defaults: one in cilium-agent's command-line/config flag and one in the template, which isn't great either. Though the default in the helm values is the one users see, which is why I'd prefer to merge this fix.

@pchaigno is also right in that cilium-cli should not rely on ConfigMap as cilium does not require any config options to be set and thus the mechanism used in cilium-cli is inherently faulty and a better mechanism would be to interrogate the actual options picked at runtime.

[pchaigno]

One thing I like about not having the defaults in the ConfigMap is that it makes it very easy to understand what is different in this installation from a default installation. That's often key in understanding problems.

But we have definitely never enforced it one way or the other, so not blocking this PR :-)

[nebril]

/test

[julianwiedmann]

If we've reached consensus here (which seems to be the case), could one of the reviewers add ready-to-merge? Thank you!