Skip to content

Commit

Permalink
add ut for subnet.go and ovn-nb-acl.go (#4674)
Browse files Browse the repository at this point in the history
* add ut for subnet and ovn-nb-acl


---------

Signed-off-by: dolibali <[email protected]>
  • Loading branch information
dolibali authored Oct 30, 2024
1 parent f966ecc commit 45c54b6
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 26 deletions.
2 changes: 1 addition & 1 deletion pkg/ipam/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (s *Subnet) pushPodNic(podName, nicName string) {

func (s *Subnet) popPodNic(podName, nicName string) {
s.PodToNicList[podName] = util.RemoveString(s.PodToNicList[podName], nicName)
if s.PodToNicList[podName] == nil {
if len(s.PodToNicList[podName]) == 0 {
delete(s.PodToNicList, podName)
}
}
Expand Down
37 changes: 37 additions & 0 deletions pkg/ipam/subnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1099,3 +1099,40 @@ func TestSubnetReleaseAddr(t *testing.T) {
require.NotEmpty(t, macStr3)
subnet.releaseAddr(pod63Name, nic63Name)
}

func TestPopPodNic(t *testing.T) {
subnet, err := NewSubnet("v4Subnet", "10.0.0.0/24", nil)
require.NoError(t, err)
require.NotNil(t, subnet)

// 1. Existing pod and nic
podName := "pod1.default"
nicName := "nic1"
subnet.PodToNicList[podName] = []string{nicName}
subnet.popPodNic(podName, nicName)
require.Equal(t, 0, len(subnet.PodToNicList[podName]))

// 2. Non-existent nic
subnet.PodToNicList[podName] = []string{nicName}
subnet.popPodNic(podName, "nonexistentNic")
require.Equal(t, []string{nicName}, subnet.PodToNicList[podName])

// 3. List empty after removal
subnet.popPodNic(podName, nicName)
require.Equal(t, 0, len(subnet.PodToNicList[podName]))

// 4. Non-existent pod
subnet.popPodNic("nonexistentPod", nicName)
// Ensure no panic occurs and no changes in the map
require.Equal(t, 0, len(subnet.PodToNicList[podName]))

// 5. Multiple nics in the list
subnet.PodToNicList[podName] = []string{"nic1", "nic2", "nic3"}
subnet.popPodNic(podName, "nic2")
require.Equal(t, []string{"nic1", "nic3"}, subnet.PodToNicList[podName])

// 6. Existing pod and nil nic
subnet.PodToNicList[podName] = nil
subnet.popPodNic(podName, nicName)
require.Equal(t, 0, len(subnet.PodToNicList[podName]))
}
141 changes: 116 additions & 25 deletions pkg/ovs/ovn-nb-acl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,35 +577,51 @@ func (suite *OvnClientTestSuite) testCreateSgDenyAllACL() {
t.Parallel()

nbClient := suite.ovnNBClient
sgName := "test_create_deny_all_acl_pg"
pgName := GetSgPortGroupName(sgName)
failedNbClient := suite.faiedOvnNBClient

err := nbClient.CreatePortGroup(pgName, nil)
require.NoError(t, err)
t.Run("normal create sg deny all acl", func(t *testing.T) {
sgName := "test_create_deny_all_acl_pg"
pgName := GetSgPortGroupName(sgName)

err = nbClient.CreateSgDenyAllACL(sgName)
require.NoError(t, err)
err := nbClient.CreatePortGroup(pgName, nil)
require.NoError(t, err)

pg, err := nbClient.GetPortGroup(pgName, false)
require.NoError(t, err)
err = nbClient.CreateSgDenyAllACL(sgName)
require.NoError(t, err)

// ingress acl
match := fmt.Sprintf("outport == @%s && ip", pgName)
ingressACL, err := nbClient.GetACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, match, false)
require.NoError(t, err)
expect := newACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, match, ovnnb.ACLActionDrop, util.NetpolACLTier)
expect.UUID = ingressACL.UUID
require.Equal(t, expect, ingressACL)
require.Contains(t, pg.ACLs, ingressACL.UUID)

// egress acl
match = fmt.Sprintf("inport == @%s && ip", pgName)
egressACL, err := nbClient.GetACL(pgName, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, match, false)
require.NoError(t, err)
expect = newACL(pgName, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, match, ovnnb.ACLActionDrop, util.NetpolACLTier)
expect.UUID = egressACL.UUID
require.Equal(t, expect, egressACL)
require.Contains(t, pg.ACLs, egressACL.UUID)
pg, err := nbClient.GetPortGroup(pgName, false)
require.NoError(t, err)

// ingress acl
match := fmt.Sprintf("outport == @%s && ip", pgName)
ingressACL, err := nbClient.GetACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, match, false)
require.NoError(t, err)
expect := newACL(pgName, ovnnb.ACLDirectionToLport, util.SecurityGroupDropPriority, match, ovnnb.ACLActionDrop, util.NetpolACLTier)
expect.UUID = ingressACL.UUID
require.Equal(t, expect, ingressACL)
require.Contains(t, pg.ACLs, ingressACL.UUID)

// egress acl
match = fmt.Sprintf("inport == @%s && ip", pgName)
egressACL, err := nbClient.GetACL(pgName, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, match, false)
require.NoError(t, err)
expect = newACL(pgName, ovnnb.ACLDirectionFromLport, util.SecurityGroupDropPriority, match, ovnnb.ACLActionDrop, util.NetpolACLTier)
expect.UUID = egressACL.UUID
require.Equal(t, expect, egressACL)
require.Contains(t, pg.ACLs, egressACL.UUID)
})

t.Run("should print log err when sg name does not exist", func(t *testing.T) {
sgName := "test_nonexist_pg"
err := nbClient.CreateSgDenyAllACL(sgName)
require.Error(t, err)
})

t.Run("fail nb client should log err", func(t *testing.T) {
sgName := "test_failed_client"
err := failedNbClient.CreateSgDenyAllACL(sgName)
require.Error(t, err)
})
}

func (suite *OvnClientTestSuite) testCreateSgBaseACL() {
Expand Down Expand Up @@ -1340,6 +1356,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
t.Parallel()

nbClient := suite.ovnNBClient
failedNbClient := suite.faiedOvnNBClient
pgName := "test-del-acls-pg"
lsName := "test-del-acls-ls"
matchPrefix := "outport == @ovn.sg.test_del_acl_pg && ip"
Expand Down Expand Up @@ -1548,6 +1565,33 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
require.NoError(t, err)
require.Empty(t, ls.ACLs)
})

t.Run("should no err when acls does not exist", func(t *testing.T) {
err = nbClient.DeleteAcls("test-nonexist-ls", logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": "test-nonexist-ls"})
require.NoError(t, err)
})

t.Run("fail nb client should log err", func(t *testing.T) {
priority := "5805"
basePort := 5805
acls := make([]*ovnnb.ACL, 0, 5)

match := fmt.Sprintf("%s && udp.dst == %d", matchPrefix, basePort)
acl, err := failedNbClient.newACL(lsName, ovnnb.ACLDirectionToLport, priority, match, ovnnb.ACLActionAllowRelated, util.NetpolACLTier, func(acl *ovnnb.ACL) {
if acl.ExternalIDs == nil {
acl.ExternalIDs = make(map[string]string)
}
acl.ExternalIDs["subnet"] = lsName
})
require.NoError(t, err)
acls = append(acls, acl)

err = failedNbClient.CreateAcls(lsName, logicalSwitchKey, acls...)
require.Error(t, err)
// TODO:// should err but not for now
err = failedNbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName})
require.NoError(t, err)
})
}

func (suite *OvnClientTestSuite) testDeleteACL() {
Expand Down Expand Up @@ -2406,6 +2450,7 @@ func (suite *OvnClientTestSuite) testCreateBareACL() {
t.Parallel()

nbClient := suite.ovnNBClient
failedNbClient := suite.faiedOvnNBClient

t.Run("create bare ACL successfully", func(t *testing.T) {
err := nbClient.CreateBareACL("test-parent", "from-lport", "1000", "ip4.src == 10.0.0.1", "allow")
Expand All @@ -2417,6 +2462,11 @@ func (suite *OvnClientTestSuite) testCreateBareACL() {
require.Error(t, err)
require.Contains(t, err.Error(), "new acl direction from-lport priority 1000 match")
})

t.Run("fail nb client should log err", func(t *testing.T) {
err := failedNbClient.CreateBareACL("test-parent", "from-lport", "1000", "ip4.src == 10.0.0.1", "allow")
require.Error(t, err)
})
}

func (suite *OvnClientTestSuite) testUpdateAnpRuleACLOps() {
Expand Down Expand Up @@ -2479,10 +2529,51 @@ func (suite *OvnClientTestSuite) testUpdateACL() {
t := suite.T()

nbClient := suite.ovnNBClient
failedNbClient := suite.faiedOvnNBClient

// nbClient := suite.ovnNBClient
pgName := "test_update_acl_pg"
priority := "2000"
match := "ip4.dst == 100.64.0.0/16"

err := nbClient.CreatePortGroup(pgName, nil)
require.NoError(t, err)

acl, err := nbClient.newACL(pgName, ovnnb.ACLDirectionToLport, priority, match, ovnnb.ACLActionAllowRelated, util.NetpolACLTier)
require.NoError(t, err)

err = nbClient.CreateAcls(pgName, portGroupKey, acl)
require.NoError(t, err)

acl, err = nbClient.GetACL(pgName, ovnnb.ACLDirectionToLport, priority, match, false)
require.NoError(t, err)

t.Run("update ACL with nil input", func(t *testing.T) {
err := nbClient.UpdateACL(nil)
require.Error(t, err)
require.Contains(t, err.Error(), "address_set is nil")
})

t.Run("normal update ACL", func(t *testing.T) {
acl.Priority = 1005
err := nbClient.UpdateACL(acl)
require.NoError(t, err)

newACL, err := nbClient.GetACL(pgName, ovnnb.ACLDirectionToLport, "1005", match, false)
require.NoError(t, err)
fmt.Println(newACL.Priority)
})

t.Run("fail nb client should log err", func(t *testing.T) {
failACL, err := failedNbClient.newACL(pgName, ovnnb.ACLDirectionToLport, priority, match, ovnnb.ACLActionAllowRelated, util.NetpolACLTier)
require.NoError(t, err)

err = failedNbClient.CreateAcls(pgName, portGroupKey, failACL)
require.Error(t, err)

failACL.Priority = 1009
err = failedNbClient.UpdateACL(failACL)
// TODO:// should err but not for now
require.NoError(t, err)
})
}

0 comments on commit 45c54b6

Please sign in to comment.