Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only set the loadBalancerClass property when the user sets it to a non-empty value #50

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

rgl
Copy link
Contributor

@rgl rgl commented Apr 26, 2023

Description

only set the loadBalancerClass property when the user sets it to a non-empty value.

this allows us to easily use the cluster default load balancer class.

Why is this needed

this is required to use metallb instead of kube-vip.

here's an usage example:

helm upgrade --install \
  stack \
  . \
  --create-namespace \
  --namespace tink-system \
  --wait \
  --values <(cat <<EOF
stack:
  ...
  lbClass: ""
  kubevip:
    enabled: false
boots:
  service:
    class: ""
  ...
EOF
)

How Has This Been Tested?

It was tested in a k3s cluster using metallb.

How are existing users impacted? What migration steps/scripts do we need?

It does not impact existing users as there are no breaking changes.

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

…n-empty value

this allows us to easily use the cluster default load balancer class

Signed-off-by: Rui Lopes <rgl@ruilopes.com>
@chrisdoherty4
Copy link
Member

Thanks for the contribution.

@chrisdoherty4 chrisdoherty4 added the ready-to-merge Mergify: Ready for Merging label Apr 28, 2023
@mergify mergify bot merged commit c97be7b into tinkerbell:main Apr 28, 2023
@rgl rgl deleted the rgl-lb-class branch May 3, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Mergify: Ready for Merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants