fix(rt): honor gateway expose Not in the connectivity matrix - #1913
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGateway peering ChangesGateway expose exclusion support
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
f701c36 to
fffd135
Compare
pau-hedgehog
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes order-dependent handling of GatewayPeering expose.IPs[].not exclusions in the release-test expectation model by making subnet-level reachability checks ignore Not entries and then applying the exclusions at the per-endpoint-IP level when building the connectivity matrix. It also expands the release-test infrastructure to build and use a matrix of discovered endpoints (server attachments + externals), enabling more precise modeling for scenarios like exclusions and NAT overlays.
Changes:
- Make
isVPCSubnetPresentInPeeringskipNotentries (order-independent), and applyNotexclusions as per-endpoint denies inpopulateConnectivityMatrix. - Refactor VLAB/release-test flow to collect endpoints, build a
ConnectivityMatrix, and run matrix-driven connectivity checks across multiple suites and NAT scenarios. - Add unit tests and new release-test cases covering GatewayPeering
Expose Notbehavior (both excluding an in-use host and excluding an unused host).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/hhfab/vlabrunner.go | Update SetupVPCs call site to ignore the new endpoints return value. |
| pkg/hhfab/testing.go | Add reachCheckUnsupported, update SetupVPCs to return endpoints, add matrix population + gateway expose Not exclusion application. |
| pkg/hhfab/rt_utils.go | Extend gateway peering spec helpers to emit Not entries; add helper to pick an unused host address and find servers in a subnet. |
| pkg/hhfab/rt_static_external.go | Update test function signature to accept a connectivity matrix. |
| pkg/hhfab/rt_single_vpc_suite.go | Thread connectivity matrix through suite tests; use matrix-driven connectivity during failover steps. |
| pkg/hhfab/rt_on_ready_suite.go | Update test function signature to accept (unused) connectivity matrix. |
| pkg/hhfab/rt_no_vpc_suite.go | Update test function signatures to accept (unused) connectivity matrix. |
| pkg/hhfab/rt_nat_tests.go | Refactor NAT tests to use matrix repopulation + NAT overlays rather than ad-hoc probing. |
| pkg/hhfab/rt_nat_external_tests.go | Refactor external NAT tests to use matrix overlays and add ping-stability checks. |
| pkg/hhfab/rt_multi_vpc_single_subnet_suite.go | Add two new release-test cases for GatewayPeering Expose Not; migrate relevant tests to matrix-driven connectivity checks. |
| pkg/hhfab/rt_multi_vpc_multi_subnet_suite.go | Update test function signatures and migrate subnet-filtering test to matrix-driven connectivity. |
| pkg/hhfab/rt_eslag_fallback.go | Update test function signature to accept (unused) connectivity matrix. |
| pkg/hhfab/rt_base.go | Change suite runner to build/pass a ConnectivityMatrix from SetupVPCs-discovered endpoints. |
| pkg/hhfab/matrix.go | Introduce ConnectivityMatrix model, overlays, and a matrix-driven connectivity runner (ping/iperf/curl + port-forward gating). |
| pkg/hhfab/expose_not_test.go | Add unit tests for Not ordering and matrix exclusion application semantics. |
| pkg/hhfab/endpoints.go | Add endpoint discovery (CollectServerEndpoints) and matrix endpoint reconciliation (ReplaceServerEndpoints). |
| pkg/hhfab/endpoints_test.go | Add unit tests for ReplaceServerEndpoints. |
| pkg/hhfab/cmdvlab.go | Change DoVLABSetupVPCs to return discovered endpoints. |
| cmd/hhfab/main.go | Update CLI call site to ignore DoVLABSetupVPCs’s new endpoints return value. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
pkg/hhfab/rt_single_vpc_suite.go (1)
195-195: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName the unused matrix parameter
_where the body was not migrated. These functions declarematrix *ConnectivityMatrixbut never reference it — they still callDoVLABTestConnectivity. Elsewhere in the same PR (rt_eslag_fallback.go,rt_no_vpc_suite.go,rt_on_ready_suite.go) the unused parameter is spelled_, which makes "not yet matrix-driven" explicit and avoids linter noise.
pkg/hhfab/rt_single_vpc_suite.go#L195-L195: rename to_ *ConnectivityMatrixinspineFailoverTest.pkg/hhfab/rt_single_vpc_suite.go#L326-L326: rename to_ *ConnectivityMatrixingatewayFailoverTest.pkg/hhfab/rt_single_vpc_suite.go#L639-L639: rename to_ *ConnectivityMatrixinmeshFailoverTest.pkg/hhfab/rt_single_vpc_suite.go#L773-L773: rename to_ *ConnectivityMatrixinnoRestrictionsTest.pkg/hhfab/rt_single_vpc_suite.go#L791-L791: rename to_ *ConnectivityMatrixinsingleVPCWithRestrictionsTest.pkg/hhfab/rt_single_vpc_suite.go#L926-L926: rename to_ *ConnectivityMatrixindnsNtpMtuTest.pkg/hhfab/rt_single_vpc_suite.go#L1109-L1109: rename to_ *ConnectivityMatrixindhcpRenewalTest.pkg/hhfab/rt_single_vpc_suite.go#L1528-L1528: rename to_ *ConnectivityMatrixinroceBasicTest.pkg/hhfab/rt_static_external.go#L25-L25: rename to_ *ConnectivityMatrixinstaticExternalPeeringTest.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/hhfab/rt_single_vpc_suite.go` at line 195, Rename the unused matrix parameter to _ *ConnectivityMatrix in spineFailoverTest, gatewayFailoverTest, meshFailoverTest, noRestrictionsTest, singleVPCWithRestrictionsTest, dnsNtpMtuTest, dhcpRenewalTest, and roceBasicTest in pkg/hhfab/rt_single_vpc_suite.go, and staticExternalPeeringTest in pkg/hhfab/rt_static_external.go. Preserve the existing function behavior and calls to DoVLABTestConnectivity.pkg/hhfab/testing.go (1)
1861-1888: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider memoizing reachability lookups per (src, dst) server name.
IsServerReachableis keyed only on server names, so every additional endpoint on the same server repeats an identical query. With multi-attachment servers this multiplies the peering/VPC lookups without changing the result. A smallmap[[2]string]Reachabilitycache would keep the loop O(servers²) instead of O(endpoints²), and it also makes the documented TODO easier to replace later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/hhfab/testing.go` around lines 1861 - 1888, The populateConnectivityMatrix function repeatedly calls IsServerReachable for endpoint pairs sharing the same source and destination server names. Add a local cache keyed by the two server names, reuse cached Reachability results before querying, and preserve the existing unsupported-error handling and connectivity expectations.pkg/hhfab/matrix.go (1)
630-645: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
toolboxMutexesis write-only.The map is built and populated but never read anywhere in the matrix path; it only serves as the "already seen" guard. Drop it and use a plain
map[string]struct{}(or key offsshByServer) unless a follow-up commit intends to serialize toolbox invocations.♻️ Proposed cleanup
sshByServer := map[string]*sshutil.Config{} - toolboxMutexes := map[string]*sync.Mutex{} for _, ep := range matrix.AllEndpoints { if ep.Server == nil { continue } name := ep.Server.Name - if _, ok := toolboxMutexes[name]; ok { + if _, ok := sshByServer[name]; ok { continue } ssh, ok := sshConfigs[name] if !ok { return fmt.Errorf("no ssh config for server %q referenced by matrix", name) //nolint:goerr113 } sshByServer[name] = ssh - toolboxMutexes[name] = &sync.Mutex{} }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/hhfab/matrix.go` around lines 630 - 645, Replace the write-only toolboxMutexes map in the matrix endpoint loop with a plain seen-server set, or reuse sshByServer as the duplicate guard. Preserve the existing behavior of skipping repeated server names and returning the missing SSH configuration error for unseen servers; do not retain mutex values unless they are actually used to serialize toolbox invocations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/hhfab/rt_base.go`:
- Around line 417-425: Update VPCPeeringTestCtx.setupTest’s noSetup branch to
discover the attached server endpoints and build a populated connectivity matrix
instead of returning NewConnectivityMatrix() directly. Reuse
CollectServerEndpoints and BuildConnectivityMatrix (or the existing equivalent
flow), preserve WaitReady, and propagate endpoint-discovery or
matrix-construction errors.
In `@pkg/hhfab/rt_utils.go`:
- Around line 827-847: Update pickUnusedHostAddress to compute hostCount using
uint64 so a /0 prefix does not overflow and produce an invalid loop bound;
preserve the existing offset range and candidate-address construction while
ensuring the loop remains correctly bounded for all valid IPv4 prefix lengths.
- Around line 1279-1325: Update findAllServersInSubnet and the related
single-server helper to handle NativeVLAN attachments by using the base
interface name without a “.0” suffix when subnet.VLAN is zero; retain the VLAN
suffix for tagged VLANs.
---
Nitpick comments:
In `@pkg/hhfab/matrix.go`:
- Around line 630-645: Replace the write-only toolboxMutexes map in the matrix
endpoint loop with a plain seen-server set, or reuse sshByServer as the
duplicate guard. Preserve the existing behavior of skipping repeated server
names and returning the missing SSH configuration error for unseen servers; do
not retain mutex values unless they are actually used to serialize toolbox
invocations.
In `@pkg/hhfab/rt_single_vpc_suite.go`:
- Line 195: Rename the unused matrix parameter to _ *ConnectivityMatrix in
spineFailoverTest, gatewayFailoverTest, meshFailoverTest, noRestrictionsTest,
singleVPCWithRestrictionsTest, dnsNtpMtuTest, dhcpRenewalTest, and roceBasicTest
in pkg/hhfab/rt_single_vpc_suite.go, and staticExternalPeeringTest in
pkg/hhfab/rt_static_external.go. Preserve the existing function behavior and
calls to DoVLABTestConnectivity.
In `@pkg/hhfab/testing.go`:
- Around line 1861-1888: The populateConnectivityMatrix function repeatedly
calls IsServerReachable for endpoint pairs sharing the same source and
destination server names. Add a local cache keyed by the two server names, reuse
cached Reachability results before querying, and preserve the existing
unsupported-error handling and connectivity expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d19e5876-703f-4ab4-b6cd-6934478c0b6a
📒 Files selected for processing (19)
cmd/hhfab/main.gopkg/hhfab/cmdvlab.gopkg/hhfab/endpoints.gopkg/hhfab/endpoints_test.gopkg/hhfab/expose_not_test.gopkg/hhfab/matrix.gopkg/hhfab/rt_base.gopkg/hhfab/rt_eslag_fallback.gopkg/hhfab/rt_multi_vpc_multi_subnet_suite.gopkg/hhfab/rt_multi_vpc_single_subnet_suite.gopkg/hhfab/rt_nat_external_tests.gopkg/hhfab/rt_nat_tests.gopkg/hhfab/rt_no_vpc_suite.gopkg/hhfab/rt_on_ready_suite.gopkg/hhfab/rt_single_vpc_suite.gopkg/hhfab/rt_static_external.gopkg/hhfab/rt_utils.gopkg/hhfab/testing.gopkg/hhfab/vlabrunner.go
fffd135 to
8a9d082
Compare
|
The earlier reviews ran while this branch still carried pre-merge copies of #1761's commits, so the diff showed 19 files and most findings landed on code that is now merged in master. Rebased onto master, the diff is 4 files and one finding was in scope. Fixed: Out of scope, all in code merged with #1761 rather than added here: the One correction: the suggestion that |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/hhfab/rt_utils.go (1)
842-849: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider normalizing
usedlookups. Candidates come fromnetip.AddrFrom4, so any 4-in-6 mapped address inused(e.g. from a source that produced::ffff:a.b.c.d) will not match and could be handed out as "unused". ApplyingUnmap()when building or probing the set removes that dependency on caller hygiene.♻️ Optional hardening
candidate := netip.AddrFrom4(b) - if !used[candidate] { + if !used[candidate] && !used[netip.AddrFrom16(candidate.As16())] { return candidate, nil }Alternatively, normalize with
Unmap()at the call sites that populateused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/hhfab/rt_utils.go` around lines 842 - 849, Normalize IPv4-mapped addresses before checking occupancy in the candidate allocation loop around netip.AddrFrom4 and the used lookup. Apply Unmap() when constructing or probing the used set so mapped forms such as ::ffff:a.b.c.d are treated as equivalent to their IPv4 candidates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/hhfab/rt_multi_vpc_single_subnet_suite.go`:
- Around line 446-473: Update the subnet selection before pickUnusedHostAddress
in the VPC2 flow to choose the first parseable IPv4 prefix by checking
prefix.Addr().Is4(), rather than always using subnetNames[0]. If VPC2 has no
IPv4 subnet, return the existing skip-style result with a clear reason; only
construct the /32 exclusion from the selected IPv4 prefix and address.
---
Nitpick comments:
In `@pkg/hhfab/rt_utils.go`:
- Around line 842-849: Normalize IPv4-mapped addresses before checking occupancy
in the candidate allocation loop around netip.AddrFrom4 and the used lookup.
Apply Unmap() when constructing or probing the used set so mapped forms such as
::ffff:a.b.c.d are treated as equivalent to their IPv4 candidates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d62e3d74-92ce-400c-93a0-ff11cbbedc88
📒 Files selected for processing (4)
pkg/hhfab/expose_not_test.gopkg/hhfab/rt_multi_vpc_single_subnet_suite.gopkg/hhfab/rt_utils.gopkg/hhfab/testing.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
pkg/hhfab/rt_multi_vpc_single_subnet_suite.go (1)
461-472: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSelect an IPv4 subnet before generating the
/32exclusion.
Unmap()correctly normalizes IPv4-mapped endpoint addresses, butprefixis still derived from the lexicographically first subnet. If that subnet is IPv6,unused.String() + "/32"creates an IPv6/32rather than a single-host exclusion. Select a valid IPv4 prefix first, or return the existing skip-style result when none exists. This is the same unresolved issue previously reported for this test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/hhfab/rt_multi_vpc_single_subnet_suite.go` around lines 461 - 472, Update the logic around usedIPs and the subsequent unused-host exclusion to select an IPv4 prefix before constructing the /32 exclusion. Do not derive prefix from the lexicographically first subnet when it may be IPv6; choose a valid IPv4 subnet, or return the existing skip-style result if none exists, while preserving Unmap normalization and IPv4 filtering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@pkg/hhfab/rt_multi_vpc_single_subnet_suite.go`:
- Around line 461-472: Update the logic around usedIPs and the subsequent
unused-host exclusion to select an IPv4 prefix before constructing the /32
exclusion. Do not derive prefix from the lexicographically first subnet when it
may be IPv6; choose a valid IPv4 subnet, or return the existing skip-style
result if none exists, while preserving Unmap normalization and IPv4 filtering.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18b974de-35ed-4f9e-a305-ef45b357da14
📒 Files selected for processing (1)
pkg/hhfab/rt_multi_vpc_single_subnet_suite.go
|
On the IPv4 subnet selection: On normalizing |
edipascale
left a comment
edipascale
left a comment
There was a problem hiding this comment.
I think it's great to add support for Not statements, but we need to make sure that the reach check stays honest, e.g. in case it is used from the kubectl plugin or with legacy test connectivity checks.
As a side note, it's easier for me to look at the PR if you answer the agent's comments directly (and ideally resolve them if it's the case) rather than as separate comments. I also think we should start doing a pass after an agent-aided PR and trim all unnecessary comments - I know I have been guilty of not doing this. Maybe there's a way to tune claude's verbosity with instructions in the CLAUDE.md and we should consider adding one, but this is a separate topic of course.
b62f5a6 to
969d1d2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
pkg/hhfab/expose_not_test.go:169
TestPickUnusedHostAddressusest.Run(tc.prefix, ...), but there are two cases with the same prefix ("10.0.1.0/24"), so subtest names collide and failures become harder to triage (and-runfiltering is ambiguous). Use a unique subtest name per case (e.g. includewant/wantErr).
t.Run(tc.prefix, func(t *testing.T) {
270ca06 to
ad951b1
Compare
4f19065 to
f21bd75
Compare
f21bd75 to
9785d62
Compare
pau-hedgehog
added
ci:+hlab
9785d62 to
a42316a
Compare
Release Tests 9 files 45 suites 4h 30m 9s ⏱️ Results for commit db228b8. ♻️ This comment has been updated with latest results. |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pau Capdevila <pau@githedgehog.com>
Isolates whether the excluded-server test's real drops (sent 5, rcvd 0 both directions) come from host-scoped exclusion or a prefix-collapse dataplane bug: subtracting a /32 from vpc-04's /24 fragments the covering route, and if FRR's Ge(len) prefix-list matching can't match the /24 against those fragments, the whole subnet could go dark regardless of which address was excluded. This clone excludes a /32 that is not assigned to any discovered endpoint, so the matrix expectations stay all-Allow. PASS confirms host-scoped exclusion and closes the open question in ISSUE-expose-not-order-dependent.md with no dataplane bug; a repeat server-3<->server-4 failure means prefix collapse is real and gets filed as its own bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Pau Capdevila <pau@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pau Capdevila <pau@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pau Capdevila <pau@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pau Capdevila <pau@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pau Capdevila <pau@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pau Capdevila <pau@githedgehog.com>
0281c3c to
b4aec91
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Pau Capdevila <pau@githedgehog.com>
I have addressed this. I will take into account all your (as usual) good points from now on |
edipascale
left a comment
edipascale
left a comment
There was a problem hiding this comment.
Thanks @pau-hedgehog, LGTM. I asked Claude to have a look too and it found something, but it's a nit rather than a blocker
|
|
||
| // pickUnusedHostAddress returns the first host address inside prefix that is not present | ||
| // in used, skipping the network and broadcast addresses. IPv4 only. | ||
| func pickUnusedHostAddress(prefix netip.Prefix, used map[netip.Addr]bool) (netip.Addr, error) { |
There was a problem hiding this comment.
pickUnusedHostAddress returns the subnet gateway. It returns the first host address not claimed by a discovered endpoint, and used is seeded only from matrix.AllEndpoints. VPC subnet gateways default to the first IP in the subnet (vpc_types.go:93), and servers get .2, so it always returns .1. From the CI log:
23:19:11 INF Excluding an unused address from gateway peering expose vpc=vpc-08 subnet=10.0.8.0/24 not=10.0.8.1/32
10.0.8.1 is vpc-08's anycast gateway on the leaf, not an unused address. The test still passes and still exercises the prefix-fragmentation question it was written for, so this isn't a correctness bug — but the test name, the log line and the doc comment all claim "an address nothing uses", and that's not what it picks. It also means the test's signal is entangled with however the dataplane treats the SVI address, which is a bad thing to have quietly baked into a regression test.
Fix is one line in gatewayPeeringExposeNotUnusedTest: seed usedIPs with vpc2.Spec.Subnets[subnetNames[0]].Gateway (falling back to the subnet's first address when the field is empty) before calling pickUnusedHostAddress.
isVPCSubnetPresentInPeeringhandled exposeNotentries inconsistently depending on their position inexpose.IPs: an include listed first silently ignored the exclusion, an exclusion listed first returnedreachCheckUnsupported. Same configuration, opposite verdicts, decided by list order.Per the gateway API a
Notis a standalone entry alongside the includes, and it narrows an already-exposed range down to individual addresses, which a per-subnet boolean cannot express. The helper now scans every entry of an expose before answering, so the verdict no longer depends on order, and it returns that expose'sNotranges inReachability.SourceExclusionsandDestExclusions. Callers that know the endpoint addresses apply them:populateConnectivityMatrixrecords Deny as it builds the expectation, andTestConnectivitynarrows its own verdict with the server addresses it already discovered, so aNotis covered off the matrix path as well. Both directions are denied, matching the dataplane: traffic addressed to an excluded host is dropped, and the excluded host's own traffic has no return path across the peering.An expose entry setting none of cidr, not or vpcSubnet is a malformed peering that admission rejects, so it now returns a plain error instead of
reachCheckUnsupported.Two release-test cases.
Gateway Peering Expose Notpeers the first two VPCs and excludes the address of a server discovered in the second.Gateway Peering Expose Not Unusedis the same setup but excludes an address nothing uses, derived from the subnet and checked against every discovered endpoint IP. With one server per VPC the first case cannot distinguish the excluded host being correctly denied from the whole exposed subnet going dark; the second can.Fixes #1906