Skip to content

Commit

Permalink
fix: crash NPM if unable to locate kube chain
Browse files Browse the repository at this point in the history
Signed-off-by: Hunter Gregory <[email protected]>
  • Loading branch information
huntergregory committed Nov 7, 2024
1 parent f0a409b commit cd4cfcf
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 12 deletions.
15 changes: 9 additions & 6 deletions npm/pkg/dataplane/policies/chain-management_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ var (

listHintChainArgs = []string{"KUBE-IPTABLES-HINT", util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag}
listCanaryChainArgs = []string{"KUBE-KUBELET-CANARY", util.IptablesTableFlag, util.IptablesMangleTable, util.IptablesNumericFlag}

errDetectingIptablesVersion = errors.New("unable to locate which iptables version kube proxy is using")
)

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

// 0.1. Detect iptables version
pMgr.detectIptablesVersion()
if err := pMgr.detectIptablesVersion(); err != nil {
return npmerrors.SimpleErrorWrapper("failed to detect iptables version", err)
}

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

klog.Info("second attempt detecting iptables version. looking for hint/canary chain in iptables-legacy")
if pMgr.hintOrCanaryChainExist(util.IptablesLegacy) {
util.SetIptablesToLegacy()
return
return nil
}

// default to nft if nothing is found
util.SetIptablesToNft()
return errDetectingIptablesVersion
}

func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool {
Expand Down
18 changes: 12 additions & 6 deletions npm/pkg/dataplane/policies/chain-management_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,7 @@ func TestDetectIptablesVersion(t *testing.T) {
name string
calls []testutils.TestCmd
expectedIptablesVersion string
expectedErr bool
}

tests := []args{
Expand Down Expand Up @@ -942,7 +943,7 @@ func TestDetectIptablesVersion(t *testing.T) {
expectedIptablesVersion: util.IptablesLegacy,
},
{
name: "no kube chains: default nft",
name: "no kube chains: error",
calls: []testutils.TestCmd{
{
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
Expand All @@ -961,10 +962,10 @@ func TestDetectIptablesVersion(t *testing.T) {
ExitCode: 1,
},
},
expectedIptablesVersion: util.IptablesNft,
expectedErr: true,
},
{
name: "nft and legacy both fail: default nft",
name: "nft and legacy both fail: error",
calls: []testutils.TestCmd{
{
Cmd: []string{"iptables-nft", "-w", "60", "-L", "KUBE-IPTABLES-HINT", "-t", "mangle", "-n"},
Expand All @@ -983,7 +984,7 @@ func TestDetectIptablesVersion(t *testing.T) {
ExitCode: 2,
},
},
expectedIptablesVersion: util.IptablesNft,
expectedErr: true,
},
}

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

require.Equal(t, tt.expectedIptablesVersion, util.Iptables)
err := pMgr.detectIptablesVersion()
if tt.expectedErr {
require.Error(t, err)
} else {
require.NoError(t, err)
require.Equal(t, tt.expectedIptablesVersion, util.Iptables)
}
})
}
}
Expand Down

0 comments on commit cd4cfcf

Please sign in to comment.