Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve reconnect issue between Agent and OVS #4091

Merged
merged 1 commit into from
Aug 17, 2022

Conversation

wenyingd
Copy link
Contributor

@wenyingd wenyingd commented Aug 9, 2022

Bump up ofnet version

ofnet resolves a reconnect issue, this change is to involve the fix.

Fixes #4092

Signed-off-by: wenyingd [email protected]

@codecov
Copy link

codecov bot commented Aug 9, 2022

Codecov Report

Merging #4091 (93774c0) into main (d1c6a43) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4091      +/-   ##
==========================================
+ Coverage   65.70%   65.74%   +0.04%     
==========================================
  Files         304      304              
  Lines       46604    46779     +175     
==========================================
+ Hits        30621    30757     +136     
- Misses      13557    13622      +65     
+ Partials     2426     2400      -26     
Flag Coverage Δ
e2e-tests 39.39% <ø> (?)
integration-tests 34.95% <ø> (+<0.01%) ⬆️
kind-e2e-tests 48.35% <ø> (-0.99%) ⬇️
unit-tests 44.41% <ø> (+0.03%) ⬆️
Impacted Files Coverage Δ
pkg/agent/cniserver/ipam/antrea_ipam.go 51.08% <0.00%> (-22.95%) ⬇️
pkg/agent/flowexporter/exporter/certificate.go 27.77% <0.00%> (-22.23%) ⬇️
pkg/controller/networkpolicy/crd_utils.go 74.88% <0.00%> (-14.84%) ⬇️
...agent/flowexporter/connections/deny_connections.go 67.74% <0.00%> (-13.98%) ⬇️
pkg/agent/cniserver/ipam/ipam_service.go 51.68% <0.00%> (-12.36%) ⬇️
pkg/agent/flowexporter/exporter/exporter.go 67.82% <0.00%> (-10.15%) ⬇️
pkg/agent/flowexporter/utils.go 72.34% <0.00%> (-8.52%) ⬇️
pkg/ipam/poolallocator/allocator.go 49.76% <0.00%> (-5.96%) ⬇️
.../flowexporter/connections/conntrack_connections.go 76.66% <0.00%> (-4.77%) ⬇️
pkg/agent/cniserver/server.go 73.94% <0.00%> (-3.41%) ⬇️
... and 37 more

@wenyingd
Copy link
Contributor Author

wenyingd commented Aug 9, 2022

/test-all
/test-windows-all

@wenyingd wenyingd force-pushed the reconnect_issue branch 3 times, most recently from eb5a328 to 2ac2636 Compare August 16, 2022 11:57
@@ -931,6 +931,11 @@ func (c *client) ReplayFlows() {
c.replayMutex.Lock()
defer c.replayMutex.Unlock()

// Delete the existing groups to avoid unexpected error "OFPGMFC_GROUP_EXISTS" when replaying groups.
if err := c.bridge.DeleteAllGroups(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding here is not enough? what about agent restart case?
Anyway I wonder if this change is relevant to the bug we want to fix here. If not, could we make this change in next release?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should keep this grouped with the call to c.bridge.DeleteMeterAll. They both fulfill the same purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I revert the change of deleting groups in antrea, and I would do it in another patch since the related change in ofnet is also reverted.

@wenyingd
Copy link
Contributor Author

/test-all
/test-windows-all
/test-ipv6-all
/test-ipv6-only-all

@wenyingd
Copy link
Contributor Author

/test-flexible-ipam-e2e
/test-multicluster-e2e

ofnet resolves a reconnect issue, this change is to involve the fix.

Signed-off-by: wenyingd <[email protected]>
@wenyingd
Copy link
Contributor Author

Update go.mod/go.sum to refer to antrea.io/ofnet since the dependent patch is merged. All the tests are passed before this update.

@wenyingd
Copy link
Contributor Author

/test-all

@wenyingd wenyingd requested a review from tnqn August 17, 2022 10:47
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tnqn tnqn merged commit 9c9b098 into antrea-io:main Aug 17, 2022
heanlan pushed a commit to heanlan/antrea that referenced this pull request Mar 29, 2023
ofnet resolves a reconnect issue, this change is to involve the fix.

Signed-off-by: wenyingd <[email protected]>
@wenyingd wenyingd deleted the reconnect_issue branch May 30, 2023 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Agent fails to re-connect to OVS
3 participants