From 593a720634a33123ddbcc1158a5338ad420986f8 Mon Sep 17 00:00:00 2001 From: Andrew Rudoi Date: Mon, 1 Apr 2019 07:06:53 -0700 Subject: [PATCH 1/6] fix: get container ID from kube rather than docker --- ipamd/ipamd.go | 24 --------- ipamd/ipamd_test.go | 64 +++++++++++++--------- pkg/docker/docker.go | 91 -------------------------------- pkg/docker/generate_mocks.go | 16 ------ pkg/docker/mocks/docker_mocks.go | 61 --------------------- pkg/k8sapi/discovery.go | 9 +++- 6 files changed, 46 insertions(+), 219 deletions(-) delete mode 100644 pkg/docker/docker.go delete mode 100644 pkg/docker/generate_mocks.go delete mode 100644 pkg/docker/mocks/docker_mocks.go diff --git a/ipamd/ipamd.go b/ipamd/ipamd.go index 32015ad8be..e8de6da91e 100644 --- a/ipamd/ipamd.go +++ b/ipamd/ipamd.go @@ -32,7 +32,6 @@ import ( "github.com/aws/amazon-vpc-cni-k8s/ipamd/datastore" "github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils" - "github.com/aws/amazon-vpc-cni-k8s/pkg/docker" "github.com/aws/amazon-vpc-cni-k8s/pkg/eniconfig" "github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi" "github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils" @@ -148,7 +147,6 @@ type IPAMContext struct { dataStore *datastore.DataStore k8sClient k8sapi.K8SAPIs eniConfig eniconfig.ENIConfig - dockerClient docker.APIs networkClient networkutils.NetworkAPIs currentMaxAddrsPerENI int @@ -183,7 +181,6 @@ func New(k8sapiClient k8sapi.K8SAPIs, eniConfig *eniconfig.ENIConfigController) c.k8sClient = k8sapiClient c.networkClient = networkutils.New() - c.dockerClient = docker.New() c.eniConfig = eniConfig client, err := awsutils.New() @@ -324,27 +321,6 @@ func (c *IPAMContext) getLocalPodsWithRetry() ([]*k8sapi.K8SPodInfo, error) { return pods, nil } - var containers map[string]*docker.ContainerInfo - for retry := 1; retry <= maxK8SRetries; retry++ { - containers, err = c.dockerClient.GetRunningContainers() - if err == nil { - break - } - log.Infof("Not able to get local containers yet (attempt %d/%d): %v", retry, maxK8SRetries, err) - time.Sleep(retryK8SInterval) - } - - // TODO consider using map - for _, pod := range pods { - // needs to find the container ID - for _, container := range containers { - if container.K8SUID == pod.UID { - log.Debugf("Found pod(%v)'s container ID: %v ", container.Name, container.ID) - pod.Container = container.ID - break - } - } - } return pods, nil } diff --git a/ipamd/ipamd_test.go b/ipamd/ipamd_test.go index 757146264b..9ef453ed0e 100644 --- a/ipamd/ipamd_test.go +++ b/ipamd/ipamd_test.go @@ -19,18 +19,17 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/pkg/errors" "github.com/vishvananda/netlink" "github.com/aws/amazon-vpc-cni-k8s/ipamd/datastore" "github.com/aws/amazon-vpc-cni-k8s/pkg/apis/crd/v1alpha1" "github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils" - "github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils/mocks" - "github.com/aws/amazon-vpc-cni-k8s/pkg/docker" - "github.com/aws/amazon-vpc-cni-k8s/pkg/docker/mocks" - "github.com/aws/amazon-vpc-cni-k8s/pkg/eniconfig/mocks" + mock_awsutils "github.com/aws/amazon-vpc-cni-k8s/pkg/awsutils/mocks" + mock_eniconfig "github.com/aws/amazon-vpc-cni-k8s/pkg/eniconfig/mocks" "github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi" - "github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi/mocks" - "github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils/mocks" + mock_k8sapi "github.com/aws/amazon-vpc-cni-k8s/pkg/k8sapi/mocks" + mock_networkutils "github.com/aws/amazon-vpc-cni-k8s/pkg/networkutils/mocks" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" @@ -58,26 +57,23 @@ const ( func setup(t *testing.T) (*gomock.Controller, *mock_awsutils.MockAPIs, *mock_k8sapi.MockK8SAPIs, - *mock_docker.MockAPIs, *mock_networkutils.MockNetworkAPIs, *mock_eniconfig.MockENIConfig) { ctrl := gomock.NewController(t) return ctrl, mock_awsutils.NewMockAPIs(ctrl), mock_k8sapi.NewMockK8SAPIs(ctrl), - mock_docker.NewMockAPIs(ctrl), mock_networkutils.NewMockNetworkAPIs(ctrl), mock_eniconfig.NewMockENIConfig(ctrl) } func TestNodeInit(t *testing.T) { - ctrl, mockAWS, mockK8S, mockDocker, mockNetwork, _ := setup(t) + ctrl, mockAWS, mockK8S, mockNetwork, _ := setup(t) defer ctrl.Finish() mockContext := &IPAMContext{ awsClient: mockAWS, k8sClient: mockK8S, - dockerClient: mockDocker, networkClient: mockNetwork} eni1 := awsutils.ENIMetadata{ @@ -139,14 +135,8 @@ func TestNodeInit(t *testing.T) { 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{{Name: "pod1", - Namespace: "default", UID: "pod-uid", IP: ipaddr02}}, nil) - - var dockerList = make(map[string]*docker.ContainerInfo, 0) - dockerList["pod-uid"] = &docker.ContainerInfo{ID: "docker-id", - Name: k8sName, K8SUID: "pod-uid"} - mockDocker.EXPECT().GetRunningContainers().Return(dockerList, nil) + Namespace: "default", UID: "pod-uid", Container: "container-uid", IP: ipaddr02}}, nil) var rules []netlink.Rule mockNetwork.EXPECT().GetRuleList().Return(rules, nil) @@ -169,8 +159,30 @@ func TestIncreaseIPPoolCustomENI(t *testing.T) { testIncreaseIPPool(t, true) } +func TestIncreaseIPPoolCustomENINoCfg(t *testing.T) { + os.Setenv(envCustomNetworkCfg, "true") + ctrl, mockAWS, mockK8S, mockNetwork, mockENIConfig := setup(t) + defer ctrl.Finish() + + mockContext := &IPAMContext{ + awsClient: mockAWS, + k8sClient: mockK8S, + networkClient: mockNetwork, + eniConfig: mockENIConfig, + primaryIP: make(map[string]string), + } + + mockContext.dataStore = datastore.NewDataStore() + + mockAWS.EXPECT().GetENILimit().Return(4, nil) + mockENIConfig.EXPECT().MyENIConfig().Return(nil, errors.New("no POD eni config")) + + mockContext.increaseIPPool() + +} + func testIncreaseIPPool(t *testing.T, useENIConfig bool) { - ctrl, mockAWS, mockK8S, _, mockNetwork, mockENIConfig := setup(t) + ctrl, mockAWS, mockK8S, mockNetwork, mockENIConfig := setup(t) defer ctrl.Finish() mockContext := &IPAMContext{ @@ -235,8 +247,8 @@ func testIncreaseIPPool(t *testing.T, useENIConfig bool) { mockAWS.EXPECT().DescribeENI(eni2).Return( []*ec2.NetworkInterfacePrivateIpAddress{ - { PrivateIpAddress: &testAddr11, Primary: &primary }, - { PrivateIpAddress: &testAddr12, Primary: ¬Primary }, + {PrivateIpAddress: &testAddr11, Primary: &primary}, + {PrivateIpAddress: &testAddr12, Primary: ¬Primary}, }, &attachmentID, nil) @@ -249,9 +261,9 @@ func testIncreaseIPPool(t *testing.T, useENIConfig bool) { mockAWS.EXPECT().DescribeENI(eni2).Return( []*ec2.NetworkInterfacePrivateIpAddress{ - { PrivateIpAddress: &testAddr11, Primary: &primary }, - { PrivateIpAddress: &testAddr12, Primary: ¬Primary }, - { PrivateIpAddress: &testAddr12, Primary: ¬Primary }, + {PrivateIpAddress: &testAddr11, Primary: &primary}, + {PrivateIpAddress: &testAddr12, Primary: ¬Primary}, + {PrivateIpAddress: &testAddr12, Primary: ¬Primary}, }, &attachmentID, nil) @@ -260,7 +272,7 @@ func testIncreaseIPPool(t *testing.T, useENIConfig bool) { } func TestNodeIPPoolReconcile(t *testing.T) { - ctrl, mockAWS, mockK8S, _, mockNetwork, _ := setup(t) + ctrl, mockAWS, mockK8S, mockNetwork, _ := setup(t) defer ctrl.Finish() mockContext := &IPAMContext{ @@ -331,7 +343,7 @@ func TestNodeIPPoolReconcile(t *testing.T) { } func TestGetWarmENITarget(t *testing.T) { - ctrl, _, _, _, _, _ := setup(t) + ctrl, _, _, _, _ := setup(t) defer ctrl.Finish() _ = os.Setenv("WARM_IP_TARGET", "5") @@ -348,7 +360,7 @@ func TestGetWarmENITarget(t *testing.T) { } func TestGetMaxENI(t *testing.T) { - ctrl, _, _, _, _, _ := setup(t) + ctrl, _, _, _, _ := setup(t) defer ctrl.Finish() // MaxENI 5 is less than upper bound of 10, so 5 diff --git a/pkg/docker/docker.go b/pkg/docker/docker.go deleted file mode 100644 index 4ab18137e5..0000000000 --- a/pkg/docker/docker.go +++ /dev/null @@ -1,91 +0,0 @@ -package docker - -import ( - "github.com/docker/docker/api/types" - "github.com/docker/docker/client" - "golang.org/x/net/context" - - "github.com/pkg/errors" - - log "github.com/cihub/seelog" -) - -// ContainerInfo provides container information -type ContainerInfo struct { - ID string - Name string - K8SUID string -} - -// APIs provides Docker API -type APIs interface { - GetRunningContainers() (map[string]*ContainerInfo, error) -} - -type Client struct{} - -func New() *Client { - return &Client{} -} - -func (c *Client) GetRunningContainers() (map[string]*ContainerInfo, error) { - var containerInfos = make(map[string]*ContainerInfo) - - cli, err := client.NewEnvClient() - if err != nil { - return nil, err - } - defer cli.Close() - - containers, err := cli.ContainerList(context.Background(), types.ContainerListOptions{}) - if err != nil { - return nil, err - } - - for _, container := range containers { - log.Infof("GetRunningContainers: Discovered running docker: %s %s %s State: %s Status: %s ", - container.ID, container.Names[0], container.Labels["io.kubernetes.pod.uid"], container.State, container.Status) - - containerType, ok := container.Labels["io.kubernetes.docker.type"] - if !ok { - log.Infof("GetRunningContainers: skip non pause container") - continue - } - - if containerType != "podsandbox" { - log.Infof("GetRunningContainers: skip container type: %s", containerType) - continue - } - - log.Debugf("GetRunningContainers: containerType %s", containerType) - - if container.State != "running" { - log.Infof("GetRunningContainers: skip container who is not running") - continue - } - - uid := container.Labels["io.kubernetes.pod.uid"] - _, ok = containerInfos[uid] - if !ok { - containerInfos[uid] = &ContainerInfo{ - ID: container.ID, - Name: container.Names[0], - K8SUID: uid} - continue - } - - if container.Names[0] != containerInfos[uid].Name { - log.Infof("GetRunningContainers: same uid matched by container:%s, %s container id %s", - containerInfos[uid].Name, container.Names[0], container.ID) - continue - } - - if container.Names[0] == containerInfos[uid].Name { - log.Errorf("GetRunningContainers: Conflict container id %s for container %s", - container.ID, containerInfos[uid].Name) - return nil, errors.New("conflict docker runtime info") - } - } - - return containerInfos, nil -} diff --git a/pkg/docker/generate_mocks.go b/pkg/docker/generate_mocks.go deleted file mode 100644 index bfbb2667c5..0000000000 --- a/pkg/docker/generate_mocks.go +++ /dev/null @@ -1,16 +0,0 @@ -// Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - -package docker - -//go:generate go run ../../scripts/mockgen.go github.com/aws/amazon-vpc-cni-k8s/pkg/docker APIs mocks/docker_mocks.go diff --git a/pkg/docker/mocks/docker_mocks.go b/pkg/docker/mocks/docker_mocks.go deleted file mode 100644 index c7558c05b1..0000000000 --- a/pkg/docker/mocks/docker_mocks.go +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"). You may -// not use this file except in compliance with the License. A copy of the -// License is located at -// -// http://aws.amazon.com/apache2.0/ -// -// or in the "license" file accompanying this file. This file is distributed -// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either -// express or implied. See the License for the specific language governing -// permissions and limitations under the License. - -// Code generated by MockGen. DO NOT EDIT. -// Source: github.com/aws/amazon-vpc-cni-k8s/pkg/docker (interfaces: APIs) - -// Package mock_docker is a generated GoMock package. -package mock_docker - -import ( - reflect "reflect" - - docker "github.com/aws/amazon-vpc-cni-k8s/pkg/docker" - gomock "github.com/golang/mock/gomock" -) - -// MockAPIs is a mock of APIs interface -type MockAPIs struct { - ctrl *gomock.Controller - recorder *MockAPIsMockRecorder -} - -// MockAPIsMockRecorder is the mock recorder for MockAPIs -type MockAPIsMockRecorder struct { - mock *MockAPIs -} - -// NewMockAPIs creates a new mock instance -func NewMockAPIs(ctrl *gomock.Controller) *MockAPIs { - mock := &MockAPIs{ctrl: ctrl} - mock.recorder = &MockAPIsMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockAPIs) EXPECT() *MockAPIsMockRecorder { - return m.recorder -} - -// GetRunningContainers mocks base method -func (m *MockAPIs) GetRunningContainers() (map[string]*docker.ContainerInfo, error) { - ret := m.ctrl.Call(m, "GetRunningContainers") - ret0, _ := ret[0].(map[string]*docker.ContainerInfo) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetRunningContainers indicates an expected call of GetRunningContainers -func (mr *MockAPIsMockRecorder) GetRunningContainers() *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRunningContainers", reflect.TypeOf((*MockAPIs)(nil).GetRunningContainers)) -} diff --git a/pkg/k8sapi/discovery.go b/pkg/k8sapi/discovery.go index a7470124e3..00e0feb4a8 100644 --- a/pkg/k8sapi/discovery.go +++ b/pkg/k8sapi/discovery.go @@ -15,7 +15,7 @@ import ( clientset "k8s.io/client-go/kubernetes" "github.com/operator-framework/operator-sdk/pkg/k8sclient" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -246,7 +246,14 @@ func (d *Controller) handlePodUpdate(key string) error { d.workerPodsLock.Lock() defer d.workerPodsLock.Unlock() + var containerID string + + if len(pod.Status.ContainerStatuses) > 0 { + containerID = pod.Status.ContainerStatuses[0].ContainerID + } + d.workerPods[key] = &K8SPodInfo{ + Container: containerID, Name: podName, Namespace: pod.GetNamespace(), UID: string(pod.GetUID()), From a487c99ac0e85c93bf057878d4e5c13a68098d4a Mon Sep 17 00:00:00 2001 From: Andrew Rudoi Date: Mon, 8 Apr 2019 07:13:41 -0700 Subject: [PATCH 2/6] chore: add log statement when containerID is found --- pkg/k8sapi/discovery.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/k8sapi/discovery.go b/pkg/k8sapi/discovery.go index 00e0feb4a8..9bc4bda9a6 100644 --- a/pkg/k8sapi/discovery.go +++ b/pkg/k8sapi/discovery.go @@ -249,6 +249,7 @@ func (d *Controller) handlePodUpdate(key string) error { var containerID string if len(pod.Status.ContainerStatuses) > 0 { + log.Debugf("Found pod (%v)'s container ID: %v", podName, containerID) containerID = pod.Status.ContainerStatuses[0].ContainerID } From 4b5053689fc110f70541422df18ef4512995c29e Mon Sep 17 00:00:00 2001 From: Andrew Rudoi Date: Mon, 8 Apr 2019 09:43:00 -0700 Subject: [PATCH 3/6] fix: log containerID after assignment --- pkg/k8sapi/discovery.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/k8sapi/discovery.go b/pkg/k8sapi/discovery.go index 9bc4bda9a6..c72840811c 100644 --- a/pkg/k8sapi/discovery.go +++ b/pkg/k8sapi/discovery.go @@ -249,8 +249,8 @@ func (d *Controller) handlePodUpdate(key string) error { var containerID string if len(pod.Status.ContainerStatuses) > 0 { - log.Debugf("Found pod (%v)'s container ID: %v", podName, containerID) containerID = pod.Status.ContainerStatuses[0].ContainerID + log.Debugf("Found pod (%v)'s container ID: %v", podName, containerID) } d.workerPods[key] = &K8SPodInfo{ From 4a30273f296183537bc4a5e252caf0b0fb3a242e Mon Sep 17 00:00:00 2001 From: Andrew Rudoi Date: Mon, 13 May 2019 07:51:03 -0700 Subject: [PATCH 4/6] test: fix ipamd tests --- ipamd/ipamd_test.go | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/ipamd/ipamd_test.go b/ipamd/ipamd_test.go index 9ef453ed0e..4e4eed3782 100644 --- a/ipamd/ipamd_test.go +++ b/ipamd/ipamd_test.go @@ -19,7 +19,6 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" - "github.com/pkg/errors" "github.com/vishvananda/netlink" "github.com/aws/amazon-vpc-cni-k8s/ipamd/datastore" @@ -159,28 +158,6 @@ func TestIncreaseIPPoolCustomENI(t *testing.T) { testIncreaseIPPool(t, true) } -func TestIncreaseIPPoolCustomENINoCfg(t *testing.T) { - os.Setenv(envCustomNetworkCfg, "true") - ctrl, mockAWS, mockK8S, mockNetwork, mockENIConfig := setup(t) - defer ctrl.Finish() - - mockContext := &IPAMContext{ - awsClient: mockAWS, - k8sClient: mockK8S, - networkClient: mockNetwork, - eniConfig: mockENIConfig, - primaryIP: make(map[string]string), - } - - mockContext.dataStore = datastore.NewDataStore() - - mockAWS.EXPECT().GetENILimit().Return(4, nil) - mockENIConfig.EXPECT().MyENIConfig().Return(nil, errors.New("no POD eni config")) - - mockContext.increaseIPPool() - -} - func testIncreaseIPPool(t *testing.T, useENIConfig bool) { ctrl, mockAWS, mockK8S, mockNetwork, mockENIConfig := setup(t) defer ctrl.Finish() @@ -395,7 +372,7 @@ func TestGetMaxENI(t *testing.T) { } func TestGetWarmIPTargetState(t *testing.T) { - ctrl, mockAWS, mockK8S, _, mockNetwork, _ := setup(t) + ctrl, mockAWS, mockK8S, mockNetwork, _ := setup(t) defer ctrl.Finish() mockContext := &IPAMContext{ From 1eaee7192afa3f2eab4abd956be6a87964da284f Mon Sep 17 00:00:00 2001 From: Claes Mogren Date: Thu, 13 Jun 2019 11:31:41 -0700 Subject: [PATCH 5/6] Fix unit tests --- ipamd/ipamd_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipamd/ipamd_test.go b/ipamd/ipamd_test.go index 386079bc88..c1f8118613 100644 --- a/ipamd/ipamd_test.go +++ b/ipamd/ipamd_test.go @@ -237,7 +237,7 @@ func testIncreaseIPPool(t *testing.T, useENIConfig bool) { func TestTryAddIPToENI(t *testing.T) { _ = os.Unsetenv(envCustomNetworkCfg) - ctrl, mockAWS, mockK8S, _, mockNetwork, mockENIConfig := setup(t) + ctrl, mockAWS, mockK8S, mockNetwork, mockENIConfig := setup(t) defer ctrl.Finish() warmIpTarget := 3 From 7235b95e75efa0e74f4a34c31a593c3e1866cb32 Mon Sep 17 00:00:00 2001 From: Claes Mogren Date: Sat, 13 Jul 2019 14:34:50 -0700 Subject: [PATCH 6/6] Update rpc_handler_test.go Fix tests --- ipamd/rpc_handler_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ipamd/rpc_handler_test.go b/ipamd/rpc_handler_test.go index b75016094d..c96550ca1e 100644 --- a/ipamd/rpc_handler_test.go +++ b/ipamd/rpc_handler_test.go @@ -26,7 +26,7 @@ import ( ) func TestServer_AddNetwork(t *testing.T) { - ctrl, mockAWS, mockK8S, mockDocker, mockNetwork, _ := setup(t) + ctrl, mockAWS, mockK8S, mockNetwork, _ := setup(t) defer ctrl.Finish() mockContext := &IPAMContext{ @@ -36,7 +36,6 @@ func TestServer_AddNetwork(t *testing.T) { maxENI: 4, warmENITarget: 1, warmIPTarget: 3, - dockerClient: mockDocker, networkClient: mockNetwork, dataStore: datastore.NewDataStore(), }