v1.8 backports 2020-10-09 #13486
Merged
v1.8 backports 2020-10-09 #13486
Conversation
This file contains 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
[ upstream commit 208c69c ] So far, the rate limiting has been enforced before enforcing parallel requests. In theory, this is better because we can return error code 429 earlier to tell callers to slow down. In practice, many callers will retry immediately anyway which means that all the limiting will happen by the rate limiter. The rate limiter relies on reservations that need to be canceled. If too many requests happen in parallel, reservations can't be canceled quickly enough. By swapping the enforcement of parallel requests with the rate limiter, all requests will block for at least MaxWaitDuration if more than the allowed number of parallel requests are pending which will naturally pace the callers. Swapping the enforcement order requires the acquired semaphore to be released in error cases of the rate limiter. This requires to change the structure of Wait() to have a single error handling structure. By reusing finishRequest(), the metrics handler has to be adjusted slightly to account for new outcomes as it now bumps the metric for canceled requests as well. What remains unchanged is that only successful API requests are used to calculate the mean processing duration. Fixes: 3141e65 ("rate: Add API rate limiting system") Signed-off-by: Thomas Graf <thomas@cilium.io> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 75159c6 ] Simulate stress by feeding many parallel requests through the API rate limiter. The maximum wait duration will soon be hit for requests causing them to fail. Without the fix, this test would often fail. If it succeeded, the number of retries is in the range of 60-80K. With the fix, the test succeeds consistently and retries are in the range of 6-8K. That's a 10x reduction of retries. Signed-off-by: Thomas Graf <thomas@cilium.io> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 269aa1b ] Test that failing requests will not impact the rate limiter. This is already working correctly so this test only adds additional coverage. Signed-off-by: Thomas Graf <thomas@cilium.io> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 30160bc ] If both Spec and Specs are considered the same we don't need to re-add the CNP into the policy repository. The commit 134fdb5 should have added the Specs.DeepEqual and not remove the Spec.DeepEqual. Fixes: 134fdb5 ("k8s/watchers: fix missing missing CNP/CCNP updates") Signed-off-by: André Martins <andre@cilium.io> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 94ddcd3 ] MapStringEqualsIgnoreKeys will be used to compare maps while ignoring certain keys. Signed-off-by: André Martins <andre@cilium.io> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit bafc881 ] An equal function should never modify its fields, by doing it so it might cause race conditions. Fixes: 100d83c ("k8s: ignore kubectl.kubernetes.io/last-applied-configuration annotation") Signed-off-by: André Martins <andre@cilium.io> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
[ upstream commit 837caf8 ] - Use k8s secret key name 'ca.crt' for 'ca-certificates.crt' so that the example CNP works - Change the example monitor output from lyft.com to artii.herokuapp.com - Add deletion of the secrets to clean-up - Fix indentation and white-spacing in enumerated lists so that they are rendered properly Signed-off-by: Jarno Rajahalme <jarno@covalent.io> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
qmonnet
added
backport/1.8
kind/backports
|
test-backport-1.8 |
tklauser
approved these changes
Oct 13, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
pkg/k8s/apis/cilium.io/v2/types.goinstead of.../cnp_types.go, where the code has been moved for v1.9 in commit 8449d88 (k8s: Separate out CiliumNetworkPolicy CRD type)&& in.deepEqual(other)to the value returned by the function, as was the case in branch v1.8 (changed in v1.9 with commit c813a15 (k8s: Explicitly embed CRD types into CCNP).Once this PR is merged, you can update the PR labels via: