Skip to content

Fix logic for service type and loadBalancerIP rendering#1

Merged
AlePini merged 3 commits intosim500:mainfrom
JakobStadlhuber:main
Apr 10, 2025
Merged

Fix logic for service type and loadBalancerIP rendering#1
AlePini merged 3 commits intosim500:mainfrom
JakobStadlhuber:main

Conversation

@JakobStadlhuber
Copy link

Reverse incorrect conditions for service.ingress.enabled. Ensures that service type and loadBalancerIP are appropriately configured based on the ingress setting, aligning with the expected configuration behavior.

Reverse incorrect conditions for service.ingress.enabled. Ensures that service type and loadBalancerIP are appropriately configured based on the ingress setting, aligning with the expected configuration behavior.
@JakobStadlhuber
Copy link
Author

@sim500 could you also please merge this back in your current open pr of the offical repo

@sim500
Copy link
Owner

sim500 commented Apr 9, 2025

Thank you @JakobStadlhuber . I assigned @AlePini to review this PR and eventual others PRs to the original project. Since the majority of refactoring has been done by him.

@AlePini
Copy link
Collaborator

AlePini commented Apr 10, 2025

Hi @JakobStadlhuber, thanks for you contribution!

I'm not quite sure about your issue here: when you want to access your application via Ingress, the corresponding service should be ClusterIP. Load balancing is already handled by the Ingress-Controller.

I think a better fix for this is to set service type based on passed values:

...
spec:
  type: {{ .Values.service.type }}
  
  {{- if eq .Values.service.type "LoadBalancer" }}
  loadBalancerIP: {{ .Values.global.externalIP }}
  externalTrafficPolicy: Cluster
  {{- end }}
...

Let me know it this works for you :)

@JakobStadlhuber
Copy link
Author

@AlePini that works too. I will change the PR. thank you for your contribution.

Updated the service template to use `.Values.service.type` directly instead of a ternary operation. This change ensures better flexibility and aligns with user-defined configurations, particularly for LoadBalancer settings.
@AlePini AlePini merged commit dbcce9b into sim500:main Apr 10, 2025
@AlePini
Copy link
Collaborator

AlePini commented Apr 10, 2025

@JakobStadlhuber I'll also add this change to this PR hoppscotch/pull/13

I hope they will merge these changes quickly 😅

AlePini pushed a commit that referenced this pull request Apr 10, 2025
Updated the service template to use `.Values.service.type` directly instead of a ternary operation. This change ensures better flexibility and aligns with user-defined configurations, particularly for LoadBalancer settings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants