Skip to content

Commit 7d38577

Browse files
rejain456huntergregory
authored andcommitted
[NPM] [Forward Port] fix: crash NPM if unable to locate kube chain (#3144)
fix: crash NPM if unable to locate kube chain Signed-off-by: Hunter Gregory <[email protected]> Co-authored-by: Hunter Gregory <[email protected]>
1 parent e872373 commit 7d38577

File tree

2 files changed

+21
-12
lines changed

2 files changed

+21
-12
lines changed

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ var (
8989

9090
listHintChainArgs = []string{"KUBE-IPTABLES-HINT", util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag}
9191
listCanaryChainArgs = []string{"KUBE-KUBELET-CANARY", util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag}
92+
93+
errDetectingIptablesVersion = errors.New("unable to locate which iptables version kube proxy is using")
9294
)
9395

9496
type exitErrorInfo struct {
@@ -187,7 +189,9 @@ func (pMgr *PolicyManager) bootup(_ []string) error {
187189
klog.Infof("booting up iptables Azure chains")
188190

189191
// 0.1. Detect iptables version
190-
pMgr.detectIptablesVersion()
192+
if err := pMgr.detectIptablesVersion(); err != nil {
193+
return npmerrors.SimpleErrorWrapper("failed to detect iptables version", err)
194+
}
191195

192196
// Stop reconciling so we don't contend for iptables, and so we don't update the staleChains at the same time as reconcile()
193197
// Reconciling would only be happening if this function were called to reset iptables well into the azure-npm pod lifecycle.
@@ -245,21 +249,20 @@ func (pMgr *PolicyManager) bootupAfterDetectAndCleanup() error {
245249
// NPM should use the same iptables version as kube-proxy.
246250
// kube-proxy creates an iptables chain as a hint for which version it uses.
247251
// For more details, see: https://kubernetes.io/blog/2022/09/07/iptables-chains-not-api/#use-case-iptables-mode
248-
func (pMgr *PolicyManager) detectIptablesVersion() {
252+
func (pMgr *PolicyManager) detectIptablesVersion() error {
249253
klog.Info("first attempt detecting iptables version. looking for hint/canary chain in iptables-nft")
250254
if pMgr.hintOrCanaryChainExist(util.IptablesNft) {
251255
util.SetIptablesToNft()
252-
return
256+
return nil
253257
}
254258

255259
klog.Info("second attempt detecting iptables version. looking for hint/canary chain in iptables-legacy")
256260
if pMgr.hintOrCanaryChainExist(util.IptablesLegacy) {
257261
util.SetIptablesToLegacy()
258-
return
262+
return nil
259263
}
260264

261-
// default to nft if nothing is found
262-
util.SetIptablesToNft()
265+
return errDetectingIptablesVersion
263266
}
264267

265268
func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool {

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,7 @@ func TestDetectIptablesVersion(t *testing.T) {
896896
name string
897897
calls []testutils.TestCmd
898898
expectedIptablesVersion string
899+
expectedErr bool
899900
}
900901

901902
tests := []args{
@@ -942,7 +943,7 @@ func TestDetectIptablesVersion(t *testing.T) {
942943
expectedIptablesVersion: util.IptablesLegacy,
943944
},
944945
{
945-
name: "no kube chains: default nft",
946+
name: "no kube chains: error",
946947
calls: []testutils.TestCmd{
947948
{
948949
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
@@ -961,10 +962,10 @@ func TestDetectIptablesVersion(t *testing.T) {
961962
ExitCode: 1,
962963
},
963964
},
964-
expectedIptablesVersion: util.IptablesNft,
965+
expectedErr: true,
965966
},
966967
{
967-
name: "nft and legacy both fail: default nft",
968+
name: "nft and legacy both fail: error",
968969
calls: []testutils.TestCmd{
969970
{
970971
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
@@ -983,7 +984,7 @@ func TestDetectIptablesVersion(t *testing.T) {
983984
ExitCode: 2,
984985
},
985986
},
986-
expectedIptablesVersion: util.IptablesNft,
987+
expectedErr: true,
987988
},
988989
}
989990

@@ -1001,9 +1002,14 @@ func TestDetectIptablesVersion(t *testing.T) {
10011002
PlaceAzureChainFirst: util.PlaceAzureChainFirst,
10021003
}
10031004
pMgr := NewPolicyManager(ioshim, cfg)
1004-
pMgr.detectIptablesVersion()
10051005

1006-
require.Equal(t, tt.expectedIptablesVersion, util.Iptables)
1006+
err := pMgr.detectIptablesVersion()
1007+
if tt.expectedErr {
1008+
require.Error(t, err)
1009+
} else {
1010+
require.NoError(t, err)
1011+
require.Equal(t, tt.expectedIptablesVersion, util.Iptables)
1012+
}
10071013
})
10081014
}
10091015
}

0 commit comments

Comments
 (0)