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

feat: AKS: remove resource group on clean #283

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mook-as
Copy link
Member

@mook-as mook-as commented Sep 15, 2020

We create a resource group in AKS on deployment; for mirroring, we should also remove it when cleaning up.

We create a resource group in AKS on deployment; for mirroring, we
should also remove it when cleaning up.
@mook-as mook-as added the type: enhancement New feature or request label Sep 15, 2020
@mook-as mook-as requested a review from viccuad September 15, 2020 17:39
Copy link
Member

@viccuad viccuad left a comment

Choose a reason for hiding this comment

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

I'm not sure we need to remove the resource group.

From cap-terraform's readme, the resource group is a precondition for terraform, and it is personal:
https://github.com/SUSE/cap-terraform/blob/8831476c6c81758c54c162d780c5fcf039514b58/aks/README.md

This resource group will contain all resources of, and in support of your SUSE CAP cluster. Make note of the name and location which must be supplied in terraform variables.

And it seems that terraform doesn't create it either (following var https://github.com/SUSE/cap-terraform/blob/8831476c6c81758c54c162d780c5fcf039514b58/aks/variables.tf.json#L19).

So I see it as auth setup for the user, and in that case I think we should let the user decide when and how the resource group will be deleted, as it can be reused between deployments.

@mook-as
Copy link
Member Author

mook-as commented Sep 16, 2020

The problem was that, if we don't remove the resource group, the next deployment fails for me (because it already exists).

I'll try again later to get the exact error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants