Merge pull request #117 from mogren/master

Fix fmt, vet and ineffassign issues

README.md

@@ -1,6 +1,15 @@
 # amazon-vpc-cni-k8s
 Networking plugin for pod networking in [Kubernetes](https://kubernetes.io/) using [Elastic Network Interfaces](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-eni.html) on AWS.
 
+[![BuildStatus Widget]][BuildStatus Result]
+[![GoReport Widget]][GoReport Status]
+
+
+[BuildStatus Result]: https://travis-ci.org/aws/amazon-vpc-cni-k8s
+[BuildStatus Widget]: https://travis-ci.org/aws/amazon-vpc-cni-k8s.svg?branch=master
+
+[GoReport Status]: https://goreportcard.com/report/github.com/aws/amazon-vpc-cni-k8s
+[GoReport Widget]: https://goreportcard.com/badge/github.com/aws/amazon-vpc-cni-k8s
 ## Setup
 Download the latest version of the [yaml](./config/) and apply it the cluster.
 ```

ipamd/datastore/data_store_test.go

@@ -134,9 +134,8 @@ func TestGetENIIPPools(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, len(eniIPPool), 2)
 
-	eniIPPool, err = ds.GetENIIPPools("dummy-eni")
+	_, err = ds.GetENIIPPools("dummy-eni")
 	assert.Error(t, err)
-
 }
 
 func TestDelENIIPv4Address(t *testing.T) {
@@ -193,11 +192,14 @@ func TestPodIPv4Address(t *testing.T) {
 	ip, _, err := ds.AssignPodIPv4Address(&podInfo)
 
 	assert.NoError(t, err)
-	assert.Equal(t, ip, "1.1.1.1")
-	assert.Equal(t, ds.total, 3)
-	assert.Equal(t, len(ds.eniIPPools["eni-1"].IPv4Addresses), 2)
-	assert.Equal(t, ds.eniIPPools["eni-1"].AssignedIPv4Addresses, 1)
+	assert.Equal(t, "1.1.1.1", ip)
+	assert.Equal(t, 3, ds.total)
+	assert.Equal(t, 2, len(ds.eniIPPools["eni-1"].IPv4Addresses))
+	assert.Equal(t, 1, ds.eniIPPools["eni-1"].AssignedIPv4Addresses)
+
 	ip, _, err = ds.AssignPodIPv4Address(&podInfo)
+	assert.NoError(t, err)
+	assert.Equal(t, "1.1.1.1", ip)
 
 	podsInfos := ds.GetPodInfos()
 	assert.Equal(t, len(*podsInfos), 1)
@@ -217,7 +219,7 @@ func TestPodIPv4Address(t *testing.T) {
 		IP:        "1.1.2.10",
 	}
 
-	ip, _, err = ds.AssignPodIPv4Address(&podInfo)
+	_, _, err = ds.AssignPodIPv4Address(&podInfo)
 	assert.Error(t, err)
 
 	podInfo = k8sapi.K8SPodInfo{
@@ -277,7 +279,7 @@ func TestPodIPv4Address(t *testing.T) {
 		Namespace: "ns-2",
 	}
 
-	ip, deviceNum, err := ds.UnAssignPodIPv4Address(&podInfo)
+	_, deviceNum, err := ds.UnAssignPodIPv4Address(&podInfo)
 	assert.NoError(t, err)
 	assert.Equal(t, ds.total, 3)
 	assert.Equal(t, ds.assigned, 2)

ipamd/ipamd.go

@@ -157,7 +157,7 @@ func (c *IPAMContext) nodeInit() error {
 
 	enis, err := c.awsClient.GetAttachedENIs()
 	if err != nil {
-		log.Error("Failed to retrive ENI info")
+		log.Error("Failed to retrieve ENI info")
 		return errors.New("ipamd init: failed to retrieve attached ENIs info")
 	}
 
@@ -455,7 +455,7 @@ func (c *IPAMContext) addENIaddressesToDataStore(ec2Addrs []*ec2.NetworkInterfac
 	return primaryIP
 }
 
-// returns all addresses on eni, the primary adderss on eni, error
+// returns all addresses on eni, the primary address on eni, error
 func (c *IPAMContext) getENIaddresses(eni string) ([]*ec2.NetworkInterfacePrivateIpAddress, string, error) {
 	ec2Addrs, _, err := c.awsClient.DescribeENI(eni)
 	if err != nil {
@@ -525,7 +525,7 @@ func logPoolStats(total, used, currentMaxAddrsPerENI, maxAddrsPerENI int) {
 		total, used, currentMaxAddrsPerENI, maxAddrsPerENI)
 }
 
-//nodeIPPoolTooLow returns true if IP pool is below low threshhold
+//nodeIPPoolTooLow returns true if IP pool is below low threshold
 func (c *IPAMContext) nodeIPPoolTooLow() bool {
 	warmENITarget := getWarmENITarget()
 	total, used := c.dataStore.GetStats()
@@ -535,7 +535,7 @@ func (c *IPAMContext) nodeIPPoolTooLow() bool {
 	return (available <= c.currentMaxAddrsPerENI*warmENITarget)
 }
 
-// NodeIPPoolTooHigh returns true if IP pool is above high threshhold
+// NodeIPPoolTooHigh returns true if IP pool is above high threshold
 func (c *IPAMContext) nodeIPPoolTooHigh() bool {
 	warmENITarget := getWarmENITarget()
 	total, used := c.dataStore.GetStats()
@@ -640,7 +640,7 @@ func (c *IPAMContext) eniIPPoolReconcile(ipPool map[string]*datastore.AddressInf
 		if err != nil {
 			log.Errorf("Failed to reconcile IP %s on eni %s", localIP, eni)
 			ipamdErrInc("ipReconcileAdd", err)
-			// contine instead of bailout due to one ip
+			// continue instead of bailout due to one ip
 			continue
 		}
 		reconcileCnt.With(prometheus.Labels{"fn": "eniIPPoolReconcileAdd"}).Inc()
@@ -655,7 +655,7 @@ func (c *IPAMContext) eniIPPoolReconcile(ipPool map[string]*datastore.AddressInf
 		if err != nil {
 			log.Errorf("Failed to reconcile and delete IP %s on eni %s, %v", existingIP, eni, err)
 			ipamdErrInc("ipReconcileDel", err)
-			// contine instead of bailout due to one ip
+			// continue instead of bailout due to one ip
 			continue
 		}
 		reconcileCnt.With(prometheus.Labels{"fn": "eniIPPoolReconcileDel"}).Inc()

ipamd/ipamd_test.go

@@ -107,9 +107,9 @@ func TestNodeInit(t *testing.T) {
 	testAddr2 := ipaddr02
 	primary := true
 	eniResp := []*ec2.NetworkInterfacePrivateIpAddress{
-		&ec2.NetworkInterfacePrivateIpAddress{
+		{
 			PrivateIpAddress: &testAddr1, Primary: &primary},
-		&ec2.NetworkInterfacePrivateIpAddress{
+		{
 			PrivateIpAddress: &testAddr2, Primary: &primary}}
 	mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
 	mockAWS.EXPECT().DescribeENI(primaryENIid).Return(eniResp, &attachmentID, nil)
@@ -122,17 +122,17 @@ func TestNodeInit(t *testing.T) {
 	testAddr12 := ipaddr12
 	primary = false
 	eniResp = []*ec2.NetworkInterfacePrivateIpAddress{
-		&ec2.NetworkInterfacePrivateIpAddress{
+		{
 			PrivateIpAddress: &testAddr11, Primary: &primary},
-		&ec2.NetworkInterfacePrivateIpAddress{
+		{
 			PrivateIpAddress: &testAddr12, Primary: &primary}}
 	mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
 	mockAWS.EXPECT().DescribeENI(secENIid).Return(eniResp, &attachmentID, nil)
 	mockNetwork.EXPECT().SetupENINetwork(gomock.Any(), secMAC, secDevice, secSubnet)
 
 	mockAWS.EXPECT().GetLocalIPv4().Return(ipaddr01)
 	k8sName := "/k8s_POD_" + "pod1" + "_" + "default" + "_" + "pod-uid" + "_0"
-	mockK8S.EXPECT().K8SGetLocalPodIPs().Return([]*k8sapi.K8SPodInfo{&k8sapi.K8SPodInfo{Name: "pod1",
+	mockK8S.EXPECT().K8SGetLocalPodIPs().Return([]*k8sapi.K8SPodInfo{{Name: "pod1",
 		Namespace: "default", UID: "pod-uid", IP: ipaddr02}}, nil)
 
 	var dockerList = make(map[string]*docker.ContainerInfo, 0)
@@ -165,14 +165,14 @@ func TestIncreaseIPPool(t *testing.T) {
 	mockAWS.EXPECT().AllocAllIPAddress(eni2)
 
 	mockAWS.EXPECT().GetAttachedENIs().Return([]awsutils.ENIMetadata{
-		awsutils.ENIMetadata{
+		{
 			ENIID:          primaryENIid,
 			MAC:            primaryMAC,
 			DeviceNumber:   primaryDevice,
 			SubnetIPv4CIDR: primarySubnet,
 			LocalIPv4s:     []string{ipaddr01, ipaddr02},
 		},
-		awsutils.ENIMetadata{
+		{
 			ENIID:          secENIid,
 			MAC:            secMAC,
 			DeviceNumber:   secDevice,
@@ -189,9 +189,9 @@ func TestIncreaseIPPool(t *testing.T) {
 
 	mockAWS.EXPECT().DescribeENI(eni2).Return(
 		[]*ec2.NetworkInterfacePrivateIpAddress{
-			&ec2.NetworkInterfacePrivateIpAddress{
+			{
 				PrivateIpAddress: &testAddr11, Primary: &primary},
-			&ec2.NetworkInterfacePrivateIpAddress{
+			{
 				PrivateIpAddress: &testAddr12, Primary: &primary}}, &attachmentID, nil)
 
 	mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
@@ -215,7 +215,7 @@ func TestNodeIPPoolReconcile(t *testing.T) {
 	mockContext.dataStore = datastore.NewDataStore()
 
 	mockAWS.EXPECT().GetAttachedENIs().Return([]awsutils.ENIMetadata{
-		awsutils.ENIMetadata{
+		{
 			ENIID:          primaryENIid,
 			MAC:            primaryMAC,
 			DeviceNumber:   primaryDevice,
@@ -234,9 +234,9 @@ func TestNodeIPPoolReconcile(t *testing.T) {
 
 	mockAWS.EXPECT().DescribeENI(primaryENIid).Return(
 		[]*ec2.NetworkInterfacePrivateIpAddress{
-			&ec2.NetworkInterfacePrivateIpAddress{
+			{
 				PrivateIpAddress: &testAddr1, Primary: &primary},
-			&ec2.NetworkInterfacePrivateIpAddress{
+			{
 				PrivateIpAddress: &testAddr2, Primary: &notPrimary}}, &attachmentID, nil)
 	mockAWS.EXPECT().GetPrimaryENI().Return(primaryENIid)
 
@@ -248,7 +248,7 @@ func TestNodeIPPoolReconcile(t *testing.T) {
 
 	// remove 1 IP
 	mockAWS.EXPECT().GetAttachedENIs().Return([]awsutils.ENIMetadata{
-		awsutils.ENIMetadata{
+		{
 			ENIID:          primaryENIid,
 			MAC:            primaryMAC,
 			DeviceNumber:   primaryDevice,

main.go

@@ -48,16 +48,16 @@ func _main() int {
 	discoverController := k8sapi.NewController(kubeClient)
 	go discoverController.DiscoverK8SPods()
 
-	aws_k8s_agent, err := ipamd.New(discoverController)
+	awsK8sAgent, err := ipamd.New(discoverController)
 
 	if err != nil {
 		log.Error("initialization failure", err)
 		return 1
 	}
 
-	go aws_k8s_agent.StartNodeIPPoolManager()
-	go aws_k8s_agent.SetupHTTP()
-	aws_k8s_agent.RunRPCHandler()
+	go awsK8sAgent.StartNodeIPPoolManager()
+	go awsK8sAgent.SetupHTTP()
+	awsK8sAgent.RunRPCHandler()
 
 	return 0
 }

pkg/awsutils/awsutils.go

@@ -342,7 +342,7 @@ func (cache *EC2InstanceMetadataCache) setPrimaryENI() error {
 			if err != nil {
 				awsAPIErrInc("GetMetadata", err)
 				log.Errorf("Failed to retrieve owner ID from instance metadata %v", err)
-				return errors.Wrap(err, "set primary eni: failed to retrive ownerID")
+				return errors.Wrap(err, "set primary eni: failed to retrieve ownerID")
 			}
 			log.Debugf("Found account ID: %s", ownerID)
 			cache.accountID = ownerID
@@ -694,7 +694,7 @@ func (cache *EC2InstanceMetadataCache) FreeENI(eniName string) error {
 	_, attachID, err := cache.DescribeENI(eniName)
 	if err != nil {
 		awsUtilsErrInc("FreeENIDescribeENIFailed", err)
-		log.Errorf("Failed to retrive eni %s attachment id %d", eniName, err)
+		log.Errorf("Failed to retrieve eni %s attachment id %d", eniName, err)
 		return errors.Wrap(err, "free eni: failed to retrieve eni's attachment id")
 	}
 

pkg/awsutils/awsutils_test.go

@@ -306,7 +306,7 @@ func TestAWSGetFreeDeviceNumberNoDevice(t *testing.T) {
 		ec2ENIs = append(ec2ENIs, ec2ENI)
 	}
 	result := &ec2.DescribeInstancesOutput{
-		Reservations: []*ec2.Reservation{&ec2.Reservation{Instances: []*ec2.Instance{&ec2.Instance{NetworkInterfaces: ec2ENIs}}}}}
+		Reservations: []*ec2.Reservation{{Instances: []*ec2.Instance{{NetworkInterfaces: ec2ENIs}}}}}
 
 	mockEC2.EXPECT().DescribeInstances(gomock.Any()).Return(result, nil)
 	ins := &EC2InstanceMetadataCache{ec2SVC: mockEC2}
@@ -341,7 +341,7 @@ func TestAllocENI(t *testing.T) {
 	ec2ENIs = append(ec2ENIs, ec2ENI)
 
 	result := &ec2.DescribeInstancesOutput{
-		Reservations: []*ec2.Reservation{&ec2.Reservation{Instances: []*ec2.Instance{&ec2.Instance{NetworkInterfaces: ec2ENIs}}}}}
+		Reservations: []*ec2.Reservation{{Instances: []*ec2.Instance{{NetworkInterfaces: ec2ENIs}}}}}
 
 	mockEC2.EXPECT().DescribeInstances(gomock.Any()).Return(result, nil)
 	attachmentID := "eni-attach-58ddda9d"
@@ -377,7 +377,7 @@ func TestAllocENINoFreeDevice(t *testing.T) {
 		ec2ENIs = append(ec2ENIs, ec2ENI)
 	}
 	result := &ec2.DescribeInstancesOutput{
-		Reservations: []*ec2.Reservation{&ec2.Reservation{Instances: []*ec2.Instance{&ec2.Instance{NetworkInterfaces: ec2ENIs}}}}}
+		Reservations: []*ec2.Reservation{{Instances: []*ec2.Instance{{NetworkInterfaces: ec2ENIs}}}}}
 
 	mockEC2.EXPECT().DescribeInstances(gomock.Any()).Return(result, nil)
 
@@ -413,7 +413,7 @@ func TestAllocENIMaxReached(t *testing.T) {
 	ec2ENIs = append(ec2ENIs, ec2ENI)
 
 	result := &ec2.DescribeInstancesOutput{
-		Reservations: []*ec2.Reservation{&ec2.Reservation{Instances: []*ec2.Instance{&ec2.Instance{NetworkInterfaces: ec2ENIs}}}}}
+		Reservations: []*ec2.Reservation{{Instances: []*ec2.Instance{{NetworkInterfaces: ec2ENIs}}}}}
 
 	mockEC2.EXPECT().DescribeInstances(gomock.Any()).Return(result, nil)
 	mockEC2.EXPECT().AttachNetworkInterface(gomock.Any()).Return(nil, errors.New("AttachmentLimitExceeded"))
@@ -431,7 +431,7 @@ func TestFreeENI(t *testing.T) {
 	attachmentID := eniAttachID
 	attachment := &ec2.NetworkInterfaceAttachment{AttachmentId: &attachmentID}
 	result := &ec2.DescribeNetworkInterfacesOutput{
-		NetworkInterfaces: []*ec2.NetworkInterface{&ec2.NetworkInterface{Attachment: attachment}}}
+		NetworkInterfaces: []*ec2.NetworkInterface{{Attachment: attachment}}}
 	mockEC2.EXPECT().DescribeNetworkInterfaces(gomock.Any()).Return(result, nil)
 	mockEC2.EXPECT().DetachNetworkInterface(gomock.Any()).Return(nil, nil)
 	mockEC2.EXPECT().DeleteNetworkInterface(gomock.Any()).Return(nil, nil)
@@ -448,7 +448,7 @@ func TestFreeENIRetry(t *testing.T) {
 	attachmentID := eniAttachID
 	attachment := &ec2.NetworkInterfaceAttachment{AttachmentId: &attachmentID}
 	result := &ec2.DescribeNetworkInterfacesOutput{
-		NetworkInterfaces: []*ec2.NetworkInterface{&ec2.NetworkInterface{Attachment: attachment}}}
+		NetworkInterfaces: []*ec2.NetworkInterface{{Attachment: attachment}}}
 	mockEC2.EXPECT().DescribeNetworkInterfaces(gomock.Any()).Return(result, nil)
 
 	// retry 2 times
@@ -468,7 +468,7 @@ func TestFreeENIRetryMax(t *testing.T) {
 	attachmentID := eniAttachID
 	attachment := &ec2.NetworkInterfaceAttachment{AttachmentId: &attachmentID}
 	result := &ec2.DescribeNetworkInterfacesOutput{
-		NetworkInterfaces: []*ec2.NetworkInterface{&ec2.NetworkInterface{Attachment: attachment}}}
+		NetworkInterfaces: []*ec2.NetworkInterface{{Attachment: attachment}}}
 	mockEC2.EXPECT().DescribeNetworkInterfaces(gomock.Any()).Return(result, nil)
 
 	mockEC2.EXPECT().DetachNetworkInterface(gomock.Any()).Return(nil, nil)