kubernetes / cloud-provider-openstack

Apache License 2.0
599 stars 597 forks source link

[occm] Support for `spec.loadBalancerClass`? #2598

Closed sylvainOL closed 3 weeks ago

sylvainOL commented 1 month ago

Is this a BUG REPORT or FEATURE REQUEST?:

/kind feature

What happened:

After reading code and documentation, I'm not sure OCCM supports spec.loadBalancerClass from services

What you expected to happen:

create a service with spec.loadBalancerClass set to one of OCCM configured loadBalancerClass.

OCCM picks it and use it.

Anything else we need to know?:

it would help using OCCM in parallel with other load balancers

dulek commented 1 month ago

This isn't what this feature is about, even if we have a naming conflict here. The feature you link is about skipping cloud provider altogether when loadBalancerClass is set to allow some other controller to handle the Service. I'm fairly sure it's implemented in the generic cloud-provider code. The loadbalancer.openstack.org/class on the OCCM level is a separate, OCCM-specific entity and will need to remain in annotations.

We could add some docs to clarify this, so I'll leave this open.

dulek commented 1 month ago

Yup, on the functional level this is solved, if you set loadBalancerClass, the Service will be ignored by the OCCM: https://github.com/kubernetes/cloud-provider/blob/d2f5e75a5fd6d31f75b1519fe20879b1ab5347b8/controllers/service/controller.go#L876-L879

sylvainOL commented 1 month ago

Hi @dulek,

first thanks for quick reply.

That's what I thought.

What I'd actually like is that OCCM could leverage the spec.loadBalancerClass so it does answer only to its own request (the one matching loadBalancerClass and not all of them / the one without loadBalancerClass) like metallb can do for example (https://metallb.universe.tf/installation/)

dulek commented 1 month ago

Hi @dulek,

first thanks for quick reply.

That's what I thought.

What I'd actually like is that OCCM could leverage the spec.loadBalancerClass so it does answer only to its own request (the one matching loadBalancerClass and not all of them / the one without loadBalancerClass) like metallb can do for example (https://metallb.universe.tf/installation/)

Oh, okay, I get it. Well, that change would need to be made in kubernetes/cloud-provider and not here. Currently we directly use the controller from that project and the code I linked in the previous comment is making sure we ignore Services with loadBalancerClass set. I can imagine cloud providers accepting --load-balancer-class parameter and comparing with it if it's set.

Anyway to do it solely in OCCM would be pretty hard - you'd need to basically fork the original controller.

sylvainOL commented 1 month ago

oki understood.

I'll take a look on the parent repo.

thanks again!

zetaab commented 3 weeks ago

/close

k8s-ci-robot commented 3 weeks ago

@zetaab: Closing this issue.

In response to [this](https://github.com/kubernetes/cloud-provider-openstack/issues/2598#issuecomment-2161354141): >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.