Skip to content

Commit 5acf87e

Browse files
huntergregoryhuntergregory
committed
fix: listing kube chain. get stderr too. also add missing ut
Signed-off-by: Hunter Gregory <[email protected]>
1 parent 36b3356 commit 5acf87e

File tree

3 files changed

+60
-32
lines changed

3 files changed

+60
-32
lines changed

‎npm/pkg/dataplane/policies/chain-management_linux.go‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,8 @@ var (
9494
util.IptablesAzureChain,
9595
}
9696

97-
listChainArgs = []string{util.IptablesWaitFlag, util.IptablesDefaultWaitTime, util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag, util.IptablesListFlag}
98-
listHintChainArgs = append(listChainArgs, "KUBE-IPTABLES-HINT")
99-
listCanaryChainArgs = append(listChainArgs, "KUBE-KUBELET-CANARY")
97+
listHintChainArgs = []string{"KUBE-IPTABLES-HINT", util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag}
98+
listCanaryChainArgs = []string{"KUBE-KUBELET-CANARY", util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag}
10099
)
101100

102101
type exitErrorInfo struct {
@@ -300,22 +299,21 @@ func (pMgr *PolicyManager) detectIptablesVersion() error {
300299

301300
func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool {
302301
// hint chain should exist since k8s 1.24 (see https://kubernetes.io/blog/2022/09/07/iptables-chains-not-api/#use-case-iptables-mode)
303-
hintCmd := pMgr.ioShim.Exec.Command(iptablesCmd, listHintChainArgs...)
304-
_, hintErr := hintCmd.CombinedOutput()
305-
if hintErr != nil {
306-
klog.Infof("failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error())
307-
metrics.SendErrorLogAndMetric(util.IptmID, "failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error())
308-
} else {
302+
prevIptables := util.Iptables
303+
util.Iptables = iptablesCmd
304+
defer func() {
305+
util.Iptables = prevIptables
306+
}()
307+
308+
_, hintErr := pMgr.runIPTablesCommand(util.IptablesListFlag, listHintChainArgs...)
309+
if hintErr == nil {
309310
metrics.SendLog(util.IptmID, "found hint chain. will use iptables version: %s"+iptablesCmd, metrics.DonotPrint)
310311
return true
311312
}
312313

313314
// check for canary chain
314-
canaryCmd := pMgr.ioShim.Exec.Command(iptablesCmd, listCanaryChainArgs...)
315-
_, canaryErr := canaryCmd.CombinedOutput()
315+
_, canaryErr := pMgr.runIPTablesCommand(util.IptablesListFlag, listCanaryChainArgs...)
316316
if canaryErr != nil {
317-
klog.Infof("failed to list canary chain. cmd: %s. error: %s", iptablesCmd, canaryErr.Error())
318-
metrics.SendErrorLogAndMetric(util.IptmID, "failed to list canary chain. cmd: %s. error: %s", iptablesCmd, canaryErr.Error())
319317
return false
320318
}
321319

‎npm/pkg/dataplane/policies/chain-management_linux_test.go‎

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ func TestDetectIptablesVersion(t *testing.T) {
915915
name: "nft has hint chain",
916916
calls: []testutils.TestCmd{
917917
{
918-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
918+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
919919
ExitCode: 0,
920920
},
921921
},
@@ -926,11 +926,11 @@ func TestDetectIptablesVersion(t *testing.T) {
926926
name: "nft has only canary chain",
927927
calls: []testutils.TestCmd{
928928
{
929-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
929+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
930930
ExitCode: 1,
931931
},
932932
{
933-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
933+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"},
934934
ExitCode: 0,
935935
},
936936
},
@@ -941,15 +941,15 @@ func TestDetectIptablesVersion(t *testing.T) {
941941
name: "legacy has hint chain",
942942
calls: []testutils.TestCmd{
943943
{
944-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
944+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
945945
ExitCode: 1,
946946
},
947947
{
948-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
948+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"},
949949
ExitCode: 1,
950950
},
951951
{
952-
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
952+
Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
953953
ExitCode: 0,
954954
},
955955
},
@@ -961,19 +961,19 @@ func TestDetectIptablesVersion(t *testing.T) {
961961
kernelVersion: 5,
962962
calls: []testutils.TestCmd{
963963
{
964-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
964+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
965965
ExitCode: 2,
966966
},
967967
{
968-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
968+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"},
969969
ExitCode: 2,
970970
},
971971
{
972-
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
972+
Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
973973
ExitCode: 2,
974974
},
975975
{
976-
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
976+
Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"},
977977
ExitCode: 2,
978978
},
979979
},
@@ -985,19 +985,19 @@ func TestDetectIptablesVersion(t *testing.T) {
985985
kernelVersion: 4,
986986
calls: []testutils.TestCmd{
987987
{
988-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
988+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
989989
ExitCode: 1,
990990
},
991991
{
992-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
992+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"},
993993
ExitCode: 1,
994994
},
995995
{
996-
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
996+
Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
997997
ExitCode: 1,
998998
},
999999
{
1000-
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
1000+
Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"},
10011001
ExitCode: 1,
10021002
},
10031003
},
@@ -1009,19 +1009,19 @@ func TestDetectIptablesVersion(t *testing.T) {
10091009
kernelVersionErr: errKernelVersion,
10101010
calls: []testutils.TestCmd{
10111011
{
1012-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
1012+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
10131013
ExitCode: 1,
10141014
},
10151015
{
1016-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
1016+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"},
10171017
ExitCode: 1,
10181018
},
10191019
{
1020-
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
1020+
Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
10211021
ExitCode: 1,
10221022
},
10231023
{
1024-
Cmd: []string{"iptables", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-KUBELET-CANARY"},
1024+
Cmd: []string{"iptables", "-w", "60", "-L", "KUBE-KUBELET-CANARY", "-t", "mangle", "-n"},
10251025
ExitCode: 1,
10261026
},
10271027
},
@@ -1173,6 +1173,36 @@ func TestCleanupOtherChains(t *testing.T) {
11731173
},
11741174
expectedErr: true,
11751175
},
1176+
{
1177+
name: "don't flush azure chain if it isn't there",
1178+
startWithNft: true,
1179+
calls: []testutils.TestCmd{
1180+
{
1181+
Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM"},
1182+
ExitCode: 1,
1183+
},
1184+
{
1185+
Cmd: []string{"iptables", "-w", "60", "-D", "FORWARD", "-j", "AZURE-NPM", "-m", "conntrack", "--ctstate", "NEW"},
1186+
ExitCode: 1,
1187+
},
1188+
{Cmd: []string{"iptables", "-w", "60", "-t", "filter", "-n", "-L"}, PipedToCommand: true},
1189+
{
1190+
Cmd: []string{"grep", "Chain AZURE-NPM"},
1191+
Stdout: "Chain AZURE-NPM-INGRESS (1 references)\n",
1192+
},
1193+
{
1194+
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
1195+
ExitCode: 1,
1196+
},
1197+
{
1198+
Cmd: []string{"iptables-restore", "-w", "60", "-T", "filter", "--noflush"},
1199+
ExitCode: 1,
1200+
},
1201+
{Cmd: []string{"iptables", "-w", "60", "-F", "AZURE-NPM-INGRESS"}},
1202+
{Cmd: []string{"iptables", "-w", "60", "-X", "AZURE-NPM-INGRESS"}},
1203+
},
1204+
expectedErr: false,
1205+
},
11761206
{
11771207
name: "cleanup legacy errors ok if deleted jump (non-deprecated)",
11781208
startWithNft: true,

‎npm/pkg/dataplane/policies/testutils_linux.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func GetBootupTestCalls() []testutils.TestCmd {
5353
bootUp := []testutils.TestCmd{
5454
// detect iptables version to be nft
5555
{
56-
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
56+
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
5757
ExitCode: 0,
5858
},
5959
// legacy clean up

0 commit comments

Comments
 (0)