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

Add logic to dynamically discover primary interface name #196

Merged
merged 1 commit into from
Oct 8, 2018

Conversation

liwenwu-amazon
Copy link
Contributor

Issue #171 , if available:

Description of changes:

  • use metadata service to find out the mac address on primary interface
  • go through all the interfaces and find the interface which has same mac address as primary interface's mac.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

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

On what images have you tested this change? Ubuntu? CentOS?

pkg/awsutils/awsutils.go Outdated Show resolved Hide resolved
pkg/networkutils/network.go Outdated Show resolved Hide resolved

log.Debugf("Trying to find primary interface that has mac : %s", primaryMAC)

interfaces, err := net.Interfaces()
Copy link
Contributor

Choose a reason for hiding this comment

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

This call to list all interfaces is probably the biggest change. 😄

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 have only tested using amazon linux worker AMI. Since net.Interface() should works on all flavors of Linux, such as CentOS or Ubuntu, this change should work for CentOS and Ubuntu.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mogren I just unit tested the net.Interfaces() call on ubuntu t3.large instance. It is working as expected.
Here is the test program

cat test.go
package main

import (
	"fmt"
	"net"
)

func main() {

	interfaces, _ := net.Interfaces()

	for _, intf := range interfaces {
		fmt.Printf("interface %v, mac %v\n", intf.Name, intf.HardwareAddr)
	}
}

Here is the test output

root@ip-172-31-0-186:/home/ubuntu# ./test
interface lo, mac 
interface ens5, mac 0a:5f:92:e3:59:86

Here is the output of ifconfig

root@ip-172-31-0-186:/home/ubuntu# ifconfig
ens5: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 9001
        inet 172.31.0.186  netmask 255.255.240.0  broadcast 172.31.15.255
        inet6 fe80::85f:92ff:fee3:5986  prefixlen 64  scopeid 0x20<link>
        ether 0a:5f:92:e3:59:86  txqueuelen 1000  (Ethernet)
        RX packets 6072  bytes 7297970 (7.2 MB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 1699  bytes 199849 (199.8 KB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

lo: flags=73<UP,LOOPBACK,RUNNING>  mtu 65536
        inet 127.0.0.1  netmask 255.0.0.0
        inet6 ::1  prefixlen 128  scopeid 0x10<host>
        loop  txqueuelen 1000  (Local Loopback)
        RX packets 191  bytes 14956 (14.9 KB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 191  bytes 14956 (14.9 KB)
        TX errors 0  dropped 0 overruns 0  carrier 0  collisions 0

@liwenwu-amazon
Copy link
Contributor Author

@ewbankkit @phyllisstein, can you verify if this PR fixed your CNI in your environment? thanks

@phyllisstein
Copy link

phyllisstein commented Oct 8, 2018

@liwenwu-amazon This is awesome, thanks so much for taking up the issue! I just migrated from Kops with Ubuntu back to EKS with Amazon's Kube AMI, but I'll try rebuilding my Kops cluster again with this change in the next couple of days.

@ewbankkit
Copy link
Contributor

Yes, I should be able to verify in the next couple of days too.

Copy link
Contributor

@mogren mogren left a comment

Choose a reason for hiding this comment

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

Thanks for testing this!

The change will fix this issue, but it does feel a bit strange that we need to use the MAC address to figure out the primary interface name.

@liwenwu-amazon liwenwu-amazon merged commit 9a4a018 into aws:master Oct 8, 2018
@ewbankkit
Copy link
Contributor

@liwenwu-amazon I have verified that building from the latest master source and running the CNI plugin on CentOS Linux release 7.5.1804 is now successful.

@phyllisstein
Copy link

Confirmed that v1.2.1 does the trick with Ubuntu 18.04 as well. Thanks again!

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.

4 participants