Skip to content

Commit

Permalink
fix e2e
Browse files Browse the repository at this point in the history
  • Loading branch information
bobz965 committed May 16, 2023
1 parent 904ce36 commit 98e890b
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 66 deletions.
7 changes: 4 additions & 3 deletions pkg/controller/vpc_nat_gw_eip.go
Original file line number Diff line number Diff line change
Expand Up @@ -836,13 +836,13 @@ func (c *Controller) getIptablesEipNat(eipV4IP string) (string, error) {
}
fips, err := c.iptablesFipsLister.List(selector)
if err != nil {
klog.Errorf("failed to get dnats, %v", err)
klog.Errorf("failed to get fips, %v", err)
return "", err
}
if len(fips) != 0 {
nats = append(nats, util.FipUsingEip)
}
snats, err := c.iptablesDnatRulesLister.List(selector)
snats, err := c.iptablesSnatRulesLister.List(selector)
if err != nil {
klog.Errorf("failed to get snats, %v", err)
return "", err
Expand Down Expand Up @@ -885,7 +885,8 @@ func (c *Controller) patchEipStatus(key, v4ip, redo, qos string, ready bool) err
klog.Error(err)
return err
}
if ready && nat != "" && eip.Status.Nat != nat {
// nat record all kinds of nat rules using this eip
if nat != "" && eip.Status.Nat != nat {
eip.Status.Nat = nat
changed = true
}
Expand Down
125 changes: 72 additions & 53 deletions pkg/controller/vpc_nat_gw_nat.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,19 +517,11 @@ func (c *Controller) handleAddIptablesFip(key string) error {
return err
}

// check if has another fip using this eip already
selector := labels.SelectorFromSet(labels.Set{util.IptablesEipV4IPLabel: eip.Spec.V4ip})
usingfips, err := c.iptablesFipsLister.List(selector)
if err != nil {
klog.Errorf("failed to get dnats, %v", err)
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
}
for _, uf := range usingfips {
if uf.Name != key {
err = fmt.Errorf("failed to create fip %s, eip '%s' is used by other fip %s", key, eipName, uf.Name)
return err
}
}

// create fip nat
if err = c.createFipInPod(eip.Spec.NatGwDp, eip.Status.IP, fip.Spec.InternalIp); err != nil {
Expand All @@ -540,20 +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.patchEipStatus(eipName, "", "", "", true); err != nil {
// refresh eip nats
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
}
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
}

Expand Down Expand Up @@ -601,19 +611,11 @@ func (c *Controller) handleUpdateIptablesFip(key string) error {
return err
}

// check if has another fip using this eip already
selector := labels.SelectorFromSet(labels.Set{util.IptablesEipV4IPLabel: eip.Spec.V4ip})
usingfips, err := c.iptablesFipsLister.List(selector)
if err != nil {
klog.Errorf("failed to get dnats, %v", err)
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
}
for _, uf := range usingfips {
if uf.Name != key {
err = fmt.Errorf("failed to create fip %s, eip '%s' is used by other fip %s", key, eipName, uf.Name)
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 {
Expand All @@ -630,16 +632,16 @@ func (c *Controller) handleUpdateIptablesFip(key string) error {
}
// fip change eip
if c.fipChangeEip(cachedFip, eip) {
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
}
// 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.patchEipStatus(eipName, "", "", "", true); err != nil {
// refresh eip nats
klog.Errorf("failed to patch fip use eip %s, %v", key, err)
return err
}
return nil
}
// redo
Expand Down Expand Up @@ -714,18 +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.patchEipStatus(eipName, "", "", "", true); err != nil {
// refresh eip nats
klog.Errorf("failed to patch fip 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)
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
Expand Down Expand Up @@ -800,16 +802,16 @@ 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.patchEipStatus(eipName, "", "", "", true); err != nil {
// refresh eip nats
klog.Errorf("failed to patch fip 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.patchEipStatus(eipName, "", "", "", true); err != nil {
// refresh eip nats
klog.Errorf("failed to patch dnat use eip %s, %v", key, err)
return err
}
}
// redo
if !cachedDnat.Status.Ready &&
Expand Down Expand Up @@ -886,17 +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.patchEipStatus(eipName, "", "", "", true); err != nil {
// refresh eip nats
klog.Errorf("failed to patch fip 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
}
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
Expand Down Expand Up @@ -972,15 +974,15 @@ func (c *Controller) handleUpdateIptablesSnatRule(key string) error {
}
// snat change eip
if c.snatChangeEip(cachedSnat, eip) {
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
}
if err = c.patchSnatLabel(key, eip); err != nil {
klog.Errorf("failed to patch label for snat %s, %v", key, err)
return err
}
}
// redo
if !cachedSnat.Status.Ready &&
Expand Down Expand Up @@ -1291,18 +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.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
}
}
Expand Down Expand Up @@ -1425,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 {
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
}
}
Expand Down Expand Up @@ -1548,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 {
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
}
}
Expand Down
32 changes: 22 additions & 10 deletions test/e2e/iptables-vpc-nat-gw/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const dockerNetworkName = "kube-ovn-vlan"
const vpcNatGWConfigMapName = "ovn-vpc-nat-gw-config"
const networkAttachDefName = "ovn-vpc-external-network"
const externalSubnetProvider = "ovn-vpc-external-network.kube-system"
const sharedEipStatusNat = "dnat,fip,snat"

var _ = framework.Describe("[group:iptables-vpc-nat-gw]", func() {
f := framework.NewDefaultFramework("iptables-vpc-nat-gw")
Expand Down Expand Up @@ -76,7 +75,6 @@ var _ = framework.Describe("[group:iptables-vpc-nat-gw]", func() {
sharedEipSnatName = "shared-eip-snat-" + framework.RandomSuffix()
sharedEipFipShoudOkName = "shared-eip-fip-should-ok-" + framework.RandomSuffix()
sharedEipFipShoudFailName = "shared-eip-fip-should-fail-" + framework.RandomSuffix()
dnatName = "dnat-" + framework.RandomSuffix()

snatEipName = "snat-eip-" + framework.RandomSuffix()
snatName = "snat-" + framework.RandomSuffix()
Expand Down Expand Up @@ -283,29 +281,41 @@ var _ = framework.Describe("[group:iptables-vpc-nat-gw]", func() {
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("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.CreateSync(shareFipShouldFail)

ginkgo.By("Get share eip")
shareEip = iptablesEIPClient.Get(sharedEipName)
shareDnat = iptablesDnatRuleClient.Get(sharedEipName)
shareSnat = iptablesSnatRuleClient.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, ","))
Expand All @@ -332,6 +342,8 @@ 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)
Expand Down

0 comments on commit 98e890b

Please sign in to comment.