Repository: keikoproj/instance-manager (go)

Issue:
Subnets field should deduplicate if duplicate values are provided
If someone mistakenly provides dupe values, e.g.:
```yaml
  subnets:
  - subnet-0310e3af3100225b9
  - subnet-0310e3af3100225b9
  - subnet-05c09777fce590171
```

The call will fail with:
```
"error": "provisioner eks reconcile failed: failed to update scaling group: ValidationError: Duplicate subnets specified: [subnet-0310e3af3100225b9]
```

We should consider having the controller dedupe values to avoid the error in this case and only use the two subnets.

Downside is that maybe we would like to explicitly provide this error if the user made a mistake and provided wrong subnets.

Pull request:
fix: Deduplicate Subnets
Signed-off-by: Eytan Avisror <eytan_avisror@intuit.com>

Fixes #339 

This ignores duplicate values if provided in subnet field to avoid blocking reconcile.

Patch:
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
index 90f591c..83dce64 100644
--- a/api/v1alpha1/zz_generated.deepcopy.go
+++ b/api/v1alpha1/zz_generated.deepcopy.go
@@ -1,3 +1,4 @@
+//go:build !ignore_autogenerated
 // +build !ignore_autogenerated
 
 /*
diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go
index 143f812..37ebb64 100644
--- a/controllers/provisioners/eks/helpers.go
+++ b/controllers/provisioners/eks/helpers.go
@@ -65,9 +65,18 @@ func (ctx *EksInstanceGroupContext) ResolveSubnets() []string {
 		}
 		resolved = append(resolved, aws.StringValue(sn.SubnetId))
 	}
-	sort.Strings(resolved)
 
-	return resolved
+	var dedupe = make([]string, 0)
+	for _, item := range resolved {
+		if common.ContainsString(dedupe, item) {
+			ctx.Log.Info("ignoring duplicate value in subnet list", "subnet", item)
+		} else {
+			dedupe = append(dedupe, item)
+		}
+	}
+
+	sort.Strings(dedupe)
+	return dedupe
 }
 
 func (ctx *EksInstanceGroupContext) ResolveSecurityGroups() []string {
@@ -512,7 +521,7 @@ func (ctx *EksInstanceGroupContext) GetComputedBootstrapOptions() *v1alpha1.Boot
 		}
 
 		// Don't set maxPods above Kubernetes-recommended 110 per node for large clusters.
-		maxPods = common.Min(enis*((aws.Int64Value(instanceTypeNetworkInfo.Ipv4AddressesPerInterface)-1)*ipsPerInterface) + hostNetworkPods, 110)
+		maxPods = common.Min(enis*((aws.Int64Value(instanceTypeNetworkInfo.Ipv4AddressesPerInterface)-1)*ipsPerInterface)+hostNetworkPods, 110)
 
 		if configuration.BootstrapOptions == nil {
 			return &v1alpha1.BootstrapOptions{
diff --git a/go.sum b/go.sum
index b40dcb6..21adef4 100644
--- a/go.sum
+++ b/go.sum
@@ -72,6 +72,7 @@ github.com/blang/semver v3.5.0+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnweb
 github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ=
 github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QHaoyV4aDUVVkXQJJJ3NXXM=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
+github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko=
 github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
 github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY=
 github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=


Test patch:
diff --git a/controllers/provisioners/eks/helpers_test.go b/controllers/provisioners/eks/helpers_test.go
index 7329263..2fdcc6e 100644
--- a/controllers/provisioners/eks/helpers_test.go
+++ b/controllers/provisioners/eks/helpers_test.go
@@ -494,6 +494,7 @@ func TestResolveSubnets(t *testing.T) {
 		withErr   bool
 	}{
 		{requested: []string{"subnet-111", "subnet-222"}, subnets: []*ec2.Subnet{MockSubnet("subnet-111", false, ""), MockSubnet("subnet-222", false, "")}, result: []string{"subnet-111", "subnet-222"}, withErr: false},
+		{requested: []string{"subnet-111", "subnet-111", "subnet-222"}, subnets: []*ec2.Subnet{MockSubnet("subnet-111", false, ""), MockSubnet("subnet-222", false, "")}, result: []string{"subnet-111", "subnet-222"}, withErr: false},
 		{requested: []string{"my-subnet-1", "subnet-222"}, subnets: []*ec2.Subnet{MockSubnet("subnet-111", true, "my-subnet-1"), MockSubnet("subnet-222", false, "")}, result: []string{"subnet-111", "subnet-222"}, withErr: false},
 		{requested: []string{"my-subnet-1", "my-subnet-2"}, subnets: []*ec2.Subnet{MockSubnet("subnet-111", true, "my-subnet-1"), MockSubnet("subnet-222", true, "my-subnet-2")}, result: []string{"subnet-111", "subnet-222"}, withErr: false},
 		{requested: []string{"my-subnet-2", "my-subnet-1"}, subnets: []*ec2.Subnet{MockSubnet("subnet-111", true, "my-subnet-1"), MockSubnet("subnet-222", true, "my-subnet-2")}, result: []string{"subnet-111", "subnet-222"}, withErr: false},