Skip to content

chore: appease the linter, pt 1 of ? #922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 8, 2021
Merged

chore: appease the linter, pt 1 of ? #922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions cni/network/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,9 +674,8 @@ func (plugin *netPlugin) Add(args *cniSkel.CmdArgs) error {
}

epPolicies := getPoliciesFromRuntimeCfg(nwCfg)
for _, epPolicy := range epPolicies {
epInfo.Policies = append(epInfo.Policies, epPolicy)
}

epInfo.Policies = append(epInfo.Policies, epPolicies...)

// Populate addresses.
for _, ipconfig := range result.IPs {
Expand Down
2 changes: 1 addition & 1 deletion cnm/plugin/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func main() {

// Relay these incoming signals to OS signal channel.
osSignalChannel := make(chan os.Signal, 1)
signal.Notify(osSignalChannel, os.Interrupt, os.Kill, syscall.SIGTERM)
signal.Notify(osSignalChannel, os.Interrupt, syscall.SIGTERM)
Copy link
Member

@tamilmani1989 tamilmani1989 Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did we test this change?

Copy link
Collaborator Author

@rbtr rbtr Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.Kill cannot be trapped, so including it in the Notify list is ineffective.


// Wait until receiving a signal.
select {
Expand Down
4 changes: 1 addition & 3 deletions cnms/service/networkmonitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func main() {
tb.ConnectToTelemetryService(telemetryNumRetries, telemetryWaitTimeInMilliseconds)
defer tb.Close()

for true {
for {
config.Store, err = store.NewJsonFileStore(platform.CNIRuntimePath + pluginName + ".json")
if err != nil {
fmt.Printf("[monitor] Failed to create store: %v\n", err)
Expand Down Expand Up @@ -178,6 +178,4 @@ func main() {
time.Sleep(time.Duration(timeout) * time.Second)
nm = nil
}

log.Close()
Copy link
Contributor

@vakalapa vakalapa Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont we have to defer log.close() this ?

Copy link
Collaborator Author

@rbtr rbtr Jul 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the above for loop has no break this is unreachable code and we don't currently ever call Close().
so i don't know if we need to, but if it hasn't been a problem i would guess no.

Copy link
Member

@tamilmani1989 tamilmani1989 Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its good practice to have defer log.Close()..if someone changes this behaviour tomorrow, then they may fail to add log.close()

Copy link
Collaborator Author

@rbtr rbtr Jul 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tamilmani1989 I read through the log package and I don't think it's safe to call log.Close() (even deferred) the way that it's written right now. We could accidentally close stdout or stderr and prevent the go runtime from writing the crash logs during a panic. That needs some work and is out of scope for this linting fix imo.

}
18 changes: 5 additions & 13 deletions cns/service/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ const (
name = "azure-cns"
pluginName = "azure-vnet"
defaultCNINetworkConfigFileName = "10-azure.conflist"
configFileName = "config.json"
dncApiVersion = "?api-version=2018-03-01"
poolIPAMRefreshRateInMilliseconds = 1000

Expand Down Expand Up @@ -321,19 +320,14 @@ func sendRegisterNodeRequest(
nodeRegisterRequest cns.NodeRegisterRequest,
registerURL string) (bool, error) {

var (
body bytes.Buffer
response *http.Response
err = fmt.Errorf("")
)

err = json.NewEncoder(&body).Encode(nodeRegisterRequest)
var body bytes.Buffer
err := json.NewEncoder(&body).Encode(nodeRegisterRequest)
if err != nil {
log.Errorf("[Azure CNS] Failed to register node while encoding json failed with non-retriable err %v", err)
return false, err
}

response, err = httpc.Post(registerURL, "application/json", &body)
response, err := httpc.Post(registerURL, "application/json", &body)
if err != nil {
logger.Errorf("[Azure CNS] Failed to register node with retriable err: %+v", err)
return false, nil
Expand Down Expand Up @@ -610,10 +604,8 @@ func main() {
// Periodically poll DNC for node updates
tickerChannel := time.Tick(time.Duration(cnsconfig.ManagedSettings.NodeSyncIntervalInSeconds) * time.Second)
for {
select {
case <-tickerChannel:
httpRestService.SyncNodeStatus(ep, vnet, node, json.RawMessage{})
}
<-tickerChannel
httpRestService.SyncNodeStatus(ep, vnet, node, json.RawMessage{})
}
}(privateEndpoint, infravnet, nodeID)
}
Expand Down
4 changes: 1 addition & 3 deletions network/bridge_endpointclient_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ func NewLinuxBridgeEndpointClient(
mode: mode,
}

for _, ipAddr := range extIf.IPAddresses {
client.hostIPAddresses = append(client.hostIPAddresses, ipAddr)
}
client.hostIPAddresses = append(client.hostIPAddresses, extIf.IPAddresses...)

return client
}
Expand Down
8 changes: 2 additions & 6 deletions network/endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,9 @@ func (ep *endpoint) getInfo() *EndpointInfo {
NetworkContainerID: ep.NetworkContainerID,
}

for _, route := range ep.Routes {
info.Routes = append(info.Routes, route)
}
info.Routes = append(info.Routes, ep.Routes...)

for _, gw := range ep.Gateways {
info.Gateways = append(info.Gateways, gw)
}
info.Gateways = append(info.Gateways, ep.Gateways...)

// Call the platform implementation.
ep.getInfoImpl(info)
Expand Down
9 changes: 2 additions & 7 deletions network/endpoint_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,7 @@ func (nw *network) newEndpointImpl(epInfo *EndpointInfo) (*endpoint, error) {
PODNameSpace: epInfo.PODNameSpace,
}

for _, route := range epInfo.Routes {
ep.Routes = append(ep.Routes, route)
}

ep.Routes = append(ep.Routes, epInfo.Routes...)
return ep, nil
}

Expand Down Expand Up @@ -381,9 +378,7 @@ func (nw *network) updateEndpointImpl(existingEpInfo *EndpointInfo, targetEpInfo
}

// Update existing endpoint state with the new routes to persist
for _, route := range targetEpInfo.Routes {
ep.Routes = append(ep.Routes, route)
}
ep.Routes = append(ep.Routes, targetEpInfo.Routes...)

return ep, nil
}
Expand Down
4 changes: 2 additions & 2 deletions network/epcommon/endpoint_common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func BlockIPAddresses(bridgeName string, action string) error {
func EnableIPForwarding(ifName string) error {
// Enable ip forwading on linux vm.
// sysctl -w net.ipv4.ip_forward=1
cmd := fmt.Sprintf(enableIPForwardCmd)
cmd := fmt.Sprint(enableIPForwardCmd)
Copy link
Member

@tamilmani1989 tamilmani1989 Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we change this? what's the use?

Copy link
Collaborator Author

@rbtr rbtr Jul 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are not doing a string interpolation so it's unnecessary to call Sprintf, and using the minimal Sprint is more correct

_, err := platform.ExecuteCommand(cmd)
if err != nil {
log.Printf("[net] Enable ipforwarding failed with: %v", err)
Expand All @@ -206,7 +206,7 @@ func EnableIPForwarding(ifName string) error {
}

func EnableIPV6Forwarding() error {
cmd := fmt.Sprintf(enableIPV6ForwardCmd)
cmd := fmt.Sprint(enableIPV6ForwardCmd)
_, err := platform.ExecuteCommand(cmd)
if err != nil {
log.Printf("[net] Enable ipv6 forwarding failed with: %v", err)
Expand Down
5 changes: 4 additions & 1 deletion network/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ const (
)

var (
Ipv4DefaultRouteDstPrefix = net.IPNet{net.IPv4zero, net.IPv4Mask(0, 0, 0, 0)}
Ipv4DefaultRouteDstPrefix = net.IPNet{
IP: net.IPv4zero,
Mask: net.IPv4Mask(0, 0, 0, 0),
}
)

type NetworkClient interface {
Expand Down
1 change: 0 additions & 1 deletion network/network_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ func (nm *networkManager) newNetworkImpl(nwInfo *NetworkInfo, extIf *externalInt
handleCommonOptions(extIf.BridgeName, nwInfo)
case opModeTransparent:
handleCommonOptions(extIf.Name, nwInfo)
break
default:
return nil, errNetworkModeInvalid
}
Expand Down
8 changes: 4 additions & 4 deletions npm/ipsm/ipsm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,17 +373,17 @@ func (ipsMgr *IpsetManager) AddToSet(setName, ip, spec, podKey string) error {
exists, _ := ipsMgr.SetExists(setName)

if !exists {
if err := ipsMgr.CreateSet(setName, append([]string{spec})); err != nil {
if err := ipsMgr.CreateSet(setName, []string{spec}); err != nil {
return err
}
}

var resultSpec []string
if strings.Contains(ip, util.IpsetNomatch) {
ip = strings.Trim(ip, util.IpsetNomatch)
resultSpec = append([]string{ip, util.IpsetNomatch})
resultSpec = []string{ip, util.IpsetNomatch}
} else {
resultSpec = append([]string{ip})
resultSpec = []string{ip}
}

entry := &ipsEntry{
Expand Down Expand Up @@ -440,7 +440,7 @@ func (ipsMgr *IpsetManager) DeleteFromSet(setName, ip, podKey string) error {
entry := &ipsEntry{
operationFlag: util.IpsetDeletionFlag,
set: util.GetHashedName(setName),
spec: append([]string{ip}),
spec: []string{ip},
}

if errCode, err := ipsMgr.Run(entry); err != nil {
Expand Down
22 changes: 11 additions & 11 deletions npm/ipsm/ipsm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestAddToList(t *testing.T) {
}
}()

if err := ipsMgr.CreateSet("test-set", append([]string{util.IpsetNetHashFlag})); err != nil {
if err := ipsMgr.CreateSet("test-set", []string{util.IpsetNetHashFlag}); err != nil {
t.Errorf("TestAddToList failed @ ipsMgr.CreateSet")
}

Expand Down Expand Up @@ -116,7 +116,7 @@ func TestDeleteFromList(t *testing.T) {

// Create set and validate set is created.
setName := "test-set"
if err := ipsMgr.CreateSet(setName, append([]string{util.IpsetNetHashFlag})); err != nil {
if err := ipsMgr.CreateSet(setName, []string{util.IpsetNetHashFlag}); err != nil {
t.Errorf("TestDeleteFromList failed @ ipsMgr.CreateSet")
}

Expand All @@ -138,7 +138,7 @@ func TestDeleteFromList(t *testing.T) {
entry = &ipsEntry{
operationFlag: util.IpsetTestFlag,
set: util.GetHashedName(listName),
spec: append([]string{util.GetHashedName(setName)}),
spec: []string{util.GetHashedName(setName)},
}

if _, err := ipsMgr.Run(entry); err != nil {
Expand All @@ -163,7 +163,7 @@ func TestDeleteFromList(t *testing.T) {
entry = &ipsEntry{
operationFlag: util.IpsetTestFlag,
set: util.GetHashedName(listName),
spec: append([]string{util.GetHashedName(setName)}),
spec: []string{util.GetHashedName(setName)},
}

if _, err := ipsMgr.Run(entry); err == nil {
Expand Down Expand Up @@ -224,13 +224,13 @@ func TestCreateSet(t *testing.T) {
}

testSet2Name := "test-set-with-maxelem"
spec := append([]string{util.IpsetNetHashFlag, util.IpsetMaxelemName, util.IpsetMaxelemNum})
spec := []string{util.IpsetNetHashFlag, util.IpsetMaxelemName, util.IpsetMaxelemNum}
if err := ipsMgr.CreateSet(testSet2Name, spec); err != nil {
t.Errorf("TestCreateSet failed @ ipsMgr.CreateSet when set maxelem")
}

testSet3Name := "test-set-with-port"
spec = append([]string{util.IpsetIPPortHashFlag})
spec = []string{util.IpsetIPPortHashFlag}
if err := ipsMgr.CreateSet(testSet3Name, spec); err != nil {
t.Errorf("TestCreateSet failed @ ipsMgr.CreateSet when creating port set")
}
Expand Down Expand Up @@ -270,7 +270,7 @@ func TestDeleteSet(t *testing.T) {
}()

testSetName := "test-delete-set"
if err := ipsMgr.CreateSet(testSetName, append([]string{util.IpsetNetHashFlag})); err != nil {
if err := ipsMgr.CreateSet(testSetName, []string{util.IpsetNetHashFlag}); err != nil {
t.Errorf("TestDeleteSet failed @ ipsMgr.CreateSet")
}

Expand Down Expand Up @@ -492,7 +492,7 @@ func TestClean(t *testing.T) {
}
}()

if err := ipsMgr.CreateSet("test-set", append([]string{util.IpsetNetHashFlag})); err != nil {
if err := ipsMgr.CreateSet("test-set", []string{util.IpsetNetHashFlag}); err != nil {
t.Errorf("TestClean failed @ ipsMgr.CreateSet with err %+v", err)
}

Expand Down Expand Up @@ -535,7 +535,7 @@ func TestDestroy(t *testing.T) {
entry := &ipsEntry{
operationFlag: util.IpsetTestFlag,
set: util.GetHashedName(setName),
spec: append([]string{testIP}),
spec: []string{testIP},
}

if _, err := ipsMgr.Run(entry); err == nil {
Expand All @@ -559,7 +559,7 @@ func TestRun(t *testing.T) {
entry := &ipsEntry{
operationFlag: util.IpsetCreationFlag,
set: "test-set",
spec: append([]string{util.IpsetNetHashFlag}),
spec: []string{util.IpsetNetHashFlag},
}
if _, err := ipsMgr.Run(entry); err != nil {
t.Errorf("TestRun failed @ ipsMgr.Run with err %+v", err)
Expand All @@ -579,7 +579,7 @@ func TestRunError(t *testing.T) {
entry := &ipsEntry{
operationFlag: util.IpsetCreationFlag,
set: util.GetHashedName(setname),
spec: append([]string{util.IpsetNetHashFlag}),
spec: []string{util.IpsetNetHashFlag},
}
if _, err := ipsMgr.Run(entry); err != nil {
require.Error(t, err)
Expand Down
12 changes: 6 additions & 6 deletions npm/networkPolicyController.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ func (c *networkPolicyController) initializeDefaultAzureNpmChain() error {

ipsMgr := c.npMgr.NsMap[util.KubeAllNamespacesFlag].IpsMgr
iptMgr := c.npMgr.NsMap[util.KubeAllNamespacesFlag].iptMgr
if err := ipsMgr.CreateSet(util.KubeSystemFlag, append([]string{util.IpsetNetHashFlag})); err != nil {
if err := ipsMgr.CreateSet(util.KubeSystemFlag, []string{util.IpsetNetHashFlag}); err != nil {
return fmt.Errorf("[initializeDefaultAzureNpmChain] Error: failed to initialize kube-system ipset with err %s", err)
}
if err := iptMgr.InitNpmChains(); err != nil {
Expand Down Expand Up @@ -341,13 +341,13 @@ func (c *networkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1
sets, namedPorts, lists, ingressIPCidrs, egressIPCidrs, iptEntries := translatePolicy(netPolObj)
for _, set := range sets {
klog.Infof("Creating set: %v, hashedSet: %v", set, util.GetHashedName(set))
if err = ipsMgr.CreateSet(set, append([]string{util.IpsetNetHashFlag})); err != nil {
if err = ipsMgr.CreateSet(set, []string{util.IpsetNetHashFlag}); err != nil {
return fmt.Errorf("[syncAddAndUpdateNetPol] Error: creating ipset %s with err: %v", set, err)
}
}
for _, set := range namedPorts {
klog.Infof("Creating set: %v, hashedSet: %v", set, util.GetHashedName(set))
if err = ipsMgr.CreateSet(set, append([]string{util.IpsetIPPortHashFlag})); err != nil {
if err = ipsMgr.CreateSet(set, []string{util.IpsetIPPortHashFlag}); err != nil {
return fmt.Errorf("[syncAddAndUpdateNetPol] Error: creating ipset named port %s with err: %v", set, err)
}
}
Expand Down Expand Up @@ -451,10 +451,10 @@ func (c *networkPolicyController) cleanUpNetworkPolicy(netPolKey string, isSafeC
}

func (c *networkPolicyController) createCidrsRule(ingressOrEgress, policyName, ns string, ipsetEntries [][]string, ipsMgr *ipsm.IpsetManager) error {
spec := append([]string{util.IpsetNetHashFlag, util.IpsetMaxelemName, util.IpsetMaxelemNum})
spec := []string{util.IpsetNetHashFlag, util.IpsetMaxelemName, util.IpsetMaxelemNum}

for i, ipCidrSet := range ipsetEntries {
if ipCidrSet == nil || len(ipCidrSet) == 0 {
if len(ipCidrSet) == 0 {
continue
}
setName := policyName + "-in-ns-" + ns + "-" + strconv.Itoa(i) + ingressOrEgress
Expand Down Expand Up @@ -485,7 +485,7 @@ func (c *networkPolicyController) createCidrsRule(ingressOrEgress, policyName, n

func (c *networkPolicyController) removeCidrsRule(ingressOrEgress, policyName, ns string, ipsetEntries [][]string, ipsMgr *ipsm.IpsetManager) error {
for i, ipCidrSet := range ipsetEntries {
if ipCidrSet == nil || len(ipCidrSet) == 0 {
if len(ipCidrSet) == 0 {
continue
}
setName := policyName + "-in-ns-" + ns + "-" + strconv.Itoa(i) + ingressOrEgress
Expand Down
10 changes: 5 additions & 5 deletions npm/networkPolicyController_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,14 @@ func createNetPol() *networkingv1.NetworkPolicy {
},
Spec: networkingv1.NetworkPolicySpec{
Ingress: []networkingv1.NetworkPolicyIngressRule{
networkingv1.NetworkPolicyIngressRule{
{
From: []networkingv1.NetworkPolicyPeer{
networkingv1.NetworkPolicyPeer{
{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"app": "test"},
},
},
networkingv1.NetworkPolicyPeer{
{
IPBlock: &networkingv1.IPBlock{
CIDR: "0.0.0.0/0",
},
Expand All @@ -141,7 +141,7 @@ func createNetPol() *networkingv1.NetworkPolicy {
},
},
Egress: []networkingv1.NetworkPolicyEgressRule{
networkingv1.NetworkPolicyEgressRule{
{
To: []networkingv1.NetworkPolicyPeer{{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"app": "test"},
Expand Down Expand Up @@ -272,7 +272,7 @@ func TestAddMultipleNetworkPolicies(t *testing.T) {
netPolObj2.Namespace = fmt.Sprintf("%s-new", netPolObj1.Namespace)
netPolObj2.Name = fmt.Sprintf("%s-new", netPolObj1.Name)
// namedPort
netPolObj2.Spec.Ingress[0].Ports[0].Port = &intstr.IntOrString{StrVal: fmt.Sprintf("%s", netPolObj2.Name)}
netPolObj2.Spec.Ingress[0].Ports[0].Port = &intstr.IntOrString{StrVal: netPolObj2.Name}

fexec := exec.New()
f := newNetPolFixture(t, fexec)
Expand Down
2 changes: 1 addition & 1 deletion npm/npm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func NewNetworkPolicyManager(clientset *kubernetes.Clientset, informerFactory in

// Create ipset for the namespace.
kubeSystemNs := util.GetNSNameWithPrefix(util.KubeSystemFlag)
if err := allNs.IpsMgr.CreateSet(kubeSystemNs, append([]string{util.IpsetNetHashFlag})); err != nil {
if err := allNs.IpsMgr.CreateSet(kubeSystemNs, []string{util.IpsetNetHashFlag}); err != nil {
metrics.SendErrorLogAndMetric(util.NpmID, "Error: failed to create ipset for namespace %s.", kubeSystemNs)
}

Expand Down
Loading