From 44229a224025cc98372efba90dcb1200a3bac98e Mon Sep 17 00:00:00 2001 From: bobz965 Date: Mon, 5 Feb 2024 17:15:11 +0800 Subject: [PATCH 1/6] fix some ip can not allocate after released Signed-off-by: bobz965 --- pkg/controller/init.go | 12 +++ pkg/controller/ip.go | 8 +- pkg/controller/pod.go | 7 +- pkg/controller/subnet.go | 203 +++++++++++++++++++++++++++++------- pkg/controller/vpc.go | 2 + pkg/ipam/ip.go | 6 ++ pkg/ipam/ipam.go | 8 ++ pkg/ipam/subnet.go | 11 ++ pkg/ovs/ovn-nbctl-legacy.go | 18 +++- 9 files changed, 227 insertions(+), 48 deletions(-) diff --git a/pkg/controller/init.go b/pkg/controller/init.go index b43b8e05217..17df1d04139 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -334,6 +334,13 @@ func (c *Controller) InitIPAM() error { continue } + // retrigger pod update to delete pod in case of kube-ovn-controller crashed while pod is deleting + podKey := fmt.Sprintf("%s/%s", pod.Namespace, pod.Name) + if pod.DeletionTimestamp != nil { + klog.Infof("enqueue update for deleting pod %s", podKey) + c.updatePodQueue.Add(podKey) + } + podNets, err := c.getPodKubeovnNets(pod) if err != nil { klog.Errorf("failed to get pod kubeovn nets %s.%s address %s: %v", pod.Name, pod.Namespace, pod.Annotations[util.IpAddressAnnotation], err) @@ -569,6 +576,11 @@ func (c *Controller) initSyncCrdIPs() error { for _, ipCr := range ips.Items { ip := ipCr.DeepCopy() + // retrigger ip update to delete ip in case of kube-ovn-controller crashed while ip is deleting + if ip.DeletionTimestamp != nil && util.ContainsString(ip.Finalizers, util.ControllerName) { + klog.Infof("enqueue update for deleting ip %s", ip.Name) + c.updateIPQueue.Add(ip.Name) + } changed := false if _, ok := ipMap[ip.Name]; ok && ip.Spec.PodType == "" { ip.Spec.PodType = util.Vm diff --git a/pkg/controller/ip.go b/pkg/controller/ip.go index 976ac074745..e008695fb3b 100644 --- a/pkg/controller/ip.go +++ b/pkg/controller/ip.go @@ -249,8 +249,9 @@ func (c *Controller) handleUpdateIP(key string) error { } } if cleanIPAM { - klog.V(3).Infof("release ipam for deleted ip %s from subnet %s", cachedIP.Name, cachedIP.Spec.Subnet) - c.ipam.ReleaseAddressByPod(cachedIP.Name, cachedIP.Spec.Subnet) + podKey := fmt.Sprintf("%s/%s", cachedIP.Spec.Namespace, cachedIP.Spec.PodName) + klog.Infof("ip cr %s release ipam pod key %s from subnet %s", cachedIP.Name, podKey, cachedIP.Spec.Subnet) + c.ipam.ReleaseAddressByPod(podKey, cachedIP.Spec.Subnet) } if err = c.handleDelIPFinalizer(cachedIP, util.ControllerName); err != nil { klog.Errorf("failed to handle del ip finalizer %v", err) @@ -261,8 +262,7 @@ func (c *Controller) handleUpdateIP(key string) error { } func (c *Controller) handleDelIP(ip *kubeovnv1.IP) error { - klog.V(3).Infof("handle delete ip %s", ip.Name) - klog.V(3).Infof("enqueue update status subnet %s", ip.Spec.Subnet) + klog.Infof("deleting ip %s enqueue update status subnet %s", ip.Name, ip.Spec.Subnet) c.updateSubnetStatusQueue.Add(ip.Spec.Subnet) for _, as := range ip.Spec.AttachSubnets { klog.V(3).Infof("enqueue update attach status for subnet %s", as) diff --git a/pkg/controller/pod.go b/pkg/controller/pod.go index c0b3d00a881..b6b7f2da13d 100644 --- a/pkg/controller/pod.go +++ b/pkg/controller/pod.go @@ -1534,6 +1534,7 @@ func appendCheckPodToDel(c *Controller, pod *v1.Pod, ownerRefName, ownerRefKind klog.Errorf("failed to get namespace %s, %v", pod.Namespace, err) return false, err } + key := fmt.Sprintf("%s/%s", pod.Namespace, pod.Name) // check if subnet exist in OwnerReference var ownerRefSubnetExist bool @@ -1575,7 +1576,7 @@ func appendCheckPodToDel(c *Controller, pod *v1.Pod, ownerRefName, ownerRefKind nsSubnetNames := podNs.Annotations[util.LogicalSwitchAnnotation] // check if pod use the subnet of its ns if nsSubnetNames != "" && pod.Annotations[util.LogicalSwitchAnnotation] != "" && !util.ContainsString(strings.Split(nsSubnetNames, ","), strings.TrimSpace(pod.Annotations[util.LogicalSwitchAnnotation])) { - klog.Infof("ns %s annotation subnet is %s, which is inconstant with subnet for pod %s, delete pod", pod.Namespace, podNs.Annotations[util.LogicalSwitchAnnotation], pod.Name) + klog.Infof("ns %s annotation subnet is %s, which is inconstant with subnet for pod %s, delete pod", pod.Namespace, podNs.Annotations[util.LogicalSwitchAnnotation], key) return true, nil } } @@ -1587,12 +1588,12 @@ func appendCheckPodToDel(c *Controller, pod *v1.Pod, ownerRefName, ownerRefKind return false, err } if podSubnet != nil && !util.CIDRContainIP(podSubnet.Spec.CIDRBlock, pod.Annotations[util.IpAddressAnnotation]) { - klog.Infof("pod's ip %s is not in the range of subnet %s, delete pod", pod.Annotations[util.IpAddressAnnotation], podSubnet.Name) + klog.Infof("pod %s ip %s is not in the range of subnet %s, delete pod", key, pod.Annotations[util.IpAddressAnnotation], podSubnet.Name) return true, nil } // subnet of ownerReference(sts/vm) has been changed, it needs to handle delete pod and create port on the new logical switch if podSubnet != nil && ownerRefSubnet != "" && podSubnet.Name != ownerRefSubnet { - klog.Infof("Subnet of owner %s has been changed from %s to %s, delete pod %s/%s", ownerRefName, podSubnet.Name, ownerRefSubnet, pod.Namespace, pod.Name) + klog.Infof("Subnet of owner %s has been changed from %s to %s, delete pod %s", ownerRefName, podSubnet.Name, ownerRefSubnet, key) return true, nil } diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index f66a6ed1d1a..4a6370a13b1 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -284,6 +284,7 @@ func formatSubnet(subnet *kubeovnv1.Subnet, c *Controller) error { changed, err = checkSubnetChanged(subnet) if err != nil { + klog.Error(err) return err } if subnet.Spec.Provider == "" { @@ -299,7 +300,7 @@ func formatSubnet(subnet *kubeovnv1.Subnet, c *Controller) error { subnet.Spec.GatewayType = kubeovnv1.GWDistributedType changed = true } - if subnet.Spec.Vpc == "" { + if subnet.Spec.Vpc == "" && isOvnSubnet(subnet) { changed = true subnet.Spec.Vpc = util.DefaultVpc @@ -339,14 +340,16 @@ func checkSubnetChanged(subnet *kubeovnv1.Subnet) (bool, error) { // changed value may be overlapped, so use ret to record value changed, err = checkAndUpdateCIDR(subnet) if err != nil { - return changed, err + klog.Error(err) + return false, err } if changed { ret = true } changed, err = checkAndUpdateGateway(subnet) if err != nil { - return changed, err + klog.Error(err) + return false, err } if changed { ret = true @@ -427,7 +430,15 @@ func checkAndUpdateExcludeIps(subnet *kubeovnv1.Subnet) bool { return changed } -func (c *Controller) handleSubnetFinalizer(subnet *kubeovnv1.Subnet) (bool, error) { +func (c *Controller) handleSubnetFinalizer(key string) (bool, error) { + subnet, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Get(context.Background(), key, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + return true, nil + } + klog.Errorf("failed to get subnet %s error %v", key, err) + return false, err + } if subnet.DeletionTimestamp.IsZero() && !util.ContainsString(subnet.Finalizers, util.ControllerName) { subnet.Finalizers = append(subnet.Finalizers, util.ControllerName) if _, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Update(context.Background(), subnet, metav1.UpdateOptions{}); err != nil { @@ -483,18 +494,21 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { var err error c.subnetStatusKeyMutex.Lock(key) defer c.subnetStatusKeyMutex.Unlock(key) + defer klog.Infof("end handle add or update subnet %s", key) + klog.Infof("start handle add or update subnet %s", key) cachedSubnet, err := c.subnetsLister.Get(key) if err != nil { if k8serrors.IsNotFound(err) { return nil } + klog.Errorf("failed to get subnet %s error %v", key, err) return err } - klog.V(4).Infof("handle add or update subnet %s", cachedSubnet.Name) subnet := cachedSubnet.DeepCopy() if err = formatSubnet(subnet, c); err != nil { + klog.Errorf("failed to format subnet %s, %v", subnet.Name, err) return err } @@ -513,41 +527,45 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { return err } - vpc, err := c.vpcsLister.Get(subnet.Spec.Vpc) - if err != nil { - klog.Errorf("failed to get subnet's vpc '%s', %v", subnet.Spec.Vpc, err) - return err - } - if !vpc.Status.Standby { - err = fmt.Errorf("the vpc '%s' not standby yet", vpc.Name) - klog.Error(err) - return err - } - - if !vpc.Status.Default { - for _, ns := range subnet.Spec.Namespaces { - if !util.ContainsString(vpc.Spec.Namespaces, ns) { - err = fmt.Errorf("namespace '%s' is out of range to custom vpc '%s'", ns, vpc.Name) - klog.Error(err) - return err - } - } - } else { - vpcs, err := c.vpcsLister.List(labels.Everything()) + var vpc *kubeovnv1.Vpc + if subnet.Spec.Vpc != "" { + vpc, err := c.vpcsLister.Get(subnet.Spec.Vpc) if err != nil { - klog.Errorf("failed to list vpc, %v", err) + klog.Errorf("failed to get subnet's vpc '%s', %v", subnet.Spec.Vpc, err) return err } - for _, vpc := range vpcs { - if subnet.Spec.Vpc != vpc.Name && !vpc.Status.Default && util.IsStringsOverlap(vpc.Spec.Namespaces, subnet.Spec.Namespaces) { - err = fmt.Errorf("namespaces %v are overlap with vpc '%s'", subnet.Spec.Namespaces, vpc.Name) - klog.Error(err) + if !vpc.Status.Standby { + err = fmt.Errorf("the vpc '%s' not standby yet", vpc.Name) + klog.Error(err) + return err + } + + if !vpc.Status.Default { + for _, ns := range subnet.Spec.Namespaces { + if !util.ContainsString(vpc.Spec.Namespaces, ns) { + err = fmt.Errorf("namespace '%s' is out of range to custom vpc '%s'", ns, vpc.Name) + klog.Error(err) + return err + } + } + } else { + vpcs, err := c.vpcsLister.List(labels.Everything()) + if err != nil { + klog.Errorf("failed to list vpc, %v", err) return err } + for _, vpc := range vpcs { + if subnet.Spec.Vpc != vpc.Name && !vpc.Status.Default && util.IsStringsOverlap(vpc.Spec.Namespaces, subnet.Spec.Namespaces) { + err = fmt.Errorf("namespaces %v are overlap with vpc '%s'", subnet.Spec.Namespaces, vpc.Name) + klog.Error(err) + return err + } + } } } if err := c.ipam.AddOrUpdateSubnet(subnet.Name, subnet.Spec.CIDRBlock, subnet.Spec.Gateway, subnet.Spec.ExcludeIps); err != nil { + klog.Errorf("ipam failed to add or update subnet %s, %v", subnet.Name, err) return err } @@ -561,7 +579,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { return err } - deleted, err := c.handleSubnetFinalizer(subnet) + deleted, err := c.handleSubnetFinalizer(key) if err != nil { klog.Errorf("handle subnet finalizer failed %v", err) return err @@ -651,6 +669,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { subnet.Status.EnsureStandardConditions() // If multiple namespace use same ls name, only first one will success if err := c.ovnLegacyClient.CreateLogicalSwitch(subnet.Name, vpc.Status.Router, subnet.Spec.CIDRBlock, subnet.Spec.Gateway, needRouter); err != nil { + klog.Errorf("failed to create logical switch %s, %v", subnet.Name, err) c.patchSubnetStatus(subnet, "CreateLogicalSwitchFailed", err.Error()) return err } @@ -674,6 +693,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { } if err := c.ovnLegacyClient.SetLogicalSwitchConfig(subnet.Name, vpc.Status.Router, subnet.Spec.Protocol, subnet.Spec.CIDRBlock, gateway, subnet.Spec.ExcludeIps, needRouter); err != nil { + klog.Errorf("failed to set logical switch %s config, %v", subnet.Name, err) c.patchSubnetStatus(subnet, "SetLogicalSwitchConfigFailed", err.Error()) return err } @@ -704,7 +724,9 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { case util.NetworkTypeStt: mtu -= util.SttHeaderLength default: - return fmt.Errorf("invalid network type: %s", c.config.NetworkType) + err = fmt.Errorf("invalid network type: %s", c.config.NetworkType) + klog.Error(err) + return err } } } @@ -747,6 +769,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { } if c.config.EnableLb && subnet.Name != c.config.NodeSwitch { if err := c.ovnLegacyClient.AddLbToLogicalSwitch(subnet.Name, lbs...); err != nil { + klog.Errorf("failed to add lb %v to logical switch %s, %v", lbs, subnet.Name, err) c.patchSubnetStatus(subnet, "AddLbToLogicalSwitchFailed", err.Error()) return err } @@ -764,12 +787,14 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { if subnet.Spec.Private { if err := c.ovnLegacyClient.SetPrivateLogicalSwitch(subnet.Name, subnet.Spec.CIDRBlock, subnet.Spec.AllowSubnets); err != nil { + klog.Errorf("failed to set private logical switch %s, %v", subnet.Name, err) c.patchSubnetStatus(subnet, "SetPrivateLogicalSwitchFailed", err.Error()) return err } c.patchSubnetStatus(subnet, "SetPrivateLogicalSwitchSuccess", "") } else { if err := c.ovnLegacyClient.ResetLogicalSwitchAcl(subnet.Name); err != nil { + klog.Errorf("failed to reset logical switch acl %s, %v", subnet.Name, err) c.patchSubnetStatus(subnet, "ResetLogicalSwitchAclFailed", err.Error()) return err } @@ -777,6 +802,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { } if err := c.ovnLegacyClient.UpdateSubnetACL(subnet.Name, subnet.Spec.Acls); err != nil { + klog.Errorf("failed to update subnet acl %s, %v", subnet.Name, err) c.patchSubnetStatus(subnet, "SetLogicalSwitchAclsFailed", err.Error()) return err } @@ -788,6 +814,8 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { func (c *Controller) handleUpdateSubnetStatus(key string) error { c.subnetStatusKeyMutex.Lock(key) defer c.subnetStatusKeyMutex.Unlock(key) + defer klog.Infof("end handle add or update status for subnet %s", key) + klog.Infof("start handle add or update status for subnet %s", key) cachedSubnet, err := c.subnetsLister.Get(key) subnet := cachedSubnet.DeepCopy() @@ -798,18 +826,35 @@ func (c *Controller) handleUpdateSubnetStatus(key string) error { return err } if util.CheckProtocol(subnet.Spec.CIDRBlock) == kubeovnv1.ProtocolDual { - return calcDualSubnetStatusIP(subnet, c) + err = calcDualSubnetStatusIP(subnet, c) } else { - return calcSubnetStatusIP(subnet, c) + err = calcSubnetStatusIP(subnet, c) + } + if err != nil { + klog.Error(err) + return err + } + deleted, err := c.handleSubnetFinalizer(key) + if err != nil { + klog.Errorf("faile to handle finalizer for subnet %s, %v", key, err) + return err + } + if deleted { + return nil } + return nil } func (c *Controller) handleDeleteRoute(subnet *kubeovnv1.Subnet) error { + if subnet.Spec.Vpc == "" { + return nil + } vpc, err := c.vpcsLister.Get(subnet.Spec.Vpc) if err != nil { if k8serrors.IsNotFound(err) { return nil } + klog.Errorf("failed to get vpc %s, %v", subnet.Spec.Vpc, err) return err } return c.deleteStaticRoute(subnet.Spec.CIDRBlock, vpc.Status.Router) @@ -925,6 +970,7 @@ func (c *Controller) handleDeleteSubnet(subnet *kubeovnv1.Subnet) error { for _, vlan := range vlans { if err = c.updateVlanStatusForSubnetDeletion(vlan, subnet.Name); err != nil { + klog.Errorf("failed to update status of vlan %s: %v", vlan.Name, err) return err } } @@ -1123,12 +1169,14 @@ func (c *Controller) reconcileOvnRoute(subnet *kubeovnv1.Subnet) error { for _, pod := range pods { if pod.Annotations[util.LogicalSwitchAnnotation] == subnet.Name && pod.Annotations[util.IpAddressAnnotation] != "" { if err := c.deleteStaticRoute(pod.Annotations[util.IpAddressAnnotation], c.config.ClusterRouter); err != nil { + klog.Errorf("failed to delete static route, %v", err) return err } } } if err := c.deleteStaticRoute(subnet.Spec.CIDRBlock, c.config.ClusterRouter); err != nil { + klog.Errorf("failed to delete static route, %v", err) return err } @@ -1183,10 +1231,12 @@ func (c *Controller) reconcileOvnRoute(subnet *kubeovnv1.Subnet) error { subnet.Status.ActivateGateway = "" bytes, err := subnet.Status.Bytes() if err != nil { + klog.Error(err) return err } _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "") if err != nil { + klog.Error(err) return err } } @@ -1198,6 +1248,7 @@ func (c *Controller) reconcileOvnRoute(subnet *kubeovnv1.Subnet) error { } for _, node := range nodes { if err = c.createPortGroupForDistributedSubnet(node, subnet); err != nil { + klog.Errorf("failed to create port group for distributed subnet %s, %v", subnet.Name, err) return err } if node.Annotations[util.AllocatedAnnotation] != "true" { @@ -1320,9 +1371,14 @@ func (c *Controller) reconcileOvnRoute(subnet *kubeovnv1.Subnet) error { subnet.Status.NotReady("NoReadyGateway", "") bytes, err := subnet.Status.Bytes() if err != nil { + klog.Error(err) return err } _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") + if err != nil { + klog.Error(err) + return err + } return err } @@ -1454,6 +1510,9 @@ func (c *Controller) reconcileOvnRoute(subnet *kubeovnv1.Subnet) error { gw = strings.TrimSpace(gw) } node, err := c.nodesLister.Get(gw) + if err != nil { + klog.Errorf("failed to get gw node %s, %v", gw, err) + } if err == nil && nodeReady(node) { newActivateNode = node.Name nodeTunlIPAddr, err = getNodeTunlIP(node) @@ -1469,6 +1528,7 @@ func (c *Controller) reconcileOvnRoute(subnet *kubeovnv1.Subnet) error { subnet.Status.ActivateGateway = newActivateNode bytes, err := subnet.Status.Bytes() if err != nil { + klog.Errorf("failed to get subnet %s status: %v", subnet.Name, err) return err } if _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status"); err != nil { @@ -1560,7 +1620,11 @@ func (c *Controller) reconcileVlan(subnet *kubeovnv1.Subnet) error { } func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) error { - + if subnet.Spec.Vpc == "" { + err := fmt.Errorf("subnet %s spec should set vpc", subnet.Name) + klog.Error(err) + return err + } needCalcIP := false klog.Infof("reconcile underlay subnet %s to overlay interconnection with U2OInterconnection %v U2OInterconnectionIP %s ", subnet.Name, subnet.Spec.U2OInterconnection, subnet.Status.U2OInterconnectionIP) @@ -1634,6 +1698,32 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err return nil } +func (c *Controller) checkSubnetStatus(subnetName string, subnetCR *kubeovnv1.Subnet) error { + subnet, ok := c.ipam.Subnets[subnetName] + if !ok { + err := fmt.Errorf("ipam should has subnet %s", subnetName) + klog.Error(err) + return err + } + + // subnetCR.Status update is later than ipam subnet + // subnet status available ips should be consistent with ipam subnet ASAP, especially when ipam subnet has no available ips + if subnetCR.Status.V4AvailableIPs != 0 && len(subnet.V4FreeIPList) == 0 && len(subnet.V4ReleasedIPList) == 0 { + klog.Warningf("subnet %s v4AvailableIPs %.0f, v4FreeIPList and v4ReleasedIPList has 0 ip", subnet.Name, subnetCR.Status.V4AvailableIPs) + // subnet reinitialize, make sure the status is consistent with ipam + c.addOrUpdateSubnetQueue.Add(subnetName) + return nil + } + if subnetCR.Status.V6AvailableIPs != 0 && len(subnet.V6FreeIPList) == 0 && len(subnet.V6ReleasedIPList) == 0 { + klog.Warningf("subnet %s v6AvailableIPs %.0f, v6FreeIPList and v6ReleasedIPList has 0 ip", subnet.Name, subnetCR.Status.V6AvailableIPs) + // subnet reinitialize, make sure the status is consistent with ipam + c.addOrUpdateSubnetQueue.Add(subnetName) + return nil + } + + return nil +} + func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { if err := util.CheckCidrs(subnet.Spec.CIDRBlock); err != nil { return err @@ -1711,10 +1801,19 @@ func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { subnet.Status.V6UsingIPs = usingIPs bytes, err := subnet.Status.Bytes() if err != nil { + klog.Errorf("failed to marshal subnet %s status, %v", subnet.Name, err) return err } - _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") - return err + subnet, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") + if err != nil { + klog.Errorf("failed to patch subnet %s status, %v", subnet.Name, err) + return err + } + if err = c.checkSubnetStatus(subnet.Name, subnet); err != nil { + klog.Error(err) + return err + } + return nil } func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { @@ -1796,10 +1895,19 @@ func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { bytes, err := subnet.Status.Bytes() if err != nil { + klog.Errorf("failed to marshal subnet %s status, %v", subnet.Name, err) return err } - _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") - return err + subnet, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") + if err != nil { + klog.Errorf("failed to patch subnet %s status, %v", subnet.Name, err) + return err + } + if err = c.checkSubnetStatus(subnet.Name, subnet); err != nil { + klog.Error(err) + return err + } + return nil } func isOvnSubnet(subnet *kubeovnv1.Subnet) bool { @@ -2181,6 +2289,11 @@ func (c *Controller) deletePolicyRouteByGatewayType(subnet *kubeovnv1.Subnet, ga } func (c *Controller) addPolicyRouteForU2OInterconn(subnet *kubeovnv1.Subnet) error { + if subnet.Spec.Vpc == "" { + err := fmt.Errorf("subnet %s spec should set vpc", subnet.Name) + klog.Error(err) + return err + } var v4Gw, v6Gw string for _, gw := range strings.Split(subnet.Spec.Gateway, ",") { @@ -2295,6 +2408,9 @@ func (c *Controller) addPolicyRouteForU2OInterconn(subnet *kubeovnv1.Subnet) err } func (c *Controller) deletePolicyRouteForU2OInterconn(subnet *kubeovnv1.Subnet) error { + if subnet.Spec.Vpc == "" { + return nil + } results, err := c.ovnLegacyClient.CustomFindEntity("Logical_Router_Policy", []string{"_uuid", "match", "priority"}, "external_ids:isU2ORoutePolicy=\"true\"", @@ -2347,6 +2463,9 @@ func (c *Controller) addCustomVPCPolicyRoutesForSubnet(subnet *kubeovnv1.Subnet) } func (c *Controller) deleteCustomVPCPolicyRoutesForSubnet(subnet *kubeovnv1.Subnet) error { + if subnet.Spec.Vpc == "" { + return nil + } for _, cidr := range strings.Split(subnet.Spec.CIDRBlock, ",") { af := 4 @@ -2364,6 +2483,10 @@ func (c *Controller) deleteCustomVPCPolicyRoutesForSubnet(subnet *kubeovnv1.Subn } func (c *Controller) clearOldU2OResource(subnet *kubeovnv1.Subnet) error { + if subnet.Spec.Vpc == "" { + return nil + } + if subnet.Status.U2OInterconnectionVPC != "" && (!subnet.Spec.U2OInterconnection || (subnet.Spec.U2OInterconnection && subnet.Status.U2OInterconnectionVPC != subnet.Spec.Vpc)) { // remove old u2o lsp and lrp first diff --git a/pkg/controller/vpc.go b/pkg/controller/vpc.go index 14c8c196126..79a32d3fc10 100644 --- a/pkg/controller/vpc.go +++ b/pkg/controller/vpc.go @@ -304,6 +304,8 @@ func (c *Controller) addLoadBalancer(vpc string) (*VpcLoadBalancer, error) { } func (c *Controller) handleAddOrUpdateVpc(key string) error { + klog.Infof("handle add or update vpc %s", key) + // get latest vpc info cachedVpc, err := c.config.KubeOvnClient.KubeovnV1().Vpcs().Get(context.Background(), key, metav1.GetOptions{}) if err != nil { diff --git a/pkg/ipam/ip.go b/pkg/ipam/ip.go index 491e3e159ad..4bfcf0bcaf8 100644 --- a/pkg/ipam/ip.go +++ b/pkg/ipam/ip.go @@ -96,19 +96,24 @@ func mergeIPRangeList(iprl IPRangeList, ip IP) (bool, IPRangeList) { return false, nil } + // keep ip range list sorted for _, ipr := range iprl { if inserted || ipr.Start.LessThan(ip) { + // if exist ipr.Start < new ip, just append existing ipr + // if new ip inserted, just append existing ipr insertIPRangeList = append(insertIPRangeList, ipr) continue } if ipr.Start.GreaterThan(ip) { + // if ipr.Start > ip, inserte new ip，and add append existing ipr insertIPRangeList = append(insertIPRangeList, &IPRange{Start: ip, End: ip}, ipr) inserted = true continue } } if !inserted { + // add new ip to the end of ip range list newIpr := IPRange{Start: ip, End: ip} insertIPRangeList = append(insertIPRangeList, &newIpr) } @@ -121,6 +126,7 @@ func mergeIPRangeList(iprl IPRangeList, ip IP) (bool, IPRangeList) { } if mergedIPRangeList[len(mergedIPRangeList)-1].End.Add(1).Equal(ipr.Start) { + // if the end of the previous ipr equals the start of current ipr, merge mergedIPRangeList[len(mergedIPRangeList)-1].End = ipr.End } else { mergedIPRangeList = append(mergedIPRangeList, ipr) diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go index 9d99e7b34f2..9bb0d4b0b73 100644 --- a/pkg/ipam/ipam.go +++ b/pkg/ipam/ipam.go @@ -2,6 +2,7 @@ package ipam import ( "errors" + "fmt" "net" "strconv" "strings" @@ -44,6 +45,8 @@ func (ipam *IPAM) GetRandomAddress(podName, nicName, mac, subnetName string, ski subnet, ok := ipam.Subnets[subnetName] if !ok { + err := fmt.Errorf("ipam has no subnet %s", subnetName) + klog.Error(err) return "", "", "", ErrNoAvailable } @@ -56,6 +59,8 @@ func (ipam *IPAM) GetStaticAddress(podName, nicName, ip, mac, subnetName string, ipam.mutex.RLock() defer ipam.mutex.RUnlock() if subnet, ok := ipam.Subnets[subnetName]; !ok { + err := fmt.Errorf("ipam has no subnet %s", subnetName) + klog.Error(err) return "", "", "", ErrNoAvailable } else { var ips []IP @@ -130,6 +135,8 @@ func (ipam *IPAM) AddOrUpdateSubnet(name, cidrStr, gw string, excludeIps []strin ipam.mutex.Lock() defer ipam.mutex.Unlock() + defer klog.Infof("ipam end add or update subnet %s", name) + klog.Infof("ipam start add or update subnet %s", name) var v4cidrStr, v6cidrStr, v4Gw, v6Gw string var cidrs []*net.IPNet @@ -195,6 +202,7 @@ func (ipam *IPAM) AddOrUpdateSubnet(name, cidrStr, gw string, excludeIps []strin } } } + klog.Infof("updated subnet %s", subnet.Name) return nil } diff --git a/pkg/ipam/subnet.go b/pkg/ipam/subnet.go index a7367ff3c82..775f4b7ad60 100644 --- a/pkg/ipam/subnet.go +++ b/pkg/ipam/subnet.go @@ -41,6 +41,7 @@ func NewSubnet(name, cidrStr string, excludeIps []string) (*Subnet, error) { var cidrs []*net.IPNet for _, cidrBlock := range strings.Split(cidrStr, ",") { if _, cidr, err := net.ParseCIDR(cidrBlock); err != nil { + klog.Errorf("invalid cidr %s: %v", cidrBlock, err) return nil, ErrInvalidCIDR } else { cidrs = append(cidrs, cidr) @@ -184,10 +185,12 @@ func (subnet *Subnet) GetRandomAddress(podName, nicName string, mac string, skip func (subnet *Subnet) getDualRandomAddress(podName, nicName string, mac string, skippedAddrs []string, checkConflict bool) (IP, IP, string, error) { v4IP, _, _, err := subnet.getV4RandomAddress(podName, nicName, mac, skippedAddrs, checkConflict) if err != nil { + klog.Errorf("failed to allocate v4 ip for %s: %v", podName, err) return "", "", "", err } _, v6IP, mac, err := subnet.getV6RandomAddress(podName, nicName, mac, skippedAddrs, checkConflict) if err != nil { + klog.Errorf("failed to allocate v6 ip for %s: %v", podName, err) return "", "", "", err } @@ -211,6 +214,7 @@ func (subnet *Subnet) getV4RandomAddress(podName, nicName string, mac string, sk } if len(subnet.V4FreeIPList) == 0 { if len(subnet.V4ReleasedIPList) == 0 { + klog.Errorf("no available ip in subnet %s, v4 free ip list %v", subnet.Name, subnet.V4FreeIPList) return "", "", "", ErrNoAvailable } subnet.V4FreeIPList = subnet.V4ReleasedIPList @@ -257,6 +261,7 @@ func (subnet *Subnet) getV4RandomAddress(podName, nicName string, mac string, sk return ip, "", subnet.GetRandomMac(podName, nicName), nil } else { if err := subnet.GetStaticMac(podName, nicName, mac, checkConflict); err != nil { + klog.Errorf("failed to get static mac for pod %s: %v", podName, err) return "", "", "", err } return ip, "", mac, nil @@ -276,6 +281,7 @@ func (subnet *Subnet) getV6RandomAddress(podName, nicName string, mac string, sk if len(subnet.V6FreeIPList) == 0 { if len(subnet.V6ReleasedIPList) == 0 { + klog.Errorf("no available ip in subnet %s, v6 free ip list %v", subnet.Name, subnet.V6FreeIPList) return "", "", "", ErrNoAvailable } subnet.V6FreeIPList = subnet.V6ReleasedIPList @@ -322,6 +328,7 @@ func (subnet *Subnet) getV6RandomAddress(podName, nicName string, mac string, sk return "", ip, subnet.GetRandomMac(podName, nicName), nil } else { if err := subnet.GetStaticMac(podName, nicName, mac, checkConflict); err != nil { + klog.Errorf("failed to get static mac for pod %s: %v", podName, err) return "", "", "", err } return "", ip, mac, nil @@ -342,6 +349,7 @@ func (subnet *Subnet) GetStaticAddress(podName, nicName string, ip IP, mac strin v6 = subnet.V6CIDR != nil } if v4 && !subnet.V4CIDR.Contains(net.ParseIP(string(ip))) { + klog.Errorf("v4 ip %s is out of range", ip) return ip, mac, ErrOutOfRange } @@ -350,6 +358,7 @@ func (subnet *Subnet) GetStaticAddress(podName, nicName string, ip IP, mac strin } if v6 && !subnet.V6CIDR.Contains(net.ParseIP(string(ip))) { + klog.Errorf("v6 ip %s is out of range", ip) return ip, mac, ErrOutOfRange } @@ -361,6 +370,7 @@ func (subnet *Subnet) GetStaticAddress(podName, nicName string, ip IP, mac strin } } else { if err := subnet.GetStaticMac(podName, nicName, mac, checkConflict); err != nil { + klog.Errorf("failed to get static mac for pod %s: %v", podName, err) return ip, mac, err } } @@ -438,6 +448,7 @@ func (subnet *Subnet) GetStaticAddress(podName, nicName string, ip IP, mac strin } } } + klog.Errorf("no available ip in subnet %s", subnet.Name) return ip, mac, ErrNoAvailable } diff --git a/pkg/ovs/ovn-nbctl-legacy.go b/pkg/ovs/ovn-nbctl-legacy.go index 8d27086502f..80de28a5126 100644 --- a/pkg/ovs/ovn-nbctl-legacy.go +++ b/pkg/ovs/ovn-nbctl-legacy.go @@ -1011,6 +1011,7 @@ func (c LegacyClient) AddStaticRoute(policy, cidr, nextHop, router string, route filter := []string{fmt.Sprintf("policy=%s", policy), fmt.Sprintf(`ip_prefix="%s"`, cidrBlock), fmt.Sprintf(`nexthop!="%s"`, gw)} result, err := c.CustomFindEntity("Logical_Router_Static_Route", []string{"_uuid"}, filter...) if err != nil { + klog.Errorf("failed to find static route: %v", err) return err } @@ -1022,6 +1023,7 @@ func (c LegacyClient) AddStaticRoute(policy, cidr, nextHop, router string, route } if _, err := c.ovnNbCommand(MayExist, fmt.Sprintf("%s=%s", Policy, policy), "lr-route-add", router, cidrBlock, gw); err != nil { + klog.Errorf("failed to add static route %s: %v", cidrBlock, err) return err } } @@ -1033,8 +1035,14 @@ func (c LegacyClient) AddStaticRoute(policy, cidr, nextHop, router string, route // AddPolicyRoute add a policy route rule in ovn func (c LegacyClient) AddPolicyRoute(router string, priority int32, match, action, nextHop string, externalIDs map[string]string) error { + if router == "" { + err := fmt.Errorf("router name is empty") + klog.Error(err) + return err + } consistent, err := c.CheckPolicyRouteNexthopConsistent(router, match, nextHop, priority) if err != nil { + klog.Errorf("failed to check policy route nexthop consistent: %v", err) return err } if consistent { @@ -1082,8 +1090,12 @@ func (c LegacyClient) AddPolicyRoute(router string, priority int32, match, actio // DeletePolicyRoute delete a policy route rule in ovn func (c LegacyClient) DeletePolicyRoute(router string, priority int32, match string) error { + if router == "" { + return nil + } exist, err := c.IsPolicyRouteExist(router, priority, match) if err != nil { + klog.Errorf("failed to check policy route exist: %v", err) return err } if !exist { @@ -1099,7 +1111,11 @@ func (c LegacyClient) DeletePolicyRoute(router string, priority int32, match str } klog.Infof("remove policy route from router %s: match %s", router, match) _, err = c.ovnNbCommand(args...) - return err + if err != nil { + klog.Errorf("failed to delete policy route: %v", err) + return err + } + return nil } func (c LegacyClient) CleanPolicyRoute(router string) error { From d3cdf918cd34ca37b0f5cb677c1131abab5eff69 Mon Sep 17 00:00:00 2001 From: bobz965 Date: Mon, 5 Feb 2024 17:30:15 +0800 Subject: [PATCH 2/6] remove subnet status check Signed-off-by: bobz965 --- pkg/controller/subnet.go | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index 4a6370a13b1..2f7b5d8ba04 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -1698,32 +1698,6 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err return nil } -func (c *Controller) checkSubnetStatus(subnetName string, subnetCR *kubeovnv1.Subnet) error { - subnet, ok := c.ipam.Subnets[subnetName] - if !ok { - err := fmt.Errorf("ipam should has subnet %s", subnetName) - klog.Error(err) - return err - } - - // subnetCR.Status update is later than ipam subnet - // subnet status available ips should be consistent with ipam subnet ASAP, especially when ipam subnet has no available ips - if subnetCR.Status.V4AvailableIPs != 0 && len(subnet.V4FreeIPList) == 0 && len(subnet.V4ReleasedIPList) == 0 { - klog.Warningf("subnet %s v4AvailableIPs %.0f, v4FreeIPList and v4ReleasedIPList has 0 ip", subnet.Name, subnetCR.Status.V4AvailableIPs) - // subnet reinitialize, make sure the status is consistent with ipam - c.addOrUpdateSubnetQueue.Add(subnetName) - return nil - } - if subnetCR.Status.V6AvailableIPs != 0 && len(subnet.V6FreeIPList) == 0 && len(subnet.V6ReleasedIPList) == 0 { - klog.Warningf("subnet %s v6AvailableIPs %.0f, v6FreeIPList and v6ReleasedIPList has 0 ip", subnet.Name, subnetCR.Status.V6AvailableIPs) - // subnet reinitialize, make sure the status is consistent with ipam - c.addOrUpdateSubnetQueue.Add(subnetName) - return nil - } - - return nil -} - func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { if err := util.CheckCidrs(subnet.Spec.CIDRBlock); err != nil { return err @@ -1809,10 +1783,6 @@ func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { klog.Errorf("failed to patch subnet %s status, %v", subnet.Name, err) return err } - if err = c.checkSubnetStatus(subnet.Name, subnet); err != nil { - klog.Error(err) - return err - } return nil } @@ -1903,10 +1873,6 @@ func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { klog.Errorf("failed to patch subnet %s status, %v", subnet.Name, err) return err } - if err = c.checkSubnetStatus(subnet.Name, subnet); err != nil { - klog.Error(err) - return err - } return nil } From 2500f70fb9f3aa4484ce0cb2385fc2f73a260354 Mon Sep 17 00:00:00 2001 From: bobz965 Date: Mon, 5 Feb 2024 17:53:26 +0800 Subject: [PATCH 3/6] fix nil vpc Signed-off-by: bobz965 --- pkg/controller/subnet.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index 2f7b5d8ba04..e47f5f1de76 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -599,6 +599,14 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { return nil } + subnet, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Get(context.Background(), key, metav1.GetOptions{}) + if err != nil { + if k8serrors.IsNotFound(err) { + return nil + } + klog.Errorf("failed to get subnet %s error %v", key, err) + return err + } if err = util.ValidateSubnet(*subnet); err != nil { klog.Errorf("failed to validate subnet %s, %v", subnet.Name, err) c.patchSubnetStatus(subnet, "ValidateLogicalSwitchFailed", err.Error()) @@ -660,6 +668,11 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { } needRouter := subnet.Spec.Vlan == "" || subnet.Spec.LogicalGateway || (subnet.Status.U2OInterconnectionIP != "" && subnet.Spec.U2OInterconnection) + vpc, err = c.vpcsLister.Get(subnet.Spec.Vpc) + if err != nil { + klog.Errorf("failed to get subnet's vpc '%s', %v", subnet.Spec.Vpc, err) + return err + } // 1. overlay subnet, should add lrp, lrp ip is subnet gw // 2. underlay subnet use logical gw, should add lrp, lrp ip is subnet gw randomAllocateGW := !subnet.Spec.LogicalGateway && vpc.Spec.EnableExternal && subnet.Name == c.config.ExternalGatewaySwitch From 4facd929ba673634a4126611d4ea467a31d01266 Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 6 Feb 2024 09:15:47 +0800 Subject: [PATCH 4/6] fix ic Signed-off-by: bobz965 --- pkg/controller/ip.go | 2 +- pkg/controller/subnet.go | 109 +++++++++++------------------------- pkg/ovs/ovn-nbctl-legacy.go | 8 --- 3 files changed, 33 insertions(+), 86 deletions(-) diff --git a/pkg/controller/ip.go b/pkg/controller/ip.go index e008695fb3b..e634915c76d 100644 --- a/pkg/controller/ip.go +++ b/pkg/controller/ip.go @@ -248,7 +248,7 @@ func (c *Controller) handleUpdateIP(key string) error { } } } - if cleanIPAM { + if cleanIPAM && cachedIP.Spec.PodName != "" && cachedIP.Spec.Namespace != "" { podKey := fmt.Sprintf("%s/%s", cachedIP.Spec.Namespace, cachedIP.Spec.PodName) klog.Infof("ip cr %s release ipam pod key %s from subnet %s", cachedIP.Name, podKey, cachedIP.Spec.Subnet) c.ipam.ReleaseAddressByPod(podKey, cachedIP.Spec.Subnet) diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index e47f5f1de76..2e4a427c2a3 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -300,7 +300,7 @@ func formatSubnet(subnet *kubeovnv1.Subnet, c *Controller) error { subnet.Spec.GatewayType = kubeovnv1.GWDistributedType changed = true } - if subnet.Spec.Vpc == "" && isOvnSubnet(subnet) { + if subnet.Spec.Vpc == "" { changed = true subnet.Spec.Vpc = util.DefaultVpc @@ -430,15 +430,7 @@ func checkAndUpdateExcludeIps(subnet *kubeovnv1.Subnet) bool { return changed } -func (c *Controller) handleSubnetFinalizer(key string) (bool, error) { - subnet, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Get(context.Background(), key, metav1.GetOptions{}) - if err != nil { - if k8serrors.IsNotFound(err) { - return true, nil - } - klog.Errorf("failed to get subnet %s error %v", key, err) - return false, err - } +func (c *Controller) handleSubnetFinalizer(subnet *kubeovnv1.Subnet) (bool, error) { if subnet.DeletionTimestamp.IsZero() && !util.ContainsString(subnet.Finalizers, util.ControllerName) { subnet.Finalizers = append(subnet.Finalizers, util.ControllerName) if _, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Update(context.Background(), subnet, metav1.UpdateOptions{}); err != nil { @@ -527,40 +519,37 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { return err } - var vpc *kubeovnv1.Vpc - if subnet.Spec.Vpc != "" { - vpc, err := c.vpcsLister.Get(subnet.Spec.Vpc) - if err != nil { - klog.Errorf("failed to get subnet's vpc '%s', %v", subnet.Spec.Vpc, err) - return err + vpc, err := c.vpcsLister.Get(subnet.Spec.Vpc) + if err != nil { + klog.Errorf("failed to get subnet's vpc '%s', %v", subnet.Spec.Vpc, err) + return err + } + if !vpc.Status.Standby { + err = fmt.Errorf("the vpc '%s' not standby yet", vpc.Name) + klog.Error(err) + return err + } + + if !vpc.Status.Default { + for _, ns := range subnet.Spec.Namespaces { + if !util.ContainsString(vpc.Spec.Namespaces, ns) { + err = fmt.Errorf("namespace '%s' is out of range to custom vpc '%s'", ns, vpc.Name) + klog.Error(err) + return err + } } - if !vpc.Status.Standby { - err = fmt.Errorf("the vpc '%s' not standby yet", vpc.Name) - klog.Error(err) + } else { + vpcs, err := c.vpcsLister.List(labels.Everything()) + if err != nil { + klog.Errorf("failed to list vpc, %v", err) return err } - - if !vpc.Status.Default { - for _, ns := range subnet.Spec.Namespaces { - if !util.ContainsString(vpc.Spec.Namespaces, ns) { - err = fmt.Errorf("namespace '%s' is out of range to custom vpc '%s'", ns, vpc.Name) - klog.Error(err) - return err - } - } - } else { - vpcs, err := c.vpcsLister.List(labels.Everything()) - if err != nil { - klog.Errorf("failed to list vpc, %v", err) + for _, vpc := range vpcs { + if subnet.Spec.Vpc != vpc.Name && !vpc.Status.Default && util.IsStringsOverlap(vpc.Spec.Namespaces, subnet.Spec.Namespaces) { + err = fmt.Errorf("namespaces %v are overlap with vpc '%s'", subnet.Spec.Namespaces, vpc.Name) + klog.Error(err) return err } - for _, vpc := range vpcs { - if subnet.Spec.Vpc != vpc.Name && !vpc.Status.Default && util.IsStringsOverlap(vpc.Spec.Namespaces, subnet.Spec.Namespaces) { - err = fmt.Errorf("namespaces %v are overlap with vpc '%s'", subnet.Spec.Namespaces, vpc.Name) - klog.Error(err) - return err - } - } } } @@ -579,7 +568,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { return err } - deleted, err := c.handleSubnetFinalizer(key) + deleted, err := c.handleSubnetFinalizer(subnet) if err != nil { klog.Errorf("handle subnet finalizer failed %v", err) return err @@ -668,11 +657,6 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { } needRouter := subnet.Spec.Vlan == "" || subnet.Spec.LogicalGateway || (subnet.Status.U2OInterconnectionIP != "" && subnet.Spec.U2OInterconnection) - vpc, err = c.vpcsLister.Get(subnet.Spec.Vpc) - if err != nil { - klog.Errorf("failed to get subnet's vpc '%s', %v", subnet.Spec.Vpc, err) - return err - } // 1. overlay subnet, should add lrp, lrp ip is subnet gw // 2. underlay subnet use logical gw, should add lrp, lrp ip is subnet gw randomAllocateGW := !subnet.Spec.LogicalGateway && vpc.Spec.EnableExternal && subnet.Name == c.config.ExternalGatewaySwitch @@ -847,21 +831,15 @@ func (c *Controller) handleUpdateSubnetStatus(key string) error { klog.Error(err) return err } - deleted, err := c.handleSubnetFinalizer(key) + _, err = c.handleSubnetFinalizer(subnet) if err != nil { klog.Errorf("faile to handle finalizer for subnet %s, %v", key, err) return err } - if deleted { - return nil - } return nil } func (c *Controller) handleDeleteRoute(subnet *kubeovnv1.Subnet) error { - if subnet.Spec.Vpc == "" { - return nil - } vpc, err := c.vpcsLister.Get(subnet.Spec.Vpc) if err != nil { if k8serrors.IsNotFound(err) { @@ -1633,11 +1611,6 @@ func (c *Controller) reconcileVlan(subnet *kubeovnv1.Subnet) error { } func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) error { - if subnet.Spec.Vpc == "" { - err := fmt.Errorf("subnet %s spec should set vpc", subnet.Name) - klog.Error(err) - return err - } needCalcIP := false klog.Infof("reconcile underlay subnet %s to overlay interconnection with U2OInterconnection %v U2OInterconnectionIP %s ", subnet.Name, subnet.Spec.U2OInterconnection, subnet.Status.U2OInterconnectionIP) @@ -1791,7 +1764,7 @@ func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { klog.Errorf("failed to marshal subnet %s status, %v", subnet.Name, err) return err } - subnet, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") + _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") if err != nil { klog.Errorf("failed to patch subnet %s status, %v", subnet.Name, err) return err @@ -1881,7 +1854,7 @@ func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { klog.Errorf("failed to marshal subnet %s status, %v", subnet.Name, err) return err } - subnet, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") + _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") if err != nil { klog.Errorf("failed to patch subnet %s status, %v", subnet.Name, err) return err @@ -2268,12 +2241,6 @@ func (c *Controller) deletePolicyRouteByGatewayType(subnet *kubeovnv1.Subnet, ga } func (c *Controller) addPolicyRouteForU2OInterconn(subnet *kubeovnv1.Subnet) error { - if subnet.Spec.Vpc == "" { - err := fmt.Errorf("subnet %s spec should set vpc", subnet.Name) - klog.Error(err) - return err - } - var v4Gw, v6Gw string for _, gw := range strings.Split(subnet.Spec.Gateway, ",") { switch util.CheckProtocol(gw) { @@ -2387,10 +2354,6 @@ func (c *Controller) addPolicyRouteForU2OInterconn(subnet *kubeovnv1.Subnet) err } func (c *Controller) deletePolicyRouteForU2OInterconn(subnet *kubeovnv1.Subnet) error { - if subnet.Spec.Vpc == "" { - return nil - } - results, err := c.ovnLegacyClient.CustomFindEntity("Logical_Router_Policy", []string{"_uuid", "match", "priority"}, "external_ids:isU2ORoutePolicy=\"true\"", fmt.Sprintf("external_ids:vendor=\"%s\"", util.CniTypeName), @@ -2442,10 +2405,6 @@ func (c *Controller) addCustomVPCPolicyRoutesForSubnet(subnet *kubeovnv1.Subnet) } func (c *Controller) deleteCustomVPCPolicyRoutesForSubnet(subnet *kubeovnv1.Subnet) error { - if subnet.Spec.Vpc == "" { - return nil - } - for _, cidr := range strings.Split(subnet.Spec.CIDRBlock, ",") { af := 4 if util.CheckProtocol(cidr) == kubeovnv1.ProtocolIPv6 { @@ -2462,10 +2421,6 @@ func (c *Controller) deleteCustomVPCPolicyRoutesForSubnet(subnet *kubeovnv1.Subn } func (c *Controller) clearOldU2OResource(subnet *kubeovnv1.Subnet) error { - if subnet.Spec.Vpc == "" { - return nil - } - if subnet.Status.U2OInterconnectionVPC != "" && (!subnet.Spec.U2OInterconnection || (subnet.Spec.U2OInterconnection && subnet.Status.U2OInterconnectionVPC != subnet.Spec.Vpc)) { // remove old u2o lsp and lrp first diff --git a/pkg/ovs/ovn-nbctl-legacy.go b/pkg/ovs/ovn-nbctl-legacy.go index 80de28a5126..67f24b3c97f 100644 --- a/pkg/ovs/ovn-nbctl-legacy.go +++ b/pkg/ovs/ovn-nbctl-legacy.go @@ -1035,11 +1035,6 @@ func (c LegacyClient) AddStaticRoute(policy, cidr, nextHop, router string, route // AddPolicyRoute add a policy route rule in ovn func (c LegacyClient) AddPolicyRoute(router string, priority int32, match, action, nextHop string, externalIDs map[string]string) error { - if router == "" { - err := fmt.Errorf("router name is empty") - klog.Error(err) - return err - } consistent, err := c.CheckPolicyRouteNexthopConsistent(router, match, nextHop, priority) if err != nil { klog.Errorf("failed to check policy route nexthop consistent: %v", err) @@ -1090,9 +1085,6 @@ func (c LegacyClient) AddPolicyRoute(router string, priority int32, match, actio // DeletePolicyRoute delete a policy route rule in ovn func (c LegacyClient) DeletePolicyRoute(router string, priority int32, match string) error { - if router == "" { - return nil - } exist, err := c.IsPolicyRouteExist(router, priority, match) if err != nil { klog.Errorf("failed to check policy route exist: %v", err) From 813e00dbb6813dd49d118ce7a8eae5a1f13d801c Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 6 Feb 2024 13:03:49 +0800 Subject: [PATCH 5/6] fix subnet not delete Signed-off-by: bobz965 --- pkg/controller/init.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/controller/init.go b/pkg/controller/init.go index 17df1d04139..fdf67878818 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -623,6 +623,11 @@ func (c *Controller) initSyncCrdSubnets() error { klog.Errorf("failed to calculate subnet %s used ip: %v", subnet.Name, err) return err } + + if subnet.DeletionTimestamp != nil { + klog.Infof("enqueue update for deleting subnet %s", subnet.Name) + c.addOrUpdateSubnetQueue.Add(subnet.Name) + } } return nil } From 09310402b9de9de0fa59eba406b32cc2fdcefc4b Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 6 Feb 2024 13:28:21 +0800 Subject: [PATCH 6/6] fix ic subnet Signed-off-by: bobz965 --- pkg/controller/init.go | 2 +- pkg/controller/subnet.go | 13 ------------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/pkg/controller/init.go b/pkg/controller/init.go index fdf67878818..05de0fd1c2c 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -623,7 +623,7 @@ func (c *Controller) initSyncCrdSubnets() error { klog.Errorf("failed to calculate subnet %s used ip: %v", subnet.Name, err) return err } - + // retrigger subnet update to delete subnet in case of kube-ovn-controller crashed while subnet is deleting if subnet.DeletionTimestamp != nil { klog.Infof("enqueue update for deleting subnet %s", subnet.Name) c.addOrUpdateSubnetQueue.Add(subnet.Name) diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index 2e4a427c2a3..30076ebfaa9 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -588,14 +588,6 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { return nil } - subnet, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Get(context.Background(), key, metav1.GetOptions{}) - if err != nil { - if k8serrors.IsNotFound(err) { - return nil - } - klog.Errorf("failed to get subnet %s error %v", key, err) - return err - } if err = util.ValidateSubnet(*subnet); err != nil { klog.Errorf("failed to validate subnet %s, %v", subnet.Name, err) c.patchSubnetStatus(subnet, "ValidateLogicalSwitchFailed", err.Error()) @@ -831,11 +823,6 @@ func (c *Controller) handleUpdateSubnetStatus(key string) error { klog.Error(err) return err } - _, err = c.handleSubnetFinalizer(subnet) - if err != nil { - klog.Errorf("faile to handle finalizer for subnet %s, %v", key, err) - return err - } return nil }