Skip to content

Commit

Permalink
style: fix lints
Browse files Browse the repository at this point in the history
Signed-off-by: Hunter Gregory <[email protected]>
  • Loading branch information
huntergregory committed Nov 6, 2024
1 parent b399ec0 commit 27a8174
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 14 deletions.
20 changes: 11 additions & 9 deletions npm/pkg/dataplane/policies/chain-management_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const (
minLineNumberStringLength int = 3

detectingErrMsg = "failed to detect iptables version. failed to run iptables-legacy-save, run iptables-nft-save, and get kernel version. NPM will crash to retry"

minNftKernelVersion = 5
)

var (
Expand Down Expand Up @@ -281,7 +283,7 @@ func (pMgr *PolicyManager) detectIptablesVersion() error {
return errDetectingIptablesVersion
}

if majorVersion >= 5 {
if majorVersion >= minNftKernelVersion {
msg := "detected iptables version on third attempt. found kernel version >= 5. NPM will use iptables-nft. kernel version: %d"
klog.Infof(msg, majorVersion)
metrics.SendLog(util.IptmID, fmt.Sprintf(msg, majorVersion), metrics.DonotPrint)
Expand All @@ -304,7 +306,7 @@ func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool {
klog.Infof("failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error())
metrics.SendErrorLogAndMetric(util.IptmID, "failed to list hint chain. cmd: %s. error: %s", iptablesCmd, hintErr.Error())
} else {
metrics.SendLog(util.IptmID, fmt.Sprintf("found hint chain. will use iptables version: %s", iptablesCmd), metrics.DonotPrint)
metrics.SendLog(util.IptmID, "found hint chain. will use iptables version: %s"+iptablesCmd, metrics.DonotPrint)
return true
}

Expand All @@ -317,7 +319,7 @@ func (pMgr *PolicyManager) hintOrCanaryChainExist(iptablesCmd string) bool {
return false
}

metrics.SendLog(util.IptmID, fmt.Sprintf("found canary chain. will use iptables version: %s", iptablesCmd), metrics.DonotPrint)
metrics.SendLog(util.IptmID, "found canary chain. will use iptables version: "+iptablesCmd, metrics.DonotPrint)
return true
}

Expand Down Expand Up @@ -401,9 +403,9 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error {

creator := pMgr.creatorForCleanup(chains)
if err := restore(creator); err != nil {
msg := fmt.Sprintf("[cleanup] failed to flush all chains with error: %s", err.Error())
klog.Info(msg)
metrics.SendErrorLogAndMetric(util.IptmID, msg)
msg := "[cleanup] failed to flush all chains with error: %s"
klog.Infof(msg, err.Error())
metrics.SendErrorLogAndMetric(util.IptmID, msg, err.Error())

// 3.2. if we failed to flush all chains, then try to flush and delete them one by one
var aggregateError error
Expand Down Expand Up @@ -432,7 +434,7 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error {
if aggregateError == nil {
aggregateError = npmerrors.SimpleError(currentErrString)
} else {
aggregateError = npmerrors.SimpleErrorWrapper(fmt.Sprintf("%s and had previous error", currentErrString), aggregateError)
aggregateError = npmerrors.SimpleErrorWrapper(currentErrString+" and had previous error", aggregateError)
}
}
}
Expand All @@ -455,7 +457,7 @@ func (pMgr *PolicyManager) cleanupOtherIptables() error {
if aggregateError == nil {
aggregateError = npmerrors.SimpleError(currentErrString)
} else {
aggregateError = npmerrors.SimpleErrorWrapper(fmt.Sprintf("%s and had previous error", currentErrString), aggregateError)
aggregateError = npmerrors.SimpleErrorWrapper(currentErrString+" and had previous error", aggregateError)
}
}
}
Expand All @@ -473,7 +475,7 @@ func (pMgr *PolicyManager) creatorForCleanup(chains []string) *ioutil.FileCreato
// pass nil because we don't need to add any lines like ":CHAIN-NAME - -" because that is for creating chains
creator := pMgr.newCreatorWithChains(nil)
for _, chain := range chains {
creator.AddLine("", nil, fmt.Sprintf("-F %s", chain))
creator.AddLine("", nil, "-F "+chain)
}
creator.AddLine("", nil, util.IptablesRestoreCommit)
return creator
Expand Down
4 changes: 3 additions & 1 deletion npm/pkg/dataplane/policies/chain-management_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ Chain AZURE-NPM-INGRESS (1 references)
`
)

var errKernelVersion = fmt.Errorf("kernel error")

Check failure on line 47 in npm/pkg/dataplane/policies/chain-management_linux_test.go

View workflow job for this annotation

GitHub Actions / Lint (1.22.x, ubuntu-latest)

fmt.Errorf can be replaced with errors.New (perfsprint)

Check failure on line 47 in npm/pkg/dataplane/policies/chain-management_linux_test.go

View workflow job for this annotation

GitHub Actions / Lint (1.23.x, ubuntu-latest)

fmt.Errorf can be replaced with errors.New (perfsprint)

func TestStaleChainsForceLock(t *testing.T) {
testChains := []string{}
for i := 0; i < 100000; i++ {
Expand Down Expand Up @@ -1003,7 +1005,7 @@ func TestDetectIptablesVersion(t *testing.T) {
},
{
name: "no kube chains: kernel version error",
kernelVersionErr: fmt.Errorf("kernel version error"),
kernelVersionErr: errKernelVersion,
calls: []testutils.TestCmd{
{
Cmd: []string{"iptables-nft", "-w", "60", "-t", "mangle", "-n", "-L", "KUBE-IPTABLES-HINT"},
Expand Down
6 changes: 3 additions & 3 deletions npm/pkg/dataplane/policies/policymanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ const (

type PolicyManagerCfg struct {
// debug is only used for testing. Currently just indicating whether to use debug kernel version
debug bool
debug bool //nolint:unused // only used in linux
// debugKernelVersion is only used for testing
debugKernelVersion int
debugKernelVersion int //nolint:unused // only used in linux
// debugKernelVersionErr is only used for testing
debugKernelVersionErr error
debugKernelVersionErr error //nolint:unused // only used in linux

// NodeIP is only used in Windows
NodeIP string
Expand Down
3 changes: 2 additions & 1 deletion npm/util/sysinfo_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package util
// this file has bits copied from github.com/zcalusic/sysinfo (v1.1.2)

import (
"errors"
"fmt"
"os"
"strconv"
Expand All @@ -11,7 +12,7 @@ import (

const kernelReleaseFilepath = "/proc/sys/kernel/osrelease"

var errNoKernelRelease = fmt.Errorf("error finding kernel release")
var errNoKernelRelease = errors.New("error finding kernel release")

func KernelReleaseMajorVersion() (int, error) {
rel, err := slurpFile(kernelReleaseFilepath)
Expand Down

0 comments on commit 27a8174

Please sign in to comment.