ciliumidentity resiliency improvement #28912
Merged
Conversation
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
|
Commit f6baedd does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
2b58614 to
3761d25
Compare
|
/test |
1 similar comment
|
/test |
3761d25 to
2a0392a
Compare
|
/test |
d5e4786 to
c497e09
Compare
c497e09 to
ce82268
Compare
tommyp1ckles
commented
Nov 1, 2023
schlosna
reviewed
Nov 2, 2023
nathanjsweet
approved these changes
Nov 2, 2023
giorio94
requested changes
Nov 3, 2023
240ed3e to
6cab386
Compare
giorio94
approved these changes
Nov 6, 2023
6cab386 to
6f35c61
Compare
|
/test |
|
Hitting: #26351 in e2e |
|
Seeing on the ginkgo tests, seems like possibly a sporadic issue with the docker registry, retrying... |
6f35c61 to
1a04e3f
Compare
|
/test |
|
rebasing back one commit on main to see if that resolves/narrows down the gw conformance failures. |
|
/test |
1 similar comment
|
/test |
|
/test |
OnDelete k8s handler will now use a controller to reattempt master key recreation (i.e. situations where the owning agent sees a delete event on the k8s cid crd). This will use the localKeys store as a source of truth of local ciliumidentities, with sync attempts only happening if the numeric identity exists in localKeys. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
onDeleteLocked is used when draining the cache to remove all keys. In this case we want to remove all keys regardless of master keys. This switches this functionality to make the master key recreation optional, and only using this behavior when handling OnDelete events. Other uses of onDeleteLocked should always remove the identity locally. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
This test should cover: * OnDelete with master key protection enabled causing resilient sync of cid crd. * OnDelete without master key protection. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
These tests have been commented out for over four years, so we should probably just get rid of them. Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
1a04e3f to
452c640
Compare
|
/test |
|
@tommyp1ckles can we also have a release note for this one? Thanks! |
yup added |
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.
Currently, the CiliumIdentity K8s watcher OnDelete handler has a mechanism to recreate wrongly deleted CiliumIdentities.
This is done by checking if:
enableMasterKeyProtectionis enabled (default=true).If so, K8s Delete events are handled by recreating the local CID object. However this is not done resiliently - where if there is a failure at that time with the create request to apiserver then the error is logged and the issue never resolved.
To improve this, this will perform the request attempt in a controller, terminating only if:
This will provide resiliency against issues where the apiserver is temporarily unavailable, the agent has been rate limited, etc.
Testing
Kind: ensuring ciliumidentities are recreated by deleting the cid and looking for relevant logs: