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: The execution of the terraformDestroy method fails and the appli… #366

Merged
merged 2 commits into from
Jun 1, 2023

Conversation

wujiuye
Copy link
Contributor

@wujiuye wujiuye commented May 26, 2023

…cation cannot be uninstalled(#363)

@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Patch coverage: 86.63% and project coverage change: -2.49 ⚠️

Comparison is base (3a96f68) 79.31% compared to head (e4e3813) 76.82%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #366      +/-   ##
==========================================
- Coverage   79.31%   76.82%   -2.49%     
==========================================
  Files          23       24       +1     
  Lines        1750     2054     +304     
==========================================
+ Hits         1388     1578     +190     
- Misses        278      379     +101     
- Partials       84       97      +13     
Flag Coverage Δ
e2e 0.00% <ø> (∅)
unit 79.61% <86.63%> (+0.30%) ⬆️

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

Impacted Files 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 78.49% <87.77%> (+1.13%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

…ation cannot be uninstalled kubevela#363

Signed-off-by: 吴就业 <wujiuye@lizhi.fm>
controllers/configuration_controller.go Show resolved Hide resolved
Comment on lines 1138 to 1141
if len(tfState.Outputs) == 0 {
return false
}
return true
Copy link
Member

Choose a reason for hiding this comment

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

return len(tfState.Outputs) > 0

Comment on lines 185 to 187
// fix: The failure of terraformApply does not mean that it has never been successful,
// but if the 'tfstate' file does not exist, then there is no need for terraformDestroy,
// which can quickly clean up resources.
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
// fix: The failure of terraformApply does not mean that it has never been successful,
// but if the 'tfstate' file does not exist, then there is no need for terraformDestroy,
// which can quickly clean up resources.
// If no tfState has been generated, then perform a quick cleanup without dispatching destroying job.
} else {
klog.Infof("No need to execute terraform destroy command, because tfstate file not found: %s/%s", configuration.Namespace, configuration.Name)
if err := r.cleanUpSubResources(ctx, configuration, meta); err != nil {
klog.Warningf("Failed to clean up sub-resources, but it's ignored as the resources are being forced to delete: %s", err)
Copy link
Member

Choose a reason for hiding this comment

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

No clue suggests the "configuration" is force to delete.

Suggested change
klog.Warningf("Failed to clean up sub-resources, but it's ignored as the resources are being forced to delete: %s", err)
klog.Warningf("Ignoring error when clean up sub-resources, for no resource is actually created: %s", err)
Signed-off-by: 吴就业 <wujiuye@lizhi.fm>
@chivalryq chivalryq merged commit d2e6f21 into kubevela:master Jun 1, 2023
8 of 9 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