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

Add retries to Resource Group deletion #74

Merged
merged 1 commit into from
May 21, 2024
Merged

Conversation

nirarg
Copy link
Contributor

@nirarg nirarg commented May 21, 2024

In some cases, the resource group can't be deleted immediately.
We need to wait until all resources inside it are completely destroyed.
Add retries with a 5-second delay, with a maximum of 40 retries.

In some cases, the resource group can't be deleted immediately.
We need to wait until all resources inside it are completely destroyed.
Add retries with a 5-second delay, with a maximum of 40 retries.
@prabinovRedhat
Copy link
Collaborator

lgtm

@@ -16,13 +16,12 @@
resource_group: "{{ azure_manage_resource_group_name }}"
with_items: "{{ result.locks }}"

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Nir,

The original wait_for 10 seconds was because it sometimes take a bit for a the lock to go away after deleting it right? I think it would be better to do the retry on a new lock_info task inside the above block. This way we wait for the lock to go away only when we actually delete the lock. If the below resource_group delete fails for another reason then the lock we end up retrying for a long time instead of just failing right away,

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the retries here in order to allow deleting the resource group in case of the following cases:

  1. Its take time for the lock to be actual removed
  2. Its takes time for resource inside the rg to be actual destroyed (related to the discussion we had here)

Solving the issue this way would solve also the same issue we might with other resources that might take longer time to delete.
In case user try to delete none empty resource group without indicating the "force" flag, it will wait 200 seconds, is it acceptable trade-off?

Any suggestion how to solve the second point, in case we use your suggestion for the first one?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation. I think your solution is a good compromise considering.

Copy link
Contributor

@p3ck p3ck left a comment

Choose a reason for hiding this comment

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

lgtm!

@nirarg nirarg merged commit d136cb7 into redhat-cop:main May 21, 2024
15 checks passed
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.

3 participants