From a30daea4f0a571d0db4e1950a8744cf8476255d8 Mon Sep 17 00:00:00 2001 From: bobz965 Date: Tue, 16 May 2023 18:42:55 +0800 Subject: [PATCH] Iptables nat support share eip (#2805) * support eip share in different nat --- pkg/controller/vpc_nat_gateway.go | 2 +- pkg/controller/vpc_nat_gw_eip.go | 203 +++++++---------------- pkg/controller/vpc_nat_gw_nat.go | 178 ++++++++++---------- pkg/util/const.go | 1 + test/e2e/iptables-vpc-nat-gw/e2e_test.go | 70 ++++++++ 5 files changed, 217 insertions(+), 237 deletions(-) diff --git a/pkg/controller/vpc_nat_gateway.go b/pkg/controller/vpc_nat_gateway.go index 450c24217aa..e53e170e057 100644 --- a/pkg/controller/vpc_nat_gateway.go +++ b/pkg/controller/vpc_nat_gateway.go @@ -486,7 +486,7 @@ func (c *Controller) handleUpdateVpcEip(natGwKey string) error { for _, eip := range eips { if eip.Spec.NatGwDp == natGwKey && eip.Status.Redo != NAT_GW_CREATED_AT && eip.Annotations[util.VpcNatAnnotation] == "" { klog.V(3).Infof("redo eip %s", eip.Name) - if err = c.patchEipStatus(eip.Name, "", NAT_GW_CREATED_AT, "", "", false); err != nil { + if err = c.patchEipStatus(eip.Name, "", NAT_GW_CREATED_AT, "", false); err != nil { klog.Errorf("failed to update eip '%s' to re-apply, %v", eip.Name, err) return err } diff --git a/pkg/controller/vpc_nat_gw_eip.go b/pkg/controller/vpc_nat_gw_eip.go index 755d154314c..cf5f04ae15d 100644 --- a/pkg/controller/vpc_nat_gw_eip.go +++ b/pkg/controller/vpc_nat_gw_eip.go @@ -263,74 +263,22 @@ func (c *Controller) handleAddIptablesEip(key string) error { return nil } -func (c *Controller) checkEipBindNat(key string, eip *kubeovnv1.IptablesEIP) (bool, string, error) { - var notUse bool - var natName string - - switch eip.Status.Nat { - case util.FipUsingEip: - notUse = true - case util.DnatUsingEip: - // nat change eip not that fast - dnats, err := c.iptablesDnatRulesLister.List(labels.SelectorFromSet(labels.Set{util.VpcNatGatewayNameLabel: key})) - if err != nil { - klog.Errorf("failed to get dnats, %v", err) - return notUse, natName, err - } - notUse = true - for _, item := range dnats { - if item.Annotations[util.VpcEipAnnotation] == key { - notUse = false - natName = item.Name - break - } - } - case util.SnatUsingEip: - // nat change eip not that fast - snats, err := c.iptablesSnatRulesLister.List(labels.SelectorFromSet(labels.Set{util.VpcNatGatewayNameLabel: key})) - if err != nil { - klog.Errorf("failed to get snats, %v", err) - return notUse, natName, err - } - notUse = true - for _, item := range snats { - if item.Annotations[util.VpcEipAnnotation] == key { - notUse = false - natName = item.Name - break - } - } - default: - notUse = true - } - return notUse, natName, nil -} - func (c *Controller) handleResetIptablesEip(key string) error { klog.V(3).Infof("handle reset eip %s", key) - eip, err := c.iptablesEipsLister.Get(key) - if err != nil { + if _, err := c.iptablesEipsLister.Get(key); err != nil { if k8serrors.IsNotFound(err) { return nil } return err } - notUse, _, err := c.checkEipBindNat(key, eip) - if err != nil { - klog.Errorf("failed to check nats bound to eip, %v", err) + if err := c.patchEipLabel(key); err != nil { + klog.Errorf("failed to patch label for eip %s, %v", key, err) return err } - - if notUse { - if err := c.natLabelEip(key, "", eip.Spec.QoSPolicy); err != nil { - klog.Errorf("failed to clean label for eip %s, %v", key, err) - return err - } - if err := c.patchResetEipStatusNat(key, ""); err != nil { - klog.Errorf("failed to clean status for eip %s, %v", key, err) - return err - } + if err := c.patchEipStatus(key, "", "", "", true); err != nil { + klog.Errorf("failed to reset nat for eip %s, %v", key, err) + return err } return nil } @@ -403,13 +351,7 @@ func (c *Controller) handleUpdateIptablesEip(key string) error { } } - _, natName, err := c.checkEipBindNat(key, cachedEip) - if err != nil { - klog.Errorf("failed to check nats bound to eip, %v", err) - return err - } - - if err = c.natLabelEip(key, natName, cachedEip.Spec.QoSPolicy); err != nil { + if err = c.patchEipLabel(key); err != nil { klog.Errorf("failed to label qos in eip, %v", err) return err } @@ -447,7 +389,7 @@ func (c *Controller) handleUpdateIptablesEip(key string) error { } } - if err = c.patchEipStatus(key, "", "", "", cachedEip.Spec.QoSPolicy, true); err != nil { + if err = c.patchEipStatus(key, "", "", cachedEip.Spec.QoSPolicy, true); err != nil { klog.Errorf("failed to patch status for eip %s, %v", key, err) return err } @@ -698,6 +640,7 @@ func (c *Controller) createOrUpdateCrdEip(key, v4ip, v6ip, mac, natGwDp, qos, ex Name: key, Labels: map[string]string{ util.SubnetNameLabel: externalNet, + util.IptablesEipV4IPLabel: v4ip, util.VpcNatGatewayNameLabel: natGwDp, }, }, @@ -759,6 +702,7 @@ func (c *Controller) createOrUpdateCrdEip(key, v4ip, v6ip, mac, natGwDp, qos, ex eip.Labels = map[string]string{ util.SubnetNameLabel: externalNetwork, util.VpcNatGatewayNameLabel: natGwDp, + util.IptablesEipV4IPLabel: v4ip, } needUpdateLabel = true } else if eip.Labels[util.SubnetNameLabel] != externalNetwork { @@ -879,7 +823,38 @@ func (c *Controller) patchEipQoSStatus(key, qos string) error { return nil } -func (c *Controller) patchEipStatus(key, v4ip, redo, nat, qos string, ready bool) error { +func (c *Controller) getIptablesEipNat(eipV4IP string) (string, error) { + nats := make([]string, 0, 3) + selector := labels.SelectorFromSet(labels.Set{util.IptablesEipV4IPLabel: eipV4IP}) + dnats, err := c.iptablesDnatRulesLister.List(selector) + if err != nil { + klog.Errorf("failed to get dnats, %v", err) + return "", err + } + if len(dnats) != 0 { + nats = append(nats, util.DnatUsingEip) + } + fips, err := c.iptablesFipsLister.List(selector) + if err != nil { + klog.Errorf("failed to get fips, %v", err) + return "", err + } + if len(fips) != 0 { + nats = append(nats, util.FipUsingEip) + } + snats, err := c.iptablesSnatRulesLister.List(selector) + if err != nil { + klog.Errorf("failed to get snats, %v", err) + return "", err + } + if len(snats) != 0 { + nats = append(nats, util.SnatUsingEip) + } + nat := strings.Join(nats, ",") + return nat, nil +} + +func (c *Controller) patchEipStatus(key, v4ip, redo, qos string, ready bool) error { oriEip, err := c.iptablesEipsLister.Get(key) if err != nil { if k8serrors.IsNotFound(err) { @@ -903,7 +878,15 @@ func (c *Controller) patchEipStatus(key, v4ip, redo, nat, qos string, ready bool eip.Status.IP = v4ip changed = true } - if ready && nat != "" && eip.Status.Nat != nat { + + nat, err := c.getIptablesEipNat(oriEip.Spec.V4ip) + if err != nil { + err := fmt.Errorf("failed to get eip nat") + klog.Error(err) + return err + } + // nat record all kinds of nat rules using this eip + if nat != "" && eip.Status.Nat != nat { eip.Status.Nat = nat changed = true } @@ -930,62 +913,7 @@ func (c *Controller) patchEipStatus(key, v4ip, redo, nat, qos string, ready bool return nil } -func (c *Controller) patchEipNat(key, nat string) error { - oriEip, err := c.iptablesEipsLister.Get(key) - if err != nil { - if k8serrors.IsNotFound(err) { - return nil - } - return err - } - if oriEip.Status.Nat == nat { - return nil - } - eip := oriEip.DeepCopy() - eip.Status.Nat = nat - bytes, err := eip.Status.Bytes() - if err != nil { - klog.Errorf("failed to marshal eip %s, %v", eip.Name, err) - return err - } - if _, err = c.config.KubeOvnClient.KubeovnV1().IptablesEIPs().Patch(context.Background(), key, types.MergePatchType, - bytes, metav1.PatchOptions{}, "status"); err != nil { - if k8serrors.IsNotFound(err) { - return nil - } - klog.Errorf("failed to patch eip %s, %v", eip.Name, err) - return err - } - return nil -} - -func (c *Controller) patchResetEipStatusNat(key, nat string) error { - oriEip, err := c.iptablesEipsLister.Get(key) - if err != nil { - if k8serrors.IsNotFound(err) { - return nil - } - return err - } - eip := oriEip.DeepCopy() - if eip.Status.Nat != nat { - eip.Status.Nat = nat - bytes, err := eip.Status.Bytes() - if err != nil { - return err - } - if _, err = c.config.KubeOvnClient.KubeovnV1().IptablesEIPs().Patch(context.Background(), key, types.MergePatchType, - bytes, metav1.PatchOptions{}, "status"); err != nil { - if k8serrors.IsNotFound(err) { - return nil - } - klog.Errorf("failed to patch eip '%s' nat type, %v", eip.Name, err) - return err - } - } - return nil -} -func (c *Controller) natLabelEip(eipName, natName, qosName string) error { +func (c *Controller) patchEipLabel(eipName string) error { oriEip, err := c.iptablesEipsLister.Get(eipName) if err != nil { if k8serrors.IsNotFound(err) { @@ -996,7 +924,7 @@ func (c *Controller) natLabelEip(eipName, natName, qosName string) error { externalNetwork := util.GetExternalNetwork(oriEip.Spec.ExternalSubnet) eip := oriEip.DeepCopy() - var needUpdateLabel, needUpdateAnno bool + var needUpdateLabel bool var op string if len(eip.Labels) == 0 { op = "add" @@ -1004,36 +932,21 @@ func (c *Controller) natLabelEip(eipName, natName, qosName string) error { eip.Labels = map[string]string{ util.SubnetNameLabel: externalNetwork, util.VpcNatGatewayNameLabel: eip.Spec.NatGwDp, - util.QoSLabel: qosName, + util.QoSLabel: eip.Spec.QoSPolicy, + util.IptablesEipV4IPLabel: eip.Spec.V4ip, } - } else if eip.Labels[util.VpcNatGatewayNameLabel] != eip.Spec.NatGwDp || eip.Labels[util.QoSLabel] != qosName { + } else if eip.Labels[util.VpcNatGatewayNameLabel] != eip.Spec.NatGwDp || eip.Labels[util.QoSLabel] != eip.Spec.QoSPolicy { op = "replace" needUpdateLabel = true eip.Labels[util.SubnetNameLabel] = externalNetwork eip.Labels[util.VpcNatGatewayNameLabel] = eip.Spec.NatGwDp - eip.Labels[util.QoSLabel] = qosName + eip.Labels[util.QoSLabel] = eip.Spec.QoSPolicy + eip.Labels[util.IptablesEipV4IPLabel] = eip.Spec.V4ip } if needUpdateLabel { if err := c.updateIptableLabels(eip.Name, op, "eip", eip.Labels); err != nil { return err } } - - if len(eip.Annotations) == 0 { - op = "add" - needUpdateAnno = true - eip.Annotations = map[string]string{ - util.VpcNatAnnotation: natName, - } - } else if eip.Annotations[util.VpcNatAnnotation] != natName { - op = "replace" - needUpdateAnno = true - eip.Annotations[util.VpcNatAnnotation] = natName - } - if needUpdateAnno { - if err := c.updateIptableAnnotations(eip.Name, op, "eip", eip.Annotations); err != nil { - return err - } - } return err } diff --git a/pkg/controller/vpc_nat_gw_nat.go b/pkg/controller/vpc_nat_gw_nat.go index 9c39b54649f..ddffd4da8fd 100644 --- a/pkg/controller/vpc_nat_gw_nat.go +++ b/pkg/controller/vpc_nat_gw_nat.go @@ -516,16 +516,10 @@ func (c *Controller) handleAddIptablesFip(key string) error { klog.Errorf("failed to get eip, %v", err) return err } - if eip.Status.Nat != "" && eip.Status.Nat != util.FipUsingEip { - // eip is in use by other nat - err = fmt.Errorf("failed to create fip %s, eip '%s' is used by other nat %s", key, eipName, eip.Status.Nat) - return err - } - if eip.Status.Nat == util.FipUsingEip && - eip.Annotations[util.VpcNatAnnotation] != "" && - eip.Annotations[util.VpcNatAnnotation] != fip.Name { - err = fmt.Errorf("failed to create fip %s, eip '%s' is used by other fip %s", key, eipName, eip.Annotations[util.VpcNatAnnotation]) + if err = c.fipTryUseEip(key, eip.Spec.V4ip); err != nil { + err = fmt.Errorf("failed to create fip %s, %v", key, err) + klog.Error(err) return err } @@ -538,23 +532,38 @@ func (c *Controller) handleAddIptablesFip(key string) error { klog.Errorf("failed to patch status for fip %s, %v", key, err) return err } - if err = c.patchEipNat(eipName, util.FipUsingEip); err != nil { - klog.Errorf("failed to patch fip use eip %s, %v", key, err) + // label too long cause error + if err = c.patchFipLabel(key, eip); err != nil { + klog.Errorf("failed to update label for fip %s, %v", key, err) return err } if err = c.handleAddIptablesFipFinalizer(key); err != nil { klog.Errorf("failed to handle add finalizer for fip, %v", err) return err } - // label too long cause error - if err = c.patchFipLabel(key, eip); err != nil { - klog.Errorf("failed to update label for fip %s, %v", key, err) + if err = c.patchEipStatus(eipName, "", "", "", true); err != nil { + // refresh eip nats + klog.Errorf("failed to patch fip use eip %s, %v", key, err) return err } - if err = c.natLabelEip(eipName, fip.Name, eip.Spec.QoSPolicy); err != nil { - klog.Errorf("failed to label fip '%s' in eip %s, %v", fip.Name, eipName, err) + return nil +} + +func (c *Controller) fipTryUseEip(fipName, eipV4IP string) error { + // check if has another fip using this eip already + selector := labels.SelectorFromSet(labels.Set{util.IptablesEipV4IPLabel: eipV4IP}) + usingFips, err := c.iptablesFipsLister.List(selector) + if err != nil { + klog.Errorf("failed to get fips, %v", err) return err } + for _, uf := range usingFips { + if uf.Name != fipName { + err = fmt.Errorf("%s is using by the other fip %s", eipV4IP, uf.Name) + klog.Error(err) + return err + } + } return nil } @@ -601,18 +610,12 @@ func (c *Controller) handleUpdateIptablesFip(key string) error { klog.Errorf("failed to get eip, %v", err) return err } - if eip.Status.Nat != "" && eip.Status.Nat != util.FipUsingEip { - // eip is in use by other nat - err = fmt.Errorf("failed to update fip %s, eip '%s' is used by %s", key, eipName, eip.Status.Nat) + + if err = c.fipTryUseEip(key, eip.Spec.V4ip); err != nil { + err = fmt.Errorf("failed to update fip %s, %v", key, err) klog.Error(err) return err } - if eip.Status.Nat == util.FipUsingEip && - eip.Annotations[util.VpcAnnotation] != "" && - eip.Annotations[util.VpcAnnotation] != cachedFip.Name { - err = fmt.Errorf("failed to update fip %s, eip '%s' is used by other fip %s", key, eipName, eip.Annotations[util.VpcAnnotation]) - return err - } klog.V(3).Infof("fip change ip, old ip '%s', new ip %s", cachedFip.Status.V4ip, eip.Status.IP) if err = c.deleteFipInPod(cachedFip.Status.NatGwDp, cachedFip.Status.V4ip, cachedFip.Status.InternalIp); err != nil { @@ -629,17 +632,14 @@ func (c *Controller) handleUpdateIptablesFip(key string) error { } // fip change eip if c.fipChangeEip(cachedFip, eip) { - if err = c.patchEipNat(eipName, util.FipUsingEip); err != nil { - klog.Errorf("failed to patch fip use eip %s, %v", key, err) - return err - } // label too long cause error if err = c.patchFipLabel(key, eip); err != nil { klog.Errorf("failed to update label for fip %s, %v", key, err) return err } - if err = c.natLabelEip(eipName, cachedFip.Name, eip.Spec.QoSPolicy); err != nil { - klog.Errorf("failed to label fip '%s' in eip %s, %v", cachedFip.Name, eipName, err) + if err = c.patchEipStatus(eipName, "", "", "", true); err != nil { + // refresh eip nats + klog.Errorf("failed to patch fip use eip %s, %v", key, err) return err } return nil @@ -702,11 +702,6 @@ func (c *Controller) handleAddIptablesDnatRule(key string) error { klog.Errorf("failed to get eip, %v", err) return err } - if eip.Status.Nat != "" && eip.Status.Nat != util.DnatUsingEip { - // eip is in use by other nat - err = fmt.Errorf("failed to create dnat %s, eip '%s' is used by nat %s", key, eipName, eip.Status.Nat) - return err - } if dup, err := c.isDnatDuplicated(eip.Spec.NatGwDp, eipName, dnat.Name, dnat.Spec.ExternalPort); dup || err != nil { return err } @@ -721,21 +716,18 @@ func (c *Controller) handleAddIptablesDnatRule(key string) error { klog.Errorf("failed to patch status for dnat %s, %v", key, err) return err } - if err = c.patchEipNat(eipName, util.DnatUsingEip); err != nil { - klog.Errorf("failed to patch dnat use eip %s, %v", key, err) + // label too long cause error + if err = c.patchDnatLabel(key, eip); err != nil { + klog.Errorf("failed to patch label for dnat %s, %v", key, err) return err } if err = c.handleAddIptablesDnatFinalizer(key); err != nil { klog.Errorf("failed to handle add finalizer for dnat, %v", err) return err } - // label too long cause error - if err = c.patchDnatLabel(key, eip); err != nil { - klog.Errorf("failed to patch label for dnat %s, %v", key, err) - return err - } - if err = c.natLabelEip(eipName, dnat.Name, eip.Spec.QoSPolicy); err != nil { - klog.Errorf("failed to label dnat in eip, %v", err) + if err = c.patchEipStatus(eipName, "", "", "", true); err != nil { + // refresh eip nats + klog.Errorf("failed to patch dnat use eip %s, %v", key, err) return err } return nil @@ -782,11 +774,6 @@ func (c *Controller) handleUpdateIptablesDnatRule(key string) error { klog.Errorf("failed to get eip, %v", err) return err } - if eip.Status.Nat != "" && eip.Status.Nat != "dnat" { - // eip is in use by other nat - err = fmt.Errorf("failed to update dnat %s, eip '%s' is used by nat %s", key, eipName, eip.Status.Nat) - return err - } if dup, err := c.isDnatDuplicated(cachedDnat.Status.NatGwDp, eipName, cachedDnat.Name, cachedDnat.Spec.ExternalPort); dup || err != nil { klog.Errorf("failed to update dnat, %v", err) return err @@ -815,17 +802,14 @@ func (c *Controller) handleUpdateIptablesDnatRule(key string) error { // dnat change eip if c.dnatChangeEip(cachedDnat, eip) { klog.V(3).Infof("dnat change ip, old ip '%s', new ip %s", cachedDnat.Status.V4ip, eip.Status.IP) - if err = c.patchEipNat(eipName, util.DnatUsingEip); err != nil { - klog.Errorf("failed to patch dnat use eip %s, %v", key, err) - return err - } // label too long cause error if err = c.patchDnatLabel(key, eip); err != nil { klog.Errorf("failed to patch label for dnat %s, %v", key, err) return err } - if err = c.natLabelEip(eipName, cachedDnat.Name, eip.Spec.QoSPolicy); err != nil { - klog.Errorf("failed to label dnat '%s' in eip %s, %v", cachedDnat.Name, eipName, err) + if err = c.patchEipStatus(eipName, "", "", "", true); err != nil { + // refresh eip nats + klog.Errorf("failed to patch dnat use eip %s, %v", key, err) return err } } @@ -889,11 +873,6 @@ func (c *Controller) handleAddIptablesSnatRule(key string) error { klog.Errorf("failed to get eip, %v", err) return err } - if eip.Status.Nat != "" && eip.Status.Nat != "snat" { - // eip is in use by other nat - err = fmt.Errorf("failed to create snat %s, eip '%s' is used by nat '%s'", key, eipName, eip.Status.Nat) - return err - } // create snat v4Cidr, _ := util.SplitStringIP(snat.Spec.InternalCIDR) if v4Cidr == "" { @@ -909,21 +888,17 @@ func (c *Controller) handleAddIptablesSnatRule(key string) error { klog.Errorf("failed to update status for snat %s, %v", key, err) return err } - if err = c.patchEipNat(eipName, util.SnatUsingEip); err != nil { - klog.Errorf("failed to patch snat use eip %s, %v", key, err) + if err = c.patchSnatLabel(key, eip); err != nil { + klog.Errorf("failed to patch label for snat %s, %v", key, err) return err } if err = c.handleAddIptablesSnatFinalizer(key); err != nil { klog.Errorf("failed to handle add finalizer for snat, %v", err) return err } - // label too long cause error - if err = c.natLabelEip(eipName, snat.Name, eip.Spec.QoSPolicy); err != nil { - klog.Errorf("failed to label snat '%s' in eip %s, %v", snat.Name, eipName, err) - return err - } - if err = c.patchSnatLabel(key, eip); err != nil { - klog.Errorf("failed to patch label for snat %s, %v", key, err) + if err = c.patchEipStatus(eipName, "", "", "", true); err != nil { + // refresh eip nats + klog.Errorf("failed to patch snat use eip %s, %v", key, err) return err } return nil @@ -978,11 +953,7 @@ func (c *Controller) handleUpdateIptablesSnatRule(key string) error { klog.Errorf("failed to get eip, %v", err) return err } - if eip.Status.Nat != "" && eip.Status.Nat != "snat" { - // eip is in use by other nat - err = fmt.Errorf("failed to update snat %s, eip '%s' is used by %s", key, eipName, eip.Status.Nat) - return err - } + // add or update should make sure vpc nat enabled if vpcNatEnabled != "true" { return fmt.Errorf("iptables nat gw not enable") @@ -1003,19 +974,15 @@ func (c *Controller) handleUpdateIptablesSnatRule(key string) error { } // snat change eip if c.snatChangeEip(cachedSnat, eip) { - if err = c.patchEipNat(eipName, util.SnatUsingEip); err != nil { - klog.Errorf("failed to patch snat use eip %s, %v", key, err) - return err - } - // label too long cause error - if err = c.natLabelEip(eipName, cachedSnat.Name, eip.Spec.QoSPolicy); err != nil { - klog.Errorf("failed to label snat in eip, %v", err) - return err - } if err = c.patchSnatLabel(key, eip); err != nil { klog.Errorf("failed to patch label for snat %s, %v", key, err) return err } + if err = c.patchEipStatus(eipName, "", "", "", true); err != nil { + // refresh eip nats + klog.Errorf("failed to patch fip use eip %s, %v", key, err) + return err + } } // redo if !cachedSnat.Status.Ready && @@ -1178,11 +1145,14 @@ func (c *Controller) patchFipLabel(key string, eip *kubeovnv1.IptablesEIP) error op = "add" fip.Labels = map[string]string{ util.VpcNatGatewayNameLabel: eip.Spec.NatGwDp, + util.IptablesEipV4IPLabel: eip.Spec.V4ip, } needUpdateLabel = true - } else if fip.Labels[util.SubnetNameLabel] != eip.Spec.NatGwDp { + } else if fip.Labels[util.SubnetNameLabel] != eip.Spec.NatGwDp || + fip.Labels[util.IptablesEipV4IPLabel] != eip.Spec.V4ip { op = "replace" fip.Labels[util.VpcNatGatewayNameLabel] = eip.Spec.NatGwDp + fip.Labels[util.IptablesEipV4IPLabel] = eip.Spec.V4ip needUpdateLabel = true } if needUpdateLabel { @@ -1323,15 +1293,25 @@ func (c *Controller) redoFip(key, redo string, eipReady bool) error { if k8serrors.IsNotFound(err) { return nil } + klog.Errorf("failed to get fip %s, %v", key, err) return err } if redo != "" && redo != fip.Status.Redo { if !eipReady { - if err = c.patchEipStatus(fip.Spec.EIP, "", redo, "", "", false); err != nil { + if err = c.patchEipLabel(fip.Spec.EIP); err != nil { + err = fmt.Errorf("failed to patch eip %s, %v", fip.Spec.EIP, err) + klog.Error(err) + return err + } + if err = c.patchEipStatus(fip.Spec.EIP, "", redo, "", false); err != nil { + err = fmt.Errorf("failed to patch eip %s, %v", fip.Spec.EIP, err) + klog.Error(err) return err } } if err = c.patchFipStatus(key, "", "", "", redo, false); err != nil { + err = fmt.Errorf("failed to patch fip %s, %v", fip.Name, err) + klog.Error(err) return err } } @@ -1354,12 +1334,15 @@ func (c *Controller) patchDnatLabel(key string, eip *kubeovnv1.IptablesEIP) erro dnat.Labels = map[string]string{ util.VpcNatGatewayNameLabel: eip.Spec.NatGwDp, util.VpcDnatEPortLabel: dnat.Spec.ExternalPort, + util.IptablesEipV4IPLabel: eip.Spec.V4ip, } needUpdateLabel = true - } else if dnat.Labels[util.SubnetNameLabel] != eip.Spec.NatGwDp { + } else if dnat.Labels[util.SubnetNameLabel] != eip.Spec.NatGwDp || + dnat.Labels[util.IptablesEipV4IPLabel] != eip.Spec.V4ip { op = "replace" dnat.Labels[util.VpcNatGatewayNameLabel] = eip.Spec.NatGwDp dnat.Labels[util.VpcDnatEPortLabel] = dnat.Spec.ExternalPort + dnat.Labels[util.IptablesEipV4IPLabel] = eip.Spec.V4ip needUpdateLabel = true } if needUpdateLabel { @@ -1451,15 +1434,20 @@ func (c *Controller) redoDnat(key, redo string, eipReady bool) error { if k8serrors.IsNotFound(err) { return nil } + klog.Errorf("failed to get dnat %s, %v", key, err) return err } if redo != "" && redo != dnat.Status.Redo { if !eipReady { - if err = c.patchEipStatus(dnat.Spec.EIP, "", redo, "", "", false); err != nil { + if err = c.patchEipStatus(dnat.Spec.EIP, "", redo, "", false); err != nil { + err = fmt.Errorf("failed to patch eip %s, %v", dnat.Spec.EIP, err) + klog.Error(err) return err } } if err = c.patchDnatStatus(key, "", "", "", redo, false); err != nil { + err = fmt.Errorf("failed to patch dnat %s, %v", key, err) + klog.Error(err) return err } } @@ -1481,11 +1469,14 @@ func (c *Controller) patchSnatLabel(key string, eip *kubeovnv1.IptablesEIP) erro op = "add" snat.Labels = map[string]string{ util.VpcNatGatewayNameLabel: eip.Spec.NatGwDp, + util.IptablesEipV4IPLabel: eip.Spec.V4ip, } needUpdateLabel = true - } else if snat.Labels[util.SubnetNameLabel] != eip.Spec.NatGwDp { + } else if snat.Labels[util.SubnetNameLabel] != eip.Spec.NatGwDp || + snat.Labels[util.IptablesEipV4IPLabel] != eip.Spec.V4ip { op = "replace" snat.Labels[util.VpcNatGatewayNameLabel] = eip.Spec.NatGwDp + snat.Labels[util.IptablesEipV4IPLabel] = eip.Spec.V4ip needUpdateLabel = true } if needUpdateLabel { @@ -1571,15 +1562,20 @@ func (c *Controller) redoSnat(key, redo string, eipReady bool) error { if k8serrors.IsNotFound(err) { return nil } + klog.Errorf("failed to get snat %s, %v", key, err) return err } if redo != "" && redo != snat.Status.Redo { if !eipReady { - if err = c.patchEipStatus(snat.Spec.EIP, "", redo, "", "", false); err != nil { + if err = c.patchEipStatus(snat.Spec.EIP, "", redo, "", false); err != nil { + err = fmt.Errorf("failed to patch eip %s, %v", snat.Spec.EIP, err) + klog.Error(err) return err } } if err = c.patchSnatStatus(key, "", "", "", redo, false); err != nil { + err = fmt.Errorf("failed to patch snat %s, %v", key, err) + klog.Error(err) return err } } @@ -1748,7 +1744,7 @@ func (c *Controller) isDnatDuplicated(gwName, eipName, dnatName, externalPort st } if len(dnats) != 0 { for _, d := range dnats { - if d.Name != dnatName && d.Annotations[util.VpcEipAnnotation] == eipName { + if d.Name != dnatName && d.Spec.EIP == eipName { err = fmt.Errorf("failed to create dnat %s, duplicate, same eip %s, same external port '%s' is using by dnat %s", dnatName, eipName, externalPort, d.Name) return true, err } diff --git a/pkg/util/const.go b/pkg/util/const.go index 5c28ad4d703..3ab56619957 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -47,6 +47,7 @@ const ( VpcEipAnnotation = "ovn.kubernetes.io/vpc_eip" VpcDnatEPortLabel = "ovn.kubernetes.io/vpc_dnat_eport" VpcNatAnnotation = "ovn.kubernetes.io/vpc_nat" + IptablesEipV4IPLabel = "ovn.kubernetes.io/iptables_eip_v4_ip" OvnEipUsageLabel = "ovn.kubernetes.io/ovn_eip_usage" OvnLrpEipEnableBfdLabel = "ovn.kubernetes.io/ovn_lrp_eip_enable_bfd" diff --git a/test/e2e/iptables-vpc-nat-gw/e2e_test.go b/test/e2e/iptables-vpc-nat-gw/e2e_test.go index 089b1bfa5b6..7dc507579d8 100644 --- a/test/e2e/iptables-vpc-nat-gw/e2e_test.go +++ b/test/e2e/iptables-vpc-nat-gw/e2e_test.go @@ -44,6 +44,8 @@ var _ = framework.Describe("[group:iptables-vpc-nat-gw]", func() { var vpcNatGwClient *framework.VpcNatGatewayClient var subnetClient *framework.SubnetClient var fipVipName, fipEipName, fipName, dnatVipName, dnatEipName, dnatName, snatEipName, snatName string + // sharing case + var sharedVipName, sharedEipName, sharedEipDnatName, sharedEipSnatName, sharedEipFipShoudOkName, sharedEipFipShoudFailName string var vipClient *framework.VipClient var ipClient *framework.IpClient var iptablesEIPClient *framework.IptablesEIPClient @@ -66,6 +68,14 @@ var _ = framework.Describe("[group:iptables-vpc-nat-gw]", func() { dnatEipName = "dnat-eip-" + framework.RandomSuffix() dnatName = "dnat-" + framework.RandomSuffix() + // sharing case + sharedVipName = "shared-vip-" + framework.RandomSuffix() + sharedEipName = "shared-eip-" + framework.RandomSuffix() + sharedEipDnatName = "shared-eip-dnat-" + framework.RandomSuffix() + sharedEipSnatName = "shared-eip-snat-" + framework.RandomSuffix() + sharedEipFipShoudOkName = "shared-eip-fip-should-ok-" + framework.RandomSuffix() + sharedEipFipShoudFailName = "shared-eip-fip-should-fail-" + framework.RandomSuffix() + snatEipName = "snat-eip-" + framework.RandomSuffix() snatName = "snat-" + framework.RandomSuffix() overlaySubnetName = "overlay-subnet-" + framework.RandomSuffix() @@ -263,6 +273,62 @@ var _ = framework.Describe("[group:iptables-vpc-nat-gw]", func() { dnat := framework.MakeIptablesDnatRule(dnatName, dnatEipName, "80", "tcp", dnatVip.Status.V4ip, "8080") _ = iptablesDnatRuleClient.CreateSync(dnat) + // share eip case + ginkgo.By("Creating share vip") + shareVip := framework.MakeVip(sharedVipName, overlaySubnetName, "", "") + _ = vipClient.CreateSync(shareVip) + fipVip = vipClient.Get(fipVipName) + ginkgo.By("Creating share iptables eip") + shareEip := framework.MakeIptablesEIP(sharedEipName, "", "", "", vpcNatGwName) + _ = iptablesEIPClient.CreateSync(shareEip) + ginkgo.By("Creating the first iptables fip with share eip vip should be ok") + shareFipShouldOk := framework.MakeIptablesFIPRule(sharedEipFipShoudOkName, sharedEipName, fipVip.Status.V4ip) + _ = iptablesFIPClient.CreateSync(shareFipShouldOk) + ginkgo.By("Creating the second iptables fip with share eip vip should be failed") + shareFipShouldFail := framework.MakeIptablesFIPRule(sharedEipFipShoudFailName, sharedEipName, fipVip.Status.V4ip) + _ = iptablesFIPClient.Create(shareFipShouldFail) + ginkgo.By("Creating iptables dnat for dnat with share eip vip") + shareDnat := framework.MakeIptablesDnatRule(sharedEipDnatName, sharedEipName, "80", "tcp", fipVip.Status.V4ip, "8080") + _ = iptablesDnatRuleClient.CreateSync(shareDnat) + ginkgo.By("Creating iptables snat with share eip vip") + shareSnat := framework.MakeIptablesSnatRule(sharedEipSnatName, sharedEipName, overlaySubnetV4Cidr) + _ = iptablesSnatRuleClient.CreateSync(shareSnat) + + ginkgo.By("Get share eip") + shareEip = iptablesEIPClient.Get(sharedEipName) + ginkgo.By("Get share dnat") + shareDnat = iptablesDnatRuleClient.Get(sharedEipDnatName) + ginkgo.By("Get share snat") + shareSnat = iptablesSnatRuleClient.Get(sharedEipSnatName) + ginkgo.By("Get share fip should ok") + shareFipShouldOk = iptablesFIPClient.Get(sharedEipFipShoudOkName) + ginkgo.By("Get share fip should fail") + shareFipShouldFail = iptablesFIPClient.Get(sharedEipFipShoudFailName) + + ginkgo.By("Check share eip should has the external ip label") + framework.ExpectHaveKeyWithValue(shareEip.Labels, util.IptablesEipV4IPLabel, shareEip.Spec.V4ip) + ginkgo.By("Check share dnat should has the external ip label") + framework.ExpectHaveKeyWithValue(shareDnat.Labels, util.IptablesEipV4IPLabel, shareEip.Spec.V4ip) + ginkgo.By("Check share snat should has the external ip label") + framework.ExpectHaveKeyWithValue(shareSnat.Labels, util.IptablesEipV4IPLabel, shareEip.Spec.V4ip) + ginkgo.By("Check share fip should ok should has the external ip label") + framework.ExpectHaveKeyWithValue(shareFipShouldOk.Labels, util.IptablesEipV4IPLabel, shareEip.Spec.V4ip) + ginkgo.By("Check share fip should fail should not be ready") + framework.ExpectEqual(shareFipShouldFail.Status.Ready, false) + + // define a list and strings join + nats := []string{util.DnatUsingEip, util.FipUsingEip, util.SnatUsingEip} + framework.ExpectEqual(shareEip.Status.Nat, strings.Join(nats, ",")) + + ginkgo.By("Deleting share iptables fip " + sharedEipFipShoudOkName) + iptablesFIPClient.DeleteSync(sharedEipFipShoudOkName) + ginkgo.By("Deleting share iptables fip " + sharedEipFipShoudFailName) + iptablesFIPClient.DeleteSync(sharedEipFipShoudFailName) + ginkgo.By("Deleting share iptables dnat " + dnatName) + iptablesDnatRuleClient.DeleteSync(dnatName) + ginkgo.By("Deleting share iptables snat " + snatName) + iptablesSnatRuleClient.DeleteSync(snatName) + ginkgo.By("Deleting iptables fip " + fipName) iptablesFIPClient.DeleteSync(fipName) ginkgo.By("Deleting iptables dnat " + dnatName) @@ -276,11 +342,15 @@ var _ = framework.Describe("[group:iptables-vpc-nat-gw]", func() { iptablesEIPClient.DeleteSync(dnatEipName) ginkgo.By("Deleting iptables eip " + snatEipName) iptablesEIPClient.DeleteSync(snatEipName) + ginkgo.By("Deleting iptables share eip " + sharedEipName) + iptablesEIPClient.DeleteSync(sharedEipName) ginkgo.By("Deleting vip " + fipVipName) vipClient.DeleteSync(fipVipName) ginkgo.By("Deleting vip " + dnatVipName) vipClient.DeleteSync(dnatVipName) + ginkgo.By("Deleting vip " + sharedVipName) + vipClient.DeleteSync(sharedVipName) ginkgo.By("Deleting custom vpc " + vpcName) vpcClient.DeleteSync(vpcName)