fluent / helm-charts

Helm Charts for Fluentd and Fluent Bit
Apache License 2.0
366 stars 438 forks source link

New sha256sum uid for grafana dashboard exceeds 40 chars limit. #511

Open iblexisnexis opened 1 month ago

iblexisnexis commented 1 month ago

New sha256sum uid for grafana dashboard exceeds 40 chars limit.

Fluent-bit chart version: 0.46.7

We get errors on grafana sidecar trying to save the dashboard:

log=logger=provisioning.dashboard type=file name=sidecarProvider t=2024-05-27T17:07:30.857026945Z level=error msg="failed to save dashboard" file=/tmp/dashboards/fluent-bit-fluent-bit.json error="uid too long, max 40 characters"

UID generated: 1b51768e3e61639b7fab3ec96497b1000ac59698053923ee3b891167b8050c70 which is 64 chars long.

pmcgrath commented 1 month ago

Yep, seeing the same

See https://github.com/grafana/grafana/issues/11620

Being set https://github.com/fluent/helm-charts/blob/429420f56365f6c152369bc90cc192b2f9d516b3/charts/fluent-bit/dashboards/fluent-bit.json#L1562

which uses https://github.com/fluent/helm-charts/blob/429420f56365f6c152369bc90cc192b2f9d516b3/charts/fluent-bit/templates/_helpers.tpl#L143

This PR introduced the uid which is too long https://github.com/fluent/helm-charts/pull/503

Have seen people ask about having multiple installations, see https://github.com/fluent/helm-charts/issues/289

Not sure if this is something people need with fluent-bit, but can see there is an option to deploy as a deployment rather than a daemonset, in which case it may make sense to NOT have a hard coded uid

Maybe having an optional way to pass a uid value via the values.yaml would work ? See https://github.com/fluent/helm-charts/blob/429420f56365f6c152369bc90cc192b2f9d516b3/charts/fluent-bit/values.yaml#L179

Can see there is only a single dashboard at the moment, but what if more dashboards are added at https://github.com/fluent/helm-charts/tree/main/charts/fluent-bit/dashboards

So would be best to have as a lookup value I guess

pmcgrath commented 1 month ago

Another easier effort might be to replace the sha256sum usage with https://helm.sh/docs/chart_template_guide/function_list/#adler32sum

Can see it working with this with different release names we get between 9 and 10 chars

for rel_name in fluent-bit fluent-bit-a rel-a rel-b really-long-release-name-aaaaaaaaaaaaaaaaaaaaaaaaaa; do
    suffix=-fluent-bit
        [[ ${rel_name:0:10} == "fluent-bit" ]] && suffix=

    cm_name=${rel_name}${suffix}-dashboard-fluent-bit
    json_file_name=${rel_name}${suffix}-fluent-bit.json

    helm template ${rel_name} . \
        --namespace dest-ns \
        --set dashboards.enabled=True | \
        yq '. | select(.kind == "ConfigMap" and .metadata.name == "'${cm_name}'") | .data["'${json_file_name}'"]' | \
        jq -r '.title + " " + .uid'; 
done | column -ts ' '

fluent-bit                                                      1097533160
fluent-bit-a                                                    1341523830
rel-a-fluent-bit                                                577176767
rel-b-fluent-bit                                                577242304
really-long-release-name-aaaaaaaaaaaaaaaaaaaaaaaaaa-fluent-bit  2620200503

could then pad out to 40 chars which matches the grafana restriction

pmcgrath commented 1 month ago

See https://github.com/fluent/helm-charts/pull/509

marcofranssen commented 1 month ago

Facing the same, would be great to see #509 fix merged and released.