Skip to content

Commit

Permalink
u2o: fix ip cr's mac field does not match with lrp mac (#4107)
Browse files Browse the repository at this point in the history
Signed-off-by: zhangzujian <[email protected]>
  • Loading branch information
zhangzujian committed May 31, 2024
1 parent db821d2 commit d62aace
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 20 deletions.
2 changes: 2 additions & 0 deletions charts/kube-ovn/templates/kube-ovn-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,8 @@ spec:
type: string
u2oInterconnectionIP:
type: string
u2oInterconnectionMAC:
type: string
u2oInterconnectionVPC:
type: string
v4usingIPrange:
Expand Down
2 changes: 2 additions & 0 deletions dist/images/install.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2208,6 +2208,8 @@ spec:
type: string
u2oInterconnectionIP:
type: string
u2oInterconnectionMAC:
type: string
u2oInterconnectionVPC:
type: string
v4usingIPrange:
Expand Down
16 changes: 8 additions & 8 deletions mocks/pkg/ovs/interface.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pkg/apis/kubeovn/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ type SubnetStatus struct {
DHCPv4OptionsUUID string `json:"dhcpV4OptionsUUID"`
DHCPv6OptionsUUID string `json:"dhcpV6OptionsUUID"`
U2OInterconnectionIP string `json:"u2oInterconnectionIP"`
U2OInterconnectionMAC string `json:"u2oInterconnectionMAC"`
U2OInterconnectionVPC string `json:"u2oInterconnectionVPC"`
NatOutgoingPolicyRules []NatOutgoingPolicyRuleStatus `json:"natOutgoingPolicyRules"`
}
Expand Down
16 changes: 15 additions & 1 deletion pkg/controller/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/config"
Expand Down Expand Up @@ -301,7 +302,20 @@ func (c *Controller) InitIPAM() error {
u2oInterconnName := fmt.Sprintf(util.U2OInterconnName, subnet.Spec.Vpc, subnet.Name)
u2oInterconnLrpName := fmt.Sprintf("%s-%s", subnet.Spec.Vpc, subnet.Name)
if subnet.Status.U2OInterconnectionIP != "" {
if _, _, _, err = c.ipam.GetStaticAddress(u2oInterconnName, u2oInterconnLrpName, subnet.Status.U2OInterconnectionIP, nil, subnet.Name, true); err != nil {
var mac *string
if subnet.Status.U2OInterconnectionMAC != "" {
mac = ptr.To(subnet.Status.U2OInterconnectionMAC)
} else {
lrp, err := c.OVNNbClient.GetLogicalRouterPort(u2oInterconnLrpName, true)
if err != nil {
klog.Errorf("failed to get logical router port %s: %v", u2oInterconnLrpName, err)
return err
}
if lrp != nil {
mac = ptr.To(lrp.MAC)
}
}
if _, _, _, err = c.ipam.GetStaticAddress(u2oInterconnName, u2oInterconnLrpName, subnet.Status.U2OInterconnectionIP, mac, subnet.Name, true); err != nil {
klog.Errorf("failed to init subnet %q u2o interconnection ip to ipam %v", subnet.Name, err)
}
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/controller/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -823,8 +823,10 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error {
// 3. underlay subnet use physical gw, vpc has eip, lrp managed in vpc process, lrp ip is random allocation, not subnet gw

gateway := subnet.Spec.Gateway
var gatewayMAC string
if subnet.Status.U2OInterconnectionIP != "" && subnet.Spec.U2OInterconnection {
gateway = subnet.Status.U2OInterconnectionIP
gatewayMAC = subnet.Status.U2OInterconnectionMAC
}

if err := c.clearOldU2OResource(subnet); err != nil {
Expand All @@ -833,7 +835,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error {
}

// create or update logical switch
if err := c.OVNNbClient.CreateLogicalSwitch(subnet.Name, vpc.Status.Router, subnet.Spec.CIDRBlock, gateway, needRouter, randomAllocateGW); err != nil {
if err := c.OVNNbClient.CreateLogicalSwitch(subnet.Name, vpc.Status.Router, subnet.Spec.CIDRBlock, gateway, gatewayMAC, needRouter, randomAllocateGW); err != nil {
klog.Errorf("create logical switch %s: %v", subnet.Name, err)
return err
}
Expand Down Expand Up @@ -1959,7 +1961,7 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err
u2oInterconnLrpName := fmt.Sprintf("%s-%s", subnet.Spec.Vpc, subnet.Name)
var v4ip, v6ip, mac string
var err error
if subnet.Spec.U2OInterconnectionIP == "" && subnet.Status.U2OInterconnectionIP == "" {
if subnet.Spec.U2OInterconnectionIP == "" && (subnet.Status.U2OInterconnectionIP == "" || subnet.Status.U2OInterconnectionMAC == "") {
v4ip, v6ip, mac, err = c.acquireIPAddress(subnet.Name, u2oInterconnName, u2oInterconnLrpName)
if err != nil {
klog.Errorf("failed to acquire underlay to overlay interconnection ip address for subnet %s, %v", subnet.Name, err)
Expand Down Expand Up @@ -1992,13 +1994,15 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err
return err
}

subnet.Status.U2OInterconnectionMAC = mac
needCalcIP = true
}
} else if subnet.Status.U2OInterconnectionIP != "" {
u2oInterconnName := fmt.Sprintf(util.U2OInterconnName, subnet.Spec.Vpc, subnet.Name)
klog.Infof("release underlay to overlay interconnection ip address %s for subnet %s", subnet.Status.U2OInterconnectionIP, subnet.Name)
c.ipam.ReleaseAddressByPod(u2oInterconnName, subnet.Name)
subnet.Status.U2OInterconnectionIP = ""
subnet.Status.U2OInterconnectionMAC = ""
subnet.Status.U2OInterconnectionVPC = ""

if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), u2oInterconnName, metav1.DeleteOptions{}); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/ovs/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type BFD interface {
}

type LogicalSwitch interface {
CreateLogicalSwitch(lsName, lrName, cidrBlock, gateway string, needRouter, randomAllocateGW bool) error
CreateLogicalSwitch(lsName, lrName, cidrBlock, gateway, gatewayMAC string, needRouter, randomAllocateGW bool) error
CreateBareLogicalSwitch(lsName string) error
LogicalSwitchUpdateLoadBalancers(lsName string, op ovsdb.Mutator, lbNames ...string) error
LogicalSwitchUpdateOtherConfig(lsName string, op ovsdb.Mutator, otherConfig map[string]string) error
Expand Down
11 changes: 8 additions & 3 deletions pkg/ovs/ovn-nb-logical_switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

// CreateLogicalSwitch create logical switch
func (c *OVNNbClient) CreateLogicalSwitch(lsName, lrName, cidrBlock, gateway string, needRouter, randomAllocateGW bool) error {
func (c *OVNNbClient) CreateLogicalSwitch(lsName, lrName, cidrBlock, gateway, gatewayMAC string, needRouter, randomAllocateGW bool) error {
lspName := fmt.Sprintf("%s-%s", lsName, lrName)
lrpName := fmt.Sprintf("%s-%s", lrName, lsName)

Expand All @@ -41,7 +41,12 @@ func (c *OVNNbClient) CreateLogicalSwitch(lsName, lrName, cidrBlock, gateway str
Name: lrpName,
Networks: strings.Split(networks, ","),
}
if err := c.UpdateLogicalRouterPort(lrp, &lrp.Networks); err != nil {
fields := []interface{}{&lrp.Networks}
if gatewayMAC != "" {
lrp.MAC = gatewayMAC
fields = append(fields, &lrp.MAC)
}
if err := c.UpdateLogicalRouterPort(lrp, fields...); err != nil {
return fmt.Errorf("update logical router port %s", lrpName)
}
} else {
Expand All @@ -51,7 +56,7 @@ func (c *OVNNbClient) CreateLogicalSwitch(lsName, lrName, cidrBlock, gateway str
}

if needRouter {
if err := c.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, networks, util.GenerateMac()); err != nil {
if err := c.CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, networks, gatewayMAC); err != nil {
return err
}
} else {
Expand Down
15 changes: 10 additions & 5 deletions pkg/ovs/ovn-nb-logical_switch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,15 @@ func (suite *OvnClientTestSuite) testCreateLogicalSwitch() {
ovnClient := suite.ovnClient
lsName := "test-create-ls-ls"
lrName := "test-create-ls-lr"
mac := util.GenerateMac()
lspName := fmt.Sprintf("%s-%s", lsName, lrName)
lrpName := fmt.Sprintf("%s-%s", lrName, lsName)

err := ovnClient.CreateLogicalRouter(lrName)
require.NoError(t, err)

t.Run("create logical switch and router type port when logical switch does't exist and needRouter is true", func(t *testing.T) {
err = ovnClient.CreateLogicalSwitch(lsName, lrName, "192.168.2.0/24,fd00::c0a8:6400/120", "192.168.2.1,fd00::c0a8:6401", true, false)
err = ovnClient.CreateLogicalSwitch(lsName, lrName, "192.168.2.0/24,fd00::c0a8:6400/120", "192.168.2.1,fd00::c0a8:6401", mac, true, false)
require.NoError(t, err)

_, err := ovnClient.GetLogicalSwitch(lsName, false)
Expand All @@ -47,26 +48,30 @@ func (suite *OvnClientTestSuite) testCreateLogicalSwitch() {
_, err = ovnClient.GetLogicalSwitchPort(lspName, false)
require.NoError(t, err)

_, err = ovnClient.GetLogicalRouterPort(lrpName, false)
lrp, err := ovnClient.GetLogicalRouterPort(lrpName, false)
require.NoError(t, err)
require.NotNil(t, lrp)
require.Equal(t, mac, lrp.MAC)
})

t.Run("only update networks when logical switch exist and router type port exist and needRouter is true", func(t *testing.T) {
err = ovnClient.CreateLogicalSwitch(lsName, lrName, "192.168.2.0/24,fd00::c0a8:9900/120", "192.168.2.1,fd00::c0a8:9901", true, false)
err = ovnClient.CreateLogicalSwitch(lsName, lrName, "192.168.2.0/24,fd00::c0a8:9900/120", "192.168.2.1,fd00::c0a8:9901", mac, true, false)
require.NoError(t, err)

lrp, err := ovnClient.GetLogicalRouterPort(lrpName, false)
require.NoError(t, err)
require.NotNil(t, lrp)
require.ElementsMatch(t, []string{"192.168.2.1/24", "fd00::c0a8:9901/120"}, lrp.Networks)
require.Equal(t, mac, lrp.MAC)
})

t.Run("remove router type port when needRouter is false", func(t *testing.T) {
err = ovnClient.CreateLogicalSwitch(lsName, lrName, "192.168.2.0/24,fd00::c0a8:9900/120", "192.168.2.1,fd00::c0a8:9901", false, false)
err = ovnClient.CreateLogicalSwitch(lsName, lrName, "192.168.2.0/24,fd00::c0a8:9900/120", "192.168.2.1,fd00::c0a8:9901", "", false, false)
require.NoError(t, err)
})

t.Run("should no err when router type port doest't exist", func(t *testing.T) {
err = ovnClient.CreateLogicalSwitch(lsName+"-1", lrName+"-1", "192.168.2.0/24,fd00::c0a8:9900/120", "192.168.2.1,fd00::c0a8:9901", false, false)
err = ovnClient.CreateLogicalSwitch(lsName+"-1", lrName+"-1", "192.168.2.0/24,fd00::c0a8:9900/120", "192.168.2.1,fd00::c0a8:9901", "", false, false)
require.NoError(t, err)
})
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/ovs/ovn-nb.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ func (c *OVNNbClient) CreateLogicalPatchPort(lsName, lrName, lspName, lrpName, i
return err
}
}
if mac == "" {
mac = util.GenerateMac()
}

/* create router port */
ops, err := c.CreateRouterPortOp(lsName, lrName, lspName, lrpName, ip, mac)
Expand Down

0 comments on commit d62aace

Please sign in to comment.