Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve stack template descriptions #312

Merged
merged 2 commits into from
Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions pkg/cfn/builder/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,9 @@ import (
)

const (
clusterTemplateDescription = "EKS cluster"
clusterTemplateDescriptionDefaultFeatures = " (with dedicated VPC & IAM role) "

nodeGroupTemplateDescription = "EKS nodes"
nodeGroupTemplateDescriptionDefaultFeatures = " (Amazon Linux 2 with SSH) "

templateDescriptionSuffix = " [created and managed by eksctl]"
clusterTemplateDescription = "EKS cluster"
nodeGroupTemplateDescription = "EKS nodes"
templateDescriptionSuffix = "[created and managed by eksctl]"
)

type awsCloudFormationResource struct {
Expand Down
39 changes: 32 additions & 7 deletions pkg/cfn/builder/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ const (
)

type Template struct {
Resources map[string]struct {
Description string
Resources map[string]struct {
Properties struct {
Tags []struct {
Key interface{}
Expand Down Expand Up @@ -319,7 +320,6 @@ var _ = Describe("CloudFormation template builder API", func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(templateBody).ShouldNot(BeEmpty())
})

obj := Template{}
It("should parse JSON withon errors", func() {
err := json.Unmarshal(templateBody, &obj)
Expand Down Expand Up @@ -412,7 +412,7 @@ var _ = Describe("CloudFormation template builder API", func() {
})
})

Describe("NodeGroup{PrivateNetworking=true}", func() {
Describe("NodeGroup{PrivateNetworking=true AllowSSH=true}", func() {
cfg := api.NewClusterConfig()
ng := cfg.NewNodeGroup()

Expand All @@ -422,6 +422,7 @@ var _ = Describe("CloudFormation template builder API", func() {
ng.AllowSSH = true
ng.InstanceType = "t2.medium"
ng.PrivateNetworking = true
ng.AMIFamily = "AmazonLinux2"

rs := NewNodeGroupResourceSet(cfg, "eksctl-test-private-ng", 0)
rs.AddAllResources()
Expand All @@ -436,6 +437,12 @@ var _ = Describe("CloudFormation template builder API", func() {
Expect(err).ShouldNot(HaveOccurred())
})

It("should have correct description", func() {
Expect(obj.Description).To(ContainSubstring("AMI family: AmazonLinux2"))
Expect(obj.Description).To(ContainSubstring("SSH access: true"))
Expect(obj.Description).To(ContainSubstring("subnet topology: Private"))
})

It("should have correct resources and attributes", func() {
Expect(len(obj.Resources)).ToNot(Equal(0))

Expand Down Expand Up @@ -473,6 +480,7 @@ var _ = Describe("CloudFormation template builder API", func() {
ng.AllowSSH = true
ng.InstanceType = "t2.medium"
ng.PrivateNetworking = false
ng.AMIFamily = "AmazonLinux2"

rs := NewNodeGroupResourceSet(cfg, "eksctl-test-public-ng", 0)
rs.AddAllResources()
Expand All @@ -487,6 +495,12 @@ var _ = Describe("CloudFormation template builder API", func() {
Expect(err).ShouldNot(HaveOccurred())
})

It("should have correct description", func() {
Expect(obj.Description).To(ContainSubstring("AMI family: AmazonLinux2"))
Expect(obj.Description).To(ContainSubstring("SSH access: true"))
Expect(obj.Description).To(ContainSubstring("subnet topology: Public"))
})

It("should have correct resources and attributes", func() {
Expect(len(obj.Resources)).ToNot(Equal(0))

Expand Down Expand Up @@ -557,6 +571,7 @@ var _ = Describe("CloudFormation template builder API", func() {
ng.AllowSSH = false
ng.InstanceType = "t2.medium"
ng.PrivateNetworking = false
ng.AMIFamily = "AmazonLinux2"

It("should have 1 AZ for the nodegroup", func() {
Expect(ng.AvailabilityZones).To(Equal([]string{"us-west-2a"}))
Expand All @@ -575,6 +590,12 @@ var _ = Describe("CloudFormation template builder API", func() {
Expect(err).ShouldNot(HaveOccurred())
})

It("should have correct description", func() {
Expect(obj.Description).To(ContainSubstring("AMI family: AmazonLinux2"))
Expect(obj.Description).To(ContainSubstring("SSH access: false"))
Expect(obj.Description).To(ContainSubstring("subnet topology: Public"))
})

It("should have correct resources and attributes", func() {
Expect(len(obj.Resources)).ToNot(Equal(0))

Expand Down Expand Up @@ -646,9 +667,8 @@ var _ = Describe("CloudFormation template builder API", func() {
It("should serialise JSON without errors", func() {
Expect(err).ShouldNot(HaveOccurred())
})

obj := Template{}
It("should parse JSON withon errors and extract valid cloud-config using our implementation", func() {
obj := Template{}
err = json.Unmarshal(template, &obj)
Expect(err).ShouldNot(HaveOccurred())
Expect(len(obj.Resources)).ToNot(Equal(0))
Expand Down Expand Up @@ -713,9 +733,8 @@ var _ = Describe("CloudFormation template builder API", func() {
It("should serialise JSON without errors", func() {
Expect(err).ShouldNot(HaveOccurred())
})

obj := Template{}
It("should parse JSON withon errors and extract valid cloud-config using our implementation", func() {
obj := Template{}
err = json.Unmarshal(template, &obj)
Expect(err).ShouldNot(HaveOccurred())
Expect(len(obj.Resources)).ToNot(Equal(0))
Expand All @@ -727,6 +746,12 @@ var _ = Describe("CloudFormation template builder API", func() {
Expect(err).ShouldNot(HaveOccurred())
})

It("should have correct description", func() {
Expect(obj.Description).To(ContainSubstring("AMI family: Ubuntu1804"))
Expect(obj.Description).To(ContainSubstring("SSH access: false"))
Expect(obj.Description).To(ContainSubstring("subnet topology: Public"))
})

It("should have packages, scripts and commands in cloud-config", func() {
Expect(c).ToNot(BeNil())

Expand Down
20 changes: 11 additions & 9 deletions pkg/cfn/builder/cluster.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package builder

import (
"fmt"

cfn "github.com/aws/aws-sdk-go/service/cloudformation"
gfn "github.com/awslabs/goformation/cloudformation"

Expand Down Expand Up @@ -31,18 +33,22 @@ func NewClusterResourceSet(provider api.ClusterProvider, spec *api.ClusterConfig

// AddAllResources adds all the information about the cluster to the resource set
func (c *ClusterResourceSet) AddAllResources() error {
dedicatedVPC := c.spec.VPC.ID == ""

templateDescriptionFeatures := clusterTemplateDescriptionDefaultFeatures
c.rs.template.Description = fmt.Sprintf(
"%s (dedicated VPC: %v, dedicatd IAM: %v) %s",
clusterTemplateDescription,
dedicatedVPC, true,
templateDescriptionSuffix)

if err := c.spec.HasSufficientSubnets(); err != nil {
return err
}

if c.spec.VPC.ID != "" {
c.importResourcesForVPC()
templateDescriptionFeatures = " (with shared VPC and dedicated IAM role) "
} else {
if dedicatedVPC {
c.addResourcesForVPC()
} else {
c.importResourcesForVPC()
}
c.addOutputsForVPC()

Expand All @@ -52,10 +58,6 @@ func (c *ClusterResourceSet) AddAllResources() error {

c.rs.newOutput(cfnOutputClusterStackName, gfn.RefStackName, false)

c.rs.template.Description = clusterTemplateDescription
c.rs.template.Description += templateDescriptionFeatures
c.rs.template.Description += templateDescriptionSuffix

return nil
}

Expand Down
8 changes: 5 additions & 3 deletions pkg/cfn/builder/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,11 @@ func NewNodeGroupResourceSet(spec *api.ClusterConfig, clusterStackName string, i

// AddAllResources adds all the information about the node group to the resource set
func (n *NodeGroupResourceSet) AddAllResources() error {
n.rs.template.Description = nodeGroupTemplateDescription
n.rs.template.Description += nodeGroupTemplateDescriptionDefaultFeatures
n.rs.template.Description += templateDescriptionSuffix
n.rs.template.Description = fmt.Sprintf(
"%s (AMI family: %s, SSH access: %v, subnet topology: %s) %s",
nodeGroupTemplateDescription,
n.spec.AMIFamily, n.spec.AllowSSH, n.spec.SubnetTopology(),
templateDescriptionSuffix)

n.vpc = makeImportValue(n.clusterStackName, cfnOutputClusterVPC)

Expand Down
2 changes: 1 addition & 1 deletion pkg/nodebootstrap/userdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func makeClientConfigData(spec *api.ClusterConfig, nodeGroupID int) ([]byte, err
func clusterDNS(spec *api.ClusterConfig) string {
// Default service network is 10.100.0.0, but it gets set 172.20.0.0 automatically when pod network
// is anywhere within 10.0.0.0/8
if spec.VPC.CIDR.IP[0] == 10 {
if spec.VPC.CIDR != nil && spec.VPC.CIDR.IP[0] == 10 {
return "172.20.0.10"
}
return "10.100.0.10"
Expand Down