-
Notifications
You must be signed in to change notification settings - Fork 748
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
Add MTU to the plugin config #676
Conversation
16f7b42
to
3e13f7c
Compare
With
Without setting the environment variable
|
2daee65
to
80b3df3
Compare
plugins/routed-eni/cni.go
Outdated
@@ -67,6 +69,9 @@ type NetConf struct { | |||
// veth device name. It should be no more than four characters, and | |||
// defaults to 'eni'. | |||
VethPrefix string `json:"vethPrefix"` | |||
|
|||
// MTU for eth0 | |||
Mtu string `json:"mtu"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something I noticed... but golang naming practices generally encourage acronyms to be capitalized when exported, so this really should be "MTU", not "Mtu", like "CNIVersion" is capitalized above...
See more here: https://github.com/golang/go/wiki/CodeReviewComments#initialisms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, should be all upper case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Issue #665
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.