From 0284bc3388ea737fb4a98effa89df792d8f126a1 Mon Sep 17 00:00:00 2001 From: Nicolas Sadin <5952290+SlevinWasAlreadyTaken@users.noreply.github.com> Date: Mon, 25 Apr 2022 10:17:19 +0200 Subject: [PATCH] Added Propagation of ManagedNodeGroup Tags to their corresponding AutoScalingGroups (#5002) * Added Propagation of ManagedNodeGroup Tags to their AutoScalingGroups Changes to ensure that AutoScalingGroups Tags are the same as their ManagedNodeGroup. All tags are copied from the ManagedNodeGroup to the AutoScalingGroup. If the tags already exists, it is overridden. This is the default behaviour (as it is for Unmanaged NodeGroup) and can be enabled using propagateASGTags boolean configuration. Issue #1571 * Update generated files * fix existing unit tests * update documentation * Update generated files * consider new way of configuring tags propagation after the rebase * re-add disableASGTagPropagation doc for managed nodegroup * consider tags limits, move logic to a more appropriated place, re-add test for managernodegroup * fix usage of aws-sdk-go-v2, improve task parallelisation for managednodegroup creation * fix test linting * add unit tests * improve unit test structure for PropagateManagedNodeGroupTagsToASG, rename astypes package to asTypes * use const for auto-scaling-group string * improve some code structure --- .../eksctl.io/v1alpha5/assets/schema.json | 21 ++-- pkg/apis/eksctl.io/v1alpha5/types.go | 13 +- .../v1alpha5/zz_generated.deepcopy.go | 10 +- pkg/cfn/builder/nodegroup.go | 1 + pkg/cfn/manager/api.go | 99 +++++++++++++++- pkg/cfn/manager/api_test.go | 111 ++++++++++++++++++ pkg/cfn/manager/create_tasks.go | 10 ++ pkg/cfn/manager/fakes/fake_stack_manager.go | 85 ++++++++++++++ pkg/cfn/manager/interface.go | 1 + pkg/cfn/manager/nodegroup.go | 24 ++++ pkg/cfn/manager/tasks.go | 12 ++ pkg/cfn/manager/tasks_test.go | 27 +++++ pkg/eks/eks_test.go | 7 -- userdocs/src/usage/autoscaling.md | 2 +- userdocs/src/usage/eks-managed-nodes.md | 2 - 15 files changed, 392 insertions(+), 33 deletions(-) diff --git a/pkg/apis/eksctl.io/v1alpha5/assets/schema.json b/pkg/apis/eksctl.io/v1alpha5/assets/schema.json index 5e3c9e1850..4dd06abb20 100755 --- a/pkg/apis/eksctl.io/v1alpha5/assets/schema.json +++ b/pkg/apis/eksctl.io/v1alpha5/assets/schema.json @@ -1076,6 +1076,11 @@ "x-intellij-html-description": "Enable private networking for nodegroup", "default": "false" }, + "propagateASGTags": { + "type": "boolean", + "description": "Propagate all taints and labels to the ASG automatically.", + "x-intellij-html-description": "Propagate all taints and labels to the ASG automatically." + }, "releaseVersion": { "type": "string", "description": "the AMI version of the EKS optimized AMI to use", @@ -1108,8 +1113,8 @@ "type": "string" }, "type": "object", - "description": "Applied to the Autoscaling Group and to the EC2 instances (unmanaged), Applied to the EKS Nodegroup resource and to the EC2 instances (managed)", - "x-intellij-html-description": "Applied to the Autoscaling Group and to the EC2 instances (unmanaged), Applied to the EKS Nodegroup resource and to the EC2 instances (managed)", + "description": "Applied to the Autoscaling Group and to the EC2 instances (unmanaged), Applied to the Autoscaling Group, the EKS Nodegroup resource and to the EC2 instances (managed)", + "x-intellij-html-description": "Applied to the Autoscaling Group and to the EC2 instances (unmanaged), Applied to the Autoscaling Group, the EKS Nodegroup resource and to the EC2 instances (managed)", "default": "{}" }, "taints": { @@ -1191,6 +1196,7 @@ "additionalVolumes", "preBootstrapCommands", "overrideBootstrapCommand", + "propagateASGTags", "disableIMDSv1", "disablePodIMDS", "placement", @@ -1321,8 +1327,9 @@ }, "disableASGTagPropagation": { "type": "boolean", - "description": "disable the tag propagation in case desired capacity is 0.", - "x-intellij-html-description": "disable the tag propagation in case desired capacity is 0." + "description": "disables the tag propagation to ASG in case desired capacity is 0.", + "x-intellij-html-description": "disables the tag propagation to ASG in case desired capacity is 0.", + "default": false }, "disableIMDSv1": { "type": "boolean", @@ -1450,8 +1457,8 @@ "type": "string" }, "type": "object", - "description": "Applied to the Autoscaling Group and to the EC2 instances (unmanaged), Applied to the EKS Nodegroup resource and to the EC2 instances (managed)", - "x-intellij-html-description": "Applied to the Autoscaling Group and to the EC2 instances (unmanaged), Applied to the EKS Nodegroup resource and to the EC2 instances (managed)", + "description": "Applied to the Autoscaling Group and to the EC2 instances (unmanaged), Applied to the Autoscaling Group, the EKS Nodegroup resource and to the EC2 instances (managed)", + "x-intellij-html-description": "Applied to the Autoscaling Group and to the EC2 instances (unmanaged), Applied to the Autoscaling Group, the EKS Nodegroup resource and to the EC2 instances (managed)", "default": "{}" }, "taints": { @@ -1538,6 +1545,7 @@ "additionalVolumes", "preBootstrapCommands", "overrideBootstrapCommand", + "propagateASGTags", "disableIMDSv1", "disablePodIMDS", "placement", @@ -1555,7 +1563,6 @@ "clusterDNS", "kubeletExtraConfig", "containerRuntime", - "propagateASGTags", "disableASGTagPropagation", "maxInstanceLifetime" ], diff --git a/pkg/apis/eksctl.io/v1alpha5/types.go b/pkg/apis/eksctl.io/v1alpha5/types.go index a5be4c92cf..904fa9c4a9 100644 --- a/pkg/apis/eksctl.io/v1alpha5/types.go +++ b/pkg/apis/eksctl.io/v1alpha5/types.go @@ -997,11 +997,8 @@ type NodeGroup struct { // +optional ContainerRuntime *string `json:"containerRuntime,omitempty"` - // Propagate all taints and labels to the ASG automatically. - // +optional - PropagateASGTags *bool `json:"propagateASGTags,omitempty"` - - // DisableASGTagPropagation disable the tag propagation in case desired capacity is 0. + // DisableASGTagPropagation disables the tag propagation to ASG in case desired capacity is 0. + // Defaults to `false` // +optional DisableASGTagPropagation *bool `json:"disableASGTagPropagation,omitempty"` @@ -1315,7 +1312,7 @@ type NodeGroupBase struct { // +optional PrivateNetworking bool `json:"privateNetworking"` // Applied to the Autoscaling Group and to the EC2 instances (unmanaged), - // Applied to the EKS Nodegroup resource and to the EC2 instances (managed) + // Applied to the Autoscaling Group, the EKS Nodegroup resource and to the EC2 instances (managed) // +optional Tags map[string]string `json:"tags,omitempty"` // +optional @@ -1368,6 +1365,10 @@ type NodeGroupBase struct { // +optional OverrideBootstrapCommand *string `json:"overrideBootstrapCommand,omitempty"` + // Propagate all taints and labels to the ASG automatically. + // +optional + PropagateASGTags *bool `json:"propagateASGTags,omitempty"` + // DisableIMDSv1 requires requests to the metadata service to use IMDSv2 tokens // Defaults to `false` // +optional diff --git a/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go b/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go index 189c9963b5..037d12ff44 100644 --- a/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go +++ b/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go @@ -1015,11 +1015,6 @@ func (in *NodeGroup) DeepCopyInto(out *NodeGroup) { *out = new(string) **out = **in } - if in.PropagateASGTags != nil { - in, out := &in.PropagateASGTags, &out.PropagateASGTags - *out = new(bool) - **out = **in - } if in.DisableASGTagPropagation != nil { in, out := &in.DisableASGTagPropagation, &out.DisableASGTagPropagation *out = new(bool) @@ -1156,6 +1151,11 @@ func (in *NodeGroupBase) DeepCopyInto(out *NodeGroupBase) { *out = new(string) **out = **in } + if in.PropagateASGTags != nil { + in, out := &in.PropagateASGTags, &out.PropagateASGTags + *out = new(bool) + **out = **in + } if in.DisableIMDSv1 != nil { in, out := &in.DisableIMDSv1, &out.DisableIMDSv1 *out = new(bool) diff --git a/pkg/cfn/builder/nodegroup.go b/pkg/cfn/builder/nodegroup.go index d0327913c9..9bc8eb3bd0 100644 --- a/pkg/cfn/builder/nodegroup.go +++ b/pkg/cfn/builder/nodegroup.go @@ -23,6 +23,7 @@ import ( // MaximumTagNumber for ASGs as described here https://docs.aws.amazon.com/autoscaling/ec2/userguide/autoscaling-tagging.html const MaximumTagNumber = 50 +const MaximumCreatedTagNumberPerCall = 25 // NodeGroupResourceSet stores the resource information of the nodegroup type NodeGroupResourceSet struct { diff --git a/pkg/cfn/manager/api.go b/pkg/cfn/manager/api.go index da9dd2b626..59b90fca2f 100644 --- a/pkg/cfn/manager/api.go +++ b/pkg/cfn/manager/api.go @@ -7,6 +7,8 @@ import ( "strings" "time" + "github.com/aws/aws-sdk-go-v2/service/autoscaling" + asTypes "github.com/aws/aws-sdk-go-v2/service/autoscaling/types" "github.com/aws/aws-sdk-go-v2/service/cloudformation" "github.com/aws/aws-sdk-go-v2/service/cloudformation/types" "github.com/aws/aws-sdk-go-v2/service/cloudtrail" @@ -26,11 +28,12 @@ import ( ) const ( - resourcesRootPath = "Resources" - outputsRootPath = "Outputs" - mappingsRootPath = "Mappings" - ourStackRegexFmt = "^(eksctl|EKS)-%s-((cluster|nodegroup-.+|addon-.+|fargate|karpenter)|(VPC|ServiceRole|ControlPlane|DefaultNodeGroup))$" - clusterStackRegex = "eksctl-.*-cluster" + resourcesRootPath = "Resources" + resourceTypeAutoScalingGroup = "auto-scaling-group" + outputsRootPath = "Outputs" + mappingsRootPath = "Mappings" + ourStackRegexFmt = "^(eksctl|EKS)-%s-((cluster|nodegroup-.+|addon-.+|fargate|karpenter)|(VPC|ServiceRole|ControlPlane|DefaultNodeGroup))$" + clusterStackRegex = "eksctl-.*-cluster" ) var ( @@ -238,6 +241,92 @@ func (c *StackCollection) createStackRequest(ctx context.Context, stackName stri return stack, nil } +func (c *StackCollection) PropagateManagedNodeGroupTagsToASG(ngName string, ngTags map[string]string, asgNames []string, errCh chan error) error { + go func() { + defer close(errCh) + // build the input tags for all ASGs attached to the managed nodegroup + asgTags := []asTypes.Tag{} + + for _, asgName := range asgNames { + // skip directly if not tags are required to be created + if len(ngTags) == 0 { + continue + } + + // check if the number of tags on the ASG would go over the defined limit + if err := c.checkASGTagsNumber(ngName, asgName, ngTags); err != nil { + errCh <- err + return + } + // build the list of tags to attach to the ASG + for ngTagKey, ngTagValue := range ngTags { + asgTag := asTypes.Tag{ + ResourceId: aws.String(asgName), + ResourceType: aws.String(resourceTypeAutoScalingGroup), + Key: aws.String(ngTagKey), + Value: aws.String(ngTagValue), + PropagateAtLaunch: aws.Bool(false), + } + asgTags = append(asgTags, asgTag) + } + } + + // consider the maximum number of tags we can create at once... + var chunkedASGTags [][]asTypes.Tag + chunkSize := builder.MaximumCreatedTagNumberPerCall + for start := 0; start < len(asgTags); start += chunkSize { + end := start + chunkSize + if end > len(asgTags) { + end = len(asgTags) + } + chunkedASGTags = append(chunkedASGTags, asgTags[start:end]) + } + // ...then create all of them in a loop + for _, asgTags := range chunkedASGTags { + input := &autoscaling.CreateOrUpdateTagsInput{Tags: asgTags} + if _, err := c.asgAPI.CreateOrUpdateTags(context.Background(), input); err != nil { + errCh <- errors.Wrapf(err, "creating or updating asg tags for managed nodegroup %q", ngName) + return + } + } + errCh <- nil + }() + return nil +} + +// checkASGTagsNumber limit considering the new propagated tags +func (c *StackCollection) checkASGTagsNumber(ngName, asgName string, propagatedTags map[string]string) error { + tagsFilter := &autoscaling.DescribeTagsInput{ + Filters: []asTypes.Filter{ + { + Name: aws.String(resourceTypeAutoScalingGroup), + Values: []string{asgName}, + }, + }, + } + output, err := c.asgAPI.DescribeTags(context.Background(), tagsFilter) + if err != nil { + return errors.Wrapf(err, "describing asg %q tags for managed nodegroup %q", asgName, ngName) + } + asgTags := output.Tags + // intersection of key tags to consider the number of tags going + // to be attached to the ASG + uniqueTagKeyCount := len(asgTags) + len(propagatedTags) + for ngTagKey := range propagatedTags { + for _, asgTag := range asgTags { + // decrease the unique tag key count if there is a match + if aws.StringValue(asgTag.Key) == ngTagKey { + uniqueTagKeyCount-- + break + } + } + } + if uniqueTagKeyCount > builder.MaximumTagNumber { + return fmt.Errorf("number of tags is exceeding the maximum amount for asg %d, was: %d", builder.MaximumTagNumber, uniqueTagKeyCount) + } + return nil +} + // UpdateStack will update a CloudFormation stack by creating and executing a ChangeSet func (c *StackCollection) UpdateStack(ctx context.Context, options UpdateStackOptions) error { logger.Info(options.Description) diff --git a/pkg/cfn/manager/api_test.go b/pkg/cfn/manager/api_test.go index 295f56b0fe..c096c73ac2 100644 --- a/pkg/cfn/manager/api_test.go +++ b/pkg/cfn/manager/api_test.go @@ -5,21 +5,132 @@ import ( "errors" "fmt" + "github.com/aws/aws-sdk-go-v2/service/autoscaling" + asTypes "github.com/aws/aws-sdk-go-v2/service/autoscaling/types" cfn "github.com/aws/aws-sdk-go-v2/service/cloudformation" "github.com/aws/aws-sdk-go-v2/service/cloudformation/types" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/awstesting" + . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" "github.com/stretchr/testify/mock" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" + "github.com/weaveworks/eksctl/pkg/cfn/builder" "github.com/weaveworks/eksctl/pkg/testutils/mockprovider" ) var _ = Describe("StackCollection", func() { + Context("PropagateManagedNodeGroupTagsToASG", func() { + var ( + asgName string + ngName string + ngTags map[string]string + errCh chan error + p *mockprovider.MockProvider + ) + BeforeEach(func() { + asgName = "asg-test-name" + ngName = "ng-test-name" + ngTags = map[string]string{ + "tag_key_1": "tag_value_1", + } + errCh = make(chan error) + p = mockprovider.NewMockProvider() + }) + + It("can create propagate tag", func() { + // DescribeTags classic mock + describeTagsInput := &autoscaling.DescribeTagsInput{ + Filters: []asTypes.Filter{{Name: aws.String(resourceTypeAutoScalingGroup), Values: []string{asgName}}}, + } + p.MockASG().On("DescribeTags", mock.Anything, describeTagsInput).Return(&autoscaling.DescribeTagsOutput{}, nil) + + // CreateOrUpdateTags classic mock + createOrUpdateTagsInput := &autoscaling.CreateOrUpdateTagsInput{ + Tags: []asTypes.Tag{ + { + ResourceId: aws.String(asgName), + ResourceType: aws.String(resourceTypeAutoScalingGroup), + Key: aws.String("tag_key_1"), + Value: aws.String("tag_value_1"), + PropagateAtLaunch: aws.Bool(false), + }, + }, + } + p.MockASG().On("CreateOrUpdateTags", mock.Anything, createOrUpdateTagsInput).Return(&autoscaling.CreateOrUpdateTagsOutput{}, nil) + + sm := NewStackCollection(p, api.NewClusterConfig()) + err := sm.PropagateManagedNodeGroupTagsToASG(ngName, ngTags, []string{asgName}, errCh) + Expect(err).NotTo(HaveOccurred()) + err = <-errCh + Expect(err).NotTo(HaveOccurred()) + }) + It("cannot propagate tags in chunks of 25", func() { + // populate the createOrUpdateTagsSliceInput for easier generation of chunks + createOrUpdateTagsSliceInput := []asTypes.Tag{} + for i := 0; i < 30; i++ { + tagKey, tagValue := fmt.Sprintf("tag_key_%d", i), fmt.Sprintf("tag_value_%d", i) + ngTags[tagKey] = tagValue + createOrUpdateTagsSliceInput = append(createOrUpdateTagsSliceInput, asTypes.Tag{ + ResourceId: aws.String(asgName), + ResourceType: aws.String(resourceTypeAutoScalingGroup), + Key: aws.String(tagKey), + Value: aws.String(tagValue), + PropagateAtLaunch: aws.Bool(false), + }) + } + + // DescribeTags classic mock + describeTagsInput := &autoscaling.DescribeTagsInput{ + Filters: []asTypes.Filter{{Name: aws.String(resourceTypeAutoScalingGroup), Values: []string{asgName}}}, + } + p.MockASG().On("DescribeTags", mock.Anything, describeTagsInput).Return(&autoscaling.DescribeTagsOutput{}, nil) + + // CreateOrUpdateTags chunked mock + // generate the expected chunk of tags + chunkSize := builder.MaximumCreatedTagNumberPerCall + firstchunkLenMatcher := func(input *autoscaling.CreateOrUpdateTagsInput) bool { + return len(input.Tags) == len(createOrUpdateTagsSliceInput[:chunkSize]) + } + secondChunkLenMatcher := func(input *autoscaling.CreateOrUpdateTagsInput) bool { + return len(input.Tags) == len(createOrUpdateTagsSliceInput[chunkSize:]) + } + + // setup the call verification of the two chunks + // NOTE: because of the use of map (unordered processing), we just verify size of chunk + p.MockASG().On("CreateOrUpdateTags", mock.Anything, mock.MatchedBy(firstchunkLenMatcher)).Return(&autoscaling.CreateOrUpdateTagsOutput{}, nil) + p.MockASG().On("CreateOrUpdateTags", mock.Anything, mock.MatchedBy(secondChunkLenMatcher)).Return(&autoscaling.CreateOrUpdateTagsOutput{}, nil) + + sm := NewStackCollection(p, api.NewClusterConfig()) + err := sm.PropagateManagedNodeGroupTagsToASG(ngName, ngTags, []string{asgName}, errCh) + Expect(err).NotTo(HaveOccurred()) + err = <-errCh + Expect(err).NotTo(HaveOccurred()) + }) + It("cannot propagate if too many tags", func() { + // fill parameters + for i := 0; i < builder.MaximumTagNumber+1; i++ { + ngTags[fmt.Sprintf("tag_key_%d", i)] = fmt.Sprintf("tag_value_%d", i) + } + + // DescribeTags classic mock + describeTagsInput := &autoscaling.DescribeTagsInput{ + Filters: []asTypes.Filter{{Name: aws.String(resourceTypeAutoScalingGroup), Values: []string{asgName}}}, + } + p.MockASG().On("DescribeTags", mock.Anything, describeTagsInput).Return(&autoscaling.DescribeTagsOutput{}, nil) + + sm := NewStackCollection(p, api.NewClusterConfig()) + err := sm.PropagateManagedNodeGroupTagsToASG(ngName, ngTags, []string{asgName}, errCh) + Expect(err).NotTo(HaveOccurred()) + err = <-errCh + Expect(err).To(MatchError(ContainSubstring("maximum amount for asg"))) + }) + }) + Context("UpdateStack", func() { It("succeeds if no changes required", func() { // Order of AWS SDK invocation diff --git a/pkg/cfn/manager/create_tasks.go b/pkg/cfn/manager/create_tasks.go index 4366590e71..683ff8605f 100644 --- a/pkg/cfn/manager/create_tasks.go +++ b/pkg/cfn/manager/create_tasks.go @@ -106,6 +106,16 @@ func (c *StackCollection) NewManagedNodeGroupTask(ctx context.Context, nodeGroup info: fmt.Sprintf("create managed nodegroup %q", ng.Name), ctx: ctx, }) + if api.IsEnabled(ng.PropagateASGTags) { + // disable parallelisation if any tags propagation is done + // since nodegroup must be created to propagate tags to its ASGs + taskTree.Parallel = false + taskTree.Append(&managedNodeGroupTagsToASGPropagationTask{ + stackCollection: c, + nodeGroup: ng, + info: fmt.Sprintf("propagate tags to ASG for managed nodegroup %q", ng.Name), + }) + } } return taskTree } diff --git a/pkg/cfn/manager/fakes/fake_stack_manager.go b/pkg/cfn/manager/fakes/fake_stack_manager.go index 5a0212aa8b..5c8c40f3d6 100644 --- a/pkg/cfn/manager/fakes/fake_stack_manager.go +++ b/pkg/cfn/manager/fakes/fake_stack_manager.go @@ -725,6 +725,20 @@ type FakeStackManager struct { newUnmanagedNodeGroupTaskReturnsOnCall map[int]struct { result1 *tasks.TaskTree } + PropagateManagedNodeGroupTagsToASGStub func(string, map[string]string, []string, chan error) error + propagateManagedNodeGroupTagsToASGMutex sync.RWMutex + propagateManagedNodeGroupTagsToASGArgsForCall []struct { + arg1 string + arg2 map[string]string + arg3 []string + arg4 chan error + } + propagateManagedNodeGroupTagsToASGReturns struct { + result1 error + } + propagateManagedNodeGroupTagsToASGReturnsOnCall map[int]struct { + result1 error + } RefreshFargatePodExecutionRoleARNStub func(context.Context) error refreshFargatePodExecutionRoleARNMutex sync.RWMutex refreshFargatePodExecutionRoleARNArgsForCall []struct { @@ -4166,6 +4180,75 @@ func (fake *FakeStackManager) NewUnmanagedNodeGroupTaskReturnsOnCall(i int, resu }{result1} } +func (fake *FakeStackManager) PropagateManagedNodeGroupTagsToASG(arg1 string, arg2 map[string]string, arg3 []string, arg4 chan error) error { + var arg3Copy []string + if arg3 != nil { + arg3Copy = make([]string, len(arg3)) + copy(arg3Copy, arg3) + } + fake.propagateManagedNodeGroupTagsToASGMutex.Lock() + ret, specificReturn := fake.propagateManagedNodeGroupTagsToASGReturnsOnCall[len(fake.propagateManagedNodeGroupTagsToASGArgsForCall)] + fake.propagateManagedNodeGroupTagsToASGArgsForCall = append(fake.propagateManagedNodeGroupTagsToASGArgsForCall, struct { + arg1 string + arg2 map[string]string + arg3 []string + arg4 chan error + }{arg1, arg2, arg3Copy, arg4}) + stub := fake.PropagateManagedNodeGroupTagsToASGStub + fakeReturns := fake.propagateManagedNodeGroupTagsToASGReturns + fake.recordInvocation("PropagateManagedNodeGroupTagsToASG", []interface{}{arg1, arg2, arg3Copy, arg4}) + fake.propagateManagedNodeGroupTagsToASGMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3, arg4) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeStackManager) PropagateManagedNodeGroupTagsToASGCallCount() int { + fake.propagateManagedNodeGroupTagsToASGMutex.RLock() + defer fake.propagateManagedNodeGroupTagsToASGMutex.RUnlock() + return len(fake.propagateManagedNodeGroupTagsToASGArgsForCall) +} + +func (fake *FakeStackManager) PropagateManagedNodeGroupTagsToASGCalls(stub func(string, map[string]string, []string, chan error) error) { + fake.propagateManagedNodeGroupTagsToASGMutex.Lock() + defer fake.propagateManagedNodeGroupTagsToASGMutex.Unlock() + fake.PropagateManagedNodeGroupTagsToASGStub = stub +} + +func (fake *FakeStackManager) PropagateManagedNodeGroupTagsToASGArgsForCall(i int) (string, map[string]string, []string, chan error) { + fake.propagateManagedNodeGroupTagsToASGMutex.RLock() + defer fake.propagateManagedNodeGroupTagsToASGMutex.RUnlock() + argsForCall := fake.propagateManagedNodeGroupTagsToASGArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 +} + +func (fake *FakeStackManager) PropagateManagedNodeGroupTagsToASGReturns(result1 error) { + fake.propagateManagedNodeGroupTagsToASGMutex.Lock() + defer fake.propagateManagedNodeGroupTagsToASGMutex.Unlock() + fake.PropagateManagedNodeGroupTagsToASGStub = nil + fake.propagateManagedNodeGroupTagsToASGReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeStackManager) PropagateManagedNodeGroupTagsToASGReturnsOnCall(i int, result1 error) { + fake.propagateManagedNodeGroupTagsToASGMutex.Lock() + defer fake.propagateManagedNodeGroupTagsToASGMutex.Unlock() + fake.PropagateManagedNodeGroupTagsToASGStub = nil + if fake.propagateManagedNodeGroupTagsToASGReturnsOnCall == nil { + fake.propagateManagedNodeGroupTagsToASGReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.propagateManagedNodeGroupTagsToASGReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeStackManager) RefreshFargatePodExecutionRoleARN(arg1 context.Context) error { fake.refreshFargatePodExecutionRoleARNMutex.Lock() ret, specificReturn := fake.refreshFargatePodExecutionRoleARNReturnsOnCall[len(fake.refreshFargatePodExecutionRoleARNArgsForCall)] @@ -4582,6 +4665,8 @@ func (fake *FakeStackManager) Invocations() map[string][][]interface{} { defer fake.newTasksToDeleteOIDCProviderWithIAMServiceAccountsMutex.RUnlock() fake.newUnmanagedNodeGroupTaskMutex.RLock() defer fake.newUnmanagedNodeGroupTaskMutex.RUnlock() + fake.propagateManagedNodeGroupTagsToASGMutex.RLock() + defer fake.propagateManagedNodeGroupTagsToASGMutex.RUnlock() fake.refreshFargatePodExecutionRoleARNMutex.RLock() defer fake.refreshFargatePodExecutionRoleARNMutex.RUnlock() fake.stackStatusIsNotReadyMutex.RLock() diff --git a/pkg/cfn/manager/interface.go b/pkg/cfn/manager/interface.go index 70be901e33..e413f13f3f 100644 --- a/pkg/cfn/manager/interface.go +++ b/pkg/cfn/manager/interface.go @@ -91,6 +91,7 @@ type StackManager interface { NewTasksToDeleteNodeGroups(stacks []NodeGroupStack, shouldDelete func(_ string) bool, wait bool, cleanup func(chan error, string) error) (*tasks.TaskTree, error) NewTasksToDeleteOIDCProviderWithIAMServiceAccounts(ctx context.Context, oidc *iamoidc.OpenIDConnectManager, clientSetGetter kubernetes.ClientSetGetter) (*tasks.TaskTree, error) NewUnmanagedNodeGroupTask(ctx context.Context, nodeGroups []*v1alpha5.NodeGroup, forceAddCNIPolicy bool, importer vpc.Importer) *tasks.TaskTree + PropagateManagedNodeGroupTagsToASG(ngName string, ngTags map[string]string, asgNames []string, errCh chan error) error RefreshFargatePodExecutionRoleARN(ctx context.Context) error StackStatusIsNotReady(s *Stack) bool StackStatusIsNotTransitional(s *Stack) bool diff --git a/pkg/cfn/manager/nodegroup.go b/pkg/cfn/manager/nodegroup.go index 62de21706a..155581e219 100644 --- a/pkg/cfn/manager/nodegroup.go +++ b/pkg/cfn/manager/nodegroup.go @@ -78,6 +78,30 @@ func (c *StackCollection) createManagedNodeGroupTask(ctx context.Context, errorC return c.CreateStack(ctx, name, stack, ng.Tags, nil, errorCh) } +func (c *StackCollection) propagateManagedNodeGroupTagsToASGTask(errorCh chan error, ng *api.ManagedNodeGroup) error { + // describe node group to retrieve ASG names + input := &eks.DescribeNodegroupInput{ + ClusterName: aws.String(c.spec.Metadata.Name), + NodegroupName: aws.String(ng.Name), + } + res, err := c.eksAPI.DescribeNodegroup(input) + if err != nil { + return errors.Wrapf(err, "couldn't get managed nodegroup details for nodegroup %q", ng.Name) + } + + if res.Nodegroup.Resources == nil { + return nil + } + + asgNames := []string{} + for _, asg := range res.Nodegroup.Resources.AutoScalingGroups { + if asg.Name != nil && *asg.Name != "" { + asgNames = append(asgNames, *asg.Name) + } + } + return c.PropagateManagedNodeGroupTagsToASG(ng.Name, ng.Tags, asgNames, errorCh) +} + // DescribeNodeGroupStacks calls DescribeStacks and filters out nodegroups func (c *StackCollection) DescribeNodeGroupStacks(ctx context.Context) ([]*Stack, error) { stacks, err := c.DescribeStacks(ctx) diff --git a/pkg/cfn/manager/tasks.go b/pkg/cfn/manager/tasks.go index a2e95229fb..67129e598e 100644 --- a/pkg/cfn/manager/tasks.go +++ b/pkg/cfn/manager/tasks.go @@ -61,6 +61,18 @@ func (t *managedNodeGroupTask) Do(errorCh chan error) error { return t.stackCollection.createManagedNodeGroupTask(t.ctx, errorCh, t.nodeGroup, t.forceAddCNIPolicy, t.vpcImporter) } +type managedNodeGroupTagsToASGPropagationTask struct { + info string + nodeGroup *api.ManagedNodeGroup + stackCollection *StackCollection +} + +func (t *managedNodeGroupTagsToASGPropagationTask) Describe() string { return t.info } + +func (t *managedNodeGroupTagsToASGPropagationTask) Do(errorCh chan error) error { + return t.stackCollection.propagateManagedNodeGroupTagsToASGTask(errorCh, t.nodeGroup) +} + type clusterCompatTask struct { info string stackCollection *StackCollection diff --git a/pkg/cfn/manager/tasks_test.go b/pkg/cfn/manager/tasks_test.go index ce8df56237..b264bb86ec 100644 --- a/pkg/cfn/manager/tasks_test.go +++ b/pkg/cfn/manager/tasks_test.go @@ -124,6 +124,21 @@ var _ = Describe("StackCollection Tasks", func() { create managed nodegroup "m2", } } +`)) + } + { + tasks := stackManager.NewTasksToCreateClusterWithNodeGroups(context.Background(), makeNodeGroups("bar", "foo"), makeManagedNodeGroupsWithPropagatedTags("m1", "m2")) + Expect(tasks.Describe()).To(Equal(` +2 sequential tasks: { create cluster control plane "test-cluster", + 6 parallel sub-tasks: { + create nodegroup "bar", + create nodegroup "foo", + create managed nodegroup "m1", + propagate tags to ASG for managed nodegroup "m1", + create managed nodegroup "m2", + propagate tags to ASG for managed nodegroup "m2", + } +} `)) } { @@ -224,3 +239,15 @@ func makeManagedNodeGroups(names ...string) []*api.ManagedNodeGroup { } return managedNodeGroups } + +func makeManagedNodeGroupsWithPropagatedTags(names ...string) []*api.ManagedNodeGroup { + propagate := true + var managedNodeGroups []*api.ManagedNodeGroup + for _, name := range names { + ng := api.NewManagedNodeGroup() + ng.Name = name + ng.PropagateASGTags = &propagate + managedNodeGroups = append(managedNodeGroups, ng) + } + return managedNodeGroups +} diff --git a/pkg/eks/eks_test.go b/pkg/eks/eks_test.go index 49b6e25f71..b3a9327eed 100644 --- a/pkg/eks/eks_test.go +++ b/pkg/eks/eks_test.go @@ -218,13 +218,6 @@ var _ = Describe("EKS API wrapper", func() { }) }) - type managedNodesSupportCase struct { - platformVersion string - - expectError bool - supports bool - } - type platformVersionCase struct { platformVersion string expectedVersion int diff --git a/userdocs/src/usage/autoscaling.md b/userdocs/src/usage/autoscaling.md index 672172e291..fd9993ad21 100644 --- a/userdocs/src/usage/autoscaling.md +++ b/userdocs/src/usage/autoscaling.md @@ -44,7 +44,7 @@ nodeGroups: k8s.io/cluster-autoscaler/node-template/taint/feaster: "true:NoSchedule" ``` -For unmanaged noderoups, this is done by `eksctl` automatically if `propagateASGTags` is set to `true` like this: +For unmanaged and managed nodegroups, this is done by `eksctl` automatically if `propagateASGTags` is set to `true` like this: ```yaml nodeGroups: diff --git a/userdocs/src/usage/eks-managed-nodes.md b/userdocs/src/usage/eks-managed-nodes.md index f518432900..f0553e907b 100644 --- a/userdocs/src/usage/eks-managed-nodes.md +++ b/userdocs/src/usage/eks-managed-nodes.md @@ -246,8 +246,6 @@ eksctl scale nodegroup --name=managed-ng-1 --cluster=managed-cluster --nodes=4 - EKS Managed Nodegroups are managed by AWS EKS and do not offer the same level of configuration as unmanaged nodegroups. The unsupported options are noted below. -- Tags (managedNodeGroups[*].tags) in managed nodegroups apply to the EKS Nodegroup resource and to the EC2 instances launched as part of the nodegroup. -They do not propagate to the provisioned Autoscaling Group like in unmanaged nodegroups. - `iam.instanceProfileARN` is not supported for managed nodegroups. - The `amiFamily` field supports only `AmazonLinux2` - `instancesDistribution` field is not supported