install/kubernetes: change mapDynamicSizeRatio from number to string #39834
Conversation
|
/test |
There was a problem hiding this comment.
number is a valid schema type. Changing it to string breaks users who have set this field via a values.yaml file as you yourself suggested here (and which was confirmed to work): #39823 (comment)
Note that it possible to set the number via --set-json in Helm:
helm template cilium/cilium --set-json bpf.mapDynamicSizeRatio=0.08
Ultimately, I think the linked issue here is an issue with Cilium CLI and the fact that it has no option to pass number values.
Also note that changing the type to a string breaks this comparision here, as 0.0 is not equal "0.0":
|
@gandro thanks for the review. What if, instead of replacing number with string, I add string. This will make both modes to work. WRT the comparison part, be aware that |
|
I think adding a string should indeed work. Good point with the comparison, I didn't realize the variable was not derived from the Helm value. There is one oddity with regards to making a string work though: I think using 0.0 (as a number) will make the chart fallback to |
Add type "string" for bpf.mapDynamicSizeRatio, which caused Helm schema validation to fail when setting it as a float. This restores compatibility with the expected numeric input: --set bpf.mapDynamicSizeRatio=0.08 Fixes: 79733d2 ("helm: Introduce values.schema.json and tooling") Signed-off-by: André Martins <andre@cilium.io>
|
/test |
Reverts type change from "number" to "string" for bpf.mapDynamicSizeRatio, which caused Helm schema validation to fail when setting it as a float.
This restores compatibility with the expected numeric input:
--set bpf.mapDynamicSizeRatio=0.08
Fixes: 79733d2 ("helm: Introduce values.schema.json and tooling")
Fixes #39823