Repository: keikoproj/instance-manager (go)

Issue:
API Retries on bootstrap.sh
The bootstrap.sh script has a flag `--aws-api-retry-attempts` that defaults to 3.
If the account is getting throttled heavily, this may cause nodes to fail joining the cluster.
We should set a higher number e.g. 12 to avoid this scenario as much as possible

Pull request description:
Pass API Endpoint & CA Cert directly to bootstrap.sh
Signed-off-by: Eytan Avisror <eytan_avisror@intuit.com>
Fixes #306 

This PR uses the payload from DescribeCluster to feed CA & API endpoint directly to userdata in order to avoid making those calls and potentially getting throttled on user-data.

This also fixes the argument passing in the case of Windows so that we wrap the Kubelet arguments same as we do for Linux

### TBD
- [x] Test Linux AMI
- [x] Test Windows AMI

Patch:
diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go
index c5f89b1..6701362 100644
--- a/controllers/providers/aws/aws.go
+++ b/controllers/providers/aws/aws.go
@@ -23,7 +23,6 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/ec2metadata"
-	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/autoscaling"
 	"github.com/aws/aws-sdk-go/service/autoscaling/autoscalingiface"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -112,11 +111,12 @@ var (
 )
 
 type AwsWorker struct {
-	AsgClient  autoscalingiface.AutoScalingAPI
-	EksClient  eksiface.EKSAPI
-	IamClient  iamiface.IAMAPI
-	Ec2Client  ec2iface.EC2API
-	Parameters map[string]interface{}
+	AsgClient   autoscalingiface.AutoScalingAPI
+	EksClient   eksiface.EKSAPI
+	IamClient   iamiface.IAMAPI
+	Ec2Client   ec2iface.EC2API
+	Ec2Metadata *ec2metadata.EC2Metadata
+	Parameters  map[string]interface{}
 }
 
 func (w *AwsWorker) WithRetries(f func() bool) error {
@@ -164,18 +164,12 @@ func GetTagValueByKey(tags []*autoscaling.TagDescription, key string) string {
 	return ""
 }
 
-func GetRegion() (string, error) {
+func GetRegion(metadata *ec2metadata.EC2Metadata) (string, error) {
 	if os.Getenv("AWS_REGION") != "" {
 		return os.Getenv("AWS_REGION"), nil
 	}
-	// Try Derive
-	var config aws.Config
-	sess := session.Must(session.NewSessionWithOptions(session.Options{
-		SharedConfigState: session.SharedConfigEnable,
-		Config:            config,
-	}))
-	c := ec2metadata.New(sess)
-	region, err := c.Region()
+
+	region, err := metadata.Region()
 	if err != nil {
 		return "", err
 	}
diff --git a/controllers/providers/aws/ec2.go b/controllers/providers/aws/ec2.go
index ff74343..5c237fb 100644
--- a/controllers/providers/aws/ec2.go
+++ b/controllers/providers/aws/ec2.go
@@ -19,6 +19,7 @@ import (
 	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/ec2metadata"
 	"github.com/aws/aws-sdk-go/aws/request"
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/ec2"
@@ -56,6 +57,15 @@ func GetAwsEc2Client(region string, cacheCfg *cache.Config, maxRetries int, coll
 	return ec2.New(sess)
 }
 
+func GetAwsEc2MetadataClient() *ec2metadata.EC2Metadata {
+	var config aws.Config
+	sess := session.Must(session.NewSessionWithOptions(session.Options{
+		SharedConfigState: session.SharedConfigEnable,
+		Config:            config,
+	}))
+	return ec2metadata.New(sess)
+}
+
 func (w *AwsWorker) DescribeInstanceOfferings() ([]*ec2.InstanceTypeOffering, error) {
 	offerings := []*ec2.InstanceTypeOffering{}
 	err := w.Ec2Client.DescribeInstanceTypeOfferingsPages(&ec2.DescribeInstanceTypeOfferingsInput{}, func(page *ec2.DescribeInstanceTypeOfferingsOutput, lastPage bool) bool {
diff --git a/controllers/providers/aws/eks.go b/controllers/providers/aws/eks.go
index 7ec6c1c..8f3922d 100644
--- a/controllers/providers/aws/eks.go
+++ b/controllers/providers/aws/eks.go
@@ -16,6 +16,8 @@ limitations under the License.
 package aws
 
 import (
+	"strings"
+
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/aws/request"
@@ -210,3 +212,12 @@ func (w *AwsWorker) DescribeFargateProfile() (*eks.FargateProfile, error) {
 	}
 	return output.FargateProfile, nil
 }
+
+func (w *AwsWorker) GetDNSClusterIP(cluster *eks.Cluster) string {
+	if cluster == nil {
+		return ""
+	}
+	serviceCidr := aws.StringValue(cluster.KubernetesNetworkConfig.ServiceIpv4Cidr)
+	// addresses assigned from either the 10.100.0.0/16 or 172.20.0.0/16 CIDR blocks
+	return strings.ReplaceAll(serviceCidr, "0/16", "10")
+}
diff --git a/controllers/provisioners/eks/cloud.go b/controllers/provisioners/eks/cloud.go
index 1131041..1a23fe6 100644
--- a/controllers/provisioners/eks/cloud.go
+++ b/controllers/provisioners/eks/cloud.go
@@ -285,6 +285,10 @@ func (d *DiscoveredState) SetCluster(cluster *eks.Cluster) {
 	d.Cluster = cluster
 }
 
+func (d *DiscoveredState) GetCluster() *eks.Cluster {
+	return d.Cluster
+}
+
 func (d *DiscoveredState) SetVPCId(id string) {
 	d.VPCId = id
 }
diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go
index f785d89..1ea39f2 100644
--- a/controllers/provisioners/eks/helpers.go
+++ b/controllers/provisioners/eks/helpers.go
@@ -133,7 +133,7 @@ func (ctx *EksInstanceGroupContext) GetBasicUserData(clusterName, args string, k
     Echo "Not starting Kubelet due to warmed state."
     & C:\ProgramData\Amazon\EC2-Windows\Launch\Scripts\InitializeInstance.ps1 –Schedule
   } else {
-    & $EKSBootstrapScriptFile -EKSClusterName {{ .ClusterName }} -KubeletExtraArgs '{{ .KubeletExtraArgs }}' 3>&1 4>&1 5>&1 6>&1
+    & $EKSBootstrapScriptFile -EKSClusterName {{ .ClusterName }} {{ .Arguments }} 3>&1 4>&1 5>&1 6>&1
     {{range $post := .PostBootstrap}}{{$post}}{{end}}
   }
 </powershell>`
@@ -513,13 +513,34 @@ func (ctx *EksInstanceGroupContext) GetComputedBootstrapOptions() *v1alpha1.Boot
 func (ctx *EksInstanceGroupContext) GetBootstrapArgs() string {
 	var (
 		bootstrapOptions = ctx.GetComputedBootstrapOptions()
+		state            = ctx.GetDiscoveredState()
+		osFamily         = ctx.GetOsFamily()
+		cluster          = state.GetCluster()
+		clusterIP        = ctx.AwsWorker.GetDNSClusterIP(cluster)
 	)
 	var sb strings.Builder
+	switch strings.ToLower(osFamily) {
+	case OsFamilyWindows:
+		if state.Cluster != nil {
+			sb.WriteString(fmt.Sprintf("-Base64ClusterCA %v ", aws.StringValue(state.Cluster.CertificateAuthority.Data)))
+			sb.WriteString(fmt.Sprintf("-APIServerEndpoint %v ", aws.StringValue(state.Cluster.Endpoint)))
+		}
+		sb.WriteString(fmt.Sprintf("-KubeletExtraArgs '%v'", ctx.GetKubeletExtraArgs()))
+	case OsFamilyAmazonLinux2:
+		if bootstrapOptions != nil && bootstrapOptions.MaxPods > 0 {
+			sb.WriteString("--use-max-pods false ")
+		}
+		if state.Cluster != nil {
+			sb.WriteString(fmt.Sprintf("--b64-cluster-ca %v ", aws.StringValue(state.Cluster.CertificateAuthority.Data)))
+			sb.WriteString(fmt.Sprintf("--apiserver-endpoint %v ", aws.StringValue(state.Cluster.Endpoint)))
+			if !common.StringEmpty(clusterIP) {
+				sb.WriteString(fmt.Sprintf("--dns-cluster-ip %v ", clusterIP))
+			}
+		}
 
-	if bootstrapOptions != nil && bootstrapOptions.MaxPods > 0 {
-		sb.WriteString("--use-max-pods false ")
+		sb.WriteString(fmt.Sprintf("--kubelet-extra-args '%v'", ctx.GetKubeletExtraArgs()))
 	}
-	sb.WriteString(fmt.Sprintf("--kubelet-extra-args '%v'", ctx.GetKubeletExtraArgs()))
+
 	return sb.String()
 }
 
diff --git a/main.go b/main.go
index a2091b4..5c0153e 100644
--- a/main.go
+++ b/main.go
@@ -98,7 +98,8 @@ func main() {
 		os.Exit(1)
 	}
 
-	awsRegion, err := aws.GetRegion()
+	metadata := aws.GetAwsEc2MetadataClient()
+	awsRegion, err := aws.GetRegion(metadata)
 	if err != nil {
 		setupLog.Error(err, "unable to get AWS region")
 		os.Exit(1)
@@ -120,10 +121,11 @@ func main() {
 	cacheCollector := cacheCfg.NewCacheCollector("instance_manager")
 	controllerCollector := common.NewMetricsCollector()
 	awsWorker := aws.AwsWorker{
-		Ec2Client: aws.GetAwsEc2Client(awsRegion, cacheCfg, maxAPIRetries, controllerCollector),
-		IamClient: aws.GetAwsIamClient(awsRegion, cacheCfg, maxAPIRetries, controllerCollector),
-		AsgClient: aws.GetAwsAsgClient(awsRegion, cacheCfg, maxAPIRetries, controllerCollector),
-		EksClient: aws.GetAwsEksClient(awsRegion, cacheCfg, maxAPIRetries, controllerCollector),
+		Ec2Client:   aws.GetAwsEc2Client(awsRegion, cacheCfg, maxAPIRetries, controllerCollector),
+		IamClient:   aws.GetAwsIamClient(awsRegion, cacheCfg, maxAPIRetries, controllerCollector),
+		AsgClient:   aws.GetAwsAsgClient(awsRegion, cacheCfg, maxAPIRetries, controllerCollector),
+		EksClient:   aws.GetAwsEksClient(awsRegion, cacheCfg, maxAPIRetries, controllerCollector),
+		Ec2Metadata: metadata,
 	}
 
 	metrics.Registry.MustRegister(cacheCollector, controllerCollector)


Test patch:
diff --git a/controllers/provisioners/eks/eks_test.go b/controllers/provisioners/eks/eks_test.go
index 01966fe..e51c059 100644
--- a/controllers/provisioners/eks/eks_test.go
+++ b/controllers/provisioners/eks/eks_test.go
@@ -86,11 +86,14 @@ func MockAwsWorker(asgClient *MockAutoScalingClient, iamClient *MockIamClient, e
 func MockEksCluster(version string) *eks.Cluster {
 	return &eks.Cluster{
 		CertificateAuthority: &eks.Certificate{
-			Data: aws.String(""),
+			Data: aws.String("dGVzdA=="),
 		},
 		Endpoint:           aws.String("foo.amazonaws.com"),
 		ResourcesVpcConfig: &eks.VpcConfigResponse{},
-		Version:            &version,
+		KubernetesNetworkConfig: &eks.KubernetesNetworkConfigResponse{
+			ServiceIpv4Cidr: aws.String("172.20.0.0/16"),
+		},
+		Version: &version,
 	}
 }
 
diff --git a/controllers/provisioners/eks/helpers_test.go b/controllers/provisioners/eks/helpers_test.go
index 5627b06..c4d7a75 100644
--- a/controllers/provisioners/eks/helpers_test.go
+++ b/controllers/provisioners/eks/helpers_test.go
@@ -142,7 +142,7 @@ func TestGetBasicUserDataAmazonLinux2(t *testing.T) {
 		mounts          = ctx.GetMountOpts()
 	)
 
-	expectedData := `#!/bin/bash
+	expectedDataLinux := `#!/bin/bash
 foo
 mkfs.xfs /dev/xvda
 mkdir /mnt/foo
@@ -159,16 +159,90 @@ if [[ $(type -P $(which aws)) ]] && [[ $(type -P $(which jq)) ]] ; then
 	fi
 fi
 set -o xtrace
-/etc/eks/bootstrap.sh foo --use-max-pods false --kubelet-extra-args '--node-labels=foo=bar,instancemgr.keikoproj.io/image=ami-123456789012,node.kubernetes.io/role=instance-group-1 --register-with-taints=foo=bar:NoSchedule --eviction-hard=memory.available<300Mi,nodefs.available<5% --system-reserved=memory=2.5Gi --v=2 --max-pods=4'
+/etc/eks/bootstrap.sh foo --use-max-pods false --b64-cluster-ca dGVzdA== --apiserver-endpoint foo.amazonaws.com --dns-cluster-ip 172.20.0.10 --kubelet-extra-args '--node-labels=foo=bar,instancemgr.keikoproj.io/image=ami-123456789012,node.kubernetes.io/role=instance-group-1 --register-with-taints=foo=bar:NoSchedule --eviction-hard=memory.available<300Mi,nodefs.available<5% --system-reserved=memory=2.5Gi --v=2 --max-pods=4'
 set +o xtrace
 bar`
 	userData := ctx.GetBasicUserData("foo", args, kubeletArgs, userDataPayload, mounts)
 	basicUserDataDecoded, _ := base64.StdEncoding.DecodeString(userData)
 	basicUserDataString := string(basicUserDataDecoded)
-	if basicUserDataString != expectedData {
-		t.Fatalf("\nExpected: START>%v<END\n Got: START>%v<END", expectedData, basicUserDataString)
+	if basicUserDataString != expectedDataLinux {
+		t.Fatalf("\nExpected: START>%v<END\n Got: START>%v<END", expectedDataLinux, basicUserDataString)
 	}
+}
+
+func TestGetBasicUserDataWindows(t *testing.T) {
+	var (
+		k             = MockKubernetesClientSet()
+		ig            = MockInstanceGroup()
+		asgMock       = NewAutoScalingMocker()
+		iamMock       = NewIamMocker()
+		eksMock       = NewEksMocker()
+		ec2Mock       = NewEc2Mocker()
+		configuration = ig.GetEKSConfiguration()
+	)
+
+	w := MockAwsWorker(asgMock, iamMock, eksMock, ec2Mock)
+	ctx := MockContext(ig, k, w)
+
+	configuration.BootstrapOptions = &v1alpha1.BootstrapOptions{
+		MaxPods: 4,
+	}
+	configuration.Labels = map[string]string{
+		"foo": "bar",
+	}
+	configuration.Taints = []corev1.Taint{
+		{
+			Key:    "foo",
+			Value:  "bar",
+			Effect: "NoSchedule",
+		},
+	}
+
+	configuration.BootstrapArguments = "--eviction-hard=memory.available<300Mi,nodefs.available<5% --system-reserved=memory=2.5Gi --v=2"
+	configuration.UserData = []v1alpha1.UserDataStage{
+		{
+			Stage: "PreBootstrap",
+			Data:  "foo",
+		},
+		{
+			Stage: "PostBootstrap",
+			Data:  "bar",
+		},
+	}
+
+	ig.Annotations[OsFamilyAnnotation] = OsFamilyWindows
+
+	expectedDataWindows := `
+<powershell>
+  foo
+  [string]$EKSBinDir = "$env:ProgramFiles\Amazon\EKS"
+  [string]$EKSBootstrapScriptName = 'Start-EKSBootstrap.ps1'
+  [string]$EKSBootstrapScriptFile = "$EKSBinDir\$EKSBootstrapScriptName"
+  [string]$IMDSToken=(curl -UseBasicParsing -Method PUT "http://169.254.169.254/latest/api/token" -H @{ "X-aws-ec2-metadata-token-ttl-seconds" = "21600"} | % { Echo $_.Content})
+  [string]$InstanceID=(curl -UseBasicParsing -Method GET "http://169.254.169.254/latest/meta-data/instance-id" -H @{ "X-aws-ec2-metadata-token" = "$IMDSToken"} | % { Echo $_.Content})
+  [string]$Lifecycle = Get-ASAutoScalingInstance $InstanceID | % { Echo $_.LifecycleState}
+  if ($Lifecycle -like "*Warmed*") {
+    Echo "Not starting Kubelet due to warmed state."
+    & C:\ProgramData\Amazon\EC2-Windows\Launch\Scripts\InitializeInstance.ps1 –Schedule
+  } else {
+    & $EKSBootstrapScriptFile -EKSClusterName foo -Base64ClusterCA dGVzdA== -APIServerEndpoint foo.amazonaws.com -KubeletExtraArgs '--node-labels=foo=bar,instancemgr.keikoproj.io/image=ami-123456789012,node.kubernetes.io/role=instance-group-1 --register-with-taints=foo=bar:NoSchedule --eviction-hard=memory.available<300Mi,nodefs.available<5% --system-reserved=memory=2.5Gi --v=2 --max-pods=4' 3>&1 4>&1 5>&1 6>&1
+    bar
+  }
+</powershell>`
 
+	var (
+		args            = ctx.GetBootstrapArgs()
+		kubeletArgs     = ctx.GetKubeletExtraArgs()
+		userDataPayload = ctx.GetUserDataStages()
+		mounts          = ctx.GetMountOpts()
+	)
+
+	userData := ctx.GetBasicUserData("foo", args, kubeletArgs, userDataPayload, mounts)
+	basicUserDataDecoded, _ := base64.StdEncoding.DecodeString(userData)
+	basicUserDataString := string(basicUserDataDecoded)
+	if basicUserDataString != expectedDataWindows {
+		t.Fatalf("\nExpected: START>%v<END\n Got: START>%v<END", expectedDataWindows, basicUserDataString)
+	}
 }
 
 func TestCustomNetworkingMaxPods(t *testing.T) {
diff --git a/controllers/provisioners/eks/scaling/launchconfig_test.go b/controllers/provisioners/eks/scaling/launchconfig_test.go
index 30690c4..da7b779 100644
--- a/controllers/provisioners/eks/scaling/launchconfig_test.go
+++ b/controllers/provisioners/eks/scaling/launchconfig_test.go
@@ -480,7 +480,7 @@ func TestLaunchConfigurationDrifted(t *testing.T) {
 				LaunchConfigurationName: aws.String("my-launch-config"),
 			},
 			input: &CreateConfigurationInput{
-				SecurityGroups: []string{},
+				SecurityGroups:  []string{},
 				MetadataOptions: &v1alpha1.MetadataOptions{HttpEndpoint: "enabled"},
 			},
 			shouldDrift: true,