Skip to content

Commit 8981e0e

Browse files
Shilpa-GokulShilpa-Gokul
committed
Addressed review comments
1 parent 937f5b2 commit 8981e0e

7 files changed

+25
-23
lines changed

‎api/v1beta2/ibmvpccluster_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ type AdditionalListenerSpec struct {
128128
// +optional
129129
Protocol *VPCLoadBalancerListenerProtocol `json:"protocol,omitempty"`
130130

131-
// Selector is used to select the machines with same label to assign the listener
131+
// selector is used to select the machines with same label to assign the listener
132132
// +kubebuilder:validation:Optional
133133
Selector metav1.LabelSelector `json:"selector,omitempty"`
134134
}

‎cloud/scope/powervs_machine.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -998,30 +998,35 @@ func (m *PowerVSMachineScope) CreateVPCLoadBalancerPoolMember(ctx context.Contex
998998

999999
internalIP := m.GetMachineInternalIP()
10001000

1001-
// TODO:SHILPA- handle multiple lbs as well
1002-
// Update each LoadBalancer pool
1003-
loadBalancerListeners := map[string]infrav1beta2.AdditionalListenerSpec{}
1001+
// lbAdditionalListeners is a mapping of additionalListener's port-protocol to the additionalListener as defined in the specification
1002+
// It will be used later to get the default pool associated with the listener
1003+
lbAdditionalListeners := map[string]infrav1beta2.AdditionalListenerSpec{}
10041004
for _, additionalListener := range lb.AdditionalListeners {
1005-
// TODO:SHILPA- protocol is added irrespective of whats provided in the additionalListener protocol, need to handle this
10061005
if additionalListener.Protocol == nil {
10071006
additionalListener.Protocol = &infrav1beta2.VPCLoadBalancerListenerProtocolTCP
10081007
}
1009-
loadBalancerListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)] = additionalListener
1008+
lbAdditionalListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)] = additionalListener
10101009
}
1010+
1011+
// loadBalancerListeners is a mapping of the loadBalancer listener's defaultPoolName to the additionalListener
1012+
// as the default pool name might be empty in spec and should be fetched from the cloud's listener
1013+
loadBalancerListeners := map[string]infrav1beta2.AdditionalListenerSpec{}
10111014
for _, listener := range loadBalancer.Listeners {
10121015
listenerOptions := &vpcv1.GetLoadBalancerListenerOptions{}
10131016
listenerOptions.SetLoadBalancerID(*loadBalancer.ID)
10141017
listenerOptions.SetID(*listener.ID)
10151018
loadBalancerListener, _, err := m.IBMVPCClient.GetLoadBalancerListener(listenerOptions)
10161019
if err != nil {
1017-
return nil, fmt.Errorf("failed to list %s load balancer listener: %v", *listener.ID, err)
1020+
return nil, fmt.Errorf("failed to list %s load balancer listener: %w", *listener.ID, err)
10181021
}
1019-
if additionalListener, ok := loadBalancerListeners[fmt.Sprintf("%d-%s", *loadBalancerListener.Port, *loadBalancerListener.Protocol)]; ok {
1022+
if additionalListener, ok := lbAdditionalListeners[fmt.Sprintf("%d-%s", *loadBalancerListener.Port, *loadBalancerListener.Protocol)]; ok {
10201023
if loadBalancerListener.DefaultPool != nil {
10211024
loadBalancerListeners[*loadBalancerListener.DefaultPool.Name] = additionalListener
10221025
}
10231026
}
10241027
}
1028+
// Update each LoadBalancer pool
1029+
// For each pool, get the additionalListener associated with the pool from the loadBalancerListeners map.
10251030
for _, pool := range loadBalancer.Pools {
10261031
log.V(3).Info("Updating LoadBalancer pool member", "pool", *pool.Name, "loadBalancerName", *loadBalancer.Name, "IP", internalIP)
10271032
listOptions := &vpcv1.ListLoadBalancerPoolMembersOptions{}
@@ -1032,13 +1037,14 @@ func (m *PowerVSMachineScope) CreateVPCLoadBalancerPoolMember(ctx context.Contex
10321037
return nil, fmt.Errorf("failed to list %s VPC load balancer pool: %w", *pool.Name, err)
10331038
}
10341039
var targetPort int64
1035-
var alreadyRegistered, skipListener bool
1040+
var alreadyRegistered bool
10361041

10371042
if loadBalancerListener, ok := loadBalancerListeners[*pool.Name]; ok {
10381043
targetPort = loadBalancerListener.Port
1044+
log.V(3).Info("Label selector for configuring additional listener is %+v", loadBalancerListener.Selector)
10391045
selector, err := metav1.LabelSelectorAsSelector(&loadBalancerListener.Selector)
10401046
if err != nil {
1041-
log.V(5).Info("Skipping listener addition, failed to get label selector from spec selector")
1047+
log.V(5).Error(err, "Skipping listener addition, failed to get label selector from spec selector")
10421048
continue
10431049
}
10441050

@@ -1047,13 +1053,10 @@ func (m *PowerVSMachineScope) CreateVPCLoadBalancerPoolMember(ctx context.Contex
10471053
}
10481054
// Skip adding the listener if the selector does not match
10491055
if !selector.Empty() && !selector.Matches(labels.Set(m.IBMPowerVSMachine.Labels)) {
1050-
skipListener = true
1056+
log.V(3).Info("Skip adding listener, machine label doesn't match with the listener label selector", "pool", *pool.Name, "IP", internalIP)
1057+
continue
10511058
}
10521059
}
1053-
if skipListener {
1054-
log.V(3).Info("Skip adding listener, machine label doesn't match with the listener label selector", "pool", *pool.Name, "targetip", internalIP, "machine", m.IBMPowerVSMachine.Name, "clusterName", m.IBMPowerVSCluster.Name)
1055-
continue
1056-
}
10571060

10581061
for _, member := range listLoadBalancerPoolMembers.Members {
10591062
if target, ok := member.Target.(*vpcv1.LoadBalancerPoolMemberTarget); ok {

‎config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ spec:
302302
- udp
303303
type: string
304304
selector:
305-
description: Selector is used to select the machines with
305+
description: selector is used to select the machines with
306306
same label to assign the listener
307307
properties:
308308
matchExpressions:

‎config/crd/bases/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ spec:
337337
- udp
338338
type: string
339339
selector:
340-
description: Selector is used to select the machines
340+
description: selector is used to select the machines
341341
with same label to assign the listener
342342
properties:
343343
matchExpressions:

‎config/crd/bases/infrastructure.cluster.x-k8s.io_ibmvpcclusters.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ spec:
294294
- udp
295295
type: string
296296
selector:
297-
description: Selector is used to select the machines with
297+
description: selector is used to select the machines with
298298
same label to assign the listener
299299
properties:
300300
matchExpressions:
@@ -614,7 +614,7 @@ spec:
614614
- udp
615615
type: string
616616
selector:
617-
description: Selector is used to select the machines
617+
description: selector is used to select the machines
618618
with same label to assign the listener
619619
properties:
620620
matchExpressions:

‎config/crd/bases/infrastructure.cluster.x-k8s.io_ibmvpcclustertemplates.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ spec:
137137
- udp
138138
type: string
139139
selector:
140-
description: Selector is used to select the machines
140+
description: selector is used to select the machines
141141
with same label to assign the listener
142142
properties:
143143
matchExpressions:
@@ -466,7 +466,7 @@ spec:
466466
- udp
467467
type: string
468468
selector:
469-
description: Selector is used to select the
469+
description: selector is used to select the
470470
machines with same label to assign the listener
471471
properties:
472472
matchExpressions:

‎internal/webhooks/ibmpowervscluster.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,10 +254,9 @@ func validateAdditionalListenerSelector(newCluster, oldCluster *infrav1beta2.IBM
254254
newLoadBalancerListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)] = additionalListener.Selector
255255
}
256256
}
257-
fmt.Println(oldCluster.Spec.LoadBalancers)
258257
for _, loadbalancer := range oldCluster.Spec.LoadBalancers {
259258
for _, additionalListener := range loadbalancer.AdditionalListeners {
260-
if _, ok := newLoadBalancerListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)]; ok && !reflect.DeepEqual(newLoadBalancerListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)], additionalListener.Selector) {
259+
if selector, ok := newLoadBalancerListeners[fmt.Sprintf("%d-%s", additionalListener.Port, *additionalListener.Protocol)]; ok && !reflect.DeepEqual(selector, additionalListener.Selector) {
261260
allErrs = append(allErrs, field.Forbidden(field.NewPath("selector"), "Selector is immutable"))
262261
}
263262
}

0 commit comments

Comments
 (0)