refactor: enhance namespace destruction logic in localDestroyAllCommand #4794
Conversation
- Introduced a new constant for the ticker duration during the destruction process. - Improved the waitForNamespaceDestroyAllToComplete method to ensure comprehensive checks for resource destruction. - Added a new checkAllResourcesDestroyed method to verify that all resources, including configmaps, are properly removed after the destruction operation. This refactor aims to enhance the reliability and clarity of the namespace destruction process. Signed-off-by: Javier Lopez <[email protected]>
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #4794 +/- ##
==========================================
- Coverage 48.60% 48.59% -0.02%
==========================================
Files 364 364
Lines 30428 30438 +10
==========================================
Hits 14791 14791
- Misses 14461 14471 +10
Partials 1176 1176 🚀 New features to boost your workflow:
|
cmd/destroy/all.go
Outdated
| for _, cfg := range cfgList.Items { | ||
| for l := range cfg.GetLabels() { | ||
| if _, ok := resourcesLabels[l]; ok { | ||
| return fmt.Errorf("namespace destroy all failed: some resources where not destroyed") |
There was a problem hiding this comment.
I think we should change the logs then. Before, this was running when the destroy operation in theory finished. So, when we found something, that would mean that something wasn't deleted. Now, this can be executed while the job is still being executed, or at least, if an error happens, we keeps waiting in the main loop
cmd/destroy/all.go
Outdated
| return nil | ||
| if err := lda.checkAllResourcesDestroyed(ctx, namespace, c); err != nil { | ||
| oktetoLog.Infof("namespace destroy all failed: %v", err) | ||
| continue |
There was a problem hiding this comment.
Why do we keep within the loop in case of error?
There was a problem hiding this comment.
The case that after 10s the action has not been created but there are still resources destroying
…llCommand - Updated log message in waitForNamespaceDestroyAllToComplete method to clarify the error context when checking for destroyed resources. This change enhances the clarity of log outputs during the namespace destruction process. Signed-off-by: Javier Lopez <[email protected]>
cmd/destroy/all.go
Outdated
| for _, cfg := range cfgList.Items { | ||
| for l := range cfg.GetLabels() { | ||
| if _, ok := resourcesLabels[l]; ok { | ||
| return fmt.Errorf("namespace destroy all failed: some resources where not destroyed") |
There was a problem hiding this comment.
| return fmt.Errorf("namespace destroy all failed: some resources where not destroyed") | |
| return fmt.Errorf("namespace destroy all failed: some resources were not destroyed") |
Signed-off-by: Javier Lopez <[email protected]>
ifbyol
left a comment
There was a problem hiding this comment.
There is still one case where this might get stuck. If we don't detect the transition to DestroyingAll, the job finishes and some resource is still in the namespaces (because something failed), the command would be stuck til timeout.
I think it is fine, as it shouldn't be a common scenario, and it would require broader changes, so for now, this is fine
cmd/destroy/all.go
Outdated
| for _, cfg := range cfgList.Items { | ||
| for l := range cfg.GetLabels() { | ||
| if _, ok := resourcesLabels[l]; ok { | ||
| return fmt.Errorf("some resources where not destroyed") |
There was a problem hiding this comment.
| return fmt.Errorf("some resources where not destroyed") | |
| return fmt.Errorf("some resources were not destroyed") |
- Updated the error message in checkAllResourcesDestroyed method to fix the typo "where" to "were" for improved clarity. Signed-off-by: Javier Lopez <[email protected]>
Proposed changes
Fixes DEV-1046
How to validate
okteto destroy --allseveral times and check that it always finishesCLI Quality Reminders 🔧
For both authors and reviewers: