From 27a81745fa9b9d02b6beaec6e53116054b0aa2f1 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Tue, 5 Nov 2024 17:01:39 -0800 Subject: [PATCH] style: fix lints Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../policies/chain-management_linux.go | 20 ++++++++++--------- .../policies/chain-management_linux_test.go | 4 +++- npm/pkg/dataplane/policies/policymanager.go | 6 +++--- npm/util/sysinfo_linux.go | 3 ++- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/npm/pkg/dataplane/policies/chain-management_linux.go b/npm/pkg/dataplane/policies/chain-management_linux.go index 7a00107c8ad..e3d36305e46 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux.go +++ b/npm/pkg/dataplane/policies/chain-management_linux.go @@ -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 ( @@ -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) @@ -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 } @@ -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 } @@ -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 @@ -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) } } } @@ -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) } } } @@ -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 diff --git a/npm/pkg/dataplane/policies/chain-management_linux_test.go b/npm/pkg/dataplane/policies/chain-management_linux_test.go index f772d2697b1..a18db146290 100644 --- a/npm/pkg/dataplane/policies/chain-management_linux_test.go +++ b/npm/pkg/dataplane/policies/chain-management_linux_test.go @@ -44,6 +44,8 @@ Chain AZURE-NPM-INGRESS (1 references) ` ) +var errKernelVersion = fmt.Errorf("kernel error") + func TestStaleChainsForceLock(t *testing.T) { testChains := []string{} for i := 0; i < 100000; i++ { @@ -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"}, diff --git a/npm/pkg/dataplane/policies/policymanager.go b/npm/pkg/dataplane/policies/policymanager.go index 1ed48a3ec2a..654b8c87f0e 100644 --- a/npm/pkg/dataplane/policies/policymanager.go +++ b/npm/pkg/dataplane/policies/policymanager.go @@ -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 diff --git a/npm/util/sysinfo_linux.go b/npm/util/sysinfo_linux.go index 53869cca307..d960d859285 100644 --- a/npm/util/sysinfo_linux.go +++ b/npm/util/sysinfo_linux.go @@ -3,6 +3,7 @@ package util // this file has bits copied from github.com/zcalusic/sysinfo (v1.1.2) import ( + "errors" "fmt" "os" "strconv" @@ -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)