Skip to content

Commit

Permalink
fix(nsc): remove error for lookupFWMarkByService
Browse files Browse the repository at this point in the history
lookupFWMarkByService() was previous returning an error when no fwmark
was found in the tracking map for a given service. However, this isn't
really an error condition and shouldn't be treated as such. When it was
treated as an error condition users got a lot of confusing errors in the
logs.
  • Loading branch information
aauren authored and mrueg committed Dec 3, 2021
1 parent bf325e1 commit 5101a4f
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 12 deletions.
4 changes: 1 addition & 3 deletions pkg/controllers/proxy/service_endpoints_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,8 @@ func (nsc *NetworkServicesController) setupExternalIPServices(serviceInfoMap ser
externalIPServiceID = generateIPPortID(externalIP, svc.protocol, strconv.Itoa(svc.port))

// ensure there is NO iptables mangle table rule to FW mark the packet
fwMark, err := nsc.lookupFWMarkByService(externalIP, svc.protocol, strconv.Itoa(svc.port))
fwMark := nsc.lookupFWMarkByService(externalIP, svc.protocol, strconv.Itoa(svc.port))
switch {
case err != nil:
klog.Errorf("failed to find FW mark for the service: %v", err)
case fwMark == 0:
klog.V(2).Infof("no FW mark found for service, nothing to cleanup")
case fwMark != 0:
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/proxy/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,14 @@ func (nsc *NetworkServicesController) generateUniqueFWMark(ip, protocol, port st

// lookupFWMarkByService finds the related FW mark from the internal fwMarkMap kept by the NetworkServiceController
// given the related ip, protocol, and port. If it isn't able to find a matching FW mark, then it returns an error.
func (nsc *NetworkServicesController) lookupFWMarkByService(ip, protocol, port string) (uint32, error) {
func (nsc *NetworkServicesController) lookupFWMarkByService(ip, protocol, port string) uint32 {
needle := fmt.Sprintf("%s-%s-%s", ip, protocol, port)
for fwMark, serviceKey := range nsc.fwMarkMap {
if needle == serviceKey {
return fwMark, nil
return fwMark
}
}
return 0, fmt.Errorf("no key matching %s:%s:%s was found in fwMarkMap", protocol, ip, port)
return 0
}

// lookupServiceByFWMark Lookup service ip, protocol, port by given FW Mark value (reverse of lookupFWMarkByService)
Expand Down
8 changes: 2 additions & 6 deletions pkg/controllers/proxy/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,17 @@ func TestNetworkServicesController_lookupFWMarkByService(t *testing.T) {
t.Run("ensure existing FW mark is found in lookup", func(t *testing.T) {
nsc := getMoqNSC()
fwMark1, err1 := nsc.generateUniqueFWMark("10.255.0.1", "TCP", "80")
fwMark2, err2 := nsc.lookupFWMarkByService("10.255.0.1", "TCP", "80")
fwMark2 := nsc.lookupFWMarkByService("10.255.0.1", "TCP", "80")

assert.NoError(t, err1, "there shouldn't have been an error calling generateUniqueFWMark")
assert.NoError(t, err2, "there shouldn't have been an error calling lookupFWMarkByService")

assert.Equal(t, fwMark1, fwMark2,
"given the same inputs, lookupFWMarkByService should be able to find the previously generated FW mark")
})
t.Run("ensure error is returned when a service doesn't exist in FW mark map", func(t *testing.T) {
nsc := getMoqNSC()
fwMark, err := nsc.lookupFWMarkByService("10.255.0.1", "TCP", "80")
fwMark := nsc.lookupFWMarkByService("10.255.0.1", "TCP", "80")

assert.EqualErrorf(t, err,
fmt.Sprintf("no key matching %s:%s:%s was found in fwMarkMap", "TCP", "10.255.0.1", "80"),
"expected to get an error when service had not yet been added to fwMarkMap")
assert.Equal(t, uint32(0), fwMark, "expected FW mark to be 0 on error condition")
})
}
Expand Down

0 comments on commit 5101a4f

Please sign in to comment.