Few tweaks to usage of pointers

eksctl-io · May 10, 2019 · b679900 · b679900
1 parent 0ef1652
commit b679900
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 30 deletions.
diff --git a/pkg/ami/api.go b/pkg/ami/api.go
@@ -10,8 +10,9 @@ import (
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 
-	api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
 	"fmt"
+
+	api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
 )
 
 const (
@@ -41,12 +42,12 @@ var ImageClasses = []string{
 }
 
 // Use checks if a given AMI ID is available in AWS EC2 as well as checking and populating RootDevice information
-func Use(api ec2iface.EC2API, ng *api.NodeGroup) error {
+func Use(ec2api ec2iface.EC2API, ng *api.NodeGroup) error {
 	input := &ec2.DescribeImagesInput{
 		ImageIds: []*string{&ng.AMI},
 	}
 
-	output, err := api.DescribeImages(input)
+	output, err := ec2api.DescribeImages(input)
 	if err != nil {
 		return errors.Wrapf(err, "unable to find image %q", ng.AMI)
 	}
@@ -61,8 +62,8 @@ func Use(api ec2iface.EC2API, ng *api.NodeGroup) error {
 		return fmt.Errorf("%q is an instance-store AMI and EBS block device mappings not supported for instance-store AMIs", ng.AMI)
 	}
 
-	if *output.Images[0].RootDeviceType == "ebs" {
-		ng.VolumeName = *output.Images[0].RootDeviceName
+	if *output.Images[0].RootDeviceType == "ebs" && !api.IsSetAndNonEmptyString(ng.VolumeName) {
+		ng.VolumeName = output.Images[0].RootDeviceName
 	}
 
 	return nil
@@ -71,7 +72,7 @@ func Use(api ec2iface.EC2API, ng *api.NodeGroup) error {
 // FindImage will get the AMI to use for the EKS nodes by querying AWS EC2 API.
 // It will only look for images with a status of available and it will pick the
 // image with the newest creation date.
-func FindImage(api ec2iface.EC2API, ownerAccount, namePattern string) (string, error) {
+func FindImage(ec2api ec2iface.EC2API, ownerAccount, namePattern string) (string, error) {
 	input := &ec2.DescribeImagesInput{
 		Owners: []*string{&ownerAccount},
 		Filters: []*ec2.Filter{
@@ -98,7 +99,7 @@ func FindImage(api ec2iface.EC2API, ownerAccount, namePattern string) (string, e
 		},
 	}
 
-	output, err := api.DescribeImages(input)
+	output, err := ec2api.DescribeImages(input)
 	if err != nil {
 		return "", errors.Wrapf(err, "error querying AWS for images")
 	}

diff --git a/pkg/apis/eksctl.io/v1alpha5/defaults.go b/pkg/apis/eksctl.io/v1alpha5/defaults.go
@@ -45,10 +45,8 @@ func SetNodeGroupDefaults(_ int, ng *NodeGroup) error {
 		}
 	}
 
-	if *ng.VolumeSize > 0 {
-		if *ng.VolumeType == "" {
-			*ng.VolumeType = DefaultNodeVolumeType
-		}
+	if !IsSetAndNonEmptyString(ng.VolumeType) {
+		ng.VolumeType = &DefaultNodeVolumeType
 	}
 
 	if ng.IAM == nil {

diff --git a/pkg/apis/eksctl.io/v1alpha5/types.go b/pkg/apis/eksctl.io/v1alpha5/types.go
@@ -132,7 +132,6 @@ var (
 
 	// DefaultNodeVolumeSize defines the default root volume size
 	DefaultNodeVolumeSize = 0
-
 )
 
 // Enabled return pointer to true value
@@ -400,7 +399,7 @@ type NodeGroup struct {
 	// +optional
 	VolumeType *string `json:"volumeType"`
 	// +optional
-	VolumeName string `json:"volumeName,omitempty"`
+	VolumeName *string `json:"volumeName,omitempty"`
 	// +optional
 	MaxPodsPerNode int `json:"maxPodsPerNode,omitempty"`
 

diff --git a/pkg/cfn/builder/api_test.go b/pkg/cfn/builder/api_test.go
@@ -242,16 +242,16 @@ var _ = Describe("CloudFormation template builder API", func() {
 			Endpoint:                 endpoint,
 		}
 
-		volType := api.NodeVolumeTypeIO1
-		volSize := 2
-
 		cfg.AvailabilityZones = testAZs
 		ng.Name = "ng-abcd1234"
 		ng.InstanceType = "t2.medium"
 		ng.AMIFamily = "AmazonLinux2"
-		ng.VolumeSize = &volSize
-		ng.VolumeType = &volType
-		ng.VolumeName = "/dev/xvda"
+		ng.VolumeSize = new(int)
+		*ng.VolumeSize = 2
+		ng.VolumeType = new(string)
+		*ng.VolumeType = api.NodeVolumeTypeIO1
+		ng.VolumeName = new(string)
+		*ng.VolumeName = "/dev/xvda"
 
 		if withFullVPC {
 			cfg.VPC = testVPC()
@@ -325,8 +325,6 @@ var _ = Describe("CloudFormation template builder API", func() {
 	}
 
 	Describe("GetAllOutputsFromClusterStack", func() {
-		volSize := 2
-		volType := api.NodeVolumeTypeIO1
 		expected := &api.ClusterConfig{
 			TypeMeta: api.ClusterConfigTypeMeta(),
 			Metadata: &api.ClusterMeta{
@@ -357,9 +355,9 @@ var _ = Describe("CloudFormation template builder API", func() {
 						AttachIDs:  []string{},
 					},
 					DesiredCapacity: nil,
-					VolumeSize:      &volSize,
-					VolumeType:      &volType,
-					VolumeName:      "/dev/xvda",
+					VolumeSize:      aws.Int(2),
+					VolumeType:      aws.String(api.NodeVolumeTypeIO1),
+					VolumeName:      aws.String("/dev/xvda"),
 					IAM: &api.NodeGroupIAM{
 						WithAddonPolicies: api.NodeGroupIAMAddonPolicies{
 							ImageBuilder: api.Disabled(),
@@ -955,7 +953,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	Context("NodeGroupEBS", func() {
 		cfg, ng := newClusterConfigAndNodegroup(true)
 
-		*ng.VolumeSize = 0
+		ng.VolumeSize = nil
 		ng.IAM.WithAddonPolicies.EBS = api.Enabled()
 
 		build(cfg, "eksctl-test-ebs-cluster", ng)
@@ -1000,7 +998,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	Context("NodeGroupFSX", func() {
 		cfg, ng := newClusterConfigAndNodegroup(true)
 
-		*ng.VolumeSize = 0
+		ng.VolumeSize = nil
 		ng.IAM.WithAddonPolicies.FSX = api.Enabled()
 
 		build(cfg, "eksctl-test-fsx-cluster", ng)
@@ -1034,7 +1032,7 @@ var _ = Describe("CloudFormation template builder API", func() {
 	Context("NodeGroupEFS", func() {
 		cfg, ng := newClusterConfigAndNodegroup(true)
 
-		*ng.VolumeSize = 0
+		ng.VolumeSize = nil
 		ng.IAM.WithAddonPolicies.EFS = api.Enabled()
 
 		build(cfg, "eksctl-test-efs-cluster", ng)

diff --git a/pkg/cfn/builder/nodegroup.go b/pkg/cfn/builder/nodegroup.go
@@ -126,11 +126,11 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
 		launchTemplateData.KeyName = gfn.NewString(*n.spec.SSH.PublicKeyName)
 	}
 
-	if *n.spec.VolumeSize > 0 {
+	if volumeSize := n.spec.VolumeSize; volumeSize != nil && *volumeSize > 0 {
 		launchTemplateData.BlockDeviceMappings = []gfn.AWSEC2LaunchTemplate_BlockDeviceMapping{{
-			DeviceName: gfn.NewString(n.spec.VolumeName),
+			DeviceName: gfn.NewString(*n.spec.VolumeName),
 			Ebs: &gfn.AWSEC2LaunchTemplate_Ebs{
-				VolumeSize: gfn.NewInteger(*n.spec.VolumeSize),
+				VolumeSize: gfn.NewInteger(*volumeSize),
 				VolumeType: gfn.NewString(*n.spec.VolumeType),
 			},
 		}}