And use the fake interface in the unit tests, removing the dependency
on setting up FakeExec stuff when conntrack cleanup will be invoked.
Also, remove the isIPv6 argument to CleanStaleEntries, because it can
be inferred from the other args.
The iptables and nftables proxy backends had 2 unit tests
(TestDeleteEndpointConnections and TestProxierDeleteNodePortStaleUDP)
that were effectively testing that:
- If the proxy saw various Service/EndpointSlice events this would
result in specific changes to the service/endpoints trackers, AND
- If the service/endpoints trackers changed in those specific ways
this would result in specific UpdateServiceMapResult and
UpdateEndpointsMapResult values being generated, AND
- If you passed those specific UpdateServiceMapResult and
UpdateEndpointsMapResult values to conntrack.CleanStaleEntries it
would make specific calls to the lower-level conntrack methods,
AND
- If you called the lower-level conntrack methods with those
specific arguments, it would result in specific executions of the
conntrack binary, mixed with a specific number of klog
invocations.
This... is not a good unit test. We already test the change tracker
behavior in other unit tests, and we already tested the
Update{Service,Endpoints}MapResult behavior in the pkg/proxy unit
tests, and we already tested the conntrack exec behavior in
pkg/proxy/conntrack/conntrack_test.go, and we now test the
CleanStaleEntries behavior in pkg/proxy/conntrack/cleanup_test.go. So
there is no need to try to test the top-to-bottom behavior as a "unit
test".
Add an interface between CleanStaleEntries and the lower-level
conntrack helpers (ClearEntriesForIP, etc), and a fake implementation
of that interface, so that we can explicitly test CleanStaleEntries's
logic.
Remove some comments from conntrack.go that were explaining the
functions' callers rather than explaining the functions themselves
(and which were redundant with other comments in the callers anyway).
Fix the test names to match the functions they are testing.
Abstract out the repetitive FakeExec handling.
Explicitly specify the "expectCommand" in each one, to make it clearer
that that's really the part that we're testing.
For everything except TestExec(), test each case with both a "success"
result and a "nothing to delete" result from the conntrack binary.
This is to support the goal of enabling a feature by default for a single component only when the
feature in question is consumed by multiple components.
Overriden defaults are reflected in KnownFeatures and registered flag text.
This is a no-op change that makes the internal encryption config
hash more specific to it use and explicitly marks it as unstable.
Signed-off-by: Monis Khan <mok@microsoft.com>
ReplaceFeatureGates logs a warning when the default env var
implementation has been already used.
Such a situation indicates a potential ordering issue and usually is unwanted.
This PR add a feature gates mechanisim to client-go
as described in https://docs.google.com/document/d/1g9BGCRw-7ucUxO6OtCWbb3lfzUGA_uU9178wLdXAIfs
In particular:
- Adds a default feature gate implementation based on environment variables.
- Adds a set of methods for reading, overwriting the default implementation, and adding features to an external registry.
Co-authored-by: deads2k <deads@redhat.com>
Co-authored-by: Ben Luddy <bluddy@redhat.com>
Previously, the firewall-check chain was run in input, forward, and
output hook but not prerouting hook. When the LoadBalancer traffic
arrived at input or forward hook, it had been DNATed to endpoint IP and
port, so the firewall-check chain didn't take effect, traffic from out
of LoadBalancerSourceRanges was not dropped.
It was not detected by unit test because the chains were sorted by
priority only, while hook should be taken into consideration.
The commit links the firewall-check chain to prerouting hook and unlinks
it from input and forward hook to ensure the traffic is filtered before
DNAT. The priorities of filter chains are updated from "DNATPriority-1"
to "DNATPriority-10" to allow third parties to insert something else
between them.
Signed-off-by: Quan Tian <qtian@vmware.com>
