Skip to content

Commit

Permalink
Improve labels and tags
Browse files Browse the repository at this point in the history
- fix bug where tags slice was shared accidentially
- move tag constants to api package
- automatically set node labels for cluster and nodegroup names
- ensure labels have max 1 '/' separator, otherwise kubelet fails to start
  • Loading branch information
errordeveloper committed Jan 6, 2019
1 parent d25af70 commit 3651823
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 32 deletions.
14 changes: 14 additions & 0 deletions pkg/apis/eksctl.io/v1alpha3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ const (

// DefaultNodeType is the default instance type to use for nodes
DefaultNodeType = "m5.large"

// ClusterNameTag defines the tag of the clsuter name
ClusterNameTag = "eksctl.cluster.k8s.io/v1alpha1/cluster-name"

// NodeGroupNameTag defines the tag of the node group name
NodeGroupNameTag = "eksctl.io/v1alpha2/nodegroup-name"
// OldNodeGroupIDTag defines the old version of tag of the node group name
OldNodeGroupIDTag = "eksctl.cluster.k8s.io/v1alpha1/nodegroup-id"

// ClusterNameLabel defines the tag of the clsuter name
ClusterNameLabel = "alpha.eksctl.io/cluster-name"

// NodeGroupNameLabel defines the label of the node group name
NodeGroupNameLabel = "alpha.eksctl.io/nodegroup-name"
)

// SupportedRegions are the regions where EKS is available
Expand Down
41 changes: 19 additions & 22 deletions pkg/cfn/manager/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,6 @@ import (
"github.com/weaveworks/eksctl/pkg/cfn/builder"
)

const (
// ClusterNameTag defines the tag of the clsuter name
ClusterNameTag = "eksctl.cluster.k8s.io/v1alpha1/cluster-name"

// NodeGroupNameTag defines the tag of the node group name
NodeGroupNameTag = "eksctl.io/v1alpha2/nodegroup-name"
oldNodeGroupIDTag = "eksctl.cluster.k8s.io/v1alpha1/nodegroup-id"
)

var (
stackCapabilitiesIAM = aws.StringSlice([]string{cloudformation.CapabilityCapabilityIam})
)
Expand All @@ -34,9 +25,9 @@ type ChangeSet = cloudformation.DescribeChangeSetOutput

// StackCollection stores the CloudFormation stack information
type StackCollection struct {
provider api.ClusterProvider
spec *api.ClusterConfig
tags []*cloudformation.Tag
provider api.ClusterProvider
spec *api.ClusterConfig
sharedTags []*cloudformation.Tag
}

func newTag(key, value string) *cloudformation.Tag {
Expand All @@ -46,25 +37,31 @@ 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.Metadata.Name),
newTag(api.ClusterNameTag, spec.Metadata.Name),
}
for key, value := range spec.Metadata.Tags {
tags = append(tags, newTag(key, value))
}
logger.Debug("tags = %#v", tags)
return &StackCollection{
provider: provider,
spec: spec,
tags: tags,
provider: provider,
spec: spec,
sharedTags: tags,
}
}

func (c *StackCollection) doCreateStackRequest(i *Stack, templateBody []byte, parameters map[string]string, withIAM bool) error {
func (c *StackCollection) doCreateStackRequest(i *Stack, templateBody []byte, tags, parameters map[string]string, withIAM bool) error {
input := &cloudformation.CreateStackInput{
StackName: i.StackName,
}

input.SetTags(c.tags)
for _, t := range c.sharedTags {
input.Tags = append(input.Tags, t)
}
for k, v := range tags {
input.Tags = append(input.Tags, newTag(k, v))
}

input.SetTemplateBody(string(templateBody))

if withIAM {
Expand Down Expand Up @@ -97,15 +94,15 @@ func (c *StackCollection) doCreateStackRequest(i *Stack, templateBody []byte, pa
// any errors will be written to errs channel, when nil is written,
// assume completion, do not expect more then one error value on the
// channel, it's closed immediately after it is written to
func (c *StackCollection) CreateStack(name string, stack builder.ResourceSet, parameters map[string]string, errs chan error) error {
func (c *StackCollection) CreateStack(name string, stack builder.ResourceSet, tags, parameters map[string]string, errs chan error) error {
i := &Stack{StackName: &name}
templateBody, err := stack.RenderJSON()
if err != nil {
return errors.Wrapf(err, "rendering template for %q stack", *i.StackName)
}
logger.Debug("templateBody = %s", string(templateBody))

if err := c.doCreateStackRequest(i, templateBody, parameters, stack.WithIAM()); err != nil {
if err := c.doCreateStackRequest(i, templateBody, tags, parameters, stack.WithIAM()); err != nil {
return err
}

Expand Down Expand Up @@ -205,7 +202,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.Metadata.Name {
if *tag.Key == api.ClusterNameTag && *tag.Value == c.spec.Metadata.Name {
input := &cloudformation.DeleteStackInput{
StackName: i.StackId,
}
Expand All @@ -223,7 +220,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.Metadata.Name))
fmt.Sprintf("%s:%s", api.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
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func (c *StackCollection) CreateCluster(errs chan error, _ interface{}) error {
return err
}

return c.CreateStack(name, stack, nil, errs)
return c.CreateStack(name, stack, c.spec.Metadata.Tags, nil, errs)
}

// DeleteCluster deletes the cluster
Expand Down
15 changes: 7 additions & 8 deletions pkg/cfn/manager/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,12 @@ func (c *StackCollection) CreateNodeGroup(errs chan error, data interface{}) err
return err
}

c.tags = append(c.tags, newTag(NodeGroupNameTag, fmt.Sprintf("%s", ng.Name)))

for k, v := range ng.Tags {
c.tags = append(c.tags, newTag(k, v))
if ng.Tags == nil {
ng.Tags = make(map[string]string)
}
ng.Tags[api.NodeGroupNameTag] = ng.Name

return c.CreateStack(name, stack, nil, errs)
return c.CreateStack(name, stack, ng.Tags, nil, errs)
}

// DescribeNodeGroupStacks calls DescribeStacks and filters out nodegroups
Expand Down Expand Up @@ -210,10 +209,10 @@ func (c *StackCollection) mapStackToNodeGroupSummary(stack *Stack) (*NodeGroupSu

func getNodeGroupName(s *Stack) string {
for _, tag := range s.Tags {
if *tag.Key == NodeGroupNameTag {
if *tag.Key == api.NodeGroupNameTag {
return *tag.Value
}
if *tag.Key == oldNodeGroupIDTag {
if *tag.Key == api.OldNodeGroupIDTag {
return *tag.Value
}
}
Expand All @@ -225,7 +224,7 @@ func getNodeGroupName(s *Stack) string {

func getClusterName(s *Stack) string {
for _, tag := range s.Tags {
if *tag.Key == ClusterNameTag {
if *tag.Key == api.ClusterNameTag {
return *tag.Value
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cfn/manager/nodegroup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ var _ = Describe("StackCollection NodeGroup", func() {
StackStatus: aws.String("CREATE_COMPLETE"),
Tags: []*cfn.Tag{
&cfn.Tag{
Key: aws.String(NodeGroupNameTag),
Key: aws.String(api.NodeGroupNameTag),
Value: aws.String("12345"),
},
},
Expand Down
4 changes: 4 additions & 0 deletions pkg/ctl/create/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,10 @@ func doCreateCluster(p *api.ProviderConfig, cfg *api.ClusterConfig, nameArg stri
}
logger.Info("nodegroup %q will use %q [%s/%s]", ng.Name, ng.AMI, ng.AMIFamily, cfg.Metadata.Version)

if err := ctl.SetNodeLabels(ng, meta); err != nil {
return err
}

// load or use SSH key - name includes cluster name and the
// fingerprint, so if unique keys provided, each will get
// loaded and used as intended and there is no need to have
Expand Down
5 changes: 5 additions & 0 deletions pkg/ctl/create/nodegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ func doCreateNodeGroup(p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.No
if err := ctl.EnsureAMI(meta.Version, ng); err != nil {
return err
}
logger.Info("nodegroup %q will use %q [%s/%s]", ng.Name, ng.AMI, ng.AMIFamily, cfg.Metadata.Version)

if err := ctl.SetNodeLabels(ng, meta); err != nil {
return err
}

if err := ctl.LoadSSHPublicKey(cfg.Metadata.Name, ng); err != nil {
return err
Expand Down
18 changes: 18 additions & 0 deletions pkg/eks/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"io/ioutil"
"os"
"strings"
"time"

"github.com/weaveworks/eksctl/pkg/ami"
Expand Down Expand Up @@ -223,6 +224,23 @@ func (c *ClusterProvider) EnsureAMI(version string, ng *api.NodeGroup) error {
return nil
}

// SetNodeLabels initialises and validate node labels based on cluster and nodegroup names
func (c *ClusterProvider) SetNodeLabels(ng *api.NodeGroup, meta *api.ClusterMeta) error {
if ng.Labels == nil {
ng.Labels = make(api.NodeLabels)
}

ng.Labels[api.ClusterNameLabel] = meta.Name
ng.Labels[api.NodeGroupNameLabel] = ng.Name

for l := range ng.Labels {
if len(strings.Split(l, "/")) > 2 {
return fmt.Errorf("node label key %q is of invalid format, can only use one '/' separator", l)
}
}
return nil
}

// SetAvailabilityZones sets the given (or chooses) the availability zones
func (c *ClusterProvider) SetAvailabilityZones(spec *api.ClusterConfig, given []string) error {
if len(given) == 0 {
Expand Down

0 comments on commit 3651823

Please sign in to comment.