Repository: kubernetes-sigs/aws-ebs-csi-driver
Pull request #527 (2020-06-25 06:08:21)

Add arm support
**Is this a bug fix or adding new feature?**
Fixes: #521

**What is this PR about? / Why do we need it?**
Add arm support


k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/527#" title="Author self-approved">leakingtapan</a>*

The full list of commands accepted by this bot can be found [here](?repo=kubernetes-sigs%2Faws-ebs-csi-driver).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/OWNERS)~~ [leakingtapan]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
coveralls: ## Pull Request Test Coverage Report for [Build 1123](https://coveralls.io/builds/31677205)

* **0** of **0**   changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **78.871%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/31677205/badge)](https://coveralls.io/builds/31677205) |
| :-- | --: |
| Change from base [Build 1111](https://coveralls.io/builds/31005643): |  0.0% |
| Covered Lines: | 1411 |
| Relevant Lines: | 1789 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/.github/workflows/container-image.yaml b/.github/workflows/container-image.yaml
index ec4657b4..529784f5 100644
--- a/.github/workflows/container-image.yaml
+++ b/.github/workflows/container-image.yaml
@@ -8,9 +8,18 @@ jobs:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v1
+    - name: Set up Docker Buildx
+      id: buildx
+      uses: crazy-max/ghaction-docker-buildx@v3
+      with:
+        buildx-version: latest
+        qemu-version: latest
     - name: Build container image
       run: |
-        docker build -t aws-ebs-csi-driver .
+        docker buildx build \
+          -t aws-ebs-csi-driver \
+          --platform=linux/arm64,linux/amd64 \
+          --output="type=image,push=false" .
     - name: Push to Github registry
       run: |
         USER=$(echo $GITHUB_REPOSITORY | cut -d'/' -f1)
@@ -22,6 +31,7 @@ jobs:
           TAG=$BRANCH
         fi
         docker login docker.pkg.github.com -u $USER -p ${{ secrets.REGISTRY_TOKEN }}
+        docker build -t aws-ebs-csi-driver .
         docker tag aws-ebs-csi-driver docker.pkg.github.com/$GITHUB_REPOSITORY/$IMAGE:$TAG
         docker push docker.pkg.github.com/$GITHUB_REPOSITORY/$IMAGE:$TAG
     - name: Push to Dockerhub registry
@@ -34,5 +44,8 @@ jobs:
           TAG=$BRANCH
         fi
         docker login -u ${{ secrets.DOCKERHUB_USER }} -p ${{ secrets.DOCKERHUB_TOKEN }}
-        docker tag aws-ebs-csi-driver $REPO:$TAG
-        docker push $REPO:$TAG
+        docker buildx build \
+          -t $REPO:$TAG \
+          --platform=linux/arm64,linux/amd64 \
+          --output="type=image,push=true" .
+
diff --git a/aws-ebs-csi-driver/templates/controller.yaml b/aws-ebs-csi-driver/templates/controller.yaml
index 1e7b68dc..041b946e 100644
--- a/aws-ebs-csi-driver/templates/controller.yaml
+++ b/aws-ebs-csi-driver/templates/controller.yaml
@@ -23,7 +23,6 @@ spec:
     spec:
       nodeSelector:
         kubernetes.io/os: linux
-        kubernetes.io/arch: amd64
         {{- with .Values.nodeSelector }}
 {{ toYaml . | indent 8 }}
         {{- end }}
diff --git a/aws-ebs-csi-driver/templates/node.yaml b/aws-ebs-csi-driver/templates/node.yaml
index 8eea8a0f..d6eadbcf 100644
--- a/aws-ebs-csi-driver/templates/node.yaml
+++ b/aws-ebs-csi-driver/templates/node.yaml
@@ -31,7 +31,6 @@ spec:
                 - fargate
       nodeSelector:
         kubernetes.io/os: linux
-        kubernetes.io/arch: amd64
       hostNetwork: true
       priorityClassName: system-node-critical
       tolerations:
diff --git a/cmd/main.go b/cmd/main.go
index 855bbb63..6bf98fba 100644
--- a/cmd/main.go
+++ b/cmd/main.go
@@ -32,7 +32,6 @@ func main() {
 		driver.WithEndpoint(options.ServerOptions.Endpoint),
 		driver.WithExtraVolumeTags(options.ControllerOptions.ExtraVolumeTags),
 		driver.WithMode(options.DriverMode),
-		driver.WithVolumeAttachLimit(options.NodeOptions.VolumeAttachLimit),
 	)
 	if err != nil {
 		klog.Fatalln(err)
diff --git a/cmd/options/node_options.go b/cmd/options/node_options.go
index 91190f16..0f696e03 100644
--- a/cmd/options/node_options.go
+++ b/cmd/options/node_options.go
@@ -21,18 +21,6 @@ import (
 )
 
 // NodeOptions contains options and configuration settings for the node service.
-type NodeOptions struct {
-	// VolumeAttachLimit specifies the value that shall be reported as "maximum number of attachable volumes"
-	// in CSINode objects. It is similar to https://kubernetes.io/docs/concepts/storage/storage-limits/#custom-limits
-	// which allowed administrators to specify custom volume limits by configuring the kube-scheduler. Also, each AWS
-	// machine type has different volume limits. By default, the EBS CSI driver parses the machine type name and then
-	// decides the volume limit. However, this is only a rough approximation and not good enough in most cases.
-	// Specifying the volume attach limit via command line is the alternative until a more sophisticated solution presents
-	// itself (dynamically discovering the maximum number of attachable volume per EC2 machine type, see also
-	// https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/347).
-	VolumeAttachLimit int64
-}
+type NodeOptions struct{}
 
-func (o *NodeOptions) AddFlags(fs *flag.FlagSet) {
-	fs.Int64Var(&o.VolumeAttachLimit, "volume-attach-limit", -1, "Value value for the maximum number of volumes attachable for all nodes. If the flag is not specified then the value is approximated from the instance type.")
-}
+func (s *NodeOptions) AddFlags(fs *flag.FlagSet) {}
diff --git a/deploy/kubernetes/base/controller.yaml b/deploy/kubernetes/base/controller.yaml
index 0769a1a6..d619e817 100644
--- a/deploy/kubernetes/base/controller.yaml
+++ b/deploy/kubernetes/base/controller.yaml
@@ -22,7 +22,6 @@ spec:
     spec:
       nodeSelector:
         kubernetes.io/os: linux
-        kubernetes.io/arch: amd64
       serviceAccountName: ebs-csi-controller-sa
       priorityClassName: system-cluster-critical
       tolerations:
diff --git a/deploy/kubernetes/base/node.yaml b/deploy/kubernetes/base/node.yaml
index 6c28a04b..22781dee 100644
--- a/deploy/kubernetes/base/node.yaml
+++ b/deploy/kubernetes/base/node.yaml
@@ -30,7 +30,6 @@ spec:
                 - fargate
       nodeSelector:
         kubernetes.io/os: linux
-        kubernetes.io/arch: amd64
       hostNetwork: true
       priorityClassName: system-node-critical
       tolerations:
@@ -43,7 +42,6 @@ spec:
           args:
             - node
             - --endpoint=$(CSI_ENDPOINT)
-          # - --volume-attach-limit=42
             - --logtostderr
             - --v=5
           env:
diff --git a/docs/design.md b/docs/design.md
index 96c3ffae..d3490942 100644
--- a/docs/design.md
+++ b/docs/design.md
@@ -235,15 +235,3 @@ Example 2: `AWS_REGION=us-west-1 /bin/aws-ebs-csi-driver controller --extra-volu
 
 - `node`: This will only start the node service of the CSI driver.\
 Example: `/bin/aws-ebs-csi-driver node --endpoint=unix://...`
-
-## Custom volume limits
-
-For the Kubernetes in-tree volume provisioners (including the `kubernetes.io/aws-ebs` provisioner) it was possible for administrators to provide a custom volume limit overwrite (see https://kubernetes.io/docs/concepts/storage/storage-limits/#custom-limits).
-This solution is not working with CSI any longer.
-As part of [#347](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/347) we discuss how we can implement a sophisticated computation of the volume attach limit per node (e.g., based on the used machine types and already attached network interfaces).
-However, it turns out that such optimal implementation is not easily achievable.
-Each AWS machine type has different volume limits.
-Today, the EBS CSI driver parses the machine type name and then decides the volume limit.
-Unfortunately, this is only a rough approximation and not good enough in most cases.
-In order to allow existing clusters that are leveraging/relying on this feature to migrate to CSI, the EBS CSI driver is supporting the `--volume-attach-limit` flag.
-Specifying the volume attach limit via command line is the alternative until a more sophisticated solution presents itself (dynamically discovering the maximum number of attachable volume per EC2 machine type).
diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go
index 60305d5b..e54df9b9 100644
--- a/pkg/driver/driver.go
+++ b/pkg/driver/driver.go
@@ -53,10 +53,9 @@ type Driver struct {
 }
 
 type DriverOptions struct {
-	endpoint          string
-	extraVolumeTags   map[string]string
-	mode              Mode
-	volumeAttachLimit int64
+	endpoint        string
+	extraVolumeTags map[string]string
+	mode            Mode
 }
 
 func NewDriver(options ...func(*DriverOptions)) (*Driver, error) {
@@ -82,10 +81,10 @@ func NewDriver(options ...func(*DriverOptions)) (*Driver, error) {
 	case ControllerMode:
 		driver.controllerService = newControllerService(&driverOptions)
 	case NodeMode:
-		driver.nodeService = newNodeService(&driverOptions)
+		driver.nodeService = newNodeService()
 	case AllMode:
 		driver.controllerService = newControllerService(&driverOptions)
-		driver.nodeService = newNodeService(&driverOptions)
+		driver.nodeService = newNodeService()
 	default:
 		return nil, fmt.Errorf("unknown mode: %s", driverOptions.mode)
 	}
@@ -156,9 +155,3 @@ func WithMode(mode Mode) func(*DriverOptions) {
 		o.mode = mode
 	}
 }
-
-func WithVolumeAttachLimit(volumeAttachLimit int64) func(*DriverOptions) {
-	return func(o *DriverOptions) {
-		o.volumeAttachLimit = volumeAttachLimit
-	}
-}
diff --git a/pkg/driver/node.go b/pkg/driver/node.go
index cea55ad7..649e7c94 100644
--- a/pkg/driver/node.go
+++ b/pkg/driver/node.go
@@ -70,25 +70,23 @@ var (
 
 // nodeService represents the node service of CSI driver
 type nodeService struct {
-	metadata      cloud.MetadataService
-	mounter       Mounter
-	inFlight      *internal.InFlight
-	driverOptions *DriverOptions
+	metadata cloud.MetadataService
+	mounter  Mounter
+	inFlight *internal.InFlight
 }
 
 // newNodeService creates a new node service
 // it panics if failed to create the service
-func newNodeService(driverOptions *DriverOptions) nodeService {
+func newNodeService() nodeService {
 	metadata, err := cloud.NewMetadata()
 	if err != nil {
 		panic(err)
 	}
 
 	return nodeService{
-		metadata:      metadata,
-		mounter:       newNodeMounter(),
-		inFlight:      internal.NewInFlight(),
-		driverOptions: driverOptions,
+		metadata: metadata,
+		mounter:  newNodeMounter(),
+		inFlight: internal.NewInFlight(),
 	}
 }
 
@@ -516,9 +514,6 @@ func findNvmeVolume(findName string) (device string, err error) {
 
 // getVolumesLimit returns the limit of volumes that the node supports
 func (d *nodeService) getVolumesLimit() int64 {
-	if d.driverOptions.volumeAttachLimit >= 0 {
-		return d.driverOptions.volumeAttachLimit
-	}
 	ebsNitroInstanceTypeRegex := "^[cmr]5.*|t3|z1d"
 	instanceType := d.metadata.GetInstanceType()
 	if ok, _ := regexp.MatchString(ebsNitroInstanceTypeRegex, instanceType); ok {


Test patch:
diff --git a/cmd/options/node_options_test.go b/cmd/options/node_options_test.go
index dc4191a5..483d7cac 100644
--- a/cmd/options/node_options_test.go
+++ b/cmd/options/node_options_test.go
@@ -27,11 +27,6 @@ func TestNodeOptions(t *testing.T) {
 		flag  string
 		found bool
 	}{
-		{
-			name:  "lookup desired flag",
-			flag:  "volume-attach-limit",
-			found: true,
-		},
 		{
 			name:  "fail for non-desired flag",
 			flag:  "some-flag",
diff --git a/cmd/options_test.go b/cmd/options_test.go
index ef4bffe1..cd58e19d 100644
--- a/cmd/options_test.go
+++ b/cmd/options_test.go
@@ -20,7 +20,6 @@ import (
 	"flag"
 	"os"
 	"reflect"
-	"strconv"
 	"testing"
 
 	"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver"
@@ -46,9 +45,6 @@ func TestGetOptions(t *testing.T) {
 			extraVolumeTagKey: extraVolumeTagValue,
 		}
 
-		VolumeAttachLimitFlagName := "volume-attach-limit"
-		var VolumeAttachLimit int64 = 42
-
 		args := append([]string{
 			"aws-ebs-csi-driver",
 		}, additionalArgs...)
@@ -59,9 +55,6 @@ func TestGetOptions(t *testing.T) {
 		if withControllerOptions {
 			args = append(args, "-"+extraVolumeTagsFlagName+"="+extraVolumeTagKey+"="+extraVolumeTagValue)
 		}
-		if withNodeOptions {
-			args = append(args, "-"+VolumeAttachLimitFlagName+"="+strconv.FormatInt(VolumeAttachLimit, 10))
-		}
 
 		oldArgs := os.Args
 		defer func() { os.Args = oldArgs }()
@@ -89,16 +82,6 @@ func TestGetOptions(t *testing.T) {
 			}
 		}
 
-		if withNodeOptions {
-			VolumeAttachLimitFlag := flagSet.Lookup(VolumeAttachLimitFlagName)
-			if VolumeAttachLimitFlag == nil {
-				t.Fatalf("expected %q flag to be added but it is not", VolumeAttachLimitFlagName)
-			}
-			if options.NodeOptions.VolumeAttachLimit != VolumeAttachLimit {
-				t.Fatalf("expected VolumeAttachLimit to be %d but it is %d", VolumeAttachLimit, options.NodeOptions.VolumeAttachLimit)
-			}
-		}
-
 		return options
 	}
 
diff --git a/pkg/driver/driver_test.go b/pkg/driver/driver_test.go
deleted file mode 100644
index 70f86002..00000000
--- a/pkg/driver/driver_test.go
+++ /dev/null
@@ -1,58 +0,0 @@
-/*
-Copyright 2020 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License 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 driver
-
-import (
-	"reflect"
-	"testing"
-)
-
-func TestWithEndpoint(t *testing.T) {
-	value := "endpoint"
-	options := &DriverOptions{}
-	WithEndpoint(value)(options)
-	if options.endpoint != value {
-		t.Fatalf("expected endpoint option got set to %q but is set to %q", value, options.endpoint)
-	}
-}
-
-func TestWithExtraVolumeTags(t *testing.T) {
-	value := map[string]string{"foo": "bar"}
-	options := &DriverOptions{}
-	WithExtraVolumeTags(value)(options)
-	if !reflect.DeepEqual(options.extraVolumeTags, value) {
-		t.Fatalf("expected extraVolumeTags option got set to %+v but is set to %+v", value, options.extraVolumeTags)
-	}
-}
-
-func TestWithMode(t *testing.T) {
-	value := Mode("mode")
-	options := &DriverOptions{}
-	WithMode(value)(options)
-	if options.mode != value {
-		t.Fatalf("expected mode option got set to %q but is set to %q", value, options.mode)
-	}
-}
-
-func TestWithVolumeAttachLimit(t *testing.T) {
-	var value int64 = 42
-	options := &DriverOptions{}
-	WithVolumeAttachLimit(value)(options)
-	if options.volumeAttachLimit != value {
-		t.Fatalf("expected volumeAttachLimit option got set to %d but is set to %d", value, options.volumeAttachLimit)
-	}
-}
diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go
index 8592b2db..36714e4e 100644
--- a/pkg/driver/node_test.go
+++ b/pkg/driver/node_test.go
@@ -1243,44 +1243,25 @@ func TestNodeGetCapabilities(t *testing.T) {
 
 func TestNodeGetInfo(t *testing.T) {
 	testCases := []struct {
-		name              string
-		instanceID        string
-		instanceType      string
-		availabilityZone  string
-		volumeAttachLimit int64
-		expMaxVolumes     int64
+		name             string
+		instanceID       string
+		instanceType     string
+		availabilityZone string
+		expMaxVolumes    int64
 	}{
 		{
-			name:              "success normal",
-			instanceID:        "i-123456789abcdef01",
-			instanceType:      "t2.medium",
-			availabilityZone:  "us-west-2b",
-			volumeAttachLimit: -1,
-			expMaxVolumes:     39,
+			name:             "success normal",
+			instanceID:       "i-123456789abcdef01",
+			instanceType:     "t2.medium",
+			availabilityZone: "us-west-2b",
+			expMaxVolumes:    39,
 		},
 		{
-			name:              "success normal with overwrite",
-			instanceID:        "i-123456789abcdef01",
-			instanceType:      "t2.medium",
-			availabilityZone:  "us-west-2b",
-			volumeAttachLimit: 42,
-			expMaxVolumes:     42,
-		},
-		{
-			name:              "success normal with NVMe",
-			instanceID:        "i-123456789abcdef01",
-			instanceType:      "m5d.large",
-			availabilityZone:  "us-west-2b",
-			volumeAttachLimit: -1,
-			expMaxVolumes:     25,
-		},
-		{
-			name:              "success normal with NVMe and overwrite",
-			instanceID:        "i-123456789abcdef01",
-			instanceType:      "m5d.large",
-			availabilityZone:  "us-west-2b",
-			volumeAttachLimit: 30,
-			expMaxVolumes:     30,
+			name:             "success normal with NVMe",
+			instanceID:       "i-123456789abcdef01",
+			instanceType:     "m5d.large",
+			availabilityZone: "us-west-2b",
+			expMaxVolumes:    25,
 		},
 	}
 	for _, tc := range testCases {
@@ -1288,25 +1269,17 @@ func TestNodeGetInfo(t *testing.T) {
 			mockCtl := gomock.NewController(t)
 			defer mockCtl.Finish()
 
-			driverOptions := &DriverOptions{
-				volumeAttachLimit: tc.volumeAttachLimit,
-			}
-
-			mockMounter := mocks.NewMockMounter(mockCtl)
-
 			mockMetadata := mocks.NewMockMetadataService(mockCtl)
 			mockMetadata.EXPECT().GetInstanceID().Return(tc.instanceID)
+			mockMetadata.EXPECT().GetInstanceType().Return(tc.instanceType)
 			mockMetadata.EXPECT().GetAvailabilityZone().Return(tc.availabilityZone)
 
-			if tc.volumeAttachLimit < 0 {
-				mockMetadata.EXPECT().GetInstanceType().Return(tc.instanceType)
-			}
+			mockMounter := mocks.NewMockMounter(mockCtl)
 
 			awsDriver := &nodeService{
-				metadata:      mockMetadata,
-				mounter:       mockMounter,
-				inFlight:      internal.NewInFlight(),
-				driverOptions: driverOptions,
+				metadata: mockMetadata,
+				mounter:  mockMounter,
+				inFlight: internal.NewInFlight(),
 			}
 
 			resp, err := awsDriver.NodeGetInfo(context.TODO(), &csi.NodeGetInfoRequest{})
diff --git a/pkg/driver/sanity_test.go b/pkg/driver/sanity_test.go
index 8afb81dc..799eafdb 100644
--- a/pkg/driver/sanity_test.go
+++ b/pkg/driver/sanity_test.go
@@ -53,9 +53,8 @@ func TestSanity(t *testing.T) {
 				Region:           "region",
 				AvailabilityZone: "az",
 			},
-			mounter:       newFakeMounter(),
-			inFlight:      internal.NewInFlight(),
-			driverOptions: &DriverOptions{},
+			mounter:  newFakeMounter(),
+			inFlight: internal.NewInFlight(),
 		},
 	}
 	defer func() {