Support service.Type="LoadBalancer" in Helm chart

[JakeCooper]

Is your feature request related to a problem? Please describe.

Trying to expose our Pyroscope instance internally via the chart provided. Went to do something like this

pyroscope:
  pyroscope:
    service:
      type: LoadBalancer
      serviceAnnotations:
        networking.gke.io/load-balancer-type: "Internal"
        networking.gke.io/internal-load-balancer-allow-global-access: "true"

But unfortunately this breaks the {{ name }}-headless service with the following error

Error: UPGRADE FAILED: cannot patch "pyroscope-headless" with kind Service: Service "pyroscope-headless" is invalid: spec.clusterIPs[0]: Invalid value: "None": may not be set to 'None' for LoadBalancer services

Describe the solution you'd like

The above yaml works (or please feel free to tell me if I'm holding it wrong!)

Describe alternatives you've considered

Attempting to go through the microservice-values way ATM

Additional context

Add any other context or screenshots about the feature request here.

[kolesnikovae]

Indeed, I think we should change the way the service type is configured. Specifically, for our "headless" services, type should always be ClusterIP – as this is the default value, we can simply drop the attribute (line 37: type: {{ .Values.pyroscope.service.type }}):

pyroscope/operations/pyroscope/helm/pyroscope/templates/services.yaml

Lines 28 to 46 in 8cf80c8

	apiVersion: v1
	kind: Service
	metadata:
	name: {{ $cfg.name }}-headless
	namespace: {{ .Release.Namespace }}
	labels:
	{{- include "pyroscope.labels" . | nindent 4 }}
	app.kubernetes.io/component: {{ $component | quote }}
	spec:
	type: {{ .Values.pyroscope.service.type }}
	clusterIP: None
	ports:
	- port: {{ .Values.pyroscope.service.port }}
	targetPort: {{ .Values.pyroscope.service.port_name }}
	protocol: TCP
	name: {{ .Values.pyroscope.service.port_name }}
	selector:
	{{- include "pyroscope.selectorLabels" . | nindent 4 }}
	app.kubernetes.io/component: {{ $component | quote }}

@JakeCooper may I ask you to try to apply your configuration with this change to confirm this is the cause of the problem?

[JakeCooper]

That'll do it! If we just set type: ClusterIP on L:37 then it won't cause the above issue

Do you need me to PR the chart or?

[kolesnikovae]

That would be just great!

[JakeCooper]

GH accidently closed it as I cited my personal PR on my personal repo as closes! Re-opening

[JakeCooper]

PR up @kolesnikovae if you want to have a look

Noting that this is absolutely NOT tested! If you point me to a guide to how to test charts locally, I'm game to test it!

[JakeCooper]

Also, just so we're clear, there's definitely no way in the chart currently to pass annotations to the all service right? I do see a pyroscope.podAnnotations, but something like this absolutely doesn't work

pyroscope:
  pyroscope:
    service:
      type: LoadBalancer
    podAnnotations:
      networking.gke.io/load-balancer-type: "Internal"
      networking.gke.io/internal-load-balancer-allow-global-access: "true"

Still results in

service/pyroscope                        LoadBalancer   10.30.30.245   35.197.38.17   4040:30388/TCP                                  38s
service/pyroscope-agent                  ClusterIP      10.30.31.8     <none>         80/TCP                                          38s
service/pyroscope-agent-cluster          ClusterIP      None           <none>         80/TCP                                          38s
service/pyroscope-memberlist             ClusterIP      None           <none>         7946/TCP                                        38s

[kolesnikovae]

I think you're right. Moreover, I don't think service annotation worked at all.
Here is the fix: #2700

[JakeCooper]

😍 Awesome! Signed the CLA for the PR but the make helm/check command runs but doesn't output anything?

See the #2699