diff --git a/pkg/controller/init.go b/pkg/controller/init.go index 8511773c284..d2c3de85226 100644 --- a/pkg/controller/init.go +++ b/pkg/controller/init.go @@ -101,7 +101,7 @@ func (c *Controller) initDefaultLogicalSwitch() error { if util.CheckProtocol(c.config.DefaultCIDR) == kubeovnv1.ProtocolDual { subnet := subnet.DeepCopy() subnet.Spec.CIDRBlock = c.config.DefaultCIDR - if _, err = formatSubnet(subnet, c); err != nil { + if _, err = c.formatSubnet(subnet); err != nil { klog.Errorf("init format subnet %s failed: %v", c.config.DefaultLogicalSwitch, err) return err } @@ -155,7 +155,7 @@ func (c *Controller) initNodeSwitch() error { // single-stack upgrade to dual-stack subnet := subnet.DeepCopy() subnet.Spec.CIDRBlock = c.config.NodeSwitchCIDR - if _, err = formatSubnet(subnet, c); err != nil { + if _, err = c.formatSubnet(subnet); err != nil { klog.Errorf("init format subnet %s failed: %v", c.config.NodeSwitch, err) return err } @@ -612,9 +612,9 @@ func (c *Controller) initSyncCrdSubnets() error { for _, orisubnet := range subnets { subnet := orisubnet.DeepCopy() if util.CheckProtocol(subnet.Spec.CIDRBlock) == kubeovnv1.ProtocolDual { - err = calcDualSubnetStatusIP(subnet, c) + subnet, err = c.calcDualSubnetStatusIP(subnet) } else { - err = calcSubnetStatusIP(subnet, c) + subnet, err = c.calcSubnetStatusIP(subnet) } if err != nil { klog.Errorf("failed to calculate subnet %s used ip: %v", subnet.Name, err) diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go index 47c9d806faa..8f7bd2a9361 100644 --- a/pkg/controller/subnet.go +++ b/pkg/controller/subnet.go @@ -272,7 +272,7 @@ func (c *Controller) processNextDeleteSubnetWorkItem() bool { return true } -func formatSubnet(subnet *kubeovnv1.Subnet, c *Controller) (*kubeovnv1.Subnet, error) { +func (c *Controller) formatSubnet(subnet *kubeovnv1.Subnet) (*kubeovnv1.Subnet, error) { var ( changed bool err error @@ -332,13 +332,14 @@ func formatSubnet(subnet *kubeovnv1.Subnet, c *Controller) (*kubeovnv1.Subnet, e klog.Infof("format subnet %v, changed %v", subnet.Name, changed) if changed { - subnet, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Update(context.Background(), subnet, metav1.UpdateOptions{}) + newSubnet, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Update(context.Background(), subnet, metav1.UpdateOptions{}) if err != nil { klog.Errorf("failed to update subnet %s, %v", subnet.Name, err) return nil, err } + return newSubnet, nil } - return subnet.DeepCopy(), nil + return subnet, nil } func (c *Controller) updateNatOutgoingPolicyRulesStatus(subnet *kubeovnv1.Subnet) error { @@ -708,16 +709,17 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error { } klog.V(3).Infof("handle add or update subnet %s", cachedSubnet.Name) - subnet, err := formatSubnet(cachedSubnet.DeepCopy(), c) + subnet := cachedSubnet.DeepCopy() + subnet, err = c.formatSubnet(subnet) if err != nil { klog.Error(err) return err } if subnet.Spec.Protocol == kubeovnv1.ProtocolDual { - err = calcDualSubnetStatusIP(subnet, c) + subnet, err = c.calcDualSubnetStatusIP(subnet) } else { - err = calcSubnetStatusIP(subnet, c) + subnet, err = c.calcSubnetStatusIP(subnet) } if err != nil { klog.Errorf("calculate subnet %s used ip failed, %v", subnet.Name, err) @@ -917,9 +919,17 @@ func (c *Controller) handleUpdateSubnetStatus(key string) error { } if util.CheckProtocol(subnet.Spec.CIDRBlock) == kubeovnv1.ProtocolDual { - return calcDualSubnetStatusIP(subnet, c) + if _, err := c.calcDualSubnetStatusIP(subnet); err != nil { + klog.Error(err) + return err + } + return nil } - return calcSubnetStatusIP(subnet, c) + if _, err = c.calcSubnetStatusIP(subnet); err != nil { + klog.Error(err) + return err + } + return nil } func (c *Controller) handleDeleteLogicalSwitch(key string) (err error) { @@ -1992,11 +2002,13 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err klog.Infof("reconcile underlay subnet %s to overlay interconnection with U2OInterconnection %v U2OInterconnectionIP %s ", subnet.Name, subnet.Spec.U2OInterconnection, subnet.Status.U2OInterconnectionIP) if subnet.Spec.Protocol == kubeovnv1.ProtocolDual { - if err := calcDualSubnetStatusIP(subnet, c); err != nil { + if _, err := c.calcDualSubnetStatusIP(subnet); err != nil { + klog.Error(err) return err } } else { - if err := calcSubnetStatusIP(subnet, c); err != nil { + if _, err := c.calcSubnetStatusIP(subnet); err != nil { + klog.Error(err) return err } } @@ -2004,15 +2016,15 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err return nil } -func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { +func (c *Controller) calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet) (*kubeovnv1.Subnet, error) { if err := util.CheckCidrs(subnet.Spec.CIDRBlock); err != nil { - return err + return nil, err } // Get the number of pods, not ips. For one pod with two ip(v4 & v6) in dual-stack, num of Items is 1 podUsedIPs, err := c.ipsLister.List(labels.SelectorFromSet(labels.Set{subnet.Name: ""})) if err != nil { klog.Error(err) - return err + return nil, err } // subnet.Spec.ExcludeIps contains both v4 and v6 addresses @@ -2034,7 +2046,7 @@ func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { })) if err != nil { klog.Error(err) - return err + return nil, err } usingIPs += float64(len(vips)) @@ -2043,7 +2055,7 @@ func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { labels.SelectorFromSet(labels.Set{util.SubnetNameLabel: subnet.Name})) if err != nil { klog.Error(err) - return err + return nil, err } usingIPs += float64(len(eips)) } @@ -2066,7 +2078,7 @@ func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { subnet.Status.V6UsingIPRange == v6UsingIPStr && subnet.Status.V4AvailableIPRange == v4AvailableIPStr && subnet.Status.V6AvailableIPRange == v6AvailableIPStr { - return nil + return subnet, nil } subnet.Status.V4AvailableIPs = v4availableIPs @@ -2081,22 +2093,22 @@ func calcDualSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { bytes, err := subnet.Status.Bytes() if err != nil { klog.Error(err) - return err + return nil, err } - _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") - return err + newSubnet, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") + return newSubnet, err } -func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { +func (c *Controller) calcSubnetStatusIP(subnet *kubeovnv1.Subnet) (*kubeovnv1.Subnet, error) { _, cidr, err := net.ParseCIDR(subnet.Spec.CIDRBlock) if err != nil { klog.Error(err) - return err + return nil, err } podUsedIPs, err := c.ipsLister.List(labels.SelectorFromSet(labels.Set{subnet.Name: ""})) if err != nil { klog.Error(err) - return err + return nil, err } // gateway always in excludeIPs toSubIPs := util.ExpandExcludeIPs(subnet.Spec.ExcludeIps, subnet.Spec.CIDRBlock) @@ -2108,7 +2120,7 @@ func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { })) if err != nil { klog.Error(err) - return err + return nil, err } usingIPs += float64(len(vips)) ovnEips, err := c.ovnEipsLister.List(labels.SelectorFromSet(labels.Set{ @@ -2116,7 +2128,7 @@ func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { })) if err != nil { klog.Error(err) - return err + return nil, err } usingIPs += float64(len(ovnEips)) if !isOvnSubnet(subnet) { @@ -2124,7 +2136,7 @@ func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { labels.SelectorFromSet(labels.Set{util.SubnetNameLabel: subnet.Name})) if err != nil { klog.Error(err) - return err + return nil, err } usingIPs += float64(len(eips)) } @@ -2174,16 +2186,16 @@ func calcSubnetStatusIP(subnet *kubeovnv1.Subnet, c *Controller) error { subnet.Status.V6UsingIPRange, subnet.Status.V6AvailableIPRange, } { - return nil + return subnet, nil } bytes, err := subnet.Status.Bytes() if err != nil { klog.Error(err) - return err + return nil, err } - _, err = c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") - return err + newSubnet, err := c.config.KubeOvnClient.KubeovnV1().Subnets().Patch(context.Background(), subnet.Name, types.MergePatchType, bytes, metav1.PatchOptions{}, "status") + return newSubnet, err } func isOvnSubnet(subnet *kubeovnv1.Subnet) bool { diff --git a/pkg/controller/vip.go b/pkg/controller/vip.go index d33768de9f3..b7e098d611d 100644 --- a/pkg/controller/vip.go +++ b/pkg/controller/vip.go @@ -482,11 +482,15 @@ func (c *Controller) handleUpdateVirtualParents(key string) error { func (c *Controller) subnetCountIP(subnet *kubeovnv1.Subnet) error { var err error if util.CheckProtocol(subnet.Spec.CIDRBlock) == kubeovnv1.ProtocolDual { - err = calcDualSubnetStatusIP(subnet, c) + _, err = c.calcDualSubnetStatusIP(subnet) } else { - err = calcSubnetStatusIP(subnet, c) + _, err = c.calcSubnetStatusIP(subnet) } - return err + if err != nil { + klog.Error(err) + return err + } + return nil } func (c *Controller) createOrUpdateCrdVip(key, ns, subnet, v4ip, v6ip, mac, pV4ip, pV6ip, pmac string) error { diff --git a/pkg/controller/vlan.go b/pkg/controller/vlan.go index b9df03f7ae6..1ca43ee7dfa 100644 --- a/pkg/controller/vlan.go +++ b/pkg/controller/vlan.go @@ -182,7 +182,6 @@ func (c *Controller) handleAddVlan(key string) error { klog.Errorf("failed to update vlan %s, %v", vlan.Name, err) return err } - vlan = vlan.DeepCopy() } subnets, err := c.subnetsLister.List(labels.Everything()) @@ -200,7 +199,7 @@ func (c *Controller) handleAddVlan(key string) error { } if needUpdate { - _, err = c.config.KubeOvnClient.KubeovnV1().Vlans().UpdateStatus(context.Background(), vlan, metav1.UpdateOptions{}) + vlan, err = c.config.KubeOvnClient.KubeovnV1().Vlans().UpdateStatus(context.Background(), vlan, metav1.UpdateOptions{}) if err != nil { klog.Errorf("failed to update status of vlan %s: %v", vlan.Name, err) return err diff --git a/pkg/controller/vpc.go b/pkg/controller/vpc.go index 4f18b547211..260b071bc77 100644 --- a/pkg/controller/vpc.go +++ b/pkg/controller/vpc.go @@ -274,7 +274,7 @@ func (c *Controller) handleAddOrUpdateVpc(key string) error { } vpc = cachedVpc.DeepCopy() - if err = formatVpc(vpc, c); err != nil { + if vpc, err = c.formatVpc(vpc); err != nil { klog.Errorf("failed to format vpc %s: %v", key, err) return err } @@ -839,7 +839,7 @@ func getStaticRouteItemKey(item *kubeovnv1.StaticRoute) string { return key } -func formatVpc(vpc *kubeovnv1.Vpc, c *Controller) error { +func (c *Controller) formatVpc(vpc *kubeovnv1.Vpc) (*kubeovnv1.Vpc, error) { var changed bool for _, item := range vpc.Spec.StaticRoutes { // check policy @@ -848,19 +848,19 @@ func formatVpc(vpc *kubeovnv1.Vpc, c *Controller) error { changed = true } if item.Policy != kubeovnv1.PolicyDst && item.Policy != kubeovnv1.PolicySrc { - return fmt.Errorf("unknown policy type: %s", item.Policy) + return nil, fmt.Errorf("unknown policy type: %s", item.Policy) } // check cidr if strings.Contains(item.CIDR, "/") { if _, _, err := net.ParseCIDR(item.CIDR); err != nil { - return fmt.Errorf("invalid cidr %s: %w", item.CIDR, err) + return nil, fmt.Errorf("invalid cidr %s: %w", item.CIDR, err) } } else if ip := net.ParseIP(item.CIDR); ip == nil { - return fmt.Errorf("invalid ip %s", item.CIDR) + return nil, fmt.Errorf("invalid ip %s", item.CIDR) } // check next hop ip if ip := net.ParseIP(item.NextHopIP); ip == nil { - return fmt.Errorf("invalid next hop ip %s", item.NextHopIP) + return nil, fmt.Errorf("invalid next hop ip %s", item.NextHopIP) } } @@ -876,20 +876,22 @@ func formatVpc(vpc *kubeovnv1.Vpc, c *Controller) error { if ip := net.ParseIP(ipStr); ip == nil { err := fmt.Errorf("invalid next hop ips: %s", route.NextHopIP) klog.Error(err) - return err + return nil, err } } } } if changed { - if _, err := c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{}); err != nil { + newVpc, err := c.config.KubeOvnClient.KubeovnV1().Vpcs().Update(context.Background(), vpc, metav1.UpdateOptions{}) + if err != nil { klog.Errorf("failed to update vpc %s: %v", vpc.Name, err) - return err + return nil, err } + return newVpc, err } - return nil + return vpc, nil } func convertPolicies(list []*kubeovnv1.PolicyRoute) string { diff --git a/test/e2e/ovn-vpc-nat-gw/e2e_test.go b/test/e2e/ovn-vpc-nat-gw/e2e_test.go index 1e7829a8512..8a8b5dd6334 100644 --- a/test/e2e/ovn-vpc-nat-gw/e2e_test.go +++ b/test/e2e/ovn-vpc-nat-gw/e2e_test.go @@ -748,7 +748,7 @@ var _ = framework.Describe("[group:ovn-vpc-nat-gw]", func() { CIDR: noBfdExtraSubnetV4Cidr, NextHopIP: gatewayV4, }) - _ = vpcClient.PatchSync(cachedVpc, noBfdVpc, nil, 180*time.Second) + _, err = vpcClient.Update(context.Background(), noBfdVpc, metav1.UpdateOptions{}) framework.ExpectNoError(err) ginkgo.By("Creating overlay subnet " + noBfdExtraSubnetName)