Skip to content

Commit

Permalink
fix: [NPM] [Linux] improve iptables version detection and cleanup (#3090
Browse files Browse the repository at this point in the history
)

* fix: improve iptables version detection

Signed-off-by: Hunter Gregory <[email protected]>

* fix: redo everything and add tests

Signed-off-by: Hunter Gregory <[email protected]>

* fix: address comments

Signed-off-by: Hunter Gregory <[email protected]>

* fix: avoid segfault by only listing one chain

Signed-off-by: Hunter Gregory <[email protected]>

* style: log the kernel version

Signed-off-by: Hunter Gregory <[email protected]>

* style: fix lints

Signed-off-by: Hunter Gregory <[email protected]>

* fix: don't use stale chains. add comments. minor style change

Signed-off-by: Hunter Gregory <[email protected]>

* fix: listing kube chain. get stderr too. also add missing ut

Signed-off-by: Hunter Gregory <[email protected]>

* fix: log messages

Signed-off-by: Hunter Gregory <[email protected]>

* fix: stop checking kernel version. default nft, never crash

Signed-off-by: Hunter Gregory <[email protected]>

* style: fix lint

Signed-off-by: Hunter Gregory <[email protected]>

* style: try fixing gci/gofumpt lint

Signed-off-by: Hunter Gregory <[email protected]>

* test: fix unit tests referencing iptables legacy

Signed-off-by: Hunter Gregory <[email protected]>

* style: fix lint in iptm_test.go

Signed-off-by: Hunter Gregory <[email protected]>

---------

Signed-off-by: Hunter Gregory <[email protected]>
  • Loading branch information
huntergregory authored Nov 6, 2024
1 parent 7195484 commit 73a22fc
Show file tree
Hide file tree
Showing 12 changed files with 1,044 additions and 443 deletions.
348 changes: 178 additions & 170 deletions npm/iptm/iptm_test.go

Large diffs are not rendered by default.

3 changes: 0 additions & 3 deletions npm/pkg/dataplane/dataplane_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package dataplane

import (
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies"
"github.com/Azure/azure-container-networking/npm/util"
npmerrors "github.com/Azure/azure-container-networking/npm/util/errors"
)

Expand All @@ -21,8 +20,6 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error {
}

func (dp *DataPlane) bootupDataPlane() error {
util.DetectIptablesVersion(dp.ioShim)

// It is important to keep order to clean-up ACLs before ipsets. Otherwise we won't be able to delete ipsets referenced by ACLs
if err := dp.policyMgr.Bootup(nil); err != nil {
return npmerrors.ErrorWrapper(npmerrors.BootupDataplane, false, "failed to reset policy dataplane", err)
Expand Down
16 changes: 6 additions & 10 deletions npm/pkg/dataplane/dataplane_linux_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package dataplane

import (
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -74,9 +73,6 @@ func TestNetPolInBackgroundUpdatePolicy(t *testing.T) {
calls := append(getBootupTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...)
calls = append(calls, getRemovePolicyTestCallsForDP(&testPolicyobj)...)
calls = append(calls, getAddPolicyTestCallsForDP(&updatedTestPolicyobj)...)
for _, call := range calls {
fmt.Println(call)
}
ioshim := common.NewMockIOShim(calls)
defer ioshim.VerifyCalls(t, calls)
dp, err := NewDataPlane("testnode", ioshim, netpolInBackgroundCfg, nil)
Expand Down Expand Up @@ -133,31 +129,31 @@ func TestNetPolInBackgroundFailureToAddFirstTime(t *testing.T) {
},
// restore will try twice per pMgr.AddPolicies() call
testutils.TestCmd{
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
ExitCode: 1,
},
testutils.TestCmd{
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
ExitCode: 1,
},
// first policy succeeds
testutils.TestCmd{
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
ExitCode: 0,
},
// second policy succeeds
testutils.TestCmd{
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
ExitCode: 0,
},
// third policy fails
// restore will try twice per pMgr.AddPolicies() call
testutils.TestCmd{
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
ExitCode: 1,
},
testutils.TestCmd{
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
ExitCode: 1,
},
)
Expand Down
6 changes: 1 addition & 5 deletions npm/pkg/dataplane/dataplane_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package dataplane

import (
"fmt"
"testing"

"github.com/Azure/azure-container-networking/common"
Expand Down Expand Up @@ -262,9 +261,6 @@ func TestUpdatePolicy(t *testing.T) {
calls := append(getBootupTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...)
calls = append(calls, getRemovePolicyTestCallsForDP(&testPolicyobj)...)
calls = append(calls, getAddPolicyTestCallsForDP(&updatedTestPolicyobj)...)
for _, call := range calls {
fmt.Println(call)
}
ioshim := common.NewMockIOShim(calls)
defer ioshim.VerifyCalls(t, calls)
dp, err := NewDataPlane("testnode", ioshim, dpCfg, nil)
Expand Down Expand Up @@ -420,7 +416,7 @@ func TestUpdatePodCache(t *testing.T) {
}

func getBootupTestCalls() []testutils.TestCmd {
return append(policies.GetBootupTestCalls(true), ipsets.GetResetTestCalls()...)
return append(policies.GetBootupTestCalls(), ipsets.GetResetTestCalls()...)
}

func getAddPolicyTestCallsForDP(networkPolicy *policies.NPMNetworkPolicy) []testutils.TestCmd {
Expand Down
2 changes: 1 addition & 1 deletion npm/pkg/dataplane/parse/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestParseIptablesObjectFileV2(t *testing.T) {

func TestParseIptablesObject(t *testing.T) {
calls := []testutils.TestCmd{
{Cmd: []string{"iptables-save", "-t", "filter"}},
{Cmd: []string{"iptables-nft-save", "-t", "filter"}},
}

parser := IPTablesParser{
Expand Down
Loading

0 comments on commit 73a22fc

Please sign in to comment.