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

Fix: Keep the backend secret when clean up SubResources if the deleteResou… #381

Conversation

imzhangsiwei
Copy link
Contributor

…rces is false.
#380

@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Patch coverage: 65.90% and project coverage change: -6.40% ⚠️

Comparison is base (3a96f68) 79.31% compared to head (2d33b98) 72.92%.
Report is 19 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
- Coverage   79.31%   72.92%   -6.40%     
==========================================
  Files          23       27       +4     
  Lines        1750     2068     +318     
==========================================
+ Hits         1388     1508     +120     
- Misses        278      471     +193     
- Partials       84       89       +5     
Flag Coverage Δ
e2e 0.00% <ø> (∅)
unit 75.51% <65.90%> (-3.81%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
controllers/configuration/configuration.go 92.22% <0.00%> (-3.19%) ⬇️
controllers/terraform/status.go 77.41% <ø> (ø)
e2e/normal/regression.go 0.00% <ø> (ø)
controllers/configuration_controller.go 66.91% <68.23%> (-10.45%) ⬇️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -1569,6 +1572,16 @@ func TestTerraformDestroy(t *testing.T) {
objects: []client.Object{readyProvider, configurationWithConnSecret, baseConfigurationCM, completeDestroyJob, baseVariableSecret, connectionSecret},
deletedResources: []client.Object{baseConfigurationCM, completeDestroyJob, baseVariableSecret, connectionSecret},
},
{
name: "destroy job has completes, cleanup resources but backend secret",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: "destroy job has completes, cleanup resources but backend secret",
name: "destroy job has completes, cleanup resources except backend secret",
},
want: want{},
objects: []client.Object{readyProvider, configurationWithConnSecret, baseConfigurationCM, completeDestroyJob, baseVariableSecret, connectionSecret},
deletedResources: []client.Object{baseConfigurationCM, completeDestroyJob, baseVariableSecret, connectionSecret},
Copy link
Member

Choose a reason for hiding this comment

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

Backend secret should be added to keptResources to test this PR. I think now this condition is not tested.

Copy link
Contributor Author

@imzhangsiwei imzhangsiwei Sep 22, 2023

Choose a reason for hiding this comment

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

I missed this test case about keptResources , and I will add it.

imzhangsiwei and others added 2 commits September 22, 2023 21:40
…rces is false.

Signed-off-by: imzhangsiwei <flysqrlboy@gmail.com>
…ce set to false

Signed-off-by: imzhangsiwei <flysqrlboy@gmail.com>
Copy link
Member

@chivalryq chivalryq left a comment

Choose a reason for hiding this comment

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

LGTM, Please sign the DCO.

@imzhangsiwei imzhangsiwei force-pushed the fix-keep-backendsecret-deleteResource-false branch from 8fb9882 to 2d33b98 Compare September 22, 2023 13:43
@chivalryq chivalryq merged commit 3536da5 into kubevela:master Sep 23, 2023
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants