Added tracking for deleted namespace status check in restore flow #8233

sangitaray2021 · 2024-09-20T08:40:59Z

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #8234

Please indicate you've done the following:

Accepted the DCO. Commits without the DCO will delay acceptance.
Created a changelog file (make new-changelog) or comment /kind changelog-not-required on this PR.
Updated the corresponding documentation in site/content/docs/main.

anshulahuja98 · 2024-09-20T09:38:15Z

For - #8234

codecov · 2024-09-20T09:44:21Z

Codecov Report

Attention: Patch coverage is 89.79592% with 5 lines in your changes missing coverage. Please review.

Project coverage is 58.98%. Comparing base (3f9c2dc) to head (6a5e8c2).
Report is 75 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/util/kube/resource_deletionstatus_tracker.go	76.19%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8233      +/-   ##
==========================================
- Coverage   59.15%   58.98%   -0.18%     
==========================================
  Files         367      368       +1     
  Lines       30777    38929    +8152     
==========================================
+ Hits        18206    22962    +4756     
- Misses      11113    14505    +3392     
- Partials     1458     1462       +4

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

kaovilai

Please add changelog and run linter.

reasonerjt · 2024-09-23T07:34:24Z

Given we've reached FC and this may impact the restore flow, let's hold this one until the branch is cut for v1.15

Signed-off-by: sangitaray2021 <sangitaray@microsoft.com> fixed unittest Signed-off-by: sangitaray2021 <sangitaray@microsoft.com> refactored tracker execution and caller Signed-off-by: sangitaray2021 <sangitaray@microsoft.com> added change log Signed-off-by: sangitaray2021 <sangitaray@microsoft.com> Author: sangitaray2021 <sangitaray@microsft.com> Author: sangitaray2021 <sangitaray@microsoft.com> Date: Thu Sep 19 02:26:14 2024 +0530 Signed-off-by: sangitaray2021 <sangitaray@microsoft.com>

Signed-off-by: sangitaray2021 <sangitaray@microsoft.com>

anshulahuja98 · 2024-09-26T09:03:50Z

@kaovilai can you help review the PR?

kaovilai

IIUC from reviewing, this change reduce potential duplicate await on if namespace exists (wait for 10 min polling) to just one instance per restore. Is that correct?

It's not meant to add polling until deletion is complete to resume some other process?

sangitaray2021 · 2024-09-30T03:20:49Z

IIUC from reviewing, this change reduce potential duplicate await on if namespace exists (wait for 10 min polling) to just one instance per restore. Is that correct?

It's not meant to add polling until deletion is complete to resume some other process?

yes thats correct. We are reducing duplicate wait by caching the namespace which is under deletion.

kaovilai · 2024-09-30T15:11:55Z

pkg/util/kube/namespace_deletionstatus_tracker.go

+type NamespaceDeletionStatusTracker interface {
+	// Add informs the tracker that a polling is in progress to check namespace deletion status.
+	Add(ns, name string)
+	// Delete informs the tracker that a namespace deletion is completed.
+	Delete(ns, name string)
+	// Contains returns true if the tracker is tracking the namespace deletion progress.
+	Contains(ns, name string) bool
+}


Why are there both ns and name parameters?
What purpose does that serve other than duplicate value params?

Is this interface meant to be more generic? Perhaps namespaceDeletionStatusTrackerKey(ns, name) was meant to be more like resourceDeletionStatusTrackerKey(kind, ns, name string) string ?

Signed-off-by: sangitaray2021 <sangitaray@microsoft.com>

kaovilai

lgtm

pkg/util/kube/resource_deletionstatus_tracker.go

pkg/util/kube/utils.go

Signed-off-by: sangitaray2021 <sangitaray@microsoft.com>

anshulahuja98 · 2024-11-08T09:37:43Z

@reasonerjt can you review this PR now given 1.15 is cut?

github-actions bot requested review from reasonerjt and ywk253100 September 20, 2024 08:41

github-actions bot added the has-unit-tests label Sep 20, 2024

github-actions bot assigned sangitaray2021 Sep 20, 2024

anshulahuja98 self-requested a review September 20, 2024 09:42

kaovilai suggested changes Sep 20, 2024

View reviewed changes

sangitaray2021 force-pushed the fixrestore_ns branch from d617a73 to e6a32e8 Compare September 23, 2024 05:32

github-actions bot added the has-changelog label Sep 23, 2024

reasonerjt added the defer-candidate label Sep 23, 2024

sangitaray2021 force-pushed the fixrestore_ns branch 3 times, most recently from 72d3b56 to c63dd9b Compare September 26, 2024 07:28

sangitaray2021 force-pushed the fixrestore_ns branch from c63dd9b to d147cba Compare September 26, 2024 08:36

fixed linter issuer

963e878

Signed-off-by: sangitaray2021 <sangitaray@microsoft.com>

kaovilai reviewed Sep 26, 2024

View reviewed changes

kaovilai reviewed Sep 30, 2024

View reviewed changes

incorporated PR comments

e069031

Signed-off-by: sangitaray2021 <sangitaray@microsoft.com>

kaovilai approved these changes Oct 30, 2024

View reviewed changes

pkg/util/kube/resource_deletionstatus_tracker.go Outdated Show resolved Hide resolved

anshulahuja98 reviewed Nov 1, 2024

View reviewed changes

pkg/util/kube/utils.go Outdated Show resolved Hide resolved

resolved comments

6a5e8c2

Signed-off-by: sangitaray2021 <sangitaray@microsoft.com>

anshulahuja98 approved these changes Nov 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added tracking for deleted namespace status check in restore flow #8233

Added tracking for deleted namespace status check in restore flow #8233

sangitaray2021 commented Sep 20, 2024 •

edited by anshulahuja98

Loading

anshulahuja98 commented Sep 20, 2024

codecov bot commented Sep 20, 2024 •

edited

Loading

kaovilai left a comment

reasonerjt commented Sep 23, 2024

anshulahuja98 commented Sep 26, 2024

kaovilai left a comment

sangitaray2021 commented Sep 30, 2024

kaovilai Sep 30, 2024 •

edited

Loading

kaovilai left a comment

anshulahuja98 commented Nov 8, 2024

Added tracking for deleted namespace status check in restore flow #8233

Are you sure you want to change the base?

Added tracking for deleted namespace status check in restore flow #8233

Conversation

sangitaray2021 commented Sep 20, 2024 • edited by anshulahuja98 Loading

Please add a summary of your change

Does your change fix a particular issue?

Please indicate you've done the following:

anshulahuja98 commented Sep 20, 2024

codecov bot commented Sep 20, 2024 • edited Loading

Codecov Report

kaovilai left a comment

Choose a reason for hiding this comment

reasonerjt commented Sep 23, 2024

anshulahuja98 commented Sep 26, 2024

kaovilai left a comment

Choose a reason for hiding this comment

sangitaray2021 commented Sep 30, 2024

kaovilai Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

kaovilai left a comment

Choose a reason for hiding this comment

anshulahuja98 commented Nov 8, 2024

sangitaray2021 commented Sep 20, 2024 •

edited by anshulahuja98

Loading

codecov bot commented Sep 20, 2024 •

edited

Loading

kaovilai Sep 30, 2024 •

edited

Loading