Remove/clarify two FIXME comments in the proxier unit test

When I first wrote TestInternalExternalMasquerade, I put "FIXME"
comments in all of the cases that seemed wrong to me, most of which
got removed as we fixed the corner cases. But there were two cases
where we decided that the implemented behavior, though odd, was
correct, and those FIXMEs never got removed.
This commit is contained in:
Dan Winship 2023-07-13 09:37:19 -04:00
parent 1437594786
commit 967ef29378

View File

@ -6931,10 +6931,12 @@ func TestInternalExternalMasquerade(t *testing.T) {
destIP: testNodeIP, destIP: testNodeIP,
destPort: 3002, destPort: 3002,
// FIXME: The short-circuit rule means we potentially send to a remote // See the comment below in the "pod to LB with eTP:Local" case.
// endpoint without masquerading, which is inconsistent with the // It doesn't actually make sense to short-circuit here, since if
// eTP:Cluster case. We should either be masquerading here, or NOT // you connect directly to a NodePort from outside the cluster,
// masquerading in the "pod to NodePort" case above. // you only get the local endpoints. But it's simpler for us and
// slightly more convenient for users to have this case get
// short-circuited too.
output: "10.180.0.2:80, 10.180.1.2:80", output: "10.180.0.2:80, 10.180.1.2:80",
masq: false, masq: false,
}, },
@ -6944,10 +6946,13 @@ func TestInternalExternalMasquerade(t *testing.T) {
destIP: "5.6.7.8", destIP: "5.6.7.8",
destPort: 80, destPort: 80,
// FIXME: The short-circuit rule means we potentially send to a remote // The short-circuit rule is supposed to make this behave the same
// endpoint without masquerading, which is inconsistent with the // way it would if the packet actually went out to the LB and then
// eTP:Cluster case. We should either be masquerading here, or NOT // came back into the cluster. So it gets routed to all endpoints,
// masquerading in the "pod to LB" case above. // not just local ones. In reality, if the packet actually left
// the cluster, it would have to get masqueraded, but since we can
// avoid doing that in the short-circuit case, and not masquerading
// is more useful, we avoid masquerading.
output: "10.180.0.2:80, 10.180.1.2:80", output: "10.180.0.2:80, 10.180.1.2:80",
masq: false, masq: false,
}, },