Improve resiliency on ip context policy routes #513
Merged
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
LionelJouin
force-pushed
the
policy-resiliency
branch
3 times, most recently
from
45d41ad
to
c9eaf56
Compare
When a forwarder was restarting, the handled table IDs by the forwarded were forgotten which caused duplicated tables/rules/routes in the pod and potentially left routes which could cause traffic disturbances in some cases. Now, the forwarder is checking if the tables which exist in the pod are corresponding to the requested policies before creating any other ones. In case such tables exist, the forwarder will consider it as its own table. Signed-off-by: Lionel Jouin <[email protected]>
LionelJouin
force-pushed
the
policy-resiliency
branch
from
c9eaf56
to
7d6d26a
Compare
| } | ||
| ps = make(map[int]*networkservice.PolicyRoute) | ||
| tableIDs.Store(conn.GetId(), ps) | ||
| } |
There was a problem hiding this comment.
We will be here only once for each client, so if the map is not empty, then nothing needs to be restored.
else {
return nil
}
| missingPolicies, _ := getPolicyDifferences(ps, conn.Context.IpContext.Policies) | ||
|
|
||
| // try to find the corresponding missing policies in the network namespace of the pod | ||
| for _, policy := range missingPolicies { |
There was a problem hiding this comment.
I don't think we need missingPolicies. Because if forwarder was restarted then ps is always empty and we need to add all of conn.Context.IpContext.Policies
for _, policy := range conn.Context.IpContext.Policies {
...
}
There was a problem hiding this comment.
right, both comments come together. I was thinking maybe it would be possible the rule to be already existing in the pod while not registered in the map. But you are right, that could probably not happen. I will fix it
* Fix based on comment in PR/513 * After a forwarder restart, the routes could have been removed, so instead of trying to recover the rule/route, the heal part will delete them and they will be recreated. Signed-off-by: Lionel Jouin <[email protected]>
LionelJouin
force-pushed
the
policy-resiliency
branch
from
21c4e0c
to
72a60e6
Compare
|
@glazychev-art , in the new commit I changed the way to handle the healing. In case of a forwarder restart, It could happen that the routes are removed, the most simple way I found is to flush the rule/route, so they will be recreated in the common.go. |
|
LGTM |
glazychev-art
approved these changes
Sep 7, 2022
nsmbot
pushed a commit
to networkservicemesh/cmd-nse-l7-proxy
that referenced
this pull request
Sep 20, 2022
…k-kernel@main PR link: networkservicemesh/sdk-kernel#513 Commit: 0a1f4be Author: Ed Warnicke Date: 2022-09-20 09:53:28 -0500 Message: - Merge pull request #513 from Nordix/policy-resiliency Signed-off-by: NSMBot <[email protected]>
nsmbot
pushed a commit
to networkservicemesh/sdk-sriov
that referenced
this pull request
Sep 20, 2022
…k-kernel@main PR link: networkservicemesh/sdk-kernel#513 Commit: 0a1f4be Author: Ed Warnicke Date: 2022-09-20 09:53:28 -0500 Message: - Merge pull request #513 from Nordix/policy-resiliency Signed-off-by: NSMBot <[email protected]>
This was referenced Sep 20, 2022
nsmbot
pushed a commit
to networkservicemesh/sdk-vpp
that referenced
this pull request
Sep 20, 2022
…k-kernel@main PR link: networkservicemesh/sdk-kernel#513 Commit: 0a1f4be Author: Ed Warnicke Date: 2022-09-20 09:53:28 -0500 Message: - Merge pull request #513 from Nordix/policy-resiliency Signed-off-by: NSMBot <[email protected]>
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.
When a forwarder was restarting, the handled table IDs by the forwarded
were forgotten which caused duplicated tables/rules/routes in the pod
and potentially left routes which could cause traffic disturbances in
some cases.
Now, the forwarder is checking if the tables which exist in the pod are
corresponding to the requested policies before creating any other ones.
In case such tables exist, the forwarder will consider it as its own
table.
#426