diff --git a/.dockerignore b/.dockerignore index d224bb3eddf..b33de4ac0c1 100644 --- a/.dockerignore +++ b/.dockerignore @@ -4,4 +4,5 @@ cni-metrics-helper grpc-health-probe portmap loopback +bandwidth routed-eni-cni-plugin diff --git a/.gitignore b/.gitignore index 8ccf2dc3c38..55cd5b5abe5 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,4 @@ verify-network portmap grpc-health-probe cni-metrics-helper +coverage.txt diff --git a/Makefile b/Makefile index 77a648964f2..9c40b373924 100644 --- a/Makefile +++ b/Makefile @@ -74,9 +74,9 @@ ALLPKGS = $(shell go list ./...) # BINS is the set of built command executables. BINS = aws-k8s-agent aws-cni grpc-health-probe cni-metrics-helper # Plugin binaries -# Not copied: bandwidth bridge dhcp firewall flannel host-device host-local ipvlan macvlan ptp sbr static tuning vlan +# Not copied: bridge dhcp firewall flannel host-device host-local ipvlan macvlan ptp sbr static tuning vlan # For gnu tar, the full path in the tar file is required -PLUGIN_BINS = ./loopback ./portmap +PLUGIN_BINS = ./loopback ./portmap ./bandwidth # DOCKER_ARGS is extra arguments passed during container image build. DOCKER_ARGS = @@ -136,6 +136,7 @@ unit-test: go test -v -coverprofile=coverage.txt -covermode=atomic $(ALLPKGS) # Run unit tests with race detection (can only be run natively) +unit-test-race: export AWS_VPC_K8S_CNI_LOG_FILE=stdout unit-test-race: CGO_ENABLED=1 unit-test-race: GOARCH= unit-test-race: @@ -170,7 +171,7 @@ docker-metrics: -f scripts/dockerfiles/Dockerfile.metrics \ -t "$(METRICS_IMAGE_NAME)" \ . - @echo "Built Docker image \"amazon/cni-metrics-helper:$(VERSION)\"" + @echo "Built Docker image \"$(METRICS_IMAGE_NAME)\"" # Run metrics helper unit test suite (must be run natively). metrics-unit-test: CGO_ENABLED=1 diff --git a/cmd/cni-metrics-helper/README.md b/cmd/cni-metrics-helper/README.md index 6c221c96aea..bcb5760c6f4 100644 --- a/cmd/cni-metrics-helper/README.md +++ b/cmd/cni-metrics-helper/README.md @@ -11,7 +11,7 @@ By default ipamd will publish prometheus metrics on `:61678/metrics`. The following diagram shows how `cni-metrics-helper` works in a cluster: -![](../docs/images/cni-metrics-helper.png) +![](../../docs/images/cni-metrics-helper.png) ### Installing the cni-metrics-helper ``` diff --git a/cmd/routed-eni-cni-plugin/cni.go b/cmd/routed-eni-cni-plugin/cni.go index 97a7933dd40..6501df7d161 100644 --- a/cmd/routed-eni-cni-plugin/cni.go +++ b/cmd/routed-eni-cni-plugin/cni.go @@ -215,15 +215,18 @@ func add(args *skel.CmdArgs, cniTypes typeswrapper.CNITYPES, grpcClient grpcwrap return errors.Wrap(err, "add command: failed to setup network") } + interfaceIndex := 0 ips := []*current.IPConfig{ { - Version: "4", - Address: *addr, + Version: "4", + Address: *addr, + Interface: &interfaceIndex, }, } result := ¤t.Result{ - IPs: ips, + IPs: ips, + Interfaces: []*current.Interface{{Name: hostVethName}}, } return cniTypes.PrintResult(result, conf.CNIVersion) diff --git a/cmd/routed-eni-cni-plugin/cni_test.go b/cmd/routed-eni-cni-plugin/cni_test.go index 71962fbcdbd..dd282b4fb9c 100644 --- a/cmd/routed-eni-cni-plugin/cni_test.go +++ b/cmd/routed-eni-cni-plugin/cni_test.go @@ -69,12 +69,6 @@ func setup(t *testing.T) (*gomock.Controller, mock_driver.NewMockNetworkAPIs(ctrl) } -type rpcConn struct{} - -func (*rpcConn) Close() error { - return nil -} - func TestCmdAdd(t *testing.T) { ctrl, mocksTypes, mocksGRPC, mocksRPC, mocksNetwork := setup(t) defer ctrl.Finish() diff --git a/cmd/routed-eni-cni-plugin/driver/driver_test.go b/cmd/routed-eni-cni-plugin/driver/driver_test.go index b91064b78a7..ec378729eb1 100644 --- a/cmd/routed-eni-cni-plugin/driver/driver_test.go +++ b/cmd/routed-eni-cni-plugin/driver/driver_test.go @@ -99,28 +99,28 @@ func (m *testMocks) mockWithFailureAt(t *testing.T, failAt string) *createVethPa // veth pair if failAt == "link-add" { - call = m.netlink.EXPECT().LinkAdd(gomock.Any()).Return(errors.New("error on LinkAdd")) + m.netlink.EXPECT().LinkAdd(gomock.Any()).Return(errors.New("error on LinkAdd")) return mockContext } call = m.netlink.EXPECT().LinkAdd(gomock.Any()).Return(nil) //hostVeth if failAt == "link-by-name" { - call = m.netlink.EXPECT().LinkByName(gomock.Any()).Return(nil, errors.New("error on LinkByName host")).After(call) + m.netlink.EXPECT().LinkByName(gomock.Any()).Return(nil, errors.New("error on LinkByName host")).After(call) return mockContext } call = m.netlink.EXPECT().LinkByName(gomock.Any()).Return(mockHostVeth, nil).After(call) //host side setup if failAt == "link-setup" { - call = m.netlink.EXPECT().LinkSetUp(mockHostVeth).Return(errors.New("error on LinkSetup")).After(call) + m.netlink.EXPECT().LinkSetUp(mockHostVeth).Return(errors.New("error on LinkSetup")).After(call) return mockContext } call = m.netlink.EXPECT().LinkSetUp(mockHostVeth).Return(nil).After(call) //container side if failAt == "link-byname" { - call = m.netlink.EXPECT().LinkByName(gomock.Any()).Return(mockContVeth, errors.New("error on LinkByName container")).After(call) + m.netlink.EXPECT().LinkByName(gomock.Any()).Return(mockContVeth, errors.New("error on LinkByName container")).After(call) return mockContext } call = m.netlink.EXPECT().LinkByName(gomock.Any()).Return(mockContVeth, nil).After(call) @@ -131,20 +131,20 @@ func (m *testMocks) mockWithFailureAt(t *testing.T, failAt string) *createVethPa call = mockContVeth.EXPECT().Attrs().Return(mockLinkAttrs).After(call) if failAt == "route-replace" { - call = m.netlink.EXPECT().RouteReplace(gomock.Any()).Return(errors.New("error on RouteReplace")).After(call) + m.netlink.EXPECT().RouteReplace(gomock.Any()).Return(errors.New("error on RouteReplace")).After(call) return mockContext } call = m.netlink.EXPECT().RouteReplace(gomock.Any()).Return(nil).After(call) if failAt == "add-defaultroute" { - call = m.ip.EXPECT().AddDefaultRoute(gomock.Any(), mockContVeth).Return(errors.New("error on AddDefaultRoute")).After(call) + m.ip.EXPECT().AddDefaultRoute(gomock.Any(), mockContVeth).Return(errors.New("error on AddDefaultRoute")).After(call) return mockContext } call = m.ip.EXPECT().AddDefaultRoute(gomock.Any(), mockContVeth).Return(nil).After(call) // container addr if failAt == "addr-add" { - call = m.netlink.EXPECT().AddrAdd(mockContVeth, gomock.Any()).Return(errors.New("error on AddrAdd")).After(call) + m.netlink.EXPECT().AddrAdd(mockContVeth, gomock.Any()).Return(errors.New("error on AddrAdd")).After(call) return mockContext } call = m.netlink.EXPECT().AddrAdd(mockContVeth, gomock.Any()).Return(nil).After(call) @@ -154,7 +154,7 @@ func (m *testMocks) mockWithFailureAt(t *testing.T, failAt string) *createVethPa // hostVethMAC call = mockHostVeth.EXPECT().Attrs().Return(mockLinkAttrs).After(call) if failAt == "neigh-add" { - call = m.netlink.EXPECT().NeighAdd(gomock.Any()).Return(errors.New("error on NeighAdd")).After(call) + m.netlink.EXPECT().NeighAdd(gomock.Any()).Return(errors.New("error on NeighAdd")).After(call) return mockContext } call = m.netlink.EXPECT().NeighAdd(gomock.Any()).Return(nil).After(call) @@ -162,10 +162,10 @@ func (m *testMocks) mockWithFailureAt(t *testing.T, failAt string) *createVethPa call = m.netns.EXPECT().Fd().Return(uintptr(testFD)).After(call) // move it host namespace if failAt == "link-setns" { - call = m.netlink.EXPECT().LinkSetNsFd(mockHostVeth, testFD).Return(errors.New("error on LinkSetNsFd")).After(call) + m.netlink.EXPECT().LinkSetNsFd(mockHostVeth, testFD).Return(errors.New("error on LinkSetNsFd")).After(call) return mockContext } - call = m.netlink.EXPECT().LinkSetNsFd(mockHostVeth, testFD).Return(nil).After(call) + m.netlink.EXPECT().LinkSetNsFd(mockHostVeth, testFD).Return(nil).After(call) return mockContext } diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 80f56971bb7..a5739e567b2 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -190,5 +190,5 @@ go_goroutines 20 ## cni-metrics-helper -See the [cni-metrics-helper README](../cni-metrics-helper/README.md). +See the [cni-metrics-helper README](../cmd/cni-metrics-helper/README.md). diff --git a/pkg/awsutils/awsutils.go b/pkg/awsutils/awsutils.go index abc5fecfbc3..1c84ecc7187 100644 --- a/pkg/awsutils/awsutils.go +++ b/pkg/awsutils/awsutils.go @@ -142,8 +142,8 @@ type APIs interface { // GetVPCIPv4CIDR returns VPC's 1st CIDR GetVPCIPv4CIDR() string - // GetVPCIPv4CIDRs returns VPC's CIDRs - GetVPCIPv4CIDRs() []*string + // GetVPCIPv4CIDRs returns VPC's CIDRs from instance metadata + GetVPCIPv4CIDRs() []string // GetLocalIPv4 returns the primary IP address on the primary ENI interface GetLocalIPv4() string @@ -166,7 +166,6 @@ type EC2InstanceMetadataCache struct { // metadata info securityGroups StringSet subnetID string - cidrBlock string localIPv4 string instanceID string instanceType string @@ -228,17 +227,14 @@ func prometheusRegister() { //StringSet is a set of strings type StringSet struct { sync.RWMutex - data sets.String + data sets.String } -func (ss *StringSet) AWSStrings() []*string { +func (ss *StringSet) SortedList() []string { ss.RLock() defer ss.RUnlock() - var dataSlice []*string - for key, _ := range ss.data { - dataSlice = append(dataSlice, aws.String(key)) - } - return dataSlice + // sets.String.List() returns a sorted list + return ss.data.List() } func (ss *StringSet) Set(items []string) { @@ -247,16 +243,11 @@ func (ss *StringSet) Set(items []string) { ss.data = sets.NewString(items...) } -func (ss *StringSet) IsEmpty() bool { - if ss.data.Len() == 0 { - return true - } - return false -} - -func (ss *StringSet) Difference (other *StringSet) *StringSet { +func (ss *StringSet) Difference(other *StringSet) *StringSet { ss.RLock() + other.RLock() defer ss.RUnlock() + defer other.RUnlock() //example: s1 = {a1, a2, a3} s2 = {a1, a2, a4, a5} s1.Difference(s2) = {a3} s2.Difference(s1) = {a4, a5} return &StringSet{data: ss.data.Difference(other.data)} } @@ -280,9 +271,7 @@ func New() (*EC2InstanceMetadataCache, error) { cache.region = region log.Debugf("Discovered region: %s", cache.region) - sess, err := session.NewSession( - &aws.Config{Region: aws.String(cache.region), - MaxRetries: aws.Int(15)}) + sess, err := session.NewSession(&aws.Config{Region: aws.String(cache.region), MaxRetries: aws.Int(15)}) if err != nil { log.Errorf("Failed to initialize AWS SDK session %v", err) return nil, errors.Wrap(err, "instance metadata: failed to initialize AWS SDK session") @@ -373,19 +362,19 @@ func (cache *EC2InstanceMetadataCache) initWithEC2Metadata(ctx context.Context) } log.Debugf("Found vpc-ipv4-cidr-block: %s ", cache.vpcIPv4CIDR) - // initSGIDs retrieves security groups + // retrieve security groups err = cache.refreshSGIDs(mac) if err != nil { return err } - // initVPCIPv4CIDRs retrieves VPC IPv4 CIDR blocks + // retrieve VPC IPv4 CIDR blocks err = cache.refreshVPCIPv4CIDRs(mac) if err != nil { return err } - // refresh security groups and VPC CIDR blocks in the background + // Refresh security groups and VPC CIDR blocks in the background // Ignoring errors since we will retry in 30s go wait.Forever(func() { _ = cache.refreshSGIDs(mac) }, 30*time.Second) go wait.Forever(func() { _ = cache.refreshVPCIPv4CIDRs(mac) }, 30*time.Second) @@ -396,7 +385,6 @@ func (cache *EC2InstanceMetadataCache) initWithEC2Metadata(ctx context.Context) return nil default: } - return nil } @@ -409,25 +397,20 @@ func (cache *EC2InstanceMetadataCache) refreshSGIDs(mac string) error { return errors.Wrap(err, "get instance metadata: failed to retrieve security-group-ids") } - sgIDs := strings.Fields(metadataSGIDs) + sgIDs := strings.Fields(metadataSGIDs) newSGs := StringSet{} newSGs.Set(sgIDs) - addedSGs := newSGs.Difference(&cache.securityGroups) + addedSGs := newSGs.Difference(&cache.securityGroups) deletedSGs := cache.securityGroups.Difference(&newSGs) - if !addedSGs.IsEmpty() { - for _, sg := range addedSGs.AWSStrings() { - log.Infof("Found %s, added to ipamd cache", *sg) - } + for _, sg := range addedSGs.SortedList() { + log.Infof("Found %s, added to ipamd cache", sg) } - if !deletedSGs.IsEmpty() { - for _, sg := range deletedSGs.AWSStrings() { - log.Infof("Removed %s from ipamd cache", *sg) - } + for _, sg := range deletedSGs.SortedList() { + log.Infof("Removed %s from ipamd cache", sg) } cache.securityGroups.Set(sgIDs) - return nil } @@ -444,21 +427,16 @@ func (cache *EC2InstanceMetadataCache) refreshVPCIPv4CIDRs(mac string) error { newVpcIPv4CIDRs := StringSet{} newVpcIPv4CIDRs.Set(vpcIPv4CIDRs) - addedVpcIPv4CIDRs := newVpcIPv4CIDRs.Difference(&cache.securityGroups) - deletedVpcIPv4CIDRs := cache.securityGroups.Difference(&newVpcIPv4CIDRs) + addedVpcIPv4CIDRs := newVpcIPv4CIDRs.Difference(&cache.vpcIPv4CIDRs) + deletedVpcIPv4CIDRs := cache.vpcIPv4CIDRs.Difference(&newVpcIPv4CIDRs) - if !addedVpcIPv4CIDRs.IsEmpty() { - for _, vpcIPv4CIDR := range addedVpcIPv4CIDRs.AWSStrings() { - log.Infof("Found %s, added to ipamd cache", *vpcIPv4CIDR) - } + for _, vpcIPv4CIDR := range addedVpcIPv4CIDRs.SortedList() { + log.Infof("Found %s, added to ipamd cache", vpcIPv4CIDR) } - if !deletedVpcIPv4CIDRs.IsEmpty() { - for _, vpcIPv4CIDR := range deletedVpcIPv4CIDRs.AWSStrings() { - log.Infof("Removed %s from ipamd cache", *vpcIPv4CIDR) - } + for _, vpcIPv4CIDR := range deletedVpcIPv4CIDRs.SortedList() { + log.Infof("Removed %s from ipamd cache", vpcIPv4CIDR) } cache.vpcIPv4CIDRs.Set(vpcIPv4CIDRs) - return nil } @@ -761,7 +739,7 @@ func (cache *EC2InstanceMetadataCache) createENI(useCustomCfg bool, sg []*string eniDescription := eniDescriptionPrefix + cache.instanceID input := &ec2.CreateNetworkInterfaceInput{ Description: aws.String(eniDescription), - Groups: cache.securityGroups.AWSStrings(), + Groups: aws.StringSlice(cache.securityGroups.SortedList()), SubnetId: aws.String(cache.subnetID), } @@ -1367,8 +1345,8 @@ func (cache *EC2InstanceMetadataCache) GetVPCIPv4CIDR() string { } // GetVPCIPv4CIDRs returns VPC CIDRs -func (cache *EC2InstanceMetadataCache) GetVPCIPv4CIDRs() []*string { - return cache.vpcIPv4CIDRs.AWSStrings() +func (cache *EC2InstanceMetadataCache) GetVPCIPv4CIDRs() []string { + return cache.vpcIPv4CIDRs.SortedList() } // GetLocalIPv4 returns the primary IP address on the primary interface diff --git a/pkg/awsutils/awsutils_test.go b/pkg/awsutils/awsutils_test.go index 9aa526c5acd..daef69afb47 100644 --- a/pkg/awsutils/awsutils_test.go +++ b/pkg/awsutils/awsutils_test.go @@ -18,7 +18,6 @@ import ( "errors" "os" "sort" - "strings" "testing" "time" @@ -69,18 +68,12 @@ func setup(t *testing.T) (*gomock.Controller, } func TestInitWithEC2metadata(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 5 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() metadataVPCIPv4CIDRs := "192.168.0.0/16 100.66.0.0/1" - vpcIPv4CIDRs := strings.Fields(metadataVPCIPv4CIDRs) - var cidr []*string - for _, vpcCIDR := range vpcIPv4CIDRs { - log.Debugf("Found VPC CIDR: %s", vpcCIDR) - cidr = append(cidr, aws.String(vpcCIDR)) - } mockMetadata.EXPECT().GetMetadata(metadataAZ).Return(az, nil) mockMetadata.EXPECT().GetMetadata(metadataLocalIP).Return(localIP, nil) @@ -103,14 +96,14 @@ func TestInitWithEC2metadata(t *testing.T) { assert.Equal(t, localIP, ins.localIPv4) assert.Equal(t, ins.instanceID, instanceID) assert.Equal(t, ins.primaryENImac, primaryMAC) - assert.Equal(t, len(ins.securityGroups.data), 2) + assert.Equal(t, len(ins.securityGroups.SortedList()), 2) assert.Equal(t, subnetID, ins.subnetID) assert.Equal(t, vpcCIDR, ins.vpcIPv4CIDR) - assert.Equal(t, len(ins.vpcIPv4CIDRs.data), 2) + assert.Equal(t, len(ins.vpcIPv4CIDRs.SortedList()), 2) } func TestInitWithEC2metadataVPCcidrErr(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() @@ -133,7 +126,7 @@ func TestInitWithEC2metadataVPCcidrErr(t *testing.T) { } func TestInitWithEC2metadataSubnetErr(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() @@ -155,7 +148,7 @@ func TestInitWithEC2metadataSubnetErr(t *testing.T) { } func TestInitWithEC2metadataSGErr(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() @@ -179,7 +172,7 @@ func TestInitWithEC2metadataSGErr(t *testing.T) { } func TestInitWithEC2metadataENIErrs(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() @@ -197,7 +190,7 @@ func TestInitWithEC2metadataENIErrs(t *testing.T) { } func TestInitWithEC2metadataMACErr(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() @@ -214,7 +207,7 @@ func TestInitWithEC2metadataMACErr(t *testing.T) { } func TestInitWithEC2metadataLocalIPErr(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() @@ -228,7 +221,7 @@ func TestInitWithEC2metadataLocalIPErr(t *testing.T) { } func TestInitWithEC2metadataInstanceErr(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() @@ -243,7 +236,7 @@ func TestInitWithEC2metadataInstanceErr(t *testing.T) { } func TestInitWithEC2metadataAZErr(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, _ := setup(t) defer ctrl.Finish() @@ -441,7 +434,7 @@ func TestDescribeAllENIs(t *testing.T) { } func TestTagEni(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1 * time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) defer cancel() ctrl, mockMetadata, mockEC2 := setup(t) defer ctrl.Finish() diff --git a/pkg/awsutils/mocks/awsutils_mocks.go b/pkg/awsutils/mocks/awsutils_mocks.go index fc1169cadc7..134a6e5f89b 100644 --- a/pkg/awsutils/mocks/awsutils_mocks.go +++ b/pkg/awsutils/mocks/awsutils_mocks.go @@ -253,10 +253,10 @@ func (mr *MockAPIsMockRecorder) GetVPCIPv4CIDR() *gomock.Call { } // GetVPCIPv4CIDRs mocks base method -func (m *MockAPIs) GetVPCIPv4CIDRs() []*string { +func (m *MockAPIs) GetVPCIPv4CIDRs() []string { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetVPCIPv4CIDRs") - ret0, _ := ret[0].([]*string) + ret0, _ := ret[0].([]string) return ret0 } diff --git a/pkg/eniconfig/eniconfig_test.go b/pkg/eniconfig/eniconfig_test.go index 911856aa4af..6bc91ff0af6 100644 --- a/pkg/eniconfig/eniconfig_test.go +++ b/pkg/eniconfig/eniconfig_test.go @@ -13,6 +13,7 @@ package eniconfig import ( + "context" "fmt" "os" "testing" @@ -21,7 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/aws/amazon-vpc-cni-k8s/pkg/apis/crd/v1alpha1" - sdk "github.com/operator-framework/operator-sdk/pkg/sdk" + "github.com/operator-framework/operator-sdk/pkg/sdk" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" @@ -38,11 +39,10 @@ func updateENIConfig(hdlr sdk.Handler, name string, eniConfig v1alpha1.ENIConfig Deleted: toDelete, } - hdlr.Handle(nil, event) + _ = hdlr.Handle(context.TODO(), event) } func updateNodeAnnotation(hdlr sdk.Handler, nodeName string, configName string, toDelete bool) { - node := corev1.Node{ TypeMeta: metav1.TypeMeta{APIVersion: corev1.SchemeGroupVersion.String()}, ObjectMeta: metav1.ObjectMeta{ @@ -66,7 +66,7 @@ func updateNodeAnnotation(hdlr sdk.Handler, nodeName string, configName string, eniAnnotations[eniConfigAnnotationDef] = configName } accessor.SetAnnotations(eniAnnotations) - hdlr.Handle(nil, event) + _ = hdlr.Handle(context.TODO(), event) } func updateNodeLabel(hdlr sdk.Handler, nodeName string, configName string, toDelete bool) { @@ -94,11 +94,10 @@ func updateNodeLabel(hdlr sdk.Handler, nodeName string, configName string, toDel eniLabels[eniConfigLabelDef] = configName } accessor.SetLabels(eniLabels) - hdlr.Handle(nil, event) + _ = hdlr.Handle(context.TODO(), event) } func TestENIConfig(t *testing.T) { - testENIConfigController := NewENIConfigController() testHandler := NewHandler(testENIConfigController) @@ -136,7 +135,7 @@ func TestENIConfig(t *testing.T) { func TestNodeENIConfig(t *testing.T) { myNodeName := "testMyNodeWithAnnotation" myENIConfig := "testMyENIConfig" - os.Setenv("MY_NODE_NAME", myNodeName) + _ = os.Setenv("MY_NODE_NAME", myNodeName) testENIConfigController := NewENIConfigController() testHandler := NewHandler(testENIConfigController) @@ -177,7 +176,7 @@ func TestNodeENIConfig(t *testing.T) { func TestNodeENIConfigLabel(t *testing.T) { myNodeName := "testMyNodeWithLabel" myENIConfig := "testMyENIConfig" - os.Setenv("MY_NODE_NAME", myNodeName) + _ = os.Setenv("MY_NODE_NAME", myNodeName) testENIConfigController := NewENIConfigController() testHandler := NewHandler(testENIConfigController) @@ -216,25 +215,25 @@ func TestNodeENIConfigLabel(t *testing.T) { } func TestGetEniConfigAnnotationDefDefault(t *testing.T) { - os.Unsetenv(envEniConfigAnnotationDef) + _ = os.Unsetenv(envEniConfigAnnotationDef) eniConfigAnnotationDef := getEniConfigAnnotationDef() assert.Equal(t, eniConfigAnnotationDef, defaultEniConfigAnnotationDef) } func TestGetEniConfigAnnotationlDefCustom(t *testing.T) { - os.Setenv(envEniConfigAnnotationDef, "k8s.amazonaws.com/eniConfigCustom") + _ = os.Setenv(envEniConfigAnnotationDef, "k8s.amazonaws.com/eniConfigCustom") eniConfigAnnotationDef := getEniConfigAnnotationDef() assert.Equal(t, eniConfigAnnotationDef, "k8s.amazonaws.com/eniConfigCustom") } func TestGetEniConfigLabelDefDefault(t *testing.T) { - os.Unsetenv(envEniConfigLabelDef) + _ = os.Unsetenv(envEniConfigLabelDef) eniConfigLabelDef := getEniConfigLabelDef() assert.Equal(t, eniConfigLabelDef, defaultEniConfigLabelDef) } func TestGetEniConfigLabelDefCustom(t *testing.T) { - os.Setenv(envEniConfigLabelDef, "k8s.amazonaws.com/eniConfigCustom") + _ = os.Setenv(envEniConfigLabelDef, "k8s.amazonaws.com/eniConfigCustom") eniConfigLabelDef := getEniConfigLabelDef() assert.Equal(t, eniConfigLabelDef, "k8s.amazonaws.com/eniConfigCustom") } diff --git a/pkg/ipamd/datastore/data_store.go b/pkg/ipamd/datastore/data_store.go index 53726c34876..0d26622c1cb 100644 --- a/pkg/ipamd/datastore/data_store.go +++ b/pkg/ipamd/datastore/data_store.go @@ -26,10 +26,8 @@ import ( ) const ( - minLifeTime = 1 * time.Minute - // addressENICoolingPeriod is used to ensure ENI will NOT get freed back to EC2 control plane if one of - // its secondary IP addresses is used for a Pod within last addressENICoolingPeriod - addressENICoolingPeriod = 1 * time.Minute + // minENILifeTime is the shortest time before we consider deleting a newly created ENI + minENILifeTime = 1 * time.Minute // addressCoolingPeriod is used to ensure an IP not get assigned to a Pod if this IP is used by a different Pod // in addressCoolingPeriod @@ -64,9 +62,6 @@ const BackfillNetworkIface = "unknown" // ErrUnknownPod is an error when there is no pod in data store matching pod name, namespace, sandbox id var ErrUnknownPod = errors.New("datastore: unknown pod") -// ErrUnknownPodIP is an error where pod's IP address is not found in data store -var ErrUnknownPodIP = errors.New("datastore: pod using unknown IP address") - var ( enis = prometheus.NewGauge( prometheus.GaugeOpts{ @@ -574,7 +569,7 @@ func (ds *DataStore) getDeletableENI(warmIPTarget int, minimumIPTarget int) *ENI // IsTooYoung returns true if the ENI hasn't been around long enough to be deleted. func (e *ENI) isTooYoung() bool { - return time.Since(e.createTime) < minLifeTime + return time.Since(e.createTime) < minENILifeTime } // HasIPInCooling returns true if an IP address was unassigned recently. diff --git a/pkg/ipamd/ipamd.go b/pkg/ipamd/ipamd.go index a3104c27e42..32dfcc0fe19 100644 --- a/pkg/ipamd/ipamd.go +++ b/pkg/ipamd/ipamd.go @@ -47,8 +47,6 @@ const ( eniAttachTime = 10 * time.Second nodeIPPoolReconcileInterval = 60 * time.Second decreaseIPPoolInterval = 30 * time.Second - maxK8SRetries = 5 - retryK8SInterval = 3 * time.Second // ipReconcileCooldown is the amount of time that an IP address must wait until it can be added to the data store // during reconciliation after being discovered on the EC2 instance metadata. @@ -203,7 +201,7 @@ type UnmanagedENISet struct { func (u *UnmanagedENISet) isUnmanaged(eniID string) bool { val, ok := u.data[eniID] - return ok && val == true + return ok && val } func (u *UnmanagedENISet) reset() { @@ -340,18 +338,13 @@ func (c *IPAMContext) nodeInit() error { return err } - var pbVPCcidrs []string - vpcCIDRs := c.awsClient.GetVPCIPv4CIDRs() - - for _, cidr := range vpcCIDRs { - pbVPCcidrs = append(pbVPCcidrs, *cidr) - } _, vpcCIDR, err := net.ParseCIDR(c.awsClient.GetVPCIPv4CIDR()) if err != nil { return errors.Wrap(err, "ipamd init: failed to retrieve VPC CIDR") } + vpcCIDRs := c.awsClient.GetVPCIPv4CIDRs() primaryIP := net.ParseIP(c.awsClient.GetLocalIPv4()) err = c.networkClient.SetupHostNetwork(vpcCIDR, vpcCIDRs, c.awsClient.GetPrimaryENImac(), &primaryIP) if err != nil { @@ -398,7 +391,7 @@ func (c *IPAMContext) nodeInit() error { return err } - if err = c.configureIPRulesForPods(pbVPCcidrs); err != nil { + if err = c.configureIPRulesForPods(vpcCIDRs); err != nil { return err } @@ -410,8 +403,8 @@ func (c *IPAMContext) nodeInit() error { return err } - //Spawning checkAndUpdateRules go-routine - go wait.Forever(func() { pbVPCcidrs = c.checkVPCCIDRsAndRules(pbVPCcidrs)}, 30*time.Second) + // Spawning updateCIDRsRulesOnChange go-routine + go wait.Forever(func() { vpcCIDRs = c.updateCIDRsRulesOnChange(vpcCIDRs) }, 30*time.Second) return nil } @@ -436,17 +429,13 @@ func (c *IPAMContext) configureIPRulesForPods(pbVPCcidrs []string) error { return nil } -func (c *IPAMContext) checkVPCCIDRsAndRules(oldVPCCidrs []string) []string { - var pbVPCCIDRs []string +func (c *IPAMContext) updateCIDRsRulesOnChange(oldVPCCidrs []string) []string { newVPCCIDRs := c.awsClient.GetVPCIPv4CIDRs() - for _, cidr := range newVPCCIDRs { - pbVPCCIDRs = append(pbVPCCIDRs, *cidr) - } - if len(oldVPCCidrs) != len(pbVPCCIDRs) || !reflect.DeepEqual(oldVPCCidrs, pbVPCCIDRs) { - _ = c.configureIPRulesForPods(pbVPCCIDRs) + if len(oldVPCCidrs) != len(newVPCCIDRs) || !reflect.DeepEqual(oldVPCCidrs, newVPCCIDRs) { + _ = c.configureIPRulesForPods(newVPCCIDRs) } - return pbVPCCIDRs + return newVPCCIDRs } func (c *IPAMContext) updateIPStats(unmanaged int) { diff --git a/pkg/ipamd/ipamd_test.go b/pkg/ipamd/ipamd_test.go index d232488d847..47728dfc212 100644 --- a/pkg/ipamd/ipamd_test.go +++ b/pkg/ipamd/ipamd_test.go @@ -95,7 +95,7 @@ func TestNodeInit(t *testing.T) { eni1, eni2 := getDummyENIMetadata() - var cidrs []*string + var cidrs []string m.awsutils.EXPECT().GetENILimit().Return(4, nil) m.awsutils.EXPECT().GetENIipLimit().Return(14, nil) m.awsutils.EXPECT().GetIPv4sFromEC2(eni1.ENIID).AnyTimes().Return(eni1.IPv4Addresses, nil) @@ -287,15 +287,6 @@ func TestTryAddIPToENI(t *testing.T) { mockContext.dataStore = testDatastore() - podENIConfig := &v1alpha1.ENIConfigSpec{ - SecurityGroups: []string{"sg1-id", "sg2-id"}, - Subnet: "subnet1", - } - var sg []*string - for _, sgID := range podENIConfig.SecurityGroups { - sg = append(sg, aws.String(sgID)) - } - m.awsutils.EXPECT().AllocENI(false, nil, "").Return(secENIid, nil) m.awsutils.EXPECT().AllocIPAddresses(secENIid, warmIpTarget) m.awsutils.EXPECT().GetAttachedENIs().Return([]awsutils.ENIMetadata{ diff --git a/pkg/ipamd/rpc_handler.go b/pkg/ipamd/rpc_handler.go index 7882d0089cf..7d8a2cee478 100644 --- a/pkg/ipamd/rpc_handler.go +++ b/pkg/ipamd/rpc_handler.go @@ -53,10 +53,9 @@ func (s *server) AddNetwork(ctx context.Context, in *rpc.AddNetworkRequest) (*rp } addr, deviceNumber, err := s.ipamContext.dataStore.AssignPodIPv4Address(ipamKey) - var pbVPCcidrs []string - for _, cidr := range s.ipamContext.awsClient.GetVPCIPv4CIDRs() { - log.Debugf("VPC CIDR %s", *cidr) - pbVPCcidrs = append(pbVPCcidrs, *cidr) + pbVPCcidrs := s.ipamContext.awsClient.GetVPCIPv4CIDRs() + for _, cidr := range pbVPCcidrs { + log.Debugf("VPC CIDR %s", cidr) } useExternalSNAT := s.ipamContext.networkClient.UseExternalSNAT() diff --git a/pkg/ipamd/rpc_handler_test.go b/pkg/ipamd/rpc_handler_test.go index 7634062696b..fd04dfa32b0 100644 --- a/pkg/ipamd/rpc_handler_test.go +++ b/pkg/ipamd/rpc_handler_test.go @@ -18,7 +18,6 @@ import ( "testing" "github.com/aws/amazon-vpc-cni-k8s/pkg/ipamd/datastore" - "github.com/aws/aws-sdk-go/aws" pb "github.com/aws/amazon-vpc-cni-k8s/rpc" @@ -48,11 +47,11 @@ func TestServer_AddNetwork(t *testing.T) { IfName: "eni", } - vpcCIDRs := []*string{aws.String(vpcCIDR)} + vpcCIDRs := []string{vpcCIDR} testCases := []struct { name string useExternalSNAT bool - vpcCIDRs []*string + vpcCIDRs []string snatExclusionCIDRs []string }{ { @@ -80,11 +79,7 @@ func TestServer_AddNetwork(t *testing.T) { assert.Equal(t, tc.useExternalSNAT, addNetworkReply.UseExternalSNAT, tc.name) - var expectedCIDRs []string - for _, cidr := range tc.vpcCIDRs { - expectedCIDRs = append(expectedCIDRs, *cidr) - } - expectedCIDRs = append([]string{vpcCIDR}, tc.snatExclusionCIDRs...) + expectedCIDRs := append([]string{vpcCIDR}, tc.snatExclusionCIDRs...) assert.Equal(t, expectedCIDRs, addNetworkReply.VPCcidrs, tc.name) } } diff --git a/pkg/k8sapi/discovery.go b/pkg/k8sapi/discovery.go index d338b9ecac7..b4c7f312e69 100644 --- a/pkg/k8sapi/discovery.go +++ b/pkg/k8sapi/discovery.go @@ -12,8 +12,6 @@ import ( "github.com/aws/amazon-vpc-cni-k8s/pkg/utils/logger" - clientset "k8s.io/client-go/kubernetes" - "github.com/operator-framework/operator-sdk/pkg/k8sclient" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/fields" @@ -82,7 +80,7 @@ func NewController(clientset kubernetes.Interface) *Controller { } // CreateKubeClient creates a k8s client -func CreateKubeClient() (clientset.Interface, error) { +func CreateKubeClient() (kubernetes.Interface, error) { kubeClient := k8sclient.GetKubeClient() // Informers don't seem to do a good job logging error messages when it // can't reach the server, making debugging hard. This makes it easier to @@ -121,12 +119,6 @@ func (d *Controller) DiscoverCNIK8SPods() { d.DiscoverK8SPods(cache.NewListWatchFromClient(d.kubeClient.CoreV1().RESTClient(), "pods", metav1.NamespaceSystem, fields.Everything())) } -// DiscoverLocalK8SPods discovers local pods running on the node -func (d *Controller) DiscoverLocalK8SPods() { - // create the pod watcher - d.DiscoverK8SPods(cache.NewListWatchFromClient(d.kubeClient.CoreV1().RESTClient(), "pods", metav1.NamespaceAll, fields.OneTermEqualSelector("spec.nodeName", d.myNodeName))) -} - // DiscoverK8SPods takes a watcher and updates the Controller cache func (d *Controller) DiscoverK8SPods(podListWatcher *cache.ListWatch) { // create the workqueue diff --git a/pkg/networkutils/mocks/network_mocks.go b/pkg/networkutils/mocks/network_mocks.go index 4715077646a..b17dedc6e16 100644 --- a/pkg/networkutils/mocks/network_mocks.go +++ b/pkg/networkutils/mocks/network_mocks.go @@ -122,7 +122,7 @@ func (mr *MockNetworkAPIsMockRecorder) SetupENINetwork(arg0, arg1, arg2, arg3 in } // SetupHostNetwork mocks base method -func (m *MockNetworkAPIs) SetupHostNetwork(arg0 *net.IPNet, arg1 []*string, arg2 string, arg3 *net.IP) error { +func (m *MockNetworkAPIs) SetupHostNetwork(arg0 *net.IPNet, arg1 []string, arg2 string, arg3 *net.IP) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SetupHostNetwork", arg0, arg1, arg2, arg3) ret0, _ := ret[0].(error) diff --git a/pkg/networkutils/network.go b/pkg/networkutils/network.go index b711fc9f6d1..7685e833990 100644 --- a/pkg/networkutils/network.go +++ b/pkg/networkutils/network.go @@ -17,7 +17,6 @@ import ( "encoding/binary" "encoding/csv" "fmt" - "io" "math" "net" "os" @@ -43,9 +42,6 @@ import ( ) const ( - // 0 - 511 can be used other higher priorities - toPodRulePriority = 512 - // 513 - 1023, can be used priority lower than toPodRulePriority but higher than default nonVPC CIDR rule // 1024 is reserved for (ip rule not to table main) @@ -117,7 +113,7 @@ var log = logger.Get() // NetworkAPIs defines the host level and the ENI level network related operations type NetworkAPIs interface { // SetupNodeNetwork performs node level network configuration - SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []*string, primaryMAC string, primaryAddr *net.IP) error + SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []string, primaryMAC string, primaryAddr *net.IP) error // SetupENINetwork performs eni level network configuration SetupENINetwork(eniIP string, mac string, table int, subnetCIDR string) error UseExternalSNAT() bool @@ -134,7 +130,6 @@ type linuxNetwork struct { typeOfSNAT snatType nodePortSupportEnabled bool shouldConfigureRpFilter bool - connmark uint32 mtu int netLink netlinkwrapper.NetLink @@ -186,11 +181,6 @@ func New() NetworkAPIs { } } -type stringWriteCloser interface { - io.Closer - WriteString(s string) (int, error) -} - // find out the primary interface name func findPrimaryInterfaceName(primaryMAC string) (string, error) { log.Debugf("Trying to find primary interface that has mac : %s", primaryMAC) @@ -215,7 +205,7 @@ func findPrimaryInterfaceName(primaryMAC string) (string, error) { } // SetupHostNetwork performs node level network configuration -func (n *linuxNetwork) SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []*string, primaryMAC string, primaryAddr *net.IP) error { +func (n *linuxNetwork) SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []string, primaryMAC string, primaryAddr *net.IP) error { log.Info("Setting up host network... ") hostRule := n.netLink.NewRule() @@ -304,7 +294,7 @@ func (n *linuxNetwork) SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []*string, } var allCIDRs []snatCIDR for _, cidr := range vpcCIDRs { - allCIDRs = append(allCIDRs, snatCIDR{cidr: *cidr, isExclusion: false}) + allCIDRs = append(allCIDRs, snatCIDR{cidr: cidr, isExclusion: false}) } for _, cidr := range n.excludeSNATCIDRs { allCIDRs = append(allCIDRs, snatCIDR{cidr: cidr, isExclusion: true}) diff --git a/pkg/networkutils/network_test.go b/pkg/networkutils/network_test.go index 585cb88ec34..bc8c58b07f0 100644 --- a/pkg/networkutils/network_test.go +++ b/pkg/networkutils/network_test.go @@ -23,8 +23,6 @@ import ( "testing" "time" - "github.com/aws/aws-sdk-go/aws" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" @@ -165,7 +163,7 @@ func TestSetupHostNetworkNodePortDisabled(t *testing.T) { mockNetLink.EXPECT().NewRule().Return(&mainENIRule) mockNetLink.EXPECT().RuleDel(&mainENIRule) - var vpcCIDRs []*string + var vpcCIDRs []string err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP) assert.NoError(t, err) } @@ -290,7 +288,7 @@ func TestSetupHostNetworkNodePortEnabled(t *testing.T) { mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil) - var vpcCIDRs []*string + var vpcCIDRs []string err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP) assert.NoError(t, err) @@ -360,8 +358,7 @@ func TestSetupHostNetworkWithExcludeSNATCIDRs(t *testing.T) { mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil) - var vpcCIDRs []*string - vpcCIDRs = []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")} + vpcCIDRs := []string{"10.10.0.0/16", "10.11.0.0/16"} err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP) assert.NoError(t, err) assert.Equal(t, @@ -405,7 +402,6 @@ func TestSetupHostNetworkCleansUpStaleSNATRules(t *testing.T) { mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil) - vpcCIDRs := []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")} _ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-0", "!", "-d", "10.10.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAN", "-j", "AWS-SNAT-CHAIN-1") //AWS SNAT CHAN proves backwards compatibility _ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-1", "!", "-d", "10.11.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAIN", "-j", "AWS-SNAT-CHAIN-2") _ = mockIptables.Append("nat", "AWS-SNAT-CHAIN-2", "!", "-d", "10.12.0.0/16", "-m", "comment", "--comment", "AWS SNAT CHAIN EXCLUSION", "-j", "AWS-SNAT-CHAIN-3") @@ -414,6 +410,7 @@ func TestSetupHostNetworkCleansUpStaleSNATRules(t *testing.T) { _ = mockIptables.NewChain("nat", "AWS-SNAT-CHAIN-5") _ = mockIptables.Append("nat", "POSTROUTING", "-m", "comment", "--comment", "AWS SNAT CHAIN", "-j", "AWS-SNAT-CHAIN-0") + vpcCIDRs := []string{"10.10.0.0/16", "10.11.0.0/16"} err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP) assert.NoError(t, err) @@ -466,7 +463,7 @@ func TestSetupHostNetworkExcludedSNATCIDRsIdempotent(t *testing.T) { _ = mockIptables.Append("nat", "POSTROUTING", "-m", "comment", "--comment", "AWS SNAT CHAIN", "-j", "AWS-SNAT-CHAIN-0") // remove exclusions - vpcCIDRs := []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")} + vpcCIDRs := []string{"10.10.0.0/16", "10.11.0.0/16"} err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP) assert.NoError(t, err) @@ -510,8 +507,7 @@ func TestSetupHostNetworkMultipleCIDRs(t *testing.T) { mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil) - var vpcCIDRs []*string - vpcCIDRs = []*string{aws.String("10.10.0.0/16"), aws.String("10.11.0.0/16")} + vpcCIDRs := []string{"10.10.0.0/16", "10.11.0.0/16"} err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP) assert.NoError(t, err) } @@ -562,7 +558,7 @@ func TestSetupHostNetworkIgnoringRpFilterUpdate(t *testing.T) { } setupNetLinkMocks(ctrl, mockNetLink) - var vpcCIDRs []*string + var vpcCIDRs []string err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP) assert.NoError(t, err) } @@ -670,24 +666,3 @@ func (ipt *mockIptables) HasRandomFully() bool { // TODO: Work out how to write a test case for this return true } - -type mockFile struct { - closed bool - data string -} - -func (f *mockFile) WriteString(s string) (int, error) { - if f.closed { - panic("write call on closed file") - } - f.data += s - return len(s), nil -} - -func (f *mockFile) Close() error { - if f.closed { - panic("close call on closed file") - } - f.closed = true - return nil -} diff --git a/pkg/utils/logger/logger.go b/pkg/utils/logger/logger.go index fa602c49d62..34d50b71c05 100644 --- a/pkg/utils/logger/logger.go +++ b/pkg/utils/logger/logger.go @@ -13,14 +13,8 @@ package logger -import ( - "sync" -) - const pluginBinaryName = "aws-cni" -var once sync.Once - //Log is global variable so that log functions can be directly accessed var log Logger diff --git a/pkg/utils/logger/logger_test.go b/pkg/utils/logger/logger_test.go index dfcc6641fc6..5b12b4bceb1 100644 --- a/pkg/utils/logger/logger_test.go +++ b/pkg/utils/logger/logger_test.go @@ -38,15 +38,13 @@ func TestLogLevelReturnsOverriddenLevel(t *testing.T) { _ = os.Setenv(envLogLevel, "INFO") defer os.Unsetenv(envLogLevel) - var expectedLogLevel zapcore.Level - expectedLogLevel = zapcore.InfoLevel + expectedLogLevel := zapcore.InfoLevel inputLogLevel := getLogLevel() assert.Equal(t, expectedLogLevel, getZapLevel(inputLogLevel)) } func TestLogLevelReturnsDefaultLevelWhenEnvNotSet(t *testing.T) { - var expectedLogLevel zapcore.Level - expectedLogLevel = zapcore.DebugLevel + expectedLogLevel := zapcore.DebugLevel inputLogLevel := getLogLevel() assert.Equal(t, expectedLogLevel, getZapLevel(inputLogLevel)) } diff --git a/scripts/dockerfiles/Dockerfile.init b/scripts/dockerfiles/Dockerfile.init index f922e34bf92..1b9d77716a9 100644 --- a/scripts/dockerfiles/Dockerfile.init +++ b/scripts/dockerfiles/Dockerfile.init @@ -24,6 +24,7 @@ WORKDIR /init COPY --from=builder \ /go/src/github.com/aws/amazon-vpc-cni-k8s/loopback \ /go/src/github.com/aws/amazon-vpc-cni-k8s/portmap \ + /go/src/github.com/aws/amazon-vpc-cni-k8s/bandwidth \ /go/src/github.com/aws/amazon-vpc-cni-k8s/aws-cni-support.sh \ /go/src/github.com/aws/amazon-vpc-cni-k8s/scripts/init.sh /init/ diff --git a/scripts/dockerfiles/Dockerfile.release b/scripts/dockerfiles/Dockerfile.release index 94b3387a94a..7a6ec5c993e 100644 --- a/scripts/dockerfiles/Dockerfile.release +++ b/scripts/dockerfiles/Dockerfile.release @@ -28,7 +28,9 @@ WORKDIR /app COPY --from=builder /go/src/github.com/aws/amazon-vpc-cni-k8s/aws-cni \ /go/src/github.com/aws/amazon-vpc-cni-k8s/misc/10-aws.conflist \ + /go/src/github.com/aws/amazon-vpc-cni-k8s/loopback \ /go/src/github.com/aws/amazon-vpc-cni-k8s/portmap \ + /go/src/github.com/aws/amazon-vpc-cni-k8s/bandwidth \ /go/src/github.com/aws/amazon-vpc-cni-k8s/aws-cni-support.sh \ /go/src/github.com/aws/amazon-vpc-cni-k8s/aws-k8s-agent \ /go/src/github.com/aws/amazon-vpc-cni-k8s/grpc-health-probe \ diff --git a/scripts/entrypoint.sh b/scripts/entrypoint.sh index 2789bcb5e5e..488aa5ac337 100755 --- a/scripts/entrypoint.sh +++ b/scripts/entrypoint.sh @@ -14,7 +14,7 @@ # As mentioned above, Kubelet considers a CNI plugin "ready" when it sees the # binary and configuration file for the plugin in a well-known directory. For # the AWS VPC CNI plugin binary, we only want to copy the CNI plugin binary -# into that well-known directory AFTER we have succeessfully started the IPAM +# into that well-known directory AFTER we have successfully started the IPAM # daemon and know that it can connect to Kubernetes and the local EC2 metadata # service. This is why this entrypoint script exists; we start the IPAM daemon # and wait until we know it is up and running successfully before copying the @@ -26,13 +26,22 @@ set -eu # turn on bash's job control set -m +log_in_json() +{ + FILENAME="${0##*/}" + LOGTYPE=$1 + MSG=$2 + TIMESTAMP=$(date +%FT%T.%3NZ) + printf '{"level":"%s","ts":"%s","caller":"%s","msg":"%s"}\n' "$LOGTYPE" "$TIMESTAMP" "$FILENAME" "$MSG" +} + # Check for all the required binaries before we go forward if [ ! -f aws-k8s-agent ]; then - echo "Required aws-k8s-agent executable not found." + log_in_json error "Required aws-k8s-agent executable not found." exit 1 fi if [ ! -f grpc-health-probe ]; then - echo "Required grpc-health-probe executable not found." + log_in_json error "Required grpc-health-probe executable not found." exit 1 fi @@ -61,35 +70,28 @@ wait_for_ipam() { # If there is no init container, copy the required files if [[ "$AWS_VPC_K8S_CNI_CONFIGURE_RPFILTER" != "false" ]]; then # Copy files - echo "Copying CNI plugin binaries ... " - PLUGIN_BINS="portmap aws-cni-support.sh" + log_in_json info "Copying CNI plugin binaries ... " + PLUGIN_BINS="loopback portmap bandwidth aws-cni-support.sh" for b in $PLUGIN_BINS; do - # If the file exist, delete it first - if [[ -f "$HOST_CNI_BIN_PATH/$b" ]]; then - rm "$HOST_CNI_BIN_PATH/$b" - fi - cp "$b" "$HOST_CNI_BIN_PATH" + # Install the binary + install "$b" "$HOST_CNI_BIN_PATH" done fi -echo -n "Starting IPAM daemon in the background ... " +log_in_json info "Starting IPAM daemon in the background ... " ./aws-k8s-agent | tee -i "$AGENT_LOG_PATH" 2>&1 & -echo "ok." -echo -n "Checking for IPAM connectivity ... " +log_in_json info "Checking for IPAM connectivity ... " if ! wait_for_ipam; then - echo " failed." - echo "Timed out waiting for IPAM daemon to start:" + log_in_json error "Timed out waiting for IPAM daemon to start:" cat "$AGENT_LOG_PATH" >&2 exit 1 fi -echo "ok." - -echo -n "Copying CNI plugin binary and config file ... " +log_in_json info "Copying CNI plugin binary and config file ... " -cp aws-cni "$HOST_CNI_BIN_PATH" +install aws-cni "$HOST_CNI_BIN_PATH" sed -i s~__VETHPREFIX__~"${AWS_VPC_K8S_CNI_VETHPREFIX}"~g 10-aws.conflist sed -i s~__MTU__~"${AWS_VPC_ENI_MTU}"~g 10-aws.conflist @@ -97,12 +99,12 @@ sed -i s~__PLUGINLOGFILE__~"${AWS_VPC_K8S_PLUGIN_LOG_FILE}"~g 10-aws.conflist sed -i s~__PLUGINLOGLEVEL__~"${AWS_VPC_K8S_PLUGIN_LOG_LEVEL}"~g 10-aws.conflist cp 10-aws.conflist "$HOST_CNI_CONFDIR_PATH" -echo "ok." +log_in_json info "Successfully copied CNI plugin binary and config file." if [[ -f "$HOST_CNI_CONFDIR_PATH/aws.conf" ]]; then rm "$HOST_CNI_CONFDIR_PATH/aws.conf" fi # Bring the aws-k8s-agent process back into the foreground -echo "Foregrounding IPAM daemon ... " -fg %1 >/dev/null 2>&1 || { echo "failed (process terminated)" && cat "$AGENT_LOG_PATH" && exit 1; } +log_in_json info "Foregrounding IPAM daemon ..." +fg %1 >/dev/null 2>&1 || { log_in_json error "failed (process terminated)" && cat "$AGENT_LOG_PATH" && exit 1; } diff --git a/scripts/init.sh b/scripts/init.sh index 8ce1d079f1f..b5c038595d9 100755 --- a/scripts/init.sh +++ b/scripts/init.sh @@ -2,7 +2,7 @@ set -euxo pipefail -PLUGIN_BINS="loopback portmap aws-cni-support.sh" +PLUGIN_BINS="loopback portmap bandwidth aws-cni-support.sh" for b in $PLUGIN_BINS; do if [ ! -f "$b" ]; then @@ -17,11 +17,8 @@ HOST_CNI_BIN_PATH=${HOST_CNI_BIN_PATH:-"/host/opt/cni/bin"} echo "Copying CNI plugin binaries ... " for b in $PLUGIN_BINS; do - # If the file exist, delete it first - if [[ -f "$HOST_CNI_BIN_PATH/$b" ]]; then - rm "$HOST_CNI_BIN_PATH/$b" - fi - cp "$b" "$HOST_CNI_BIN_PATH" + # Install the binary + install "$b" "$HOST_CNI_BIN_PATH" done # Configure rp_filter diff --git a/scripts/lib/aws.sh b/scripts/lib/aws.sh index bce4f9fb482..cb74a109251 100644 --- a/scripts/lib/aws.sh +++ b/scripts/lib/aws.sh @@ -15,7 +15,7 @@ ensure_ecr_repo() { } ensure_aws_k8s_tester() { - TESTER_RELEASE=${TESTER_RELEASE:-v1.3.9} + TESTER_RELEASE=${TESTER_RELEASE:-v1.4.0} TESTER_DOWNLOAD_URL=https://github.com/aws/aws-k8s-tester/releases/download/$TESTER_RELEASE/aws-k8s-tester-$TESTER_RELEASE-$OS-$ARCH # Download aws-k8s-tester if not yet diff --git a/scripts/run-integration-tests.sh b/scripts/run-integration-tests.sh index 71a437d0c02..54e322b8215 100755 --- a/scripts/run-integration-tests.sh +++ b/scripts/run-integration-tests.sh @@ -51,6 +51,7 @@ TEST_CONFIG_DIR="$TEST_DIR/config" : "${CLUSTER_ID:=$RANDOM}" CLUSTER_NAME=cni-test-$CLUSTER_ID TEST_CLUSTER_DIR=/tmp/cni-test/cluster-$CLUSTER_NAME +CLUSTER_MANAGE_LOG_PATH=$TEST_CLUSTER_DIR/cluster-manage.log : "${CLUSTER_CONFIG:=${TEST_CLUSTER_DIR}/${CLUSTER_NAME}.yaml}" : "${KUBECONFIG_PATH:=${TEST_CLUSTER_DIR}/kubeconfig}" @@ -104,6 +105,7 @@ ensure_ecr_repo "$AWS_ACCOUNT_ID" "$AWS_INIT_ECR_REPO_NAME" # image and push it to the Docker repository if [[ $(docker images -q "$IMAGE_NAME:$TEST_IMAGE_VERSION" 2> /dev/null) ]]; then echo "CNI image $IMAGE_NAME:$TEST_IMAGE_VERSION already exists in repository. Skipping image build..." + DOCKER_BUILD_DURATION=0 else echo "CNI image $IMAGE_NAME:$TEST_IMAGE_VERSION does not exist in repository." if [[ $TEST_IMAGE_VERSION != "$LOCAL_GIT_VERSION" ]]; then @@ -206,7 +208,7 @@ if [[ $TEST_PASS -eq 0 && "$RUN_CONFORMANCE" == true ]]; then go install github.com/onsi/ginkgo/ginkgo wget -qO- https://dl.k8s.io/v$K8S_VERSION/kubernetes-test.tar.gz | tar -zxvf - --strip-components=4 -C /tmp kubernetes/platforms/linux/amd64/e2e.test - ginkgo -p --focus="Conformance" --failFast --flakeAttempts 2 \ + $GOPATH/bin/ginkgo -p --focus="Conformance" --failFast --flakeAttempts 2 \ --skip="(should support remote command execution over websockets)|(should support retrieving logs from the container over websockets)|\[Slow\]|\[Serial\]" /tmp/e2e.test -- --kubeconfig=$KUBECONFIG /tmp/e2e.test --ginkgo.focus="\[Serial\].*Conformance" --kubeconfig=$KUBECONFIG --ginkgo.failFast --ginkgo.flakeAttempts 2 \