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.

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

Allow StackCollection.DeleteStackByName() to succeed #2308

Merged
merged 1 commit into from Jun 9, 2020

Conversation

rndstr
Copy link
Contributor

@rndstr rndstr commented Jun 8, 2020

Fixes #2307

StackCollection.DeleteStackBySpec(*Stack) only deletes the stack if
its tags match the clustername. For that reason we not only need to pass
in the StackName and StackId but also Tags.

@@ -337,8 +337,7 @@ func defaultStackStatusFilter() []*string {

// DeleteStackByName sends a request to delete the stack
func (c *StackCollection) DeleteStackByName(name string) (*Stack, error) {
i := &Stack{StackName: &name}
s, err := c.DescribeStack(i)
i, err := c.DescribeStack(&Stack{StackName: &name})
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we rename i to stack?

Copy link
Contributor Author

@rndstr rndstr Jun 9, 2020

Choose a reason for hiding this comment

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

thanks, it should be s to be consistent with other usages throughout eksctl.

(eksctl seems to use i to describe something like "input stack to perform action on". for actions that return a stack it is usually assigned to s which would denote a complete stack object having all the fields filled out.)

Copy link
Contributor

@martina-if martina-if left a comment

Choose a reason for hiding this comment

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

Good catch 👍 I only have one nitpick comment but it's approved

Copy link
Collaborator

@cPu1 cPu1 left a comment

Choose a reason for hiding this comment

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

Good catch! ✨

`StackCollection.DeleteStackBySpec(*Stack)` only deletes the stack if
its tags match the clustername. For that reason we not only need to pass
in the `StackName` and `StackId` but also `Tags`.
@rndstr rndstr merged commit 043fff5 into master Jun 9, 2020
@rndstr rndstr deleted the fix-delete-stack-by-name branch June 9, 2020 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StackCollection.DeleteStackByName() cannot succeed
3 participants