Default perf ports when the Perf block is partially specified - #43
Open
caseydavenport wants to merge 1 commit into
Open
Default perf ports when the Perf block is partially specified#43caseydavenport wants to merge 1 commit into
caseydavenport wants to merge 1 commit into
Conversation
The ports were only defaulted when Perf was absent entirely, so a config that set direct/service but left the ports out reached the dataplane with port 0 and the apiserver rejected the test policy. Move the range checks out of the external-only branch too, so a bad port fails at config load rather than 20 seconds into a run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A test config that supplies a
Perfblock without ports fails about 20 seconds into the run, when the apiserver rejects the test policy for having port 0. The defaults were only applied whenPerfwas missing entirely, so a block that setdirect/serviceand nothing else left both ports at zero.Ports are now defaulted per-field for non-external tests. External tests still require them explicitly, since the test tools don't work through NAT and the ports have to match whatever the service is exposed on.
The range checks moved out of the external-only branch too, so an out-of-range port is caught at config load instead of part way into a run.
Found while setting up an iptables/nftables comparison, where the config that triggered it was just
direct: true, service: true, external: false- which is what the defaults already are.