pkg/proxy/userspace/roundrobin: Make lb.services nil check standardized

[carlosdmz]

As suggested in here, some portions of the code contains the map checking for nil values in lb.services map, and others don't have. Even though we're never assigning nil values to this map, we should standardize the portions that does not have this check in roundrobin module.

Example:

state, exists := lb.services[svcPort]
return exists && state != nil && len(state.endpoints) > 0

Should be like this:

state, exists := lb.services[svcPort]
if !exists || state == nil {
      return false
}
return len(state.endpoints) > 0

I can take care of this issue.

[carlosdmz]

/sig network

[uablrek]

There have been discussions of deprecating the user-space proxier. Maybe transfer it to kpng. A question is if anybody still uses it. Seems like you do, so if possible please explain if you have a use-case where the user-space proxier is wanted/needed.

[carlosdmz]

Hey @uablrek. Actually, I was casually reading the source code and I saw a TODO over there regarding the nil checks. Regarding your question, we should check if anyone actually uses it though, or maybe discuss its usage or deprecation a little further.

[carlosdmz]

Quick question though: I saw in the kpng news that the user-space proxy porting was near completion, maybe it's worth talking to them in order to check upon the status.

[thockin]

I don't object to making it clearer or more consistent, if you want to spend time on it, but it's not a high priority. That said, I think a nil value in that map would be really bad (and should be impossible) and I don't know how we would raise a signal about it.

[thockin]

let us know if you want to try to fix it, otherwise we can just close this.

[carlosdmz]

@thockin I already opened a PR with the clearer nil checks for this specific module pkg/proxy/userspace/roundrobin.go in #108259, so you can review it. I think whether this would have nil values into the service map we should discuss this apart and try to reproduce this case (if any would happen).

[dcbw]

PR submitted, so let's review and discuss further there.
/assign dcbw
/triage accepted