Skip to content

Commit 699f129

Browse files
committed
Address review feedback from kayrus
- Remove duplicate clusterUID field from the OpenStack struct so the identifier lives only on the LoadBalancer struct that actually uses it (better cohesion). - Drop fetchClusterUID() out of Initialize() and call it lazily inside the LoadBalancer() factory instead. Clusters that disable LB now skip the kube-system namespace lookup entirely, and the change touches only the LB construction path. - Remove the unused eventLBStolen constant. It was a leftover from an earlier draft that intended to emit a Warning event from getLoadbalancerByName(); plumbing the eventRecorder + *v1.Service into that free-standing function felt out of scope, so the emission was dropped but the constant was left behind.
1 parent ce06ef6 commit 699f129

2 files changed

Lines changed: 2 additions & 13 deletions

File tree

pkg/openstack/loadbalancer.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,6 @@ const (
112112
// OCCM to disambiguate load balancers when multiple Kubernetes clusters
113113
// share the same OpenStack project and a service name happens to collide.
114114
clusterIDTagPrefix = "kube_cluster_id_"
115-
116-
// eventLBStolen is emitted when getLoadbalancerByName finds a load
117-
// balancer with a matching name that belongs to a different Kubernetes
118-
// cluster (different cluster-id tag). The lookup is treated as NotFound
119-
// so OCCM creates a new load balancer instead of stealing an existing one.
120-
eventLBStolen = "LoadBalancerNameCollision"
121115
)
122116

123117
// clusterIDTag formats the Octavia load balancer tag carrying the cluster

pkg/openstack/openstack.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,6 @@ type OpenStack struct {
167167
kclient kubernetes.Interface
168168
nodeInformer coreinformers.NodeInformer
169169
nodeInformerHasSynced func() bool
170-
// clusterUID is the kube-system namespace UID, used as a stable cluster
171-
// identifier on OpenStack load balancer tags. May be empty if the lookup
172-
// failed or RBAC does not allow it; in that case OCCM falls back to the
173-
// legacy name-based load balancer identification.
174-
clusterUID string
175170

176171
eventBroadcaster record.EventBroadcaster
177172
eventRecorder record.EventRecorder
@@ -211,7 +206,6 @@ func (os *OpenStack) Initialize(clientBuilder cloudprovider.ControllerClientBuil
211206
os.eventBroadcaster = record.NewBroadcaster()
212207
os.eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: os.kclient.CoreV1().Events("")})
213208
os.eventRecorder = os.eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "cloud-provider-openstack"})
214-
os.clusterUID = fetchClusterUID(clientset)
215209
}
216210

217211
// fetchClusterUID returns the UID of the kube-system namespace, which is a
@@ -405,7 +399,8 @@ func (os *OpenStack) LoadBalancer() (cloudprovider.LoadBalancer, bool) {
405399

406400
klog.V(1).Info("Claiming to support LoadBalancer")
407401

408-
return &LbaasV2{LoadBalancer{secret, network, lb, os.lbOpts, os.kclient, os.eventRecorder, os.clusterUID}}, true
402+
clusterUID := fetchClusterUID(os.kclient)
403+
return &LbaasV2{LoadBalancer{secret, network, lb, os.lbOpts, os.kclient, os.eventRecorder, clusterUID}}, true
409404
}
410405

411406
// Zones indicates that we support zones

0 commit comments

Comments
 (0)