Skip to content

Commit 5f3fa4b

Browse files
committed
fix: redo everything and add tests
Signed-off-by: Hunter Gregory <[email protected]>
1 parent 00b385d commit 5f3fa4b

14 files changed

+912
-288
lines changed

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ require (
130130
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/monitor/armmonitor v0.11.0
131131
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v5 v5.2.0
132132
github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0
133-
github.com/zcalusic/sysinfo v1.1.2
134133
golang.org/x/sync v0.8.0
135134
gotest.tools/v3 v3.5.1
136135
k8s.io/kubectl v0.28.5

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,6 @@ github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZla
294294
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
295295
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
296296
github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k=
297-
github.com/zcalusic/sysinfo v1.1.2 h1:38KUgZQmCxlN9vUTt4miis4rU5ISJXGXOJ2rY7bMC8g=
298-
github.com/zcalusic/sysinfo v1.1.2/go.mod h1:NX+qYnWGtJVPV0yWldff9uppNKU4h40hJIRPf/pGLv4=
299297
go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0=
300298
go.opencensus.io v0.24.0/go.mod h1:vNK8G9p7aAivkbmorf4v+7Hgx+Zs0yY+0fOtgBfjQKo=
301299
go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
Lines changed: 0 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,10 @@
11
package dataplane
22

33
import (
4-
"errors"
5-
"fmt"
6-
"strconv"
7-
"strings"
8-
9-
"github.com/Azure/azure-container-networking/common"
10-
"github.com/Azure/azure-container-networking/npm/metrics"
114
"github.com/Azure/azure-container-networking/npm/pkg/dataplane/policies"
12-
"github.com/Azure/azure-container-networking/npm/util"
135
npmerrors "github.com/Azure/azure-container-networking/npm/util/errors"
14-
15-
"github.com/zcalusic/sysinfo"
16-
"k8s.io/klog"
176
)
187

19-
const detectingErrMsg = "failed to detect iptables version. failed to find KUBE chains in iptables-legacy-save and iptables-nft-save and failed to get kernel version. NPM will crash to retry"
20-
21-
var errDetectingIptablesVersion = errors.New(detectingErrMsg)
22-
238
func (dp *DataPlane) getEndpointsToApplyPolicies(_ []*policies.NPMNetworkPolicy) (map[string]string, error) {
249
// NOOP in Linux
2510
return nil, nil
@@ -35,10 +20,6 @@ func (dp *DataPlane) updatePod(pod *updateNPMPod) error {
3520
}
3621

3722
func (dp *DataPlane) bootupDataPlane() error {
38-
if err := detectIptablesVersion(dp.ioShim); err != nil {
39-
return npmerrors.ErrorWrapper(npmerrors.BootupDataplane, false, "failed to detect iptables version", err)
40-
}
41-
4223
// It is important to keep order to clean-up ACLs before ipsets. Otherwise we won't be able to delete ipsets referenced by ACLs
4324
if err := dp.policyMgr.Bootup(nil); err != nil {
4425
return npmerrors.ErrorWrapper(npmerrors.BootupDataplane, false, "failed to reset policy dataplane", err)
@@ -53,81 +34,3 @@ func (dp *DataPlane) refreshPodEndpoints() error {
5334
// NOOP in Linux
5435
return nil
5536
}
56-
57-
// detectIptablesVersion sets the global iptables variable to nft if detected or legacy if detected.
58-
// NPM will crash if it fails to detect either.
59-
// This global variable is referenced in all iptables related functions.
60-
func detectIptablesVersion(ioShim *common.IOShim) error {
61-
klog.Info("first attempt detecting iptables version. running: iptables-nft-save -t mangle")
62-
cmd := ioShim.Exec.Command(util.IptablesSaveNft, "-t", "mangle")
63-
output, err := cmd.CombinedOutput()
64-
if err == nil && strings.Contains(string(output), "KUBE-IPTABLES-HINT") || strings.Contains(string(output), "KUBE-KUBELET-CANARY") {
65-
msg := "detected iptables version on first attempt. found KUBE chains in nft tables. NPM will use iptables-nft"
66-
klog.Info(msg)
67-
metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint)
68-
util.Iptables = util.IptablesNft
69-
util.IptablesSave = util.IptablesSaveNft
70-
util.IptablesRestore = util.IptablesRestoreNft
71-
return nil
72-
}
73-
74-
if err != nil {
75-
msg := fmt.Sprintf("failed to detect iptables version on first attempt. error running iptables-nft-save. will try detecting using iptables-legacy-save. err: %w", err)
76-
klog.Info(msg)
77-
metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg)
78-
}
79-
80-
klog.Info("second attempt detecting iptables version. running: iptables-legacy-save -t mangle")
81-
lCmd := ioShim.Exec.Command(util.IptablesSaveLegacy, "-t", "mangle")
82-
loutput, err := lCmd.CombinedOutput()
83-
if err == nil && strings.Contains(string(loutput), "KUBE-IPTABLES-HINT") || strings.Contains(string(loutput), "KUBE-KUBELET-CANARY") {
84-
msg := "detected iptables version on second attempt. found KUBE chains in legacy tables. NPM will use iptables-legacy"
85-
klog.Info(msg)
86-
metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint)
87-
util.Iptables = util.IptablesLegacy
88-
util.IptablesSave = util.IptablesSaveLegacy
89-
util.IptablesRestore = util.IptablesRestoreLegacy
90-
return nil
91-
}
92-
93-
if err != nil {
94-
msg := fmt.Sprintf("failed to detect iptables version on second attempt. error running iptables-legacy-save. will try detecting using kernel version. err: %w", err)
95-
klog.Info(msg)
96-
metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg)
97-
}
98-
99-
klog.Info("third attempt detecting iptables version. getting kernel version")
100-
var si sysinfo.SysInfo
101-
si.GetSysInfo()
102-
kernelVersion := strings.Split(si.Kernel.Release, ".")
103-
if kernelVersion[0] == "" {
104-
msg := fmt.Sprintf("failed to detect iptables version on third attempt. error getting kernel version. err: %w", err)
105-
klog.Info(msg)
106-
metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg)
107-
return errDetectingIptablesVersion
108-
}
109-
110-
majorVersion, err := strconv.Atoi(kernelVersion[0])
111-
if err != nil {
112-
msg := fmt.Sprintf("failed to detect iptables version on third attempt. error converting kernel version to int. err: %w", err)
113-
klog.Info(msg)
114-
metrics.SendErrorLogAndMetric(util.DaemonDataplaneID, msg)
115-
return errDetectingIptablesVersion
116-
}
117-
118-
if majorVersion >= 5 {
119-
msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft"
120-
klog.Info(msg)
121-
metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint)
122-
util.Iptables = util.IptablesNft
123-
util.IptablesSave = util.IptablesSaveNft
124-
util.IptablesRestore = util.IptablesRestoreNft
125-
return nil
126-
}
127-
128-
msg := "detected iptables version on third attempt. found kernel version < 5. NPM will use iptables-legacy"
129-
klog.Info(msg)
130-
metrics.SendLog(util.DaemonDataplaneID, msg, metrics.DonotPrint)
131-
132-
return nil
133-
}

npm/pkg/dataplane/dataplane_linux_test.go

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package dataplane
22

33
import (
4-
"fmt"
54
"testing"
65
"time"
76

@@ -74,9 +73,6 @@ func TestNetPolInBackgroundUpdatePolicy(t *testing.T) {
7473
calls := append(getBootupTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...)
7574
calls = append(calls, getRemovePolicyTestCallsForDP(&testPolicyobj)...)
7675
calls = append(calls, getAddPolicyTestCallsForDP(&updatedTestPolicyobj)...)
77-
for _, call := range calls {
78-
fmt.Println(call)
79-
}
8076
ioshim := common.NewMockIOShim(calls)
8177
defer ioshim.VerifyCalls(t, calls)
8278
dp, err := NewDataPlane("testnode", ioshim, netpolInBackgroundCfg, nil)
@@ -133,31 +129,31 @@ func TestNetPolInBackgroundFailureToAddFirstTime(t *testing.T) {
133129
},
134130
// restore will try twice per pMgr.AddPolicies() call
135131
testutils.TestCmd{
136-
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
132+
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
137133
ExitCode: 1,
138134
},
139135
testutils.TestCmd{
140-
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
136+
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
141137
ExitCode: 1,
142138
},
143139
// first policy succeeds
144140
testutils.TestCmd{
145-
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
141+
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
146142
ExitCode: 0,
147143
},
148144
// second policy succeeds
149145
testutils.TestCmd{
150-
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
146+
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
151147
ExitCode: 0,
152148
},
153149
// third policy fails
154150
// restore will try twice per pMgr.AddPolicies() call
155151
testutils.TestCmd{
156-
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
152+
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
157153
ExitCode: 1,
158154
},
159155
testutils.TestCmd{
160-
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
156+
Cmd: []string{"iptables-nft-restore", "-w", "60", "-T", "filter", "--noflush"},
161157
ExitCode: 1,
162158
},
163159
)

npm/pkg/dataplane/dataplane_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package dataplane
22

33
import (
4-
"fmt"
54
"testing"
65

76
"github.com/Azure/azure-container-networking/common"
@@ -262,9 +261,6 @@ func TestUpdatePolicy(t *testing.T) {
262261
calls := append(getBootupTestCalls(), getAddPolicyTestCallsForDP(&testPolicyobj)...)
263262
calls = append(calls, getRemovePolicyTestCallsForDP(&testPolicyobj)...)
264263
calls = append(calls, getAddPolicyTestCallsForDP(&updatedTestPolicyobj)...)
265-
for _, call := range calls {
266-
fmt.Println(call)
267-
}
268264
ioshim := common.NewMockIOShim(calls)
269265
defer ioshim.VerifyCalls(t, calls)
270266
dp, err := NewDataPlane("testnode", ioshim, dpCfg, nil)
@@ -420,7 +416,7 @@ func TestUpdatePodCache(t *testing.T) {
420416
}
421417

422418
func getBootupTestCalls() []testutils.TestCmd {
423-
return append(policies.GetBootupTestCalls(true), ipsets.GetResetTestCalls()...)
419+
return append(policies.GetBootupTestCalls(), ipsets.GetResetTestCalls()...)
424420
}
425421

426422
func getAddPolicyTestCallsForDP(networkPolicy *policies.NPMNetworkPolicy) []testutils.TestCmd {

0 commit comments

Comments
 (0)