[grafana] Added support for loadBalancerClass

[Sheikh-Abubaker]

Fixes #2717

[zanhsieh]

@Sheikh-Abubaker
I believe you should come out service type equals to LoadBalancer case. If service type is not LoadBalancer then all subsequent attributes / stanzas (e.g. LoadBalancerSourceRanges) shall be hide. Also, you should consider LoadBalancer auto-assign case since the user has no idea he would need to hardcode the IP ranges / load balancer class in order to work.

[Sheikh-Abubaker]

@Sheikh-Abubaker I believe you should come out service type equals to LoadBalancer case.

@zanhsieh I remember some weeks ago when I did changed service type to LoadBalancer you forbidden me to do it as it doesn't support most of the use case and due to that the CI lint test also kept failing.

[zanhsieh]

@zanhsieh I remember some weeks ago when I did changed service type to LoadBalancer you forbidden me to do it as it doesn't support most of the use case and due to that the CI lint test also kept failing.

@Sheikh-Abubaker Yes, but that does not stop you to put your implementation in the templates/services.yaml. The current implementation is half-mature.

[Sheikh-Abubaker]

Also, you should consider LoadBalancer auto-assign case since the user has no idea he would need to hardcode the IP ranges / load balancer class in order to work.

@zanhsieh have a look at this is this what you meant ?

[Sheikh-Abubaker]

@Sheikh-Abubaker Yes, but that does not stop you to put your implementation in the templates/services.yaml. The current implementation is half-mature.

@zanhsieh I did that but the lint test is failing again as before.

[zanhsieh]

Also, you should consider LoadBalancer auto-assign case since the user has no idea he would need to hardcode the IP ranges / load balancer class in order to work.

@zanhsieh have a look at this is this what you meant ?

No. You shall put something on template/service.yaml as once service type is LoadBalancer, the related values shall show up; otherwise all of LoadBalancer related values shall be automatically hidden.

[Sheikh-Abubaker]

I believe you should come out service type equals to LoadBalancer case.

check line 8

If service type is not LoadBalancer then all subsequent attributes / stanzas (e.g. LoadBalancerSourceRanges) shall be hide

check line 9-18 to hide the atrributes I've adjusted the indentation.

Also, you should consider LoadBalancer auto-assign case since the user has no idea he would need to hardcode the IP ranges / load balancer class in order to work.

check line 7, I've added (empty .Values.service.type) in the same line to auto assign LoadBalancer even if the service is empty

@zanhsieh Have a look, does the below image fulfills all the things you mentioned above?

If I did not get it correct even this time, can you please explain the issue in simple terms and the fundamentals I need to study which would help me in contributing to this issue, I am just a beginner looking forward to learn.

[Sheikh-Abubaker]

@zanhsieh is everything all right about this PR ?

[zanhsieh]

@Sheikh-Abubaker
Everything is ok. Just currently Grafana.com, the owner of this community repo, now required two approvals to get this PR merge.

[Sheikh-Abubaker]

@Sheikh-Abubaker Everything is ok. Just currently Grafana.com, the owner of this community repo, now required two approvals to get this PR merge.

Thank you so much @zanhsieh for being patient and guiding me through this issue, I've learned a lot about helm and k8s while being contributing to this issue, looking forward to learn and contribute to more issues under the guidance of a maintainer like you, once again thank you!!