Skip to content

Commit

Permalink
Merge pull request #435 from weaveworks/fix-nodegroup-az-selection
Browse files Browse the repository at this point in the history
Fix issue preventing to create single-AZ nodegroups
  • Loading branch information
errordeveloper authored Jan 15, 2019
2 parents f8eae72 + 191bb1a commit 7d763b8
Show file tree
Hide file tree
Showing 10 changed files with 106 additions and 50 deletions.
8 changes: 8 additions & 0 deletions pkg/apis/eksctl.io/v1alpha3/vpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ const (
SubnetTopologyPublic SubnetTopology = "Public"
)

// SubnetTopologies returns a list of topologies
func SubnetTopologies() []SubnetTopology {
return []SubnetTopology{
SubnetTopologyPrivate,
SubnetTopologyPublic,
}
}

// DefaultCIDR returns default global CIDR for VPC
func DefaultCIDR() ipnet.IPNet {
return ipnet.IPNet{
Expand Down
24 changes: 12 additions & 12 deletions pkg/cfn/builder/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (c *ClusterResourceSet) AddAllResources() error {
c.addResourcesForIAM()
c.addResourcesForControlPlane()

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

return nil
}
Expand Down Expand Up @@ -93,9 +93,9 @@ func (c *ClusterResourceSet) addResourcesForControlPlane() {
ResourcesVpcConfig: clusterVPC,
})

c.rs.newOutputFromAtt(cfnOutputClusterCertificateAuthorityData, "ControlPlane.CertificateAuthorityData", false)
c.rs.newOutputFromAtt(cfnOutputClusterEndpoint, "ControlPlane.Endpoint", true)
c.rs.newOutputFromAtt(cfnOutputClusterARN, "ControlPlane.Arn", true)
c.rs.newOutputFromAtt(CfnOutputClusterCertificateAuthorityData, "ControlPlane.CertificateAuthorityData", false)
c.rs.newOutputFromAtt(CfnOutputClusterEndpoint, "ControlPlane.Endpoint", true)
c.rs.newOutputFromAtt(CfnOutputClusterARN, "ControlPlane.Arn", true)
}

// GetAllOutputs collects all outputs of the cluster
Expand All @@ -104,27 +104,27 @@ func (c *ClusterResourceSet) GetAllOutputs(stack cfn.Stack) error {
return err
}

c.spec.VPC.ID = c.outputs[cfnOutputClusterVPC]
c.spec.VPC.SecurityGroup = c.outputs[cfnOutputClusterSecurityGroup]
c.spec.VPC.ID = c.outputs[CfnOutputClusterVPC]
c.spec.VPC.SecurityGroup = c.outputs[CfnOutputClusterSecurityGroup]

if err := vpc.UseSubnets(c.provider, c.spec, api.SubnetTopologyPrivate, strings.Split(c.outputs[cfnOutputClusterSubnetsPrivate], ",")); err != nil {
if err := vpc.UseSubnets(c.provider, c.spec, api.SubnetTopologyPrivate, strings.Split(c.outputs[CfnOutputClusterSubnetsPrivate], ",")); err != nil {
return err
}

if err := vpc.UseSubnets(c.provider, c.spec, api.SubnetTopologyPublic, strings.Split(c.outputs[cfnOutputClusterSubnetsPublic], ",")); err != nil {
if err := vpc.UseSubnets(c.provider, c.spec, api.SubnetTopologyPublic, strings.Split(c.outputs[CfnOutputClusterSubnetsPublic], ",")); err != nil {
return err
}

caData, err := base64.StdEncoding.DecodeString(c.outputs[cfnOutputClusterCertificateAuthorityData])
caData, err := base64.StdEncoding.DecodeString(c.outputs[CfnOutputClusterCertificateAuthorityData])
if err != nil {
return errors.Wrap(err, "decoding certificate authority data")
}

c.spec.Status = &api.ClusterStatus{
CertificateAuthorityData: caData,
Endpoint: c.outputs[cfnOutputClusterEndpoint],
ARN: c.outputs[cfnOutputClusterARN],
StackName: c.outputs[cfnOutputClusterStackName],
Endpoint: c.outputs[CfnOutputClusterEndpoint],
ARN: c.outputs[CfnOutputClusterARN],
StackName: c.outputs[CfnOutputClusterStackName],
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/cfn/builder/iam.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,5 @@ func (n *NodeGroupResourceSet) addResourcesForIAM() {
)
}

n.rs.newOutputFromAtt(cfnOutputNodeGroupInstanceRoleARN, "NodeInstanceRole.Arn", true)
n.rs.newOutputFromAtt(CfnOutputNodeGroupInstanceRoleARN, "NodeInstanceRole.Arn", true)
}
6 changes: 3 additions & 3 deletions pkg/cfn/builder/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (n *NodeGroupResourceSet) AddAllResources() error {
n.spec.AMIFamily, n.spec.AllowSSH, n.spec.SubnetTopology(),
templateDescriptionSuffix)

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

userData, err := nodebootstrap.NewUserData(n.clusterSpec, n.spec)
if err != nil {
Expand Down Expand Up @@ -142,7 +142,7 @@ func (n *NodeGroupResourceSet) addResourcesForNodeGroup() error {
vpcZoneIdentifier = map[string][]interface{}{
gfn.FnSplit: []interface{}{
",",
makeImportValue(n.clusterStackName, cfnOutputClusterSubnets+string(n.spec.SubnetTopology())),
makeImportValue(n.clusterStackName, CfnOutputClusterSubnets+string(n.spec.SubnetTopology())),
},
}
}
Expand Down Expand Up @@ -199,7 +199,7 @@ func (n *NodeGroupResourceSet) GetAllOutputs(stack cfn.Stack) error {
return err
}

n.spec.IAM.InstanceRoleARN = n.outputs[cfnOutputNodeGroupInstanceRoleARN]
n.spec.IAM.InstanceRoleARN = n.outputs[CfnOutputNodeGroupInstanceRoleARN]

return nil
}
21 changes: 11 additions & 10 deletions pkg/cfn/builder/outputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,22 @@ import (
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha3"
)

// Stack output names
const (
// outputs from cluster stack
cfnOutputClusterVPC = "VPC"
cfnOutputClusterSecurityGroup = "SecurityGroup"
cfnOutputClusterSubnets = "Subnets"
cfnOutputClusterSubnetsPrivate = string(cfnOutputClusterSubnets + api.SubnetTopologyPrivate)
cfnOutputClusterSubnetsPublic = string(cfnOutputClusterSubnets + api.SubnetTopologyPublic)
CfnOutputClusterVPC = "VPC"
CfnOutputClusterSecurityGroup = "SecurityGroup"
CfnOutputClusterSubnets = "Subnets"
CfnOutputClusterSubnetsPrivate = string(CfnOutputClusterSubnets + api.SubnetTopologyPrivate)
CfnOutputClusterSubnetsPublic = string(CfnOutputClusterSubnets + api.SubnetTopologyPublic)

cfnOutputClusterCertificateAuthorityData = "CertificateAuthorityData"
cfnOutputClusterEndpoint = "Endpoint"
cfnOutputClusterARN = "ARN"
cfnOutputClusterStackName = "ClusterStackName"
CfnOutputClusterCertificateAuthorityData = "CertificateAuthorityData"
CfnOutputClusterEndpoint = "Endpoint"
CfnOutputClusterARN = "ARN"
CfnOutputClusterStackName = "ClusterStackName"

// outputs from nodegroup stack
cfnOutputNodeGroupInstanceRoleARN = "InstanceRoleARN"
CfnOutputNodeGroupInstanceRoleARN = "InstanceRoleARN"
)

// newOutput defines a new output and optionally exports it
Expand Down
10 changes: 5 additions & 5 deletions pkg/cfn/builder/vpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ func (c *ClusterResourceSet) importResourcesForVPC() {
}

func (c *ClusterResourceSet) addOutputsForVPC() {
c.rs.newOutput(cfnOutputClusterVPC, c.vpc, true)
c.rs.newOutput(CfnOutputClusterVPC, c.vpc, true)
for topology := range c.spec.VPC.Subnets {
c.rs.newJoinedOutput(cfnOutputClusterSubnets+string(topology), c.subnets[topology], true)
c.rs.newJoinedOutput(CfnOutputClusterSubnets+string(topology), c.subnets[topology], true)
}
}

Expand All @@ -106,7 +106,7 @@ func (c *ClusterResourceSet) addResourcesForSecurityGroups() {
VpcId: c.vpc,
})
c.securityGroups = []*gfn.Value{refSG}
c.rs.newJoinedOutput(cfnOutputClusterSecurityGroup, c.securityGroups, true)
c.rs.newJoinedOutput(CfnOutputClusterSecurityGroup, c.securityGroups, true)
}

func (n *NodeGroupResourceSet) addResourcesForSecurityGroups() {
Expand All @@ -127,9 +127,9 @@ func (n *NodeGroupResourceSet) addResourcesForSecurityGroups() {
nodeMaxPort = gfn.NewInteger(65535)
)

refCP := makeImportValue(n.clusterStackName, cfnOutputClusterSecurityGroup)
refCP := makeImportValue(n.clusterStackName, CfnOutputClusterSecurityGroup)
refSG := n.newResource("SG", &gfn.AWSEC2SecurityGroup{
VpcId: makeImportValue(n.clusterStackName, cfnOutputClusterVPC),
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.Metadata.Name),
Expand Down
2 changes: 1 addition & 1 deletion pkg/cfn/manager/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (c *StackCollection) BlockingWaitDeleteStack(name string) error {
}

func fmtStacksRegexForCluster(name string) string {
const ourStackRegexFmt = "^(eksctl|EKS)-%s-((cluster|nodegroup-.+)|(VPC|ServiceRole|DefaultNodeGroup))$"
const ourStackRegexFmt = "^(eksctl|EKS)-%s-((cluster|nodegroup-.+)|(VPC|ServiceRole|ControlPlane|DefaultNodeGroup))$"
return fmt.Sprintf(ourStackRegexFmt, name)
}

Expand Down
37 changes: 37 additions & 0 deletions pkg/cfn/manager/cluster.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package manager

import (
"strings"

cfn "github.com/aws/aws-sdk-go/service/cloudformation"
"github.com/kris-nova/logger"
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha3"
"github.com/weaveworks/eksctl/pkg/cfn/builder"
)

Expand All @@ -22,6 +26,24 @@ func (c *StackCollection) CreateCluster(errs chan error, _ interface{}) error {
return c.CreateStack(name, stack, nil, nil, errs)
}

// DescribeClusterStack calls DescribeStacks and filters out cluster stack
func (c *StackCollection) DescribeClusterStack() (*Stack, error) {
stacks, err := c.DescribeStacks()
if err != nil {
return nil, err
}

for _, s := range stacks {
if *s.StackStatus == cfn.StackStatusDeleteComplete {
continue
}
if getClusterName(s) != "" {
return s, nil
}
}
return nil, nil
}

// DeleteCluster deletes the cluster
func (c *StackCollection) DeleteCluster() error {
_, err := c.DeleteStack(c.makeClusterStackName())
Expand All @@ -32,3 +54,18 @@ func (c *StackCollection) DeleteCluster() error {
func (c *StackCollection) WaitDeleteCluster() error {
return c.BlockingWaitDeleteStack(c.makeClusterStackName())
}

func getClusterName(s *Stack) string {
for _, tag := range s.Tags {
if *tag.Key == api.ClusterNameTag {
if strings.HasSuffix(*s.StackName, "-cluster") {
return *tag.Value
}
}
}

if strings.HasPrefix(*s.StackName, "EKS-") && strings.HasSuffix(*s.StackName, "-ControlPlane") {
return strings.TrimPrefix("EKS-", strings.TrimSuffix(*s.StackName, "-ControlPlane"))
}
return ""
}
9 changes: 0 additions & 9 deletions pkg/cfn/manager/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,12 +249,3 @@ func getNodeGroupName(s *Stack) string {
}
return ""
}

func getClusterName(s *Stack) string {
for _, tag := range s.Tags {
if *tag.Key == api.ClusterNameTag {
return *tag.Value
}
}
return ""
}
37 changes: 28 additions & 9 deletions pkg/eks/eks.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import (
"strings"
"time"

"github.com/weaveworks/eksctl/pkg/cfn/builder"
"github.com/weaveworks/eksctl/pkg/vpc"

awseks "github.com/aws/aws-sdk-go/service/eks"
"github.com/kris-nova/logger"
"github.com/pkg/errors"
api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha3"
"github.com/weaveworks/eksctl/pkg/printers"
"github.com/weaveworks/eksctl/pkg/vpc"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/client-go/kubernetes"
)
Expand Down Expand Up @@ -86,25 +88,42 @@ func (c *ClusterProvider) GetCredentials(spec *api.ClusterConfig) error {

// GetClusterVPC retrieves the VPC configuration
func (c *ClusterProvider) GetClusterVPC(spec *api.ClusterConfig) error {
// Check the cluster exists and is active
cluster, err := c.DescribeControlPlaneMustBeActive(spec.Metadata)
cluster, err := c.NewStackManager(spec).DescribeClusterStack()
if err != nil {
return err
}
logger.Debug("cluster = %#v", cluster)

outputs := map[string]string{}
for _, x := range cluster.Outputs {
outputs[*x.OutputKey] = *x.OutputValue
}

if spec.VPC == nil {
spec.VPC = &api.ClusterVPC{}
}

if err := vpc.ImportVPC(c.Provider, spec, *cluster.ResourcesVpcConfig.VpcId); err != nil {
return err
requiredKeyErrFmt := "cluster stack has no output key %q"

if vpc, ok := outputs[builder.CfnOutputClusterVPC]; ok {
spec.VPC.ID = vpc
} else {
return fmt.Errorf(requiredKeyErrFmt, builder.CfnOutputClusterVPC)
}

if numSGs := len(cluster.ResourcesVpcConfig.SecurityGroupIds); numSGs == 1 {
spec.VPC.SecurityGroup = *cluster.ResourcesVpcConfig.SecurityGroupIds[0]
if securityGroup, ok := outputs[builder.CfnOutputClusterSecurityGroup]; ok {
spec.VPC.SecurityGroup = securityGroup
} else {
return fmt.Errorf("cluster %q has %d security groups, expected 1", spec.Metadata.Name, numSGs)
return fmt.Errorf(requiredKeyErrFmt, builder.CfnOutputClusterSecurityGroup)
}

for _, topology := range api.SubnetTopologies() {
// either of subnet topologies are optional
if subnets, ok := outputs[builder.CfnOutputClusterSubnets+string(topology)]; ok {
subnets := strings.Split(subnets, ",")
if err := vpc.UseSubnets(c.Provider, spec, topology, subnets); err != nil {
return err
}
}
}

return nil
Expand Down

0 comments on commit 7d763b8

Please sign in to comment.