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.

Sign up for GitHub

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

Clear separation between api.ClusterConfig and api.ClusterProvider #309

Merged
merged 1 commit into from
Nov 8, 2018
Merged

Clear separation between api.ClusterConfig and api.ClusterProvider #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 0 additions & 4 deletions pkg/ami/auto_resolver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/stretchr/testify/mock"
. "github.com/weaveworks/eksctl/pkg/ami"
"github.com/weaveworks/eksctl/pkg/eks"
"github.com/weaveworks/eksctl/pkg/eks/api"
"github.com/weaveworks/eksctl/pkg/testutils"
)

Expand Down Expand Up @@ -170,9 +169,6 @@ func createProviders() (*eks.ClusterProvider, *testutils.MockProvider) {

c := &eks.ClusterProvider{
Provider: p,
Spec: &api.ClusterConfig{
Region: "eu-west-1",
},
}

return c, p
Expand Down
4 changes: 0 additions & 4 deletions pkg/az/az_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

. "github.com/weaveworks/eksctl/pkg/az"
"github.com/weaveworks/eksctl/pkg/eks"
"github.com/weaveworks/eksctl/pkg/eks/api"
"github.com/weaveworks/eksctl/pkg/testutils"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -255,9 +254,6 @@ func createProviders() (*eks.ClusterProvider, *testutils.MockProvider) {

c := &eks.ClusterProvider{
Provider: p,
Spec: &api.ClusterConfig{
Region: "us-west-1",
},
}

return c, p
Expand Down
31 changes: 17 additions & 14 deletions pkg/cfn/builder/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/weaveworks/eksctl/pkg/eks"
"github.com/weaveworks/eksctl/pkg/eks/api"
"github.com/weaveworks/eksctl/pkg/nodebootstrap"
"github.com/weaveworks/eksctl/pkg/testutils"
)

const (
Expand Down Expand Up @@ -105,8 +106,8 @@ var _ = Describe("CloudFormation template builder API", func() {
cfg := api.NewClusterConfig()
ng := cfg.NewNodeGroup()

cfg.Region = "us-west-2"
cfg.ClusterName = clusterName
cfg.Metadata.Region = "us-west-2"
cfg.Metadata.Name = clusterName
cfg.AvailabilityZones = testAZs
ng.InstanceType = "t2.medium"
ng.AMIFamily = "AmazonLinux2"
Expand All @@ -121,8 +122,10 @@ var _ = Describe("CloudFormation template builder API", func() {
It("should not error", func() { Expect(err).ShouldNot(HaveOccurred()) })

expected := &api.ClusterConfig{
Region: "us-west-2",
ClusterName: clusterName,
Metadata: &api.ClusterMeta{
Region: "us-west-2",
Name: clusterName,
},
Endpoint: endpoint,
CertificateAuthorityData: caCertData,
ARN: arn,
Expand Down Expand Up @@ -197,10 +200,10 @@ var _ = Describe("CloudFormation template builder API", func() {
}

cfg := newClusterConfig()
ctl := eks.New(cfg)
ctl := eks.New(testutils.ProviderConfig, cfg)

It("should not error when calling SetSubnets", func() {
err := ctl.SetSubnets()
err := ctl.SetSubnets(cfg)
Expect(err).ShouldNot(HaveOccurred())
})

Expand Down Expand Up @@ -286,8 +289,8 @@ var _ = Describe("CloudFormation template builder API", func() {
cfg := api.NewClusterConfig()
ng := cfg.NewNodeGroup()

cfg.Region = "us-west-2"
cfg.ClusterName = clusterName
cfg.Metadata.Region = "us-west-2"
cfg.Metadata.Name = clusterName
cfg.AvailabilityZones = testAZs
ng.InstanceType = "t2.medium"

Expand Down Expand Up @@ -359,8 +362,8 @@ var _ = Describe("CloudFormation template builder API", func() {
cfg := api.NewClusterConfig()
ng := cfg.NewNodeGroup()

cfg.Region = "us-west-2"
cfg.ClusterName = clusterName
cfg.Metadata.Region = "us-west-2"
cfg.Metadata.Name = clusterName
cfg.AvailabilityZones = testAZs
ng.AllowSSH = true
ng.InstanceType = "t2.medium"
Expand Down Expand Up @@ -410,8 +413,8 @@ var _ = Describe("CloudFormation template builder API", func() {
cfg := api.NewClusterConfig()
ng := cfg.NewNodeGroup()

cfg.Region = "us-west-2"
cfg.ClusterName = clusterName
cfg.Metadata.Region = "us-west-2"
cfg.Metadata.Name = clusterName
cfg.AvailabilityZones = testAZs
ng.AllowSSH = true
ng.InstanceType = "t2.medium"
Expand Down Expand Up @@ -463,8 +466,8 @@ var _ = Describe("CloudFormation template builder API", func() {
cfg := api.NewClusterConfig()
ng := cfg.NewNodeGroup()

cfg.Region = "us-west-2"
cfg.ClusterName = clusterName
cfg.Metadata.Region = "us-west-2"
cfg.Metadata.Name = clusterName
cfg.AvailabilityZones = testAZs
cfg.VPC = &api.ClusterVPC{
Network: api.Network{
Expand Down
2 changes: 1 addition & 1 deletion pkg/cfn/builder/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (c *ClusterResourceSet) addResourcesForControlPlane(version string) {
}

c.newResource("ControlPlane", &gfn.AWSEKSCluster{
Name: gfn.NewString(c.spec.ClusterName),
Name: gfn.NewString(c.spec.Metadata.Name),
RoleArn: gfn.MakeFnGetAttString("ServiceRole.Arn"),
Version: gfn.NewString(version),
ResourcesVpcConfig: clusterVPC,
Expand Down
4 changes: 2 additions & 2 deletions pkg/cfn/builder/nodegroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func NewNodeGroupResourceSet(spec *api.ClusterConfig, clusterStackName string, i
rs: newResourceSet(),
id: id,
clusterStackName: clusterStackName,
nodeGroupName: fmt.Sprintf("%s-%d", spec.ClusterName, id),
nodeGroupName: fmt.Sprintf("%s-%d", spec.Metadata.Name, id),
clusterSpec: spec,
spec: spec.NodeGroups[id],
}
Expand Down Expand Up @@ -156,7 +156,7 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
"VPCZoneIdentifier": vpcZoneIdentifier,
"Tags": []map[string]interface{}{
{"Key": "Name", "Value": fmt.Sprintf("%s-Node", n.nodeGroupName), "PropagateAtLaunch": "true"},
{"Key": "kubernetes.io/cluster/" + n.clusterSpec.ClusterName, "Value": "owned", "PropagateAtLaunch": "true"},
{"Key": "kubernetes.io/cluster/" + n.clusterSpec.Metadata.Name, "Value": "owned", "PropagateAtLaunch": "true"},
},
},
UpdatePolicy: map[string]map[string]string{
Expand Down
2 changes: 1 addition & 1 deletion pkg/cfn/builder/vpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func (n *NodeGroupResourceSet) addResourcesForSecurityGroups() {
VpcId: makeImportValue(n.clusterStackName, cfnOutputClusterVPC),
GroupDescription: gfn.NewString("Communication between the control plane and " + desc),
Tags: []gfn.Tag{{
Key: gfn.NewString("kubernetes.io/cluster/" + n.clusterSpec.ClusterName),
Key: gfn.NewString("kubernetes.io/cluster/" + n.clusterSpec.Metadata.Name),
Value: gfn.NewString("owned"),
}},
})
Expand Down
22 changes: 12 additions & 10 deletions pkg/cfn/manager/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ type ChangeSet = cloudformation.DescribeChangeSetOutput

// StackCollection stores the CloudFormation stack information
type StackCollection struct {
cfn cloudformationiface.CloudFormationAPI
spec *api.ClusterConfig
tags []*cloudformation.Tag
cfn cloudformationiface.CloudFormationAPI
waitTimeout time.Duration
spec *api.ClusterConfig
Copy link
Contributor Author

@errordeveloper errordeveloper Nov 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still keep a pointer here, but it's arguably more sensible, as there aren't many different kinds of calls to stack manager from the outside, so it's sensible to set it once. It basically gets handed off at this point, and the stack manager is coupled the cluster itself, there isn't a good reason to have it decoupled at this point, unlike the instance of the provider.

tags []*cloudformation.Tag
}

func newTag(key, value string) *cloudformation.Tag {
Expand All @@ -48,16 +49,17 @@ func newTag(key, value string) *cloudformation.Tag {
// NewStackCollection create a stack manager for a single cluster
func NewStackCollection(provider api.ClusterProvider, spec *api.ClusterConfig) *StackCollection {
tags := []*cloudformation.Tag{
newTag(ClusterNameTag, spec.ClusterName),
newTag(ClusterNameTag, spec.Metadata.Name),
}
for key, value := range spec.Tags {
for key, value := range spec.Metadata.Tags {
tags = append(tags, newTag(key, value))
}
logger.Debug("tags = %#v", tags)
return &StackCollection{
cfn: provider.CloudFormation(),
spec: spec,
tags: tags,
cfn: provider.CloudFormation(),
waitTimeout: provider.WaitTimeout(),
spec: spec,
tags: tags,
}
}

Expand Down Expand Up @@ -203,7 +205,7 @@ func (c *StackCollection) DeleteStack(name string) (*Stack, error) {
}
i.StackId = s.StackId
for _, tag := range s.Tags {
if *tag.Key == ClusterNameTag && *tag.Value == c.spec.ClusterName {
if *tag.Key == ClusterNameTag && *tag.Value == c.spec.Metadata.Name {
input := &cloudformation.DeleteStackInput{
StackName: i.StackId,
}
Expand All @@ -217,7 +219,7 @@ func (c *StackCollection) DeleteStack(name string) (*Stack, error) {
}

return nil, fmt.Errorf("cannot delete stack %q as it doesn't bare our %q tag", *s.StackName,
fmt.Sprintf("%s:%s", ClusterNameTag, c.spec.ClusterName))
fmt.Sprintf("%s:%s", ClusterNameTag, c.spec.Metadata.Name))
}

// WaitDeleteStack kills a stack by name and waits for DELETED status
Expand Down
2 changes: 1 addition & 1 deletion pkg/cfn/manager/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
)

func (c *StackCollection) makeClusterStackName() string {
return "eksctl-" + c.spec.ClusterName + "-cluster"
return "eksctl-" + c.spec.Metadata.Name + "-cluster"
}

// CreateCluster creates the cluster
Expand Down
8 changes: 4 additions & 4 deletions pkg/cfn/manager/deprecated.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package manager
// DeprecatedDeleteStackVPC deletes the VPC stack
func (c *StackCollection) DeprecatedDeleteStackVPC(wait bool) error {
var err error
stackName := "EKS-" + c.spec.ClusterName + "-VPC"
stackName := "EKS-" + c.spec.Metadata.Name + "-VPC"

if wait {
err = c.WaitDeleteStack(stackName)
Expand All @@ -17,7 +17,7 @@ func (c *StackCollection) DeprecatedDeleteStackVPC(wait bool) error {
// DeprecatedDeleteStackServiceRole deletes the service role stack
func (c *StackCollection) DeprecatedDeleteStackServiceRole(wait bool) error {
var err error
stackName := "EKS-" + c.spec.ClusterName + "-ServiceRole"
stackName := "EKS-" + c.spec.Metadata.Name + "-ServiceRole"

if wait {
err = c.WaitDeleteStack(stackName)
Expand All @@ -31,7 +31,7 @@ func (c *StackCollection) DeprecatedDeleteStackServiceRole(wait bool) error {
// DeprecatedDeleteStackDefaultNodeGroup deletes the default node group stack
func (c *StackCollection) DeprecatedDeleteStackDefaultNodeGroup(wait bool) error {
var err error
stackName := "EKS-" + c.spec.ClusterName + "-DefaultNodeGroup"
stackName := "EKS-" + c.spec.Metadata.Name + "-DefaultNodeGroup"

if wait {
err = c.WaitDeleteStack(stackName)
Expand All @@ -45,7 +45,7 @@ func (c *StackCollection) DeprecatedDeleteStackDefaultNodeGroup(wait bool) error
// DeprecatedDeleteStackControlPlane deletes the control plane stack
func (c *StackCollection) DeprecatedDeleteStackControlPlane(wait bool) error {
var err error
stackName := "EKS-" + c.spec.ClusterName + "-ControlPlane"
stackName := "EKS-" + c.spec.Metadata.Name + "-ControlPlane"

if wait {
err = c.WaitDeleteStack(stackName)
Expand Down
4 changes: 2 additions & 2 deletions pkg/cfn/manager/nodegroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const (
)

func (c *StackCollection) makeNodeGroupStackName(id int) string {
return fmt.Sprintf("eksctl-%s-nodegroup-%d", c.spec.ClusterName, id)
return fmt.Sprintf("eksctl-%s-nodegroup-%d", c.spec.Metadata.Name, id)
}

// CreateNodeGroup creates the nodegroup
Expand All @@ -45,7 +45,7 @@ func (c *StackCollection) CreateNodeGroup(errs chan error, data interface{}) err
}

func (c *StackCollection) listAllNodeGroups() ([]string, error) {
stacks, err := c.ListStacks(fmt.Sprintf("^eksctl-%s-nodegroup-\\d$", c.spec.ClusterName))
stacks, err := c.ListStacks(fmt.Sprintf("^eksctl-%s-nodegroup-\\d$", c.spec.Metadata.Name))
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cfn/manager/waiters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (c *StackCollection) waitWithAcceptors(i *Stack, acceptors []request.Waiter
desiredStatus := fmt.Sprintf("%v", acceptors[0].Expected)
msg := fmt.Sprintf("waiting for CloudFormation stack %q to reach %q status", *i.StackName, desiredStatus)

ctx, cancel := context.WithTimeout(context.Background(), c.spec.WaitTimeout)
ctx, cancel := context.WithTimeout(context.Background(), c.waitTimeout)
defer cancel()

startTime := time.Now()
Expand Down Expand Up @@ -118,7 +118,7 @@ func (c *StackCollection) waitWithAcceptors(i *Stack, acceptors []request.Waiter
func (c *StackCollection) waitWithAcceptorsChangeSet(i *Stack, changesetName *string, acceptors []request.WaiterAcceptor) error {
desiredStatus := fmt.Sprintf("%v", acceptors[0].Expected)
msg := fmt.Sprintf("waiting for CloudFormation changeset %q for stack %q to reach %q status", *changesetName, *i.StackName, desiredStatus)
ctx, cancel := context.WithTimeout(context.Background(), c.spec.WaitTimeout)
ctx, cancel := context.WithTimeout(context.Background(), c.waitTimeout)
defer cancel()
startTime := time.Now()
w := request.Waiter{
Expand Down
Loading