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

Auto discovery mtu #909

Merged
merged 1 commit into from
Jul 14, 2020
Merged

Auto discovery mtu #909

merged 1 commit into from
Jul 14, 2020

Conversation

reachjainrahul
Copy link
Contributor

Discover mtu of primary interface if defaultMTU param is not
set in the antrea.yaml

@vmwclabot
Copy link

@reachjainrahul, VMware has rejected your signed contributor license agreement. The merge can not proceed until the agreement has been resigned. Click here to resign the agreement.

@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-whole-conformance: to trigger all conformance tests on linux.
  • /skip-whole-conformance: to skip all conformance tests on linux.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-windows-conformance: to trigger windows conformance tests.
  • /skip-windows-conformance: to skip windows conformance tests.
  • /test-all: to trigger all tests (except whole conformance).
  • /skip-all: to skip all tests (except whole conformance).

These commands can only be run by members of the vmware-tanzu organization.

@reachjainrahul reachjainrahul force-pushed the antrea-eks branch 4 times, most recently from b97ac7a to c76abd2 Compare July 3, 2020 09:04
@reachjainrahul
Copy link
Contributor Author

/test-windows-conformance

1 similar comment
@wenyingd
Copy link
Contributor

wenyingd commented Jul 6, 2020

/test-windows-conformance

@wenyingd
Copy link
Contributor

wenyingd commented Jul 6, 2020

/test-windows-networkpolicy

wenyingd
wenyingd reviewed Jul 6, 2020
View reviewed changes
pkg/agent/agent.go Outdated Show resolved Hide resolved
pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
antoninbas
antoninbas reviewed Jul 6, 2020
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

minor comments, changes LGTM

pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
pkg/agent/agent.go
return i.networkConfig.DefaultMTU, nil
}
mtu := localIntf.MTU
if mtu <= 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add a comment to explain when this case could happen (or if it is not supposed to happen ever, but you feel like we should keep it, maybe add a comment to say that this is just defensive-programming)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MTU is assigned via DHCP option. If for whatever reason, its not set, this sanity check handles the case.

jianjuns
jianjuns reviewed Jul 7, 2020
View reviewed changes
cmd/antrea-agent/agent.go Outdated Show resolved Hide resolved
pkg/agent/agent.go Outdated Show resolved Hide resolved
jianjuns
jianjuns reviewed Jul 7, 2020
View reviewed changes
Copy link
Contributor

@jianjuns jianjuns left a comment

Choose a reason for hiding this comment

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

Also update documentation (EKS/cloud, docs/configuration.md) and yamls (antrea-agent.conf in build/yamls/base/conf/ and build/yamls/windows/base/conf/, and then "make manifest" to generate new yamls).

@jianjuns
Copy link
Contributor

jianjuns commented Jul 8, 2020

LGTM
/test-all

jianjuns
jianjuns reviewed Jul 8, 2020
View reviewed changes
build/yamls/base/conf/antrea-agent.conf Outdated Show resolved Hide resolved
pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
antoninbas
antoninbas reviewed Jul 10, 2020
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

some more nits, LGTM

build/yamls/base/conf/antrea-agent.conf Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
pkg/agent/agent.go Outdated Show resolved Hide resolved
Auto discovery mtu
d11fe51 
Discover mtu of primary interface if defaultMTU param is not
set in the antrea.yaml
@jianjuns
Copy link
Contributor

/test-all

@wenyingd wenyingd merged commit b654e12 into antrea-io:master Jul 14, 2020
@jianjuns jianjuns mentioned this pull request Aug 25, 2020
Closed
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Sep 22, 2020
@reachjainrahul @GraysonWu
Auto discovery mtu (antrea-io#909)
e26f50c 
Discover mtu of primary interface if defaultMTU param is not
set in the antrea.yaml

Signed-off-by:: Rahul Jain <rahulj@rahulj-a01.vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnqn tnqn tnqn left review comments

@wenyingd wenyingd wenyingd left review comments

@antoninbas antoninbas antoninbas left review comments

@jianjuns jianjuns jianjuns approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@reachjainrahul @vmwclabot @antrea-bot @wenyingd @jianjuns @antoninbas @tnqn