Skip to content

Commit 68b9dce

Browse files
zbb88888zbb88888
authored
fix: handle empty IP and CIDR in GetIPAddrWithMask function (#6002)
fix: log message for skipping IP allocation in NAT gateway Signed-off-by: zbb88888 <jmdxjsjgcxy@gmail.com>
1 parent 21d0ff8 commit 68b9dce

File tree

5 files changed

+134
-21
lines changed

5 files changed

+134
-21
lines changed

‎dist/images/vpcnatgateway/nat-gateway.sh‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,11 @@ function init() {
8787
>&2 echo "Error: Expected two interfaces separated by a comma (e.g., net1,net2)"
8888
exit 1
8989
fi
90-
90+
9191
# Trim any remaining leading and trailing whitespace
9292
VPC_INTERFACE=$(echo "${interface_array[0]}" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')
9393
EXTERNAL_INTERFACE=$(echo "${interface_array[1]}" | sed 's/^[[:space:]]*//;s/[[:space:]]*$//')
94-
94+
9595
# Validate interface names are not empty after trimming
9696
if [ -z "$VPC_INTERFACE" ] || [ -z "$EXTERNAL_INTERFACE" ]; then
9797
>&2 echo "Error: Interface names cannot be empty"
@@ -136,8 +136,16 @@ function init() {
136136
$iptables_cmd -t nat -A SNAT_FILTER -j SHARED_SNAT
137137

138138
# Send gratuitous ARP for all the IPs on the external network interface at initialization
139-
# This is especially useful to update the MAC of the nexthop we announce to the BGP speaker
140-
ip -4 addr show dev $EXTERNAL_INTERFACE | awk '/inet /{print $2}' | cut -d/ -f1 | xargs -n1 arping -I $EXTERNAL_INTERFACE -c 3 -U
139+
# This is especially useful to update the MAC of the nexthop we announce to the BGP speaker
140+
# Only send ARP if there are IP addresses on the external interface (skip in no-IPAM mode)
141+
external_ips=$(ip -4 addr show dev $EXTERNAL_INTERFACE | awk '/inet /{print $2}' | cut -d/ -f1)
142+
if [ -n "$external_ips" ]; then
143+
echo "Sending gratuitous ARP for external IPs on $EXTERNAL_INTERFACE"
144+
echo "$external_ips" | xargs -n1 arping -I $EXTERNAL_INTERFACE -c 3 -U
145+
echo "Gratuitous ARP completed"
146+
else
147+
echo "INFO: No IP addresses on $EXTERNAL_INTERFACE, skipping gratuitous ARP (no-IPAM mode or waiting for EIP allocation)"
148+
fi
141149
}
142150

143151

@@ -195,14 +203,14 @@ function add_eip() {
195203
exec_cmd "ip addr replace $eip dev $EXTERNAL_INTERFACE"
196204
exec_cmd "arping -I $EXTERNAL_INTERFACE -c 3 -U $eip_without_prefix"
197205
done
198-
206+
199207
if [ -n "$GATEWAY_V4" ]; then
200208
exec_cmd "ip route replace default via $GATEWAY_V4 dev $EXTERNAL_INTERFACE"
201209
fi
202-
210+
203211
if [ -n "$GATEWAY_V6" ]; then
204212
exec_cmd "ip -6 route replace default via $GATEWAY_V6 dev $EXTERNAL_INTERFACE"
205-
fi
213+
fi
206214
}
207215

208216
function del_eip() {

‎pkg/controller/vpc_nat_gateway.go‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -358,9 +358,10 @@ func (c *Controller) handleInitVpcNatGw(key string) error {
358358
}
359359
}
360360
if err = c.execNatGwRules(pod, natGwInit, interfaces); err != nil {
361-
err = fmt.Errorf("failed to init vpc nat gateway, %w", err)
362-
klog.Error(err)
363-
return err
361+
// Check if this is a transient initialization error (e.g., first attempt before iptables chains are created)
362+
// The init script may fail on first run but succeed on retry after chains are established
363+
klog.Warningf("vpc nat gateway %s init attempt failed (will retry): %v", key, err)
364+
return fmt.Errorf("failed to init vpc nat gateway, %w", err)
364365
}
365366

366367
if gw.Spec.QoSPolicy != "" {
@@ -731,21 +732,21 @@ func (c *Controller) execNatGwRules(pod *corev1.Pod, operation string, rules []s
731732
}()
732733

733734
cmd := fmt.Sprintf("bash /kube-ovn/nat-gateway.sh %s %s", operation, strings.Join(rules, " "))
734-
klog.V(3).Info(cmd)
735+
klog.V(3).Infof("executing NAT gateway command: %s", cmd)
735736
stdOutput, errOutput, err := util.ExecuteCommandInContainer(c.config.KubeClient, c.config.KubeRestConfig, pod.Namespace, pod.Name, "vpc-nat-gw", []string{"/bin/bash", "-c", cmd}...)
736737
if err != nil {
737738
if len(errOutput) > 0 {
738-
klog.Errorf("failed to ExecuteCommandInContainer, errOutput: %v", errOutput)
739+
klog.Errorf("NAT gateway command failed - stderr: %v", errOutput)
739740
}
740741
if len(stdOutput) > 0 {
741-
klog.V(3).Infof("failed to ExecuteCommandInContainer, stdOutput: %v", stdOutput)
742+
klog.Infof("NAT gateway command failed - stdout: %v", stdOutput)
742743
}
743-
klog.Error(err)
744+
klog.Errorf("NAT gateway command execution error: %v", err)
744745
return err
745746
}
746747

747748
if len(stdOutput) > 0 {
748-
klog.V(3).Infof("ExecuteCommandInContainer stdOutput: %v", stdOutput)
749+
klog.V(3).Infof("NAT gateway command succeeded - stdout: %v", stdOutput)
749750
}
750751

751752
if len(errOutput) > 0 {
@@ -978,12 +979,16 @@ func (c *Controller) genNatGwStatefulSet(gw *kubeovnv1.VpcNatGateway, oldSts *v1
978979
if v6Gateway != "" {
979980
routes = append(routes, request.Route{Destination: "::/0", Gateway: v6Gateway})
980981
}
982+
// TODO:// check NAD if has ipam to disable ipam
981983
if !gw.Spec.NoDefaultEIP {
982984
if err = setPodRoutesAnnotation(annotations, subnet.Spec.Provider, routes); err != nil {
983985
klog.Error(err)
984986
return nil, err
985987
}
986988
} else {
989+
// NAT gateway uses no-IPAM mode in network attachment definition when NoDefaultEIP is enabled
990+
// This allows macvlan/other CNI plugins to work without IP allocation from Kube-OVN
991+
klog.Infof("skipping IP allocation for NAT gateway %s (NoDefaultEIP enabled)", gw.Name)
987992
annotations[fmt.Sprintf(util.AllocatedAnnotationTemplate, subnet.Spec.Provider)] = "true"
988993
}
989994

‎pkg/daemon/handler.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func (csh cniServerHandler) handleAdd(req *restful.Request, resp *restful.Respon
9191
var gatewayCheckMode int
9292
var macAddr, ip, ipAddr, cidr, gw, subnet, ingress, egress, providerNetwork, ifName, nicType, podNicName, vmName, latency, limit, loss, jitter, u2oInterconnectionIP, oldPodName string
9393
var routes []request.Route
94-
var isDefaultRoute bool
94+
var isDefaultRoute, noIPAM bool
9595
var pod *v1.Pod
9696
var err error
9797
for range 20 {
@@ -130,7 +130,7 @@ func (csh cniServerHandler) handleAdd(req *restful.Request, resp *restful.Respon
130130
jitter = pod.Annotations[fmt.Sprintf(util.NetemQosJitterAnnotationTemplate, podRequest.Provider)]
131131
providerNetwork = pod.Annotations[fmt.Sprintf(util.ProviderNetworkTemplate, podRequest.Provider)]
132132
vmName = pod.Annotations[fmt.Sprintf(util.VMAnnotationTemplate, podRequest.Provider)]
133-
ipAddr, err = util.GetIPAddrWithMask(ip, cidr)
133+
ipAddr, noIPAM, err = util.GetIPAddrWithMaskForCNI(ip, cidr)
134134
if err != nil {
135135
errMsg := fmt.Errorf("failed to get ip address with mask, %w", err)
136136
klog.Error(errMsg)
@@ -210,11 +210,13 @@ func (csh cniServerHandler) handleAdd(req *restful.Request, resp *restful.Respon
210210
if subnet == "" && podSubnet != nil {
211211
subnet = podSubnet.Name
212212
}
213-
if err := csh.UpdateIPCR(podRequest, subnet, ip); err != nil {
214-
if err := resp.WriteHeaderAndEntity(http.StatusInternalServerError, request.CniResponse{Err: err.Error()}); err != nil {
215-
klog.Errorf("failed to write response, %v", err)
213+
if !noIPAM {
214+
if err := csh.UpdateIPCR(podRequest, subnet, ip); err != nil {
215+
if err := resp.WriteHeaderAndEntity(http.StatusInternalServerError, request.CniResponse{Err: err.Error()}); err != nil {
216+
klog.Errorf("failed to write response, %v", err)
217+
}
218+
return
216219
}
217-
return
218220
}
219221

220222
if isDefaultRoute && pod.Annotations[fmt.Sprintf(util.RoutedAnnotationTemplate, podRequest.Provider)] != "true" && util.IsOvnProvider(podRequest.Provider) {

‎pkg/util/net.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,20 @@ func GetStringIP(v4IP, v6IP string) string {
334334
return strings.Join(ipList, ",")
335335
}
336336

337+
// GetIPAddrWithMaskForCNI returns IP address with mask for CNI plugin.
338+
// When ip is empty, it indicates no-IPAM mode (e.g., NAT gateway macvlan without default EIP).
339+
// Returns (ipAddr, noIPAM, error) where noIPAM is true when IP allocation is skipped.
340+
func GetIPAddrWithMaskForCNI(ip, cidr string) (string, bool, error) {
341+
if ip == "" {
342+
// Network attachment definition using no-IPAM plugin (e.g., NAT gateway net1 macvlan with no default EIP)
343+
// IP is not allocated by Kube-OVN, but cidr still comes from subnet configuration
344+
klog.V(3).Infof("skipping IP allocation: ip is empty for cidr %s (no-IPAM mode)", cidr)
345+
return "", true, nil
346+
}
347+
ipAddr, err := GetIPAddrWithMask(ip, cidr)
348+
return ipAddr, false, err
349+
}
350+
337351
func GetIPAddrWithMask(ip, cidr string) (string, error) {
338352
var ipAddr string
339353
ips := strings.Split(ip, ",")

‎pkg/util/net_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,90 @@ func TestGetIPAddrWithMask(t *testing.T) {
937937
}
938938
}
939939

940+
func TestGetIPAddrWithMaskForCNI(t *testing.T) {
941+
tests := []struct {
942+
name string
943+
ip string
944+
cidr string
945+
wantAddr string
946+
wantNoIP bool
947+
wantError bool
948+
}{
949+
{
950+
name: "Normal IPv4 address",
951+
ip: "192.168.1.1",
952+
cidr: "192.168.1.0/24",
953+
wantAddr: "192.168.1.1/24",
954+
wantNoIP: false,
955+
wantError: false,
956+
},
957+
{
958+
name: "Normal IPv6 address",
959+
ip: "2001:db8::1",
960+
cidr: "2001:db8::/32",
961+
wantAddr: "2001:db8::1/32",
962+
wantNoIP: false,
963+
wantError: false,
964+
},
965+
{
966+
name: "Dual stack addresses",
967+
ip: "192.168.1.1,2001:db8::1",
968+
cidr: "192.168.1.0/24,2001:db8::/32",
969+
wantAddr: "192.168.1.1/24,2001:db8::1/32",
970+
wantNoIP: false,
971+
wantError: false,
972+
},
973+
{
974+
name: "No-IPAM mode (empty IP with CIDR)",
975+
ip: "",
976+
cidr: "192.168.1.0/24",
977+
wantAddr: "",
978+
wantNoIP: true,
979+
wantError: false,
980+
},
981+
{
982+
name: "No-IPAM mode (empty IP with dual-stack CIDR)",
983+
ip: "",
984+
cidr: "192.168.1.0/24,2001:db8::/32",
985+
wantAddr: "",
986+
wantNoIP: true,
987+
wantError: false,
988+
},
989+
{
990+
name: "Error: invalid dual stack ip format",
991+
ip: "192.168.1.1",
992+
cidr: "192.168.1.0/24,2001:db8::/32",
993+
wantAddr: "",
994+
wantNoIP: false,
995+
wantError: true,
996+
},
997+
}
998+
999+
for _, tt := range tests {
1000+
t.Run(tt.name, func(t *testing.T) {
1001+
gotAddr, gotNoIP, gotErr := GetIPAddrWithMaskForCNI(tt.ip, tt.cidr)
1002+
1003+
if tt.wantError {
1004+
if gotErr == nil {
1005+
t.Errorf("GetIPAddrWithMaskForCNI() expected error but got nil")
1006+
}
1007+
} else {
1008+
if gotErr != nil {
1009+
t.Errorf("GetIPAddrWithMaskForCNI() unexpected error: %v", gotErr)
1010+
}
1011+
}
1012+
1013+
if gotAddr != tt.wantAddr {
1014+
t.Errorf("GetIPAddrWithMaskForCNI() gotAddr = %v, want %v", gotAddr, tt.wantAddr)
1015+
}
1016+
1017+
if gotNoIP != tt.wantNoIP {
1018+
t.Errorf("GetIPAddrWithMaskForCNI() gotNoIP = %v, want %v", gotNoIP, tt.wantNoIP)
1019+
}
1020+
})
1021+
}
1022+
}
1023+
9401024
func TestGetIPWithoutMask(t *testing.T) {
9411025
tests := []struct {
9421026
name string

0 commit comments

Comments
 (0)