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

alibabacloud slb support map same TCP and UDP port , eg 8000/TCPUDP #197

Merged
merged 6 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions cloudprovider/alibabacloud/slb.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,16 @@ func initLbCache(svcList []corev1.Service, minPort, maxPort int32, blockPorts []
var ports []int32
for _, port := range getPorts(svc.Spec.Ports) {
if port <= maxPort && port >= minPort {
newCache[lbId][port] = true
ports = append(ports, port)
value, ok := newCache[lbId][port]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot redeploy the service that not controlled by kruise-game but use same port.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cache just save lbId + port when design, it has no Protocol Attribute;

when use 8000/TCPUDP, cache just lbid:8000 and map to tcp:8000 udp:8000 two svc items;

when reinit cache , list tcp:8000 and udp:8000 should repeat to lbid:8000

else when 8000/TCPUDP, 9000/TCP will map error ....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here svc all filter by SlbIdLabelKey , all controlled by kruise-game ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is a problem to set map true for two times. And the svc using SlbIdLabelKey may be deployed by user self, not all controlled by kruise-game.

Copy link
Contributor Author

@gaopeiliang gaopeiliang Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. set cache map true two times has no problem , but append podAllocate two times maybe problem; eg: when config 8000/TCPUDP,9000/TCP , first create podAllocate is lb:8000,9000 , when reinit podAllocate with append two times, result is lb:8000,8000,9000, it will be error when resync svc;

  2. filter all svc by SlbIdLabelKey manager by kruise-game or not, record lb port to cache avoid allocated; I think is right , podAllocate svc item not control by kruise-game never used ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got your point

if !ok || !value {
newCache[lbId][port] = true
ports = append(ports, port)
}
}
}
if len(ports) != 0 {
newPodAllocate[svc.GetNamespace()+"/"+svc.GetName()] = lbId + ":" + util.Int32SliceToString(ports, ",")
log.Infof("svc %s/%s allocate slb %s ports %v", svc.Namespace, svc.Name, lbId, ports)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The log is unnecessary, cause podAllocate will be printed when finished the initialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes ,,,

}
}
}
Expand Down Expand Up @@ -577,12 +581,29 @@ func (s *SlbPlugin) consSvc(sc *slbConfig, pod *corev1.Pod, c client.Client, ctx

svcPorts := make([]corev1.ServicePort, 0)
for i := 0; i < len(sc.targetPorts); i++ {
svcPorts = append(svcPorts, corev1.ServicePort{
Name: strconv.Itoa(sc.targetPorts[i]),
Port: ports[i],
Protocol: sc.protocols[i],
TargetPort: intstr.FromInt(sc.targetPorts[i]),
})
if sc.protocols[i] == ProtocolTCPUDP {
svcPorts = append(svcPorts, corev1.ServicePort{
Name: fmt.Sprintf("%s-%s", strconv.Itoa(sc.targetPorts[i]), corev1.ProtocolTCP),
Port: ports[i],
Protocol: corev1.ProtocolTCP,
TargetPort: intstr.FromInt(sc.targetPorts[i]),
})

svcPorts = append(svcPorts, corev1.ServicePort{
Name: fmt.Sprintf("%s-%s", strconv.Itoa(sc.targetPorts[i]), corev1.ProtocolUDP),
Port: ports[i],
Protocol: corev1.ProtocolUDP,
TargetPort: intstr.FromInt(sc.targetPorts[i]),
})

} else {
svcPorts = append(svcPorts, corev1.ServicePort{
Name: fmt.Sprintf("%s-%s", strconv.Itoa(sc.targetPorts[i]), sc.protocols[i]),
Port: ports[i],
Protocol: sc.protocols[i],
TargetPort: intstr.FromInt(sc.targetPorts[i]),
})
}
}

svcAnnotations := map[string]string{
Expand Down
2 changes: 1 addition & 1 deletion docs/en/user_manuals/network.md
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ SlbIds
PortProtocols

- Meaning: the ports in the pod to be exposed and the protocols. You can specify multiple ports and protocols.
- Value: in the format of port1/protocol1,port2/protocol2,... The protocol names must be in uppercase letters.
- Value: in the format of port1/protocol1,port2/protocol2,... (same protocol port should like 8000/TCPUDP) The protocol names must be in uppercase letters.
- Configuration change supported or not: yes.

Fixed
Expand Down
2 changes: 1 addition & 1 deletion docs/中文/用户手册/网络模型.md
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ SlbIds
PortProtocols

- 含义:pod暴露的端口及协议,支持填写多个端口/协议
- 格式:port1/protocol1,port2/protocol2,...(协议需大写)
- 格式:port1/protocol1,port2/protocol2,... (多协议相同端口 比如: 8000/TCPUDP)(协议需大写)
- 是否支持变更:支持

Fixed
Expand Down
Loading