eksctl icon indicating copy to clipboard operation
eksctl copied to clipboard

Unify StackManager Delete funcs

Open vflaux opened this issue 2 years ago • 2 comments

Description

Unifies StackManager funcs DeleteStackBySpec, DeleteStackBySpecSync & DeleteStackSync into one DeleteStack func (#5451).

type DeleteStackOptions struct {
	Stack *Stack
	Wait  bool
	ErrCh chan error
}

// DeleteStack sends a request to delete the stack.
// If Wait is true, it waits until status is DELETE_COMPLETE;
// If ErrCh is given, any errors will be written to it, assume completion when nil is written,
// do not expect more than one error value on the channel, it's closed immediately after it is written to.
// Wait and ErrCh are mutually exclusive
DeleteStack(ctx context.Context, options DeleteStackOptions) error

Changes taskWithStackSpec & asynctaskWithStackSpec into deleteStackTask with a wait option. Also add a context parameter to NewTasksToDeleteNodeGroups func.

Checklist

  • [x] Added tests that cover your change (if possible)
  • [ ] Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • [x] Manually tested
  • [ ] Made sure the title of the PR is a good description that can go into the release notes
  • [x] (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! :exploding_head:

  • [ ] Backfilled missing tests for code in same general area :tada:
  • [x] Refactored something and made the world a better place :star2:

vflaux avatar Jul 04 '22 15:07 vflaux

@vflaux - beautiful piece of refactoring, thanks a lot for opening this PR ✨ !! From a code's logic perspective, LGTM. I've generally left minor stylistic comments. Could you perhaps provide some relevant outputs from your manual testing?

TiberiuGC avatar Aug 25 '22 09:08 TiberiuGC

I tested manually with a delete nodegroup on an unamanged nodegroup.

New:

$ ./eksct version
0.110.0-dev+22f59427b.2022-08-25T17:47:57Z
$ ./eksctl --cluster foo delete nodegroup --name ng-1
2022-08-25 17:49:22 [ℹ]  1 nodegroup (ng-1) was included (based on the include/exclude rules)
2022-08-25 17:49:28 [ℹ]  will drain 1 nodegroup(s) in cluster "foo"
2022-08-25 17:49:28 [ℹ]  starting parallel draining, max in-flight of 1
2022-08-25 17:49:28 [!]  no nodes found in nodegroup "ng-1" (label selector: "alpha.eksctl.io/nodegroup-name=ng-1")
2022-08-25 17:49:28 [ℹ]  will delete 1 nodegroups from cluster "foo"
2022-08-25 17:49:37 [ℹ]  1 task: { 1 task: { delete nodegroup "ng-1" [async] } }
2022-08-25 17:49:37 [ℹ]  will delete stack "eksctl-foo-nodegroup-ng-1"
2022-08-25 17:49:37 [ℹ]  will delete 1 nodegroups from auth ConfigMap in cluster "foo"
2022-08-25 17:49:37 [ℹ]  removing identity "arn:aws:iam::123456789012:role/eksctl-foo-NodeInstanceRole-ABCDEFGHIJKLM" from auth ConfigMap (username = "system:node:{{EC2PrivateDNSName}}", groups = ["system:bootstrappers" "system:nodes"])
2022-08-25 17:49:37 [✔]  deleted 1 nodegroup(s) from cluster "foo"

Current:

$ eksctl version
0.109.0
$ eksctl --cluster foo delete nodegroup --name ng-1
2022-08-25 18:09:49 [ℹ]  1 nodegroup (ng-1) was included (based on the include/exclude rules)
2022-08-25 18:09:53 [ℹ]  will drain 1 nodegroup(s) in cluster "foo"
2022-08-25 18:09:53 [ℹ]  starting parallel draining, max in-flight of 1
2022-08-25 18:09:53 [!]  no nodes found in nodegroup "ng-1" (label selector: "alpha.eksctl.io/nodegroup-name=ng-1")
2022-08-25 18:09:53 [ℹ]  will delete 1 nodegroups from cluster "foo"
2022-08-25 18:10:03 [ℹ]  1 task: { 1 task: { delete nodegroup "ng-1" [async] } }
2022-08-25 18:10:03 [ℹ]  will delete stack "eksctl-foo-nodegroup-ng-1"
2022-08-25 18:10:03 [ℹ]  will delete 1 nodegroups from auth ConfigMap in cluster "foo"
2022-08-25 18:10:03 [ℹ]  removing identity "arn:aws:iam::123456789012:role/eksctl-foo-NodeInstanceRole-ABCDEFGHIJKLM" from auth ConfigMap (username = "system:node:{{EC2PrivateDNSName}}", groups = ["system:bootstrappers" "system:nodes"])
2022-08-25 18:10:04 [✔]  deleted 1 nodegroup(s) from cluster "foo"

vflaux avatar Aug 25 '22 16:08 vflaux