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.

Sign up for GitHub

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

Jump to bottom

[Windows] Bugfix for MTU configuration on nonexistent interface #2778

Merged
merged 1 commit into from
Sep 17, 2021
Merged

[Windows] Bugfix for MTU configuration on nonexistent interface #2778

merged 1 commit into from
Sep 17, 2021

Conversation

wenyingd
Copy link
Contributor

Use a separate goroutine to process interface configuration async. The
configurations are taken after the host interface is created.

Signed-off-by: wenyingd wenyingd@vmware.com

Fixes #2773

@wenyingd
Copy link
Contributor Author

/test-all
/skip-ipv6-all
/skip-ipv6-only-all

@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2021 •
edited
Loading

Codecov Report

Merging #2778 (725cfb2) into main (4cdd0b2) will decrease coverage by 0.53%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2778      +/-   ##
==========================================
- Coverage   60.91%   60.38%   -0.54%     
==========================================
  Files         285      284       -1     
  Lines       23041    23549     +508     
==========================================
+ Hits        14035    14219     +184     
- Misses       7497     7790     +293     
- Partials     1509     1540      +31
Flag Coverage Δ
kind-e2e-tests 47.81% <50.00%> (-0.55%) ⬇️
unit-tests 41.11% <0.00%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...g/agent/cniserver/interface_configuration_linux.go 16.15% <ø> (ø)
pkg/agent/cniserver/sriov.go 0.00% <0.00%> (ø)
pkg/agent/cniserver/pod_configuration.go 55.55% <100.00%> (ø)
pkg/agent/util/net.go 16.66% <0.00%> (-19.16%) ⬇️
pkg/agent/route/route_linux.go 28.52% <0.00%> (-14.98%) ⬇️
pkg/agent/proxy/proxier.go 58.13% <0.00%> (-5.97%) ⬇️
pkg/agent/proxy/types/groupcounter.go 83.33% <0.00%> (-4.17%) ⬇️
pkg/agent/openflow/pipeline.go 71.83% <0.00%> (-3.33%) ⬇️
...gent/controller/networkpolicy/status_controller.go 72.60% <0.00%> (-2.74%) ⬇️
pkg/agent/openflow/client.go 57.69% <0.00%> (-0.61%) ⬇️
... and 15 more

@AnishShah
Copy link

Fixes #2754
cc: @lzhecheng

@jianjuns jianjuns requested a review from tnqn September 15, 2021 00:20
tnqn
tnqn reviewed Sep 16, 2021
View reviewed changes
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.

s/nonexist/nonexistent/

pkg/agent/cniserver/interface_configuration_windows.go Show resolved Hide resolved
pkg/agent/cniserver/interface_configuration_windows.go Outdated Show resolved Hide resolved
pkg/agent/cniserver/interface_configuration_windows.go Outdated Show resolved Hide resolved
@wenyingd wenyingd changed the title [Windows] Bugfix for MTU configuration on nonexist interface [Windows] Bugfix for MTU configuration on nonexistent interface Sep 16, 2021
@wenyingd
Copy link
Contributor Author

/test-all
/skip-ipv6-all
/skip-ipv6-only-all

@wenyingd
Copy link
Contributor Author

/test-all
/skip-ipv6-all
/skip-ipv6-only-all

@wenyingd
Copy link
Contributor Author

/test-all
/skip-ipv6-all
/skip-ipv6-only-all

tnqn
tnqn previously approved these changes Sep 16, 2021
View reviewed changes
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
Copy link
Member

tnqn commented Sep 16, 2021

@wenyingd do we have any test that can detect this issue in the future if it's broken?

jianjuns
jianjuns previously approved these changes Sep 16, 2021
View reviewed changes
pkg/agent/cniserver/interface_configuration_windows.go Outdated Show resolved Hide resolved
@wenyingd
Copy link
Contributor Author

wenyingd commented Sep 17, 2021 •
edited
Loading

@wenyingd do we have any test that can detect this issue in the future if it's broken?

Not yet. I am thinking maybe we could add check in the e2e test in another PR.

@tnqn
Copy link
Member

tnqn commented Sep 17, 2021

@wenyingd do we have any test that can detect this issue in the future if it's broken?

Not yet. I am thinking maybe we could add check in the e2e test in another PR.

We don't have any e2e test using containerd?

@wenyingd
Copy link
Contributor Author

We don't have any e2e test using containerd?

No, the current windows-e2e-test is running Windows setup using Docker. @lzhecheng could correct me.

@wenyingd
[Windows] Bugfix for MTU configuration on nonexistent interface
725cfb2 
Configure MTU in a separate goroutine after the host interface is
created.

Signed-off-by: wenyingd <wenyingd@vmware.com>
@tnqn
Copy link
Member

tnqn commented Sep 17, 2021

/skip-all
/test-windows-e2e
/test-windows-networkpolicy
/test-windows-conformance

@lzhecheng
Copy link
Contributor

/test-windows-e2e
/test-windows-conformance

@tnqn tnqn merged commit b0e2299 into antrea-io:main Sep 17, 2021
This was referenced Sep 17, 2021
Merged
Merged
@AnishShah AnishShah mentioned this pull request Sep 17, 2021
Closed
@wenyingd wenyingd deleted the bug_2773 branch October 8, 2021 14:01
@tnqn tnqn added the action/backport Indicates a PR that requires backports. label Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnqn tnqn tnqn approved these changes

@jianjuns jianjuns jianjuns left review comments

Assignees
No one assigned
Labels
action/backport Indicates a PR that requires backports.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Windows] Failed to configure MTU when using containerd as the runtime
6 participants
@wenyingd @codecov-commenter @AnishShah @tnqn @lzhecheng @jianjuns