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

gha: delete azure RG only if it exists #9007

Merged
merged 1 commit into from Feb 4, 2024

Conversation

wainersm
Copy link
Contributor

@wainersm wainersm commented Feb 2, 2024

delete_cluster() has tried to delete the az resources group regardless if it exists. In some cases the result of that operation is ignored, i.e., fail to resource group not found, but the log messages get a little dirty. Let's delete the RG only if it exists then.

Fixes #8989
Signed-off-by: Wainer dos Santos Moschetta wainersm@redhat.com


Split of #8990

@wainersm wainersm added no-backport-needed area/ci Issues affecting the continuous integration labels Feb 2, 2024
@wainersm wainersm requested a review from sprt February 2, 2024 13:50
@katacontainersbot katacontainersbot added the size/small Small and simple task label Feb 2, 2024
Copy link
Contributor

@sprt sprt left a comment

Choose a reason for hiding this comment

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

LGTM, thank you Wainer! Left a couple nits but happy to merge as is. 🙂

local rg
rg="$(_print_rg_name ${test_type})"

if [ "$(az group exists -n "${rg}")" == "true" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if [ "$(az group exists -n "${rg}")" == "true" ]; then
if [ "$(az group exists -g "${rg}")" == "true" ]; then

If we want to be consistent with other commands.

Comment on lines 111 to 113
az group delete \
-g "${rg}" \
--yes
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
az group delete \
-g "${rg}" \
--yes
az group delete -g "${rg}" --yes

Tiny bit more readable.

@sprt sprt added the ok-to-test label Feb 2, 2024
delete_cluster() has tried to delete the az resources group regardless
if it exists. In some cases the result of that operation is ignored,
i.e., fail to resource group not found, but the log messages get a
little dirty. Let's delete the RG only if it exists then.

Fixes kata-containers#8989
Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
@katacontainersbot katacontainersbot added size/tiny Smallest and simplest task and removed size/small Small and simple task labels Feb 2, 2024
@wainersm
Copy link
Contributor Author

wainersm commented Feb 2, 2024

Just updated to address @sprt 's suggestions.

@justxuewei
Copy link
Member

/test

Copy link
Member

@justxuewei justxuewei left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks!

@justxuewei justxuewei merged commit fa01a86 into kata-containers:main Feb 4, 2024
291 of 299 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Issues affecting the continuous integration ok-to-test size/tiny Smallest and simplest task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: azure aks deploy sometimes fail to find the resources group
5 participants