[promtail helm chart] - Expand promtail syslog svc to support values

[billimek]

What this PR does / why we need it:

The promtail helm chart syslog service configuration in the values.yaml
supports elements not accounted-for in the actual syslog service
template.

This change modifies the promtail syslog service to account for those
missing elements.

Special notes for your reviewer:

This fixes items missed in #1617

Checklist

• Documentation added
• Tests updated

[joe-elliott]

Let's remove the if..else if...else on service type and just allow any field to be configurable for any service type. I know that this will technically allow a user to create a misconfigured service that k8s rejects, but I'd prefer that to silently ignoring some fields based on the service type. It would also clean up the yaml some.

Also, the values.yaml currently has documentation for the different syslogService fields. Please add additional docs for the added fields.

[billimek]

Let's remove the if..else if...else on service type and just allow any field to be configurable for any service type. I know that this will technically allow a user to create a misconfigured service that k8s rejects, but I'd prefer that to silently ignoring some fields based on the service type. It would also clean up the yaml some.

Hi @joe-elliott, to clarify: you want to remove the conditionals around the service type part of the syslog-service to allow the user to input whatever they want (i.e. asking for a loadbalancerIP for a ClusterIP type?)

Also, the values.yaml currently has documentation for the different syslogService fields. Please add additional docs for the added fields.

Where would you like additional docs added for all? of the fields for the syslog service? I see a docs/installation/helm.md but that doesn't currently have a comprehensive coverage for all of the other values, so I'm not sure if this is what you're thinking about.

[joe-elliott]

Yup. I understand that configuring a loadBalancerIP on a ClusterIP is an incorrect configuration but I prefer the k8s api returning that error so the operator can see their error clearly. With the conditionals it would just silently ignore certain combinations of fields.

Regarding documentation: In a past PR you nicely included some commented out fields with additional information here:

loki/production/helm/promtail/values.yaml

Line 147 in cdd60cd

syslogService:

Now that I'm looking closer I can see that it already has comments about the fields you're adding in this PR (loadBalancerIP, externalTrafficPolicy). No additional documentation is necessary.

[codecov-io]

Codecov Report

Merging #1731 into master will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1731      +/-   ##
==========================================
+ Coverage   64.16%   64.23%   +0.07%     
==========================================
  Files         121      121              
  Lines        9024     9024              
==========================================
+ Hits         5790     5797       +7     
+ Misses       2829     2825       -4     
+ Partials      405      402       -3

Impacted Files	Coverage Δ
pkg/logql/evaluator.go	91.22% <0%> (-0.59%)	⬇️
pkg/ingester/transfer.go	66.42% <0%> (+1.42%)	⬆️
pkg/promtail/targets/filetarget.go	70.55% <0%> (+1.84%)	⬆️
pkg/promtail/targets/tailer.go	78.16% <0%> (+4.59%)	⬆️

[billimek]

@joe-elliott the changes should be implemented. Can you please take a look again?

[joe-elliott]

Thanks for the changes. LGTM