Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Add logic for clusterboostrap webhook to handle deleting cluster #3932

Merged
merged 2 commits into from
Nov 17, 2022

Conversation

adduarte
Copy link

@adduarte adduarte commented Nov 16, 2022

When a cluster is being deleted is possible that some of the objects the clusterbootstrap webhook is looking for might be deleted by the time the webhook checks for them.
In that case the validation should not fail because the cluster is being deleted.

fixes: #3931

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes # 3931

Describe testing done for PR

Release note

Clusterbootstrap validation is handled different when cluster is marked by deletion. 
Deleted resources do not fail validation

Additional information

Special notes for your reviewer

When a cluster is being deleted is possible that some of the
objects the clusterbootstrap webhook is looking for might be deleted
by the time the webhook checks for them.
In that case the validation should not fail because the cluster is
being deleted.

fixes: 3931
@codecov
Copy link

codecov bot commented Nov 17, 2022

Codecov Report

Merging #3932 (6ab9666) into main (7f8c8f7) will decrease coverage by 0.45%.
The diff coverage is 53.57%.

@@            Coverage Diff             @@
##             main    #3932      +/-   ##
==========================================
- Coverage   46.95%   46.50%   -0.46%     
==========================================
  Files         403      445      +42     
  Lines       40314    43259    +2945     
==========================================
+ Hits        18929    20117    +1188     
- Misses      19628    21214    +1586     
- Partials     1757     1928     +171     
Impacted Files Coverage Δ
addons/pkg/util/cluster_util.go 4.28% <0.00%> (-0.07%) ⬇️
tkg/client/management_components.go 24.82% <40.00%> (+12.86%) ⬆️
addons/webhooks/clusterbootstrap_webhook.go 27.96% <82.35%> (+3.93%) ⬆️
tkg/managementcomponents/helper.go 94.95% <100.00%> (ø)
cli/runtime/config/lock.go 51.35% <0.00%> (-8.11%) ⬇️
cli/runtime/config/conversion.go 69.27% <0.00%> (-6.92%) ⬇️
tkg/client/upgrade_region.go 24.68% <0.00%> (-0.51%) ⬇️
tkg/client/init.go 0.00% <0.00%> (ø)
cli/runtime/plugin/root.go 100.00% <0.00%> (ø)
tkg/client/delete_region.go 0.00% <0.00%> (ø)
... and 54 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@maralavi
Copy link
Contributor

@adduarte Thanks for the PR. Please add release note:)

@shyaamsn
Copy link
Contributor

@adduarte Could you please check if its possible to add unit test for this ? Thanks

@adduarte adduarte added the ok-to-merge PRs should be labelled with this before merging label Nov 17, 2022
@adduarte adduarte merged commit 8758769 into main Nov 17, 2022
@adduarte adduarte deleted the topic/adduarte/cb_wh_handle_deleting_cluster branch November 17, 2022 23:36
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clusterbootstap webhook does not handle cluster deletion correctly
4 participants